From 695a1b76d0e14b1093f93757232efa5c2818d0da Mon Sep 17 00:00:00 2001 From: Mitul Shah Date: Mon, 31 Aug 2026 11:34:29 -0400 Subject: [PATCH 1/8] fix(web-shared): describe map-like iterables via entries() (#3806) * fix(web-shared): expand Web API iterables Signed-off-by: Cursor Agent * fix(web-shared): inspect generic iterables Signed-off-by: Cursor Agent * fix(web-shared): compare iterator identity safely Signed-off-by: Cursor Agent * test(web-shared): cover inspector iterable entries Signed-off-by: Cursor Agent * fix(web-shared): describe map-like iterables via entries() Signed-off-by: Cursor Agent --------- Signed-off-by: Cursor Agent Co-authored-by: Cursor Agent --- .changeset/tidy-rivers-expand.md | 5 + .../src/components/ui/data-inspector.tsx | 114 ++++++++++- .../web-shared/test/data-inspector.test.ts | 186 ++++++++++++++++++ 3 files changed, 296 insertions(+), 9 deletions(-) create mode 100644 .changeset/tidy-rivers-expand.md create mode 100644 packages/web-shared/test/data-inspector.test.ts diff --git a/.changeset/tidy-rivers-expand.md b/.changeset/tidy-rivers-expand.md new file mode 100644 index 0000000000..a90b9a796f --- /dev/null +++ b/.changeset/tidy-rivers-expand.md @@ -0,0 +1,5 @@ +--- +'@workflow/web-shared': patch +--- + +Render Headers, URLSearchParams, and other iterables in the data inspector. Map-like values use `entries()`; other iterables render as lists. diff --git a/packages/web-shared/src/components/ui/data-inspector.tsx b/packages/web-shared/src/components/ui/data-inspector.tsx index b2da375fe7..95877cc6c8 100644 --- a/packages/web-shared/src/components/ui/data-inspector.tsx +++ b/packages/web-shared/src/components/ui/data-inspector.tsx @@ -344,7 +344,7 @@ function BytesDisplayValue({ display }: { display: BytesDisplay }) { // Tree renderer // --------------------------------------------------------------------------- -type Entry = [field: string | undefined, value: unknown]; +type Entry = [field: string | undefined, value: unknown, key?: string | number]; interface NodeContext { level: number; @@ -357,10 +357,51 @@ function formatField(field: string): string { return field === '' ? '""' : field; } +function isGenericIterable( + value: unknown +): value is object & Iterable { + if ( + value === null || + (typeof value !== 'object' && typeof value !== 'function') || + Array.isArray(value) || + value instanceof Map || + value instanceof Set + ) { + return false; + } + return ( + typeof (value as { [Symbol.iterator]?: unknown })[Symbol.iterator] === + 'function' + ); +} + +function isEntryIterable( + value: object & Iterable +): value is object & Iterable & { entries(): Iterable } { + return typeof (value as { entries?: unknown }).entries === 'function'; +} + +function collectEntries( + iterable: Iterable, + asPairs: boolean +): Entry[] { + return Array.from(iterable, (item, index) => { + if (asPairs && Array.isArray(item) && item.length >= 2) { + return [String(item[0]), collapseRefs(item[1]), index]; + } + return [undefined, collapseRefs(item), index]; + }); +} + +function isSelfIterableIterator(value: object & Iterable): boolean { + return Object.is(value[Symbol.iterator](), value); +} + /** - * Describe an object/array/map/set as an expandable container. Returns null for - * values that should render as a primitive. `prefix` carries a class name shown - * before the opening bracket (Map/Set and named class instances). + * Describe an object/array/iterable as an expandable container. Returns null + * for values that should render as a primitive. `prefix` carries a class name + * shown before the opening bracket (Map/Set, generic iterables, and named class + * instances). */ function describeContainer( value: unknown @@ -391,6 +432,25 @@ function describeContainer( prefix: 'Set', }; } + if (isGenericIterable(value)) { + const name = (value as { constructor?: { name?: string } }).constructor + ?.name; + const prefix = name && name !== 'Object' ? name : undefined; + if (isEntryIterable(value)) { + return { + entries: collectEntries(value.entries(), true), + open: '{', + close: '}', + prefix, + }; + } + return { + entries: collectEntries(value, false), + open: '[', + close: ']', + prefix, + }; + } if (value !== null && typeof value === 'object') { const name = (value as { constructor?: { name?: string } }).constructor ?.name; @@ -616,9 +676,9 @@ function ExpandableContainer({ {expanded ? ( // biome-ignore lint/a11y/useSemanticElements: ARIA tree group is the correct role here
    - {entries.map(([childField, childValue], index) => ( + {entries.map(([childField, childValue, entryKey], index) => ( , + b: Iterable, + seen: WeakMap +): boolean { + const aIterator = a[Symbol.iterator](); + const bIterator = b[Symbol.iterator](); + while (true) { + const aResult = aIterator.next(); + const bResult = bIterator.next(); + if (aResult.done || bResult.done) { + return aResult.done === bResult.done; + } + if (!isDeepEqual(aResult.value, bResult.value, seen)) return false; + } +} + +export function isDeepEqual( + a: unknown, + b: unknown, + seen = new WeakMap() +): boolean { if (Object.is(a, b)) return true; if (isBytesDisplay(a) || isBytesDisplay(b)) { @@ -977,6 +1058,21 @@ function isDeepEqual(a: unknown, b: unknown, seen = new WeakMap()): boolean { return a.source === b.source && a.flags === b.flags; } + if (isGenericIterable(a) || isGenericIterable(b)) { + if (!isGenericIterable(a) || !isGenericIterable(b)) return false; + if (Object.getPrototypeOf(a) !== Object.getPrototypeOf(b)) return false; + if (isSelfIterableIterator(a) || isSelfIterableIterator(b)) return false; + if (seen.get(a) === b) return true; + seen.set(a, b); + const aHasEntries = isEntryIterable(a); + const bHasEntries = isEntryIterable(b); + if (aHasEntries !== bHasEntries) return false; + if (aHasEntries && bHasEntries) { + return haveSameIterableValues(a.entries(), b.entries(), seen); + } + return haveSameIterableValues(a, b, seen); + } + if (a instanceof Map && b instanceof Map) { if (a.size !== b.size) return false; for (const [key, value] of a.entries()) { diff --git a/packages/web-shared/test/data-inspector.test.ts b/packages/web-shared/test/data-inspector.test.ts new file mode 100644 index 0000000000..8086b1d859 --- /dev/null +++ b/packages/web-shared/test/data-inspector.test.ts @@ -0,0 +1,186 @@ +import { createElement } from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { describe, expect, it } from 'vitest'; +import { + DataInspector, + isDeepEqual, +} from '../src/components/ui/data-inspector.js'; +import { getWebRevivers } from '../src/lib/hydration.js'; + +const REVIVERS = getWebRevivers(); + +class TestIterable implements Iterable { + constructor(private readonly items: string[]) {} + + *[Symbol.iterator]() { + yield* this.items; + } +} + +class CoordinatePairs implements Iterable<[number, number]> { + *[Symbol.iterator]() { + yield [1, 2]; + yield [3, 4]; + } +} + +class MapLike { + *[Symbol.iterator]() { + yield 'wrong'; + } + + *entries() { + yield ['color', 'blue']; + } +} + +function hydrateHeaders(entries: [string, string][]): Headers { + return REVIVERS.Headers(entries) as Headers; +} + +function hydrateSearchParams(value: string): URLSearchParams { + return REVIVERS.URLSearchParams(value) as URLSearchParams; +} + +function hydrateRequest(headers: [string, string][]): object { + return REVIVERS.Request({ + method: 'GET', + url: 'https://example.com', + headers, + body: null, + }) as object; +} + +function render(data: unknown, expandLevel = 1): string { + return visibleText( + renderToStaticMarkup(createElement(DataInspector, { data, expandLevel })) + ); +} + +function visibleText(markup: string): string { + return markup + .replace(//g, '') + .replace(/<[^>]+>/g, '') + .replace(/"/g, '"') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/&/g, '&'); +} + +describe('DataInspector iterables', () => { + it('renders hydrated Headers as an expandable container', () => { + const markup = renderToStaticMarkup( + createElement(DataInspector, { + data: hydrateHeaders([['content-type', 'application/json']]), + expandLevel: 0, + }) + ); + + expect(markup).toContain('data-json-expander'); + expect(visibleText(markup)).toContain('Headers'); + expect(visibleText(markup)).not.toContain('application/json'); + }); + + it('expands hydrated Headers to render their entries', () => { + const text = render(hydrateHeaders([['content-type', 'application/json']])); + + expect(text).toContain('content-type:'); + expect(text).toContain('"application/json"'); + }); + + it('renders nested Headers on a hydrated Request', () => { + const text = render( + hydrateRequest([['content-type', 'application/json']]), + 2 + ); + + expect(text).toContain('Request'); + expect(text).toContain('headers:'); + expect(text).toContain('Headers'); + expect(text).toContain('content-type:'); + expect(text).toContain('"application/json"'); + }); + + it('renders and expands hydrated URLSearchParams entries', () => { + const text = render(hydrateSearchParams('page=2&sort=created')); + + expect(text).toContain('URLSearchParams'); + expect(text).toContain('page:'); + expect(text).toContain('"2"'); + expect(text).toContain('sort:'); + expect(text).toContain('"created"'); + }); + + it('keeps duplicate URLSearchParams keys', () => { + const text = render(hydrateSearchParams('tag=a&tag=b')); + + expect(text).toContain('tag:'); + expect(text).toContain('"a"'); + expect(text).toContain('"b"'); + }); + + it('expands custom Symbol.iterator implementations as a list', () => { + const text = render(new TestIterable(['first', 'second'])); + + expect(text).toContain('TestIterable'); + expect(text).toContain('"first"'); + expect(text).toContain('"second"'); + expect(text).not.toContain('0:'); + }); + + it('does not treat 2-element yields as fields', () => { + const text = render(new CoordinatePairs(), 2); + + expect(text).not.toContain('1:'); + expect(text).not.toContain('3:'); + expect(text).toContain('1'); + expect(text).toContain('2'); + expect(text).toContain('3'); + expect(text).toContain('4'); + }); + + it('renders map-like values from entries() rather than the default iterator', () => { + const text = render(new MapLike()); + + expect(text).toContain('MapLike'); + expect(text).toContain('color:'); + expect(text).toContain('"blue"'); + expect(text).not.toContain('"wrong"'); + }); + + it('treats Headers with different entries as unequal', () => { + expect( + isDeepEqual( + hydrateHeaders([['x-version', 'one']]), + hydrateHeaders([['x-version', 'two']]) + ) + ).toBe(false); + }); + + it('treats Headers with the same entries as equal', () => { + expect( + isDeepEqual( + hydrateHeaders([['x-version', 'one']]), + hydrateHeaders([['x-version', 'one']]) + ) + ).toBe(true); + }); + + it('treats URLSearchParams with different entries as unequal', () => { + expect( + isDeepEqual(hydrateSearchParams('page=1'), hydrateSearchParams('page=2')) + ).toBe(false); + }); + + it('treats generic iterables with different values as unequal', () => { + expect( + isDeepEqual(new TestIterable(['one']), new TestIterable(['two'])) + ).toBe(false); + }); + + it('treats generic iterables with the same values as equal', () => { + expect( + isDeepEqual(new TestIterable(['one']), new TestIterable(['one'])) + ).toBe(true); + }); +}); From aaeb8e44633c1974f087ee7ce38666dbc8ad8a4d Mon Sep 17 00:00:00 2001 From: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:44:07 -0700 Subject: [PATCH 2/8] [world-vercel] Classify incomplete replay streams as transport failures (#3546) * Classify replay stream failures by ownership Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> * fix(world-vercel): resume truncated replay streams * fix(world-vercel): require replay continuation cursor * refactor(world-vercel): simplify replay recovery * fix(world-vercel): append replay pages safely * fix(world-vercel): bound partial stream continuations * fix(world-vercel): bound replay continuations * fix(world-vercel): retain recovered replay cursors * fix(world-vercel): validate each recovery cursor * test(world-vercel): include list response cursors * refactor(world-vercel): simplify replay recovery * Update packages/world-vercel/src/events-v4.ts Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> --------- Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> --- .changeset/typed-replay-stream-failures.md | 5 + packages/world-vercel/src/events-v4.test.ts | 387 +++++++++++++++++--- packages/world-vercel/src/events-v4.ts | 244 ++++++++---- packages/world-vercel/src/events.test.ts | 121 ++++-- packages/world-vercel/src/events.ts | 10 +- packages/world-vercel/src/frames.ts | 18 +- 6 files changed, 619 insertions(+), 166 deletions(-) create mode 100644 .changeset/typed-replay-stream-failures.md diff --git a/.changeset/typed-replay-stream-failures.md b/.changeset/typed-replay-stream-failures.md new file mode 100644 index 0000000000..3bcec935c4 --- /dev/null +++ b/.changeset/typed-replay-stream-failures.md @@ -0,0 +1,5 @@ +--- +'@workflow/world-vercel': patch +--- + +Classify malformed replay responses as typed world failures and resume incomplete streams from their last validated event. diff --git a/packages/world-vercel/src/events-v4.test.ts b/packages/world-vercel/src/events-v4.test.ts index bf21528397..5532638a14 100644 --- a/packages/world-vercel/src/events-v4.test.ts +++ b/packages/world-vercel/src/events-v4.test.ts @@ -456,7 +456,10 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { {}, { token: 'test-token', dispatcher: agent } ) - ).rejects.toThrow(); + ).rejects.toMatchObject({ + name: 'WorkflowWorldError', + code: 'SCHEMA_VALIDATION', + }); agent.assertNoPendingInterceptors(); }); @@ -536,53 +539,13 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { {}, { token: 'test-token', dispatcher: agent } ) - ).rejects.toThrow(); - }); - - it('throws when the stream ends without the end sentinel (truncated response)', async () => { - const origin = - WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; - const agent = new MockAgent(); - agent.disableNetConnect(); - - // A complete event frame but NO `{_end: 1}` sentinel — what a response - // truncated on a frame boundary looks like. Returning this as a - // successful page would silently drop events with hasMore=false. - const frames = encodeFrame( - { - eventId: 'evnt_1', - runId: 'wrun_1', - eventType: 'run_created', - createdAt: '2026-06-10T00:00:00.000Z', - eventData: { - deploymentId: 'dpl_1', - workflowName: 'workflow', - input: null, - }, - }, - new Uint8Array(0) - ); - - agent - .get(origin) - .intercept({ - path: '/api/v4/runs/wrun_1/events?limit=500', - method: 'GET', - }) - .reply(200, frames, { - headers: { 'content-type': V4_FRAME_CONTENT_TYPE }, - }); - - await expect( - getWorkflowRunEventsV4( - 'wrun_1', - { limit: 500 }, - { token: 'test-token', dispatcher: agent } - ) - ).rejects.toThrow(/end-of-stream sentinel/); + ).rejects.toMatchObject({ + name: 'WorkflowWorldError', + code: 'SCHEMA_VALIDATION', + }); }); - it('resumes a truncated full stream after its last accepted event', async () => { + it('resumes a truncated full stream after its last complete event', async () => { const origin = WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; const agent = new MockAgent(); @@ -620,23 +583,26 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { }) .reply( 200, - Buffer.concat([ - encodeFrame( - { - eventId: 'evnt_2', - runId: 'wrun_1', - eventType: 'run_started', - createdAt: CREATED_AT, - }, - new Uint8Array() - ), - encodeFrame( - { _end: 1, next: 'eid:evnt_2', hasMore: false }, - new Uint8Array() - ), - ]), + encodeFrame( + { + eventId: 'evnt_2', + runId: 'wrun_1', + eventType: 'run_started', + createdAt: CREATED_AT, + }, + new Uint8Array() + ), { headers: { 'content-type': V4_FRAME_CONTENT_TYPE } } ); + agent + .get(origin) + .intercept({ + path: '/api/v4/runs/wrun_1/events?returnAll=true&cursor=eid%3Aevnt_2', + method: 'GET', + }) + .reply(200, encodeFrame({ _end: 1, hasMore: false }, new Uint8Array()), { + headers: { 'content-type': V4_FRAME_CONTENT_TYPE }, + }); const result = await getWorkflowRunEventsV4( 'wrun_1', @@ -652,6 +618,94 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { expect(result.hasMore).toBe(false); agent.assertNoPendingInterceptors(); }); + + it('limits truncated full-stream recovery to three continuations', async () => { + const origin = + WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; + const agent = new MockAgent(); + agent.disableNetConnect(); + + for (const [cursor, eventId] of [ + [undefined, 'evnt_1'], + ['eid:evnt_1', 'evnt_2'], + ['eid:evnt_2', 'evnt_3'], + ] as const) { + agent + .get(origin) + .intercept({ + path: + '/api/v4/runs/wrun_1/events?returnAll=true' + + (cursor ? `&cursor=${encodeURIComponent(cursor)}` : ''), + method: 'GET', + }) + .reply( + 200, + encodeFrame( + { + eventId, + runId: 'wrun_1', + eventType: 'run_started', + createdAt: CREATED_AT, + }, + new Uint8Array() + ), + { headers: { 'content-type': V4_FRAME_CONTENT_TYPE } } + ); + } + + await expect( + getWorkflowRunEventsV4( + 'wrun_1', + {}, + { token: 'test-token', dispatcher: agent } + ) + ).rejects.toThrow( + 'frame stream ended without the end-of-stream sentinel (1 events read)' + ); + agent.assertNoPendingInterceptors(); + }); + + it('surfaces a truncated stream that provides no recovery cursor', async () => { + const origin = + WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; + const agent = new MockAgent(); + agent.disableNetConnect(); + const completeFrame = encodeFrame( + { + eventId: 'evnt_1', + runId: 'wrun_1', + eventType: 'run_created', + createdAt: CREATED_AT, + eventData: { + deploymentId: 'dpl_1', + workflowName: 'workflow', + input: null, + }, + }, + new Uint8Array() + ); + + agent + .get(origin) + .intercept({ + path: '/api/v4/runs/wrun_1/events?returnAll=true', + method: 'GET', + }) + .reply(200, completeFrame.slice(0, -1), { + headers: { 'content-type': V4_FRAME_CONTENT_TYPE }, + }); + await expect( + getWorkflowRunEventsV4( + 'wrun_1', + {}, + { token: 'test-token', dispatcher: agent } + ) + ).rejects.toMatchObject({ + name: 'WorkflowWorldError', + code: 'TRANSPORT', + }); + agent.assertNoPendingInterceptors(); + }); }); /** @@ -861,6 +915,50 @@ describe('v4 transport uses global fetch (observability)', () => { }); describe('createWorkflowRunEventV4 over HTTP', () => { + it.each([ + ['an empty body', () => new Response(), 'PARSE_ERROR'], + [ + 'malformed CBOR', + () => new Response(new Uint8Array([0xff, 0xfe, 0xfd])), + 'PARSE_ERROR', + ], + [ + 'a body read failure', + () => + new Response( + new ReadableStream({ + start(controller) { + controller.error(new Error('socket closed')); + }, + }) + ), + 'TRANSPORT', + ], + ])('classifies %s', async (_case, response, code) => { + const fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockResolvedValueOnce(response()); + + try { + await expect( + createWorkflowRunEventV4( + { + runId: 'wrun_1', + eventType: 'step_completed', + specVersion: 2, + correlationId: 'step_1', + }, + { token: 'test-token' } + ) + ).rejects.toMatchObject({ + name: 'WorkflowWorldError', + code, + }); + } finally { + fetchSpy.mockRestore(); + } + }); + it('POSTs to the /events/:eventType alias and decodes the response', async () => { const origin = WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; @@ -1035,6 +1133,173 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent.assertNoPendingInterceptors(); }); + it.each([ + ['continues a truncated run_started replay', 'eid:evnt_2', true], + ['rejects a continuation without its trailing cursor', undefined, false], + ['rejects an empty continuation cursor', '', false], + ['rejects a non-advancing continuation cursor', 'eid:evnt_1', false], + ])('%s', async (_name, suffixCursor, succeeds) => { + const origin = + WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; + const agent = new MockAgent(); + agent.disableNetConnect(); + + agent + .get(origin) + .intercept({ + path: '/api/v4/runs/wrun_1/events/run_started', + method: 'POST', + headers: { accept: V4_FRAME_CONTENT_TYPE }, + }) + .reply( + 200, + encodeFrame( + { + eventId: 'evnt_1', + runId: 'wrun_1', + eventType: 'run_created', + createdAt: CREATED_AT, + eventData: { + deploymentId: 'dpl_1', + workflowName: 'workflow', + input: null, + }, + }, + new Uint8Array() + ), + { + headers: { + 'content-type': V4_FRAME_CONTENT_TYPE, + 'x-wf-max-events': '10000', + }, + } + ); + agent + .get(origin) + .intercept({ + path: '/api/v4/runs/wrun_1/events?returnAll=true&cursor=eid%3Aevnt_1&remoteRefBehavior=resolve', + method: 'GET', + }) + .reply( + 200, + Buffer.concat([ + encodeFrame( + { + eventId: 'evnt_2', + runId: 'wrun_1', + eventType: 'run_started', + createdAt: CREATED_AT, + }, + new Uint8Array() + ), + encodeFrame( + { + _end: 1, + ...(suffixCursor !== undefined ? { next: suffixCursor } : {}), + hasMore: false, + }, + new Uint8Array() + ), + ]), + { headers: { 'content-type': V4_FRAME_CONTENT_TYPE } } + ); + + const request = createWorkflowRunStartedEventV4( + { runId: 'wrun_1', specVersion: 5 }, + { token: 'test-token', dispatcher: agent } + ); + if (succeeds) { + const result = await request; + expect(result.events.map((event) => event.eventId)).toEqual([ + 'evnt_1', + 'evnt_2', + ]); + expect(result.cursor).toBe(suffixCursor); + } else { + await expect(request).rejects.toMatchObject({ + code: 'SCHEMA_VALIDATION', + message: 'v4 listEvents: response did not advance cursor', + }); + } + agent.assertNoPendingInterceptors(); + }); + + it('shares the three-continuation limit with a partial run_started POST', async () => { + const origin = + WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; + const agent = new MockAgent(); + agent.disableNetConnect(); + + agent + .get(origin) + .intercept({ + path: '/api/v4/runs/wrun_1/events/run_started', + method: 'POST', + headers: { accept: V4_FRAME_CONTENT_TYPE }, + }) + .reply( + 200, + encodeFrame( + { + eventId: 'evnt_1', + runId: 'wrun_1', + eventType: 'run_created', + createdAt: CREATED_AT, + eventData: { + deploymentId: 'dpl_1', + workflowName: 'workflow', + input: null, + }, + }, + new Uint8Array() + ), + { + headers: { + 'content-type': V4_FRAME_CONTENT_TYPE, + 'x-wf-max-events': '10000', + }, + } + ); + + for (const [cursor, eventId] of [ + ['eid:evnt_1', 'evnt_2'], + ['eid:evnt_2', 'evnt_3'], + ['eid:evnt_3', 'evnt_4'], + ] as const) { + agent + .get(origin) + .intercept({ + path: + '/api/v4/runs/wrun_1/events?returnAll=true' + + `&cursor=${encodeURIComponent(cursor)}&remoteRefBehavior=resolve`, + method: 'GET', + }) + .reply( + 200, + encodeFrame( + { + eventId, + runId: 'wrun_1', + eventType: 'run_started', + createdAt: CREATED_AT, + }, + new Uint8Array() + ), + { headers: { 'content-type': V4_FRAME_CONTENT_TYPE } } + ); + } + + await expect( + createWorkflowRunStartedEventV4( + { runId: 'wrun_1', specVersion: 5 }, + { token: 'test-token', dispatcher: agent } + ) + ).rejects.toThrow( + 'frame stream ended without the end-of-stream sentinel (1 events read)' + ); + agent.assertNoPendingInterceptors(); + }); + it('requires the event-stream response requested by run_started', async () => { const origin = WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; diff --git a/packages/world-vercel/src/events-v4.ts b/packages/world-vercel/src/events-v4.ts index 6ffdbc51da..12bc8adea5 100644 --- a/packages/world-vercel/src/events-v4.ts +++ b/packages/world-vercel/src/events-v4.ts @@ -42,6 +42,7 @@ import { type DecodedFrame, decodeFrames, encodeFrame, + IncompleteFrameError, V4_FRAME_CONTENT_TYPE, } from './frames.js'; import { @@ -809,7 +810,9 @@ export async function createWorkflowRunEventV4( const contentType = response.headers.get('content-type'); if (contentType?.startsWith(V4_FRAME_CONTENT_TYPE)) { - throw new Error('v4 createEvent: unexpected event page'); + throw new WorkflowWorldError('v4 createEvent: unexpected event page', { + code: 'SCHEMA_VALIDATION', + }); } return decodeCreateEventResponse(response, input.eventType); @@ -822,9 +825,19 @@ async function decodeCreateEventResponse( response: FrameResponseLike, eventType: T ): Promise & { event: Event }> { - const bodyBytes = new Uint8Array(await response.arrayBuffer()); + let bodyBytes: Uint8Array; + try { + bodyBytes = new Uint8Array(await response.arrayBuffer()); + } catch (cause) { + throw new WorkflowWorldError( + 'v4 createEvent: failed to read response body', + { code: 'TRANSPORT', cause } + ); + } if (bodyBytes.byteLength === 0) { - throw new Error('v4 createEvent: empty response body'); + throw new WorkflowWorldError('v4 createEvent: empty response body', { + code: 'PARSE_ERROR', + }); } const schema: z.ZodType & { event: Event }> = CreateEventV4BodySchemas[eventType].refine( @@ -833,7 +846,16 @@ async function decodeCreateEventResponse( (eventType === 'hook_created' && event.eventType === 'hook_conflict'), { path: ['event', 'eventType'] } ); - const parsedBody = schema.safeParse(decode(bodyBytes)); + let decoded: unknown; + try { + decoded = decode(bodyBytes); + } catch (cause) { + throw new WorkflowWorldError('v4 createEvent: invalid CBOR response body', { + code: 'PARSE_ERROR', + cause, + }); + } + const parsedBody = schema.safeParse(decoded); if (!parsedBody.success) { throw new WorkflowWorldError('v4 createEvent: invalid response body', { code: 'SCHEMA_VALIDATION', @@ -852,9 +874,13 @@ export async function createWorkflowRunStartedEventV4( 'event-stream', config ); - const events: Event[] = []; - const page = await consumeEventFrameStream(response, 'createEvent', events); - assert(page.cursor, 'v4 createEvent: event stream missing cursor'); + const page = await consumeReplayLogResponse(response, input.runId, config); + if (!page.cursor) { + throw new WorkflowWorldError( + 'v4 createEvent: event stream missing cursor', + { code: 'SCHEMA_VALIDATION' } + ); + } const maxEvents = MaxEventsHeaderSchema.safeParse( response.headers.get(MAX_EVENTS_HEADER) ); @@ -865,7 +891,7 @@ export async function createWorkflowRunStartedEventV4( }); } - return { events, ...page, maxEvents: maxEvents.data }; + return { ...page, maxEvents: maxEvents.data }; } /** One event of a v4 batch POST, index-aligned with the response results. */ @@ -1286,9 +1312,10 @@ export type HookReceivedPreloadV4Result = * A server that supports the lazy-hook replay stream answers the consumer's * idempotent re-ensure with the run's complete replay log as v4 frames: * the same event-frame sequence LIST uses, ending with the `_end` sentinel. - * A truncated stream (EOF without the sentinel) throws; the write is - * deduplicated by the server's `(runId, resumeId)` constraint, so retrying - * the whole request is safe and converges on the same canonical event. + * A truncated stream resumes after its last validated event. If it ends before + * any event is available to form a cursor, the write is deduplicated by the + * server's `(runId, resumeId)` constraint, so retrying the whole request is + * still safe and converges on the same canonical event. */ export async function createHookReceivedPreloadEventV4( input: CreateEventV4InputBase, @@ -1308,14 +1335,12 @@ export async function createHookReceivedPreloadEventV4( }; } - const events: Event[] = []; - const page = await consumeEventFrameStream(response, 'createEvent', events); + const page = await consumeReplayLogResponse(response, input.runId, config); const maxEvents = MaxEventsHeaderSchema.safeParse( response.headers.get(MAX_EVENTS_HEADER) ); return { kind: 'stream', - events, ...page, canonicalEventId: response.headers.get(EVENT_ID_HEADER) ?? undefined, maxEvents: maxEvents.success ? maxEvents.data : undefined, @@ -1445,40 +1470,115 @@ function streamErrorFrameToError( ); } +type EventFrameStreamResult = ListEventsV4Result & { + partialError?: WorkflowWorldError; +}; + +const MAX_PARTIAL_STREAM_RETRIES = 2; + +function partialEventFrameStream( + events: Event[], + partialError: WorkflowWorldError +): EventFrameStreamResult { + const eventId = events.at(-1)?.eventId; + if (!eventId) throw partialError; + return { events, cursor: `eid:${eventId}`, hasMore: true, partialError }; +} + async function consumeEventFrameStream( response: Response, - opName: string, - events: Event[] -): Promise> { + opName: string +): Promise { const contentType = response.headers.get('content-type'); if (!contentType?.startsWith(V4_FRAME_CONTENT_TYPE)) { - throw new Error( - `v4 ${opName}: expected ${V4_FRAME_CONTENT_TYPE}, got ${contentType ?? '(none)'}` + throw new WorkflowWorldError( + `v4 ${opName}: expected ${V4_FRAME_CONTENT_TYPE}, got ${contentType ?? '(none)'}`, + { code: 'SCHEMA_VALIDATION' } ); } + if (!response.body) { + throw new WorkflowWorldError(`v4 ${opName}: response body is missing`, { + code: 'TRANSPORT', + }); + } - const chunks = response.body as unknown as AsyncIterable; - - for await (const frame of decodeFrames(chunks)) { - if (frame.meta._end === 1) { - const end = EventStreamEndSchema.parse(frame.meta); - return { cursor: end.next ?? null, hasMore: end.hasMore }; + const events: Event[] = []; + try { + for await (const frame of decodeFrames(response.body)) { + if (frame.meta._end === 1) { + const end = EventStreamEndSchema.parse(frame.meta); + return { + events, + cursor: end.next ?? null, + hasMore: end.hasMore, + }; + } + if (frame.meta._error === 1) { + throw streamErrorFrameToError(frame.meta, opName); + } + if (Object.keys(frame.meta).some((key) => key.startsWith('_'))) { + throw new Error(`v4 ${opName}: unexpected control frame`); + } + events.push(decodeEventFrame(frame)); } - if (frame.meta._error === 1) { - throw streamErrorFrameToError(frame.meta, opName); + } catch (cause) { + if (CorruptedEventLogError.is(cause) || WorkflowWorldError.is(cause)) { + throw cause; } - if (Object.keys(frame.meta).some((key) => key.startsWith('_'))) { - throw new Error(`v4 ${opName}: unexpected control frame`); - } - events.push(decodeEventFrame(frame)); + const incomplete = cause instanceof IncompleteFrameError; + const error = new WorkflowWorldError( + `v4 ${opName}: ${incomplete ? 'incomplete' : 'invalid'} event frame stream`, + { + code: incomplete ? 'TRANSPORT' : 'SCHEMA_VALIDATION', + cause, + } + ); + if (!incomplete) throw error; + return partialEventFrameStream(events, error); } - throw new Error( - `v4 ${opName}: frame stream ended without the end-of-stream sentinel ` + - `(${events.length} events read) — truncated response?` + return partialEventFrameStream( + events, + new WorkflowWorldError( + `v4 ${opName}: frame stream ended without the end-of-stream sentinel ` + + `(${events.length} events read)`, + { code: 'TRANSPORT' } + ) ); } +/** + * Finish a replay-log POST without throwing away frames that were already + * validated. A graceful partial page and a transport-truncated body both + * continue with the ordinary GET endpoint from the response's last cursor. + */ +async function consumeReplayLogResponse( + response: Response, + runId: string, + config?: APIConfig +): Promise { + const page = await consumeEventFrameStream(response, 'createEvent'); + if (!page.hasMore) return page; + if (!page.cursor) { + if (page.partialError) throw page.partialError; + throw new WorkflowWorldError( + 'v4 createEvent: partial event stream missing cursor', + { code: 'SCHEMA_VALIDATION' } + ); + } + + const suffix = await getWorkflowRunEventsV4( + runId, + { cursor: page.cursor, remoteRefBehavior: 'resolve' }, + config + ); + return { + events: [...page.events, ...suffix.events], + cursor: suffix.cursor ?? page.cursor, + hasMore: suffix.hasMore, + }; +} + /** * Drive a v4 frame-stream list response into an in-memory page. Used by * both the by-runId and by-correlationId list endpoints. The wire @@ -1492,16 +1592,15 @@ async function consumeListFrameStream( url: string, headers: Headers, config: APIConfig | undefined, - opName: string, - events: Event[] -): Promise> { + opName: string +): Promise { const response = await fetchV4( url, { method: 'GET', headers }, config, opName ); - return consumeEventFrameStream(response, opName, events); + return consumeEventFrameStream(response, opName); } /** @@ -1533,8 +1632,10 @@ function paginationToQuery(params: ListEventsV4Params): string { * cursor from the sentinel frame. * * Eagerly drains the stream into memory to match the existing - * `getWorkflowRunEvents` contract. A truncated full response resumes - * after its last validated event instead of downloading accepted frames again. + * `getWorkflowRunEvents` contract. A truncated full response resumes after its + * last validated event until the sentinel arrives, for up to two retries. A + * forward-progress guard prevents retry loops. Explicitly paginated requests + * retain their one-page contract and surface truncation to the caller. */ export async function getWorkflowRunEventsV4( runId: string, @@ -1543,36 +1644,46 @@ export async function getWorkflowRunEventsV4( ): Promise { const { baseUrl, headers } = await getHttpConfig(config); const events: Event[] = []; - let cursor = params.cursor; + let cursor = params.cursor ?? null; + let partialStreamRetries = 0; + let consumed: EventFrameStreamResult; - while (true) { + do { const url = `${baseUrl}/v4/runs/${encodeURIComponent(runId)}/events` + - paginationToQuery({ ...params, cursor }); - try { - const page = await consumeListFrameStream( - url, - headers, - config, - 'listEvents', - events - ); - return { events, ...page }; - } catch (error) { - if (CorruptedEventLogError.is(error) || WorkflowWorldError.is(error)) { - throw error; - } - const lastEvent = events.at(-1); + paginationToQuery({ ...params, cursor: cursor ?? undefined }); + consumed = await consumeListFrameStream(url, headers, config, 'listEvents'); + const cursorAdvanced = !!consumed.cursor && consumed.cursor !== cursor; + if (consumed.partialError) { if ( params.limit !== undefined || - !lastEvent || - `eid:${lastEvent.eventId}` === cursor + !cursorAdvanced || + partialStreamRetries === MAX_PARTIAL_STREAM_RETRIES ) { - throw error; + throw consumed.partialError; } - cursor = `eid:${lastEvent.eventId}`; + assert(consumed.cursor); + partialStreamRetries++; + cursor = consumed.cursor; + } else if ( + !cursorAdvanced && + (consumed.events.length > 0 || consumed.hasMore) + ) { + throw new WorkflowWorldError( + 'v4 listEvents: response did not advance cursor', + { code: 'SCHEMA_VALIDATION' } + ); } - } + for (const event of consumed.events) { + events.push(event); + } + } while (consumed.partialError); + + return { + events, + cursor: consumed.cursor || (partialStreamRetries > 0 ? cursor : null), + hasMore: consumed.hasMore, + }; } /** @@ -1601,13 +1712,12 @@ export async function getEventsByCorrelationIdV4( sp.set('runId', runId); appendListParams(sp, params); const url = `${baseUrl}/v4/events?${sp.toString()}`; - const events: Event[] = []; - const page = await consumeListFrameStream( + const consumed = await consumeListFrameStream( url, headers, config, - 'listEventsByCorrelationId', - events + 'listEventsByCorrelationId' ); - return { events, ...page }; + if (consumed.partialError) throw consumed.partialError; + return consumed; } diff --git a/packages/world-vercel/src/events.test.ts b/packages/world-vercel/src/events.test.ts index 5fea9675a4..4a36c778a5 100644 --- a/packages/world-vercel/src/events.test.ts +++ b/packages/world-vercel/src/events.test.ts @@ -4,7 +4,7 @@ import type { AnyEventRequest, CreateEventParams } from '@workflow/world'; import { decode, encode } from 'cbor-x'; import { ulid } from 'ulid'; import { MockAgent } from 'undici'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { createWorkflowRunEvent, getWorkflowRunEvents, @@ -1046,6 +1046,52 @@ describe('createWorkflowRunEvent response coercion', () => { agent.assertNoPendingInterceptors(); }); + it('classifies a run_started stream missing lifecycle events as a world schema error', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce( + new Response( + Buffer.concat([ + encodeFrame( + { + eventId: 'evnt_1', + runId: 'wrun_1', + eventType: 'run_started', + createdAt: STARTED_AT, + specVersion: 5, + eventData: {}, + }, + new Uint8Array() + ), + encodeFrame( + { _end: 1, next: 'eid:evnt_1', hasMore: false }, + new Uint8Array() + ), + ]), + { + headers: { + 'content-type': V4_FRAME_CONTENT_TYPE, + 'x-wf-max-events': '10000', + }, + } + ) + ); + + try { + await expect( + createWorkflowRunEvent( + 'wrun_1', + { eventType: 'run_started', specVersion: 5 }, + undefined, + { token: 'test-token' } + ) + ).rejects.toMatchObject({ + name: 'WorkflowWorldError', + code: 'SCHEMA_VALIDATION', + }); + } finally { + fetchSpy.mockRestore(); + } + }); + it('threads the wait entity through to the EventResult', async () => { const agent = mockAgent(); agent @@ -1182,7 +1228,10 @@ describe('getWorkflowRunEvents remoteRefBehavior mapping', () => { }, body ), - encodeFrame({ _end: 1, hasMore: false }, new Uint8Array(0)), + encodeFrame( + { _end: 1, next: 'eid:evnt_1', hasMore: false }, + new Uint8Array(0) + ), ]); } @@ -1329,7 +1378,10 @@ describe('getWorkflowRunEvents legacy structured-error compatibility', () => { }, body ), - encodeFrame({ _end: 1, hasMore: false }, new Uint8Array(0)), + encodeFrame( + { _end: 1, next: 'eid:evnt_1', hasMore: false }, + new Uint8Array(0) + ), ]); } @@ -1557,8 +1609,8 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { return out; } - function hookReplayStreamResponse(): Uint8Array { - return concatFrames([ + function hookReplayFrames(): Uint8Array[] { + return [ encodeFrame( { eventId: 'evnt_1', @@ -1614,7 +1666,11 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { { _end: 1, next: 'eid:evnt_4', hasMore: false }, new Uint8Array() ), - ]); + ]; + } + + function hookReplayStreamResponse(): Uint8Array { + return concatFrames(hookReplayFrames()); } it('decodes a streamed replay log into event + reconstructed run + page', async () => { @@ -1842,7 +1898,7 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { agent.assertNoPendingInterceptors(); }); - it('rejects a truncated preload stream (no end sentinel)', async () => { + it('continues a truncated preload after its last validated event', async () => { const agent = mockAgent(); agent .get(ORIGIN) @@ -1851,30 +1907,35 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) - .reply( - 200, - encodeFrame( - { - eventId: 'evnt_4', - runId: 'wrun_1', - eventType: 'hook_received', - correlationId: 'hook_1', - createdAt: new Date('2026-06-10T00:00:03.000Z'), - specVersion: 2, - resumeId: RESUME_ID, - eventData: { token: 'tok-preload' }, - }, - PAYLOAD - ), - { headers: { 'content-type': V4_FRAME_CONTENT_TYPE } } - ); - - await expect( - createWorkflowRunEvent('wrun_1', hookReceivedRequest(), preloadParams, { - token: 'test-token', - dispatcher: agent, + .reply(200, concatFrames(hookReplayFrames().slice(0, 2)), { + headers: { + 'content-type': V4_FRAME_CONTENT_TYPE, + 'x-wf-event-id': 'evnt_4', + 'x-wf-max-events': '10000', + }, + }); + agent + .get(ORIGIN) + .intercept({ + path: /\/api\/v4\/runs\/wrun_1\/events\?.*cursor=eid%3Aevnt_2/, + method: 'GET', }) - ).rejects.toThrow(/end-of-stream sentinel/); + .reply(200, concatFrames(hookReplayFrames().slice(2)), { + headers: { + 'content-type': V4_FRAME_CONTENT_TYPE, + }, + }); + + const result = await createWorkflowRunEvent( + 'wrun_1', + hookReceivedRequest(), + preloadParams, + { token: 'test-token', dispatcher: agent } + ); + + expect(result.event?.eventId).toBe('evnt_4'); + expect(result.events).toHaveLength(4); + expect(result.maxEvents).toBe(10000); agent.assertNoPendingInterceptors(); }); diff --git a/packages/world-vercel/src/events.ts b/packages/world-vercel/src/events.ts index 92d0be103a..3588849b10 100644 --- a/packages/world-vercel/src/events.ts +++ b/packages/world-vercel/src/events.ts @@ -759,13 +759,15 @@ async function createWorkflowRunEventInner( (event) => event.eventType === 'run_started' ); if (!runCreated) { - throw new Error( - 'v4 createEvent: run_started stream is missing run_created' + throw new WorkflowWorldError( + 'v4 createEvent: run_started stream is missing run_created', + { code: 'SCHEMA_VALIDATION' } ); } if (!runStarted) { - throw new Error( - 'v4 createEvent: run_started stream is missing run_started' + throw new WorkflowWorldError( + 'v4 createEvent: run_started stream is missing run_started', + { code: 'SCHEMA_VALIDATION' } ); } diff --git a/packages/world-vercel/src/frames.ts b/packages/world-vercel/src/frames.ts index 93809ab179..2e839dd6e7 100644 --- a/packages/world-vercel/src/frames.ts +++ b/packages/world-vercel/src/frames.ts @@ -18,6 +18,9 @@ export interface DecodedFrame { body: Uint8Array; } +/** The response body stopped before the next complete frame was available. */ +export class IncompleteFrameError extends Error {} + // The protocol consumer validates the event or control-frame shape after the // body is available. The byte codec only requires a CBOR object here. const CborObjectSchema = z.record(z.string(), z.unknown()); @@ -71,7 +74,14 @@ export async function* decodeFrames( const parts: Uint8Array[] = [buffer]; let byteLength = buffer.byteLength; while (byteLength < needed) { - const chunk = await chunks.next(); + let chunk: IteratorResult; + try { + chunk = await chunks.next(); + } catch (cause) { + throw new IncompleteFrameError('decodeFrames: source stream failed', { + cause, + }); + } if (chunk.done) return false; if (chunk.value.byteLength === 0) continue; parts.push(chunk.value); @@ -104,12 +114,12 @@ export async function* decodeFrames( take(4); if (!(await refill(metaLen))) { - throw new Error('decodeFrames: truncated meta block'); + throw new IncompleteFrameError('decodeFrames: truncated meta block'); } const meta = CborObjectSchema.parse(decode(take(metaLen))); if (!(await refill(4))) { - throw new Error('decodeFrames: truncated body length'); + throw new IncompleteFrameError('decodeFrames: truncated body length'); } const bodyLen = new DataView( buffer.buffer, @@ -119,7 +129,7 @@ export async function* decodeFrames( take(4); if (bodyLen > 0 && !(await refill(bodyLen))) { - throw new Error('decodeFrames: truncated body bytes'); + throw new IncompleteFrameError('decodeFrames: truncated body bytes'); } // Slice (not subarray) so the yielded body owns its bytes, so later // reads into the buffer won't overwrite it; bodyLen 0 yields empty. From 07ec212fe762e0659d4528913716c59870fd6c7d Mon Sep 17 00:00:00 2001 From: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> Date: Mon, 31 Aug 2026 12:03:45 -0700 Subject: [PATCH 3/8] fix(core): re-arm late-claimed hook deliveries (#3879) * fix(core): re-arm late-claimed hook deliveries * refactor(core): clarify delivery barrier lifecycle --- .changeset/late-hooks-rearm.md | 5 + .../async-deserialization-ordering.test.ts | 46 +++++++- .../src/delivery-barrier-dispenser.test.ts | 12 ++ packages/core/src/private.ts | 109 ++++++++++++------ packages/core/src/retained-vm-loop.test.ts | 79 +++++++++++++ 5 files changed, 213 insertions(+), 38 deletions(-) create mode 100644 .changeset/late-hooks-rearm.md diff --git a/.changeset/late-hooks-rearm.md b/.changeset/late-hooks-rearm.md new file mode 100644 index 0000000000..9ca2387ad0 --- /dev/null +++ b/.changeset/late-hooks-rearm.md @@ -0,0 +1,5 @@ +--- +"@workflow/core": patch +--- + +Keep late-claimed buffered hook payloads from being preempted by workflow suspension in retained VMs. diff --git a/packages/core/src/async-deserialization-ordering.test.ts b/packages/core/src/async-deserialization-ordering.test.ts index 41d4684563..fc2fa96607 100644 --- a/packages/core/src/async-deserialization-ordering.test.ts +++ b/packages/core/src/async-deserialization-ordering.test.ts @@ -5,7 +5,7 @@ import { monotonicFactory } from 'ulid'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import { registerSerializationClass } from './class-serialization.js'; import { EventsConsumer } from './events-consumer.js'; -import type { WorkflowOrchestratorContext } from './private.js'; +import { isDeliveryIdle, type WorkflowOrchestratorContext } from './private.js'; import { ReplayPayloadCache } from './replay-payload-cache.js'; import { dehydrateStepError, @@ -764,4 +764,48 @@ describe('async deserialization ordering', () => { expect(ctx.pendingDeliveryBarriers?.size).toBe(0); }); + + it('should restore delivery protection when a buffered hook payload is claimed after its barrier retires', async () => { + const payload = await dehydrateStepReturnValue( + 'buffered', + 'wrun_test', + undefined + ); + const ctx = setupWorkflowContext([ + { + eventId: 'evnt_0', + runId: 'wrun_test', + eventType: 'hook_received', + correlationId: 'hook_01K11TFZ62YS0YYFDQ3E8B9YCV', + eventData: { payload }, + createdAt: new Date(), + }, + ]); + + // Model another payload hydrating in the same replay so the idle safety + // net cannot retire this hook's barrier before the test observes it. + ctx.pendingDeliveries = 1; + const createHook = createCreateHook(ctx); + const hook = createHook(); + + // The payload arrived before a consumer requested it, so it starts with + // an unarmed barrier and remains buffered after the idle safety net retires + // that barrier. + await vi.waitFor(() => { + expect(ctx.pendingDeliveryBarriers?.size).toBe(1); + }); + await ctx.promiseQueue; + expect(ctx.pendingDeliveryBarriers?.size).toBe(1); + ctx.pendingDeliveries = 0; + await vi.waitFor(() => { + expect(ctx.pendingDeliveryBarriers?.size).toBe(0); + }); + + // Claiming the buffered payload commits it to reaching this consumer. Its + // delivery must become non-idle again until the claim resolves. + const delivery = hook.then((value) => value); + expect(isDeliveryIdle(ctx)).toBe(false); + await expect(delivery).resolves.toBe('buffered'); + expect(isDeliveryIdle(ctx)).toBe(true); + }); }); diff --git a/packages/core/src/delivery-barrier-dispenser.test.ts b/packages/core/src/delivery-barrier-dispenser.test.ts index b979b178da..10206e260b 100644 --- a/packages/core/src/delivery-barrier-dispenser.test.ts +++ b/packages/core/src/delivery-barrier-dispenser.test.ts @@ -128,6 +128,18 @@ const resumeAtA = new Date('2026-07-27T12:00:05.000Z'); const resumeAtB = new Date('2026-07-27T12:00:06.000Z'); describe('barrier safety-net dispenser', () => { + it('rejects a second barrier owner for the same event index', () => { + const ctx = setupWorkflowContext([]); + const barrier = registerDeliveryBarrier(ctx, 0, 'hook', { armed: false }); + + expect(() => registerDeliveryBarrier(ctx, 0, 'step')).toThrowError( + 'Delivery barrier already registered at event index 0' + ); + + barrier.markDelivered(); + expect(isDeliveryIdle(ctx)).toBe(true); + }); + it('suspends only after deliveries parked behind an unclaimed payload have run', async () => { const ops: Promise[] = []; const payload = await dehydrateStepReturnValue( diff --git a/packages/core/src/private.ts b/packages/core/src/private.ts index 7141fe54ee..63883ec49a 100644 --- a/packages/core/src/private.ts +++ b/packages/core/src/private.ts @@ -2,6 +2,7 @@ * Utils used by the bundler when transforming code */ +import { WorkflowRuntimeError } from '@workflow/errors'; import { withResolvers } from '@workflow/utils'; import type { WorldCapabilities } from '@workflow/world'; import type { EventsConsumer } from './events-consumer.js'; @@ -233,8 +234,8 @@ export type DeliveryKind = 'hook' | 'wait' | 'step'; interface DeliveryBarrierEntry { kind: DeliveryKind; - /** Resolves once this delivery has resolved to the workflow. */ - delivered: Promise; + /** Resolves once this delivery is handed to the workflow or retired. */ + released: Promise; /** * Whether this delivery is committed to reaching the workflow without any * further action by workflow code. True for wait completions and step @@ -247,11 +248,15 @@ interface DeliveryBarrierEntry { * once a consumer takes the payload. */ armed: boolean; + /** Whether this entry has been removed and its `released` promise settled. */ + retired: boolean; /** - * Retire this entry: resolve `delivered` and remove it from the registry, - * exactly as `markDelivered` would. Called only by the context's safety-net - * dispenser ({@link ensureBarrierSafetyNet}), and only on the lowest-index - * entry at delivery idle. Idempotent. + * Retire this entry: resolve `released` and remove it from the registry, + * without marking the handle delivered to the workflow. A safety-retired + * buffered payload may therefore install a fresh entry if it is claimed by + * a retained VM later. Called only by the context's safety-net dispenser + * ({@link ensureBarrierSafetyNet}), and only on the lowest-index entry at + * delivery idle. Idempotent. */ retire: () => void; } @@ -573,7 +578,7 @@ export async function awaitEarlierDeliveries( if (!gatesOn(kind, eventIndex, index, entry)) { continue; } - earlier.push(entry.delivered); + earlier.push(entry.released); } if (earlier.length > 0) { await Promise.all(earlier); @@ -612,7 +617,7 @@ export async function awaitEarlierDeliveries( export interface DeliveryBarrier { /** * Mark this delivery as delivered to the workflow. Resolves its - * `delivered` promise so any later-in-log delivery gated on it (via + * `released` promise so any later-in-log delivery gated on it (via * {@link awaitEarlierDeliveries}) may proceed, and removes it from the * registry. Idempotent. */ @@ -658,41 +663,71 @@ export function registerDeliveryBarrier( return { markDelivered: () => {}, arm: () => {} }; } - let done = false; - const { promise, resolve } = withResolvers(); - - const finish = () => { - if (done) { - return; - } - done = true; - if (barriers.get(eventIndex) === entry) { - barriers.delete(eventIndex); + const install = (armed: boolean): DeliveryBarrierEntry => { + if (barriers.has(eventIndex)) { + throw new WorkflowRuntimeError( + `Delivery barrier already registered at event index ${eventIndex}` + ); } - resolve(); + const { promise, resolve } = withResolvers(); + const entry: DeliveryBarrierEntry = { + kind, + released: promise, + armed, + retired: false, + retire: () => { + if (entry.retired) { + return; + } + entry.retired = true; + if (barriers.get(eventIndex) === entry) { + barriers.delete(eventIndex); + } + resolve(); + }, + }; + barriers.set(eventIndex, entry); + + // Safety net: if this delivery is never delivered to the workflow (its + // branch was not taken / the run is suspending, or a buffered hook payload + // is only claimed after a later delivery the workflow is still waiting + // on), it is retired at idle so a later delivery gated on it cannot + // deadlock and the registry cannot leak an entry per abandoned delivery. + // Retirement goes through the context's single ordered dispenser rather + // than a per-barrier idle poll. See {@link ensureBarrierSafetyNet} for why + // the ORDER of these retirements is load-bearing. + ensureBarrierSafetyNet(ctx); + return entry; }; - const entry: DeliveryBarrierEntry = { - kind, - delivered: promise, - armed: options.armed ?? true, - retire: finish, - }; - barriers.set(eventIndex, entry); - - // Safety net: if this delivery is never delivered to the workflow (its - // branch was not taken / the run is suspending, or a buffered hook payload - // is only claimed after a later delivery the workflow is still waiting on), - // it is retired at idle so a later delivery gated on it cannot deadlock and - // the registry cannot leak an entry per abandoned delivery. Retirement goes - // through the context's single ordered dispenser rather than a per-barrier - // idle poll. See {@link ensureBarrierSafetyNet} for why the ORDER of these - // retirements is load-bearing. - ensureBarrierSafetyNet(ctx); + let entry = install(options.armed ?? true); + let deliveredToWorkflow = false; return { - markDelivered: finish, + markDelivered: () => { + if (deliveredToWorkflow) { + return; + } + deliveredToWorkflow = true; + entry.retire(); + }, arm: () => { + if (deliveredToWorkflow) { + return; + } + // The idle safety net may retire an unclaimed buffered hook payload + // while a retained VM keeps its `claim()` closure alive. If workflow + // code later claims that payload, replace the settled entry so delivery + // remains non-idle until the claim reaches the workflow. + if (entry.retired) { + entry = install(true); + return; + } + if (barriers.get(eventIndex) !== entry) { + throw new WorkflowRuntimeError( + `Delivery barrier lost ownership of event index ${eventIndex}` + ); + } entry.armed = true; }, }; diff --git a/packages/core/src/retained-vm-loop.test.ts b/packages/core/src/retained-vm-loop.test.ts index 37d809e2a6..0a47b7341f 100644 --- a/packages/core/src/retained-vm-loop.test.ts +++ b/packages/core/src/retained-vm-loop.test.ts @@ -28,6 +28,38 @@ vi.mock('./vm/index.js', async (importActual) => { return { ...actual, createContext: vi.fn(actual.createContext) }; }); +const barrierArmObservations = vi.hoisted( + (): Array<{ before: number; after: number }> => [] +); + +// Preserve the production implementation while recording the registry +// transition made by each arm(). This lets the retained-session regression +// assert the short-lived protection window directly instead of depending on a +// timer winning the same race reliably on every test runner. +vi.mock('./private.js', async (importActual) => { + const actual = await importActual(); + return { + ...actual, + registerDeliveryBarrier: ( + ...args: Parameters + ) => { + const [ctx] = args; + const barrier = actual.registerDeliveryBarrier(...args); + return { + markDelivered: barrier.markDelivered, + arm: () => { + const before = ctx.pendingDeliveryBarriers?.size ?? 0; + barrier.arm(); + barrierArmObservations.push({ + before, + after: ctx.pendingDeliveryBarriers?.size ?? 0, + }); + }, + }; + }, + }; +}); + const { createContext } = await import('./vm/index.js'); const { registerSerializationClass } = await import('./class-serialization.js'); const { registerStepFunction } = await import('./private.js'); @@ -130,6 +162,29 @@ const openHookRaceWorkflow = `const createHook = globalThis[Symbol.for("WORKFLOW } globalThis.__private_workflows = new Map([["workflow", workflow]]);`; +// The payload arrives while the workflow is waiting on s1, before any hook +// consumer exists. The next pass buffers it, advances through s1, and suspends +// on s2; delivery idle retires the payload's unarmed barrier at that boundary. +// A retained resume arms an empty hook before it claims the buffered one. The +// empty read schedules suspension while the buffered read resolves a separate +// workflow promise from its continuation, matching Eve's multiplexed inbox. +// The late claim must re-arm delivery until that continuation can wake the body. +const bufferedHookAcrossStepWorkflow = `const createHook = globalThis[Symbol.for("WORKFLOW_CREATE_HOOK")]; + const s1 = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("r_s1"); + const s2 = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("r_s2"); + async function workflow() { + const buffered = createHook({ token: "retained-buffered-hook" }); + const empty = createHook({ token: "retained-empty-hook" }); + await s1(); + const stepValue = await s2(); + const payload = await new Promise((resolve, reject) => { + void empty.then(resolve, reject); + void buffered.then(resolve, reject); + }); + return stepValue + (payload.source === "external-hook" ? 1000 : 0); + } + globalThis.__private_workflows = new Map([["workflow", workflow]]);`; + // The first invocation owns r_concurrent_s1 while a hook wake starts a cold // peer. The peer takes the hook branch and owns r_concurrent_s2 before the // retained invocation resumes, exercising the real two-replay ownership race. @@ -677,6 +732,7 @@ async function driveConcurrentHookWakeRace(runId: string) { describe('retained VM through the inline replay loop', () => { beforeEach(() => { createContextSpy.mockClear(); + barrierArmObservations.length = 0; }); afterEach(() => { delete process.env.WORKFLOW_RETAINED_VM; @@ -757,6 +813,29 @@ describe('retained VM through the inline replay loop', () => { expect(on.durableLog).toEqual(off.durableLog); }); + it('claims a buffered hook after its barrier retires across a retained step boundary', async () => { + process.env.WORKFLOW_RETAINED_VM = '0'; + const off = await drive( + 'wrun_retained_buffered_hook_after_step', + bufferedHookAcrossStepWorkflow, + { type: 'inject-hook' } + ); + createContextSpy.mockClear(); + barrierArmObservations.length = 0; + delete process.env.WORKFLOW_RETAINED_VM; + + const on = await drive( + 'wrun_retained_buffered_hook_after_step', + bufferedHookAcrossStepWorkflow, + { type: 'inject-hook' } + ); + expect(off.result).toBe(1020); + expect(on.result).toBe(1020); + expect(on.vmBuilds).toBe(1); + expect(barrierArmObservations).toContainEqual({ before: 0, after: 1 }); + expect(on.durableLog).toEqual(off.durableLog); + }); + it('matches cold replay when a hook wake races the retained invocation', async () => { process.env.WORKFLOW_RETAINED_VM = '0'; const off = await driveConcurrentHookWakeRace( From 1c28eeca159f022c73912326baf78d69152db876 Mon Sep 17 00:00:00 2001 From: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> Date: Mon, 31 Aug 2026 12:05:41 -0700 Subject: [PATCH 4/8] [core] Trace fresh workflow replay phases (#3797) * Trace fresh workflow replay phases Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> * refactor(core): simplify workflow script cache API Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> * Fix retained workflow tracing Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> * fix(core): keep tracing failure-safe Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> * Trace replay event loading Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> --------- Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> --- .changeset/trace-replay-phases.md | 5 + packages/core/src/runtime-trace-mode.test.ts | 13 +- packages/core/src/runtime.ts | 59 +++-- packages/core/src/runtime/helpers.ts | 184 +++++++------- packages/core/src/telemetry.ts | 65 ++++- .../src/telemetry/semantic-conventions.ts | 14 ++ packages/core/src/vm/script-cache.test.ts | 74 +++--- packages/core/src/vm/script-cache.ts | 19 +- packages/core/src/workflow-tracing.test.ts | 228 ++++++++++++++++++ packages/core/src/workflow.ts | 119 ++++++--- 10 files changed, 585 insertions(+), 195 deletions(-) create mode 100644 .changeset/trace-replay-phases.md create mode 100644 packages/core/src/workflow-tracing.test.ts diff --git a/.changeset/trace-replay-phases.md b/.changeset/trace-replay-phases.md new file mode 100644 index 0000000000..6ec248ced6 --- /dev/null +++ b/.changeset/trace-replay-phases.md @@ -0,0 +1,5 @@ +--- +"@workflow/core": patch +--- + +Trace event loading, workflow VM creation, bundle compilation and evaluation, input hydration, and replay execution. diff --git a/packages/core/src/runtime-trace-mode.test.ts b/packages/core/src/runtime-trace-mode.test.ts index bf119ac4ac..22721b4961 100644 --- a/packages/core/src/runtime-trace-mode.test.ts +++ b/packages/core/src/runtime-trace-mode.test.ts @@ -201,7 +201,6 @@ async function driveHandler(opts: { const getWorldSpan = exporter .getFinishedSpans() .find((s) => s.name === 'workflow.route.get_world'); - return { workflowSpan, routeSpan, @@ -287,7 +286,6 @@ describe('workflowEntrypoint trace modes', () => { ); expect(getWorldSpan).toBeDefined(); expect(getWorldSpan?.parentSpanId).toBe(routeSpan?.spanContext().spanId); - expect(workflowSpan).toBeDefined(); // Child of the local /flow route span — same trace, so one // invocation is a single bounded trace rather than a new root. @@ -316,6 +314,17 @@ describe('workflowEntrypoint trace modes', () => { runStartedCreateEvent?.attributes['workflow.run_started.skip_preload'] ).toBe(false); + const replayLoadSpan = exporter + .getFinishedSpans() + .find((finished) => finished.name === 'workflow.replay.load'); + expect(replayLoadSpan?.parentSpanId).toBe( + workflowSpan?.spanContext().spanId + ); + expect(replayLoadSpan?.attributes).toMatchObject({ + 'workflow.replay.load.source': 'run_started', + 'workflow.events.count': 0, + }); + // Queue-delivered invocation spans use the CONSUMER kind, matching // queue-delivered step.execute spans. expect(workflowSpan?.kind).toBe(SpanKind.CONSUMER); diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index 426fdcc14e..6cc4ce4415 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -969,6 +969,24 @@ export function workflowEntrypoint( return result; }; + const traceReplayLoad = ( + source: Attribute.WorkflowReplayLoadSource, + load: () => Promise + ): Promise => + trace('workflow.replay.load', async (loadSpan) => { + loadSpan?.setAttributes({ + ...Attribute.WorkflowRunId(runId), + ...Attribute.WorkflowReplayLoadSource(source), + }); + const result = await load(); + loadSpan?.setAttributes( + Attribute.WorkflowEventsCount( + result.events?.length ?? 0 + ) + ); + return result; + }); + /** * The slot snapshot for a write issued from this loop: how * much of the run's log the decision behind it was made @@ -2034,23 +2052,25 @@ export function workflowEntrypoint( span?.addEvent('workflow.hook_received.create.start', { 'workflow.hook_received.preload_events': true, }); - const result = await createEvent( - { - eventType: 'hook_received', - specVersion: SPEC_VERSION_CURRENT, - correlationId: hookResumeInput.hookId, - eventData: { - token: hookResumeInput.token, - payload: hookResumeInput.payload, + const result = await traceReplayLoad('hook_preload', () => + createEvent( + { + eventType: 'hook_received', + specVersion: SPEC_VERSION_CURRENT, + correlationId: hookResumeInput.hookId, + eventData: { + token: hookResumeInput.token, + payload: hookResumeInput.payload, + }, }, - }, - { - requestId, - occurredAt, - resumeId: hookResumeInput.resumeId, - resumePayloadDigest: hookResumeInput.payloadDigest, - preloadEvents: true, - } + { + requestId, + occurredAt, + resumeId: hookResumeInput.resumeId, + resumePayloadDigest: hookResumeInput.payloadDigest, + preloadEvents: true, + } + ) ); hookEnsured = true; // Note: unlike the re-ensure below, this hoisted write @@ -2324,9 +2344,10 @@ export function workflowEntrypoint( span?.addEvent('workflow.run_started.create.start', { 'workflow.run_started.skip_preload': false, }); - const result = await createEvent(runStartedEvent, { - requestId, - }); + const result = await traceReplayLoad( + 'run_started', + () => createEvent(runStartedEvent, { requestId }) + ); workflowRun = result.run; maxEventsLimit = clampMaxEvents(result.maxEvents); // Anchors RSFS, see the declaration above. diff --git a/packages/core/src/runtime/helpers.ts b/packages/core/src/runtime/helpers.ts index 8c7a4ce530..5f28ca0af3 100644 --- a/packages/core/src/runtime/helpers.ts +++ b/packages/core/src/runtime/helpers.ts @@ -597,108 +597,108 @@ export async function loadWorkflowRunEvents( afterCursor?: string ): Promise { const incremental = afterCursor !== undefined; - return trace( - incremental ? 'workflow.loadNewEvents' : 'workflow.loadEvents', - async (span) => { - span?.setAttributes({ - ...Attribute.WorkflowRunId(runId), - }); - - const loadedEvents: Event[] = []; - const loadedEventIds = new Set(); - const requestedCursors = new Set(); - let cursor: string | null = afterCursor ?? null; - let hasMore = true; - let pagesLoaded = 0; - let retriedWithoutCursor = false; - - const world = await getWorldLazy(); - const loadStart = Date.now(); - while (hasMore) { - // TODO: we're currently loading all the data with resolveRef behavior. We need to update this - // to lazyload the data from the world instead so that we can optimize and make the event log loading - // much faster and memory efficient - const pageStart = Date.now(); - const requestedCursor = cursor; - recordRequestedEventCursor(runId, requestedCursor, requestedCursors); - - let response: Awaited>; - try { - response = await world.events.list({ - runId, - pagination: { - sortOrder: 'asc', - cursor: requestedCursor ?? undefined, - }, - }); - } catch (error) { - if ( - shouldRetryWithoutEventCursor( - error, - requestedCursor, - retriedWithoutCursor - ) - ) { - runtimeLogger.warn( - 'Event cursor was rejected; retrying with a full event reload.', - { workflowRunId: runId } - ); - loadedEvents.length = 0; - loadedEventIds.clear(); - requestedCursors.clear(); - cursor = null; - retriedWithoutCursor = true; - continue; - } - throw error; - } - - appendUniqueEvents(loadedEvents, response.data, loadedEventIds); - hasMore = response.hasMore; - assertEventPaginationProgress( + return trace('workflow.replay.load', async (span) => { + span?.setAttributes({ + ...Attribute.WorkflowRunId(runId), + ...Attribute.WorkflowReplayLoadSource( + incremental ? 'events_list_incremental' : 'events_list' + ), + }); + + const loadedEvents: Event[] = []; + const loadedEventIds = new Set(); + const requestedCursors = new Set(); + let cursor: string | null = afterCursor ?? null; + let hasMore = true; + let pagesLoaded = 0; + let retriedWithoutCursor = false; + + const world = await getWorldLazy(); + const loadStart = Date.now(); + while (hasMore) { + // TODO: we're currently loading all the data with resolveRef behavior. We need to update this + // to lazyload the data from the world instead so that we can optimize and make the event log loading + // much faster and memory efficient + const pageStart = Date.now(); + const requestedCursor = cursor; + recordRequestedEventCursor(runId, requestedCursor, requestedCursors); + + let response: Awaited>; + try { + response = await world.events.list({ runId, - hasMore, - response.cursor, - requestedCursors - ); - // 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 DB query hit the limit exactly and returned a - // precautionary `LastEvaluatedKey`. 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. - cursor = response.cursor ?? cursor; - pagesLoaded++; - - runtimeLogger.debug('Loaded event page', { - workflowRunId: runId, - incremental, - page: pagesLoaded, - pageEvents: response.data.length, - totalEvents: loadedEvents.length, - hasMore, - pageMs: Date.now() - pageStart, + pagination: { + sortOrder: 'asc', + cursor: requestedCursor ?? undefined, + }, }); + } catch (error) { + if ( + shouldRetryWithoutEventCursor( + error, + requestedCursor, + retriedWithoutCursor + ) + ) { + runtimeLogger.warn( + 'Event cursor was rejected; retrying with a full event reload.', + { workflowRunId: runId } + ); + loadedEvents.length = 0; + loadedEventIds.clear(); + requestedCursors.clear(); + cursor = null; + retriedWithoutCursor = true; + continue; + } + throw error; } - runtimeLogger.debug('Event load complete', { + appendUniqueEvents(loadedEvents, response.data, loadedEventIds); + hasMore = response.hasMore; + assertEventPaginationProgress( + runId, + hasMore, + response.cursor, + requestedCursors + ); + // 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 DB query hit the limit exactly and returned a + // precautionary `LastEvaluatedKey`. 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. + cursor = response.cursor ?? cursor; + pagesLoaded++; + + runtimeLogger.debug('Loaded event page', { workflowRunId: runId, incremental, + page: pagesLoaded, + pageEvents: response.data.length, totalEvents: loadedEvents.length, - pagesLoaded, - totalMs: Date.now() - loadStart, + hasMore, + pageMs: Date.now() - pageStart, }); + } - span?.setAttributes({ - ...Attribute.WorkflowEventsCount(loadedEvents.length), - ...Attribute.WorkflowEventsPagesLoaded(pagesLoaded), - }); + runtimeLogger.debug('Event load complete', { + workflowRunId: runId, + incremental, + totalEvents: loadedEvents.length, + pagesLoaded, + totalMs: Date.now() - loadStart, + }); - return { events: loadedEvents, cursor }; - } - ); + span?.setAttributes({ + ...Attribute.WorkflowEventsCount(loadedEvents.length), + ...Attribute.WorkflowEventsPagesLoaded(pagesLoaded), + }); + + return { events: loadedEvents, cursor }; + }); } /** diff --git a/packages/core/src/telemetry.ts b/packages/core/src/telemetry.ts index e23ef6fc45..7b6ebdee2d 100644 --- a/packages/core/src/telemetry.ts +++ b/packages/core/src/telemetry.ts @@ -214,6 +214,23 @@ const StepExecutionDurationHistogram = once(async () => { // OTel registration, which is the whole point of the log. With several copies // in a process, each one's view is what is worth seeing. let otelDiagLogged = false; + +function describeThrownValue(value: unknown): string { + try { + if ( + typeof value === 'object' && + value !== null && + 'message' in value && + typeof value.message === 'string' + ) { + return value.message; + } + return String(value); + } catch { + return 'Unknown error'; + } +} + function logOtelDiagnosticOnce(otel: typeof api, tracer: api.Tracer): void { const debugEnabled = typeof process !== 'undefined' && @@ -278,7 +295,7 @@ export async function trace( } else { span.setStatus({ code: otel.SpanStatusCode.ERROR, - message: (e as Error).message, + message: describeThrownValue(e), }); } throw e; @@ -288,6 +305,52 @@ export async function trace( }); } +/** Starts a child span without installing it as the active context. */ +export async function startTraceSpan(spanName: string) { + const [tracer, otel] = await Promise.all([Tracer.value, OtelApi.value]); + if (!tracer || !otel) return { end() {}, fail() {} }; + + const span = tracer.startSpan(spanName); + let ended = false; + const finish = (status: api.SpanStatus) => { + if (ended) return; + ended = true; + span.setStatus(status); + span.end(); + }; + + return { + end: () => finish({ code: otel.SpanStatusCode.OK }), + fail: (error: unknown) => + finish({ + code: otel.SpanStatusCode.ERROR, + message: describeThrownValue(error), + }), + }; +} + +/** Keeps a parked workflow's ambient trace context aligned with each resume. */ +export async function createRefreshableTraceContext() { + const otel = await OtelApi.value; + if (!otel) { + return { refresh() {}, run: (fn: () => T): T => fn() }; + } + + let current = otel.context.active(); + const context: api.Context = { + getValue: (key) => current.getValue(key), + setValue: (key, value) => current.setValue(key, value), + deleteValue: (key) => current.deleteValue(key), + }; + + return { + refresh: () => { + current = otel.context.active(); + }, + run: (fn: () => T): T => otel.context.with(context, fn), + }; +} + /** * Emit a span whose start is back-dated to `startEpochMs` and whose end is now, * so its duration reflects an interval only measurable at its end (e.g. diff --git a/packages/core/src/telemetry/semantic-conventions.ts b/packages/core/src/telemetry/semantic-conventions.ts index fb6754017f..cb445c7976 100644 --- a/packages/core/src/telemetry/semantic-conventions.ts +++ b/packages/core/src/telemetry/semantic-conventions.ts @@ -82,6 +82,20 @@ export const WorkflowExecutionMode = SemanticConvention<'replay' | 'retained'>( 'workflow.execution.mode' ); +/** Whether the compiled application workflow bundle was cached. */ +export const WorkflowBundleCompileCacheHit = SemanticConvention( + 'workflow.bundle.compile.cache_hit' +); + +/** Operation that supplied events to the current replay. */ +export type WorkflowReplayLoadSource = + | 'run_started' + | 'hook_preload' + | 'events_list' + | 'events_list_incremental'; +export const WorkflowReplayLoadSource = + SemanticConvention('workflow.replay.load.source'); + /** * Events the replay walked past that no consumer claimed, still held when the * replay stopped. diff --git a/packages/core/src/vm/script-cache.test.ts b/packages/core/src/vm/script-cache.test.ts index 399f6b2c69..1dc2b34db5 100644 --- a/packages/core/src/vm/script-cache.test.ts +++ b/packages/core/src/vm/script-cache.test.ts @@ -1,10 +1,9 @@ -import { runInContext } from 'node:vm'; +import { type Context, runInContext } from 'node:vm'; import { afterEach, describe, expect, it } from 'vitest'; import { createContext } from './index.js'; import { clearWorkflowScriptCache, getCachedWorkflowScript, - runCachedWorkflowScript, workflowScriptCacheSize, } from './script-cache.js'; @@ -37,26 +36,43 @@ function buildBundle(marker: string, workflowCount = 12): string { return `globalThis.__private_workflows = new Map();\n${defs.join('\n')}\n`; } +function getScript(code: string, filename: string) { + return getCachedWorkflowScript(code, filename).script; +} + +function runScript(code: string, filename: string, context: Context) { + return getScript(code, filename).runInContext(context); +} + describe('script-cache', () => { afterEach(() => { clearWorkflowScriptCache(); }); it('returns the same compiled Script for identical (code, filename)', () => { - const a = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); - const b = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + const a = getScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + const b = getScript(SAMPLE_BUNDLE, 'workflows/a.ts'); expect(a).toBe(b); }); + it('reports whether compilation was served from cache', () => { + const first = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + const second = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + + expect(first.cacheHit).toBe(false); + expect(second.cacheHit).toBe(true); + expect(second.script).toBe(first.script); + }); + it('returns distinct Scripts for the same code under different filenames', () => { - const a = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); - const b = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/b.ts'); + const a = getScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + const b = getScript(SAMPLE_BUNDLE, 'workflows/b.ts'); expect(a).not.toBe(b); }); it('returns distinct Scripts for different code under the same filename', () => { - const a = getCachedWorkflowScript('1 + 1', 'workflows/a.ts'); - const b = getCachedWorkflowScript('2 + 2', 'workflows/a.ts'); + const a = getScript('1 + 1', 'workflows/a.ts'); + const b = getScript('2 + 2', 'workflows/a.ts'); expect(a).not.toBe(b); }); @@ -64,8 +80,8 @@ describe('script-cache', () => { // Cached path: run the bundle then look up the workflow, mirroring // runWorkflow's two-step evaluation. const { context: cachedCtx } = createContext({ seed, fixedTimestamp }); - runCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts', cachedCtx); - const cachedFn = runCachedWorkflowScript( + runScript(SAMPLE_BUNDLE, 'workflows/a.ts', cachedCtx); + const cachedFn = runScript( `globalThis.__private_workflows?.get('my/workflow')`, 'workflows/a.ts', cachedCtx @@ -90,16 +106,14 @@ describe('script-cache', () => { }); it('reuses the compiled Script across multiple runs against fresh contexts', async () => { - const script = getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts'); + const script = getScript(SAMPLE_BUNDLE, 'workflows/a.ts'); const results: string[] = []; for (let i = 0; i < 3; i++) { const { context } = createContext({ seed, fixedTimestamp }); - runCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts', context); + runScript(SAMPLE_BUNDLE, 'workflows/a.ts', context); // The same cached Script object is used every iteration. - expect(getCachedWorkflowScript(SAMPLE_BUNDLE, 'workflows/a.ts')).toBe( - script - ); + expect(getScript(SAMPLE_BUNDLE, 'workflows/a.ts')).toBe(script); const fn = runInContext( `globalThis.__private_workflows?.get('my/workflow')`, context @@ -119,7 +133,7 @@ describe('script-cache', () => { const editCount = 100; const filename = 'workflows/a.ts'; for (let i = 0; i < editCount; i++) { - getCachedWorkflowScript(buildBundle(`edit-${i}`), filename); + getScript(buildBundle(`edit-${i}`), filename); } const size = workflowScriptCacheSize(); @@ -130,9 +144,7 @@ describe('script-cache', () => { // The cache still serves correctly after heavy churn: the most-recently // inserted bundle is retained and repeated lookups return the same Script. const latest = buildBundle(`edit-${editCount - 1}`); - expect(getCachedWorkflowScript(latest, filename)).toBe( - getCachedWorkflowScript(latest, filename) - ); + expect(getScript(latest, filename)).toBe(getScript(latest, filename)); }); it('keeps the most-recently-used bundle and evicts the stale one', () => { @@ -141,18 +153,18 @@ describe('script-cache', () => { // unrelated bundles churn through. LRU must NOT evict the bundle we keep // using, even though it was inserted first. const hot = buildBundle('hot'); - const hotScript = getCachedWorkflowScript(hot, filename); + const hotScript = getScript(hot, filename); for (let i = 0; i < 50; i++) { - getCachedWorkflowScript(buildBundle(`cold-${i}`), filename); + getScript(buildBundle(`cold-${i}`), filename); // Re-access the hot bundle so it stays most-recently-used. - expect(getCachedWorkflowScript(hot, filename)).toBe(hotScript); + expect(getScript(hot, filename)).toBe(hotScript); } // After all that churn the hot bundle is still the *same* cached Script — // proving LRU recency (touch-on-access), not mere insertion order, governs // eviction. - expect(getCachedWorkflowScript(hot, filename)).toBe(hotScript); + expect(getScript(hot, filename)).toBe(hotScript); }); it('never returns the wrong Script across realistic multi-workflow bundles', async () => { @@ -165,10 +177,10 @@ describe('script-cache', () => { const fileA = 'workflows/a.ts'; const fileB = 'workflows/b.ts'; - const xa = getCachedWorkflowScript(bundleX, fileA); - const xb = getCachedWorkflowScript(bundleX, fileB); - const ya = getCachedWorkflowScript(bundleY, fileA); - const yb = getCachedWorkflowScript(bundleY, fileB); + const xa = getScript(bundleX, fileA); + const xb = getScript(bundleX, fileB); + const ya = getScript(bundleY, fileA); + const yb = getScript(bundleY, fileB); // All four (code, filename) combinations are distinct Script objects. const scripts = [xa, xb, ya, yb]; @@ -179,12 +191,12 @@ describe('script-cache', () => { } // Same (code, filename) is stable across lookups. - expect(getCachedWorkflowScript(bundleX, fileA)).toBe(xa); - expect(getCachedWorkflowScript(bundleY, fileB)).toBe(yb); + expect(getScript(bundleX, fileA)).toBe(xa); + expect(getScript(bundleY, fileB)).toBe(yb); // Running each bundle yields its OWN marker, confirming no cross-wiring. const { context: ctxX } = createContext({ seed, fixedTimestamp }); - runCachedWorkflowScript(bundleX, fileA, ctxX); + runScript(bundleX, fileA, ctxX); const fnX = runInContext( `globalThis.__private_workflows?.get('app/workflow-3')`, ctxX @@ -192,7 +204,7 @@ describe('script-cache', () => { expect(await fnX('z')).toContain('bundle-X:3:z'); const { context: ctxY } = createContext({ seed, fixedTimestamp }); - runCachedWorkflowScript(bundleY, fileA, ctxY); + runScript(bundleY, fileA, ctxY); const fnY = runInContext( `globalThis.__private_workflows?.get('app/workflow-3')`, ctxY diff --git a/packages/core/src/vm/script-cache.ts b/packages/core/src/vm/script-cache.ts index fe69884523..09db402497 100644 --- a/packages/core/src/vm/script-cache.ts +++ b/packages/core/src/vm/script-cache.ts @@ -1,4 +1,4 @@ -import { type Context, Script } from 'node:vm'; +import { Script } from 'node:vm'; import { globalSingleton } from '@workflow/utils'; /** @@ -107,7 +107,7 @@ function touchBundle(code: string): Map | undefined { export function getCachedWorkflowScript( code: string, filename: string -): Script { +): { script: Script; cacheHit: boolean } { let byFilename = touchBundle(code); if (byFilename === undefined) { byFilename = new Map(); @@ -123,23 +123,12 @@ export function getCachedWorkflowScript( } } let script = byFilename.get(filename); + const cacheHit = script !== undefined; if (script === undefined) { script = new Script(code, { filename }); byFilename.set(filename, script); } - return script; -} - -/** - * Runs the cached workflow-bundle `Script` against `context`. Compiles and - * caches the `Script` on first use for the given `(code, filename)`. - */ -export function runCachedWorkflowScript( - code: string, - filename: string, - context: Context -): unknown { - return getCachedWorkflowScript(code, filename).runInContext(context); + return { script, cacheHit }; } /** diff --git a/packages/core/src/workflow-tracing.test.ts b/packages/core/src/workflow-tracing.test.ts new file mode 100644 index 0000000000..730868451d --- /dev/null +++ b/packages/core/src/workflow-tracing.test.ts @@ -0,0 +1,228 @@ +import { + context, + trace as otelTrace, + SpanStatusCode, +} from '@opentelemetry/api'; +import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks'; +import { + BasicTracerProvider, + InMemorySpanExporter, + SimpleSpanProcessor, +} from '@opentelemetry/sdk-trace-base'; +import type { Event, WorkflowRun } from '@workflow/world'; +import { + afterAll, + afterEach, + assert, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; +import { ReplayPayloadCache } from './replay-payload-cache.js'; +import { + dehydrateStepReturnValue, + dehydrateWorkflowArguments, +} from './serialization.js'; +import { createContext } from './vm/index.js'; +import { clearWorkflowScriptCache } from './vm/script-cache.js'; +import { replayWorkflow, resumeWorkflow, runWorkflow } from './workflow.js'; + +vi.mock('./vm/index.js', async (importActual) => { + const actual = await importActual(); + return { ...actual, createContext: vi.fn(actual.createContext) }; +}); + +const exporter = new InMemorySpanExporter(); +const provider = new BasicTracerProvider(); +const contextManager = new AsyncLocalStorageContextManager(); + +beforeAll(() => { + provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); + contextManager.enable(); + context.setGlobalContextManager(contextManager); + otelTrace.setGlobalTracerProvider(provider); +}); + +afterAll(async () => { + await provider.shutdown(); + context.disable(); + otelTrace.disable(); +}); + +beforeEach(() => { + clearWorkflowScriptCache(); +}); + +afterEach(() => { + exporter.reset(); + vi.restoreAllMocks(); +}); + +async function makeRun(): Promise { + const runId = 'wrun_trace_replay'; + return { + runId, + workflowName: 'workflow', + status: 'running', + input: await dehydrateWorkflowArguments(['hello'], runId, undefined, []), + createdAt: new Date('2024-01-01T00:00:00.000Z'), + updatedAt: new Date('2024-01-01T00:00:00.000Z'), + startedAt: new Date('2024-01-01T00:00:00.000Z'), + deploymentId: 'test-deployment', + }; +} + +function spans(name: string) { + return exporter.getFinishedSpans().filter((span) => span.name === name); +} + +const workflowCode = ` +async function workflow(value) { return value; } +globalThis.__private_workflows = new Map(); +globalThis.__private_workflows.set('workflow', workflow); +`; + +describe('fresh replay tracing', () => { + it('breaks workflow.run into blocking replay phases', async () => { + const run = await makeRun(); + await runWorkflow(workflowCode, run, [], undefined); + + const allSpans = exporter.getFinishedSpans(); + const [workflowRun] = spans('workflow.run workflow'); + expect(workflowRun).toBeDefined(); + + const childNames = allSpans + .filter((span) => span.parentSpanId === workflowRun?.spanContext().spanId) + .map((span) => span.name); + expect(childNames).toEqual( + expect.arrayContaining([ + 'workflow.vm.create_context', + 'workflow.bundle.compile', + 'workflow.bundle.evaluate', + 'workflow.input.hydrate', + 'workflow.replay.execute', + ]) + ); + }); + + it('records VM bootstrap failures on the create-context span', async () => { + vi.mocked(createContext).mockImplementationOnce(() => { + throw new Error('test bootstrap failure'); + }); + + await expect( + runWorkflow(workflowCode, await makeRun(), [], undefined) + ).rejects.toThrow('test bootstrap failure'); + + expect(spans('workflow.vm.create_context')[0]?.status).toEqual({ + code: SpanStatusCode.ERROR, + message: 'test bootstrap failure', + }); + }); + + it('ends the replay span when workflow code throws null', async () => { + const nullThrowingWorkflow = ` +async function workflow() { throw null; } +globalThis.__private_workflows = new Map([['workflow', workflow]]); +`; + + await expect( + runWorkflow(nullThrowingWorkflow, await makeRun(), [], undefined) + ).rejects.toBeNull(); + + expect(spans('workflow.replay.execute')[0]?.status).toEqual({ + code: SpanStatusCode.ERROR, + message: 'null', + }); + }); + + it('parents retained workflow continuations to the retained run', async () => { + const run = await makeRun(); + const code = `const step = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step"); + async function workflow() { await step(); console.log("resumed"); } + globalThis.__private_workflows = new Map([["workflow", workflow]]); + `; + const activeSpanIds: (string | undefined)[] = []; + vi.spyOn(console, 'log').mockImplementation((message) => { + if (message === 'resumed') { + activeSpanIds.push(otelTrace.getActiveSpan()?.spanContext().spanId); + } + }); + + const first = await replayWorkflow({ + workflowCode: code, + workflowRun: run, + events: [], + encryptionKey: undefined, + replayPayloadCache: new ReplayPayloadCache(undefined), + }); + assert(first.type === 'suspended'); + expect(spans('workflow.replay.execute')).toHaveLength(1); + const step = first.suspension.steps[0]; + assert(step?.type === 'step'); + const result = await dehydrateStepReturnValue( + undefined, + run.runId, + undefined + ); + + const completed = await resumeWorkflow(first.session, [ + { + eventId: 'event-step-completed', + runId: run.runId, + eventType: 'step_completed', + correlationId: step.correlationId, + eventData: { stepName: 'step', result }, + createdAt: run.updatedAt, + }, + ] as Event[]); + assert(completed.type === 'completed'); + + expect(spans('workflow.replay.execute')).toHaveLength(1); + const retainedRun = spans('workflow.run workflow').find( + (span) => span.attributes['workflow.execution.mode'] === 'retained' + ); + expect(activeSpanIds).toEqual([retainedRun?.spanContext().spanId]); + }); + + it('marks bundle compilation cache hits on later fresh replays', async () => { + const run = await makeRun(); + await runWorkflow(workflowCode, run, [], undefined); + await runWorkflow(workflowCode, run, [], undefined); + + const compileSpans = spans('workflow.bundle.compile'); + expect(compileSpans).toHaveLength(2); + expect( + compileSpans.map( + (span) => span.attributes['workflow.bundle.compile.cache_hit'] + ) + ).toEqual([false, true]); + }); + + it('reports a bundle hit when only a different workflow lookup compiles', async () => { + const firstName = 'workflow//./workflows/shared//first'; + const secondName = 'workflow//./workflows/shared//second'; + const sharedBundle = ` +async function first(value) { return value; } +async function second(value) { return value; } +globalThis.__private_workflows = new Map(); +globalThis.__private_workflows.set(${JSON.stringify(firstName)}, first); +globalThis.__private_workflows.set(${JSON.stringify(secondName)}, second); +`; + const firstRun = { ...(await makeRun()), workflowName: firstName }; + const secondRun = { ...(await makeRun()), workflowName: secondName }; + + await runWorkflow(sharedBundle, firstRun, [], undefined); + await runWorkflow(sharedBundle, secondRun, [], undefined); + + const compileSpans = spans('workflow.bundle.compile'); + expect( + compileSpans.map( + (span) => span.attributes['workflow.bundle.compile.cache_hit'] + ) + ).toEqual([false, true]); + }); +}); diff --git a/packages/core/src/workflow.ts b/packages/core/src/workflow.ts index 0a158b58f9..f138806fd9 100644 --- a/packages/core/src/workflow.ts +++ b/packages/core/src/workflow.ts @@ -42,10 +42,15 @@ import { WORKFLOW_USE_STEP, } from './symbols.js'; import * as Attribute from './telemetry/semantic-conventions.js'; -import { applyWorkflowSuspensionToSpan, trace } from './telemetry.js'; +import { + applyWorkflowSuspensionToSpan, + createRefreshableTraceContext, + startTraceSpan, + trace, +} from './telemetry.js'; import { getWorkflowRunStreamId } from './util.js'; import { createContext } from './vm/index.js'; -import { runCachedWorkflowScript } from './vm/script-cache.js'; +import { getCachedWorkflowScript } from './vm/script-cache.js'; import { createAbortSignalStatics, createCreateAbortController, @@ -305,15 +310,26 @@ export async function runWorkflow( return result.output; } -async function createWorkflowSession({ - workflowCode, - workflowRun, - events, - encryptionKey, - replayPayloadCache, - runReadyBarrier, - worldCapabilities, -}: WorkflowSessionOptions): Promise<{ +async function createWorkflowSession(options: WorkflowSessionOptions) { + const vmTrace = await startTraceSpan('workflow.vm.create_context'); + return createWorkflowSessionInner(options, vmTrace.end).catch((error) => { + vmTrace.fail(error); + throw error; + }); +} + +async function createWorkflowSessionInner( + { + workflowCode, + workflowRun, + events, + encryptionKey, + replayPayloadCache, + runReadyBarrier, + worldCapabilities, + }: WorkflowSessionOptions, + endVmTrace: () => void +): Promise<{ session: WorkflowSession; execution: Promise; }> { @@ -344,7 +360,10 @@ async function createWorkflowSession({ ? `https://${process.env.VERCEL_URL}` : `http://localhost:${(await getPortLazy()) ?? 3000}` ); - + // Include both node:vm's context creation and the host-side sandbox wiring + // below. Most of the bootstrap lives in this function (EventsConsumer, + // workflow globals, Web API shims), so tracing createContext() alone would + // materially under-report VM startup. const { context, globalThis: vmGlobalThis, @@ -1071,22 +1090,40 @@ async function createWorkflowSession({ vmGlobalThis[SYMBOL_FOR_REQ_CONTEXT] = (globalThis as any)[ SYMBOL_FOR_REQ_CONTEXT ]; + endVmTrace(); // Get a reference to the user-defined workflow function. // The filename parameter ensures stack traces show a meaningful name // (e.g., "example/workflows/99_e2e.ts") instead of "evalmachine.". const parsedName = parseWorkflowName(workflowRun.workflowName); const filename = parsedName?.moduleSpecifier || workflowRun.workflowName; + const workflowLookupCode = `globalThis.__private_workflows?.get(${JSON.stringify(workflowRun.workflowName)})`; // Reuse compiled scripts by `(code, filename)`: compilation is deterministic // and the filename preserves workflow source attribution in stack traces. // The bundle registers workflows on `globalThis.__private_workflows`. - runCachedWorkflowScript(workflowCode, filename, context); - const workflowFn = runCachedWorkflowScript( - `globalThis.__private_workflows?.get(${JSON.stringify(workflowRun.workflowName)})`, - filename, - context + const { bundleScript, workflowLookupScript } = await trace( + 'workflow.bundle.compile', + async (span) => { + const bundle = getCachedWorkflowScript(workflowCode, filename); + const lookup = getCachedWorkflowScript(workflowLookupCode, filename); + span?.setAttributes({ + // This attribute intentionally describes the workflow bundle. The + // tiny workflow-name lookup script has its own cache entry and may + // miss when another workflow from the same source file runs, but that + // does not mean V8 recompiled the application bundle. + ...Attribute.WorkflowBundleCompileCacheHit(bundle.cacheHit), + }); + return { + bundleScript: bundle.script, + workflowLookupScript: lookup.script, + }; + } ); + const workflowFn = await trace('workflow.bundle.evaluate', async () => { + bundleScript.runInContext(context); + return workflowLookupScript.runInContext(context); + }); if (typeof workflowFn !== 'function') { throw new WorkflowNotRegisteredError(workflowRun.workflowName); @@ -1098,25 +1135,23 @@ async function createWorkflowSession({ // workflow function subscribing its first step callbacks. let args: unknown[] = []; workflowContext.promiseQueue = workflowContext.promiseQueue.then(async () => { - const prepared = await replayPayloadCache.prepareWorkflowInput(workflowRun); - args = await hydrateWorkflowArguments( - workflowRun.input, - workflowRun.runId, - encryptionKey, - vmGlobalThis, - {}, - prepared - ); + // Include any residual payload preparation plus VM-local deserialization + // in the blocking boundary. + args = await trace('workflow.input.hydrate', async () => { + const prepared = + await replayPayloadCache.prepareWorkflowInput(workflowRun); + return hydrateWorkflowArguments( + workflowRun.input, + workflowRun.runId, + encryptionKey, + vmGlobalThis, + {}, + prepared + ); + }); }); await workflowContext.promiseQueue; - // The user function's promise. It may stay pending across many resumes - // (each parked step promise holds it up) and is raced against the current - // attempt's interruption in waitForExecution. - const workflowBody = (async (): Promise => { - return await workflowFn(...args); - })(); - const failWorkflow = async (error: unknown): Promise => { // Control-flow signals are handled by the runtime and do not mean the // workflow has terminally failed. `onWorkflowError` usually already moved @@ -1143,6 +1178,7 @@ async function createWorkflowSession({ }; const waitForExecution = async ( + workflowBody: Promise, interruption: PromiseWithResolvers ): Promise => { let result: unknown; @@ -1205,6 +1241,12 @@ async function createWorkflowSession({ } }; + const workflowTraceContext = await createRefreshableTraceContext(); + const replayTrace = await startTraceSpan('workflow.replay.execute'); + const workflowBody = workflowTraceContext.run(async () => + workflowFn(...args) + ); + const session: WorkflowSession = { workflowRun, argumentCount: args.length, @@ -1229,8 +1271,9 @@ async function createWorkflowSession({ const interruption = withResolvers(); state = { type: 'running', interruption }; workflowContext.suspensionGeneration++; + workflowTraceContext.refresh(); eventsConsumer.append(nextEvents.slice(knownEvents.length)); - return waitForExecution(interruption); + return waitForExecution(workflowBody, interruption); } case 'replay': return { type: 'replay' }; @@ -1244,8 +1287,14 @@ async function createWorkflowSession({ }, }; + // The replay span measures the user function without becoming its ambient + // context. The workflow promise stays pending across retained resumes, so an + // active replay span here would remain captured after that span has ended. + const execution = waitForExecution(workflowBody, initialInterruption); + void execution.then(replayTrace.end, replayTrace.fail); + return { session, - execution: waitForExecution(initialInterruption), + execution, }; } From 4f6cc69eb127fa1340f48943799b83de44a394c0 Mon Sep 17 00:00:00 2001 From: Karthik Kalyan <105607645+karthikscale3@users.noreply.github.com> Date: Mon, 31 Aug 2026 12:31:01 -0700 Subject: [PATCH 5/8] Prevent payload flicker during decryption (#3906) --- .changeset/calm-events-wait.md | 5 +++++ packages/web-shared/src/components/event-list-view.tsx | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 .changeset/calm-events-wait.md diff --git a/.changeset/calm-events-wait.md b/.changeset/calm-events-wait.md new file mode 100644 index 0000000000..a28c60352d --- /dev/null +++ b/.changeset/calm-events-wait.md @@ -0,0 +1,5 @@ +--- +'@workflow/web-shared': patch +--- + +Prevent event payload flicker during decryption. diff --git a/packages/web-shared/src/components/event-list-view.tsx b/packages/web-shared/src/components/event-list-view.tsx index f8b90fb40e..87f71cb09a 100644 --- a/packages/web-shared/src/components/event-list-view.tsx +++ b/packages/web-shared/src/components/event-list-view.tsx @@ -1007,10 +1007,10 @@ export function EventRow({ }, []); // When encryption key changes and this event was previously loaded, - // re-load to get decrypted data + // re-load to get decrypted data. Keep the encrypted value visible until the + // refreshed data arrives so the payload does not flash empty while decrypting. useEffect(() => { if (encryptionKey && hasAttemptedLoad && onLoadEventData) { - setLoadedEventData(null); setHasAttemptedLoad(false); onLoadEventData(event) .then((data) => { From ee6f917cdbfcf50a5fd697c7a9cb70dd1294f931 Mon Sep 17 00:00:00 2001 From: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> Date: Mon, 31 Aug 2026 14:10:40 -0700 Subject: [PATCH 6/8] [core] Overlap workflow compilation with replay loading (#3798) * Overlap workflow compile with replay loading Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> * Fix replay compilation scheduling --------- Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> --- .changeset/overlap-workflow-compile.md | 5 ++ packages/core/src/runtime-trace-mode.test.ts | 78 +++++++++++++++++++- packages/core/src/runtime.ts | 48 +++++++++++- packages/core/src/runtime/vm-mode.ts | 4 +- packages/core/src/vm/script-cache.ts | 53 ++++++------- packages/core/src/workflow.ts | 69 ++++++++++------- 6 files changed, 193 insertions(+), 64 deletions(-) create mode 100644 .changeset/overlap-workflow-compile.md diff --git a/.changeset/overlap-workflow-compile.md b/.changeset/overlap-workflow-compile.md new file mode 100644 index 0000000000..ec1dd241f7 --- /dev/null +++ b/.changeset/overlap-workflow-compile.md @@ -0,0 +1,5 @@ +--- +"@workflow/core": patch +--- + +Compile Node.js workflow scripts while loading the replay event log. diff --git a/packages/core/src/runtime-trace-mode.test.ts b/packages/core/src/runtime-trace-mode.test.ts index 22721b4961..1d2d83389e 100644 --- a/packages/core/src/runtime-trace-mode.test.ts +++ b/packages/core/src/runtime-trace-mode.test.ts @@ -81,7 +81,10 @@ const simpleWorkflow = `async function workflow() { return 'done'; }${getWorkflowTransformCode('workflow')}`; -async function makeRunningRun(runId: string): Promise { +async function makeRunningRun( + runId: string, + executionContext?: WorkflowRun['executionContext'] +): Promise { return { runId, workflowName: 'workflow', @@ -91,6 +94,7 @@ async function makeRunningRun(runId: string): Promise { updatedAt: new Date('2024-01-01T00:00:00.000Z'), startedAt: new Date('2024-01-01T00:00:00.000Z'), deploymentId: 'test-deployment', + executionContext, }; } @@ -105,12 +109,16 @@ async function driveHandler(opts: { workflowCode: string; traceCarrier?: Record; routeModuleBodyStartedAt?: number; + includeRunInput?: boolean; + executionContext?: WorkflowRun['executionContext']; + whileRunStartedPending?: () => Promise; }) { - const workflowRun = await makeRunningRun(opts.runId); + const workflowRun = await makeRunningRun(opts.runId, opts.executionContext); const queuedMessages: any[] = []; const eventsCreate = vi.fn(async (_runId: string, data: any) => { if (data.eventType === 'run_started') { + await opts.whileRunStartedPending?.(); return { run: workflowRun, events: [] as Event[] }; } return { @@ -136,10 +144,23 @@ async function driveHandler(opts: { runId: workflowRun.runId, requestedAt: new Date('2024-01-01T00:00:00.000Z'), traceCarrier: opts.traceCarrier, + ...(opts.includeRunInput + ? { + runInput: { + input: workflowRun.input, + deploymentId: workflowRun.deploymentId, + workflowName: workflowRun.workflowName, + specVersion: SPEC_VERSION_CURRENT, + executionContext: workflowRun.executionContext, + }, + } + : {}), }, { requestId: 'req_test', - attempt: 1, + // Keep this trace harness on the awaited run_started path even + // when a test supplies runInput for pre-response VM selection. + attempt: opts.includeRunInput ? 2 : 1, queueName: '__wkf_workflow_workflow', messageId: 'msg_test', } @@ -246,6 +267,57 @@ describe('getWorkflowTraceMode', () => { }); describe('workflowEntrypoint trace modes', () => { + it('compiles while run_started is loading, without evaluating early', async () => { + let observedOverlap = false; + await driveHandler({ + runId: 'wrun_trace_compile_overlap', + workflowCode: simpleWorkflow, + includeRunInput: true, + whileRunStartedPending: async () => { + expect( + exporter + .getFinishedSpans() + .find((span) => span.name === 'workflow.bundle.evaluate') + ).toBeUndefined(); + await vi.waitFor(() => { + expect( + exporter + .getFinishedSpans() + .find((span) => span.name === 'workflow.bundle.compile') + ).toBeDefined(); + }); + expect( + exporter + .getFinishedSpans() + .find((span) => span.name === 'workflow.bundle.evaluate') + ).toBeUndefined(); + observedOverlap = true; + }, + }); + + expect(observedOverlap).toBe(true); + expect( + exporter + .getFinishedSpans() + .find((span) => span.name === 'workflow.bundle.evaluate') + ).toBeDefined(); + }); + + it('does not compile a Node bundle for a known QuickJS run', async () => { + await driveHandler({ + runId: 'wrun_trace_quickjs_compile', + workflowCode: simpleWorkflow, + includeRunInput: true, + executionContext: { workflowVm: 'quickjs' }, + }); + + expect( + exporter + .getFinishedSpans() + .find((span) => span.name === 'workflow.bundle.compile') + ).toBeUndefined(); + }); + it('linked (default): nests under the flow route context with a link to the run-origin context', async () => { const { workflowSpan, diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index 6cc4ce4415..15e756f679 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -134,6 +134,7 @@ import { import { getErrorName, getErrorStack, normalizeUnknownError } from './types.js'; import { buildWorkflowSuspensionMessage } from './util.js'; import { + compileWorkflowBundle, replayWorkflow, resumeWorkflow, type WorkflowResumeResult, @@ -948,6 +949,26 @@ export function workflowEntrypoint( const replayRecoveryReporter = replayDivergence ? new ReplayRecoveryReporter(replayDivergence.count) : ReplayRecoveryReporter.inert(); + // Compilation is useful only for the Node VM. Wait until the + // run's engine selection is known so QuickJS deliveries never + // parse and cache an unused node:vm Script. The promise is + // invocation-scoped and reused by every cold replay; + // evaluation still waits for a fresh VM context. + let compiledWorkflowScripts: + | ReturnType + | undefined; + const startWorkflowCompile = ( + workflow?: Pick + ) => { + if (!workflow || useQuickJSVm(workflow)) return; + compiledWorkflowScripts ??= compileWorkflowBundle( + workflowCode, + workflowName + ); + // Terminal runs can return without awaiting compilation. + void compiledWorkflowScripts.catch(() => {}); + return compiledWorkflowScripts; + }; // Every write this loop makes carries the cursor of the log // it was computed against, and folds a complete returned // delta into that log. @@ -1892,6 +1913,11 @@ export function workflowEntrypoint( // All steps done: fall through to the main replay loop. // Set up shared state so the loop can continue. + // The step body itself needs no workflow VM. Start Node + // compilation only now, once this delivery is known to + // continue into a workflow replay rather than return for + // a still-pending sibling. + startWorkflowCompile(bgRun); runtimeLogger.debug( 'All parallel steps done, replaying inline after background step', { workflowRunId: runId } @@ -2052,7 +2078,7 @@ export function workflowEntrypoint( span?.addEvent('workflow.hook_received.create.start', { 'workflow.hook_received.preload_events': true, }); - const result = await traceReplayLoad('hook_preload', () => + const replayLoad = traceReplayLoad('hook_preload', () => createEvent( { eventType: 'hook_received', @@ -2072,6 +2098,7 @@ export function workflowEntrypoint( } ) ); + const result = await replayLoad; hookEnsured = true; // Note: unlike the re-ensure below, this hoisted write // does NOT set HookResilientResumeMaterialized: it @@ -2161,6 +2188,7 @@ export function workflowEntrypoint( return; } workflowRun = result.run; + startWorkflowCompile(workflowRun); maxEventsLimit = clampMaxEvents(result.maxEvents); // Anchors RSFS, see the declaration above. This // response plays run_started's role on this path. @@ -2276,6 +2304,7 @@ export function workflowEntrypoint( // wasted list+resolve it would otherwise compute. { requestId, skipPreload: true } ); + startWorkflowCompile(runInput); runReadyBarrier = startedPromise; // Turbo backgrounds run_started, so the non-turbo // assignment below never runs. Thread the per-run event @@ -2344,10 +2373,14 @@ export function workflowEntrypoint( span?.addEvent('workflow.run_started.create.start', { 'workflow.run_started.skip_preload': false, }); - const result = await traceReplayLoad( - 'run_started', - () => createEvent(runStartedEvent, { requestId }) + const replayLoad = traceReplayLoad('run_started', () => + createEvent(runStartedEvent, { requestId }) ); + // Initial deliveries carry runInput, so Node compilation + // can overlap this load without guessing the VM engine. + // Continuations learn the engine from result.run below. + startWorkflowCompile(runInput); + const result = await replayLoad; workflowRun = result.run; maxEventsLimit = clampMaxEvents(result.maxEvents); // Anchors RSFS, see the declaration above. @@ -2392,6 +2425,7 @@ export function workflowEntrypoint( return; } + startWorkflowCompile(workflowRun); } catch (err) { // Run was concurrently completed/failed/canceled if ( @@ -3084,12 +3118,18 @@ export function workflowEntrypoint( if (workflowResult.type === 'replay') { retainedSession = null; + const compiled = startWorkflowCompile(workflowRun); + assert( + compiled, + 'Node workflow replay requires compiled scripts' + ); workflowResult = await replayWorkflow({ workflowCode, workflowRun, events: eventLog.events, encryptionKey, replayPayloadCache, + compiledWorkflowScripts: await compiled, // Turbo: the end-of-run drain inside workflow // execution commits fire-and-forget `*_created` // events before the terminal `awaitRunReady()` below. diff --git a/packages/core/src/runtime/vm-mode.ts b/packages/core/src/runtime/vm-mode.ts index 1ea4349af7..876bd56d4e 100644 --- a/packages/core/src/runtime/vm-mode.ts +++ b/packages/core/src/runtime/vm-mode.ts @@ -58,7 +58,9 @@ export function getWorkflowVmFromEnv( * Throws if `WORKFLOW_VM` or `executionContext.workflowVm` is set to an * unknown value. */ -export function useQuickJSVm(workflowRun: WorkflowRun): boolean { +export function useQuickJSVm( + workflowRun: Pick +): boolean { const vmFromRun = ( workflowRun.executionContext as { workflowVm?: string } | undefined )?.workflowVm; diff --git a/packages/core/src/vm/script-cache.ts b/packages/core/src/vm/script-cache.ts index 09db402497..f1f6da93a9 100644 --- a/packages/core/src/vm/script-cache.ts +++ b/packages/core/src/vm/script-cache.ts @@ -39,27 +39,21 @@ import { globalSingleton } from '@workflow/utils'; * function bodies the duplicated work is the (cheap) top-level parse, not full * per-workflow codegen. * - * We use a nested Map (code -> filename -> Script) so that evicting a bundle - * (e.g. a new deployment/hot-reload producing a different `code`) drops the old - * code string and all of its per-filename scripts together. + * We use a nested Map (code -> filename -> Script) so that evicting a source + * string drops all of its per-filename scripts together. Most entries are full + * workflow bundles; `compileWorkflowBundle` also caches its tiny workflow-name + * lookup snippets here. * * Bounding * -------- * The top-level (`code`-keyed) map is an insertion-ordered LRU capped at - * `MAX_BUNDLES` entries. In production this bound is never reached: a - * deployment is its own process serving exactly one build-time bundle literal - * (skew protection runs old versions as separate processes), so there is a - * single `code` key for the process lifetime. The bound exists for dev/watch - * mode, where the dev route re-reads `workflowCode` from disk and re-invokes - * the entrypoint on every edit: each edit produces a NEW bundle string, which - * without a bound would pin every historical version forever (~0.8MB per edit, - * growing monotonically with edit count). The dev path only ever needs the - * latest bundle, so an LRU that keeps the few most-recent bundles and evicts - * the rest preserves the pre-cache GC behaviour while still serving the - * steady-state single-bundle case for free. The per-`filename` inner map is not - * separately bounded: it is naturally bounded by the (small) number of workflow - * source files in a bundle and is dropped wholesale when its parent `code` - * entry is evicted. + * `MAX_SCRIPT_SOURCES` entries. A production deployment has one large bundle + * source plus small lookup sources; the bundle is touched immediately before + * its lookup on every compilation, so lookup churn cannot evict the expensive + * entry in normal use. The bound primarily protects dev/watch mode, where every + * edit produces a new bundle string that would otherwise pin all historical + * versions. The per-`filename` inner map is naturally bounded by the workflows + * compiled from that source and is dropped wholesale with its parent entry. */ // On `globalThis` (see `globalSingleton`): compiling a bundle is the expensive // part this cache exists to skip, and per-copy caches would pay it once per @@ -69,13 +63,10 @@ const scripts = globalSingleton('@workflow/core//vmScriptCache', 1, () => ({ })); /** - * Max number of distinct bundle (`code`) versions to retain. One is enough for - * production; a handful covers pathological dev hot-reload / repeated-rebuild - * churn within a single long-lived process (e.g. a watch session or a test - * file) without unbounded growth. Kept deliberately small: there is no value - * in retaining stale bundles, only a memory cost. + * Maximum number of distinct script source strings to retain. Kept deliberately + * small because stale bundles and one-off lookup snippets have no lasting value. */ -const MAX_BUNDLES = 8; +const MAX_SCRIPT_SOURCES = 8; /** * Looks up the per-filename map for `code`, marking it most-recently-used. @@ -83,7 +74,7 @@ const MAX_BUNDLES = 8; * existing key moves it to the end (newest), so the first key is always the * least-recently-used eviction candidate. */ -function touchBundle(code: string): Map | undefined { +function touchScriptSource(code: string): Map | undefined { const byFilename = scripts.byCode.get(code); if (byFilename === undefined) { return undefined; @@ -95,9 +86,9 @@ function touchBundle(code: string): Map | undefined { } /** - * Returns a compiled `vm.Script` for the given workflow bundle code and - * filename, compiling and caching it on first use. Subsequent calls with the - * same `(code, filename)` return the cached `Script`. + * Returns a compiled `vm.Script` for the given source code and filename, + * compiling and caching it on first use. Subsequent calls with the same + * `(code, filename)` return the cached `Script`. * * The returned `Script` is not yet bound to any context; the caller runs it * against a specific VM context via `script.runInContext(context)`. This is @@ -108,13 +99,13 @@ export function getCachedWorkflowScript( code: string, filename: string ): { script: Script; cacheHit: boolean } { - let byFilename = touchBundle(code); + let byFilename = touchScriptSource(code); if (byFilename === undefined) { byFilename = new Map(); scripts.byCode.set(code, byFilename); - // Evict the least-recently-used bundle(s) when over the cap. New bundles + // Evict the least-recently-used source(s) when over the cap. New sources // are appended at the end, so the oldest live at the front. - while (scripts.byCode.size > MAX_BUNDLES) { + while (scripts.byCode.size > MAX_SCRIPT_SOURCES) { const oldest = scripts.byCode.keys().next().value; if (oldest === undefined) { break; @@ -140,7 +131,7 @@ export function clearWorkflowScriptCache(): void { } /** - * Number of distinct bundle (`code`) versions currently retained. Exposed for + * Number of distinct script source strings currently retained. Exposed for * tests asserting the LRU bound; not used on the hot path. */ export function workflowScriptCacheSize(): number { diff --git a/packages/core/src/workflow.ts b/packages/core/src/workflow.ts index f138806fd9..ac97eb94b6 100644 --- a/packages/core/src/workflow.ts +++ b/packages/core/src/workflow.ts @@ -1,3 +1,4 @@ +import type { Script } from 'node:vm'; import type { Span } from '@opentelemetry/api'; import { ERROR_SLUGS, @@ -130,10 +131,49 @@ interface WorkflowSessionOptions { readonly events: Event[]; readonly encryptionKey: PayloadKey | undefined; readonly replayPayloadCache: ReplayPayloadCache; + readonly compiledWorkflowScripts?: CompiledWorkflowScripts; readonly runReadyBarrier?: Promise; readonly worldCapabilities?: WorldCapabilities; } +/** Context-independent V8 scripts that can be evaluated in any fresh VM. */ +export interface CompiledWorkflowScripts { + readonly bundleScript: Script; + readonly workflowLookupScript: Script; +} + +/** + * Compile the workflow bundle before its run snapshot is available. + * + * Compilation depends only on the route's bundle string and workflow name, + * not the event log or VM context. The runtime starts this promise while + * `run_started` loads the replay snapshot, then evaluates the scripts only + * after it has created the fresh context. + */ +export function compileWorkflowBundle( + workflowCode: string, + workflowName: string +): Promise { + const parsedName = parseWorkflowName(workflowName); + const filename = parsedName?.moduleSpecifier || workflowName; + const workflowLookupCode = `globalThis.__private_workflows?.get(${JSON.stringify(workflowName)})`; + + return trace('workflow.bundle.compile', async (span) => { + const bundle = getCachedWorkflowScript(workflowCode, filename); + const lookup = getCachedWorkflowScript(workflowLookupCode, filename); + span?.setAttributes({ + // This attribute intentionally describes the workflow bundle. The tiny + // lookup script may miss when another workflow from the same source file + // runs, but that does not mean V8 recompiled the application bundle. + ...Attribute.WorkflowBundleCompileCacheHit(bundle.cacheHit), + }); + return { + bundleScript: bundle.script, + workflowLookupScript: lookup.script, + }; + }); +} + /** * A live workflow VM, parked at a suspension boundary. `resume` advances it * by appending events instead of replaying from scratch. @@ -325,6 +365,7 @@ async function createWorkflowSessionInner( events, encryptionKey, replayPayloadCache, + compiledWorkflowScripts, runReadyBarrier, worldCapabilities, }: WorkflowSessionOptions, @@ -1092,34 +1133,12 @@ async function createWorkflowSessionInner( ]; endVmTrace(); - // Get a reference to the user-defined workflow function. - // The filename parameter ensures stack traces show a meaningful name - // (e.g., "example/workflows/99_e2e.ts") instead of "evalmachine.". - const parsedName = parseWorkflowName(workflowRun.workflowName); - const filename = parsedName?.moduleSpecifier || workflowRun.workflowName; - const workflowLookupCode = `globalThis.__private_workflows?.get(${JSON.stringify(workflowRun.workflowName)})`; - // Reuse compiled scripts by `(code, filename)`: compilation is deterministic // and the filename preserves workflow source attribution in stack traces. // The bundle registers workflows on `globalThis.__private_workflows`. - const { bundleScript, workflowLookupScript } = await trace( - 'workflow.bundle.compile', - async (span) => { - const bundle = getCachedWorkflowScript(workflowCode, filename); - const lookup = getCachedWorkflowScript(workflowLookupCode, filename); - span?.setAttributes({ - // This attribute intentionally describes the workflow bundle. The - // tiny workflow-name lookup script has its own cache entry and may - // miss when another workflow from the same source file runs, but that - // does not mean V8 recompiled the application bundle. - ...Attribute.WorkflowBundleCompileCacheHit(bundle.cacheHit), - }); - return { - bundleScript: bundle.script, - workflowLookupScript: lookup.script, - }; - } - ); + const { bundleScript, workflowLookupScript } = + compiledWorkflowScripts ?? + (await compileWorkflowBundle(workflowCode, workflowRun.workflowName)); const workflowFn = await trace('workflow.bundle.evaluate', async () => { bundleScript.runInContext(context); return workflowLookupScript.runInContext(context); From e9d5c56701821b090108a85b74bf8b0cbef8ea8e Mon Sep 17 00:00:00 2001 From: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> Date: Mon, 31 Aug 2026 15:42:46 -0700 Subject: [PATCH 7/8] [core] Prepare replay payloads as event frames arrive (#3548) * Prepare replay payloads from streamed events Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> * Fix streamed replay preparation invariants * fix(core): gate replay startup work by VM engine * refactor(core): simplify replay startup state * fix(core): preserve replay startup ordering * refactor(core): simplify setup failure handling * fix(core): observe replay load after setup failure * refactor(core): simplify replay encryption key promise * fix(core): scan only appended replay events * refactor(world-vercel): type replay stream outcomes --------- Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> --- .../prepare-streamed-replay-payloads.md | 7 + .../core/src/replay-payload-cache.test.ts | 62 +++-- packages/core/src/replay-payload-cache.ts | 140 +++++----- packages/core/src/runtime-trace-mode.test.ts | 175 +++++++++++-- packages/core/src/runtime.test.ts | 34 ++- packages/core/src/runtime.ts | 241 +++++++++++------- packages/core/src/runtime/helpers.test.ts | 2 +- packages/core/src/runtime/helpers.ts | 64 ++--- packages/core/src/telemetry.ts | 8 + packages/core/src/workflow.ts | 8 +- packages/world-vercel/src/event-retry.ts | 13 + packages/world-vercel/src/events-v4.test.ts | 7 +- packages/world-vercel/src/events-v4.ts | 134 +++++++--- packages/world-vercel/src/events.test.ts | 37 +++ packages/world-vercel/src/events.ts | 98 +++---- packages/world/src/events.ts | 7 + 16 files changed, 682 insertions(+), 355 deletions(-) create mode 100644 .changeset/prepare-streamed-replay-payloads.md diff --git a/.changeset/prepare-streamed-replay-payloads.md b/.changeset/prepare-streamed-replay-payloads.md new file mode 100644 index 0000000000..37adc00a88 --- /dev/null +++ b/.changeset/prepare-streamed-replay-payloads.md @@ -0,0 +1,7 @@ +--- +"@workflow/core": patch +"@workflow/world": patch +"@workflow/world-vercel": patch +--- + +Prepare replay payloads as validated event frames arrive, reuse primitive step results across fresh VMs, and preserve replay startup overlap. diff --git a/packages/core/src/replay-payload-cache.test.ts b/packages/core/src/replay-payload-cache.test.ts index 0d3d930619..e1f0da4702 100644 --- a/packages/core/src/replay-payload-cache.test.ts +++ b/packages/core/src/replay-payload-cache.test.ts @@ -55,6 +55,26 @@ function makeEvents(payloads: unknown[]): Event[] { } describe('ReplayPayloadCache', () => { + it('prepares a streamed event as soon as its deferred key resolves', async () => { + const payload = new Uint8Array([1]); + let resolveKey!: (key: undefined) => void; + const key = new Promise((resolve) => { + resolveKey = resolve; + }); + const preparer = vi.fn((value) => ({ data: value })); + const cache = new ReplayPayloadCache(key, preparer); + const [event] = makeEvents([payload]); + + cache.prepareEvent(event); + expect(preparer).not.toHaveBeenCalled(); + + resolveKey(undefined); + await expect( + cache.prepareEventPayload(event.eventId, 'result', payload) + ).resolves.toEqual({ data: payload }); + expect(preparer).toHaveBeenCalledOnce(); + }); + it('deduplicates preparation and accepts a synchronous preparer', async () => { const payload = new Uint8Array([1]); const preparer = vi.fn((value) => ({ data: value })); @@ -103,6 +123,7 @@ describe('ReplayPayloadCache', () => { const events = makeEvents(payloads.slice(1)); const warming = cache.prewarm(run, events); + await Promise.resolve(); expect(preparer).toHaveBeenCalledTimes(4); for (const resolve of resolvers.reverse()) resolve(); await warming; @@ -152,11 +173,7 @@ describe('ReplayPayloadCache', () => { expect(second.count).toBe(0); }); - it('rescans a log whose missing events were filled in below the scanned prefix', async () => { - // A stale-snapshot (412) restart replaces the log with a corrected one, so - // the events it was missing appear BELOW the length already scanned and - // shift every later position. Resuming from that length skips exactly the - // events the reload was for, which is what `resetScan` exists to prevent. + it('rescans events inserted by an authoritative log replacement', async () => { const payloads = [0, 1, 2].map((value) => new Uint8Array([value])); const preparer = vi.fn((value) => ({ data: value })); const cache = new ReplayPayloadCache(undefined, preparer); @@ -166,15 +183,11 @@ describe('ReplayPayloadCache', () => { await cache.prewarm(run, [first, second]); expect(preparer).toHaveBeenCalledTimes(2); - // Positional resume: `missing` sits inside the scanned prefix, so it is - // skipped and its payload is only prepared on demand. await cache.prewarm(run, [first, missing, second]); expect(preparer).toHaveBeenCalledTimes(2); cache.resetScan(); await cache.prewarm(run, [first, missing, second]); - // Only the inserted event is new: the other two are keyed by event id and - // stay prepared across the rescan. expect(preparer).toHaveBeenCalledTimes(3); expect(preparer).toHaveBeenLastCalledWith(payloads[1], undefined); }); @@ -205,21 +218,24 @@ describe('ReplayPayloadCache', () => { } }); - it('rehydrates mutable and oversized step results', async () => { + it('rehydrates mutable step results', async () => { + const cache = new ReplayPayloadCache(undefined); + const hydrate = vi.fn().mockImplementation(async () => ({ count: 0 })); + + const first = await cache.getStepResult('evnt_result', hydrate); + const second = await cache.getStepResult('evnt_result', hydrate); + expect(hydrate).toHaveBeenCalledTimes(2); + expect(second).not.toBe(first); + }); + + it('memoizes primitive step results without an arbitrary size cap', async () => { + const cache = new ReplayPayloadCache(undefined); const oversized = 'x'.repeat(4097); - for (const value of [{ count: 0 }, oversized]) { - const cache = new ReplayPayloadCache(undefined); - const hydrate = vi - .fn() - .mockImplementation(async () => - typeof value === 'object' ? { ...value } : value - ); - - const first = await cache.getStepResult('evnt_result', hydrate); - const second = await cache.getStepResult('evnt_result', hydrate); - expect(hydrate).toHaveBeenCalledTimes(2); - if (typeof value === 'object') expect(second).not.toBe(first); - } + const hydrate = vi.fn().mockResolvedValue(oversized); + + expect(await cache.getStepResult('evnt_result', hydrate)).toBe(oversized); + expect(await cache.getStepResult('evnt_result', hydrate)).toBe(oversized); + expect(hydrate).toHaveBeenCalledOnce(); }); it('does not memoize failed step hydration', async () => { diff --git a/packages/core/src/replay-payload-cache.ts b/packages/core/src/replay-payload-cache.ts index a66dde8670..f8a886c88a 100644 --- a/packages/core/src/replay-payload-cache.ts +++ b/packages/core/src/replay-payload-cache.ts @@ -6,19 +6,12 @@ import { type ReplayPayloadPreparer, } from './serialization.js'; -const MAX_MEMOIZED_PRIMITIVE_LENGTH = 4096; type ReplayPayloadField = 'result' | 'error' | 'payload'; function isMemoizablePrimitive(value: unknown): boolean { if (value === null) return true; const type = typeof value; if (type === 'object' || type === 'function') return false; - if (type === 'string') { - return (value as string).length <= MAX_MEMOIZED_PRIMITIVE_LENGTH; - } - if (type === 'bigint') { - return (value as bigint).toString().length <= MAX_MEMOIZED_PRIMITIVE_LENGTH; - } return true; } @@ -30,9 +23,9 @@ function isMemoizablePrimitive(value: unknown): boolean { * those replays. Deserialization still runs against each VM's globals so every * replay receives fresh object graphs and correctly revived Workflow objects. * - * Successful prepared plaintext remains resident for the invocation lifetime. - * Its memory cost is the sum of decrypted and decompressed payload sizes, but - * it never crosses workflow runs or queue deliveries. + * Successful prepared plaintext and memoized primitive step results remain + * resident for the invocation lifetime. Their memory never crosses workflow + * runs or queue deliveries. */ export class ReplayPayloadCache { private readonly preparedPayloads = new Map< @@ -40,12 +33,23 @@ export class ReplayPayloadCache { Promise >(); private readonly primitiveStepResults = new Map(); + private readonly encryptionKey: Promise; private nextUnscannedEventIndex = 0; constructor( - private readonly encryptionKey: PayloadKey | undefined, + encryptionKey: PayloadKey | undefined | Promise, private readonly preparer: ReplayPayloadPreparer = prepareReplayPayload - ) {} + ) { + this.encryptionKey = Promise.resolve(encryptionKey); + } + + /** Start preparing an event payload as soon as its frame is decoded. */ + prepareEvent(event: Event): void { + const preparation = this.prepareEventIfMissing(event); + // Streaming preparation is speculative. Its ordered consumer observes the + // original rejection and makes that cache entry retryable. + void preparation?.catch(() => {}); + } /** * Start every missing binary preparation before workflow execution. Failures @@ -54,52 +58,19 @@ export class ReplayPayloadCache { */ async prewarm(workflowRun: WorkflowRun, events: Event[]): Promise { const preparations: Promise[] = []; - const start = (cacheKey: string, value: unknown): void => { - // Legacy flattened values may be mutated by devalue's unflatten and are - // therefore prepared only by their eventual consumer, never cached. - if (!(value instanceof Uint8Array)) return; - - // Each replay scans the full event log, so awaiting cached promises here - // would add O(N^2) promise reactions over an N-step invocation. Only wait - // for preparations first discovered by this prewarm pass. - if (this.preparedPayloads.has(cacheKey)) return; - preparations.push(this.ensurePreparation(cacheKey, value)); - }; - - start(this.workflowInputKey(workflowRun.runId), workflowRun.input); - // This cache is scoped to one invocation. Incremental loads and write - // response deltas only ever append, so the scanned length locates the - // events added since the previous replay. A reload that can insert events - // BELOW that length (a stale-snapshot restart replacing the log with a - // corrected one) must call `resetScan()` first, or the inserted events are - // never scanned. Prepared entries stay valid across that: they are keyed by - // event id, not by position. + const workflowInput = this.startPreparation( + this.workflowInputKey(workflowRun.runId), + workflowRun.input + ); + if (workflowInput) preparations.push(workflowInput); for ( let index = this.nextUnscannedEventIndex; index < events.length; index++ ) { const event = events[index]; - switch (event.eventType) { - case 'step_completed': - start( - this.eventPayloadKey(event.eventId, 'result'), - event.eventData?.result - ); - break; - case 'step_failed': - start( - this.eventPayloadKey(event.eventId, 'error'), - event.eventData?.error - ); - break; - case 'hook_received': - start( - this.eventPayloadKey(event.eventId, 'payload'), - event.eventData?.payload - ); - break; - } + const preparation = this.prepareEventIfMissing(event); + if (preparation) preparations.push(preparation); } this.nextUnscannedEventIndex = events.length; @@ -108,16 +79,7 @@ export class ReplayPayloadCache { await Promise.allSettled(preparations); } - /** - * Forget how much of the event log has been scanned, so the next - * {@link prewarm} walks it from the start again. - * - * Required before a replay whose event log was reloaded rather than extended: - * a corrected log inserts the events the previous load was missing, which - * shifts every later position, so a positional resume would skip exactly the - * events the reload was for. Already-prepared payloads are kept: they are - * keyed by event id, so re-scanning re-observes them for free. - */ + /** Rescan the next event log after an authoritative replacement. */ resetScan(): void { this.nextUnscannedEventIndex = 0; } @@ -147,8 +109,8 @@ export class ReplayPayloadCache { /** * Reuse final step values only when sharing them across VMs is unobservable. - * Objects and large strings/bigints always run `hydrate` again, producing a - * fresh VM-specific value from the separately cached prepared payload. + * Objects always run `hydrate` again to produce a fresh VM-specific value; + * every primitive is safe to reuse directly. */ async getStepResult( eventId: string, @@ -199,7 +161,55 @@ export class ReplayPayloadCache { /** Normalize synchronous and asynchronous preparers to one promise contract. */ private async runPreparation(value: unknown): Promise { - return this.preparer(value, this.encryptionKey); + return this.preparer(value, await this.encryptionKey); + } + + /** Start one event's binary payload unless another path already did. */ + private prepareEventIfMissing( + event: Event + ): Promise | undefined { + let field: ReplayPayloadField; + let value: unknown; + switch (event.eventType) { + case 'run_created': + return this.startPreparation( + this.workflowInputKey(event.runId), + event.eventData.input + ); + case 'run_started': + return this.startPreparation( + this.workflowInputKey(event.runId), + event.eventData?.input + ); + case 'step_completed': + field = 'result'; + value = event.eventData?.result; + break; + case 'step_failed': + field = 'error'; + value = event.eventData?.error; + break; + case 'hook_received': + field = 'payload'; + value = event.eventData?.payload; + break; + default: + return undefined; + } + return this.startPreparation( + this.eventPayloadKey(event.eventId, field), + value + ); + } + + private startPreparation( + cacheKey: string, + value: unknown + ): Promise | undefined { + if (!(value instanceof Uint8Array) || this.preparedPayloads.has(cacheKey)) { + return undefined; + } + return this.ensurePreparation(cacheKey, value); } private workflowInputKey(runId: string): string { diff --git a/packages/core/src/runtime-trace-mode.test.ts b/packages/core/src/runtime-trace-mode.test.ts index 1d2d83389e..3b930dd0fc 100644 --- a/packages/core/src/runtime-trace-mode.test.ts +++ b/packages/core/src/runtime-trace-mode.test.ts @@ -13,6 +13,7 @@ import { type ReadableSpan, SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; +import { RUN_ERROR_CODES } from '@workflow/errors'; import { type Event, SPEC_VERSION_CURRENT, @@ -83,11 +84,12 @@ const simpleWorkflow = `async function workflow() { async function makeRunningRun( runId: string, - executionContext?: WorkflowRun['executionContext'] + executionContext?: WorkflowRun['executionContext'], + workflowName = 'workflow' ): Promise { return { runId, - workflowName: 'workflow', + workflowName, status: 'running', input: await dehydrateWorkflowArguments([], runId, undefined, []), createdAt: new Date('2024-01-01T00:00:00.000Z'), @@ -109,17 +111,48 @@ async function driveHandler(opts: { workflowCode: string; traceCarrier?: Record; routeModuleBodyStartedAt?: number; - includeRunInput?: boolean; executionContext?: WorkflowRun['executionContext']; - whileRunStartedPending?: () => Promise; + persistedWorkflowName?: string; + includeRunInput?: boolean; + streamRunCreatedBeforeResponse?: boolean; + onRunStartedRequest?: () => void; + whileRunStartedPending?: (state: { + getEncryptionKeyForRun: ReturnType; + }) => Promise; }) { - const workflowRun = await makeRunningRun(opts.runId, opts.executionContext); + const workflowRun = await makeRunningRun( + opts.runId, + opts.executionContext, + opts.persistedWorkflowName + ); const queuedMessages: any[] = []; + const getEncryptionKeyForRun = vi.fn(async () => undefined); - const eventsCreate = vi.fn(async (_runId: string, data: any) => { + const eventsCreate = vi.fn(async (_runId: string, data: any, params: any) => { if (data.eventType === 'run_started') { - await opts.whileRunStartedPending?.(); - return { run: workflowRun, events: [] as Event[] }; + opts.onRunStartedRequest?.(); + let streamedRunCreated: Event | undefined; + if (opts.streamRunCreatedBeforeResponse) { + streamedRunCreated = { + eventId: 'evnt_00000000000000000000000001', + runId: workflowRun.runId, + eventType: 'run_created', + specVersion: SPEC_VERSION_CURRENT, + createdAt: workflowRun.createdAt, + eventData: { + deploymentId: workflowRun.deploymentId, + workflowName: workflowRun.workflowName, + input: workflowRun.input, + executionContext: workflowRun.executionContext, + }, + }; + params?.replayEventObserver?.(streamedRunCreated); + } + await opts.whileRunStartedPending?.({ getEncryptionKeyForRun }); + return { + run: workflowRun, + events: streamedRunCreated ? [streamedRunCreated] : ([] as Event[]), + }; } return { event: { @@ -149,18 +182,16 @@ async function driveHandler(opts: { runInput: { input: workflowRun.input, deploymentId: workflowRun.deploymentId, - workflowName: workflowRun.workflowName, + workflowName: 'workflow', specVersion: SPEC_VERSION_CURRENT, - executionContext: workflowRun.executionContext, + executionContext: opts.executionContext, }, } : {}), }, { requestId: 'req_test', - // Keep this trace harness on the awaited run_started path even - // when a test supplies runInput for pre-response VM selection. - attempt: opts.includeRunInput ? 2 : 1, + attempt: 1, queueName: '__wkf_workflow_workflow', messageId: 'msg_test', } @@ -184,7 +215,7 @@ async function driveHandler(opts: { queuedMessages.push(message); return { messageId: null }; }), - getEncryptionKeyForRun: vi.fn(async () => undefined), + getEncryptionKeyForRun, } as any); const handler = workflowEntrypoint( @@ -230,6 +261,8 @@ async function driveHandler(opts: { getWorldSpan, deliverySpan, queuedMessages, + eventsCreate, + getEncryptionKeyForRun, }; } @@ -267,18 +300,30 @@ describe('getWorkflowTraceMode', () => { }); describe('workflowEntrypoint trace modes', () => { - it('compiles while run_started is loading, without evaluating early', async () => { - let observedOverlap = false; - await driveHandler({ - runId: 'wrun_trace_compile_overlap', - workflowCode: simpleWorkflow, + it('starts Node replay work while loading the authoritative run', async () => { + vi.stubEnv('WORKFLOW_TURBO', '0'); + const persistedWorkflowCode = `async function persistedWorkflow() { + return 'done'; + }${getWorkflowTransformCode('persistedWorkflow')}`; + + const { eventsCreate, workflowSpan } = await driveHandler({ + runId: 'wrun_trace_persisted_workflow', + workflowCode: persistedWorkflowCode, + persistedWorkflowName: 'persistedWorkflow', includeRunInput: true, - whileRunStartedPending: async () => { + streamRunCreatedBeforeResponse: true, + onRunStartedRequest: () => { expect( exporter .getFinishedSpans() - .find((span) => span.name === 'workflow.bundle.evaluate') + .find((span) => span.name === 'workflow.bundle.compile') ).toBeUndefined(); + }, + whileRunStartedPending: async ({ getEncryptionKeyForRun }) => { + expect(getEncryptionKeyForRun).toHaveBeenCalledWith( + 'wrun_trace_persisted_workflow', + undefined + ); await vi.waitFor(() => { expect( exporter @@ -291,23 +336,100 @@ describe('workflowEntrypoint trace modes', () => { .getFinishedSpans() .find((span) => span.name === 'workflow.bundle.evaluate') ).toBeUndefined(); - observedOverlap = true; }, }); - expect(observedOverlap).toBe(true); + expect( + eventsCreate.mock.calls.some( + ([, event]) => event.eventType === 'run_completed' + ) + ).toBe(true); + expect( + eventsCreate.mock.calls.some( + ([, event]) => event.eventType === 'run_failed' + ) + ).toBe(false); + const compileSpans = exporter + .getFinishedSpans() + .filter((span) => span.name === 'workflow.bundle.compile'); + expect(compileSpans).toHaveLength(2); + expect( + compileSpans.every( + (span) => span.parentSpanId === workflowSpan?.spanContext().spanId + ) + ).toBe(true); expect( exporter .getFinishedSpans() .find((span) => span.name === 'workflow.bundle.evaluate') ).toBeDefined(); + const replayLoadSpan = exporter + .getFinishedSpans() + .find((span) => span.name === 'workflow.replay.load'); + expect(replayLoadSpan?.attributes['workflow.events.count']).toBe(1); + }); + + it.each([ + '0', + '1', + ])('records invalid VM configuration as setup failure with turbo=%s', async (turbo) => { + vi.stubEnv('WORKFLOW_TURBO', turbo); + const { eventsCreate } = await driveHandler({ + runId: `wrun_trace_invalid_vm_${turbo}`, + workflowCode: simpleWorkflow, + includeRunInput: true, + executionContext: { workflowVm: 'bogus' }, + }); + + expect(eventsCreate).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + eventType: 'run_failed', + eventData: expect.objectContaining({ + errorCode: RUN_ERROR_CODES.RUNTIME_ERROR, + }), + }), + expect.anything() + ); + }); + + it('does not leak a streamed replay-load rejection after synchronous setup failure', async () => { + vi.stubEnv('WORKFLOW_TURBO', '0'); + const unhandledRejection = vi.fn(); + process.on('unhandledRejection', unhandledRejection); + + try { + const { eventsCreate } = await driveHandler({ + runId: 'wrun_trace_streamed_invalid_vm', + workflowCode: simpleWorkflow, + includeRunInput: true, + streamRunCreatedBeforeResponse: true, + executionContext: { workflowVm: 'bogus' }, + }); + + expect(eventsCreate).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + eventType: 'run_failed', + eventData: expect.objectContaining({ + errorCode: RUN_ERROR_CODES.RUNTIME_ERROR, + }), + }), + expect.anything() + ); + await new Promise((resolve) => setImmediate(resolve)); + expect(unhandledRejection).not.toHaveBeenCalled(); + } finally { + process.off('unhandledRejection', unhandledRejection); + } }); it('does not compile a Node bundle for a known QuickJS run', async () => { - await driveHandler({ + const { getEncryptionKeyForRun } = await driveHandler({ runId: 'wrun_trace_quickjs_compile', workflowCode: simpleWorkflow, includeRunInput: true, + streamRunCreatedBeforeResponse: true, executionContext: { workflowVm: 'quickjs' }, }); @@ -316,6 +438,11 @@ describe('workflowEntrypoint trace modes', () => { .getFinishedSpans() .find((span) => span.name === 'workflow.bundle.compile') ).toBeUndefined(); + expect(getEncryptionKeyForRun).toHaveBeenCalledTimes(1); + expect(getEncryptionKeyForRun).not.toHaveBeenCalledWith( + 'wrun_trace_quickjs_compile', + undefined + ); }); it('linked (default): nests under the flow route context with a link to the run-origin context', async () => { diff --git a/packages/core/src/runtime.test.ts b/packages/core/src/runtime.test.ts index 0242eb6ea0..88bc7002b6 100644 --- a/packages/core/src/runtime.test.ts +++ b/packages/core/src/runtime.test.ts @@ -2230,6 +2230,8 @@ describe('workflowEntrypoint turbo mode', () => { attempt: number; source: string; runStartedGate?: Promise; + currentDeploymentId?: string; + encryptionKeyError?: Error; }) { const { runId, attempt, source } = opts; const order = turboOrder; @@ -2296,7 +2298,12 @@ describe('workflowEntrypoint turbo mode', () => { setWorld({ specVersion: SPEC_VERSION_CURRENT, - getDeploymentId: vi.fn(async () => 'test-deployment'), + ...(opts.currentDeploymentId + ? { capabilities: { deploymentAffinity: true } } + : {}), + getDeploymentId: vi.fn( + async () => opts.currentDeploymentId ?? 'test-deployment' + ), createQueueHandler: vi.fn( (_p: string, handler: (m: unknown, md: unknown) => Promise) => async () => { @@ -2326,7 +2333,10 @@ describe('workflowEntrypoint turbo mode', () => { }, runs: { get: vi.fn(async () => runEntity) }, queue: vi.fn(async () => ({ messageId: null })), - getEncryptionKeyForRun: vi.fn(async () => undefined), + getEncryptionKeyForRun: vi.fn(async () => { + if (opts.encryptionKeyError) throw opts.encryptionKeyError; + return undefined; + }), } as any); const handlerPromise = workflowEntrypoint(source)( @@ -2375,6 +2385,26 @@ describe('workflowEntrypoint turbo mode', () => { expect(runStartedCreates).toHaveLength(1); }); + it('handles a speculative key rejection when turbo exits before replay', async () => { + const unhandledRejection = vi.fn(); + process.on('unhandledRejection', unhandledRejection); + try { + const { handlerPromise } = await driveTurbo({ + runId: 'wrun_turbo_key_rejection', + attempt: 1, + source: oneStepWorkflow, + currentDeploymentId: 'other-deployment', + encryptionKeyError: new Error('key lookup failed'), + }); + + expect((await handlerPromise).status).toBe(204); + await new Promise((resolve) => setImmediate(resolve)); + expect(unhandledRejection).not.toHaveBeenCalled(); + } finally { + process.off('unhandledRejection', unhandledRejection); + } + }); + it('does not turbo on a redelivery (attempt > 1): run_started is awaited first', async () => { const { handlerPromise, order } = await driveTurbo({ runId: 'wrun_turbo_redeliver', diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index 15e756f679..7cfa388914 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -82,6 +82,7 @@ import { parseHealthCheckPayload, preconditionEventDelta, queueMessage, + resolveRunEncryptionKey, type SlotSnapshotParams, settleEventSlotGap, slotSnapshotParams, @@ -122,6 +123,7 @@ import { dehydrateRunError } from './serialization.js'; import { remapErrorStack } from './source-map.js'; import * as Attribute from './telemetry/semantic-conventions.js'; import { + bindActiveTraceContext, buildInvocationSpanLinks, getNextTraceCarrier, getSpanKind, @@ -957,17 +959,50 @@ export function workflowEntrypoint( let compiledWorkflowScripts: | ReturnType | undefined; - const startWorkflowCompile = ( + let compiledWorkflowName: string | undefined; + const startWorkflowCompile = await bindActiveTraceContext( + ( + workflow?: Pick< + WorkflowRun, + 'workflowName' | 'executionContext' + > + ) => { + if (!workflow || useQuickJSVm(workflow)) return; + if (compiledWorkflowName !== workflow.workflowName) { + compiledWorkflowName = workflow.workflowName; + compiledWorkflowScripts = compileWorkflowBundle( + workflowCode, + workflow.workflowName + ); + // Terminal runs can return without awaiting compilation. + void compiledWorkflowScripts.catch(() => {}); + } + return compiledWorkflowScripts; + } + ); + const encryptionKey = once(() => { + const result = resolveRunEncryptionKey(world, runId); + void result.catch(() => {}); + return result; + }); + let replayPayloadCache: ReplayPayloadCache | undefined; + const startReplayPayloadCache = ( workflow?: Pick ) => { if (!workflow || useQuickJSVm(workflow)) return; - compiledWorkflowScripts ??= compileWorkflowBundle( - workflowCode, - workflowName - ); - // Terminal runs can return without awaiting compilation. - void compiledWorkflowScripts.catch(() => {}); - return compiledWorkflowScripts; + if (!replayPayloadCache) { + replayPayloadCache = new ReplayPayloadCache( + encryptionKey.value + ); + } + return replayPayloadCache; + }; + const prepareReplayEvent = (event: Event): void => { + if (event.eventType === 'run_created') { + startReplayPayloadCache(event.eventData); + startWorkflowCompile(event.eventData); + } + replayPayloadCache?.prepareEvent(event); }; // Every write this loop makes carries the cursor of the log // it was computed against, and folds a complete returned @@ -992,20 +1027,28 @@ export function workflowEntrypoint( const traceReplayLoad = ( source: Attribute.WorkflowReplayLoadSource, - load: () => Promise + load: ( + replayEventObserver: (event: Event) => void + ) => Promise ): Promise => trace('workflow.replay.load', async (loadSpan) => { + let eventsCount = 0; loadSpan?.setAttributes({ ...Attribute.WorkflowRunId(runId), ...Attribute.WorkflowReplayLoadSource(source), }); - const result = await load(); - loadSpan?.setAttributes( - Attribute.WorkflowEventsCount( - result.events?.length ?? 0 - ) - ); - return result; + try { + const result = await load((event) => { + eventsCount++; + prepareReplayEvent(event); + }); + eventsCount = result.events?.length ?? eventsCount; + return result; + } finally { + loadSpan?.setAttributes( + Attribute.WorkflowEventsCount(eventsCount) + ); + } }); /** @@ -1216,6 +1259,23 @@ export function workflowEntrypoint( } }; + const recordWorkflowSetupFailure = async ( + err: unknown + ): Promise => { + const errorCode = getWorkflowSetupErrorCode(err); + if (!errorCode) return false; + await recordFatalRunError({ + world, + workflowRun, + runId, + requestId, + err, + errorCode, + logMessage: 'Fatal runtime error during workflow setup', + }); + return true; + }; + // Re-invoke the orchestrator. Outside turbo this returns // `{ timeoutSeconds }`, which makes the queue reschedule the // CURRENT delivery's message. In turbo that is a trap: the @@ -1438,10 +1498,7 @@ export function workflowEntrypoint( // incremental load starts above the hole and never // returns it. eventLog = { type: 'loadAll' }; - // The corrected log inserts the missing events BELOW the - // length already scanned for payload prewarming, shifting - // every later position. Only a full rescan sees them. - replayPayloadCache.resetScan(); + replayPayloadCache?.resetScan(); } runtimeLogger.warn( 'Event creation rejected as stale; restarting replay in-process', @@ -2078,25 +2135,29 @@ export function workflowEntrypoint( span?.addEvent('workflow.hook_received.create.start', { 'workflow.hook_received.preload_events': true, }); - const replayLoad = traceReplayLoad('hook_preload', () => - createEvent( - { - eventType: 'hook_received', - specVersion: SPEC_VERSION_CURRENT, - correlationId: hookResumeInput.hookId, - eventData: { - token: hookResumeInput.token, - payload: hookResumeInput.payload, + const replayLoad = traceReplayLoad( + 'hook_preload', + (replayEventObserver) => + createEvent( + { + eventType: 'hook_received', + specVersion: SPEC_VERSION_CURRENT, + correlationId: hookResumeInput.hookId, + eventData: { + token: hookResumeInput.token, + payload: hookResumeInput.payload, + }, }, - }, - { - requestId, - occurredAt, - resumeId: hookResumeInput.resumeId, - resumePayloadDigest: hookResumeInput.payloadDigest, - preloadEvents: true, - } - ) + { + requestId, + occurredAt, + resumeId: hookResumeInput.resumeId, + resumePayloadDigest: + hookResumeInput.payloadDigest, + preloadEvents: true, + replayEventObserver, + } + ) ); const result = await replayLoad; hookEnsured = true; @@ -2242,6 +2303,7 @@ export function workflowEntrypoint( ); return; } + if (await recordWorkflowSetupFailure(err)) return; throw err; } } @@ -2304,8 +2366,17 @@ export function workflowEntrypoint( // wasted list+resolve it would otherwise compute. { requestId, skipPreload: true } ); - startWorkflowCompile(runInput); runReadyBarrier = startedPromise; + try { + startWorkflowCompile(runInput); + startReplayPayloadCache(runInput); + } catch (err) { + await awaitRunReady(); + if (!(await recordWorkflowSetupFailure(err))) { + throw err; + } + return; + } // Turbo backgrounds run_started, so the non-turbo // assignment below never runs. Thread the per-run event // ceiling off the backgrounded response here instead. @@ -2373,13 +2444,26 @@ export function workflowEntrypoint( span?.addEvent('workflow.run_started.create.start', { 'workflow.run_started.skip_preload': false, }); - const replayLoad = traceReplayLoad('run_started', () => - createEvent(runStartedEvent, { requestId }) + const replayLoad = traceReplayLoad( + 'run_started', + (replayEventObserver) => + createEvent(runStartedEvent, { + requestId, + replayEventObserver, + }) ); - // Initial deliveries carry runInput, so Node compilation - // can overlap this load without guessing the VM engine. - // Continuations learn the engine from result.run below. - startWorkflowCompile(runInput); + try { + startWorkflowCompile(runInput); + startReplayPayloadCache(runInput); + } catch (setupError) { + try { + await replayLoad; + } catch { + // Preserve the synchronous setup error after + // observing the in-flight replay load. + } + throw setupError; + } const result = await replayLoad; workflowRun = result.run; maxEventsLimit = clampMaxEvents(result.maxEvents); @@ -2442,20 +2526,9 @@ export function workflowEntrypoint( ); return; } else { - const errorCode = getWorkflowSetupErrorCode(err); - if (!errorCode) { + if (!(await recordWorkflowSetupFailure(err))) { throw err; } - await recordFatalRunError({ - world, - workflowRun, - runId, - requestId, - err, - errorCode, - logMessage: - 'Fatal runtime error during workflow setup', - }); return; } } @@ -2665,39 +2738,19 @@ export function workflowEntrypoint( // do we fall back to reloading the complete log. if (eventLog.type !== 'loadAll' && ensuredEvent) { insertEventByEventId(eventLog.events, ensuredEvent); + prepareReplayEvent(ensuredEvent); } else { eventLog = { type: 'loadAll' }; } } // end else (re-ensure needed) } - // Resolve the encryption key for this run's deployment. - // Used eagerly here since both workflow execution (input - // hydration / hook payload decryption) and the run_failed - // dehydrate path below need it. Memoized accessor: first - // call triggers the actual fetch / HKDF derivation, - // subsequent calls await the cached promise. - const getEncryptionKey = memoizeEncryptionKey( - world, - workflowRun - ); - const encryptionKey = await getEncryptionKey(); - - // Invocation-scoped cache of VM-independent prepared payloads - // and immutable final values. It survives the fresh workflow - // VM created by each inline replay, but never crosses runs or - // queue deliveries. - const replayPayloadCache = new ReplayPayloadCache( - encryptionKey - ); - // The live VM parked at the previous boundary, when the // retention decision kept it. null → this iteration cold- // replays. Invocation-scoped: dies with this delivery. let retainedSession: WorkflowSession | null = null; // Main replay loop - // biome-ignore lint/correctness/noConstantCondition: intentional loop while (true) { loopIteration++; @@ -2856,7 +2909,10 @@ export function workflowEntrypoint( appendEventLog(eventLog, page); eventLog = { ...eventLog, type: 'ready' }; } else { - eventLog = { ...page, type: 'ready' }; + eventLog = { + ...page, + type: 'ready', + }; } } assert(eventLog.type === 'ready'); @@ -3016,7 +3072,10 @@ export function workflowEntrypoint( events: eventLog.events, cursor: eventLog.cursor, }); - eventLog = { ...settled.log, type: 'ready' }; + eventLog = { + ...settled.log, + type: 'ready', + }; if (settled.gap !== undefined) { throw new CorruptedEventLogError( `Event log for run ${runId} has a hole at slot ${settled.gap.firstMissingSlot}: ${settled.gap.missingCount} of the ${settled.gap.maxSlot} slots up to the log's maximum hold no event.` @@ -3099,6 +3158,12 @@ export function workflowEntrypoint( // Crypto work overlaps VM setup on the replay path and // the appended events' consumption on the resume path; // consumers still deserialize and resolve in event order. + const replayPayloadCache = + startReplayPayloadCache(workflowRun); + assert( + replayPayloadCache, + 'Node workflow replay requires payload preparation' + ); const payloadPrewarm = replayPayloadCache.prewarm( workflowRun, eventLog.events @@ -3127,7 +3192,7 @@ export function workflowEntrypoint( workflowCode, workflowRun, events: eventLog.events, - encryptionKey, + encryptionKey: await encryptionKey.value, replayPayloadCache, compiledWorkflowScripts: await compiled, // Turbo: the end-of-run drain inside workflow @@ -3370,7 +3435,7 @@ export function workflowEntrypoint( error: await dehydrateRunError( suspensionError, runId, - encryptionKey, + await encryptionKey.value, globalThis, (workflowRun?.specVersion ?? 0) >= SPEC_VERSION_SUPPORTS_COMPRESSION @@ -3409,16 +3474,6 @@ export function workflowEntrypoint( }); return; } - if (suspensionResult.reportedEventCount > 0) { - // Bump-and-report merged events BELOW the tail and - // re-sorted the array to slot order, shifting every - // position the prewarm scan had already recorded. - // The cursor is deliberately left alone: the report - // is a lower bound on what was skipped, so the next - // incremental read still has to cover the same range. - replayPayloadCache.resetScan(); - } - // Open hooks/waits in the log as loaded for this // replay. Computed lazily, at most once, and shared // between the retention decision here and the @@ -4755,7 +4810,7 @@ export function workflowEntrypoint( error: await dehydrateRunError( terminalError, runId, - encryptionKey, + await encryptionKey.value, globalThis, (workflowRun?.specVersion ?? 0) >= SPEC_VERSION_SUPPORTS_COMPRESSION diff --git a/packages/core/src/runtime/helpers.test.ts b/packages/core/src/runtime/helpers.test.ts index 21852fac4b..c0b8077fb6 100644 --- a/packages/core/src/runtime/helpers.test.ts +++ b/packages/core/src/runtime/helpers.test.ts @@ -1,7 +1,7 @@ import { PreconditionFailedError, WorkflowWorldError } from '@workflow/errors'; import type { Event, World } from '@workflow/world'; import { slotToEventId } from '@workflow/world'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { bytesToBase64, deriveRunKeyPair, seal } from '../sealed-box.js'; import { decrypt, diff --git a/packages/core/src/runtime/helpers.ts b/packages/core/src/runtime/helpers.ts index 5f28ca0af3..bcd6502d51 100644 --- a/packages/core/src/runtime/helpers.ts +++ b/packages/core/src/runtime/helpers.ts @@ -612,7 +612,6 @@ export async function loadWorkflowRunEvents( let hasMore = true; let pagesLoaded = 0; let retriedWithoutCursor = false; - const world = await getWorldLazy(); const loadStart = Date.now(); while (hasMore) { @@ -1215,34 +1214,26 @@ export function getQueueOverhead(message: { requestedAt?: Date }) { } /** - * Returns a memoized accessor for a run's full encryption capability. - * - * The first call resolves the run's key material via - * `world.getEncryptionKeyForRun` (which may do HKDF derivation locally on - * Vercel, or a network fetch from external contexts) and derives a - * {@link PayloadKey} from it; subsequent calls await the same cached promise. - * If the world doesn't support encryption or the run has no key configured, - * the cached value is `undefined`. - * - * The resolved value is deliberately the *full* capability (the symmetric AES - * key plus the run's X25519 keypair), not just a `CryptoKey`. A run reading - * its own event log can encounter sealed (`encp`) payloads that another run - * wrote to it (a cross-deployment hook resumption, say), and opening those - * needs the keypair. Resolving only the symmetric key would leave those - * payloads unopenable and wedge the run. - * - * Used by step / workflow handlers to defer the (potentially expensive) - * key fetch until the first code path that actually needs it: typically - * input hydration on the success path, or error dehydration on a failure - * path. Both paths can race-call the accessor without triggering duplicate - * fetches. - * - * Errors thrown by `getEncryptionKeyForRun` propagate to every caller - * (the cached promise rejects). This is intentional: when encryption is - * configured, we never want to silently fall back to plaintext - * serialization. A propagated error in an event-emission path leaves the - * outer try/catch to log and surface the issue; the queue's redelivery - * semantics will retry the key fetch on the next attempt. + * Resolve the run's full payload-encryption capability. This includes the + * symmetric key and X25519 keypair needed to open cross-run sealed payloads. + * Missing world support or key material resolves to `undefined`; lookup and + * derivation failures propagate rather than silently falling back to plaintext. + */ +export async function resolveRunEncryptionKey( + world: World, + runOrId: WorkflowRun | string, + context?: Record +): Promise { + const rawKey = + typeof runOrId === 'string' + ? await world.getEncryptionKeyForRun?.(runOrId, context) + : await world.getEncryptionKeyForRun?.(runOrId); + return rawKey ? await deriveRunPayloadKeys(rawKey) : undefined; +} + +/** + * Return a lazy, memoized accessor around {@link resolveRunEncryptionKey}. + * Concurrent callers share the same promise, including its rejection. */ export function memoizeEncryptionKey( world: World, @@ -1251,20 +1242,7 @@ export function memoizeEncryptionKey( let cached: Promise | undefined; return () => { if (!cached) { - cached = (async () => { - // The `getEncryptionKeyForRun` overload set takes either a - // `WorkflowRun` or a `runId: string` (with optional context). Branch - // here so TypeScript picks the right overload for each shape. - const rawKey = - typeof runOrId === 'string' - ? await world.getEncryptionKeyForRun?.(runOrId) - : await world.getEncryptionKeyForRun?.(runOrId); - // Resolve the *full* capability, not just the symmetric key: a run - // reading its own event log may encounter sealed (`encp`) payloads - // that another run wrote to it, and opening those needs the run's - // X25519 scalar as well. - return rawKey ? await deriveRunPayloadKeys(rawKey) : undefined; - })(); + cached = resolveRunEncryptionKey(world, runOrId); } return cached; }; diff --git a/packages/core/src/telemetry.ts b/packages/core/src/telemetry.ts index 7b6ebdee2d..8c773ddfc4 100644 --- a/packages/core/src/telemetry.ts +++ b/packages/core/src/telemetry.ts @@ -329,6 +329,14 @@ export async function startTraceSpan(spanName: string) { }; } +/** Bind work to the currently active span even when it starts from a child. */ +export async function bindActiveTraceContext( + fn: (...args: Args) => Result +): Promise<(...args: Args) => Result> { + const otel = await OtelApi.value; + return otel ? otel.context.bind(otel.context.active(), fn) : fn; +} + /** Keeps a parked workflow's ambient trace context aligned with each resume. */ export async function createRefreshableTraceContext() { const otel = await OtelApi.value; diff --git a/packages/core/src/workflow.ts b/packages/core/src/workflow.ts index ac97eb94b6..18e7df9d0c 100644 --- a/packages/core/src/workflow.ts +++ b/packages/core/src/workflow.ts @@ -145,10 +145,10 @@ export interface CompiledWorkflowScripts { /** * Compile the workflow bundle before its run snapshot is available. * - * Compilation depends only on the route's bundle string and workflow name, - * not the event log or VM context. The runtime starts this promise while - * `run_started` loads the replay snapshot, then evaluates the scripts only - * after it has created the fresh context. + * Compilation depends only on the route's bundle string and the workflow name + * persisted on the run, not the event log or VM context. The runtime starts + * this promise while `run_started` loads the replay snapshot, then evaluates + * the scripts only after it has created the fresh context. */ export function compileWorkflowBundle( workflowCode: string, diff --git a/packages/world-vercel/src/event-retry.ts b/packages/world-vercel/src/event-retry.ts index 6d959ada52..792943a888 100644 --- a/packages/world-vercel/src/event-retry.ts +++ b/packages/world-vercel/src/event-retry.ts @@ -72,6 +72,14 @@ import { import type { EventTypeSchema } from '@workflow/world'; import type { z } from 'zod'; +/** Keeps caller-owned replay observer failures out of retry/classification. */ +export class ReplayEventObserverError extends Error { + constructor(readonly error: unknown) { + super('Replay event observer failed', { cause: error }); + this.name = 'ReplayEventObserverError'; + } +} + /** Every event type the world knows about (includes the server-only * `hook_conflict`, which the SDK never POSTs). */ type WorkflowEventType = z.infer; @@ -251,6 +259,11 @@ function collectErrorMarkers(err: unknown, depth = 0): string[] { * budgeted, Retry-After-honoring policy. */ export function isRetryableEventPostError(err: unknown): boolean { + // Observer code is caller-owned and runs only after a response frame has + // been validated. Its errors are never evidence of a failed transport, even + // when the original error happens to carry a retryable-looking code. + if (err instanceof ReplayEventObserverError) return false; + // Definitive, server-considered outcomes, never retried as *transient*. // (425 is left to the runtime's retry-after handling; 429 has its own // in-process policy in withEventPostRetry, gated by THROTTLE_RETRY_BUDGET_MS diff --git a/packages/world-vercel/src/events-v4.test.ts b/packages/world-vercel/src/events-v4.test.ts index 5532638a14..6990145dff 100644 --- a/packages/world-vercel/src/events-v4.test.ts +++ b/packages/world-vercel/src/events-v4.test.ts @@ -1117,12 +1117,14 @@ describe('createWorkflowRunEventV4 over HTTP', () => { } ); + const replayEventObserver = vi.fn(); const result = await createWorkflowRunStartedEventV4( { runId: 'wrun_1', specVersion: 5, }, - { token: 'test-token', dispatcher: agent } + { token: 'test-token', dispatcher: agent }, + replayEventObserver ); expect(result.maxEvents).toBe(10000); @@ -1130,6 +1132,9 @@ describe('createWorkflowRunEventV4 over HTTP', () => { expect(result.events[0]).toMatchObject({ eventData: { input } }); expect(result.cursor).toBe('eid:evnt_2'); expect(result.hasMore).toBe(false); + expect( + replayEventObserver.mock.calls.map(([event]) => event.eventId) + ).toEqual(['evnt_1', 'evnt_2']); agent.assertNoPendingInterceptors(); }); diff --git a/packages/world-vercel/src/events-v4.ts b/packages/world-vercel/src/events-v4.ts index 12bc8adea5..f94e4926c0 100644 --- a/packages/world-vercel/src/events-v4.ts +++ b/packages/world-vercel/src/events-v4.ts @@ -38,6 +38,7 @@ import { } from '@workflow/world'; import { decode } from 'cbor-x'; import { z } from 'zod'; +import { ReplayEventObserverError } from './event-retry.js'; import { type DecodedFrame, decodeFrames, @@ -867,14 +868,20 @@ async function decodeCreateEventResponse( export async function createWorkflowRunStartedEventV4( input: CreateEventV4InputBase, - config?: APIConfig + config?: APIConfig, + replayEventObserver?: (event: Event) => void ) { const response = await postWorkflowRunEventV4( { ...input, eventType: 'run_started' }, 'event-stream', config ); - const page = await consumeReplayLogResponse(response, input.runId, config); + const page = await consumeReplayLogResponse( + response, + input.runId, + config, + replayEventObserver + ); if (!page.cursor) { throw new WorkflowWorldError( 'v4 createEvent: event stream missing cursor', @@ -1319,7 +1326,8 @@ export type HookReceivedPreloadV4Result = */ export async function createHookReceivedPreloadEventV4( input: CreateEventV4InputBase, - config?: APIConfig + config?: APIConfig, + replayEventObserver?: (event: Event) => void ): Promise { const response = await postWorkflowRunEventV4( { ...input, eventType: 'hook_received' }, @@ -1335,7 +1343,12 @@ export async function createHookReceivedPreloadEventV4( }; } - const page = await consumeReplayLogResponse(response, input.runId, config); + const page = await consumeReplayLogResponse( + response, + input.runId, + config, + replayEventObserver + ); const maxEvents = MaxEventsHeaderSchema.safeParse( response.headers.get(MAX_EVENTS_HEADER) ); @@ -1470,24 +1483,37 @@ function streamErrorFrameToError( ); } -type EventFrameStreamResult = ListEventsV4Result & { - partialError?: WorkflowWorldError; -}; +type EventFrameStreamResult = + | (ListEventsV4Result & { kind: 'complete' }) + | { + kind: 'partial'; + events: Event[]; + cursor: string; + hasMore: true; + error: WorkflowWorldError; + }; const MAX_PARTIAL_STREAM_RETRIES = 2; function partialEventFrameStream( events: Event[], - partialError: WorkflowWorldError + error: WorkflowWorldError ): EventFrameStreamResult { const eventId = events.at(-1)?.eventId; - if (!eventId) throw partialError; - return { events, cursor: `eid:${eventId}`, hasMore: true, partialError }; + if (!eventId) throw error; + return { + kind: 'partial', + events, + cursor: `eid:${eventId}`, + hasMore: true, + error, + }; } async function consumeEventFrameStream( response: Response, - opName: string + opName: string, + replayEventObserver?: (event: Event) => void ): Promise { const contentType = response.headers.get('content-type'); if (!contentType?.startsWith(V4_FRAME_CONTENT_TYPE)) { @@ -1508,6 +1534,7 @@ async function consumeEventFrameStream( if (frame.meta._end === 1) { const end = EventStreamEndSchema.parse(frame.meta); return { + kind: 'complete', events, cursor: end.next ?? null, hasMore: end.hasMore, @@ -1519,22 +1546,35 @@ async function consumeEventFrameStream( if (Object.keys(frame.meta).some((key) => key.startsWith('_'))) { throw new Error(`v4 ${opName}: unexpected control frame`); } - events.push(decodeEventFrame(frame)); + const event = decodeEventFrame(frame); + events.push(event); + try { + replayEventObserver?.(event); + } catch (error) { + throw new ReplayEventObserverError(error); + } } } catch (cause) { - if (CorruptedEventLogError.is(cause) || WorkflowWorldError.is(cause)) { + if ( + cause instanceof ReplayEventObserverError || + CorruptedEventLogError.is(cause) || + WorkflowWorldError.is(cause) + ) { throw cause; } - const incomplete = cause instanceof IncompleteFrameError; - const error = new WorkflowWorldError( - `v4 ${opName}: ${incomplete ? 'incomplete' : 'invalid'} event frame stream`, - { - code: incomplete ? 'TRANSPORT' : 'SCHEMA_VALIDATION', + if (!(cause instanceof IncompleteFrameError)) { + throw new WorkflowWorldError(`v4 ${opName}: invalid event frame stream`, { + code: 'SCHEMA_VALIDATION', cause, - } + }); + } + return partialEventFrameStream( + events, + new WorkflowWorldError(`v4 ${opName}: incomplete event frame stream`, { + code: 'TRANSPORT', + cause, + }) ); - if (!incomplete) throw error; - return partialEventFrameStream(events, error); } return partialEventFrameStream( @@ -1555,12 +1595,22 @@ async function consumeEventFrameStream( async function consumeReplayLogResponse( response: Response, runId: string, - config?: APIConfig + config?: APIConfig, + replayEventObserver?: (event: Event) => void ): Promise { - const page = await consumeEventFrameStream(response, 'createEvent'); - if (!page.hasMore) return page; + const page = await consumeEventFrameStream( + response, + 'createEvent', + replayEventObserver + ); + if (!page.hasMore) { + return { + events: page.events, + cursor: page.cursor, + hasMore: false, + }; + } if (!page.cursor) { - if (page.partialError) throw page.partialError; throw new WorkflowWorldError( 'v4 createEvent: partial event stream missing cursor', { code: 'SCHEMA_VALIDATION' } @@ -1570,7 +1620,8 @@ async function consumeReplayLogResponse( const suffix = await getWorkflowRunEventsV4( runId, { cursor: page.cursor, remoteRefBehavior: 'resolve' }, - config + config, + replayEventObserver ); return { events: [...page.events, ...suffix.events], @@ -1592,7 +1643,8 @@ async function consumeListFrameStream( url: string, headers: Headers, config: APIConfig | undefined, - opName: string + opName: string, + replayEventObserver?: (event: Event) => void ): Promise { const response = await fetchV4( url, @@ -1600,7 +1652,7 @@ async function consumeListFrameStream( config, opName ); - return consumeEventFrameStream(response, opName); + return consumeEventFrameStream(response, opName, replayEventObserver); } /** @@ -1640,7 +1692,8 @@ function paginationToQuery(params: ListEventsV4Params): string { export async function getWorkflowRunEventsV4( runId: string, params: ListEventsV4Params = {}, - config?: APIConfig + config?: APIConfig, + replayEventObserver?: (event: Event) => void ): Promise { const { baseUrl, headers } = await getHttpConfig(config); const events: Event[] = []; @@ -1652,17 +1705,22 @@ export async function getWorkflowRunEventsV4( const url = `${baseUrl}/v4/runs/${encodeURIComponent(runId)}/events` + paginationToQuery({ ...params, cursor: cursor ?? undefined }); - consumed = await consumeListFrameStream(url, headers, config, 'listEvents'); + consumed = await consumeListFrameStream( + url, + headers, + config, + 'listEvents', + replayEventObserver + ); const cursorAdvanced = !!consumed.cursor && consumed.cursor !== cursor; - if (consumed.partialError) { + if (consumed.kind === 'partial') { if ( params.limit !== undefined || !cursorAdvanced || partialStreamRetries === MAX_PARTIAL_STREAM_RETRIES ) { - throw consumed.partialError; + throw consumed.error; } - assert(consumed.cursor); partialStreamRetries++; cursor = consumed.cursor; } else if ( @@ -1677,7 +1735,7 @@ export async function getWorkflowRunEventsV4( for (const event of consumed.events) { events.push(event); } - } while (consumed.partialError); + } while (consumed.kind === 'partial'); return { events, @@ -1718,6 +1776,10 @@ export async function getEventsByCorrelationIdV4( config, 'listEventsByCorrelationId' ); - if (consumed.partialError) throw consumed.partialError; - return consumed; + if (consumed.kind === 'partial') throw consumed.error; + return { + events: consumed.events, + cursor: consumed.cursor, + hasMore: consumed.hasMore, + }; } diff --git a/packages/world-vercel/src/events.test.ts b/packages/world-vercel/src/events.test.ts index 4a36c778a5..8689afff7f 100644 --- a/packages/world-vercel/src/events.test.ts +++ b/packages/world-vercel/src/events.test.ts @@ -1,5 +1,6 @@ import { Buffer } from 'node:buffer'; import { gzipSync } from 'node:zlib'; +import { WorkflowWorldError } from '@workflow/errors'; import type { AnyEventRequest, CreateEventParams } from '@workflow/world'; import { decode, encode } from 'cbor-x'; import { ulid } from 'ulid'; @@ -778,6 +779,42 @@ describe('splitEventDataForV4 attribute fields', () => { }); describe('createWorkflowRunEvent response coercion', () => { + it('surfaces streamed event observer failures unchanged', async () => { + const agent = mockAgent(); + const observerError = new WorkflowWorldError('observer failed', { + code: 'TRANSPORT', + }); + agent + .get(ORIGIN) + .intercept({ + path: '/api/v4/runs/wrun_1/events/run_started', + method: 'POST', + }) + .reply(200, runStartedResponse(), { + headers: { + 'content-type': V4_FRAME_CONTENT_TYPE, + 'x-wf-event-id': 'evnt_1', + 'x-wf-run-id': 'wrun_1', + 'x-wf-created-at': STARTED_AT.toISOString(), + 'x-wf-max-events': '10000', + }, + }); + + await expect( + createWorkflowRunEvent( + 'wrun_1', + { eventType: 'run_started', specVersion: 2 } as AnyEventRequest, + { + replayEventObserver: () => { + throw observerError; + }, + }, + { token: 'test-token', dispatcher: agent } + ) + ).rejects.toBe(observerError); + agent.assertNoPendingInterceptors(); + }); + it('accepts a current region-tagged run_created runId', async () => { const taggedRunId = `wrun_${encodeRunId(ulid(), REGION_IDS.sfo1)}`; const agent = mockAgent(); diff --git a/packages/world-vercel/src/events.ts b/packages/world-vercel/src/events.ts index 3588849b10..7d373a42d2 100644 --- a/packages/world-vercel/src/events.ts +++ b/packages/world-vercel/src/events.ts @@ -53,7 +53,7 @@ import { validateUlidTimestamp, type WorkflowRun, } from '@workflow/world'; -import { withEventPostRetry } from './event-retry.js'; +import { ReplayEventObserverError, withEventPostRetry } from './event-retry.js'; import { createHookReceivedPreloadEventV4, createWorkflowRunEventsBatchV4, @@ -637,6 +637,7 @@ export async function createWorkflowRunEvent( } return result as EventResult; } catch (err) { + if (err instanceof ReplayEventObserverError) throw err.error; // 404 on hook_disposed / hook_received → already-disposed hook. if ( isHookEventRequiringExistence(data.eventType) && @@ -751,51 +752,22 @@ async function createWorkflowRunEventInner( }; if (data.eventType === 'run_started' && !params?.skipPreload) { - const result = await createWorkflowRunStartedEventV4(input, config); - const runCreated = result.events.find( - (event) => event.eventType === 'run_created' - ); - const runStarted = result.events.find( - (event) => event.eventType === 'run_started' + const result = await createWorkflowRunStartedEventV4( + input, + config, + params?.replayEventObserver ); - if (!runCreated) { - throw new WorkflowWorldError( - 'v4 createEvent: run_started stream is missing run_created', - { code: 'SCHEMA_VALIDATION' } - ); - } - if (!runStarted) { + const replayRun = reconstructRunFromReplayEvents(result.events); + if (!replayRun) { throw new WorkflowWorldError( - 'v4 createEvent: run_started stream is missing run_started', + 'v4 createEvent: run_started stream is missing lifecycle events', { code: 'SCHEMA_VALIDATION' } ); } - let attributes = runCreated.eventData.attributes ?? {}; - let updatedAt = runStarted.createdAt; - for (const event of result.events) { - if (event.eventType === 'attr_set') { - attributes = applyAttributeChanges(attributes, event.eventData.changes); - updatedAt = event.createdAt; - } - } - return { - event: runStarted, - run: { - runId: runCreated.runId, - status: 'running', - deploymentId: runCreated.eventData.deploymentId, - workflowName: runCreated.eventData.workflowName, - specVersion: runCreated.specVersion, - executionContext: runCreated.eventData.executionContext, - input: runCreated.eventData.input, - attributes, - encryptionPublicKey: runCreated.eventData.encryptionPublicKey, - startedAt: runStarted.createdAt, - createdAt: runCreated.createdAt, - updatedAt, - }, + event: replayRun.event, + run: replayRun.run, events: result.events, cursor: result.cursor, hasMore: result.hasMore, @@ -820,7 +792,8 @@ async function createWorkflowRunEventInner( // an S3-backed hook payload the runtime would discard anyway. const outcome = await createHookReceivedPreloadEventV4( { ...input, remoteRefBehavior: 'lazy' }, - config + config, + params.replayEventObserver ); if (outcome.kind === 'materialized') { // Older server (or optimization declined): the write still succeeded @@ -835,10 +808,10 @@ async function createWorkflowRunEventInner( // Unlike lifecycle streams, a preload missing run_created/run_started is // not fatal here: the write has already converged, so return the page // without a run and let the runtime take its safe fallback. - const run = reconstructRunFromReplayEvents(events); + const replayRun = reconstructRunFromReplayEvents(events); return { ...(canonicalEvent ? { event: canonicalEvent } : {}), - ...(run ? { run } : {}), + ...(replayRun ? { run: replayRun.run } : {}), events, cursor, hasMore, @@ -857,20 +830,16 @@ async function createWorkflowRunEventInner( /** * Reconstruct the run entity from a streamed replay log: identity and input * from `run_created`, start time from `run_started`, later `attr_set` events - * folded into `attributes`/`updatedAt`. Returns undefined when the log does - * not contain both lifecycle events (the caller decides whether that is - * fatal). The reconstructed status is always `running`: a terminal event + * folded into `attributes`/`updatedAt`. Returns undefined when reconstruction + * is incomplete so each caller can choose whether that is fatal. The + * reconstructed status is always `running`: a terminal event * committed concurrently still rides in the log itself, and the runtime's * replay-time terminal detection handles it. */ -function reconstructRunFromReplayEvents( - events: Event[] -): (WorkflowRun & { startedAt: Date }) | undefined { +function reconstructRunFromReplayEvents(events: Event[]) { const runCreated = events.find((event) => event.eventType === 'run_created'); const runStarted = events.find((event) => event.eventType === 'run_started'); - if (!runCreated || !runStarted) { - return undefined; - } + if (!runCreated || !runStarted) return; let attributes = runCreated.eventData.attributes ?? {}; let updatedAt = runStarted.createdAt; @@ -882,17 +851,20 @@ function reconstructRunFromReplayEvents( } return { - runId: runCreated.runId, - status: 'running', - deploymentId: runCreated.eventData.deploymentId, - workflowName: runCreated.eventData.workflowName, - specVersion: runCreated.specVersion, - executionContext: runCreated.eventData.executionContext, - input: runCreated.eventData.input, - attributes, - encryptionPublicKey: runCreated.eventData.encryptionPublicKey, - startedAt: runStarted.createdAt, - createdAt: runCreated.createdAt, - updatedAt, + event: runStarted, + run: { + runId: runCreated.runId, + status: 'running' as const, + deploymentId: runCreated.eventData.deploymentId, + workflowName: runCreated.eventData.workflowName, + specVersion: runCreated.specVersion, + executionContext: runCreated.eventData.executionContext, + input: runCreated.eventData.input, + attributes, + encryptionPublicKey: runCreated.eventData.encryptionPublicKey, + startedAt: runStarted.createdAt, + createdAt: runCreated.createdAt, + updatedAt, + }, }; } diff --git a/packages/world/src/events.ts b/packages/world/src/events.ts index d3fdbd95bb..a3c192d64c 100644 --- a/packages/world/src/events.ts +++ b/packages/world/src/events.ts @@ -868,6 +868,13 @@ export interface CreateEventParams { * `resumeHook()` must not set it. */ preloadEvents?: true; + /** + * Synchronously observes each validated event in a streamed replay-log + * response. A retried request may observe the same event again; observers + * must therefore be idempotent. Throwing aborts the operation and the World + * must surface the original error without retrying or reclassifying it. + */ + replayEventObserver?: (event: Event) => void; } /** From 2668e3325ba89dec973c3c2f35c49efdb239de8d Mon Sep 17 00:00:00 2001 From: Karthik Kalyan <105607645+karthikscale3@users.noreply.github.com> Date: Mon, 31 Aug 2026 16:18:01 -0700 Subject: [PATCH 8/8] Durable hook resume: write, then wake (#3841) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(core): reproduce lazy resume disposal race * Fix durable hook resume race * Fail closed on unknown hook wakes * Improve unsupported hook wake diagnostics * Address durable hook resume review feedback * Harden producer-committed wake handling * Serialize durable hook resume: write, then wake resumeHook() now dispatches strictly serially: the hook_received event is made durable first, and the workflow wake is published only after the write is acknowledged. The wake is a plain runId message (the shape the sequential path always published), so the producer-committed wake barrier, its queue-message field, and the HOOK_RESUME_INPUT_VERSION bump are all removed — no consumer or backend coordination is needed, and either side rolls back independently to today's behavior. The pre-write ops flush now partitions serialization ops: producer-push uploads are awaited before the event commits (the payload must not point at bytes still in flight), while consumer-settled reader ops — a dehydrated WritableStream, e.g. a manual webhook's responseWritable — are backgrounded. Awaiting those deadlocked the resume against its own wake (webhookWorkflow failing across the whole e2e matrix). Also: wake retries stop on definitive 4xx errors instead of burning the retry budget; WORKFLOW_DISABLE_LAZY_HOOK_RESUME no longer gates anything and is ignored; the internal resumeHookDurable alias is removed. Co-Authored-By: Claude Opus 5 (1M context) * Address review: retry classification, wake dedup, 409 passthrough - Wake retry classification now actually fires against @vercel/queue: its errors carry no status field, so classify by the World's deployment-unavailable hook, then numeric status, then the queue client's definitive-4xx error names. - The wake publish carries idempotencyKey `hook-` on the claim path, so a retried publish whose response was lost dedups instead of costing a duplicate full replay. - EntityConflictError (HTTP 409) from the durable write is no longer re-keyed to HookNotFoundError: every 409 the backend emits on this write today is transient (slot conflict past the server's retry budget, claim race) and committed nothing, so it surfaces retryable instead of presenting as a permanent 404. - Stamp workflow.hook.resume_committed / wake_published span attributes after each leg resolves, making stranded resumes (committed event, no wake) queryable from traces. - Document on the public resumeHook signature that passing the token (not a cached Hook) is what makes the write idempotent-on-retry. - Changeset/changelog: note the ended-run behavior change (late webhook deliveries to finished runs now 404 instead of 202) and the 409 passthrough. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Pranay Prakash Co-authored-by: Claude Opus 5 (1M context) --- .changeset/durable-hook-resume.md | 18 + .changeset/lazy-hook-resume-vitest.md | 2 +- .../workflow-api/resume-hook.mdx | 8 +- docs/content/docs/v5/changelog/index.mdx | 2 +- .../docs/v5/changelog/lazy-hook-resume.mdx | 93 ++- .../docs/v5/changelog/resilient-resume.mdx | 14 +- .../docs/v5/configuration/runtime-tuning.mdx | 10 +- docs/content/worlds/v5/upgrading-to-v5.mdx | 2 +- .../core/src/abort-controller-step.test.ts | 29 +- packages/core/src/capabilities.ts | 24 +- packages/core/src/runtime.ts | 22 +- packages/core/src/runtime/constants.ts | 6 +- packages/core/src/runtime/helpers.ts | 10 +- .../resume-hook.consumer-preload.test.ts | 140 ++++- .../src/runtime/resume-hook.durable.test.ts | 517 ++++++++++++++++ .../src/runtime/resume-hook.fast-path.test.ts | 26 +- .../core/src/runtime/resume-hook.lazy.test.ts | 580 ------------------ packages/core/src/runtime/resume-hook.ts | 543 ++++++++-------- packages/core/src/runtime/resume-latency.ts | 19 +- packages/core/src/runtime/start.ts | 20 +- packages/core/src/serialization.test.ts | 31 + packages/core/src/serialization.ts | 67 +- .../src/telemetry/semantic-conventions.ts | 37 +- packages/vitest/src/index.ts | 13 +- .../src/storage/hook-resume-dedup.test.ts | 4 + packages/world/src/hooks.ts | 21 +- packages/world/src/interfaces.ts | 31 +- packages/world/src/queue.ts | 24 +- 28 files changed, 1186 insertions(+), 1127 deletions(-) create mode 100644 .changeset/durable-hook-resume.md create mode 100644 packages/core/src/runtime/resume-hook.durable.test.ts delete mode 100644 packages/core/src/runtime/resume-hook.lazy.test.ts diff --git a/.changeset/durable-hook-resume.md b/.changeset/durable-hook-resume.md new file mode 100644 index 0000000000..34a8aa0c6f --- /dev/null +++ b/.changeset/durable-hook-resume.md @@ -0,0 +1,18 @@ +--- +'@workflow/core': patch +'@workflow/world': patch +--- + +Make `resumeHook()` durable before it resolves: the `hook_received` event is +written durably first, and the workflow wake is published only after the write +is acknowledged. A disposal racing the queue delivery can no longer lose a +resume the caller was told succeeded. The wake message is unchanged, so no +consumer or backend coordination is needed; `WORKFLOW_DISABLE_LAZY_HOOK_RESUME` +is now a no-op and the internal `resumeHookDurable()` alias is removed. + +Behavior changes: a resume against an ended run now throws `HookNotFoundError` +instead of resolving (the lazy path never observed the server's rejection, so a +late webhook delivery to a finished run answered 202 where it now answers 404). +A transient write conflict (HTTP 409, e.g. an event-slot conflict under +contention) is no longer re-keyed to `HookNotFoundError`: it surfaces as a +retryable error, since its transaction committed nothing. diff --git a/.changeset/lazy-hook-resume-vitest.md b/.changeset/lazy-hook-resume-vitest.md index 238b92ba5e..80384bee73 100644 --- a/.changeset/lazy-hook-resume-vitest.md +++ b/.changeset/lazy-hook-resume-vitest.md @@ -2,4 +2,4 @@ '@workflow/vitest': patch --- -`waitForHook()` accepts `notHookId` to skip a hook the caller already resumed, whose `hook_received` may not be written yet. +`waitForHook()` accepts `notHookId` to exclude a previously observed hook when a workflow creates several hooks with the same token. diff --git a/docs/content/docs/v5/api-reference/workflow-api/resume-hook.mdx b/docs/content/docs/v5/api-reference/workflow-api/resume-hook.mdx index 8dfe6390e2..8f5401e3ca 100644 --- a/docs/content/docs/v5/api-reference/workflow-api/resume-hook.mdx +++ b/docs/content/docs/v5/api-reference/workflow-api/resume-hook.mdx @@ -12,9 +12,11 @@ related: Resumes a workflow run by sending a payload to a hook identified by its token. -It publishes a workflow invocation carrying the payload; the runtime creates the `hook_received` event and continues execution from it. +It durably writes the `hook_received` event and only then publishes a workflow wake. The call resolves only after both operations succeed, in that order. -`resumeHook()` throws `HookNotFoundError` when no hook holds the token. A run that has already ended cannot be resumed, including one whose Hook is kept by `experimental_minRetention`, but whether the call reports that depends on the path it takes: a resume dispatched without reading the run resolves and the ended state is only detected once the payload arrives, while one that reads the run, or that falls back to writing the event up front, throws `HookNotFoundError`. See [lazy hook resume](/docs/changelog/lazy-hook-resume). +`resumeHook()` throws `HookNotFoundError` when no hook holds the token or when its `hook_received` write is refused because the hook was disposed or the run ended. See [durable hook resume](/docs/changelog/lazy-hook-resume). + +If `resumeHook()` throws any other error, the outcome is ambiguous only in dispatch, never in durability: the event may already be durable even though the workflow wake failed, and any later wake of the run delivers it. Calling `resumeHook()` again creates a new `resumeId` and can append a second `hook_received`. Callers that need at-most-once behavior across separate invocations must retain and deduplicate their own request key. `resumeHook` is a runtime function that must be called from outside a workflow function. @@ -50,7 +52,7 @@ showSections={["parameters"]} ### Returns -Returns a `Promise`, a `Hook` extended with an optional `resilientResume` flag. Resolving means the resume was accepted for delivery: the payload rides the workflow queue message and the runtime materializes the `hook_received` event from it before replaying (see the [lazy hook resume changelog](/docs/changelog/lazy-hook-resume)). `resilientResume` is retained for source compatibility and is no longer set by any path. The resolved hook: +Returns a `Promise`, a `Hook` extended with an optional `resilientResume` flag. Resolving means the payload is durably recorded as `hook_received` and the workflow wake was accepted. `resilientResume` is retained for source compatibility and is no longer set by any path. The resolved hook: - Superseded by [lazy hook resume](/docs/changelog/lazy-hook-resume): - `resumeHook()` no longer writes the `hook_received` event at all on the fast - path, so the two-writer design and the `resilientResume` flag described below - are historical. The `(runId, resumeId)` constraint and the queue-carried - payload remain. + Superseded by [durable hook resume](/docs/changelog/lazy-hook-resume): + `resumeHook()` now writes the `hook_received` event durably and only then + publishes the workflow wake, so the two-writer design, the queue-carried + payload, and the `resilientResume` flag described below are historical. The + `(runId, resumeId)` constraint remains, converging transport-level retries + of the producer's own write (and legacy `hookInput` redeliveries from older + producers). ## Motivation @@ -27,4 +29,4 @@ description: resumeHook() now tolerates transient event storage failures when th ## Compatibility -The parallel fast path is gated per resume: it activates only when both the target run's queue consumer and the live backend independently attest dedup support (re-checked on every resume, so rollout and rollback both degrade safely). Otherwise (for oversized payloads, legacy runs, or with `WORKFLOW_DISABLE_LAZY_HOOK_RESUME=1`), `resumeHook()` falls back to the original sequential write-then-dispatch path. Because runs keep executing on the deployment they were created on, a resume targeting a run from an older deployment uses the sequential path. +The parallel fast path is gated per resume: it activates only when both the target run's queue consumer and the live backend independently attest dedup support (re-checked on every resume, so rollout and rollback both degrade safely). Otherwise (for oversized payloads or legacy runs), `resumeHook()` falls back to the original sequential write-then-dispatch path. Because runs keep executing on the deployment they were created on, a resume targeting a run from an older deployment uses the sequential path. diff --git a/docs/content/docs/v5/configuration/runtime-tuning.mdx b/docs/content/docs/v5/configuration/runtime-tuning.mdx index 45fd0c9502..624f7046b7 100644 --- a/docs/content/docs/v5/configuration/runtime-tuning.mdx +++ b/docs/content/docs/v5/configuration/runtime-tuning.mdx @@ -81,14 +81,6 @@ For example, a workflow can run a 10-minute inline step even with `WORKFLOW_REPL - Default: `3` - Recovery replays before replay divergence is recorded as corruption. -### `WORKFLOW_DISABLE_LAZY_HOOK_RESUME` - -- Default: enabled (lazy hook resume on) -- Resuming a hook publishes the workflow invocation and writes no event, so the resume costs one round trip. The queue message carries the payload, and the consumer creates the `hook_received` event from it before replay. A backend `(runId, resumeId)` constraint keeps redeliveries of that message converging on exactly one event. Because the message is the only copy of the payload, a failed publish fails the resume. -- The runtime falls back to the sequential path when the consumer or backend does not attest dedup support (or the payload is too large to inline on the queue message). On the sequential path, the event is written *before* dispatch, and its failure fails the resume. -- The two paths differ on ended runs: the sequential write is rejected and surfaces as `HookNotFoundError`, while the lazy path resolves because it never writes. Neither resumes the run. -- Set `1` to force the sequential path as a kill switch. The chosen strategy is reported on the resume span as `workflow.hook.resume_strategy`. - ### `WORKFLOW_DEPLOYMENT_MISMATCH_MAX_RETRIES` - Default: `3` @@ -100,7 +92,7 @@ For example, a workflow can run a 10-minute inline step even with `WORKFLOW_REPL ### `WORKFLOW_RESILIENT_STEP_DISPATCH` - Default: disabled -- When a suspension hands newly created steps to the queue, the runtime publishes each step's execution message in parallel with its `step_created` event write instead of sequencing them, cutting a round trip per dispatched step. The message also carries the serialized step input (`stepInput`), so a transient `step_created` write failure (429 / 5xx / transport) still executes the step. The queue consumer idempotently re-ensures the event before running it, converging with the producer's write on the step's correlation ID. This mirrors resilient start (`runInput`) and the lazy hook resume (`hookInput`). +- When a suspension hands newly created steps to the queue, the runtime publishes each step's execution message in parallel with its `step_created` event write instead of sequencing them, cutting a round trip per dispatched step. The message also carries the serialized step input (`stepInput`), so a transient `step_created` write failure (429 / 5xx / transport) still executes the step. The queue consumer idempotently re-ensures the event before running it, converging with the producer's write on the step's correlation ID. This mirrors resilient start (`runInput`) and the legacy lazy hook resume's `hookInput` (which current producers no longer send; see [durable hook resume](/docs/changelog/lazy-hook-resume)). - It is off by default because the publish races the create's verdict, and a create can come back refused: as a duplicate this replay should stop pursuing, or as a [stale write](#stale-reads-and-why-nothing-has-to-be-rejected) on a World that refuses rather than reports. Either way the message carrying the payload is already out, so the consumer can materialize a step whose create was refused, and nothing orders the verdict before the consumer's redelivery re-ensure. The sequential path is the only one that gives the message a happens-after edge over it. - Even when enabled, the runtime falls back to the sequential create-then-publish dispatch when the step input is too large to inline on the queue message, or when the run's queue transport cannot carry binary payloads (pre-CBOR spec versions). - Producer-side recoveries are reported on the suspension span as `workflow.step.resilient_dispatch_recovered`; a consumer that materialized the event reports `workflow.step.resilient_dispatch_materialized`. diff --git a/docs/content/worlds/v5/upgrading-to-v5.mdx b/docs/content/worlds/v5/upgrading-to-v5.mdx index 2ddadc5cab..7b1db92d79 100644 --- a/docs/content/worlds/v5/upgrading-to-v5.mdx +++ b/docs/content/worlds/v5/upgrading-to-v5.mdx @@ -128,7 +128,7 @@ None of this is required. Each entry is a hook the runtime uses if your World pr | `close()` | Releases connection pools and listeners so CLI commands and short-lived processes can exit without `process.exit()`. | | `streams.streamFlushIntervalMs` | Sets the stream flush window. The v5 default is `0`, so the first chunk flushes immediately; set a value to coalesce writes again. | | Hook token retention | `Hook.tokenRetentionUntil` marks the earliest time a token may become available after its run ends. Keep the owning run readable at least that long, and honor `hook_disposed` as an immediate release. Declare `capabilities.hookRetention.active` only once this is implemented, since the runtime otherwise rejects retained hooks before registration. | -| Hook resume dedup | `resumeHook()` writes `hook_received` and dispatches the queue message in parallel when the backend collapses concurrent writes carrying the same `(runId, resumeId)` onto one committed event. Declare `capabilities.hookResumeDedup` only if you enforce that constraint. A World that accepts `resumeId` without enforcing it must leave the flag unset, which keeps the sequential path. See [Resilient hook resumption](/docs/changelog/resilient-resume). | +| Hook resume dedup | `resumeHook()` writes `hook_received` and dispatches the queue message in parallel when the backend collapses concurrent writes carrying the same `(runId, resumeId)` onto one committed event. Declare `capabilities.hookResumeDedup` only if you enforce that constraint **and** `events.list()` returns the committed event's top-level `resumeId`. A World that omits either guarantee must leave the flag unset, which keeps the sequential path. See [Durable hook resume](/docs/changelog/lazy-hook-resume). | ## If you also maintain a build integration diff --git a/packages/core/src/abort-controller-step.test.ts b/packages/core/src/abort-controller-step.test.ts index 187fae0e54..1c123923a4 100644 --- a/packages/core/src/abort-controller-step.test.ts +++ b/packages/core/src/abort-controller-step.test.ts @@ -30,14 +30,6 @@ const mockStreamReads = vi.hoisted(() => ({ })); const mockResumeHook = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); -// The abort path must use the DURABLE entry point: a plain `resumeHook` only -// guarantees the resume was published (the lazy path defers the event write to -// the queue consumer), which would satisfy preCompletionOps while leaving the -// stale-signal race it exists to prevent. Mocked separately so a regression to -// `resumeHook` shows up as this staying uncalled. -const mockResumeHookDurable = vi.hoisted(() => - vi.fn().mockResolvedValue(undefined) -); // Mock version module vi.mock('./version.js', () => ({ version: '0.0.0-test' })); @@ -92,7 +84,6 @@ vi.mock('./runtime/get-world-lazy.js', () => ({ // Mock resume-hook vi.mock('./runtime/resume-hook.js', () => ({ resumeHook: mockResumeHook, - resumeHookDurable: mockResumeHookDurable, })); // ============================================================================ @@ -173,7 +164,7 @@ function reviveAbortController(opts: { if (opts.hookToken) { ctx.ops.push( (async () => { - await mockResumeHookDurable(opts.hookToken, { + await mockResumeHook(opts.hookToken, { aborted: true, reason, }); @@ -431,7 +422,7 @@ describe('AbortSignal deserialized in step context', () => { await Promise.allSettled(stepCtx.ops); - expect(mockResumeHookDurable).toHaveBeenCalledWith('abrt_test9', { + expect(mockResumeHook).toHaveBeenCalledWith('abrt_test9', { aborted: true, reason: 'hook-resume-test', }); @@ -625,16 +616,12 @@ describe('AbortSignal deserialized in step context', () => { * handler awaits before writing `step_completed`. The real-time stream write * (which reaches an in-flight sibling) stays in the background `ctx.ops`. * - * Routing alone stopped being sufficient once `resumeHook()` went lazy: it - * resolves when the resume is PUBLISHED, leaving the event write to the queue - * consumer, so awaiting it in `preCompletionOps` would re-open the same race. - * The abort path therefore calls `resumeHookDurable()`, which forces the eager - * write, and this suite pins both the routing and the entry point. + * `resumeHook()` is itself the durable barrier: it does not resolve until the + * hook_received write and the workflow wake have both completed. */ describe('step-initiated abort: durable hook resume is committed before completion', () => { beforeEach(() => { mockResumeHook.mockClear(); - mockResumeHookDurable.mockClear(); mockStreamReads.readResults.clear(); mockStreamReads.writeLog = []; mockStreamReads.closeLog = []; @@ -724,14 +711,10 @@ describe('step-initiated abort: durable hook resume is committed before completi // Draining preCompletionOps (what the step executor awaits before // step_completed) actually fires the resume with the correct payload. await Promise.all(preCompletionOps); - expect(mockResumeHookDurable).toHaveBeenCalledTimes(1); - expect(mockResumeHookDurable).toHaveBeenCalledWith('abrt_pre_completion', { + expect(mockResumeHook).toHaveBeenCalledTimes(1); + expect(mockResumeHook).toHaveBeenCalledWith('abrt_pre_completion', { aborted: true, reason: 'aborted from step', }); - // Routing is only half of it: the plain entry point would resolve as soon - // as the resume was published, so draining preCompletionOps would prove - // nothing about the event existing. - expect(mockResumeHook).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/capabilities.ts b/packages/core/src/capabilities.ts index ef7c7f74f8..099e633d25 100644 --- a/packages/core/src/capabilities.ts +++ b/packages/core/src/capabilities.ts @@ -37,11 +37,12 @@ * which a run only carries if the deployment that created it could also * open `encp`. The entry exists so the capability set stays a complete, * auditable description of a run's decoding ability. - * - Lazy hook resume ("consumer re-ensures `hook_received` from `hookInput`"): - * deliberately NOT tracked here. Rather than predict a release cutoff, the - * run's creating deployment stamps an explicit `hookResumeInputVersion` - * execution-context marker; `resumeHook()` gates the lazy path on that - * marker (mirrored onto the hook's resumeContext by the server). + * - Hook-resume consumer protocol ("consumer re-ensures `hook_received` from + * `hookInput`"): deliberately NOT tracked here. Rather than predict a + * release cutoff, the run's creating deployment stamps an explicit + * `hookResumeInputVersion` execution-context marker (mirrored onto the + * hook's resumeContext by the server); older producers gate their lazy path + * on that marker. */ import semver from 'semver'; @@ -115,12 +116,13 @@ const CAPABILITY_VERSION_TABLE: ReadonlyArray<{ // consumers that cannot unframe them (silent corruption); too-high merely // delays the optimization (safe). { capability: 'framedByteStreams', minVersion: '5.0.0-beta.15' }, - // NOTE: lazy hook resume ("does the consumer re-ensure `hook_received` from - // the queue message's `hookInput`?") is intentionally NOT gated here. A - // version-compare against a predicted release cutoff is a guess; instead the - // run's creating deployment stamps an explicit `hookResumeInputVersion` - // marker into its execution context, which the server mirrors onto the hook's - // resumeContext. `resumeHook()` gates the lazy path on that marker. + // NOTE: the hook-resume consumer protocol ("does the consumer re-ensure + // `hook_received` from the queue message's `hookInput`?") is intentionally + // NOT gated here. A version-compare against a predicted release cutoff is a + // guess; instead the run's creating deployment stamps an explicit + // `hookResumeInputVersion` marker into its execution context, which the + // server mirrors onto the hook's resumeContext. Older producers gate their + // lazy path on that marker. ]; /** diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index 7cfa388914..643f8347d1 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -434,18 +434,25 @@ async function recordFatalRunError({ } } -function hasRecordedTerminalRunEvent(events: Event[], runId: string): boolean { +function findRecordedTerminalRunEvent( + events: Event[], + runId: string +): Event | undefined { // Terminal run events are always last by construction (no event creation // succeeds against a terminal run), but scan the full array for // defense-in-depth: a World/backend ordering bug shouldn't make us miss an // actual termination signal. - const terminalRunEvent = events.find( + return events.find( (e) => e.runId === runId && (e.eventType === 'run_completed' || e.eventType === 'run_failed' || e.eventType === 'run_cancelled') ); +} + +function hasRecordedTerminalRunEvent(events: Event[], runId: string): boolean { + const terminalRunEvent = findRecordedTerminalRunEvent(events, runId); if (!terminalRunEvent) { return false; @@ -2587,13 +2594,10 @@ export function workflowEntrypoint( return; } - // Lazy hook resume: the producer (resumeHook fast path) - // parallelized the `hook_received` write with this queue - // publish, so the event may not be persisted yet. Idempotently - // ensure it before replay, keyed by `resumeId` so a - // concurrent producer write converges on exactly one event - // (the server resolves a matching claim as success, not an - // error). `hookInput` never rides a turbo first-delivery + // Legacy lazy hook resume: idempotently ensure the event from + // the payload-bearing `hookInput` before replay, keyed by + // `resumeId` so redeliveries converge on exactly one event. + // `hookInput` never rides a turbo first-delivery // (that path carries `runInput`, not `hookInput`), so this // only runs on the normal load-and-replay path. Skipped // entirely when the fast path above already ensured the diff --git a/packages/core/src/runtime/constants.ts b/packages/core/src/runtime/constants.ts index 4f09975b21..09c51e9dbd 100644 --- a/packages/core/src/runtime/constants.ts +++ b/packages/core/src/runtime/constants.ts @@ -233,8 +233,7 @@ export function getMaxInlineSteps(): number { * messages on its fast inline path instead of paying an S3 store+fetch * double-hop for bytes that already live in the event log. Above this size * the dispatch falls back to the sequential path (`step_created` write, then - * a payload-less queue message). Matches `MAX_INLINE_RESUME_PAYLOAD_BYTES` - * on the resilient hook resume path. + * a payload-less queue message). */ export const MAX_RESILIENT_STEP_INPUT_BYTES = 128 * 1024; @@ -244,7 +243,8 @@ export const MAX_RESILIENT_STEP_INPUT_BYTES = 128 * 1024; * step-execution queue publish, carrying the serialized step input in the * queue message (`stepInput`) so the consumer can idempotently re-ensure the * event if the direct write failed transiently. Mirrors the resilient start - * (`runInput`) and resilient hook resume (`hookInput`) patterns. + * (`runInput`) pattern (and the legacy lazy hook resume's `hookInput`, which + * current producers no longer send). * * **Off by default.** Enable via `WORKFLOW_RESILIENT_STEP_DISPATCH=1`. * diff --git a/packages/core/src/runtime/helpers.ts b/packages/core/src/runtime/helpers.ts index bcd6502d51..73d69cf4b6 100644 --- a/packages/core/src/runtime/helpers.ts +++ b/packages/core/src/runtime/helpers.ts @@ -108,11 +108,11 @@ export interface HealthCheckResult { * version at which the *consumer* (queue-message target) materializes the * `hook_received` event from `hookInput` on replay. A cross-deployment * `start()` stamps the *target's* value (not the caller's) into the new - * run's `executionContext.hookResumeInputVersion` so that `resumeHook()` - * only takes the lazy path when the deployment that will actually consume - * the queue message is known to honor `hookInput`. Omitted when the - * responding deployment predates this field (an older consumer that ignores - * `hookInput`), which fails the gate closed. + * run's `executionContext.hookResumeInputVersion`. Current producers write + * the event durably before publishing the wake and do not read the marker; + * OLDER producers still gate their lazy path on it, so it keeps being + * stamped. Omitted when the responding deployment predates this field, + * which fails that gate closed. */ hookResumeInputVersion?: number; } diff --git a/packages/core/src/runtime/resume-hook.consumer-preload.test.ts b/packages/core/src/runtime/resume-hook.consumer-preload.test.ts index 6cf8d69322..9d1e123ac5 100644 --- a/packages/core/src/runtime/resume-hook.consumer-preload.test.ts +++ b/packages/core/src/runtime/resume-hook.consumer-preload.test.ts @@ -25,6 +25,9 @@ import { type CreateEventParams, type CreateEventRequest, type Event, + HOOK_RESUME_DEDUP_VERSION, + HOOK_RESUME_INPUT_VERSION, + type Hook, SPEC_VERSION_CURRENT, slotToEventId, type WorkflowRun, @@ -46,6 +49,7 @@ import { dehydrateWorkflowArguments, } from '../serialization.js'; import { createContext } from '../vm/index.js'; +import { resumeHook } from './resume-hook.js'; import { setWorld } from './world.js'; vi.mock('@vercel/functions', () => ({ waitUntil: vi.fn() })); @@ -138,6 +142,12 @@ async function runResumeConsumerScenario(options: { * classification (consume the message vs rethrow for redelivery). */ reEnsureRejection?: Error; + /** + * Drive the queue message through the real (serial, durable-first) + * `resumeHook()` producer, then commit `hook_disposed` before delivering + * its wake to the consumer. + */ + disposeAfterDurableResume?: boolean; /** * When set, the queue message's hookInput carries the producer-stamped * pinned deployment id, activating the consumer's cheap pre-write @@ -182,6 +192,26 @@ async function runResumeConsumerScenario(options: { createdAt: startedAt, updatedAt: startedAt, }; + const hook: Hook = { + runId, + hookId: hookCorrelationId, + token: hookToken, + ownerId: 'owner_resume_consumer', + projectId: 'project_resume_consumer', + environment: 'production', + createdAt: startedAt, + specVersion: SPEC_VERSION_CURRENT, + resumeContext: { + deploymentId, + workflowName, + runSpecVersion: SPEC_VERSION_CURRENT, + workflowCoreVersion: '5.0.0', + hookResumeInputVersion: HOOK_RESUME_INPUT_VERSION, + }, + resumeCapabilities: { + hookResumeDedupVersion: HOOK_RESUME_DEDUP_VERSION, + }, + }; let eventIndex = 0; const event = (data: CreateEventRequest): Event => { @@ -244,12 +274,15 @@ async function runResumeConsumerScenario(options: { const createdEvents: CreateEventRequest[] = []; const createdParams: Array = []; + let reEnsureRejection = options.reEnsureRejection; - const listEvents = vi.fn(async () => ({ - data: [...durableEvents], - hasMore: false, - cursor: durableEvents.at(-1)?.eventId ?? null, - })); + const listEvents = vi.fn(async () => { + return { + data: [...durableEvents], + hasMore: false, + cursor: durableEvents.at(-1)?.eventId ?? null, + }; + }); const createEvent = vi.fn( async ( @@ -274,8 +307,8 @@ async function runResumeConsumerScenario(options: { // Simulate the write failing (terminal or transient) so the // consumer's error classification runs. Recorded in `createdEvents` // above, so the attempt is still observable to assertions. - if (options.reEnsureRejection !== undefined) { - throw options.reEnsureRejection; + if (reEnsureRejection !== undefined) { + throw reEnsureRejection; } // Converge on the producer's canonical event when it exists // (the (runId, resumeId) claim), otherwise persist ours with the @@ -343,6 +376,7 @@ async function runResumeConsumerScenario(options: { capturedHandler = handler; return vi.fn(); }), + hooks: { getByToken: vi.fn(async () => hook) }, events: { list: listEvents, create: createEvent }, runs: { get: runsGet }, queue, @@ -353,32 +387,49 @@ async function runResumeConsumerScenario(options: { await handler(new Request('http://localhost', { method: 'POST' })); expect(capturedHandler).toBeDefined(); - // A continuation delivery carrying the resume's hookInput (no runInput, so - // turbo is off and the lazy hook fast path runs). Capture whether the - // handler rethrew: on a transient failure it must reject so the queue + let delivery: unknown = { + runId, + hookInput: { + hookId: hookCorrelationId, + resumeId, + token: hookToken, + payload: payloadBytes, + payloadDigest, + ...(options.hookDeploymentId !== undefined + ? { deploymentId: options.hookDeploymentId } + : {}), + }, + }; + let resumedHook: Hook | undefined; + if (options.disposeAfterDurableResume) { + resumedHook = await resumeHook(hookToken, { value: 'hook-wins' }); + delivery = queue.mock.calls.at(-1)?.[1]; + + // The producer returned only after hook_received was durable. Commit + // disposal before delivering its payload-less wake: any hook_received + // write attempted from here on is refused, like the real server's + // disposal marker would refuse it. + reEnsureRejection = new HookNotFoundError(hookToken); + durableEvents.push( + event({ + eventType: 'hook_disposed', + specVersion: SPEC_VERSION_CURRENT, + correlationId: hookCorrelationId, + eventData: { token: hookToken }, + }) + ); + } + + // Deliver the continuation (no runInput, so turbo is off). Capture whether + // the handler rethrew: on a transient failure it must reject so the queue // redelivers; on a terminal one it resolves (consumes the message). let handlerError: unknown; try { - await capturedHandler?.( - { - runId, - hookInput: { - hookId: hookCorrelationId, - resumeId, - token: hookToken, - payload: payloadBytes, - payloadDigest, - ...(options.hookDeploymentId !== undefined - ? { deploymentId: options.hookDeploymentId } - : {}), - }, - }, - { - queueName: `__wkf_workflow_${workflowName}`, - messageId: 'msg_workflow', - attempt: 1, - } - ); + await capturedHandler?.(delivery, { + queueName: `__wkf_workflow_${workflowName}`, + messageId: 'msg_workflow', + attempt: 1, + }); } catch (err) { handlerError = err; } @@ -405,6 +456,9 @@ async function runResumeConsumerScenario(options: { createEvent, runsGet, handlerError, + durableEvents, + queue, + resumedHook, }; } @@ -659,6 +713,32 @@ describe('lazy hook resume consumer preload', () => { expect(runCompletedCreates).toHaveLength(0); }); + it('does not lose a resume when disposal commits after publish', async () => { + const { + durableEvents, + handlerError, + hookReceivedCreates, + queue, + resumedHook, + } = await runResumeConsumerScenario({ + preloadHasHookReceived: false, + disposeAfterDurableResume: true, + }); + + // resumeHook() returned only after hook_received was durable and the wake + // was accepted. Disposal may commit before the wake is consumed, but it + // cannot erase that event, and the delivery replays it from the log. + expect(resumedHook?.token).toBe('resume-consumer-token'); + expect(queue).toHaveBeenCalledTimes(1); + expect(hookReceivedCreates).toHaveLength(1); + expect(handlerError).toBeUndefined(); + + // Durability contract: a successful resume survives the queue gap. + expect( + durableEvents.filter((event) => event.eventType === 'hook_received') + ).toHaveLength(1); + }); + it('rethrows for queue redelivery when the hoisted write hits a transient conflict', async () => { // The (runId, resumeId) constraint exists but the matching event is not yet // observable — the producer's parallel write is still in flight, or a diff --git a/packages/core/src/runtime/resume-hook.durable.test.ts b/packages/core/src/runtime/resume-hook.durable.test.ts new file mode 100644 index 0000000000..bec1e5e397 --- /dev/null +++ b/packages/core/src/runtime/resume-hook.durable.test.ts @@ -0,0 +1,517 @@ +import { + EntityConflictError, + HookNotFoundError, + RunExpiredError, + WorkflowRuntimeError, +} from '@workflow/errors'; +import { + HOOK_RESUME_DEDUP_VERSION, + type Hook, + SPEC_VERSION_CURRENT, + type World, +} from '@workflow/world'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { resumeHook, resumeWebhook } from './resume-hook.js'; +import { setWorld } from './world.js'; + +vi.mock('@vercel/functions', () => ({ waitUntil: vi.fn() })); +const telemetrySpan = vi.hoisted(() => ({ + setAttributes: vi.fn(), + addLink: vi.fn(), +})); +vi.mock('../telemetry.js', () => ({ + linkToTraceCarrier: vi.fn(), + trace: vi.fn((_name, fn) => fn(telemetrySpan)), +})); + +const PAYLOAD_BYTES = new Uint8Array([1, 2, 3, 4]); +vi.mock('../serialization.js', async (importActual) => { + const actual = await importActual(); + return { + ...actual, + dehydrateStepReturnValue: vi.fn( + async ( + value: unknown, + _runId: string, + _key: unknown, + ops: Promise[] = [], + _global?: unknown, + _v1Compat?: boolean, + _framedByteStreams?: boolean, + _compression?: boolean, + _runReadyBarrier?: Promise, + readbackOps: Promise[] = ops + ) => { + if ( + typeof value === 'object' && + value !== null && + 'payloadOpRejection' in value + ) { + ops.push( + Promise.reject( + (value as { payloadOpRejection: unknown }).payloadOpRejection + ) + ); + } + // A producer-push upload op: the durability flush must await it + // before the hook_received write commits. + if ( + typeof value === 'object' && + value !== null && + 'payloadOp' in value + ) { + ops.push((value as { payloadOp: Promise }).payloadOp); + } + // A workflow readback pipe (e.g. a manual webhook's response + // writable): it only settles once the woken workflow writes into it, + // so the flush must background it, never await it. + if ( + typeof value === 'object' && + value !== null && + 'payloadReadbackOp' in value + ) { + readbackOps.push( + (value as { payloadReadbackOp: Promise }).payloadReadbackOp + ); + } + return PAYLOAD_BYTES; + } + ), + hydrateStepArguments: vi.fn(async (value: unknown) => value), + }; +}); + +describe('resumeHook durable resume', () => { + afterEach(() => { + setWorld(undefined); + vi.clearAllMocks(); + }); + + const baseHook = { + runId: 'wrun_resume', + hookId: 'hook_resume', + token: 'order:resume', + ownerId: 'owner_1', + projectId: 'project_1', + environment: 'production', + createdAt: new Date(), + specVersion: SPEC_VERSION_CURRENT, + } satisfies Hook; + + const currentContext = { + deploymentId: 'deployment_resume', + workflowName: 'processOrder', + runSpecVersion: SPEC_VERSION_CURRENT, + workflowCoreVersion: '5.0.0', + }; + + const makeWorld = ( + hook: Hook, + overrides: { + createEvent?: ReturnType; + queue?: ReturnType; + getByToken?: ReturnType; + } = {}, + capabilities: World['capabilities'] = { hookResumeDedup: true } + ) => { + const createEvent = overrides.createEvent ?? vi.fn(); + const queue = overrides.queue ?? vi.fn(); + const getByToken = overrides.getByToken ?? vi.fn().mockResolvedValue(hook); + setWorld({ + specVersion: SPEC_VERSION_CURRENT, + capabilities, + hooks: { getByToken }, + runs: { get: vi.fn() }, + events: { create: createEvent }, + getEncryptionKeyForRun: vi.fn().mockResolvedValue(undefined), + queue, + } as unknown as World); + return { createEvent, queue, getByToken }; + }; + + it('durably writes hook_received, then publishes a payload-less wake', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const { createEvent, queue } = makeWorld(hook); + + await expect(resumeHook(hook.token, { foo: 'bar' })).resolves.toMatchObject( + { + hookId: hook.hookId, + } + ); + + expect(createEvent).toHaveBeenCalledTimes(1); + const [, event, params] = createEvent.mock.calls[0]; + expect(event).toMatchObject({ + eventType: 'hook_received', + correlationId: hook.hookId, + eventData: { token: hook.token, payload: PAYLOAD_BYTES }, + }); + expect(params).toMatchObject({ + resumeId: expect.any(String), + resumePayloadDigest: expect.stringMatching(/^[0-9a-f]{64}$/), + }); + + expect(queue).toHaveBeenCalledTimes(1); + const [, wake, wakeOptions] = queue.mock.calls[0]; + // The wake is a plain trigger: the payload lives in the event log, so + // nothing rides on the queue message but the runId (+ timing metadata). + expect(wake.runId).toBe(hook.runId); + expect(wake.hookInput).toBeUndefined(); + expect(wake.hookResume).toBeUndefined(); + expect(wake.hookResumeTiming.strategy).toBe('sequential'); + // Publish retries whose response was lost dedup on the claim key, so a + // duplicate wake (one full replay of the run) is not enqueued. + expect(wakeOptions.idempotencyKey).toBe(`hook-${params.resumeId}`); + + const attributes = Object.assign( + {}, + ...telemetrySpan.setAttributes.mock.calls.map(([value]) => value) + ); + expect(attributes['workflow.hook.resume_strategy']).toBe('sequential'); + }); + + it('opens the resumeHook timing window at public entry', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const { queue } = makeWorld(hook); + const before = Date.now(); + + await resumeHook(hook.token, { foo: 'bar' }); + + const after = Date.now(); + const timing = queue.mock.calls[0][1].hookResumeTiming; + expect(timing.resumeRequestedAtMs).toBeGreaterThanOrEqual(before); + expect(timing.queuePublishRequestedAtMs).toBeGreaterThanOrEqual( + timing.resumeRequestedAtMs + ); + expect(timing.queuePublishRequestedAtMs).toBeLessThanOrEqual(after); + }); + + it('publishes the wake only after the durable write has resolved', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + let finishWrite!: () => void; + const createEvent = vi.fn( + () => new Promise((resolve) => (finishWrite = resolve)) + ); + const queue = vi.fn(); + makeWorld(hook, { createEvent, queue }); + + let resolved = false; + const resume = resumeHook(hook.token, { foo: 'bar' }).then(() => { + resolved = true; + }); + await vi.waitFor(() => { + expect(createEvent).toHaveBeenCalledTimes(1); + }); + // Write still pending: no wake, no resolution. + expect(queue).not.toHaveBeenCalled(); + expect(resolved).toBe(false); + + finishWrite(); + await resume; + expect(queue).toHaveBeenCalledTimes(1); + expect(resolved).toBe(true); + }); + + it('mints a distinct claim for each resume of a reusable hook', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const { createEvent } = makeWorld(hook); + + await resumeHook(hook.token, { foo: 'one' }); + await resumeHook(hook.token, { foo: 'two' }); + + expect(createEvent.mock.calls[0][2].resumeId).not.toBe( + createEvent.mock.calls[1][2].resumeId + ); + }); + + it('awaits producer uploads before writing or waking', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const upload = Promise.withResolvers(); + const { createEvent, queue } = makeWorld(hook); + + const resume = resumeHook(hook.token, { payloadOp: upload.promise }); + await Promise.resolve(); + await Promise.resolve(); + expect(createEvent).not.toHaveBeenCalled(); + expect(queue).not.toHaveBeenCalled(); + + upload.resolve(); + await resume; + expect(createEvent).toHaveBeenCalledTimes(1); + expect(queue).toHaveBeenCalledTimes(1); + }); + + it('does not await workflow readback pipes before writing and waking', async () => { + // The regression this pins: a manual webhook's `responseWritable` + // dehydrates into a server-stream reader that only settles once the woken + // workflow writes the response. Awaiting it ahead of the wake deadlocks + // the resume against its own wake. + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const readback = new Promise(() => {}); + const { createEvent, queue } = makeWorld(hook); + + await expect( + resumeHook(hook.token, { payloadReadbackOp: readback }) + ).resolves.toBeDefined(); + expect(createEvent).toHaveBeenCalledTimes(1); + expect(queue).toHaveBeenCalledTimes(1); + }); + + it('preserves the webhook bundle tolerance for undefined payload-op rejections', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const { createEvent, queue } = makeWorld(hook); + + await expect( + resumeHook(hook.token, { payloadOpRejection: undefined }) + ).resolves.toBeDefined(); + + expect(createEvent).toHaveBeenCalledTimes(1); + expect(queue).toHaveBeenCalledTimes(1); + }); + + it('surfaces non-undefined payload-op rejections before writing or waking', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const payloadError = new Error('payload upload failed'); + const { createEvent, queue } = makeWorld(hook); + + await expect( + resumeHook(hook.token, { payloadOpRejection: payloadError }) + ).rejects.toBe(payloadError); + + expect(createEvent).not.toHaveBeenCalled(); + expect(queue).not.toHaveBeenCalled(); + }); + + it('does not publish a wake when the durable write rejects a disposed or ended hook', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const createEvent = vi + .fn() + .mockRejectedValue(new RunExpiredError('run has expired')); + const { queue } = makeWorld(hook, { createEvent }); + + await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toSatisfy( + (error: unknown) => + HookNotFoundError.is(error) && + (error as HookNotFoundError).token === hook.token + ); + // Nothing was committed, so nothing may be dispatched. + expect(queue).not.toHaveBeenCalled(); + }); + + it('passes a transient write conflict (409) through as retryable, not HookNotFound', async () => { + // Every 409 the backend emits on this write today is transient (an + // event-slot conflict past the server's internal retry budget, or a + // resume-claim race mid-resolution) and its transaction committed + // nothing. Re-keying it to HookNotFoundError told the caller a retryable + // failure was permanent — a webhook route would answer 404 and the + // sender would drop the resume. + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const conflict = new EntityConflictError('event slot is already taken'); + const createEvent = vi.fn().mockRejectedValue(conflict); + const { queue } = makeWorld(hook, { createEvent }); + + await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toBe(conflict); + expect(HookNotFoundError.is(conflict)).toBe(false); + expect(queue).not.toHaveBeenCalled(); + }); + + it('passes a resumeId-reuse rejection through unmapped', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const reuseError = Object.assign( + new Error('resumeId reused with a different payload'), + { status: 422, code: 'hook-resume-id-reuse' } + ); + const createEvent = vi.fn().mockRejectedValue(reuseError); + const { queue } = makeWorld(hook, { createEvent }); + + await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toBe( + reuseError + ); + expect(queue).not.toHaveBeenCalled(); + }); + + it('retries the wake and resolves only after it is accepted', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const queue = vi + .fn() + .mockRejectedValueOnce(new Error('first failure')) + .mockRejectedValueOnce(new Error('second failure')) + .mockResolvedValueOnce(undefined); + const { createEvent } = makeWorld(hook, { queue }); + + await expect(resumeHook(hook.token, { foo: 'bar' })).resolves.toBeDefined(); + expect(createEvent).toHaveBeenCalledTimes(1); + expect(queue).toHaveBeenCalledTimes(3); + }); + + it.each([ + // @vercel/queue errors carry no status field; they classify by name. + [ + 'a named queue 4xx', + Object.assign(new Error('bad request'), { name: 'BadRequestError' }), + ], + [ + 'a numeric-status 4xx', + Object.assign(new Error('bad request'), { status: 400 }), + ], + ])('does not spend the wake retry budget on %s', async (_name, badRequest) => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const queue = vi.fn().mockRejectedValue(badRequest); + makeWorld(hook, { queue }); + + await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toBe( + badRequest + ); + expect(queue).toHaveBeenCalledTimes(1); + }); + + it('does not retry the wake against an unavailable deployment', async () => { + // A deployment the queue cannot discover will not come back within the + // ~125ms retry budget; the World's own classifier decides. + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const discovery = Object.assign(new Error('no consumer'), { + name: 'ConsumerDiscoveryError', + }); + const queue = vi.fn().mockRejectedValue(discovery); + const { getByToken } = makeWorld(hook, { queue }); + setWorld({ + specVersion: SPEC_VERSION_CURRENT, + capabilities: { hookResumeDedup: true }, + hooks: { getByToken }, + runs: { get: vi.fn() }, + events: { create: vi.fn() }, + getEncryptionKeyForRun: vi.fn().mockResolvedValue(undefined), + queue, + isDeploymentUnavailableError: (error: unknown) => + (error as Error)?.name === 'ConsumerDiscoveryError', + } as unknown as World); + + await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toBe( + discovery + ); + expect(queue).toHaveBeenCalledTimes(1); + }); + + it('surfaces a wake failure after the event has been made durable', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const queueError = new Error('queue unavailable'); + const queue = vi.fn().mockRejectedValue(queueError); + const { createEvent } = makeWorld(hook, { queue }); + + await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toBe( + queueError + ); + expect(createEvent).toHaveBeenCalledTimes(1); + expect(queue).toHaveBeenCalledTimes(3); + }); + + it('does not report HookNotFound when only the wake failed', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const queue = vi.fn().mockRejectedValue(new HookNotFoundError('queue')); + const { createEvent } = makeWorld(hook, { queue }); + + const error = await resumeHook(hook.token, { foo: 'bar' }).catch( + (caught) => caught + ); + + expect(error).toBeInstanceOf(WorkflowRuntimeError); + expect(HookNotFoundError.is(error)).toBe(false); + expect(createEvent).toHaveBeenCalledTimes(1); + expect(queue).toHaveBeenCalledTimes(3); + }); + + it('attaches no idempotency claim when the backend does not attest dedup', async () => { + const hook = { ...baseHook, resumeContext: currentContext } satisfies Hook; + const { createEvent, queue } = makeWorld( + hook, + {}, + { hookResumeDedup: false } + ); + + await resumeHook(hook.token, { foo: 'bar' }); + + const [, , params] = createEvent.mock.calls[0]; + expect(params.resumeId).toBeUndefined(); + expect(params.resumePayloadDigest).toBeUndefined(); + expect(queue).toHaveBeenCalledTimes(1); + }); + + it('only trusts dynamic backend capability from the current token lookup', async () => { + const hook = { + ...baseHook, + resumeContext: currentContext, + resumeCapabilities: { + hookResumeDedupVersion: HOOK_RESUME_DEDUP_VERSION, + }, + } satisfies Hook; + const first = makeWorld(hook, {}, {}); + + // Token string: the lookup is fresh, so the response-only attestation is + // trusted and the write carries the idempotency claim. + await resumeHook(hook.token, { foo: 'fresh' }); + expect(first.createEvent.mock.calls[0][2].resumeId).toBeDefined(); + + // Hook object: possibly cached before a server rollback; the stale + // attestation is ignored and the write stays claim-less. + const second = makeWorld(hook, {}, {}); + await resumeHook(hook, { foo: 'stale' }); + expect(second.createEvent.mock.calls[0][2].resumeId).toBeUndefined(); + }); + + it('uses the same durable path for webhooks', async () => { + const hook = { + ...baseHook, + isWebhook: true, + resumeContext: currentContext, + resumeCapabilities: { + hookResumeDedupVersion: HOOK_RESUME_DEDUP_VERSION, + }, + } satisfies Hook; + const { createEvent, queue } = makeWorld(hook, {}, {}); + + const response = await resumeWebhook(hook.token, new Request('http://x')); + + expect(response.status).toBe(202); + expect(createEvent).toHaveBeenCalledTimes(1); + // resumeWebhook's in-line lookup is fresh, so the claim rides the write. + expect(createEvent.mock.calls[0][2].resumeId).toBeDefined(); + const [, wake] = queue.mock.calls[0]; + expect(wake.hookResume).toBeUndefined(); + expect(wake.hookResumeTiming.strategy).toBe('sequential'); + }); + + it('opens the resumeWebhook timing window before its hook lookup', async () => { + let clock = 1_000; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock); + try { + const hook = { + ...baseHook, + isWebhook: true, + resumeContext: currentContext, + resumeCapabilities: { + hookResumeDedupVersion: HOOK_RESUME_DEDUP_VERSION, + }, + } satisfies Hook; + const { queue } = makeWorld( + hook, + { + getByToken: vi.fn(async () => { + clock += 500; + return hook; + }), + }, + {} + ); + + await resumeWebhook(hook.token, new Request('http://x')); + + const timing = queue.mock.calls[0][1].hookResumeTiming; + expect(timing.resumeRequestedAtMs).toBe(1_000); + expect( + timing.queuePublishRequestedAtMs - timing.resumeRequestedAtMs + ).toBeGreaterThanOrEqual(500); + } finally { + nowSpy.mockRestore(); + } + }); +}); diff --git a/packages/core/src/runtime/resume-hook.fast-path.test.ts b/packages/core/src/runtime/resume-hook.fast-path.test.ts index c9e1d3b09c..2b01a0daaa 100644 --- a/packages/core/src/runtime/resume-hook.fast-path.test.ts +++ b/packages/core/src/runtime/resume-hook.fast-path.test.ts @@ -163,24 +163,20 @@ describe('resumeHook (resumeContext fast path)', () => { expect(queue).not.toHaveBeenCalled(); }); - it('re-keys an EntityConflictError (compat / conflict-shaped rejection) from events.create to HookNotFoundError(token)', async () => { - // Current Vercel behavior returns 404 for a terminal run (mapped to - // HookNotFoundError, covered above). EntityConflictError is kept for - // compatibility with older / conflict-shaped (HTTP 409) rejection - // behavior. On the fast path it surfaces from events.create and must be - // re-keyed to HookNotFoundError(token), matching the pre-fast-path - // contract where resumeHook threw HookNotFoundError. + it('passes an EntityConflictError (transient, HTTP 409) from events.create through unmapped', async () => { + // Terminal runs and disposed hooks reject with 404 (mapped to + // HookNotFoundError, covered above) or RunExpiredError (below). A 409 is + // the transient shape — an event-slot conflict past the server's retry + // budget, or a resume-claim race — whose transaction committed nothing. + // The historical re-key to HookNotFoundError presented that retryable + // failure as permanent (a webhook route would 404 and the sender would + // drop the resume), so it now surfaces as-is. const hook = { ...baseHook, resumeContext } satisfies Hook; - const createEvent = vi - .fn() - .mockRejectedValue(new EntityConflictError('run has already ended')); + const conflict = new EntityConflictError('event slot is already taken'); + const createEvent = vi.fn().mockRejectedValue(conflict); const { runsGet, queue } = makeWorld(hook, { createEvent }); - await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toSatisfy( - (err: unknown) => - HookNotFoundError.is(err) && - (err as HookNotFoundError).token === hook.token - ); + await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toBe(conflict); expect(runsGet).not.toHaveBeenCalled(); expect(queue).not.toHaveBeenCalled(); }); diff --git a/packages/core/src/runtime/resume-hook.lazy.test.ts b/packages/core/src/runtime/resume-hook.lazy.test.ts deleted file mode 100644 index 5dcfa88906..0000000000 --- a/packages/core/src/runtime/resume-hook.lazy.test.ts +++ /dev/null @@ -1,580 +0,0 @@ -import { HookNotFoundError, RunExpiredError } from '@workflow/errors'; -import { - HOOK_RESUME_DEDUP_VERSION, - HOOK_RESUME_INPUT_VERSION, - type Hook, - SPEC_VERSION_CURRENT, - SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT, - type WorkflowRun, - type World, -} from '@workflow/world'; -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { dehydrateStepReturnValue } from '../serialization.js'; -import { resumeHook, resumeHookDurable, resumeWebhook } from './resume-hook.js'; -import { setWorld } from './world.js'; - -vi.mock('@vercel/functions', () => ({ waitUntil: vi.fn() })); -vi.mock('../telemetry.js', () => ({ - linkToTraceCarrier: vi.fn(), - trace: vi.fn((_name, fn) => fn(undefined)), -})); -// Return raw bytes from dehydration so `dehydratedPayload instanceof Uint8Array` -// is true and the lazy resume strategy activates. The sibling -// `resume-hook.fast-path.test.ts` returns a string and thus stays sequential; -// this file exercises the complementary lazy branch. -const PAYLOAD_BYTES = new Uint8Array([1, 2, 3, 4]); -vi.mock('../serialization.js', async (importActual) => { - const actual = await importActual(); - return { - ...actual, - dehydrateStepReturnValue: vi.fn(async () => PAYLOAD_BYTES), - hydrateStepArguments: vi.fn(async (value: unknown) => value), - }; -}); - -describe('resumeHook (lazy path)', () => { - afterEach(() => setWorld(undefined)); - - const baseHook = { - runId: 'wrun_lazy', - hookId: 'hook_lazy', - token: 'order:lazy', - ownerId: 'owner_1', - projectId: 'project_1', - environment: 'production', - createdAt: new Date(), - // Non-legacy run: v1Compat is false, so the lazy path is eligible. - specVersion: SPEC_VERSION_CURRENT, - } satisfies Hook; - - // The run carries an explicit `hookResumeInputVersion` marker (its creating - // deployment materializes the event from `hookInput`). Combined with a - // backend that declares `hookResumeDedup`, a CBOR-transport spec version, and - // a raw-byte payload, resumeHook takes the lazy path. - const lazyContext = { - deploymentId: 'deployment_lazy', - workflowName: 'processOrder', - runSpecVersion: SPEC_VERSION_CURRENT, - workflowCoreVersion: '5.0.0', - hookResumeInputVersion: HOOK_RESUME_INPUT_VERSION, - }; - - const makeWorld = ( - hook: Hook, - overrides: Partial>> = {}, - capabilities: World['capabilities'] = { hookResumeDedup: true } - ) => { - const createEvent = overrides.createEvent ?? vi.fn(); - const queue = overrides.queue ?? vi.fn(); - const getByToken = overrides.getByToken ?? vi.fn().mockResolvedValue(hook); - setWorld({ - specVersion: SPEC_VERSION_CURRENT, - capabilities, - hooks: { getByToken }, - runs: { get: vi.fn() }, - events: { create: createEvent }, - getEncryptionKeyForRun: vi.fn().mockResolvedValue(undefined), - queue, - } as unknown as World); - return { createEvent, queue, getByToken }; - }; - - it('publishes the resume on the queue and writes no hook_received event', async () => { - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { createEvent, queue } = makeWorld(hook); - - const result = await resumeHook(hook.token, { foo: 'bar' }); - // The flag is retained on the type but no longer produced by any path. - expect(result.resilientResume).toBeUndefined(); - - // The whole point of the lazy path: the producer performs no event write, - // so the resume costs one round trip. The consumer materializes - // `hook_received` from `hookInput` before it replays. - expect(createEvent).not.toHaveBeenCalled(); - - expect(queue).toHaveBeenCalledTimes(1); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.runId).toBe(hook.runId); - expect(payloadArg.hookInput).toEqual({ - // Idempotency key for the consumer's write: a redelivery of this message - // converges on the one committed event via (runId, resumeId). - resumeId: expect.any(String), - hookId: hook.hookId, - token: hook.token, - payload: PAYLOAD_BYTES, - payloadDigest: expect.stringMatching(/^[0-9a-f]{64}$/), - // The run's pinned deployment from the resume context, for the - // consumer's cheap pre-write affinity check. - deploymentId: 'deployment_lazy', - }); - }); - - it('mints a distinct resumeId per resume of the same hook', async () => { - // Two resumes of a reusable hook must not collide on the dedup constraint, - // or the second would be swallowed as a redelivery of the first and the - // run would only ever see one payload. - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { queue } = makeWorld(hook); - - await resumeHook(hook.token, { foo: 'one' }); - await resumeHook(hook.token, { foo: 'two' }); - - const [, first] = queue.mock.calls[0]; - const [, second] = queue.mock.calls[1]; - expect(first.hookInput.resumeId).not.toBe(second.hookInput.resumeId); - }); - - it('stamps the resume TTR window on the queue message', async () => { - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { queue } = makeWorld(hook); - - const before = Date.now(); - await resumeHook(hook.token, { foo: 'bar' }); - const after = Date.now(); - - const [, payloadArg] = queue.mock.calls[0]; - const timing = payloadArg.hookResumeTiming; - expect(timing.strategy).toBe('lazy'); - // T0 is entry into resumeHook and T1 the publish request, so both fall - // inside this call and in that order. - expect(timing.resumeRequestedAtMs).toBeGreaterThanOrEqual(before); - expect(timing.queuePublishRequestedAtMs).toBeGreaterThanOrEqual( - timing.resumeRequestedAtMs - ); - expect(timing.queuePublishRequestedAtMs).toBeLessThanOrEqual(after); - // The consumer boundaries belong to the consuming invocation. - expect(timing.consumerStartedAtMs).toBeUndefined(); - expect(timing.setupSource).toBeUndefined(); - }); - - it('throws when the queue publish fails', async () => { - // The message carries both the trigger and the only copy of the payload, - // so a failed publish is a failed resume with nothing persisted behind it. - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const queueErr = new Error('queue unavailable'); - const { queue } = makeWorld(hook, { - queue: vi.fn().mockRejectedValue(queueErr), - }); - - await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toBe(queueErr); - expect(queue).toHaveBeenCalledTimes(1); - }); - - it('accepts a resume against an ended run instead of throwing HookNotFoundError', async () => { - // Contract change from the write-then-publish paths: this resume runs off - // a stored `resumeContext`, so it never reads the run, and it never - // writes, so it cannot observe the server's rejection of `hook_received` - // for a terminal run either. It resolves. Nothing resumes — the consumer's - // own write is rejected the same way and the delivery is consumed. A World - // whose events.create would reject is never consulted. The complementary - // run-fallback case is the test below. - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const createEvent = vi - .fn() - .mockRejectedValue(new RunExpiredError('run has expired')); - const { queue } = makeWorld(hook, { createEvent }); - - await expect(resumeHook(hook.token, { foo: 'bar' })).resolves.toMatchObject( - { hookId: hook.hookId } - ); - expect(createEvent).not.toHaveBeenCalled(); - expect(queue).toHaveBeenCalledTimes(1); - }); - - it('still rejects an ended run when the hook carries no resumeContext', async () => { - // The lazy path removes the producer's write, not the run-fallback - // terminal pre-check. A World that serves no `resumeContext` on its hooks - // (world-local) makes every resume fetch the run, so an ended run is - // caught locally and throws before anything is published — even though - // that World statically attests dedup and would otherwise go lazy. - const hook = { ...baseHook } satisfies Hook; - const run = { - runId: hook.runId, - status: 'completed', - deploymentId: 'deployment_lazy', - workflowName: 'processOrder', - createdAt: new Date(), - updatedAt: new Date(), - attributes: {}, - specVersion: SPEC_VERSION_CURRENT, - } as unknown as WorkflowRun; - const createEvent = vi.fn(); - const queue = vi.fn(); - setWorld({ - specVersion: SPEC_VERSION_CURRENT, - capabilities: { hookResumeDedup: true }, - hooks: { getByToken: vi.fn().mockResolvedValue(hook) }, - runs: { get: vi.fn().mockResolvedValue(run) }, - events: { create: createEvent }, - getEncryptionKeyForRun: vi.fn().mockResolvedValue(undefined), - queue, - } as unknown as World); - - await expect(resumeHook(hook.token, { foo: 'bar' })).rejects.toSatisfy( - (e: unknown) => - HookNotFoundError.is(e) && (e as HookNotFoundError).token === hook.token - ); - expect(createEvent).not.toHaveBeenCalled(); - expect(queue).not.toHaveBeenCalled(); - }); - - it('resumeHookDurable writes the event before resolving, even when every lazy precondition passes', async () => { - // The runtime resumes a hook to record a step-issued abort in the event - // log, and that write is an ordering barrier: it must be committed before - // the aborting step completes, or the continuation `step_completed` - // enqueues can dispatch the next step with a stale, non-aborted signal. - // Publishing is not enough, so this entry point forces the eager write. - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { createEvent, queue } = makeWorld(hook); - - await resumeHookDurable(hook.token, { aborted: true }); - - expect(createEvent).toHaveBeenCalledTimes(1); - const [, eventArg, optsArg] = createEvent.mock.calls[0]; - expect(eventArg).toMatchObject({ - eventType: 'hook_received', - correlationId: hook.hookId, - }); - // Sequential shape: no idempotency key on the write, no hookInput on the - // message. The payload rides the event log. - expect(optsArg.resumeId).toBeUndefined(); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - expect(payloadArg.hookResumeTiming.strategy).toBe('sequential'); - }); - - it('resumeHookDurable surfaces a terminal-run rejection as HookNotFoundError', async () => { - // The barrier path keeps the older loud contract precisely because it - // writes: a resume recording an abort against an ended run must not look - // like it landed. - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const createEvent = vi - .fn() - .mockRejectedValue(new RunExpiredError('run has expired')); - const { queue } = makeWorld(hook, { createEvent }); - - await expect( - resumeHookDurable(hook.token, { aborted: true }) - ).rejects.toSatisfy( - (e: unknown) => - HookNotFoundError.is(e) && (e as HookNotFoundError).token === hook.token - ); - expect(queue).not.toHaveBeenCalled(); - }); - - it('forces the sequential path when WORKFLOW_DISABLE_LAZY_HOOK_RESUME=1 despite every other precondition passing', async () => { - // The operational kill switch must win over an otherwise fully lazy- - // eligible resume (marker present, dedup-capable backend, CBOR transport, - // raw-byte payload). Follows the SDK convention of other disable flags - // (e.g. WORKFLOW_DISABLE_COMPRESSION): enabled by default, strict '1'. - const ORIG = process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME; - process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME = '1'; - try { - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { createEvent, queue } = makeWorld(hook); - - await resumeHook(hook.token, { foo: 'bar' }); - - // Sequential: the event is written before the publish, with no - // idempotency key, and the queue message carries no hookInput — the - // payload rides the event log. - expect(createEvent).toHaveBeenCalledTimes(1); - const [, , optsArg] = createEvent.mock.calls[0]; - expect(optsArg.resumeId).toBeUndefined(); - expect(optsArg.resumePayloadDigest).toBeUndefined(); - - expect(queue).toHaveBeenCalledTimes(1); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - } finally { - if (ORIG === undefined) { - delete process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME; - } else { - process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME = ORIG; - } - } - }); - - it('does NOT force sequential for values other than the exact string "1"', async () => { - // Strict comparison: only '1' disables. A stray 'true'/'0'/'' must leave - // the lazy path enabled, matching the other WORKFLOW_DISABLE_* flags. - const ORIG = process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME; - process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME = 'true'; - try { - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { createEvent, queue } = makeWorld(hook); - - await resumeHook(hook.token, { foo: 'bar' }); - - expect(createEvent).not.toHaveBeenCalled(); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeDefined(); - } finally { - if (ORIG === undefined) { - delete process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME; - } else { - process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME = ORIG; - } - } - }); - - it('falls back to the sequential path when the payload exceeds the inline queue bound', async () => { - // A payload larger than the queue's inline ceiling would fail the publish - // on the lazy path, and with no eager write there would be nothing left of - // the resume. The size gate must instead select the sequential path, whose - // queue message carries only the run ID — the payload rides the event log. - const oversized = new Uint8Array(256 * 1024).fill(7); - vi.mocked(dehydrateStepReturnValue).mockResolvedValueOnce(oversized); - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { createEvent, queue } = makeWorld(hook); - - await resumeHook(hook.token, { foo: 'bar' }); - - expect(createEvent).toHaveBeenCalledTimes(1); - const [, , optsArg] = createEvent.mock.calls[0]; - expect(optsArg.resumeId).toBeUndefined(); - expect(optsArg.resumePayloadDigest).toBeUndefined(); - - expect(queue).toHaveBeenCalledTimes(1); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - // TTR is measured on both dispatch paths — the sequential message carries - // no hookInput but still reports its own strategy. - expect(payloadArg.hookResumeTiming).toMatchObject({ - strategy: 'sequential', - resumeRequestedAtMs: expect.any(Number), - queuePublishRequestedAtMs: expect.any(Number), - }); - }); - - it('falls back to the sequential path when the run lacks the hookResumeInput marker', async () => { - // The run's creating deployment did not stamp `hookResumeInputVersion`, so - // its queue consumer will NOT materialize hook_received from hookInput. - // Without an eager write the resume would be lost outright, so even with a - // dedup-capable backend, raw-byte payloads, and CBOR transport, resumeHook - // writes then publishes and carries neither resumeId nor hookInput. - expect(SPEC_VERSION_CURRENT).toBeGreaterThanOrEqual( - SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT - ); - const { hookResumeInputVersion: _omit, ...contextWithoutMarker } = - lazyContext; - const hook = { - ...baseHook, - resumeContext: contextWithoutMarker, - } satisfies Hook; - const { createEvent, queue } = makeWorld(hook); - - await resumeHook(hook.token, { foo: 'bar' }); - - expect(createEvent).toHaveBeenCalledTimes(1); - const [, , optsArg] = createEvent.mock.calls[0]; - expect(optsArg.resumeId).toBeUndefined(); - expect(optsArg.resumePayloadDigest).toBeUndefined(); - - expect(queue).toHaveBeenCalledTimes(1); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - }); - - it('falls back to the sequential path for a legacy (v1Compat) run', async () => { - // A legacy run omits `token` from the eagerly written event body, but the - // consumer's write always includes it — the same resume would produce a - // different event depending on which side wrote it. Legacy runs must stay - // sequential regardless of every other precondition. - const hook = { - ...baseHook, - specVersion: 1, - resumeContext: lazyContext, - } satisfies Hook; - const { createEvent, queue } = makeWorld(hook); - - await resumeHook(hook.token, { foo: 'bar' }); - - expect(createEvent).toHaveBeenCalledTimes(1); - const [, , optsArg] = createEvent.mock.calls[0]; - expect(optsArg.resumeId).toBeUndefined(); - expect(optsArg.resumePayloadDigest).toBeUndefined(); - - expect(queue).toHaveBeenCalledTimes(1); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - }); - - it('falls back to the sequential path when the backend does not declare hookResumeDedup', async () => { - // The target runtime supports lazy resume (marker present, CBOR transport, - // raw bytes) but the World backend has not opted in — e.g. Postgres, which - // has no (runId, resumeId) dedup. resumeHook must fail closed to the - // sequential path, or a queue redelivery would commit a second - // hook_received. - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { createEvent, queue } = makeWorld( - hook, - {}, - { hookResumeDedup: false } - ); - - await resumeHook(hook.token, { foo: 'bar' }); - - expect(createEvent).toHaveBeenCalledTimes(1); - const [, , optsArg] = createEvent.mock.calls[0]; - expect(optsArg.resumeId).toBeUndefined(); - expect(optsArg.resumePayloadDigest).toBeUndefined(); - - expect(queue).toHaveBeenCalledTimes(1); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - }); - - it('takes the lazy path on a dynamic backend attestation (resumeCapabilities) with no static capability', async () => { - // world-vercel no longer declares the static `hookResumeDedup`; it attests - // dedup support FRESH per by-token lookup via the response-only - // `resumeCapabilities`. The lazy path must engage on that signal alone, - // with an otherwise-empty World capability set. - const hook = { - ...baseHook, - resumeContext: lazyContext, - resumeCapabilities: { hookResumeDedupVersion: HOOK_RESUME_DEDUP_VERSION }, - } satisfies Hook; - const { createEvent, queue } = makeWorld(hook, {}, {}); - - await resumeHook(hook.token, { foo: 'bar' }); - - expect(createEvent).not.toHaveBeenCalled(); - expect(queue).toHaveBeenCalledTimes(1); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeDefined(); - }); - - it('falls back to sequential when neither the static capability nor resumeCapabilities attest dedup (rollback / kill switch)', async () => { - // A rolled-back or kill-switched server returns a hook with no - // `resumeCapabilities`, and world-vercel declares no static capability. - // With both attestations absent, resumeHook must fail closed — every new - // resume degrades to the eager write with no stranded hooks. - const hook = { ...baseHook, resumeContext: lazyContext } satisfies Hook; - const { createEvent, queue } = makeWorld(hook, {}, {}); - - await resumeHook(hook.token, { foo: 'bar' }); - - expect(createEvent).toHaveBeenCalledTimes(1); - const [, , optsArg] = createEvent.mock.calls[0]; - expect(optsArg.resumeId).toBeUndefined(); - expect(optsArg.resumePayloadDigest).toBeUndefined(); - - expect(queue).toHaveBeenCalledTimes(1); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - }); - - it('fails closed when a caller supplies a Hook object carrying resumeCapabilities (not freshly looked up)', async () => { - // The response-only `resumeCapabilities` is only trustworthy when fetched - // during THIS resume. A public caller passing a pre-fetched Hook object — - // e.g. one cached before a server rollback or kill switch, still carrying - // `hookResumeDedupVersion` — must NOT reactivate the lazy path against a - // backend that no longer dedups. Passing a Hook (not a token) skips the - // by-token lookup, so its capability is stale by construction and ignored. - const hook = { - ...baseHook, - resumeContext: lazyContext, - resumeCapabilities: { hookResumeDedupVersion: HOOK_RESUME_DEDUP_VERSION }, - } satisfies Hook; - // Empty world capabilities (world-vercel: no static hookResumeDedup) and - // getByToken deliberately NOT consulted — we pass the hook object directly. - const { createEvent, queue } = makeWorld(hook, {}, {}); - - await resumeHook(hook, { foo: 'bar' }); - - // Sequential: eager write with no idempotency key, no hookInput on the - // queue message. - expect(createEvent).toHaveBeenCalledTimes(1); - const [, , optsArg] = createEvent.mock.calls[0]; - expect(optsArg.resumeId).toBeUndefined(); - expect(optsArg.resumePayloadDigest).toBeUndefined(); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - }); - - it('resumeWebhook takes the lazy path via its internal fresh attestation on a dynamic-only backend', async () => { - // The complement to the "caller supplies a stale Hook" fail-closed test: - // `resumeWebhook` fetches the hook by token in-line (`getHookByTokenWithKey`) - // during this resume, then calls the private `resumeHookImpl` with the - // freshness attestation set. That is the ONLY path allowed to trust the - // response-only `resumeCapabilities` on a Hook object, so with no static - // world capability the lazy path must still engage — proving the - // attestation flows through the webhook entry point (which cannot be - // exercised through the public three-arg `resumeHook`). - const hook = { - ...baseHook, - isWebhook: true, - resumeContext: lazyContext, - resumeCapabilities: { hookResumeDedupVersion: HOOK_RESUME_DEDUP_VERSION }, - } satisfies Hook; - const { createEvent, queue } = makeWorld(hook, {}, {}); - - const response = await resumeWebhook(hook.token, new Request('http://x')); - // Default webhook (no `respondWith`) resolves to a 202. - expect(response.status).toBe(202); - - // Lazy: no event write, hookInput on the queue message. - expect(createEvent).not.toHaveBeenCalled(); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeDefined(); - }); - - it('opens the resumeWebhook TTR window before its own hook lookup', async () => { - // `resumeWebhook` does real producer-side work before it reaches the - // shared implementation: the by-token lookup and the run-key resolution - // it can trigger. Stamping T0 inside the implementation would silently - // exclude that, so webhook resumes would report a systematically shorter - // total than `resumeHook` ones into the same distribution. The clock is - // pinned and advanced only by the lookup, so the assertion is exact. - let clock = 1_000; - const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock); - try { - const hook = { - ...baseHook, - isWebhook: true, - resumeContext: lazyContext, - } satisfies Hook; - const { queue } = makeWorld(hook, { - // The lookup takes 500ms of wall clock. - getByToken: vi.fn(async () => { - clock += 500; - return hook; - }), - }); - - await resumeWebhook(hook.token, new Request('http://x')); - - const [, payloadArg] = queue.mock.calls[0]; - const timing = payloadArg.hookResumeTiming; - expect(timing.resumeRequestedAtMs).toBe(1_000); - expect( - timing.queuePublishRequestedAtMs - timing.resumeRequestedAtMs - ).toBeGreaterThanOrEqual(500); - } finally { - nowSpy.mockRestore(); - } - }); - - it('ignores a stale resumeCapabilities below the required dedup version', async () => { - // Forward-compat: a future server that lowers its attested version (or a - // corrupted/old field below HOOK_RESUME_DEDUP_VERSION) must not engage the - // lazy path — the version gate is a floor, not a mere presence check. - const hook = { - ...baseHook, - resumeContext: lazyContext, - resumeCapabilities: { - hookResumeDedupVersion: HOOK_RESUME_DEDUP_VERSION - 1, - }, - } satisfies Hook; - const { createEvent, queue } = makeWorld(hook, {}, {}); - - await resumeHook(hook.token, { foo: 'bar' }); - - expect(createEvent).toHaveBeenCalledTimes(1); - const [, , optsArg] = createEvent.mock.calls[0]; - expect(optsArg.resumeId).toBeUndefined(); - const [, payloadArg] = queue.mock.calls[0]; - expect(payloadArg.hookInput).toBeUndefined(); - }); -}); diff --git a/packages/core/src/runtime/resume-hook.ts b/packages/core/src/runtime/resume-hook.ts index 39c572e973..daab2013a3 100644 --- a/packages/core/src/runtime/resume-hook.ts +++ b/packages/core/src/runtime/resume-hook.ts @@ -1,5 +1,4 @@ import { - EntityConflictError, ERROR_SLUGS, HookNotFoundError, RunExpiredError, @@ -7,14 +6,12 @@ import { } from '@workflow/errors'; import { HOOK_RESUME_DEDUP_VERSION, - HOOK_RESUME_INPUT_VERSION, type Hook, type HookResumeContext, isLegacySpecVersion, isTerminalWorkflowRunStatus, SPEC_VERSION_CURRENT, SPEC_VERSION_LEGACY, - SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT, SPEC_VERSION_SUPPORTS_COMPRESSION, type WorkflowInvokePayload, type WorkflowRun, @@ -42,27 +39,10 @@ import { safeWaitUntil, waitedUntil } from './wait-until.js'; /** Monotonic ULID factory for per-call resume idempotency keys. */ const generateResumeId = monotonicFactory(); -/** - * Upper bound on the serialized hook payload that the lazy path will inline - * into the queue message's `hookInput`. Vercel Queues caps a single message at - * ~256 KiB, and the message also carries the runId, hookId, token, resumeId, - * digest, and trace carrier alongside CBOR framing overhead. Staying well under - * that ceiling keeps the queue publish from rejecting an oversized message, - * which on the lazy path would drop the resume entirely: the message is the - * only copy of the payload. Above this size we fall back to the sequential - * path, whose queue message carries only the run ID (the payload lives in the - * event log). - */ -const MAX_INLINE_RESUME_PAYLOAD_BYTES = 128 * 1024; - /** * Hex SHA-256 of the serialized resume payload bytes. Computed once by the - * producer and carried on the queue message's `hookInput`, so every delivery of - * that message records an identical digest against the server's - * `(runId, resumeId)` constraint and redeliveries converge on the one committed - * `hook_received`. Hashing the already-serialized bytes (not the raw value) - * keeps the digest stable across deliveries: the consumer forwards this string - * without recomputing. + * producer and sent with its durable `hook_received` write so transport and + * slot retries converge through the server's `(runId, resumeId)` constraint. */ async function computeResumePayloadDigest(bytes: Uint8Array): Promise { const digest = await crypto.subtle.digest('SHA-256', bytes); @@ -74,6 +54,78 @@ async function computeResumePayloadDigest(bytes: Uint8Array): Promise { return hex; } +const HOOK_WAKE_RETRY_DELAYS_MS = [25, 100] as const; + +/** + * A wake failure worth retrying is transport-shaped (network error, 5xx, + * throttle). A definitive rejection will not change on a 25ms retry, so + * spending the budget on it only delays the caller's error. + * + * `@vercel/queue` errors carry no `status` field — they are bare `Error` + * subclasses distinguished by `name` — so classification checks the World's + * deployment-unavailable hook first (a deployment the queue cannot discover + * will not come back within this function's ~125ms budget), then a numeric + * status when one exists (non-Vercel queue implementations), then the queue + * client's definitive-4xx error names. + */ +function isRetryableWakeError( + error: unknown, + isDeploymentUnavailableError?: (error: unknown) => boolean +): boolean { + if (isDeploymentUnavailableError?.(error)) return false; + const status = (error as { status?: unknown; statusCode?: unknown }) ?? {}; + const code = status.status ?? status.statusCode; + if (typeof code === 'number') { + return code >= 500 || code === 408 || code === 429; + } + const name = (error as Error | null)?.name; + return ( + name !== 'BadRequestError' && + name !== 'UnauthorizedError' && + name !== 'ForbiddenError' + ); +} + +// A publish may succeed even when its response is lost, so a retry can +// enqueue a duplicate wake. That is harmless: the event is already durable, +// and deterministic replay makes a second delivery of the same run a no-op. +async function publishHookWakeWithRetry( + publish: () => Promise, + isDeploymentUnavailableError?: (error: unknown) => boolean +): Promise { + let lastError: unknown; + for ( + let attempt = 0; + attempt <= HOOK_WAKE_RETRY_DELAYS_MS.length; + attempt++ + ) { + try { + await publish(); + return; + } catch (error) { + lastError = error; + if (!isRetryableWakeError(error, isDeploymentUnavailableError)) break; + const delayMs = HOOK_WAKE_RETRY_DELAYS_MS[attempt]; + if (delayMs !== undefined) { + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } + } + + // The wake only runs after the hook_received write committed, so a wake + // failure here is necessarily "durable but not yet dispatched": the event + // survives, and any later wake of the run delivers it. Do not let a queue + // implementation reuse HookNotFoundError and accidentally imply that no + // hook_received exists. + if (HookNotFoundError.is(lastError)) { + throw new WorkflowRuntimeError( + 'The hook resume was committed, but its workflow wake could not be published', + { cause: lastError } + ); + } + throw lastError; +} + /** * The resume context for a hook plus where it came from. `run` is present only * on the fallback path (pre-`resumeContext` hooks), where it also carries the @@ -204,12 +256,9 @@ export async function getHookByToken(token: string): Promise { * The result of {@link resumeHook}: a {@link Hook} augmented with an optional * resilience signal. * - * `resilientResume` is retained for source compatibility and is never set. It - * signalled a resume whose direct `hook_received` write had failed while the - * queue dispatch succeeded, back when the lazy path raced the two. The lazy - * path no longer writes the event at all (the queue consumer materializes it - * from `hookInput`), so there is no longer a distinction to report, and the - * sequential path never set the flag either. Treat the result as a plain + * `resilientResume` is retained for source compatibility and is never set. + * `resumeHook()` now requires the durable `hook_received` write and workflow + * wake to both succeed before it resolves. Treat the result as a plain * {@link Hook}. */ export type ResumedHook = Hook & { resilientResume?: boolean }; @@ -220,6 +269,22 @@ export type ResumedHook = Hook & { resilientResume?: boolean }; * This function is called externally (e.g., from an API route or server action) * to send data to a hook and resume the associated workflow run. * + * Resolving means BOTH that the `hook_received` event is durably recorded in + * the run's event log and that the workflow wake was accepted by the queue, in + * that order. A {@link HookNotFoundError} means this invocation committed no + * event. Any other error after the write is ambiguous only in dispatch, never + * in durability: the event may already be committed, and a later wake of the + * run (from any source) will deliver it. + * + * Prefer passing the token string over a cached {@link Hook} object. A token + * is looked up fresh, so the live backend can attest its atomic resume claim + * and the durable write becomes idempotent-on-retry (transport retries of the + * same write converge on one event). A supplied Hook object may carry a stale + * attestation, so it is deliberately ignored and the write is claim-less — + * meaning a lost response cannot be retried safely: retrying at the + * application level mints a fresh claim and can commit a second + * `hook_received`. + * * @param tokenOrHook - The unique token identifying the hook, or the hook object itself * @param payload - The data payload to send to the hook * @returns Promise resolving to the {@link ResumedHook} @@ -251,11 +316,12 @@ export async function resumeHook( // Public entry point. It never attests hook freshness, so a Hook object // supplied here (which may carry a `resumeCapabilities` cached before a // server rollback or kill switch) is ignored by the dynamic-dedup gate and - // fails closed to the sequential path. Only `resumeWebhook`, which fetches - // the hook by token in-line during the same resume, reaches the internal - // implementation with the fresh attestation set. Keeping the freshness flag - // off the exported signature prevents a caller from passing a stale Hook plus - // `true` and reactivating dynamic dedup against a rolled-back backend. + // fails closed to a plain, claim-less write. Only `resumeWebhook`, which + // fetches the hook by token in-line during the same resume, reaches the + // internal implementation with the fresh attestation set. Keeping the + // freshness flag off the exported signature prevents a caller from passing a + // stale Hook plus `true` and reactivating dynamic dedup against a + // rolled-back backend. // // T0 of the hook-resume TTR window is taken HERE, at the public entry point, // rather than inside the implementation; see the parameter's doc comment. @@ -264,42 +330,7 @@ export async function resumeHook( payload, encryptionKeyOverride, false, - Date.now(), - false - ); -} - -/** - * {@link resumeHook} with the `hook_received` event written BEFORE this - * resolves, for the one caller that needs the resume to be durable at that - * instant rather than merely dispatched. - * - * The lazy path leaves the write to the queue consumer, so a normal - * `resumeHook()` resolves while the event is still in flight. That is fine for - * an external resume, whose caller has nothing racing it. It is NOT fine for a - * resume the runtime itself issues as a barrier: a step that aborts a shared - * `AbortController` resumes the hook that records the abort in the event log, - * and that write has to land before the step completes, or the workflow - * continuation `step_completed` enqueues can dispatch the next step with a - * stale, non-aborted signal (see `reviveAbortController` in serialization.ts). - * - * Forcing the eager write costs the round trip the lazy path removes, which is - * the right trade here: this is an internal ordering barrier, not the - * latency-sensitive external resume the optimization targets. The resume span - * reports `resume_fallback_reason: durable_required`. - */ -export async function resumeHookDurable( - tokenOrHook: string | Hook, - payload: T, - encryptionKeyOverride?: PayloadKey -): Promise { - return resumeHookImpl( - tokenOrHook, - payload, - encryptionKeyOverride, - false, - Date.now(), - true + Date.now() ); } @@ -319,16 +350,13 @@ export async function resumeHookDurable( * resolution that hydrates hook metadata, and the `respondWith` setup) and * stamping locally would silently exclude all of it, so the two entry points * would report the same metric over different windows. - * @param requireDurableWrite - Force the sequential path so `hook_received` is - * committed before this resolves. See {@link resumeHookDurable}. */ async function resumeHookImpl( tokenOrHook: string | Hook, payload: T, encryptionKeyOverride: PayloadKey | undefined, hookFreshlyLookedUp: boolean, - resumeRequestedAtMs: number, - requireDurableWrite: boolean + resumeRequestedAtMs: number ): Promise { return await waitedUntil(() => { return trace('hook.resume', async (span) => { @@ -427,6 +455,7 @@ async function resumeHookImpl( // Dehydrate the payload for storage const ops: Promise[] = []; + const readbackOps: Promise[] = []; const v1Compat = isLegacySpecVersion(hook.specVersion); const dehydratedPayload = await dehydrateStepReturnValue( payload, @@ -436,17 +465,34 @@ async function resumeHookImpl( globalThis, v1Compat, capabilities.framedByteStreams, - compression + compression, + undefined, + readbackOps + ); + // A hook_received event is not durable while its payload still points + // at stream uploads in flight. Finish those before committing the + // event — but ONLY the producer-push ops in `ops`. A dehydrated + // WritableStream lands in `readbackOps` instead: it is a server-stream + // READER that resolves only once the woken workflow writes into it (a + // manual webhook's `responseWritable` is the canonical case), so + // awaiting it here would deadlock the resume against its own wake. + // + // A rejection with `undefined` is an expected artifact of the webhook + // bundle and was historically ignored by the background flush. Keep + // that tolerance now that the flush is awaited inline. + await Promise.all( + ops.map((op) => + op.catch((error) => { + if (error !== undefined) throw error; + }) + ) ); - // These payload-stream ops are flushed in the background; the - // promise handed to waitUntil must never reject (an unconsumed - // waitUntil rejection crashes the process as unhandledRejection), - // so unexpected failures are logged instead. - // NOTE: rejections with `undefined` are an expected artifact of the - // webhook bundle and are ignored entirely. - safeWaitUntil(Promise.all(ops), (err) => { + // Readback pipes (notably a manual webhook response writable) can only + // finish after the workflow wakes and writes to them. Keep them alive, + // but never place them in the durability barrier above. + safeWaitUntil(Promise.all(readbackOps), (err) => { if (err === undefined) return; - runtimeLogger.warn('Background flush of hook payload ops failed', { + runtimeLogger.warn('Background readback of hook payload failed', { workflowRunId: hook.runId, hookId: hook.hookId, error: err instanceof Error ? err.message : String(err), @@ -458,8 +504,8 @@ async function resumeHookImpl( }); // Link to the run-origin context from the stored trace carrier - // (skipped when absent or invalid). Resolved before dispatch so both - // the sequential and lazy paths attach it. + // (skipped when absent or invalid). Resolved before dispatch so the + // write and the wake both sit under a span that carries it. const originLink = await linkToTraceCarrier(resumeContext.traceCarrier); if (originLink) { span?.addLink?.(originLink); @@ -471,237 +517,133 @@ async function resumeHookImpl( specVersion: resumeContext.runSpecVersion ?? SPEC_VERSION_LEGACY, }; - // Decide whether the `hook_received` event is written lazily by the - // queue consumer (from the message's `hookInput`) or eagerly here, - // before the publish. The lazy path is only safe when EVERY - // precondition holds; the first that fails names the fallback reason - // (emitted as a span attribute for observability, and to make "why did - // this run sequential" answerable in production). All conditions: + // The dispatch is strictly serial: the hook_received event is made + // durable FIRST, and the workflow wake is published only after the + // write is acknowledged. This is what lets `resumeHook()` resolving + // mean "the resume survives anything that happens next" — a disposal + // or run completion racing the queue delivery cannot erase a committed + // event, and the wake itself carries no payload, so nothing rides on + // the message but the trigger. // - // - kill switch: `WORKFLOW_DISABLE_LAZY_HOOK_RESUME` forces sequential - // if the lazy path ever misbehaves. It is an SDK-deployment env var, - // so changing it generally requires redeploying the workflow - // deployment. (The backend can independently drop new resumes to the - // sequential path fleet-wide by ceasing to attest dedup support on - // the by-token lookup; see the backend-dedup condition below.) - // - backend dedup: the live backend must enforce the - // `(runId, resumeId)` constraint, or a queue redelivery would commit - // a second `hook_received`. Fail closed. Attested by EITHER a fresh, - // response-only `hook.resumeCapabilities.hookResumeDedupVersion` from - // the by-token lookup (world-vercel: recomputed every read, so a - // server rollback or kill switch drops to sequential immediately) OR - // the static `world.capabilities.hookResumeDedup` (world-local, whose - // adapter and backend ship together). The response-only capability is - // trusted ONLY when the hook was looked up by token during this - // resume (`hookResumeCapabilitiesAreFresh`); a Hook object handed in - // by a public caller may carry a capability cached before a rollback, - // so it is ignored and the path falls back to sequential. - // - consumer support: the target run's deployment must materialize the - // event from `hookInput` on replay. Attested by the run's explicit - // `hookResumeInputVersion` execution-context marker (mirrored onto - // resumeContext), NOT a version-compare against a predicted release - // cutoff. Absent → nothing would ever write the event, so the resume - // would be lost outright. - // - not legacy: v1Compat runs omit `token` from the eagerly written - // event body but the consumer always includes it, so a legacy run - // would get a different event depending on which path ran. Legacy - // stays sequential. - // - CBOR transport: the run must use CBOR queue transport so the binary - // payload survives the queue message. - // - raw bytes: the dehydrated payload must be a `Uint8Array` (the - // content digest that keys the dedup constraint is over these bytes). - // - size: on the lazy path the queue message carries the only copy of - // the payload, so a payload above the message ceiling would fail the - // publish and lose the resume. Oversized payloads stay sequential - // (their queue message carries only the run ID; the payload lives in - // the event log). - const lazyResumeDisabled = - process.env.WORKFLOW_DISABLE_LAZY_HOOK_RESUME === '1'; - // Backend dedup is supported when EITHER the live server attests it + // Backend dedup is attested when EITHER the live server attests it // fresh on this by-token hook (world-vercel: response-only, recomputed // every read, so rollback/kill-switch take effect immediately) OR the // static world capability is set (world-local: adapter + backend ship - // together). Both are re-evaluated per resume, so every rollout and - // rollback direction degrades safely to the sequential path. + // together). When attested, the write carries a per-call resumeId + + // payload digest so transport-level retries of the SAME write converge + // on exactly one committed event via the backend's (runId, resumeId) + // constraint. Without it the write is a plain single-shot create, + // exactly as before dedup existed. const backendDedupSupported = (hookResumeCapabilitiesAreFresh ? (hook.resumeCapabilities?.hookResumeDedupVersion ?? 0) : 0) >= HOOK_RESUME_DEDUP_VERSION || world.capabilities?.hookResumeDedup === true; - const fallbackReason: string | null = requireDurableWrite - ? // An internal caller needs the event committed before this - // resolves (an ordering barrier), which only the eager write - // provides. Checked first so the span names the real reason - // rather than whichever gate happens to fail alongside it. - 'durable_required' - : lazyResumeDisabled - ? 'disabled' - : !backendDedupSupported - ? 'backend_unsupported' - : (resumeContext.hookResumeInputVersion ?? 0) < - HOOK_RESUME_INPUT_VERSION - ? 'consumer_unsupported' - : v1Compat - ? 'legacy' - : (resumeContext.runSpecVersion ?? 0) < - SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT - ? 'non_cbor_transport' - : !(dehydratedPayload instanceof Uint8Array) - ? 'non_bytes' - : dehydratedPayload.byteLength > - MAX_INLINE_RESUME_PAYLOAD_BYTES - ? 'oversized' - : null; - const useLazyResume = fallbackReason === null; + const canClaimResume = + backendDedupSupported && + !v1Compat && + dehydratedPayload instanceof Uint8Array; span?.setAttributes({ - 'workflow.hook.resume_strategy': useLazyResume - ? 'lazy' - : 'sequential', - ...(fallbackReason - ? { 'workflow.hook.resume_fallback_reason': fallbackReason } - : {}), + 'workflow.hook.resume_strategy': 'sequential', }); - if (!useLazyResume) { - // Sequential path: create a hook_received event, then re-trigger. - // - // Re-key any "hook can no longer be received" rejection to - // HookNotFoundError(hook.token) so `.token` matches the historical - // contract, where resumeHook threw `HookNotFoundError(hook.token)` - // after its own terminal check. The specific error depends on the - // World: - // - a genuinely missing hook maps to HookNotFoundError (keyed on - // the event correlationId / hook ID); - // - a terminal run on Vercel rejects hook_received with 404, which - // world-vercel maps to HookNotFoundError; - // - a terminal run on world-local / world-postgres rejects with - // RunExpiredError. - // - // An EntityConflictError (HTTP 409) is also treated as "hook gone" - // here for historical / conflict-shaped-rejection compatibility: - // this path holds no queue message in flight, so a conflict has no - // consumer to converge on. - // - // The lazy path performs no write, so it raises none of these: see - // the note on its terminal-run behavior below. - const isHookGoneError = (err: unknown): boolean => - HookNotFoundError.is(err) || - EntityConflictError.is(err) || - RunExpiredError.is(err); - try { - await world.events.create( - hook.runId, - { - eventType: 'hook_received', - specVersion: SPEC_VERSION_CURRENT, - correlationId: hook.hookId, - eventData: { - ...(v1Compat ? {} : { token: hook.token }), - payload: dehydratedPayload, - }, - }, - { v1Compat } - ); - } catch (err) { - if (isHookGoneError(err)) { - throw new HookNotFoundError(hook.token); - } - throw err; - } + const resumeId = canClaimResume ? generateResumeId() : undefined; + const payloadDigest = canClaimResume + ? await computeResumePayloadDigest(dehydratedPayload) + : undefined; + if (resumeId) { + span?.setAttributes({ 'workflow.hook.resume_id': resumeId }); + } - // T1 of the TTR window. Stamped immediately before the publish so - // `producer_prep` covers exactly the work above it (hook lookup, - // key resolution, serialization, and, on this path, the awaited - // `hook_received` write, which is genuinely serial here). - const queuePublishRequestedAtMs = Date.now(); - await world.queue( - queueName, + // Re-key any "hook can no longer be received" rejection to + // HookNotFoundError(hook.token) so `.token` matches the historical + // contract. The specific error depends on the World: + // - a genuinely missing hook maps to HookNotFoundError (keyed on + // the event correlationId / hook ID); + // - a terminal run on Vercel rejects hook_received with 404, which + // world-vercel maps to HookNotFoundError; + // - a terminal run on world-local / world-postgres rejects with + // RunExpiredError. + // + // An EntityConflictError (HTTP 409) is deliberately NOT re-keyed, + // breaking with the historical mapping: every 409 the backend emits + // on this write today is TRANSIENT — a slot conflict that escaped the + // server's own retry budget under contention, or a resume-claim race + // mid-resolution — and its transaction committed nothing. Re-keying + // it to HookNotFoundError told the caller (and a webhook sender, via + // 404) that a retryable failure was permanent, silently dropping the + // resume. It now surfaces as-is: retryable, with nothing committed. + // (A 422 resumeId-reuse error likewise passes through unmapped — it + // means the caller replayed a resumeId with a different payload, and + // hiding that behind "not found" would mask the bug.) + const isHookGoneError = (err: unknown): boolean => + HookNotFoundError.is(err) || RunExpiredError.is(err); + try { + await world.events.create( + hook.runId, { - runId: hook.runId, - traceCarrier: resumeContext.traceCarrier ?? undefined, - hookResumeTiming: { - resumeRequestedAtMs, - queuePublishRequestedAtMs, - strategy: 'sequential', + eventType: 'hook_received', + specVersion: SPEC_VERSION_CURRENT, + correlationId: hook.hookId, + eventData: { + ...(v1Compat ? {} : { token: hook.token }), + payload: dehydratedPayload, }, - } satisfies WorkflowInvokePayload, - queueOptions + }, + { + v1Compat, + ...(resumeId && payloadDigest + ? { resumeId, resumePayloadDigest: payloadDigest } + : {}), + } ); - - return hook; + } catch (err) { + if (isHookGoneError(err)) { + throw new HookNotFoundError(hook.token); + } + throw err; } - - // Lazy path: publish the queue message and let the consumer write the - // `hook_received` event from `hookInput` before it replays. The - // producer writes nothing, so the resume costs exactly one round trip - // (the publish) instead of two, and the event is created once, by the - // side that is about to replay it. - // - // `resumeId` is the idempotency key the consumer sends with that write; - // `payloadDigest` lets the server detect key reuse across - // byte-different payloads. Both ride the message, so every redelivery - // of it converges on the one committed event via the backend's - // `(runId, resumeId)` constraint (the precondition gated above). - // - // Two things a caller could previously infer from a resolved resume, - // and can no longer: - // - // - Visibility. This returns once the publish is accepted, not once - // `hook_received` exists, so a caller that reads the run back - // immediately can see a log without it. Delivery is unaffected: the - // payload is on the message. - // - The run still being live. The hook lookup above still validates - // that a hook holds the token (and which run it belongs to), but - // `resumeContext` is an immutable slice and carries no status, and - // with no write there is no server rejection to observe. A resume - // against an ended run therefore resolves rather than throwing - // HookNotFoundError. It is reachable only while the hook outlives - // its run (minimum retention, or before the token is released); - // otherwise the lookup itself fails. Nothing resumes either way: - // the consumer's write is rejected the same way and the delivery is - // consumed. The paths that do observe the status are unchanged: the - // `run_fallback` terminal pre-check above, and the sequential - // path's own write. - const resumeId = generateResumeId(); - const payloadDigest = await computeResumePayloadDigest( - dehydratedPayload as Uint8Array - ); - span?.setAttributes({ 'workflow.hook.resume_id': resumeId }); - - // T1 of the TTR window, stamped at the instant the publish is - // requested: `producer_prep` covers exactly the work above it (hook - // lookup, key resolution, serialization) and nothing else, since no - // event write remains on this path. + // Stamped AFTER the write resolves (entry-time attributes cannot tell + // an attempted resume from a committed one): together with + // HookWakePublished below, this is what makes a stranded resume — a + // committed event whose wake never went out or was never delivered — + // queryable from traces. See the alerting note on HookWakePublished. + span?.setAttributes(Attribute.HookResumeCommitted(true)); + + // T1 of the TTR window. Stamped immediately before the publish so + // `producer_prep` covers exactly the work above it (hook lookup, key + // resolution, serialization, and the awaited hook_received write, + // which is genuinely serial here). const queuePublishRequestedAtMs = Date.now(); - await world.queue( - queueName, - { - runId: hook.runId, - traceCarrier: resumeContext.traceCarrier ?? undefined, - hookInput: { - resumeId, - hookId: hook.hookId, - token: hook.token, - payload: dehydratedPayload, - payloadDigest, - // Deployment affinity for the consumer's cheap pre-write - // check: lets a misrouted delivery re-route before its - // hoisted hook_received write instead of after. - deploymentId: resumeContext.deploymentId, - }, - hookResumeTiming: { - resumeRequestedAtMs, - queuePublishRequestedAtMs, - strategy: 'lazy', - }, - } satisfies WorkflowInvokePayload, - queueOptions + await publishHookWakeWithRetry( + () => + world.queue( + queueName, + { + runId: hook.runId, + traceCarrier: resumeContext.traceCarrier ?? undefined, + hookResumeTiming: { + resumeRequestedAtMs, + queuePublishRequestedAtMs, + strategy: 'sequential', + }, + } satisfies WorkflowInvokePayload, + { + ...queueOptions, + // Dedup retried publishes whose response was lost: a + // duplicate wake is harmless for correctness (deterministic + // replay) but costs a full replay of the run, and the queue + // accepts a repeated idempotency key by delivering only one + // of the messages. Claim-less writes have no resumeId and + // keep the previous behavior. + ...(resumeId ? { idempotencyKey: `hook-${resumeId}` } : {}), + } + ), + world.isDeploymentUnavailableError?.bind(world) ); + span?.setAttributes(Attribute.HookWakePublished(true)); - // A rejected publish propagates: the message is the only carrier of - // both the trigger and the payload, so a failed publish is a failed - // resume, with nothing persisted for a later delivery to pick up. return hook satisfies ResumedHook; } catch (err) { span?.setAttributes({ @@ -801,16 +743,9 @@ export async function resumeWebhook( // `hook` was just fetched via `getHookByTokenWithKey` (a fresh by-token // lookup) above, so its response-only `resumeCapabilities` reflects the live // backend. Call the internal implementation with the fresh attestation so - // the lazy path stays available without a second GET. (The public - // `resumeHook` never sets this, so a caller cannot forge it.) - await resumeHookImpl( - hook, - request, - encryptionKey, - true, - resumeRequestedAtMs, - false - ); + // the write's idempotency claim stays available without a second GET. (The + // public `resumeHook` never sets this, so a caller cannot forge it.) + await resumeHookImpl(hook, request, encryptionKey, true, resumeRequestedAtMs); if (responseReadable) { // Wait for the readable stream to emit one chunk, diff --git a/packages/core/src/runtime/resume-latency.ts b/packages/core/src/runtime/resume-latency.ts index 0aac11ea65..fe45bf5749 100644 --- a/packages/core/src/runtime/resume-latency.ts +++ b/packages/core/src/runtime/resume-latency.ts @@ -24,13 +24,11 @@ import * as Attribute from '../telemetry/semantic-conventions.js'; * T7 immediately before stepFn.apply() * ``` * - * On the lazy path the producer writes no `hook_received` at all (the - * consumer materializes it from `hookInput`), so the window has no producer - * write phase. On the sequential path that write is awaited inside - * `producer_prep`, and for messages from an older producer, which raced the - * write against the publish, it overlapped `producer_prep` rather than adding - * to it. Either way it has no phase of its own; it remains visible as a - * contextual span (`hook.resume`). + * The producer's `hook_received` write is awaited inside `producer_prep` + * (the wake is only published after it commits). Older producers may report + * `lazy`, where the consumer materializes the event from `hookInput`, or + * `parallel`, where the write raced the publish. The write has no phase of + * its own; it remains visible as a contextual span (`hook.resume`). * * T0/T1 are stamped on the producer's machine and T2..T7 on the consumer's, so * the measurement is subject to cross-machine clock skew. Rather than clamp @@ -45,10 +43,9 @@ export type ResumeTrigger = 'hook'; /** * Which `resumeHook()` dispatch path produced this resume. * - * `parallel` is only ever received from an older producer, which raced its own - * `hook_received` write against the publish. Current producers send `lazy` (no - * producer write: the consumer materializes the event from `hookInput`) or - * `sequential`. + * Current producers always send `sequential` (durable write, then wake). + * Older producers may send `lazy` (the consumer materializes the event from + * `hookInput`) or `parallel` (the write raced the publish). */ export type ResumeStrategy = 'lazy' | 'parallel' | 'sequential'; diff --git a/packages/core/src/runtime/start.ts b/packages/core/src/runtime/start.ts index 410a399373..83eadd56fe 100644 --- a/packages/core/src/runtime/start.ts +++ b/packages/core/src/runtime/start.ts @@ -347,10 +347,12 @@ export async function start( let framedByteStreams: boolean; let targetSupportsCompression: boolean; - // The consumer's hook-resume protocol version, stamped onto the new run - // so a later `resumeHook()` gates its lazy path on the deployment that - // will actually consume the queue message. `undefined` means "could - // not attest" and fails the gate closed. + // The consumer's hook-resume protocol version, stamped onto the new + // run. Current producers write the hook_received event durably before + // publishing the wake and never read it; OLDER producers gate their + // lazy (hookInput-carrying) path on the deployment that will actually + // consume the queue message. `undefined` means "could not attest" and + // fails that gate closed. let targetHookResumeInputVersion: number | undefined; // Public key of the target run, when the capability probe was able to // supply one (cross-deployment only). @@ -365,8 +367,8 @@ export async function start( framedByteStreams = false; targetSupportsCompression = false; // No probe channel to the target, so we cannot attest the consumer - // honors `hookInput`; leave the marker off (fail closed to - // sequential). + // honors `hookInput`; leave the marker off (older producers fail + // closed to their sequential path). targetHookResumeInputVersion = undefined; } else { // Ask for this run's public key while we're here. The probe already @@ -563,9 +565,9 @@ export async function start( features: { encryption: !!encryptionKey }, // Attest that the *consumer* deployment's runtime re-ensures a // `hook_received` event from a queue message's `hookInput` on replay. - // A resume of this run reads the marker (mirrored onto the hook's - // resumeContext by the server) to decide whether the parallel fast - // path is safe. For a cross-deployment start the consumer is the + // An OLDER producer resuming this run reads the marker (mirrored onto + // the hook's resumeContext by the server) to decide whether its lazy + // fast path is safe. For a cross-deployment start the consumer is the // target deployment, so we stamp the *target's* value carried back on // the health-check probe, never the caller's. Omitted when we could // not attest the target (older target, timeout, or no probe channel), diff --git a/packages/core/src/serialization.test.ts b/packages/core/src/serialization.test.ts index 6a9cbdfc23..1dc9198a1e 100644 --- a/packages/core/src/serialization.test.ts +++ b/packages/core/src/serialization.test.ts @@ -2312,6 +2312,37 @@ describe('workflow arguments', () => { } }); + it('separates producer uploads from workflow readback pipes', async () => { + const request = new Request('https://example.com/webhook', { + method: 'POST', + body: 'webhook payload', + duplex: 'half', + } as RequestInit); + request[Symbol.for('WEBHOOK_RESPONSE_WRITABLE')] = new WritableStream(); + const uploadOps: Promise[] = []; + const readbackOps: Promise[] = []; + + await dehydrateStepReturnValue( + request, + mockRunId, + noEncryptionKey, + uploadOps, + globalThis, + false, + false, + false, + undefined, + readbackOps + ); + + // The request body is producer -> workflow and must finish before the + // event commit. The manual response writable is workflow -> producer and + // cannot finish until after the workflow has been woken. + expect(uploadOps).toHaveLength(1); + expect(readbackOps).toHaveLength(1); + await Promise.allSettled([...uploadOps, ...readbackOps]); + }); + it('should throw error for an unsupported type', async () => { class Foo {} let err: WorkflowRuntimeError | undefined; diff --git a/packages/core/src/serialization.ts b/packages/core/src/serialization.ts index 794b6f7aff..776ff0b1c2 100644 --- a/packages/core/src/serialization.ts +++ b/packages/core/src/serialization.ts @@ -1882,7 +1882,13 @@ export function getExternalReducers( // first chunk can race `run_started`. Thread the run-ready barrier into that // sink so the write orders after the run exists. Undefined outside turbo / // on the await path. - runReadyBarrier?: Promise + runReadyBarrier?: Promise, + // Operations that read data back from the workflow into caller-owned + // writables. These must stay separate from producer uploads: a readback can + // only finish after the workflow runs, so awaiting it before dispatch would + // deadlock. Defaults to `ops` for existing callers that flush everything in + // the background. + readbackOps: Promise[] = ops ): Partial { return { ...getAllBaseReducers(global), @@ -1926,7 +1932,8 @@ export function getExternalReducers( runId, cryptoKey, framedByteStreams, - runReadyBarrier + runReadyBarrier, + readbackOps ), cryptoKey ) @@ -1981,7 +1988,7 @@ export function getExternalReducers( const streamId = ((global as any)[STABLE_ULID] || defaultUlid)(); const name = `strm_${streamId}`; const readable = new WorkflowServerReadableStream(runId, name); - ops.push(readable.pipeTo(value)); + readbackOps.push(readable.pipeTo(value)); return { name }; }, @@ -2212,7 +2219,8 @@ function getStepReducers( // after the body but within the same op flush, so its first chunk can race // `run_started`. Thread the run-ready barrier into the sink so that write // orders after the run exists. Undefined outside turbo / on the await path. - runReadyBarrier?: Promise + runReadyBarrier?: Promise, + readbackOps: Promise[] = ops ): Partial { return { ...getAllBaseReducers(global), @@ -2272,7 +2280,8 @@ function getStepReducers( runId, cryptoKey, framedByteStreams, - runReadyBarrier + runReadyBarrier, + readbackOps ), cryptoKey ) @@ -2296,11 +2305,11 @@ function getStepReducers( if (!name) { const streamId = ((global as any)[STABLE_ULID] || defaultUlid)(); name = `strm_${streamId}`; - ops.push( + readbackOps.push( new WorkflowServerReadableStream(runId, name) .pipeThrough( getDeserializeStream( - getStepRevivers(global, ops, runId, cryptoKey), + getStepRevivers(global, readbackOps, runId, cryptoKey), cryptoKey ) ) @@ -2606,21 +2615,13 @@ function reviveAbortController( // write above stays in `ops`: it must fire ASAP to reach an in-flight // sibling step and is not the durable record. // - // `resumeHookDurable`, not `resumeHook`: awaiting the latter only - // guarantees the resume was published, since the lazy path leaves the - // event write to the queue consumer. That would satisfy - // `preCompletionOps` while leaving the very race this ordering exists - // to prevent. - // // Swallow errors here so the promise can only ever enforce ordering // when awaited (see the no-reject contract on // StepContext.preCompletionOps); a failed resume retries on next replay. const hookResume = (async () => { try { - const { resumeHookDurable } = await import( - './runtime/resume-hook.js' - ); - await resumeHookDurable(value.hookToken, { + const { resumeHook } = await import('./runtime/resume-hook.js'); + await resumeHook(value.hookToken, { aborted: true, reason, }); @@ -3540,12 +3541,21 @@ export async function dehydrateWorkflowArguments( global: Record = globalThis, v1Compat = false, framedByteStreams = false, - compression = false + compression = false, + readbackOps: Promise[] = ops ): Promise { if (v1Compat) { const str = stringify( value, - getExternalReducers(global, ops, runId, key, framedByteStreams) + getExternalReducers( + global, + ops, + runId, + key, + framedByteStreams, + undefined, + readbackOps + ) ); return revive(str); } @@ -3554,7 +3564,15 @@ export async function dehydrateWorkflowArguments( const result = await clientModule.serialize(value, key, { global, extraReducers: getStreamAndRequestReducers( - getExternalReducers(global, ops, runId, key, framedByteStreams) + getExternalReducers( + global, + ops, + runId, + key, + framedByteStreams, + undefined, + readbackOps + ) ), compression, compressionStats, @@ -3763,7 +3781,8 @@ export async function dehydrateStepReturnValue( // Turbo optimistic start: order the first chunk of a returned stream after // the backgrounded `run_started`. Threaded into the step reducers' stream // sink. Undefined outside turbo / on the await path. - runReadyBarrier?: Promise + runReadyBarrier?: Promise, + readbackOps: Promise[] = ops ): Promise { if (v1Compat) { const str = stringify( @@ -3774,7 +3793,8 @@ export async function dehydrateStepReturnValue( runId, key, framedByteStreams, - runReadyBarrier + runReadyBarrier, + readbackOps ) ); return revive(str); @@ -3790,7 +3810,8 @@ export async function dehydrateStepReturnValue( runId, key, framedByteStreams, - runReadyBarrier + runReadyBarrier, + readbackOps ) ), compression, diff --git a/packages/core/src/telemetry/semantic-conventions.ts b/packages/core/src/telemetry/semantic-conventions.ts index cb445c7976..d7d4d8f394 100644 --- a/packages/core/src/telemetry/semantic-conventions.ts +++ b/packages/core/src/telemetry/semantic-conventions.ts @@ -445,22 +445,49 @@ export const HookFound = SemanticConvention('workflow.hook.found'); * the resume is recovered via the consumer's re-ensure. Corresponds to * `ResumedHook.resilientResume === true`. * - * No longer emitted: the lazy path writes no event to fail, and the sequential - * path has no queue-delivered payload to recover from. Retained so dashboards - * and queries built on the attribute keep resolving while older producers are - * still deployed. + * No longer emitted: current producers require the durable event write to + * succeed. Retained so dashboards and queries built on the attribute keep + * resolving while older producers are still deployed. */ export const HookResilientResume = SemanticConvention( 'workflow.hook.resilient_resume' ); +/** + * Producer-side signal (on the `hook.resume` span) that the durable + * `hook_received` write COMMITTED. Stamped after the write resolves, so it + * distinguishes a committed resume from an attempted one — an entry-time + * attribute cannot, because a span that later records an exception may have + * failed either before or after the commit. + */ +export const HookResumeCommitted = SemanticConvention( + 'workflow.hook.resume_committed' +); + +/** + * Producer-side signal (on the `hook.resume` span) that the workflow wake was + * ACCEPTED by the queue, stamped after the publish resolves. + * + * Together with {@link HookResumeCommitted} this makes stranded resumes + * queryable: `resume_committed=true` with `wake_published` absent is a resume + * whose durable event exists but whose wake publish failed (the caller was + * told, but nothing re-drives it), and `resume_committed=true` + + * `wake_published=true` with no subsequent workflow execution for the run is + * a wake the queue accepted but never delivered. Both are detection-only + * signals — nothing recovers such a run automatically today beyond a later + * wake from any other source. + */ +export const HookWakePublished = SemanticConvention( + 'workflow.hook.wake_published' +); + /** * Consumer-side signal (on the workflow execution span) that this replay * materialized the `hook_received` event from the queue message's `hookInput` * because no committed event was found, which completes the recovery path * {@link HookResilientResume} began. * - * Legacy / non-atomic re-ensure signal only. Atomic lazy resumes + * Legacy / non-atomic re-ensure signal only. Legacy atomic lazy resumes * (resumeId + digest) go through the hoisted preload write instead, whose * response cannot tell whether the producer or the consumer won the * `(runId, resumeId)` claim, so this attribute is deliberately NOT emitted diff --git a/packages/vitest/src/index.ts b/packages/vitest/src/index.ts index bcabf50a9a..787444d546 100644 --- a/packages/vitest/src/index.ts +++ b/packages/vitest/src/index.ts @@ -320,12 +320,8 @@ export async function waitForSleep( * filter that hasn't had a `hook_received` event. Returns the matching hook, * which you can then resume with `resumeHook(hook.token, data)`. * - * `resumeHook()` resolving does NOT mean the `hook_received` event is visible: - * on the lazy resume path the consuming invocation writes it, so the event - * appears once the run picks the resume up. A loop that resumes and then - * immediately calls this again can therefore be handed back the hook it just - * resumed. Pass `notHookId` with the hook you resumed to wait for the NEXT - * one, or await something that implies the run made progress. + * Pass `notHookId` to explicitly exclude a previously observed hook when a + * workflow creates several hooks with the same token. * * @example * ```ts @@ -360,10 +356,7 @@ export async function waitForHook( (h) => !receivedCorrelationIds.has(h.hookId) && (!options?.token || h.token === options.token) && - // Skip a hook the caller has already resumed. Its `hook_received` may - // not be written yet (the lazy resume path defers that to the - // consuming invocation), so "no hook_received" alone cannot tell a - // fresh hook from one whose payload is still in flight. + // Skip a hook the caller explicitly excluded. (!options?.notHookId || h.hookId !== options.notHookId) ); diff --git a/packages/world-local/src/storage/hook-resume-dedup.test.ts b/packages/world-local/src/storage/hook-resume-dedup.test.ts index 1159330c8f..1528716abe 100644 --- a/packages/world-local/src/storage/hook-resume-dedup.test.ts +++ b/packages/world-local/src/storage/hook-resume-dedup.test.ts @@ -79,6 +79,10 @@ describe('world-local hook_received resume dedup', () => { // detect this write already landed in its preload and skip the re-ensure. expect(first.event.resumeId).toBe('resume_1'); expect(second.event.resumeId).toBe('resume_1'); + const listed = (await storage.events.list({ runId })).data.find( + (event) => event.eventId === first.event.eventId + ); + expect(listed?.resumeId).toBe('resume_1'); }); it('keeps distinct resumes of a reusable hook as separate events', async () => { diff --git a/packages/world/src/hooks.ts b/packages/world/src/hooks.ts index cad3364357..e518cb4a41 100644 --- a/packages/world/src/hooks.ts +++ b/packages/world/src/hooks.ts @@ -29,12 +29,14 @@ export const HookResumeContextSchema = z.object({ encryptionPublicKey: z.string().optional(), // Feature marker: the version of the lazy-hook-resume consumer protocol the // run's creating deployment supports. Present (>= 1) means that deployment's - // `@workflow/core` re-ensures the `hook_received` event from the queue - // message's `hookInput` on replay, so `resumeHook()`'s lazy path is safe to - // use. Because a run is pinned to its creating deployment, this - // marker is a reliable per-run attestation, unlike inferring support from a - // version compare against a predicted release cutoff. Absent on runs created - // before the marker existed (fall back to the sequential path). + // `@workflow/core` re-ensures the `hook_received` event from a queue + // message's `hookInput` on replay. Current producers no longer send + // `hookInput` (the durable write happens before the wake is published), so + // they never read this marker; it remains stamped so OLDER producers, which + // still gate their lazy path on it, keep working against new runs. Because a + // run is pinned to its creating deployment, this marker is a reliable + // per-run attestation, unlike inferring support from a version compare + // against a predicted release cutoff. hookResumeInputVersion: z.number().optional(), }); @@ -45,9 +47,10 @@ export type HookResumeContext = z.infer; * deployment stamps this into its execution context (and the server mirrors it * onto `HookResumeContext.hookResumeInputVersion`) to attest that its * `@workflow/core` re-ensures the `hook_received` event from a queue message's - * `hookInput`. `resumeHook()`'s lazy path requires the target run's marker to - * be at least this value. Bump only on a breaking change to the - * `hookInput` re-ensure contract. + * `hookInput`. Current producers write the event durably BEFORE publishing the + * wake and do not read this marker; it exists for older producers whose lazy + * path requires the target run's marker to be at least this value. Bump only + * on a breaking change to the `hookInput` re-ensure contract. */ export const HOOK_RESUME_INPUT_VERSION = 1; diff --git a/packages/world/src/interfaces.ts b/packages/world/src/interfaces.ts index 0dc2705260..e688353996 100644 --- a/packages/world/src/interfaces.ts +++ b/packages/world/src/interfaces.ts @@ -493,19 +493,22 @@ export interface WorldCapabilities { /** * The World's `events.create` deduplicates concurrent `hook_received` writes * that carry the same `(runId, resumeId)`, collapsing them onto a single - * committed event and returning the canonical one to every caller. This is - * the backend half of `resumeHook()`'s lazy path: every delivery of one - * resume's queue message writes the same `resumeId` (a redelivery, a - * deployment-affinity re-route, or an older producer's direct write racing - * its own consumer), and exactly one event must survive or the run replays a - * duplicated `hook_received`. + * committed event and returning the canonical one to every caller. Two + * writers rely on it: `resumeHook()`'s durable write attaches a `resumeId` + + * payload digest so transport-level retries of one write converge on exactly + * one event, and legacy `hookInput` queue redeliveries (from older + * producers) converge through the same constraint. * - * The core runtime fails closed on this: the lazy path is taken ONLY when - * the World declares `hookResumeDedup === true` AND the target run's - * deployment can materialize from `hookInput` (see the execution-context - * marker `hookResumeInputVersion`). A World that accepts a `resumeId` but - * does not enforce the `(runId, resumeId)` constraint must leave this unset - * so the runtime keeps the sequential single-writer path. + * The core runtime fails closed on this: a `resumeId` is attached ONLY when + * the World declares `hookResumeDedup === true` (or the live backend attests + * it per-lookup, below). A World that accepts a `resumeId` but does not + * enforce the `(runId, resumeId)` constraint must leave this unset so the + * runtime keeps the plain single-shot write. + * + * Declaring this also commits the World to ROUND-TRIPPING the key: + * `events.list` must return `resumeId` on `hook_received` events it + * persisted with one, because the legacy `hookInput` consumer path detects + * an already-materialized resume by matching `resumeId` in the loaded log. * * Enabled statically for `world-local` (filesystem sidecar claim keyed on * `(runId, resumeId)`; the adapter and its backend ship together, so a static @@ -513,8 +516,8 @@ export interface WorldCapabilities { * leaves this UNSET and instead attests support per-lookup via the * server-computed, response-only `Hook.resumeCapabilities.hookResumeDedupVersion` * (see `HookResumeCapabilitiesSchema`), so a server rollback or kill switch - * degrades new resumes to the sequential path immediately without redeploying - * the adapter. `world-postgres` leaves it unset for now and stays sequential. + * degrades new resumes to plain writes immediately without redeploying + * the adapter. `world-postgres` leaves it unset for now. * * The resume gate treats EITHER signal as backend support (see * `resume-hook.ts`): this static capability OR a current diff --git a/packages/world/src/queue.ts b/packages/world/src/queue.ts index 912c19f065..1f795f8dae 100644 --- a/packages/world/src/queue.ts +++ b/packages/world/src/queue.ts @@ -135,12 +135,11 @@ export const RunInputSchema = z.object({ export type RunInput = z.infer; /** - * Lazy hook resume data carried through the queue alongside a workflow - * invocation. Present only when `resumeHook()` takes the lazy path, where the - * producer publishes this invocation and writes no event of its own. On - * receipt, a consumer that understands `hookInput` idempotently ensures the - * `hook_received` event exists (keyed by `resumeId`) before replaying, so - * repeated deliveries of the same message converge on exactly one event. + * Legacy lazy hook resume data carried through the queue alongside a workflow + * invocation. Older producers publish this invocation and write no event of + * their own. On receipt, a consumer that understands `hookInput` idempotently + * ensures the `hook_received` event exists (keyed by `resumeId`) before + * replaying, so repeated deliveries converge on exactly one event. * * The `payload` is the already-serialized (and possibly encrypted) resume * payload, and on this path the queue message is its only carrier. Every write @@ -251,8 +250,9 @@ export const HookResumeTimingSchema = z.object({ /** Epoch ms immediately before the queue publish was requested. */ queuePublishRequestedAtMs: z.number(), /** - * Which `resumeHook()` dispatch path ran: `lazy` or `sequential` (`parallel` - * from producers predating lazy-only resume). + * Which `resumeHook()` dispatch path ran. Current producers always report + * `sequential` (durable write, then wake); older producers may report + * `lazy` or `parallel`. */ strategy: z.string().optional(), /** Epoch ms the final consumer's queue handler was entered. */ @@ -331,9 +331,9 @@ export const WorkflowInvokePayloadSchema = z.object({ /** Run creation data, only present on the first queue delivery from start() */ runInput: RunInputSchema.optional(), /** - * Lazy hook resume data, only present when `resumeHook()` takes the parallel - * fast path. A consumer that understands this field idempotently ensures the - * `hook_received` event exists (keyed by `resumeId`) before replaying. + * Legacy lazy hook resume data. A consumer that understands this field + * idempotently ensures the `hook_received` event exists (keyed by `resumeId`) + * before replaying. */ hookInput: HookResumeInputSchema.optional(), /** @@ -345,7 +345,7 @@ export const WorkflowInvokePayloadSchema = z.object({ stepInput: StepDispatchInputSchema.optional(), /** * Hook-resume TTR timing. Present on both `resumeHook()` dispatch paths - * (unlike `hookInput`, which only rides the lazy path), and + * (unlike legacy `hookInput`), and * forwarded onto a dispatched step message when the resuming invocation * hands the next durable step to another invocation. Purely observational. * See {@link HookResumeTimingSchema}.