diff --git a/.changeset/per-kind-correlation-ids.md b/.changeset/per-kind-correlation-ids.md deleted file mode 100644 index 2e13a31a73..0000000000 --- a/.changeset/per-kind-correlation-ids.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -'@workflow/core': patch -'workflow': patch ---- - -Add env option to split correlation ID derivation into per-entity-type sequential ULIDs, instead of sharing one derivation source diff --git a/.changeset/slot-event-ids.md b/.changeset/slot-event-ids.md new file mode 100644 index 0000000000..70a6bbb897 --- /dev/null +++ b/.changeset/slot-event-ids.md @@ -0,0 +1,9 @@ +--- +'@workflow/world-postgres': patch +'@workflow/world-vercel': patch +'@workflow/world-local': patch +'@workflow/core': patch +'@workflow/world': patch +--- + +**Breaking**: SpecVersion 6: Event IDs are now a dense per-run slot number, allocated by the world at publish time so a rejected write leaves no gap in the event log. A replay tells the world how many events it had read and gets back the ones it did not see, so an event that arrives from outside the replay and lands ahead of an event the replay wrote no longer fails the run with `CORRUPTED_EVENT_LOG`: it is held for whichever part of the workflow awaits it. A gap in the numbering fails the run instead of being replayed over. diff --git a/docs/content/docs/v5/configuration/runtime-tuning.mdx b/docs/content/docs/v5/configuration/runtime-tuning.mdx index 5811dd1ed9..53a59e7759 100644 --- a/docs/content/docs/v5/configuration/runtime-tuning.mdx +++ b/docs/content/docs/v5/configuration/runtime-tuning.mdx @@ -83,6 +83,14 @@ For example, a workflow can run a 10-minute inline step even with `WORKFLOW_REPL - As a result, 412 volume describes a workload rather than the health of a deployment, and a run that never sees one is not evidence the guard is inactive. - Set `0` to disable. +### `WORKFLOW_SLOT_GAP_CHECK` + +- Default: enabled +- A replay checks that the [event log](/docs/how-it-works/event-sourcing#event-ids) it loaded is dense before it runs, and fails the run with [`CORRUPTED_EVENT_LOG`](/docs/errors/corrupted-event-log) when a position below the log's highest holds no event. A log missing only its first position, meaning a run whose `run_created` is still being written, is left alone. +- A position can be briefly empty while the write that occupies it is still committing, so the check re-reads the log a few times before it decides, and the replay continues from whichever log it settled on. +- The check trades one failure for another. Most holes stand for an event that never happened, and replaying past those is correct. A hole standing for an event that did happen looks identical, and replaying past that one produces a run whose result is silently wrong. Failing is the recoverable side of that trade. +- Set `0` to replay across holes instead. + ### `WORKFLOW_PRECONDITION_MAX_INPROCESS_RESTARTS` - Default: `3` @@ -101,17 +109,6 @@ For example, a workflow can run a 10-minute inline step even with `WORKFLOW_REPL - Delay before a re-invocation caused by a rejected event creation. - Unlike an in-process restart, which re-reads immediately, a re-invocation only happens once the in-process budget failed to catch up — so the delay gives the other writers a moment to quiesce. -### `WORKFLOW_PER_KIND_CORRELATION_IDS` - -- Default: disabled -- Experimental. Gives each kind of entity a workflow creates — steps, waits, hooks, attribute writes, abort controllers, stream IDs — its own sequence of correlation IDs. -- With one sequence shared by every kind, an ID is an ordinal over the whole run, so a single extra draw of any kind shifts every ID after it. Two concurrent replays of the same run that disagree about one `sleep()` then assign different IDs to every step that follows, and each writes events the other can neither match nor consume, which fails the run with `CORRUPTED_EVENT_LOG`. Per-kind sequences confine that to the kind that actually differs. -- IDs remain ordered within a kind, so hooks created by your workflow are still listed in creation order. A hook the runtime creates for you, such as the one backing an abort controller, draws from its own kind and so is listed at an arbitrary position relative to your hooks rather than at its creation position. -- A run must replay under the scheme that minted its IDs. A replay that switches schemes mid-run assigns IDs its own earlier events do not carry, so it can consume none of them and the run fails. - - On Vercel, a run keeps replaying on the deployment it started on, so it only ever sees the value baked into that deployment. Changing the setting affects new runs only. - - Elsewhere — `@workflow/world-postgres`, `@workflow/world-local`, any self-hosted process — nothing pins a run to the code that started it. Turn the setting on during a quiet window with no runs in flight, and roll the new value out to your whole fleet at once: a rolling deploy that leaves both values live replays one run under two schemes concurrently, which is the failure the setting exists to reduce. -- Set `1` to enable. - ## Inline execution ### `WORKFLOW_V2_TIMEOUT_MS` diff --git a/docs/content/docs/v5/errors/corrupted-event-log.mdx b/docs/content/docs/v5/errors/corrupted-event-log.mdx index efd1028cde..f8d31bdb05 100644 --- a/docs/content/docs/v5/errors/corrupted-event-log.mdx +++ b/docs/content/docs/v5/errors/corrupted-event-log.mdx @@ -21,15 +21,18 @@ Workflow replay diverged times after reco ## Why This Happens -Workflows persist their progress as an ordered event log. During replay, the runtime processes each event in sequence — every event must be consumed by a matching callback (e.g., a step or sleep waiting for its result). When an event has no matching consumer, the runtime cannot advance past it, which would block all subsequent events and hang the workflow indefinitely. +Workflows persist their progress as an ordered event log. During replay, the runtime processes each event in sequence — every event must be consumed by a matching callback (e.g., a step or sleep waiting for its result). An event no callback ever claims is one the runtime would have to drop to finish the run, so it fails the run instead of returning a result that silently ignored it. -Instead of silently hanging, the runtime retries a divergent replay before failing the workflow and surfacing this terminal error. +A delivery written from outside the replay, such as a hook firing or a step completing on another invocation, can land ahead of the events the replay is writing itself. That is ordinary concurrency rather than corruption, so the runtime holds such an event and offers it to each consumer the replay registers afterwards. The failure comes only when the workflow function returns while an event is still held, at which point no consumer can ever appear. A replay that suspends still holding one reports it on the span (`workflow.events.parked.count`, `.event_id`, `.event_type`) and leaves the decision to the replay that follows. + +Before failing, the runtime retries a divergent replay and surfaces this terminal error only if replay still cannot recover. Common scenarios that produce this error: -1. **Duplicate completion events** — Two `wait_completed` events for a single `wait_created`, or two `step_completed` events for the same step. The first is consumed normally, but the second has no consumer. -2. **Orphaned events** — A `step_completed` or `wait_completed` event whose `correlationId` doesn't match any step or sleep in the workflow code. +1. **Duplicate completion events** — Two `wait_completed` events for a single `wait_created`, or two `step_completed` events for the same step. The first is consumed normally, and the second resolves something already resolved, so no later consumer can claim it. +2. **Orphaned events** — A `step_completed` or `wait_completed` event whose `correlationId` doesn't match any step or sleep in the workflow code, so the replay reaches its end still holding it. 3. **Events after terminal state** — An event that arrives after its corresponding step or wait has already reached a terminal state (e.g., `step_retrying` after `step_completed`). +4. **A hole in the log** — Events are numbered by their position in the run's log, and those positions are dense, so a position below the log's highest that holds no event means the log the replay loaded is incomplete. The runtime cannot tell a position no write ever occupied from one whose event it failed to read, so it refuses to replay rather than produce a result that may be silently wrong. See [`WORKFLOW_SLOT_GAP_CHECK`](/docs/configuration/runtime-tuning#workflow_slot_gap_check). ## What To Do diff --git a/docs/content/docs/v5/how-it-works/event-sourcing.mdx b/docs/content/docs/v5/how-it-works/event-sourcing.mdx index 70ae0baf5c..31febb849a 100644 --- a/docs/content/docs/v5/how-it-works/event-sourcing.mdx +++ b/docs/content/docs/v5/how-it-works/event-sourcing.mdx @@ -260,7 +260,7 @@ On Vercel, `requestId` is the platform request ID when available. Other worlds a ## Entity IDs -All entities in the Workflow SDK use a consistent ID format: a 4-character prefix followed by an underscore and a [ULID](https://github.com/ulid/spec) (Universally Unique Lexicographically Sortable Identifier). +All entities in the Workflow SDK use a consistent ID format: a 4-character prefix followed by an underscore and a fixed-width body. For every entity except events, that body is a [ULID](https://github.com/ulid/spec) (Universally Unique Lexicographically Sortable Identifier). An event's body is its slot number, described below. | Entity | Prefix | Example | |--------|--------|---------| @@ -268,11 +268,19 @@ All entities in the Workflow SDK use a consistent ID format: a 4-character prefi | Step | `step_` | `step_01HXYZ123ABC456DEF789GHJ` | | Hook | `hook_` | `hook_01HXYZ123ABC456DEF789GHJ` | | Wait | `wait_` | `wait_01HXYZ123ABC456DEF789GHJ` | -| Event | `evnt_` | `evnt_01HXYZ123ABC456DEF789GHJ` | +| Event | `evnt_` | `evnt_00000000000000000000000042` (slot 42) | | Stream | `strm_` | `strm_01HXYZ123ABC456DEF789GHJ` | **Why this format?** - **Prefixes enable introspection**: Given any ID, you can immediately identify what type of entity it refers to. This makes debugging, logging, and cross-referencing entities across the system straightforward. -- **ULIDs enable chronological ordering**: Unlike UUIDs, ULIDs encode a timestamp in their first 48 bits, making them lexicographically sortable by creation time. This property is essential for the event log—events are always stored and retrieved in the correct chronological order simply by sorting their IDs. +- **Fixed-width bodies enable ordering**: Unlike UUIDs, these bodies sort lexicographically in creation order, so the event log is stored and retrieved in the correct order by sorting IDs alone. Slot numbers get that from counting at a fixed width, which makes string order the same as numeric order. ULIDs get it from the timestamp in their first 48 bits, which also makes a ULID's creation time recoverable from the ID itself. + +### Event IDs + +An event ID is a **slot number**: the event's 1-based position in the run's event log, zero-padded to the same width as a ULID. The world assigns it when the event is published, so two writers racing to append never claim the same position and a rejected write leaves no gap behind. Slots are dense, and unique only within a run, so an event ID identifies an event only when paired with its `runId`. + +Density is what lets a reader tell a complete log from an incomplete one by its length alone. A replay that loads a log with a position missing below the highest one it can see cannot tell an event that was never written from one it failed to read, so it fails the run with [`CORRUPTED_EVENT_LOG`](/docs/errors/corrupted-event-log) rather than replay across the hole. See [`WORKFLOW_SLOT_GAP_CHECK`](/docs/configuration/runtime-tuning#workflow_slot_gap_check). + +A slot ID carries no timestamp. Zero-padded decimal digits are a subset of the ULID alphabet, so a slot ID passes ULID validation and sorts correctly, but decoding its first 48 bits yields the Unix epoch instead of a creation time. Read `createdAt` on the event when you need to know when it was written, and don't decode the time from an `evnt_` ID you get back from an API, a log line, or a cursor. diff --git a/docs/content/worlds/v5/building-a-world.mdx b/docs/content/worlds/v5/building-a-world.mdx index 619fad3fc9..33062c2044 100644 --- a/docs/content/worlds/v5/building-a-world.mdx +++ b/docs/content/worlds/v5/building-a-world.mdx @@ -36,6 +36,8 @@ interface WorldCapabilities { hookRetention?: { active: boolean; }; + slotEventIds?: boolean; + preconditionGuard?: boolean; } interface World extends Storage, Queue, Streamer { @@ -47,7 +49,7 @@ interface World extends Storage, Queue, Streamer { } ``` -The optional `capabilities` object advertises additional behavior. Set `hookRetention.active` to `true` only when the World implements Hook token retention. The optional `start()` method initializes background tasks (for example, queue polling). The optional `close()` method releases resources like connection pools and listeners. The optional `getEncryptionKeyForRun()` method returns the AES-256 key used to encrypt data for a run; if it is not implemented, encryption is disabled. +The optional `capabilities` object advertises additional behavior. Set `hookRetention.active` to `true` only when the World implements Hook token retention, `slotEventIds` when it allocates [slot-numbered event IDs](#event-id-allocation), and `preconditionGuard` when it enforces the [precondition guard](#optional-the-event-creation-precondition-guard). The optional `start()` method initializes background tasks (for example, queue polling). The optional `close()` method releases resources like connection pools and listeners. The optional `getEncryptionKeyForRun()` method returns the AES-256 key used to encrypt data for a run; if it is not implemented, encryption is disabled. ## The Event Log Model @@ -106,6 +108,19 @@ Keep the owning Run available for at least as long as its token remains unavaila **Automatic Hook Cleanup:** When a run ends, remove its live Hooks. Make each token available unless its `tokenRetentionUntil` is still in the future. A `hook_disposed` event always makes the token available immediately. +### Event ID Allocation + +Your World assigns every event ID. An ID is `evnt_` followed by the event's 1-based position in that run's log, zero-padded to 26 characters, so the run's first event is `evnt_00000000000000000000000001`. Use `slotToEventId()` from `@workflow/world` to format one, and declare `capabilities.slotEventIds`. + +Two properties have to hold, and both are about what a reader can conclude from the log: + +- **Uniqueness.** Two writers racing to append must not both take a position. Settle it where the store settles it, with a unique constraint on `(runId, eventId)` or a conditional write, rather than reading the maximum and adding one in your own process. +- **Density.** Positions run from 1 with no holes, which is what lets a reader tell a complete log from a truncated one by its length alone. A writer that loses a race must re-derive its position from the store and take the next free one. Incrementing a local number after a loss leaves a permanent hole, and the runtime treats a hole as a log it cannot safely replay across. + +`events.create()` params carry `eventCount`: how many events the writer held in the log it replayed from, which is the position it expects to land on minus one. Attempt `eventCount + 1`. When that position is taken, **do not reject the write**. Advance to the next free position, commit there, and return the events occupying the positions you skipped over on the success response, in `events` with a matching `cursor` and `hasMore`. The writer merges them into its own log and replays once, rather than paying a second round trip to discover it was behind. A caller with a stale count is the normal case for a fan-out, and rejecting it would serialize writes the runtime issues in parallel. + +`eventCount` supersedes the `stateUpdatedAt` / `stateEventCount` / `stateCursor` triple below for a World that allocates positions. The triple approximates a position with a timestamp watermark plus a count of events at or below it, which a complete-but-stale snapshot passes: every event the writer holds is at or below its own watermark, so the count matches and no fence fires. A dense position has no such blind spot. + ### Optional: The Event Creation Precondition Guard A replay writes events derived from the event log it loaded, so a write made from an event log that no longer matches the store can commit events no correct replay would produce. To let a World fence those writes, `events.create()` params may carry a description of the snapshot the caller replayed from: diff --git a/docs/content/worlds/v5/postgres.mdx b/docs/content/worlds/v5/postgres.mdx index a9d3c5172e..82757b83fd 100644 --- a/docs/content/worlds/v5/postgres.mdx +++ b/docs/content/worlds/v5/postgres.mdx @@ -27,8 +27,8 @@ Use the same release channel for `workflow` and `@workflow/world-postgres`. If your app uses a beta or other prerelease Workflow version, install the matching prerelease Postgres World package, such as `npm install @workflow/world-postgres@beta`. Mismatched versions fail before -starting a run with an error that says the runtime requires a World with a -matching spec version. +starting a run with an error that names the spec versions the runtime supports +and the one the World declares. Configure the required environment variables to use the world and point it to your PostgreSQL database: diff --git a/packages/core/src/abort-consistency.test.ts b/packages/core/src/abort-consistency.test.ts index d99b67de1d..ec93e93227 100644 --- a/packages/core/src/abort-consistency.test.ts +++ b/packages/core/src/abort-consistency.test.ts @@ -11,10 +11,6 @@ import type { Event, WorkflowRun } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { describe, expect, it, vi } from 'vitest'; -import { - createCorrelationIdGenerator, - isPerKindCorrelationIdsEnabled, -} from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import type { WorkflowSuspension } from './global.js'; import type { WorkflowOrchestratorContext } from './private.js'; @@ -44,16 +40,13 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { replayPayloadCache: new ReplayPayloadCache(undefined), globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: () => {}, getPromiseQueue: () => Promise.resolve(), }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - perKind: isPerKindCorrelationIdsEnabled(), - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/abort-controller.test.ts b/packages/core/src/abort-controller.test.ts index 2ced0b4aca..3adf469366 100644 --- a/packages/core/src/abort-controller.test.ts +++ b/packages/core/src/abort-controller.test.ts @@ -12,10 +12,6 @@ import type { Event } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { describe, expect, it, vi } from 'vitest'; -import { - createCorrelationIdGenerator, - isPerKindCorrelationIdsEnabled, -} from './correlation-id.js'; import { DEFERRED_CHECK_DELAY_MS, EventsConsumer } from './events-consumer.js'; import type { WorkflowOrchestratorContext } from './private.js'; import { ReplayPayloadCache } from './replay-payload-cache.js'; @@ -42,16 +38,13 @@ function setupWorkflowContext( replayPayloadCache: new ReplayPayloadCache(undefined), globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent, getPromiseQueue: () => ctx.promiseQueue, }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - perKind: isPerKindCorrelationIdsEnabled(), - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/abort-replay-ordering.test.ts b/packages/core/src/abort-replay-ordering.test.ts index 8e7a1a9e98..8961a64925 100644 --- a/packages/core/src/abort-replay-ordering.test.ts +++ b/packages/core/src/abort-replay-ordering.test.ts @@ -27,10 +27,6 @@ import type { Event } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { describe, expect, it, vi } from 'vitest'; -import { - createCorrelationIdGenerator, - isPerKindCorrelationIdsEnabled, -} from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import { scheduleWhenIdle, @@ -77,16 +73,13 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { replayPayloadCache: new ReplayPayloadCache(undefined), globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: () => {}, getPromiseQueue: () => ctx.promiseQueue, }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - perKind: isPerKindCorrelationIdsEnabled(), - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/async-deserialization-ordering.test.ts b/packages/core/src/async-deserialization-ordering.test.ts index 4ad65f662c..41d4684563 100644 --- a/packages/core/src/async-deserialization-ordering.test.ts +++ b/packages/core/src/async-deserialization-ordering.test.ts @@ -4,7 +4,6 @@ import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import { registerSerializationClass } from './class-serialization.js'; -import { createCorrelationIdGenerator } from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import type { WorkflowOrchestratorContext } from './private.js'; import { ReplayPayloadCache } from './replay-payload-cache.js'; @@ -54,18 +53,13 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { replayPayloadCache: new ReplayPayloadCache(undefined), globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: () => {}, getPromiseQueue: () => Promise.resolve(), }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/correlation-id-replay.test.ts b/packages/core/src/correlation-id-replay.test.ts deleted file mode 100644 index 92b1db81b6..0000000000 --- a/packages/core/src/correlation-id-replay.test.ts +++ /dev/null @@ -1,124 +0,0 @@ -import type { Event } from '@workflow/world'; -import * as nanoid from 'nanoid'; -import { monotonicFactory } from 'ulid'; -import { describe, expect, it, vi } from 'vitest'; -import { createCorrelationIdGenerator } from './correlation-id.js'; -import { EventsConsumer } from './events-consumer.js'; -import type { WorkflowOrchestratorContext } from './private.js'; -import { ReplayPayloadCache } from './replay-payload-cache.js'; -import { dehydrateStepReturnValue } from './serialization.js'; -import { createUseStep } from './step.js'; -import { createContext } from './vm/index.js'; -import { createCreateHook } from './workflow/hook.js'; -import { createSleep } from './workflow/sleep.js'; - -/** - * Correlation-id stability seen through the primitives that actually mint ids, - * rather than through the generator alone: that a step's id survives another - * kind of entity being created alongside it, and that a replay consumes an event - * log carrying the ids a same-seeded replay derives. - * - * The rest of the replay suites author their event logs with literal correlation - * ids from the shared sequence and pin themselves to it. These fixtures derive - * their ids instead, so they hold under either scheme. - */ - -const SEED = 'test'; -const FIXED_TIMESTAMP = 1753481739458; - -function setupWorkflowContext( - events: Event[], - perKind: boolean -): WorkflowOrchestratorContext { - const context = createContext({ - seed: SEED, - fixedTimestamp: FIXED_TIMESTAMP, - }); - const ulid = monotonicFactory(() => context.globalThis.Math.random()); - return { - runId: 'wrun_test', - encryptionKey: undefined, - replayPayloadCache: new ReplayPayloadCache(undefined), - globalThis: context.globalThis, - eventsConsumer: new EventsConsumer(events, { - onUnconsumedEvent: () => {}, - getPromiseQueue: () => Promise.resolve(), - }), - invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: SEED, - fixedTimestamp: FIXED_TIMESTAMP, - positional: () => ulid(FIXED_TIMESTAMP), - perKind, - }), - generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => - new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) - ), - onWorkflowError: vi.fn(), - promiseQueue: Promise.resolve(), - pendingDeliveries: 0, - pendingDeliveryBarriers: new Map(), - }; -} - -/** - * The id the next step of a replay would claim. Nothing in the log resolves the - * step, so the returned promise stays pending by design: the queue item is what - * we are after. - */ -function probeStepId( - perKind: boolean, - before?: (ctx: WorkflowOrchestratorContext) => void -): string { - const ctx = setupWorkflowContext([], perKind); - before?.(ctx); - void createUseStep(ctx)('add')(1, 2).catch(() => {}); - const item = [...ctx.invocationsQueue.values()].find( - (entry) => entry.type === 'step' - ); - if (!item) { - throw new Error('expected a step invocation'); - } - return item.correlationId; -} - -function createHookAndSleep(ctx: WorkflowOrchestratorContext): void { - createCreateHook(ctx)(); - void createSleep(ctx)('1h').catch(() => {}); -} - -describe('correlation ids through the replay primitives', () => { - it('keeps a step id when a hook and a sleep are created before it', () => { - expect(probeStepId(true, createHookAndSleep)).toBe(probeStepId(true)); - }); - - it('renumbers that step under one sequence shared by every kind', () => { - // The failure this PR removes, and the reason the assertion above is worth - // making: with a shared sequence the hook and the sleep consume the two - // ordinals the step would otherwise have drawn from. - expect(probeStepId(false, createHookAndSleep)).not.toBe(probeStepId(false)); - }); - - it('consumes a step_completed authored with the derived id', async () => { - const correlationId = probeStepId(true, createHookAndSleep); - const ctx = setupWorkflowContext( - [ - { - eventId: 'evnt_0', - runId: 'wrun_test', - eventType: 'step_completed', - correlationId, - eventData: { - stepName: 'add', - result: await dehydrateStepReturnValue(3, 'wrun_test', undefined), - }, - createdAt: new Date(), - }, - ], - true - ); - createHookAndSleep(ctx); - await expect(createUseStep(ctx)('add')(1, 2)).resolves.toBe(3); - expect(ctx.onWorkflowError).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/core/src/correlation-id.test.ts b/packages/core/src/correlation-id.test.ts deleted file mode 100644 index db95c65df4..0000000000 --- a/packages/core/src/correlation-id.test.ts +++ /dev/null @@ -1,154 +0,0 @@ -import { decodeTime, monotonicFactory } from 'ulid'; -import { describe, expect, it } from 'vitest'; -import { - CORRELATION_ID_LENGTH, - type CorrelationIdKind, - createCorrelationIdGenerator, - isPerKindCorrelationIdsEnabled, -} from './correlation-id.js'; - -const SEED = 'wrun_abc:myWorkflow:dpl_123'; -const FIXED_TIMESTAMP = 1753481739458; - -function makeGenerator( - overrides: { seed?: string; fixedTimestamp?: number; perKind?: boolean } = {} -) { - // A stand-in for the run's shared sequence. Seeded so the positional mode is - // reproducible across the two generators a replay-stability test builds. - let counter = 0; - const ulid = monotonicFactory(() => { - counter = (counter * 1103515245 + 12345) % 2147483648; - return counter / 2147483648; - }); - const fixedTimestamp = overrides.fixedTimestamp ?? FIXED_TIMESTAMP; - return createCorrelationIdGenerator({ - seed: overrides.seed ?? SEED, - fixedTimestamp, - positional: () => ulid(fixedTimestamp), - perKind: overrides.perKind ?? true, - }); -} - -const KINDS: CorrelationIdKind[] = [ - 'step', - 'wait', - 'hook', - 'attr', - 'abort', - 'abortHook', - 'stream', -]; - -describe('createCorrelationIdGenerator', () => { - it('mints syntactically valid ULIDs carrying fixedTimestamp', () => { - const generate = makeGenerator(); - for (const kind of KINDS) { - const id = generate(kind); - expect(id).toHaveLength(CORRELATION_ID_LENGTH); - expect(id).toMatch(/^[0-9A-HJKMNP-TV-Z]+$/); - expect(decodeTime(id)).toBe(FIXED_TIMESTAMP); - } - }); - - it('is deterministic across replays of the same run', () => { - const first = makeGenerator(); - const second = makeGenerator(); - const draw = (generate: (kind: CorrelationIdKind) => string) => [ - generate('step'), - generate('step'), - generate('wait'), - generate('step'), - generate('hook'), - ]; - expect(draw(first)).toEqual(draw(second)); - }); - - it('mints different ids for different runs', () => { - const first = makeGenerator({ seed: 'wrun_one:w:dpl' }); - const second = makeGenerator({ seed: 'wrun_two:w:dpl' }); - expect(first('step')).not.toBe(second('step')); - }); - - it('gives every kind its own starting point', () => { - const generate = makeGenerator(); - const ids = KINDS.map((kind) => generate(kind)); - expect(new Set(ids).size).toBe(KINDS.length); - }); - - it('increases monotonically within a kind', () => { - const generate = makeGenerator(); - const ids = [generate('hook'), generate('hook'), generate('hook')]; - expect(ids).toEqual([...ids].sort()); - expect(new Set(ids).size).toBe(ids.length); - }); - - it('does not renumber one kind when another draws more often', () => { - // The whole point of per-kind sources: two replays that disagree about how - // many hooks, sleeps or streams were created still agree about which id - // belongs to the Nth step. - const withoutExtras = makeGenerator(); - const withExtras = makeGenerator(); - - const steps = [withoutExtras('step'), withoutExtras('step')]; - - withExtras('hook'); - const interleaved = [withExtras('step')]; - withExtras('wait'); - withExtras('stream'); - withExtras('attr'); - withExtras('abort'); - interleaved.push(withExtras('step')); - - expect(interleaved).toEqual(steps); - }); - - it('keeps abort controllers from renumbering user hooks', () => { - const withoutController = makeGenerator(); - const withController = makeGenerator(); - withController('abort'); - withController('abortHook'); - expect(withController('hook')).toBe(withoutController('hook')); - }); - - it('keeps every id on fixedTimestamp in both modes', () => { - // `monotonicFactory` returns `encodeTime(lastTime)` on its increment branch, - // so a single draw that omits the seed time latches the host wall clock and - // every later id in the run carries a timestamp that differs per replay. - // Stream ids used to be drawn that way. - for (const perKind of [true, false]) { - const generate = makeGenerator({ perKind }); - for (const kind of ['stream', 'stream', 'step', 'hook'] as const) { - expect(decodeTime(generate(kind))).toBe(FIXED_TIMESTAMP); - } - } - }); - - it('ignores the kind when per-kind sources are disabled', () => { - const generate = makeGenerator({ perKind: false }); - const shared = makeGenerator({ perKind: false }); - // Positional mode is one sequence for the whole run, so drawing `wait` - // consumes the ordinal the next `step` would otherwise have had. - expect(generate('step')).toBe(shared('step')); - expect(generate('wait')).toBe(shared('step')); - }); -}); - -describe('isPerKindCorrelationIdsEnabled', () => { - it('reads WORKFLOW_PER_KIND_CORRELATION_IDS, defaulting to disabled', () => { - const original = process.env.WORKFLOW_PER_KIND_CORRELATION_IDS; - try { - delete process.env.WORKFLOW_PER_KIND_CORRELATION_IDS; - expect(isPerKindCorrelationIdsEnabled()).toBe(false); - process.env.WORKFLOW_PER_KIND_CORRELATION_IDS = '1'; - expect(isPerKindCorrelationIdsEnabled()).toBe(true); - process.env.WORKFLOW_PER_KIND_CORRELATION_IDS = '0'; - expect(isPerKindCorrelationIdsEnabled()).toBe(false); - } finally { - if (original === undefined) { - delete process.env.WORKFLOW_PER_KIND_CORRELATION_IDS; - } else { - process.env.WORKFLOW_PER_KIND_CORRELATION_IDS = original; - } - } - }); -}); diff --git a/packages/core/src/correlation-id.ts b/packages/core/src/correlation-id.ts deleted file mode 100644 index dbed6f8f5c..0000000000 --- a/packages/core/src/correlation-id.ts +++ /dev/null @@ -1,234 +0,0 @@ -import { encodeTime, incrementBase32 } from 'ulid'; - -/** - * Correlation-id generation for the entity families a replay can create. - * - * Correlation ids are minted by the workflow VM and are the server's identity - * gate: a conditional create on the id is what makes a duplicate write from a - * second live replay idempotent instead of additive. That only works if two - * replays of the same run mint the same id for the same entity. - * - * Historically every id was the Nth draw of *one* monotonic ULID sequence per - * run, shared by steps, waits, hooks, attribute writes, abort controllers and - * stream ids alike. Every id was therefore an ordinal over the whole run, and a - * single extra draw of any kind renumbered every entity of every kind after it. - * Two replays that agreed about every step but disagreed about one `sleep()` - * would mint different ids for all subsequent steps, so their writes appended - * side by side instead of colliding, and the settled log ended up holding two - * names for one logical step. Only one of them can be consumed on the next - * replay; the other is fatal (`onUnconsumedEvent`). - * - * Per-kind sources narrow that coupling to one kind at a time: each family - * draws from its own independent incrementing sequence, so a disagreement about - * how many hooks or sleeps were created no longer renames steps. - * - * Ids stay syntactically valid ULIDs (10 Crockford characters of - * `fixedTimestamp` plus 16 of body), because correlation ids are validated as - * prefixed 26-char ULIDs by the backend, and they stay monotonic *within* a - * kind, because `hooks.list` is ordered by hook id. - * - * Monotonicity is per kind, and two kinds mint `hook_` ids (`hook` and - * `abortHook`), so listing order is only creation order *within* each of them. - * No world filters system hooks out of a listing, so a run that constructs an - * abort controller and also creates its own hooks lists that system hook at a - * position decided by its kind's hash rather than at its creation position. - * Order among the user's own hooks is unaffected. - * - * This does not make ids independent of *ordinal position within their own - * kind*: two replays that disagree about how many steps ran still mint - * different ids for the next step. That is a narrower failure than the shared - * sequence's, not an eliminated one. - */ - -/** Entity families that draw correlation ids, each from its own sequence. */ -export type CorrelationIdKind = - /** `step_` ids, one per step invocation. */ - | 'step' - /** `wait_` ids, one per `sleep()`. */ - | 'wait' - /** `hook_` ids for hooks created by workflow code. */ - | 'hook' - /** `attr_` ids, one per attribute write. */ - | 'attr' - /** - * The abort controller's own id, which becomes its stream name and hook - * token. Separate from `hook` so constructing an abort controller does not - * renumber later user hooks. - */ - | 'abort' - /** `hook_` ids for the internal system hook backing an abort controller. */ - | 'abortHook' - /** - * Ids minted during serialization (`STABLE_ULID`): stream names, and an abort - * holder's stream name and `abrt_` hook token when it reaches serialization - * without an identity yet (`reduceAbortWithListener`), which is why `abort` - * above is not the only mint path for an abort identity. Not correlation ids, - * but they drew from the same shared sequence, so a workflow that serialized - * a stream renumbered every entity created after it. - */ - | 'stream'; - -/** Mints the ULID body of a correlation id for one entity family. */ -export type CorrelationIdGenerator = (kind: CorrelationIdKind) => string; - -const CROCKFORD = '0123456789ABCDEFGHJKMNPQRSTVWXYZ'; - -/** Number of Crockford characters in a ULID's random component. */ -const BODY_CHARS = 16; - -/** Number of Crockford characters in a ULID's timestamp component. */ -const TIME_CHARS = 10; - -function mul32(a: number, b: number): number { - return Math.imul(a, b) >>> 0; -} - -function rotl32(value: number, shift: number): number { - return ((value << shift) | (value >>> (32 - shift))) >>> 0; -} - -/** MurmurHash3's 32-bit finalizer. */ -function fmix32(input: number): number { - let h = input >>> 0; - h = (h ^ (h >>> 16)) >>> 0; - h = mul32(h, 0x85ebca6b); - h = (h ^ (h >>> 13)) >>> 0; - h = mul32(h, 0xc2b2ae35); - return (h ^ (h >>> 16)) >>> 0; -} - -/** - * Deterministic 128-bit hash of a string, as four 32-bit lanes. - * - * A MurmurHash3-style mixer over UTF-16 code units, rotating which lane absorbs - * each unit and diffusing across lanes at the end. Only determinism and - * diffusion matter here: this is not a cryptographic hash, claims no - * bit-compatibility with any reference implementation, and must not be used for - * anything that outlives a deployment's replays. - */ -function hash128(input: string): [number, number, number, number] { - const lanes: [number, number, number, number] = [ - 0x9e3779b1, 0x85ebca77, 0xc2b2ae3d, 0x27d4eb2f, - ]; - for (let i = 0; i < input.length; i++) { - let k = input.charCodeAt(i) >>> 0; - k = mul32(k, 0xcc9e2d51); - k = rotl32(k, 15); - k = mul32(k, 0x1b873593); - const lane = i & 3; - let h = (lanes[lane] ^ k) >>> 0; - h = rotl32(h, 13); - lanes[lane] = (mul32(h, 5) + 0xe6546b64) >>> 0; - } - lanes[0] = (lanes[0] ^ input.length) >>> 0; - // Two passes so every lane depends on every other lane. - for (let pass = 0; pass < 2; pass++) { - for (let lane = 0; lane < 4; lane++) { - const previous = lanes[(lane + 3) & 3]; - lanes[lane] = fmix32((lanes[lane] ^ previous) >>> 0); - } - } - return lanes; -} - -/** - * Derives a kind's starting body: 80 bits of a 128-bit hash as 16 Crockford - * characters, most significant first. - * - * The leading character is confined to the alphabet's lower half so the body - * starts below half of the 80-bit space. `incrementBase32` throws on overflow, - * and without this a base that happened to land near `Z…Z` would make overflow - * reachable after few draws rather than after 2^79 of them. - */ -function deriveBody(seed: string, kind: CorrelationIdKind): string { - const lanes = hash128(`${seed} correlation-kind ${kind}`); - const bytes = [ - (lanes[0] >>> 24) & 0xff, - (lanes[0] >>> 16) & 0xff, - (lanes[0] >>> 8) & 0xff, - lanes[0] & 0xff, - (lanes[1] >>> 24) & 0xff, - (lanes[1] >>> 16) & 0xff, - (lanes[1] >>> 8) & 0xff, - lanes[1] & 0xff, - (lanes[2] >>> 24) & 0xff, - (lanes[2] >>> 16) & 0xff, - ]; - let body = ''; - let accumulator = 0; - let bits = 0; - for (const byte of bytes) { - accumulator = ((accumulator << 8) | byte) >>> 0; - bits += 8; - while (bits >= 5) { - const index = (accumulator >>> (bits - 5)) & 31; - body += CROCKFORD[body.length === 0 ? index & 15 : index]; - bits -= 5; - } - } - return body; -} - -/** - * Builds a replay's correlation-id generator. - * - * `perKind: false` returns the run's single shared monotonic sequence and - * ignores the kind entirely, so both schemes go through one call path and the - * flag is the only difference between them. - */ -export function createCorrelationIdGenerator(options: { - /** - * The run's replay-stable seed. Must not vary between replays of one run, and - * must differ between runs, or two runs would mint identical ids. - */ - seed: string; - fixedTimestamp: number; - /** The run's shared monotonic sequence, used as-is when `perKind` is false. */ - positional: () => string; - perKind: boolean; -}): CorrelationIdGenerator { - const { seed, fixedTimestamp, positional, perKind } = options; - - if (!perKind) { - return positional; - } - - const time = encodeTime(fixedTimestamp, TIME_CHARS); - const bodies = new Map(); - - return (kind: CorrelationIdKind) => { - const previous = bodies.get(kind); - const body = - previous === undefined - ? deriveBody(seed, kind) - : incrementBase32(previous); - bodies.set(kind, body); - return `${time}${body}`; - }; -} - -/** Length of a ULID, exported so tests need not restate it. */ -export const CORRELATION_ID_LENGTH = TIME_CHARS + BODY_CHARS; - -/** - * Whether each entity family draws correlation ids from its own sequence rather - * than from one sequence shared by the whole run. Off unless opted in, so an SDK - * upgrade alone never moves a run between schemes. - * - * The invariant either way: a run must replay under the scheme that minted its - * ids. A replay under the other scheme mints ids its own earlier events do not - * carry, so it can consume none of them and fails the run. Two things can break - * it, and both are about turning the flag on rather than about upgrading: - * - * - Enabling it while runs are in flight. On Vercel, skew protection keeps a run - * on the deployment that started it, so a run only ever sees the value baked - * into its own deployment. Elsewhere (world-postgres, world-local, a - * self-hosted process) nothing pins a run to the code that started it, so - * enable it during a quiet window. - * - A rolling deploy that leaves both values live, which puts two schemes on one - * run concurrently — the side-by-side append this whole mechanism exists to - * avoid. Roll the value out to the whole fleet at once. - */ -export function isPerKindCorrelationIdsEnabled(): boolean { - return process.env.WORKFLOW_PER_KIND_CORRELATION_IDS === '1'; -} diff --git a/packages/core/src/delivery-barrier-coverage.test.ts b/packages/core/src/delivery-barrier-coverage.test.ts index b37a8f597d..c40fb888cf 100644 --- a/packages/core/src/delivery-barrier-coverage.test.ts +++ b/packages/core/src/delivery-barrier-coverage.test.ts @@ -51,7 +51,6 @@ import type { Event } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { describe, expect, it, vi } from 'vitest'; -import { createCorrelationIdGenerator } from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import { WorkflowSuspension } from './global.js'; import { @@ -86,6 +85,8 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { replayPayloadCache: new ReplayPayloadCache(undefined), globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: (event) => { ctxRef.current?.onWorkflowError( new WorkflowRuntimeError(`Unconsumed event: ${event.eventType}`) @@ -94,14 +95,7 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { getPromiseQueue: () => promiseQueueHolder.current, }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/events-consumer.test.ts b/packages/core/src/events-consumer.test.ts index c85a660ea6..9bae822106 100644 --- a/packages/core/src/events-consumer.test.ts +++ b/packages/core/src/events-consumer.test.ts @@ -21,9 +21,12 @@ function createMockEvent(overrides: Partial = {}): Event { } // Default options for tests that don't care about onUnconsumedEvent +// No deliveries are modeled here, so the delivery-idle gate is always open; the +// tests that exercise the gate itself pass their own predicate. const defaultOptions = { onUnconsumedEvent: vi.fn(), getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, }; // Helper function to wait for next tick @@ -165,6 +168,7 @@ describe('EventsConsumer', () => { const consumer = new EventsConsumer([event], { onUnconsumedEvent: unconsumedReceived.resolve, getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, }); const callback1 = vi .fn() @@ -421,6 +425,7 @@ describe('EventsConsumer', () => { const consumer = new EventsConsumer([event], { onUnconsumedEvent: unconsumedReceived.resolve, getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, }); const callback = vi.fn().mockReturnValue(EventConsumerResult.NotConsumed); @@ -435,6 +440,7 @@ describe('EventsConsumer', () => { const consumer = new EventsConsumer([], { onUnconsumedEvent, getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, }); const callback = vi.fn().mockReturnValue(EventConsumerResult.NotConsumed); @@ -455,6 +461,7 @@ describe('EventsConsumer', () => { const consumer = new EventsConsumer([event], { onUnconsumedEvent, getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, }); const callback1 = vi .fn() @@ -481,6 +488,174 @@ describe('EventsConsumer', () => { }); }); + describe('parking events that carry no ordering claim', () => { + /** + * A log event of a real type. The rest of this file uses a mock shape with + * no `eventType` at all, which is deliberately unparkable, so parking + * tests need events the consumer recognizes. + */ + function logEvent(eventType: Event['eventType'], id: string): Event { + // `eventId` as well as the mock shape's `id`: the consumer reports the + // former, the matcher below keys on the latter. + return createMockEvent({ id, eventId: id, eventType } as Partial); + } + + /** Consumes exactly the events whose id is in `ids`, once each. */ + function consumerFor(ids: string[]) { + const seen: string[] = []; + const callback = (event: Event | null) => { + if (event && ids.includes(event.id) && !seen.includes(event.id)) { + seen.push(event.id); + return EventConsumerResult.Consumed; + } + return EventConsumerResult.NotConsumed; + }; + return { seen, callback }; + } + + it('walks past an unclaimed hook_received instead of declaring divergence', async () => { + const hook = logEvent('hook_received', 'hook-1'); + const wait = logEvent('wait_created', 'wait-1'); + const onUnconsumedEvent = vi.fn(); + const consumer = new EventsConsumer([hook, wait], { + onUnconsumedEvent, + getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, + }); + const waits = consumerFor(['wait-1']); + + consumer.subscribe(waits.callback); + + // The hook belongs to a consumer this replay has not registered. The + // wait behind it is this replay's own decision and must still land. + await vi.waitFor(() => { + expect(waits.seen).toEqual(['wait-1']); + }); + expect(consumer.eventIndex).toBe(2); + expect(onUnconsumedEvent).not.toHaveBeenCalled(); + }); + + it('delivers a parked event to a consumer that subscribes later', async () => { + const hook = logEvent('hook_received', 'hook-1'); + const wait = logEvent('wait_created', 'wait-1'); + const onUnconsumedEvent = vi.fn(); + const consumer = new EventsConsumer([hook, wait], { + onUnconsumedEvent, + getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, + }); + const waits = consumerFor(['wait-1']); + consumer.subscribe(waits.callback); + await vi.waitFor(() => { + expect(waits.seen).toEqual(['wait-1']); + }); + + const hooks = consumerFor(['hook-1']); + consumer.subscribe(hooks.callback); + + await vi.waitFor(() => { + expect(hooks.seen).toEqual(['hook-1']); + }); + expect(onUnconsumedEvent).not.toHaveBeenCalled(); + }); + + it('replays a parked event under the index it held in the log', async () => { + const hook = logEvent('hook_received', 'hook-1'); + const wait = logEvent('wait_created', 'wait-1'); + const consumer = new EventsConsumer([hook, wait], { + onUnconsumedEvent: vi.fn(), + getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, + }); + consumer.subscribe(consumerFor(['wait-1']).callback); + await vi.waitFor(() => { + expect(consumer.eventIndex).toBe(2); + }); + + // Delivery barriers are registered under whatever `eventIndex` reads at + // consumption time, so a late delivery must still make the ordering + // claim its log position gave it — index 0, not the walk's 2. + let indexAtDelivery: number | undefined; + consumer.subscribe((event) => { + if (event?.id !== 'hook-1') { + return EventConsumerResult.NotConsumed; + } + indexAtDelivery = consumer.eventIndex; + return EventConsumerResult.Finished; + }); + + await vi.waitFor(() => { + expect(indexAtDelivery).toBe(0); + }); + // The walk pointer is restored, not left behind at the parked index. + expect(consumer.eventIndex).toBe(2); + }); + + it('still declares divergence for an unclaimed replay-origin event', async () => { + const step = logEvent('step_created', 'step-1'); + const unconsumedReceived = withResolvers(); + const consumer = new EventsConsumer([step], { + onUnconsumedEvent: unconsumedReceived.resolve, + getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, + }); + + consumer.subscribe(() => EventConsumerResult.NotConsumed); + + expect(await unconsumedReceived.promise).toEqual(step); + }); + + it('reports what it is still holding when the walk stops', async () => { + const hook = logEvent('hook_received', 'hook-1'); + const late = logEvent('hook_received', 'hook-2'); + const wait = logEvent('wait_created', 'wait-1'); + const consumer = new EventsConsumer([hook, late, wait], { + onUnconsumedEvent: vi.fn(), + getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, + }); + expect(consumer.parkedSummary).toBeUndefined(); + + consumer.subscribe(consumerFor(['wait-1']).callback); + await vi.waitFor(() => { + expect(consumer.eventIndex).toBe(3); + }); + + // Both hooks were walked past. A suspension is not a settling point, so + // the state goes on the span instead of failing the run: the oldest one + // held is what a query across a run's spans keys on. + expect(consumer.parkedSummary).toEqual({ + count: 2, + eventId: 'hook-1', + eventType: 'hook_received', + }); + + // Once a consumer claims them the run is holding nothing, and the + // attribute stops appearing on later spans. + consumer.subscribe(consumerFor(['hook-1', 'hook-2']).callback); + await vi.waitFor(() => { + expect(consumer.parkedSummary).toBeUndefined(); + }); + }); + + it('declares divergence for an event still parked once the run has ended', async () => { + const hook = logEvent('hook_received', 'hook-1'); + const completed = logEvent('run_completed', 'done-1'); + const unconsumedReceived = withResolvers(); + const consumer = new EventsConsumer([hook, completed], { + onUnconsumedEvent: unconsumedReceived.resolve, + getPromiseQueue: () => Promise.resolve(), + isDeliveryIdle: () => true, + }); + + // Nothing can subscribe for the hook after the run has finished, so + // parking it would silently drop it. + consumer.subscribe(consumerFor(['done-1']).callback); + + expect(await unconsumedReceived.promise).toEqual(hook); + }); + }); + describe('delivery-idle gate', () => { // An event nobody claims is only evidence of divergence once the workflow // VM has stopped reacting. While a delivery is in flight the walk is diff --git a/packages/core/src/events-consumer.ts b/packages/core/src/events-consumer.ts index 2367000eab..36647ed96c 100644 --- a/packages/core/src/events-consumer.ts +++ b/packages/core/src/events-consumer.ts @@ -9,23 +9,110 @@ import { eventsLogger } from './logger.js'; */ export const DEFERRED_CHECK_DELAY_MS = 100; +/** + * Floor for the deferred-check delay, so a too-low override can't manufacture + * spurious divergence (each false positive burns a divergence-recovery retry + * and can escalate to a terminal `CorruptedEventLogError`). + * + * Exported so tests needing the shortest legal delay can ask for it instead of + * hardcoding a number this floor would silently clamp up. + */ +export const MIN_DEFERRED_CHECK_DELAY_MS = 10; + /** * Effective deferred-check delay. Override: `WORKFLOW_DEFERRED_CHECK_DELAY_MS`. * * Unlike the other timing knobs this is not a polling interval but a * determinism safety margin: firing the unconsumed-event check before the * cross-VM subscribe() chain has landed rejects a healthy run with - * `ReplayDivergenceError`. Floored at 10ms so a too-low override can't - * manufacture spurious divergence (each false positive burns a - * divergence-recovery retry and can escalate to a terminal - * `CorruptedEventLogError`). + * `ReplayDivergenceError`. */ const getDeferredCheckDelayMs = (): number => envNumber('WORKFLOW_DEFERRED_CHECK_DELAY_MS', DEFERRED_CHECK_DELAY_MS, { integer: true, - min: 10, + min: MIN_DEFERRED_CHECK_DELAY_MS, }); +/** + * Event types the ordered walk may step over and deliver later. + * + * Membership is not about who wrote the event. It is about whether the event + * can reach the head of the walk with nothing registered to consume it, which + * is the only situation parking exists for. + * + * Most types cannot. They are replay-origin: a replay emits them, in the order + * its code reaches them, so their position in the log is the record of what + * that replay decided. Reaching one of those out of order means this replay + * decided differently than the log holds, which is divergence and nothing else. + * + * Step lifecycle events are not replay-origin, and are still absent, because + * they always have a claimant. `step()` subscribes its consumer before the + * step's first event can exist; `step_created` is ordered, so the walk cannot + * pass it unless a consumer takes it; and that consumer stays subscribed until + * `step_completed` or `step_failed`, after which the World refuses any further + * write for that step. There is no window in which a replay knows about a step + * and has nothing registered to consume its events, so parking them would only + * defer reports of what is divergence either way. The same holds for + * `wait_created` and its consumer. `wait_completed` is listed anyway: a sleep + * can also be completed out of band, by the API that force-completes pending + * waits, and tolerating a stray one is the cheaper direction to be wrong in. + * + * An allowlist rather than the complement of the ordered set, so a type this + * file has not been taught about keeps the strict old behaviour. + * + * `hook_disposed` is deliberately absent despite being about a hook: it is + * written when the workflow's own `using` scope exits, so it is replay-origin. + * + * `attr_set` is listed by type even though a given instance of it may be + * replay-origin, since it is replay-origin when its writer is the workflow. + * Splitting that out per event was considered and rejected. A replay that + * reaches one of its own writes out of position has diverged, but a replay that + * reaches an event it did NOT write, sitting where its own write would go, has + * not: the writer field says who wrote the event, and not whether this replay + * is the same one. Guessing wrong in that direction fails healthy runs, which + * is the failure this file exists to stop, so the whole type is tolerated. The + * cost is that a divergence involving `attr_set` surfaces at the end of the + * replay, through `strandedEvent`, rather than at the offending event. + */ +const PARKABLE_EVENT_TYPES: ReadonlySet = new Set([ + 'hook_received', + 'hook_conflict', + 'wait_completed', + 'attr_set', + 'run_cancelled', +]); + +/** + * Parkable types that can only ever resolve their correlation id once. + * + * A second one for the same id is not a delivery this replay has not reached + * yet: it is a resolution for something already resolved, which no consumer + * this replay or any later one registers can ever claim. `hook_received` is + * absent because a hook legitimately fires many times under one id. + * + * Must stay a subset of {@link PARKABLE_EVENT_TYPES}: {@link park} is the only + * reader of what this records, and it rejects a non-parkable type before it + * looks, so an entry outside that set is dead weight on a hot path. + */ +const ONE_SHOT_EVENT_TYPES: ReadonlySet = new Set([ + 'wait_completed', +]); + +/** Identifies the thing a one-shot resolution event resolves. */ +function resolutionKey(eventType: string, correlationId: string): string { + return `${eventType}:${correlationId}`; +} + +/** + * Types that end a run. Once one is in the log, no consumer will ever be + * registered again, so a parked event still parked here will never be claimed. + */ +const TERMINAL_EVENT_TYPES: ReadonlySet = new Set([ + 'run_completed', + 'run_failed', + 'run_cancelled', +]); + export enum EventConsumerResult { /** * Callback consumed the event, but should not be removed from the callbacks list @@ -71,16 +158,36 @@ export interface EventsConsumerOptions { * flight means the workflow VM is mid-reaction, and an event it has not * claimed yet is an event it has not reached yet. * - * Defaults to always-idle so the tests that drive a consumer with no - * orchestrator context keep the pre-existing timing. + * Required rather than defaulting to always-idle: always-idle is exactly the + * pre-gate behaviour, so a defaulted option would let a construction site opt + * a whole replay path back out without saying so. Tests that drive a consumer + * with no orchestrator context pass `() => true` to keep the pre-existing + * timing, and say so at the call site. */ - isDeliveryIdle?: () => boolean; + isDeliveryIdle: () => boolean; } export class EventsConsumer { eventIndex: number; readonly events: Event[]; readonly callbacks: EventConsumerCallback[] = []; + /** + * Events the ordered walk stepped over because nobody claimed them and their + * type carries no ordering claim. Each keeps the index it held in the log: + * consumers read {@link eventIndex} at consumption time to order their + * delivery against the rest of the log, and a late delivery must still make + * the claim its position gave it. + * + * Held in log order, drained in log order, and drained before every offer so + * a consumer registered after the walk passed the event still receives it. + */ + private readonly parked: { event: Event; index: number }[] = []; + /** + * Correlation ids of the {@link ONE_SHOT_EVENT_TYPES} events consumed so + * far, so a second resolution for one of them is recognized as unclaimable + * rather than parked for a consumer that cannot exist. + */ + private readonly resolved = new Set(); private onConsumedEvent?: (event: Event) => void; private onUnconsumedEvent: (event: Event) => void; private getPromiseQueue: () => Promise; @@ -98,7 +205,42 @@ export class EventsConsumer { this.onConsumedEvent = options.onConsumedEvent; this.onUnconsumedEvent = options.onUnconsumedEvent; this.getPromiseQueue = options.getPromiseQueue; - this.isDeliveryIdle = options.isDeliveryIdle ?? (() => true); + this.isDeliveryIdle = options.isDeliveryIdle; + } + + /** + * The oldest event the walk stepped over that no consumer has claimed yet, + * if any. Parking is a bet that a consumer will be registered later, so at + * any point where no consumer ever will be again — the replay finishing is + * the definitive one — this answers which event the bet lost on. + */ + get strandedEvent(): Event | undefined { + return this.parked[0]?.event; + } + + /** + * What the walk is still holding, or `undefined` when it holds nothing. + * + * Read at every point a replay stops, including the suspensions that are not + * settling points, so the held state reaches telemetry. A replay cannot tell + * a delivery awaiting a later consumer from one no consumer will ever + * register, so it reports rather than decides: the same `eventId` reported on + * suspension after suspension of one run is the shape that says the bet + * parking made is not going to pay off, and that shape is only visible across + * replays. + */ + get parkedSummary(): + | { count: number; eventId: string; eventType: Event['eventType'] } + | undefined { + const oldest = this.parked[0]?.event; + if (!oldest) { + return undefined; + } + return { + count: this.parked.length, + eventId: oldest.eventId, + eventType: oldest.eventType, + }; } append(events: Event[]): void { @@ -160,26 +302,42 @@ export class EventsConsumer { // case no callback consumes the event and we fall through to the // cross-VM-safe deferred unconsumed-event check below, exactly as before. while (true) { + // Before every offer, not just on subscribe: a callback registered by + // the work this same pass kicked off may be the owner of something + // parked, and the parked event's delivery is ordered ahead of the head + // event's by the index it holds. + this.drainParked(); const currentEvent = this.events[this.eventIndex] ?? null; - if (!this.consumeOne(currentEvent)) { - // No callback consumed the current event; handle the terminal case. - this.handleUnconsumed(currentEvent); + const consumed = this.offer(currentEvent); + if (consumed) { + this.eventIndex++; + } + if (currentEvent === null) { + // End of log. Consumers return NotConsumed for the `null` sentinel + // (the one that recognizes it as its own boundary schedules the + // suspension as a side effect and still declines it), so the drain + // stops here rather than spinning past the end. `consumed` is only + // true for a callback that claims the sentinel outright, which no + // production consumer does. + if (!consumed) { + this.handleEndOfLog(); + } return; } - // A real event was consumed — advance to the next in the same pass. A - // consumed `null` sentinel never returns true (see consumeOne), so the - // synchronous drain can't spin past the end of the log. + if (!consumed) { + this.scheduleUnconsumedCheck(currentEvent, true); + return; + } + // A real event was consumed — advance to the next in the same pass. } }; /** * Offer `currentEvent` to each registered callback in turn. Returns true - * when a callback consumed a real (non-null) event and the drain should - * advance to the next event in the same synchronous pass; false otherwise - * (nothing consumed it, or the consumed event was the end-of-events - * sentinel). + * when a callback consumed it. Does not move {@link eventIndex}: the ordered + * walk and the parked drain advance differently, so each does its own. */ - private consumeOne(currentEvent: Event | null): boolean { + private offer(currentEvent: Event | null): boolean { for (let i = 0; i < this.callbacks.length; i++) { const callback = this.callbacks[i]; let handled = EventConsumerResult.NotConsumed; @@ -195,63 +353,163 @@ export class EventsConsumer { continue; } if (currentEvent !== null) { + if ( + currentEvent.correlationId && + ONE_SHOT_EVENT_TYPES.has(currentEvent.eventType) + ) { + this.resolved.add( + resolutionKey(currentEvent.eventType, currentEvent.correlationId) + ); + } this.notifyConsumedEvent(currentEvent); } - // consumer handled this event, so increase the event index - this.eventIndex++; // remove the callback if it has finished if (handled === EventConsumerResult.Finished) { this.callbacks.splice(i, 1); } - // Continue draining only for real events. Real consumers return - // NotConsumed for the `null` sentinel, but guard against a pathological - // callback consuming it so the drain never spins past end-of-log. - return currentEvent !== null; + return true; } return false; } - private handleUnconsumed(currentEvent: Event | null) { + /** + * Offer everything parked, oldest first, until a pass claims nothing. + * + * Each offer runs with {@link eventIndex} moved back to the position the + * parked event held in the log, because that is the position its consumer + * will register a delivery barrier under. Restoring the walk pointer + * afterwards is what keeps the two pointers from interfering. + */ + private drainParked(): void { + let progressed = this.parked.length > 0; + while (progressed) { + progressed = false; + for (let i = 0; i < this.parked.length; i++) { + const entry = this.parked[i]; + const walkIndex = this.eventIndex; + this.eventIndex = entry.index; + let consumed: boolean; + try { + consumed = this.offer(entry.event); + } finally { + this.eventIndex = walkIndex; + } + if (consumed) { + this.parked.splice(i, 1); + // A `Finished` callback was spliced out of the list this pass, so + // restart rather than keep walking a mutated array. + progressed = this.parked.length > 0; + break; + } + } + } + } + + /** + * Step the ordered walk over an event nobody claimed, holding on to it for a + * later consumer. Returns false when the event's type makes its position a + * decision record, which is the one case where nobody claiming it means the + * replay diverged. + */ + private park(event: Event): boolean { + if (!PARKABLE_EVENT_TYPES.has(event.eventType)) { + return false; + } + if ( + event.correlationId && + this.resolved.has(resolutionKey(event.eventType, event.correlationId)) + ) { + return false; + } + this.parked.push({ event, index: this.eventIndex }); + this.eventIndex++; + eventsLogger.debug('Parked an unclaimed event for later delivery', { + eventId: event.eventId, + eventType: event.eventType, + correlationId: event.correlationId, + parked: this.parked.length, + }); + return true; + } + + private handleEndOfLog() { + // Everything still parked is waiting for a consumer some later replay will + // register, which is the whole point of parking — except once the log + // already holds the run's terminal event, because then there is no later + // replay and no consumer will ever come. + if (this.parked.length === 0) { + return; + } + const last = this.events.at(-1); + if (!last || !TERMINAL_EVENT_TYPES.has(last.eventType)) { + // A later replay is still expected, so nothing here is decidable and + // escalating would fail the healthy runs parking exists to keep alive. + // Reaching the end of the log holding something is not by itself a fault, + // so this stays at `debug`; {@link parkedSummary} is what carries the + // state to the span, where a run that keeps stopping on the same held + // event is visible and a single replay's view is not. + eventsLogger.debug('Reached the end of the log still holding events', { + eventId: this.parked[0].event.eventId, + eventType: this.parked[0].event.eventType, + correlationId: this.parked[0].event.correlationId, + parked: this.parked.length, + }); + return; + } + this.scheduleUnconsumedCheck(this.parked[0].event, false); + } + + private scheduleUnconsumedCheck(currentEvent: Event, mayPark: boolean) { // All callbacks returned NotConsumed for the current event. - // If the current event is non-null (a real event, not end-of-events), - // schedule a deferred check. We chain onto the promiseQueue so that any + // Schedule a deferred check. We chain onto the promiseQueue so that any // pending async work (e.g., deserialization/decryption that triggers // resolve() → user code → subscribe()) completes first. If the event - // is still unconsumed after the queue drains, it's truly orphaned. - if (currentEvent !== null) { - const checkVersion = ++this.unconsumedCheckVersion; - this.pendingUnconsumedCheck = this.getPromiseQueue() - .then( - // Yield once after the first queue drain so promise chains resumed by - // that drain can run across the VM boundary and append any follow-up - // async work (for example: step_completed resolves -> for-await loop - // resumes -> the next hook payload starts hydrating). - () => new Promise((resolve) => setTimeout(resolve, 0)) - ) - .then(() => this.getPromiseQueue()) - .then(() => { - // Wait out any delivery still in flight before starting the timer. - // The queue draining says the host has no hydration work left; it - // does not say the VM has finished reacting to what was hydrated. - this.whenDeliveryIdle(checkVersion, () => { - // Use a delayed setTimeout once deliveries are idle. The delay must - // be long enough for promise chains to propagate across the VM - // boundary (from resolve() in the host context through to the - // workflow code calling subscribe() in the VM context). Node.js - // does not guarantee that setTimeout(0) fires after all - // cross-context microtasks settle, so we use a small but non-zero - // delay. Any subscribe() call that arrives during this window will - // cancel the check via version invalidation + clearTimeout. - this.pendingUnconsumedTimeout = setTimeout(() => { - this.pendingUnconsumedTimeout = null; - if (this.unconsumedCheckVersion === checkVersion) { - this.pendingUnconsumedCheck = null; - this.onUnconsumedEvent(currentEvent); + // is still unconsumed after the queue drains, it's truly orphaned — or, + // when its type carries no ordering claim, parked for a later consumer. + const checkVersion = ++this.unconsumedCheckVersion; + this.pendingUnconsumedCheck = this.getPromiseQueue() + .then( + // Yield once after the first queue drain so promise chains resumed by + // that drain can run across the VM boundary and append any follow-up + // async work (for example: step_completed resolves -> for-await loop + // resumes -> the next hook payload starts hydrating). + () => new Promise((resolve) => setTimeout(resolve, 0)) + ) + .then(() => this.getPromiseQueue()) + .then(() => { + // Wait out any delivery still in flight before starting the timer. + // The queue draining says the host has no hydration work left; it + // does not say the VM has finished reacting to what was hydrated. + this.whenDeliveryIdle(checkVersion, () => { + // Use a delayed setTimeout once deliveries are idle. The delay must + // be long enough for promise chains to propagate across the VM + // boundary (from resolve() in the host context through to the + // workflow code calling subscribe() in the VM context). Node.js does + // not guarantee that setTimeout(0) fires after all cross-context + // microtasks settle, so we use a small but non-zero delay. Any + // subscribe() call that arrives during this window will cancel the + // check via version invalidation + clearTimeout. + this.pendingUnconsumedTimeout = setTimeout(() => { + this.pendingUnconsumedTimeout = null; + if (this.unconsumedCheckVersion !== checkVersion) { + return; + } + this.pendingUnconsumedCheck = null; + if (mayPark) { + if (this.events[this.eventIndex] !== currentEvent) { + // An append() drain claimed it while the check was in flight. + // Only subscribe() cancels the check, so this is reachable. + return; + } + if (this.park(currentEvent)) { + this.consume(); + return; } - }, getDeferredCheckDelayMs()); - }); + } + this.onUnconsumedEvent(currentEvent); + }, getDeferredCheckDelayMs()); }); - } + }); } /** @@ -267,14 +525,34 @@ export class EventsConsumer { * batch of N parallel step results leaves N-1 of them on that detached path * with the queue already drained, so the walk sits on the ordered event the * VM is about to draw and the window is the only thing standing between a - * healthy run and `ReplayDivergenceError`. On a backend whose deliveries take - * longer than the window, that bet loses: the local race repro corrupts 34 of - * 42 runs at a 10ms window and 0 of 114 at 100ms, on the same event logs. + * healthy run and `ReplayDivergenceError`. + * + * Shortening the window shows that mechanism directly: on identical event logs + * the local race repro corrupts 34 of 42 runs at a 10ms window and 0 of 114 at + * the 100ms default. That measures how the bet loses, not that the default + * loses it, and no measurement of a delivery outrunning 100ms exists either + * way. So read this as retiring the bet rather than as repairing an observed + * failure of that number: the delay is a user-settable env override, which + * leaves the old behaviour one configuration away from losing on any backend. * * Termination is `hasParkedCommittedDelivery`'s: it counts only deliveries * that resolve on their own, so nothing here can gate its own retirement. A * genuinely orphaned event has no delivery to wait on and reaches `fn` on the * first poll. + * + * What the gate gives up: for the ordered events that still reach + * `onUnconsumedEvent` rather than {@link park}, this stops being the thing + * that catches a diverged log while a delivery is in flight. The suspension + * and this check now wake from the same `isDeliveryIdle` edge, and + * `scheduleWhenIdle` fires on the first timer tick after idle while this waits + * a further `getDeferredCheckDelayMs()`. So a run with a pending `sleep()` + * suspends first, and `onWorkflowError` drops the divergence arriving second + * (its `'suspended'` branch demotes to `'replay'` and surfaces nothing), + * leaving a later `resume()` to decline into a cold replay. Pre-gate the + * suspension already won that race whenever the delivery landed inside the + * fixed window, so what changed is that the outcome stopped depending on + * timing. Nothing should treat this check as the mechanism that reports + * divergence on a log the run is still delivering into. */ private whenDeliveryIdle(checkVersion: number, fn: () => void): void { const poll = () => { @@ -291,7 +569,13 @@ export class EventsConsumer { return; } // Held in the same field the fired check uses so subscribe() cancels a - // poll in progress exactly as it cancels the check itself. + // poll in progress too. The two cancellations are not identical: for the + // poll it is the version bump that does the work and the clearTimeout is + // belt-and-braces. Two state machines write this one field, so a poll + // invalidated between scheduling and firing can null out a handle the + // live chain has since stored, which is why every path out of `poll` and + // out of the fired check re-checks the version rather than trusting the + // handle. this.pendingUnconsumedTimeout = setTimeout(poll, 0); }); }; diff --git a/packages/core/src/hook-sleep-interaction.test.ts b/packages/core/src/hook-sleep-interaction.test.ts index 754ee1a138..9505cbb17c 100644 --- a/packages/core/src/hook-sleep-interaction.test.ts +++ b/packages/core/src/hook-sleep-interaction.test.ts @@ -4,7 +4,6 @@ import type { Event } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { createCorrelationIdGenerator } from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import { WorkflowSuspension } from './global.js'; import type { WorkflowOrchestratorContext } from './private.js'; @@ -49,6 +48,8 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { replayPayloadCache: new ReplayPayloadCache(undefined), globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: (event) => { ctxRef.current?.onWorkflowError( new WorkflowRuntimeError( @@ -59,14 +60,7 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { getPromiseQueue: () => promiseQueueHolder.current, }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/private.ts b/packages/core/src/private.ts index f7bedcd471..929fff699e 100644 --- a/packages/core/src/private.ts +++ b/packages/core/src/private.ts @@ -4,7 +4,6 @@ import { withResolvers } from '@workflow/utils'; import type { WorldCapabilities } from '@workflow/world'; -import type { CorrelationIdGenerator } from './correlation-id.js'; import type { EventsConsumer } from './events-consumer.js'; import type { QueueItem } from './global.js'; import type { ReplayPayloadCache } from './replay-payload-cache.js'; @@ -154,11 +153,11 @@ export interface WorkflowOrchestratorContext { invocationsQueue: Map; onWorkflowError: (error: Error) => void; /** - * Mints a correlation id body for one entity family. Every entity a replay - * creates draws from here, and the family is what keeps a disagreement about - * one family's count from renumbering another's. + * Mints the ULID body of a correlation id. Every entity a replay creates + * draws from this one monotonic sequence, so an id is an ordinal over the + * whole run and both replays of a run must draw in the same order. */ - generateCorrelationId: CorrelationIdGenerator; + generateUlid: () => string; generateNanoid: () => string; /** * Sequential promise queue that ensures all event-driven promise resolutions diff --git a/packages/core/src/runtime.test.ts b/packages/core/src/runtime.test.ts index fdaa4ccb39..76724c199a 100644 --- a/packages/core/src/runtime.test.ts +++ b/packages/core/src/runtime.test.ts @@ -947,17 +947,83 @@ describe('workflowEntrypoint replay guards', () => { deploymentId: 'test-deployment', }; + // `hook_created` records a hook a replay decided to create, so its + // position and identity are that replay's decision record: one the current + // replay does not create is divergence on the spot. A `hook_received` here + // would not be, since a delivery nobody claims is parked for a later + // consumer (see 'suspends rather than failing on a hook delivery that + // matches no hook' below). const events: Event[] = [ { eventId: 'event-0', runId: workflowRun.runId, - eventType: 'hook_received', + eventType: 'hook_created', correlationId: 'hook_01HK153X00VFKAJV9XFN9JXXRS', eventData: { token: 'wrong-token', + isWebhook: false, + }, + createdAt: new Date('2024-01-01T00:00:00.000Z'), + }, + ]; + + const createdEvents: unknown[] = []; + const queueCalls: QueueCall[] = []; + await runWorkflowHandlerWithEvents( + `const createHook = globalThis[Symbol.for("WORKFLOW_CREATE_HOOK")]; + async function workflow() { + const hook = createHook({ token: 'expected-token' }); + const payload = await hook; + return payload.message; + }${getWorkflowTransformCode('workflow')}`, + workflowRun, + events, + { createdEvents, queueCalls } + ); + + expect(createdEvents).not.toContainEqual( + expect.objectContaining({ eventType: 'run_failed' }) + ); + expect(queueCalls.map((c) => c.message)).toContainEqual( + expect.objectContaining({ + replayDivergence: { eventId: 'event-0', count: 1 }, + }) + ); + }); + + it('suspends rather than failing on a hook delivery that matches no hook', async () => { + const ops: Promise[] = []; + const workflowRun: WorkflowRun = { + runId: 'wrun_runtime_hook_parked', + workflowName: 'workflow', + status: 'running', + input: await dehydrateWorkflowArguments( + [], + 'wrun_runtime_hook_parked', + undefined, + ops + ), + 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', + }; + + // A delivery for a hook this replay never registers a consumer for. A + // writer that raced this replay can leave one in the log legitimately, so + // it is held for a consumer a later replay may register instead of ending + // the run. + const events: Event[] = [ + { + eventId: 'event-0', + runId: workflowRun.runId, + eventType: 'hook_received', + correlationId: 'hook_01HK153X00VFKAJV9XFN9JXXRS', + eventData: { + token: 'some-other-hook', payload: await dehydrateStepReturnValue( { message: 'hello' }, - 'wrun_runtime_hook_guard', + 'wrun_runtime_hook_parked', undefined, ops ), @@ -983,11 +1049,12 @@ describe('workflowEntrypoint replay guards', () => { expect(createdEvents).not.toContainEqual( expect.objectContaining({ eventType: 'run_failed' }) ); - expect(queueCalls.map((c) => c.message)).toContainEqual( - expect.objectContaining({ - replayDivergence: { eventId: 'event-0', count: 1 }, - }) + expect(createdEvents).toContainEqual( + expect.objectContaining({ eventType: 'hook_created' }) ); + expect( + queueCalls.filter((call) => 'replayDivergence' in (call.message ?? {})) + ).toEqual([]); }); it('replays attribute events before executing a step that loses the same race', async () => { diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index 29fd659ca3..5b34574433 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -73,13 +73,16 @@ import { handleHealthCheckMessage, insertEventByEventId, isPreconditionGuardEnabled, + isSlotGapCheckEnabled, type LoadedEventLog, loadWorkflowRunEvents, memoizeEncryptionKey, + mergeReportedEvents, parseHealthCheckPayload, preconditionEventDelta, preconditionSnapshotParams, queueMessage, + settleEventSlotGap, withHealthCheck, } from './runtime/helpers.js'; import { @@ -2499,13 +2502,34 @@ export function workflowEntrypoint( for (const waitEvent of waitsToComplete) { try { - await createEvent(waitEvent, { + const created = await createEvent(waitEvent, { requestId, ...preconditionSnapshotParams( eventLog.events, eventLog.cursor ), }); + // Bump-and-report: fold what this write skipped over + // into the snapshot the remaining waits are guarded + // against, so each asks for a slot above it. + // + // Only a complete answer. `hasMore` means the World + // returned part of what it was asked for, and the + // missing-completion check below is what decides + // whether this handler still has to fetch. Folding in + // a partial page would make the log look like it + // holds the completion when the rest of the page is + // still unread, so the fetch would be skipped on a + // snapshot that is short of the World's. + if ( + created.events?.length && + created.hasMore !== true + ) { + mergeReportedEvents( + eventLog.events, + created.events + ); + } } catch (err) { if (EntityConflictError.is(err)) { runtimeLogger.info( @@ -2586,6 +2610,30 @@ export function workflowEntrypoint( } } + // A replay reads the log as the complete record of what + // has happened, so a position nothing occupies is + // indistinguishable from an event that never occurred and + // the branch it would have decided gets decided the other + // way. Failing here is the difference between a run that + // reports its own corruption and one that silently + // returns the wrong answer. + // + // A hole that is merely a write mid-commit fills in on + // its own, so settleEventSlotGap re-reads before + // concluding, and adopts whichever log it settled on. + if (isSlotGapCheckEnabled()) { + const settled = await settleEventSlotGap(runId, { + events: eventLog.events, + cursor: eventLog.cursor, + }); + 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.` + ); + } + } + // Completing elapsed waits refreshes the event snapshot. // A concurrent handler may have written the terminal run // event after the initial snapshot but before this @@ -2898,6 +2946,16 @@ 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. This suspension's own hook/wait writes are // NOT in it — they never reach retention anyway, diff --git a/packages/core/src/runtime/helpers.test.ts b/packages/core/src/runtime/helpers.test.ts index c2e098f057..4623f006a4 100644 --- a/packages/core/src/runtime/helpers.test.ts +++ b/packages/core/src/runtime/helpers.test.ts @@ -1,5 +1,6 @@ import { PreconditionFailedError, WorkflowWorldError } from '@workflow/errors'; import type { Event, World } from '@workflow/world'; +import { slotToEventId } from '@workflow/world'; import { ulid } from 'ulid'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { bytesToBase64, deriveRunKeyPair, seal } from '../sealed-box.js'; @@ -12,15 +13,20 @@ import { } from '../serialization.js'; import { appendUniqueEvents, + findEventSlotGap, getWorkflowQueueName, handleHealthCheckMessage, healthCheck, insertEventByEventId, latestEventStateUpdatedAt, loadWorkflowRunEvents, + maxEventSlot, memoizeEncryptionKey, + mergeReportedEvents, preconditionEventDelta, preconditionSnapshotParams, + SLOT_GAP_RECHECK_ATTEMPTS, + settleEventSlotGap, } from './helpers.js'; // Mock the logger to suppress output during tests @@ -700,6 +706,256 @@ describe('preconditionSnapshotParams', () => { }); }); +describe('preconditionSnapshotParams on a slot-numbered run', () => { + let originalGuard: string | undefined; + + beforeEach(() => { + originalGuard = process.env.WORKFLOW_PRECONDITION_GUARD; + process.env.WORKFLOW_PRECONDITION_GUARD = '1'; + }); + + afterEach(() => { + if (originalGuard !== undefined) { + process.env.WORKFLOW_PRECONDITION_GUARD = originalGuard; + } else { + delete process.env.WORKFLOW_PRECONDITION_GUARD; + } + }); + + it('sends eventCount instead of the ULID triple', () => { + const events = [1, 2, 3].map((slot) => makeEvent(slotToEventId(slot))); + + expect(preconditionSnapshotParams(events, 'eid:abc')).toEqual({ + eventCount: 3, + }); + }); + + it('reports the highest slot, not the number of events', () => { + // A slot is claimed by the write that occupies it, and a write that then + // fails leaves it empty forever. Sending the count would make every later + // write in this run ask below the hole and be handed the same events back + // on every single create. + const events = [1, 2, 5].map((slot) => makeEvent(slotToEventId(slot))); + + expect(preconditionSnapshotParams(events, 'eid:abc')).toEqual({ + eventCount: 5, + }); + }); + + it('is invariant under the order the World returned the log in', () => { + const forward = [1, 2, 3].map((slot) => makeEvent(slotToEventId(slot))); + + expect(preconditionSnapshotParams([...forward].reverse(), null)).toEqual( + preconditionSnapshotParams(forward, null) + ); + }); + + it('omits eventCount when the guard is disabled', () => { + process.env.WORKFLOW_PRECONDITION_GUARD = '0'; + + expect( + preconditionSnapshotParams([makeEvent(slotToEventId(1))], null) + ).toEqual({}); + }); + + it('falls back to the ULID triple when one event is not a slot', () => { + // A log may not mix the two schemes. If it somehow does, the slot reading + // is meaningless, so the run is treated as ULID-numbered. + const time = 1_700_000_000_000; + const events = [makeEvent(slotToEventId(1)), makeUlidEvent(time)]; + + expect(preconditionSnapshotParams(events, null)).toEqual({ + stateUpdatedAt: time, + stateEventCount: 2, + }); + }); +}); + +describe('maxEventSlot', () => { + it('is undefined for a log with no slot ids', () => { + expect(maxEventSlot([])).toBeUndefined(); + expect(maxEventSlot([makeUlidEvent(1_700_000_000_000)])).toBeUndefined(); + }); +}); + +/** + * The hole check a replay runs over its loaded log. It gates whether the run + * executes at all, so it is one-sided in the opposite direction from the + * World's density counter: it reports a hole only where the log proves one, and + * says nothing about a log it cannot read as slots. + */ +describe('findEventSlotGap', () => { + const slotLog = (...slots: number[]) => + slots.map((slot) => makeEvent(slotToEventId(slot))); + + it('finds no hole in a dense log', () => { + expect(findEventSlotGap(slotLog(1, 2, 3))).toBeUndefined(); + }); + + it('names the hole and how much of the log is missing', () => { + expect(findEventSlotGap(slotLog(1, 2, 5))).toEqual({ + firstMissingSlot: 3, + missingCount: 2, + maxSlot: 5, + }); + }); + + it('reports the lowest hole when there is more than one', () => { + expect(findEventSlotGap(slotLog(1, 3, 5))).toEqual({ + firstMissingSlot: 2, + missingCount: 2, + maxSlot: 5, + }); + }); + + it('does not depend on the log being in slot order', () => { + // The loaded log is listed pages plus whatever a bump-and-report write + // handed back. mergeReportedEvents restores order, but a check that fails + // a run outright must not be the thing that notices when it did not. + expect(findEventSlotGap(slotLog(3, 1, 2))).toBeUndefined(); + expect(findEventSlotGap(slotLog(4, 1, 2))?.firstMissingSlot).toBe(3); + }); + + it('excuses a log missing only its reserved first slot', () => { + // `start()` posts run_created concurrently with the queue send, so a log + // read in that window legitimately begins at the second slot. + expect(findEventSlotGap(slotLog(2, 3))).toBeUndefined(); + }); + + it('still reports a hole above an absent first slot', () => { + expect(findEventSlotGap(slotLog(2, 4))).toEqual({ + firstMissingSlot: 3, + missingCount: 1, + maxSlot: 4, + }); + }); + + it('says nothing about a log it cannot read as slots', () => { + expect(findEventSlotGap([])).toBeUndefined(); + expect( + findEventSlotGap([makeUlidEvent(1_700_000_000_000)]) + ).toBeUndefined(); + // A ULID anywhere disarms it: the run is not slot-numbered, and a mixed + // log has no density to measure. + expect( + findEventSlotGap([ + ...slotLog(1, 2), + makeUlidEvent(1_700_000_000_000), + ...slotLog(9), + ]) + ).toBeUndefined(); + }); +}); + +/** + * The re-read that stands between a hole and a failed run. A hole can be one + * commit wide: the World allocates a slot inside the insert that occupies it, + * so a writer can commit a higher slot while a lower one is still in flight. + * Only a hole that survives the re-reads is a position no write will ever take. + */ +describe('settleEventSlotGap', () => { + beforeEach(() => { + eventsListMock.mockReset(); + }); + + const slotLog = (...slots: number[]) => + slots.map((slot) => makeEvent(slotToEventId(slot))); + + it('reports no gap for a log that is already dense', async () => { + const settled = await settleEventSlotGap('wrun_test', { + events: slotLog(1, 2, 3), + cursor: 'eid:c', + }); + + expect(settled.gap).toBeUndefined(); + // Nothing to settle, so nothing is re-read. + expect(eventsListMock).not.toHaveBeenCalled(); + }); + + it('adopts the log it re-read once the hole has filled in', async () => { + eventsListMock.mockResolvedValueOnce({ + data: slotLog(1, 2, 3), + cursor: 'eid:filled', + hasMore: false, + }); + + const settled = await settleEventSlotGap('wrun_test', { + events: slotLog(1, 3), + cursor: 'eid:stale', + }); + + expect(settled.gap).toBeUndefined(); + // The caller replays what settled, not the snapshot that looked holey. + expect(settled.log.events.map((e) => e.eventId)).toEqual( + slotLog(1, 2, 3).map((e) => e.eventId) + ); + expect(settled.log.cursor).toBe('eid:filled'); + expect(eventsListMock).toHaveBeenCalledTimes(1); + }); + + it('reports a hole that survives every re-read', async () => { + eventsListMock.mockResolvedValue({ + data: slotLog(1, 4), + cursor: 'eid:stuck', + hasMore: false, + }); + + const settled = await settleEventSlotGap('wrun_test', { + events: slotLog(1, 4), + cursor: 'eid:stuck', + }); + + expect(settled.gap).toEqual({ + firstMissingSlot: 2, + missingCount: 2, + maxSlot: 4, + }); + expect(eventsListMock).toHaveBeenCalledTimes(SLOT_GAP_RECHECK_ATTEMPTS); + }); +}); + +describe('mergeReportedEvents', () => { + it('restores slot order after folding in events below the tail', () => { + // Bump-and-report hands back events the writer had not seen, and they sit + // BELOW the write that reported them. Appending would leave the log in an + // order no replay can walk. + const target = [1, 4].map((slot) => makeEvent(slotToEventId(slot))); + + const added = mergeReportedEvents( + target, + [3, 2].map((slot) => makeEvent(slotToEventId(slot))) + ); + + expect(added).toBe(2); + expect(target.map((e) => e.eventId)).toEqual( + [1, 2, 3, 4].map(slotToEventId) + ); + }); + + it('is a no-op when every reported event is already present', () => { + const target = [1, 2].map((slot) => makeEvent(slotToEventId(slot))); + + expect(mergeReportedEvents(target, [makeEvent(slotToEventId(2))])).toBe(0); + expect(target).toHaveLength(2); + }); + + it('leaves a ULID log in receipt order', () => { + // Only a slot log has an id order the runtime may impose. A World that + // orders by (createdAt, eventId) would be reordered into a log it never + // produced. + const first = makeUlidEvent(1_700_000_000_000); + const second = makeUlidEvent(1_600_000_000_000); + const target = [first]; + + mergeReportedEvents(target, [second]); + + expect(target.map((e) => e.eventId)).toEqual([ + first.eventId, + second.eventId, + ]); + }); +}); + describe('appendUniqueEvents', () => { it('appends in receipt order', () => { const first = makeUlidEvent(1_700_000_000_000); diff --git a/packages/core/src/runtime/helpers.ts b/packages/core/src/runtime/helpers.ts index 862d907dd5..c2483d6285 100644 --- a/packages/core/src/runtime/helpers.ts +++ b/packages/core/src/runtime/helpers.ts @@ -4,13 +4,18 @@ import { WorkflowWorldError, } from '@workflow/errors'; import type { + CreateEventParams, + CreateEventRequest, Event, + EventResult, HealthCheckPayload, ValidQueueName, WorkflowRun, World, } from '@workflow/world'; import { + eventIdToSlot, + FIRST_EVENT_SLOT, getQueueTopicPrefix, HealthCheckPayloadSchema, HOOK_RESUME_INPUT_VERSION, @@ -717,6 +722,23 @@ export function isPreconditionGuardEnabled(): boolean { return process.env.WORKFLOW_PRECONDITION_GUARD !== '0'; } +/** + * Whether a replay refuses to run over a log with a hole in it (see + * {@link findEventSlotGap}). **On by default**; set + * `WORKFLOW_SLOT_GAP_CHECK=0` to replay across holes instead. + * + * The switch exists because the check trades one failure for another. A hole is + * a position claimed by a write that then failed, so most of them stand for an + * event that never happened and replaying past one is correct. But a hole + * standing for an event that *did* happen is indistinguishable from that, and + * replaying past that one produces a run whose result is wrong with nothing to + * show for it. Failing loudly is the recoverable side of the trade, and this is + * the way back out if a fleet turns out to carry benign holes. + */ +export function isSlotGapCheckEnabled(): boolean { + return process.env.WORKFLOW_SLOT_GAP_CHECK !== '0'; +} + /** * The `stateUpdatedAt` value to send with a replay-context event creation: the * *maximum* ULID time (epoch ms) over the events the runtime has loaded. Returns @@ -769,19 +791,202 @@ export function latestEventStateUpdatedAt(events: Event[]): number | undefined { return time; } +/** + * Merge the events a bump-and-report write handed back into the log it was + * derived from, and answer how many of them were new. + * + * Unlike {@link appendUniqueEvents}, this re-sorts. The reported events occupy + * slots *below* the write that reported them, so appending them would put them + * after events they precede — and on a slot-numbered run the id order is the + * World's canonical order, so restoring it is well defined rather than a guess. + * A run that is not slot-numbered cannot produce this report in the first + * place; the sort is skipped rather than applied to ids it cannot order. + */ +export function mergeReportedEvents( + target: Event[], + events: readonly Event[] +): number { + const before = target.length; + appendUniqueEvents(target, events); + const added = target.length - before; + if (added > 0 && maxEventSlot(target) !== undefined) { + target.sort((a, b) => + a.eventId < b.eventId ? -1 : a.eventId > b.eventId ? 1 : 0 + ); + } + return added; +} + +/** + * The highest slot the loaded log occupies, or `undefined` when the run is not + * slot-numbered. A run keeps the id scheme it was created under, so one event + * settles it for the whole log. + * + * The maximum, not the count, and the two are not interchangeable even though + * a healthy log makes them equal. A World hands a position to the insert that + * occupies it, so a write that never lands leaves no hole behind and the log + * stays dense. What the count cannot survive is a *partial* read: a log + * assembled from a truncated report, or read while a concurrent write is + * committing, holds fewer events than its highest position. Counting those + * would make the next write claim to have seen less than it has, so the World + * would report the same events back to it on every attempt. + * + * A hole below the maximum is therefore a property of the read, not of the log, + * which is what lets {@link settleEventSlotGap} re-read instead of giving up. + */ +export function maxEventSlot(events: Event[]): number | undefined { + let max: number | undefined; + for (const event of events) { + const slot = eventIdToSlot(event.eventId); + if (slot === null) { + return undefined; + } + if (max === undefined || slot > max) { + max = slot; + } + } + return max; +} + +/** A position the log skips over, described well enough to name in an error. */ +export interface EventSlotGap { + /** The lowest slot below the log's maximum that no event occupies. */ + firstMissingSlot: number; + /** How many slots below the maximum no event occupies. */ + missingCount: number; + /** The highest slot the log occupies. */ + maxSlot: number; +} + +/** + * The hole in a loaded log, or `undefined` when there is none to find. + * + * On a slot-numbered run the World allocates every position, so a log that + * holds `n` events below slot `n` is missing one. That matters before a replay + * and nowhere else: the replay reads the log as the complete record of what has + * happened, and an absent position is indistinguishable from an event that + * never occurred. The branch it would have decided gets decided the other way, + * and the run diverges quietly rather than failing. + * + * Order-independent, unlike the equivalent audit the World runs over a page it + * just read. A loaded log is assembled from listed pages plus whatever a + * bump-and-report write handed back, and while {@link mergeReportedEvents} + * restores id order, a check that can fail a healthy run should not depend on + * that having happened. + * + * The first slot is never counted. It belongs to `run_created`, which `start()` + * posts concurrently with the queue send, so a log read in that window + * legitimately begins at the second slot and fills in on its own. Every replay + * that races a run's own start would otherwise report a hole. + * + * Returns `undefined` for a log this cannot read as slots at all: an empty one, + * or a run numbered by ULID, where positions carry no density to check. + */ +export function findEventSlotGap( + events: readonly Event[] +): EventSlotGap | undefined { + const occupied = new Set(); + let maxSlot = 0; + for (const event of events) { + const slot = eventIdToSlot(event.eventId); + if (slot === null) { + return undefined; + } + occupied.add(slot); + if (slot > maxSlot) { + maxSlot = slot; + } + } + if (maxSlot === 0) { + return undefined; + } + const floor = occupied.has(FIRST_EVENT_SLOT) + ? FIRST_EVENT_SLOT + : FIRST_EVENT_SLOT + 1; + // Every slot is at or above `floor` by construction, so the log is dense + // exactly when it holds one event per position in `[floor, maxSlot]`. The + // scan below only runs once that has already answered no. + if (occupied.size === maxSlot - floor + 1) { + return undefined; + } + let firstMissingSlot: number | undefined; + let missingCount = 0; + for (let slot = floor; slot <= maxSlot; slot++) { + if (!occupied.has(slot)) { + firstMissingSlot ??= slot; + missingCount++; + } + } + if (firstMissingSlot === undefined) { + return undefined; + } + return { firstMissingSlot, missingCount, maxSlot }; +} + +/** + * How many times a detected hole is re-read before the log is taken at its + * word, and the backoff before each re-read (doubling per attempt). + * + * A hole can be transient. The World allocates a slot inside the insert that + * occupies it, so two concurrent writers can collide, one retry past the other, + * and the higher slot commit first — leaving a window in which the lower one is + * genuinely absent from a strongly-consistent read and fills in a moment later. + * The window is one commit wide, so a short backoff clears it; anything that + * survives all three re-reads is a position no write will ever occupy. + */ +export const SLOT_GAP_RECHECK_ATTEMPTS = 3; +const SLOT_GAP_RECHECK_BASE_DELAY_MS = 25; + +/** + * Re-read a log that looks holey until the hole fills in or the re-reads run + * out, and return the settled log alongside the hole that survived. + * + * Reads are strongly consistent, so a hole is not an artifact of *when* the log + * was read — but it can be an artifact of a write that had not committed yet + * (see {@link SLOT_GAP_RECHECK_ATTEMPTS}). Distinguishing the two costs a + * re-read, which is only ever paid by a replay that already found a hole. + * + * The reload is full rather than incremental: the missing position is below the + * log's maximum, so a cursor-anchored read starts past it and can never see it + * arrive. + */ +export async function settleEventSlotGap( + runId: string, + loaded: LoadedEventLog +): Promise<{ log: LoadedEventLog; gap: EventSlotGap | undefined }> { + let log = loaded; + let gap = findEventSlotGap(log.events); + for ( + let attempt = 0; + gap !== undefined && attempt < SLOT_GAP_RECHECK_ATTEMPTS; + attempt++ + ) { + await new Promise((resolve) => + setTimeout(resolve, SLOT_GAP_RECHECK_BASE_DELAY_MS * 2 ** attempt) + ); + log = await loadWorkflowRunEvents(runId); + gap = findEventSlotGap(log.events); + } + return { log, gap }; +} + /** * The precondition snapshot a replay-context event creation sends, describing * the event log the replay derived the event from. * - * The three fields are one indivisible unit: the backend reads the count only - * relative to the watermark, and returns its inline delta only relative to the - * cursor. Passing them as a single object is what keeps them from drifting - * apart at a call site. + * On a slot-numbered run this is `eventCount`, optionally with the set of + * correlation ids the writer is blocked on. On a ULID-numbered run it is the + * `stateUpdatedAt` / `stateEventCount` / `stateCursor` triple, whose three + * fields are one indivisible unit: the backend reads the count only relative to + * the watermark, and returns its inline delta only relative to the cursor. + * Passing them as a single object is what keeps them from drifting apart at a + * call site. */ export interface PreconditionSnapshotParams { stateUpdatedAt?: number; stateEventCount?: number; stateCursor?: string; + eventCount?: number; } /** @@ -809,6 +1014,14 @@ export function preconditionSnapshotParams( if (!isPreconditionGuardEnabled()) { return {}; } + // A slot-numbered run says with one integer everything the triple was + // approximating, so the two are alternatives rather than a pair. Sending the + // triple here would also be futile: a slot id carries no time, so + // `latestEventStateUpdatedAt` would fail open on every single write. + const eventCount = maxEventSlot(events); + if (eventCount !== undefined) { + return { eventCount }; + } const stateUpdatedAt = latestEventStateUpdatedAt(events); if (stateUpdatedAt === undefined) { return {}; @@ -865,6 +1078,12 @@ export function preconditionEventDelta( }; } +/** Creates one event on a bound run, carrying replay-recovery telemetry. */ +export type EventCreator = ( + data: CreateEventRequest, + params?: CreateEventParams +) => Promise; + /** * CORS headers for health check responses. * Allows the observability UI to check endpoint health from a different origin. diff --git a/packages/core/src/runtime/precondition-guard-replay.test.ts b/packages/core/src/runtime/precondition-guard-replay.test.ts index 49769f2518..fefa5a0bcf 100644 --- a/packages/core/src/runtime/precondition-guard-replay.test.ts +++ b/packages/core/src/runtime/precondition-guard-replay.test.ts @@ -37,7 +37,6 @@ import { dehydrateStepReturnValue, dehydrateWorkflowArguments, } from '../serialization.js'; -import { pinSharedCorrelationIds } from '../test-support/correlation-id-scheme.js'; import { createContext } from '../vm/index.js'; import { getPreconditionMaxInProcessRestarts, @@ -931,8 +930,6 @@ async function inlineClaimRejectionScenario() { }; } -pinSharedCorrelationIds(); - describe('precondition guard through the real replay loop', () => { let originalGuard: string | undefined; let originalRestartBound: string | undefined; 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 1e8473fd6e..52ee1ca227 100644 --- a/packages/core/src/runtime/resume-hook.consumer-preload.test.ts +++ b/packages/core/src/runtime/resume-hook.consumer-preload.test.ts @@ -44,7 +44,6 @@ import { dehydrateStepReturnValue, dehydrateWorkflowArguments, } from '../serialization.js'; -import { pinSharedCorrelationIds } from '../test-support/correlation-id-scheme.js'; import { createContext } from '../vm/index.js'; import { setWorld } from './world.js'; @@ -409,8 +408,6 @@ async function runResumeConsumerScenario(options: { }; } -pinSharedCorrelationIds(); - describe('lazy hook resume consumer preload', () => { afterEach(() => { setWorld(undefined); diff --git a/packages/core/src/runtime/start.test.ts b/packages/core/src/runtime/start.test.ts index 8904600556..28a62755ba 100644 --- a/packages/core/src/runtime/start.test.ts +++ b/packages/core/src/runtime/start.test.ts @@ -2,8 +2,10 @@ import { WorkflowRuntimeError, WorkflowWorldError } from '@workflow/errors'; import { SPEC_VERSION_CURRENT, SPEC_VERSION_LEGACY, + SPEC_VERSION_MAX_SUPPORTED, SPEC_VERSION_SUPPORTS_ATTRIBUTES, SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT, + SPEC_VERSION_SUPPORTS_SLOT_IDENTITY, } from '@workflow/world'; import { afterEach, @@ -136,7 +138,7 @@ describe('start', () => { } as any); await expect(start(validWorkflow, [])).rejects.toThrow( - 'requires a World with matching spec version' + 'supports Worlds with spec version' ); expect(mockEventsCreate).not.toHaveBeenCalled(); expect(mockQueue).not.toHaveBeenCalled(); @@ -174,7 +176,7 @@ describe('start', () => { } as any); await expect(start(validWorkflow, [])).rejects.toThrow( - 'requires a World with matching spec version' + 'supports Worlds with spec version' ); expect(mockEventsCreate).not.toHaveBeenCalled(); expect(mockQueue).not.toHaveBeenCalled(); @@ -186,19 +188,49 @@ describe('start', () => { }); setWorld({ - specVersion: SPEC_VERSION_CURRENT + 1, + specVersion: SPEC_VERSION_MAX_SUPPORTED + 1, getDeploymentId: vi.fn().mockResolvedValue('deploy_123'), events: { create: mockEventsCreate }, queue: mockQueue, } as any); await expect(start(validWorkflow, [])).rejects.toThrow( - 'requires a World with matching spec version' + 'supports Worlds with spec version' ); expect(mockEventsCreate).not.toHaveBeenCalled(); expect(mockQueue).not.toHaveBeenCalled(); }); + it('accepts a world that opts into a spec version above the default', async () => { + // `world-vercel` declares the slot-identity version so its new runs are + // created with slot event ids. An equality check against the default + // would make the runtime refuse the adapter shipped alongside it, and + // the failure surfaces only in e2e against that World. + const validWorkflow = Object.assign(() => Promise.resolve('result'), { + workflowId: 'test-workflow', + }); + + setWorld({ + specVersion: SPEC_VERSION_SUPPORTS_SLOT_IDENTITY, + getDeploymentId: vi.fn().mockResolvedValue('deploy_123'), + events: { create: mockEventsCreate }, + queue: mockQueue, + } as any); + + await start(validWorkflow, []); + + // The declared version is what gets stamped on `run_created`, which is + // what pins the run's id scheme for the rest of its life. + expect(mockEventsCreate).toHaveBeenCalledWith( + expect.stringMatching(/^wrun_/), + expect.objectContaining({ + eventType: 'run_created', + specVersion: SPEC_VERSION_SUPPORTS_SLOT_IDENTITY, + }), + expect.anything() + ); + }); + it('should use provided specVersion when passed in options', async () => { const validWorkflow = Object.assign(() => Promise.resolve('result'), { workflowId: 'test-workflow', diff --git a/packages/core/src/runtime/suspension-handler.test.ts b/packages/core/src/runtime/suspension-handler.test.ts index e63beaf297..6a6e883f7c 100644 --- a/packages/core/src/runtime/suspension-handler.test.ts +++ b/packages/core/src/runtime/suspension-handler.test.ts @@ -4,9 +4,11 @@ import { PreconditionFailedError, WorkflowWorldError, } from '@workflow/errors'; -import type { WorkflowRun, World } from '@workflow/world'; +import type { Event, WorkflowRun, World } from '@workflow/world'; +import { slotToEventId } from '@workflow/world'; import { describe, expect, it, vi } from 'vitest'; import { WorkflowSuspension } from '../global.js'; +import { maxEventSlot } from './helpers.js'; import { ReplayRecoveryReporter } from './replay-recovery-reporter.js'; import { handleSuspension } from './suspension-handler.js'; @@ -542,6 +544,82 @@ describe('handleSuspension', () => { }) ).rejects.toBeInstanceOf(PreconditionFailedError); }); + + describe('skipped-slot reports', () => { + /** A slot-numbered log event, minimal beyond what a snapshot reads. */ + function slotEvent(slot: number, eventType: Event['eventType']): Event { + return { + eventId: slotToEventId(slot), + eventType, + runId: run.runId, + createdAt: new Date(), + } as Event; + } + + /** One wait, so exactly one guarded write carries the report back. */ + function oneWait() { + return new Map([ + [ + 'wait_reported', + { + type: 'wait' as const, + correlationId: 'wait_reported', + resumeAt: new Date(Date.now() + 60_000), + }, + ], + ]); + } + + it('merges a complete report into the caller event log', async () => { + const eventLog = { events: [slotEvent(1, 'run_started')], cursor: null }; + const skipped = slotEvent(2, 'hook_received'); + const eventsCreate = vi.fn(async (_runId, event) => ({ + event: { ...event, eventId: slotToEventId(3) }, + events: [skipped], + })); + + const result = await handleSuspension({ + suspension: new WorkflowSuspension(oneWait(), globalThis), + world: createWorld(eventsCreate), + run, + eventLog, + }); + + expect(result.reportedEventCount).toBe(1); + // The replay that resumes from this log sees the skipped event without + // reloading, and the log still says how far it reaches. + expect(eventLog.events.map((e) => e.eventId)).toEqual([ + slotToEventId(1), + slotToEventId(2), + ]); + expect(maxEventSlot(eventLog.events)).toBe(2); + }); + + it('drops a truncated report instead of raising the log past a hole', async () => { + const eventLog = { events: [slotEvent(1, 'run_started')], cursor: null }; + // Slot 2 is on the same skipped span but absent from the report, so + // merging slot 3 would put the log's maximum above a missing position. + // Later writes read that maximum to say what they have seen, and a World + // only reports the span a write skips, so slot 2 would never be sent. + const skipped = slotEvent(3, 'hook_received'); + const eventsCreate = vi.fn(async (_runId, event) => ({ + event: { ...event, eventId: slotToEventId(4) }, + events: [skipped], + hasMore: true, + })); + + const result = await handleSuspension({ + suspension: new WorkflowSuspension(oneWait(), globalThis), + world: createWorld(eventsCreate), + run, + eventLog, + }); + + expect(result.reportedEventCount).toBe(0); + expect(eventLog.events.map((e) => e.eventId)).toEqual([slotToEventId(1)]); + expect(maxEventSlot(eventLog.events)).toBe(1); + }); + }); }); describe('retainedStepInputsSafe (serialization passivity gate)', () => { diff --git a/packages/core/src/runtime/suspension-handler.ts b/packages/core/src/runtime/suspension-handler.ts index 3d35954d03..b02826df4c 100644 --- a/packages/core/src/runtime/suspension-handler.ts +++ b/packages/core/src/runtime/suspension-handler.ts @@ -32,7 +32,12 @@ import { dehydrateStepArguments } from '../serialization.js'; import * as Attribute from '../telemetry/semantic-conventions.js'; import { getAbortStreamIdFromToken } from '../util.js'; import { getMaxInlineSteps } from './constants.js'; -import { type LoadedEventLog, preconditionSnapshotParams } from './helpers.js'; +import { + type EventCreator, + type LoadedEventLog, + mergeReportedEvents, + preconditionSnapshotParams, +} from './helpers.js'; import { ReplayRecoveryReporter } from './replay-recovery-reporter.js'; export interface SuspensionHandlerParams { @@ -81,6 +86,13 @@ export interface SuspensionHandlerResult { * into the same batch boundary. */ createdStepCorrelationIds: Set; + /** + * How many events this phase's writes reported back as occupying slots they + * skipped over, already merged into the caller's `eventLog.events`. Nonzero + * means the array was reordered to restore slot order, so any index the + * caller cached into it (payload prewarm scan position) is stale. + */ + reportedEventCount: number; /** * The steps whose `step_created` writes were intentionally deferred so the * caller can run them inline via lazy `step_started` events (which create @@ -294,16 +306,52 @@ export async function handleSuspension({ // because the event's correlation id was minted by *this* replay's seeded // sequence, so re-committing it against a corrected log would persist an // event no correct replay produces. - const createGuarded = ( - data: CreateEventRequest, - params?: CreateEventParams - ) => - eventLog - ? createEvent(data, { - ...params, - ...preconditionSnapshotParams(eventLog.events, eventLog.cursor), - }) - : createEvent(data, params); + let reportedEvents = 0; + const createGuarded: EventCreator = async (data, params) => { + if (!eventLog) { + return createEvent(data, params); + } + const log = eventLog; + const result = await createEvent(data, { + ...params, + ...preconditionSnapshotParams(log.events, log.cursor), + }); + // Bump-and-report: the write landed above the slot it asked for, so these + // are the events it was decided without. Merging them here rather than at + // each call site means the rest of this phase's writes — which read the + // same array to build their own snapshot — ask for a slot above them, and + // the replay that resumes from this log sees them without a reload. + // + // A truncated report (`hasMore`) is dropped whole rather than merged, the + // same way the wait loop treats one. It covers a span of positions but + // carries only some of the events on them, so merging it would raise the + // log's highest position past a position whose event is missing. Every + // later write of this phase reads that maximum to say what it has seen, so + // each would claim a position it never saw and the World, which only + // reports the span a write skips, would never send it. Dropping the report + // costs one more round of the same events on the next write and keeps the + // log a prefix of the truth. + if (result.events?.length && result.hasMore !== true) { + const added = mergeReportedEvents(log.events, result.events); + reportedEvents += added; + if (added > 0) { + runtimeLogger.debug('Suspension write skipped occupied slots', { + workflowRunId: runId, + eventType: data.eventType, + eventId: result.event?.eventId, + reported: added, + }); + } + } else if (result.events?.length) { + runtimeLogger.debug('Dropped a truncated skipped-slot report', { + workflowRunId: runId, + eventType: data.eventType, + eventId: result.event?.eventId, + offered: result.events.length, + }); + } + return result; + }; // Separate queue items by type const stepItems = suspension.steps.filter( (item): item is StepInvocationQueueItem => item.type === 'step' @@ -816,6 +864,7 @@ export async function handleSuspension({ hasHookEvents: hooksNeedingCreation.length > 0, hookCreationMs, retainedStepInputsSafe, + reportedEventCount: reportedEvents, }; } diff --git a/packages/core/src/runtime/wait-completion-replay.test.ts b/packages/core/src/runtime/wait-completion-replay.test.ts index a215bab7d3..8da3149a36 100644 --- a/packages/core/src/runtime/wait-completion-replay.test.ts +++ b/packages/core/src/runtime/wait-completion-replay.test.ts @@ -4,6 +4,7 @@ import { type Event, type EventResult, SPEC_VERSION_CURRENT, + slotToEventId, type WorkflowRun, type World, } from '@workflow/world'; @@ -16,7 +17,6 @@ import { dehydrateStepReturnValue, dehydrateWorkflowArguments, } from '../serialization.js'; -import { pinSharedCorrelationIds } from '../test-support/correlation-id-scheme.js'; import { createContext } from '../vm/index.js'; import { setWorld } from './world.js'; @@ -81,6 +81,12 @@ async function runStaleWaitReplayScenario(options: { returnInlineDelta?: boolean; /** Truncate that inline delta (hasMore: true), which must not be absorbed. */ inlineDeltaHasMore?: boolean; + /** + * Number the fake log with slot event ids. That is what makes the handler + * send the slot precondition, so it is the only mode in which a write can + * carry both halves of the World's answer channel. + */ + slotEventIds?: boolean; }) { vi.spyOn(Date, 'now').mockReturnValue(+fixedNow); @@ -125,7 +131,9 @@ async function runStaleWaitReplayScenario(options: { ...data, specVersion: data.specVersion ?? SPEC_VERSION_CURRENT, runId, - eventId: `evt_${eventIndex.toString().padStart(3, '0')}`, + eventId: options.slotEventIds + ? slotToEventId(eventIndex) + : `evt_${eventIndex.toString().padStart(3, '0')}`, createdAt, }) as Event; @@ -453,6 +461,7 @@ async function runStaleWaitReplayScenario(options: { listEvents, listedPages, queue, + staleEvents, preloadedEvents, preloadedCursor, staleEventsCursor, @@ -489,8 +498,6 @@ function expectHookBranchQueued( ); } -pinSharedCorrelationIds(); - describe('workflow handler wait completion replay', () => { afterEach(() => { setWorld(undefined); @@ -684,6 +691,32 @@ describe('workflow handler wait completion replay', () => { expectHookBranchQueued(result); }); + it('asks a slot-numbered World for the delta and the skipped slots at once', async () => { + // On a slot-numbered run the write carries both halves of the World's + // answer channel: `sinceCursor` asks for the delta since the handler's + // snapshot, and `eventCount` states the slot that snapshot reached so a + // bumped write can report what it was decided without. They share + // `events`/`cursor`/`hasMore` on the response, so a World that answers + // both has to pick one, and the delta is the superset. Anything narrower + // returned alongside the delta's cursor loses the difference. + const result = await runStaleWaitReplayScenario({ + includePreloadedCursor: true, + returnInlineDelta: true, + slotEventIds: true, + }); + + const waitWrite = result.createEvent.mock.calls.find( + (call) => (call[1] as CreateEventRequest).eventType === 'wait_completed' + ); + expect(waitWrite?.[2]).toEqual( + expect.objectContaining({ + sinceCursor: result.staleEventsCursor, + eventCount: result.staleEvents.length, + }) + ); + expectHookBranchQueued(result); + }); + it('falls back to the follow-up fetch when the returned delta is truncated', async () => { // hasMore means the page is not the whole delta. Absorbing it would leave // a hole between the events taken and the cursor reported, so the handler diff --git a/packages/core/src/runtime/world-compatibility.ts b/packages/core/src/runtime/world-compatibility.ts index c06c26eefb..247ad73d68 100644 --- a/packages/core/src/runtime/world-compatibility.ts +++ b/packages/core/src/runtime/world-compatibility.ts @@ -1,19 +1,44 @@ import { WorkflowRuntimeError } from '@workflow/errors'; import type { World } from '@workflow/world'; -import { SPEC_VERSION_CURRENT } from '@workflow/world'; +import { + SPEC_VERSION_CURRENT, + SPEC_VERSION_MAX_SUPPORTED, +} from '@workflow/world'; type WorldSpecVersionMetadata = Pick; +/** + * Rejects a World this runtime cannot speak to. + * + * The accepted range is `[SPEC_VERSION_CURRENT, SPEC_VERSION_MAX_SUPPORTED]`. + * Below the current version means an old World package paired with a new + * runtime, which cannot serve the protocol this runtime speaks. Above the + * ceiling means a World built against a newer spec than this runtime knows how + * to read. + * + * The range has a floor and a ceiling rather than a single value because a + * World may opt into a spec version above the default: `world-vercel` declares + * the slot-identity version so its new runs are created with slot event ids, + * while every other World stays on the default. An equality check would make + * this runtime refuse the adapter shipped alongside it. + */ export function assertWorldSupportsRuntimeProtocol( world: WorldSpecVersionMetadata ): void { - if (world.specVersion === SPEC_VERSION_CURRENT) { + const declared = world.specVersion; + if ( + declared !== undefined && + declared !== null && + declared >= SPEC_VERSION_CURRENT && + declared <= SPEC_VERSION_MAX_SUPPORTED + ) { return; } - const supportedVersion = world.specVersion ?? 'none'; + const supportedVersion = declared ?? 'none'; throw new WorkflowRuntimeError( - `This Workflow runtime requires a World with matching spec version ${SPEC_VERSION_CURRENT}, ` + + `This Workflow runtime supports Worlds with spec version ${SPEC_VERSION_CURRENT} ` + + `through ${SPEC_VERSION_MAX_SUPPORTED}, ` + `but the configured World declares spec version ${supportedVersion}. ` + 'Install a World package version compatible with the current Workflow runtime.' ); diff --git a/packages/core/src/step-delivery-hop-count.test.ts b/packages/core/src/step-delivery-hop-count.test.ts index ff511bbcf9..7653f85d4e 100644 --- a/packages/core/src/step-delivery-hop-count.test.ts +++ b/packages/core/src/step-delivery-hop-count.test.ts @@ -29,7 +29,6 @@ import type { Event } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { describe, expect, it, vi } from 'vitest'; -import { createCorrelationIdGenerator } from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import { WorkflowSuspension } from './global.js'; import type { WorkflowOrchestratorContext } from './private.js'; @@ -62,6 +61,8 @@ function setupWorkflowContext( replayPayloadCache, globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: (event) => { ctxRef.current?.onWorkflowError( new WorkflowRuntimeError(`Unconsumed event: ${event.eventType}`) @@ -70,14 +71,7 @@ function setupWorkflowContext( getPromiseQueue: () => promiseQueueHolder.current, }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/step-delivery-ordering.test.ts b/packages/core/src/step-delivery-ordering.test.ts index 60213522f8..300cb6dba4 100644 --- a/packages/core/src/step-delivery-ordering.test.ts +++ b/packages/core/src/step-delivery-ordering.test.ts @@ -4,7 +4,6 @@ import type { Event } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { createCorrelationIdGenerator } from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import { WorkflowSuspension } from './global.js'; import type { WorkflowOrchestratorContext } from './private.js'; @@ -109,6 +108,8 @@ function setupWorkflowContext( replayPayloadCache, globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: (event) => { ctxRef.current?.onWorkflowError( new WorkflowRuntimeError( @@ -119,14 +120,7 @@ function setupWorkflowContext( getPromiseQueue: () => promiseQueueHolder.current, }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/step-hydration-memoization.test.ts b/packages/core/src/step-hydration-memoization.test.ts index 8708c02e5d..e8aabc1d0d 100644 --- a/packages/core/src/step-hydration-memoization.test.ts +++ b/packages/core/src/step-hydration-memoization.test.ts @@ -2,7 +2,6 @@ import type { Event } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { createCorrelationIdGenerator } from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import type { WorkflowOrchestratorContext } from './private.js'; import { ReplayPayloadCache } from './replay-payload-cache.js'; @@ -39,18 +38,13 @@ function setupWorkflowContext( encryptionKey: undefined, globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: () => {}, getPromiseQueue: () => Promise.resolve(), }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/step.test.ts b/packages/core/src/step.test.ts index faf852a54b..574e441110 100644 --- a/packages/core/src/step.test.ts +++ b/packages/core/src/step.test.ts @@ -9,7 +9,6 @@ import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { beforeAll, describe, expect, it, vi } from 'vitest'; import { registerSerializationClass } from './class-serialization.js'; -import { createCorrelationIdGenerator } from './correlation-id.js'; import { EventsConsumer } from './events-consumer.js'; import { WorkflowSuspension } from './global.js'; import type { WorkflowOrchestratorContext } from './private.js'; @@ -58,18 +57,13 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { replayPayloadCache: new ReplayPayloadCache(undefined), globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: () => {}, getPromiseQueue: () => Promise.resolve(), }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/step.ts b/packages/core/src/step.ts index d40bcdb19b..1fcc4c11df 100644 --- a/packages/core/src/step.ts +++ b/packages/core/src/step.ts @@ -25,7 +25,7 @@ export function createUseStep(ctx: WorkflowOrchestratorContext) { ): Promise { const { promise, resolve, reject } = withResolvers(); - const correlationId = `step_${ctx.generateCorrelationId('step')}`; + const correlationId = `step_${ctx.generateUlid()}`; const queueItem: StepInvocationQueueItem = { type: 'step', diff --git a/packages/core/src/telemetry/semantic-conventions.ts b/packages/core/src/telemetry/semantic-conventions.ts index 56f8732eb8..f4fc7cbf4f 100644 --- a/packages/core/src/telemetry/semantic-conventions.ts +++ b/packages/core/src/telemetry/semantic-conventions.ts @@ -82,6 +82,32 @@ export const WorkflowExecutionMode = SemanticConvention<'replay' | 'retained'>( 'workflow.execution.mode' ); +/** + * Events the replay walked past that no consumer claimed, still held when the + * replay stopped. + * + * A non-zero count on a suspension is ordinary: an out-of-band delivery that + * landed ahead of the code that reads it waits for the replay that gets there. + * From inside one replay that is indistinguishable from an event no replay will + * ever claim, because the two differ only in what the next replay does. So the + * count goes on the span instead of failing the run, and the case worth acting + * on is a query across a run's spans: the same + * {@link WorkflowParkedEventId} held on suspension after suspension. + */ +export const WorkflowParkedEventsCount = SemanticConvention( + 'workflow.events.parked.count' +); + +/** Oldest event still held unclaimed when the replay stopped. */ +export const WorkflowParkedEventId = SemanticConvention( + 'workflow.events.parked.event_id' +); + +/** Type of the oldest event still held unclaimed when the replay stopped. */ +export const WorkflowParkedEventType = SemanticConvention( + 'workflow.events.parked.event_type' +); + /** Number of arguments passed to the workflow */ export const WorkflowArgumentsCount = SemanticConvention( 'workflow.arguments.count' diff --git a/packages/core/src/test-support/correlation-id-scheme.ts b/packages/core/src/test-support/correlation-id-scheme.ts deleted file mode 100644 index 32a000dfdd..0000000000 --- a/packages/core/src/test-support/correlation-id-scheme.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { afterAll, beforeAll } from 'vitest'; - -/** - * Pins a test file to the run-wide shared correlation-id sequence. - * - * Replay tests that drive the real `workflowEntrypoint` against an event log - * with hardcoded correlation ids can only match under the scheme those ids were - * minted by, and the fixtures in this repo predate per-kind sequences. Files - * whose fixture ids are derived rather than written out run under whichever - * scheme `WORKFLOW_PER_KIND_CORRELATION_IDS` selects; per-kind minting itself is - * covered by `correlation-id.test.ts`. - */ -export function pinSharedCorrelationIds(): void { - let original: string | undefined; - beforeAll(() => { - original = process.env.WORKFLOW_PER_KIND_CORRELATION_IDS; - process.env.WORKFLOW_PER_KIND_CORRELATION_IDS = '0'; - }); - afterAll(() => { - if (original === undefined) { - delete process.env.WORKFLOW_PER_KIND_CORRELATION_IDS; - } else { - process.env.WORKFLOW_PER_KIND_CORRELATION_IDS = original; - } - }); -} diff --git a/packages/core/src/unconsumed-check-delivery-idle.test.ts b/packages/core/src/unconsumed-check-delivery-idle.test.ts index 8a13f74dd8..5acde2631b 100644 --- a/packages/core/src/unconsumed-check-delivery-idle.test.ts +++ b/packages/core/src/unconsumed-check-delivery-idle.test.ts @@ -1,7 +1,11 @@ import { withResolvers } from '@workflow/utils'; import type { Event } from '@workflow/world'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { EventConsumerResult, EventsConsumer } from './events-consumer.js'; +import { + EventConsumerResult, + EventsConsumer, + MIN_DEFERRED_CHECK_DELAY_MS, +} from './events-consumer.js'; import type { WorkflowOrchestratorContext } from './private.js'; import { isDeliveryIdle, registerDeliveryBarrier } from './private.js'; @@ -17,11 +21,15 @@ import { isDeliveryIdle, registerDeliveryBarrier } from './private.js'; * The unconsumed-event check used to resolve that by waiting a fixed * `DEFERRED_CHECK_DELAY_MS` after the promise queue drained, which is a bet * that every delivery lands inside the window. Replaying a batch of N parallel - * step results loses it: the queue drains with N-1 of them still on the - * detached path, and the check declares `ReplayDivergenceError` against a log - * the very same replay goes on to reproduce exactly. Measured on the event-log - * race repro against world-postgres, on identical event logs: 0 of 114 runs - * corrupted with a 100ms window, 34 of 42 with a 10ms one. + * step results is what puts that bet under load: the queue drains with N-1 of + * them still on the detached path, so whether the check declares + * `ReplayDivergenceError` against a log the very same replay goes on to + * reproduce exactly comes down to the clock. Shrinking the window makes it lose. + * Measured on the event-log race repro against world-postgres, on identical + * event logs: 0 of 114 runs corrupted at the 100ms default, 34 of 42 at 10ms. + * That is evidence for the mechanism, not for the default being too short; the + * delay is also a user-settable override, so the old behaviour stayed one + * configuration away from losing. * * `hasParkedCommittedDelivery` in private.ts already documents this hazard for * the suspension path (vercel/workflow#3183). These tests pin the same guard @@ -30,6 +38,17 @@ import { isDeliveryIdle, registerDeliveryBarrier } from './private.js'; * and the check must still fire for an event no delivery is waiting on. */ +/** + * Shortest delay the check accepts, and a wait comfortably past it. Both derive + * from the floor so that raising the floor cannot quietly turn the negative + * assertions below into no-ops: were the stub a hardcoded number the floor + * outgrew, `getDeferredCheckDelayMs` would clamp it up, the delay would stop + * being shorter than the delivery, and the check would be "not fired yet" + * rather than "held off by the gate". + */ +const CHECK_DELAY_MS = MIN_DEFERRED_CHECK_DELAY_MS; +const PAST_CHECK_DELAY_MS = CHECK_DELAY_MS * 25; + function createEvent(overrides: Partial = {}): Event { return { id: 'event-1', @@ -69,7 +88,7 @@ describe('unconsumed-event check against in-flight deliveries', () => { it('does not declare divergence while a step delivery is outstanding', async () => { // Far shorter than the delivery below, so the run survives only if the // check waits for the delivery rather than for the clock. - vi.stubEnv('WORKFLOW_DEFERRED_CHECK_DELAY_MS', '10'); + vi.stubEnv('WORKFLOW_DEFERRED_CHECK_DELAY_MS', String(CHECK_DELAY_MS)); const ctx = createDeliveryContext(); const event = createEvent(); @@ -88,7 +107,7 @@ describe('unconsumed-event check against in-flight deliveries', () => { vi.fn().mockReturnValue(EventConsumerResult.NotConsumed) ); - await new Promise((resolve) => setTimeout(resolve, 250)); + await new Promise((resolve) => setTimeout(resolve, PAST_CHECK_DELAY_MS)); expect(onUnconsumedEvent).not.toHaveBeenCalled(); // The delivery lands and the workflow reaches the call this event records. @@ -98,12 +117,12 @@ describe('unconsumed-event check against in-flight deliveries', () => { await vi.waitFor(() => { expect(consumer.eventIndex).toBe(1); }); - await new Promise((resolve) => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, PAST_CHECK_DELAY_MS)); expect(onUnconsumedEvent).not.toHaveBeenCalled(); }); it('does not declare divergence while a payload is hydrating', async () => { - vi.stubEnv('WORKFLOW_DEFERRED_CHECK_DELAY_MS', '10'); + vi.stubEnv('WORKFLOW_DEFERRED_CHECK_DELAY_MS', String(CHECK_DELAY_MS)); const ctx = createDeliveryContext(); const event = createEvent(); @@ -121,7 +140,7 @@ describe('unconsumed-event check against in-flight deliveries', () => { vi.fn().mockReturnValue(EventConsumerResult.NotConsumed) ); - await new Promise((resolve) => setTimeout(resolve, 250)); + await new Promise((resolve) => setTimeout(resolve, PAST_CHECK_DELAY_MS)); expect(onUnconsumedEvent).not.toHaveBeenCalled(); ctx.pendingDeliveries--; diff --git a/packages/core/src/workflow.test.ts b/packages/core/src/workflow.test.ts index fdeb2bd97a..f47e1d1d7e 100644 --- a/packages/core/src/workflow.test.ts +++ b/packages/core/src/workflow.test.ts @@ -13,15 +13,12 @@ import { dehydrateWorkflowArguments, hydrateWorkflowReturnValue, } from './serialization.js'; -import { pinSharedCorrelationIds } from './test-support/correlation-id-scheme.js'; import { createContext } from './vm/index.js'; import { replayWorkflow, resumeWorkflow, runWorkflow } from './workflow.js'; // No encryption key = encryption disabled const noEncryptionKey = undefined; -pinSharedCorrelationIds(); - describe('runWorkflow', () => { const getWorkflowTransformCode = (workflowName?: string) => `;globalThis.__private_workflows = new Map(); @@ -382,13 +379,16 @@ describe('runWorkflow', () => { assert(suspended.type === 'suspended'); // A strict extension whose appended suffix the VM cannot consume: the - // resume starts, then diverges mid-execution. + // resume starts, then diverges mid-execution. It has to be a + // replay-origin type: the consumer walks past an unclaimed delivery and + // holds it for a later consumer, so only an event whose position is a + // replay's own decision record diverges on the spot. const alien = { eventId: 'event-alien', runId: run.runId, - eventType: 'hook_received', - correlationId: 'hook_unknown', - eventData: {}, + eventType: 'step_created', + correlationId: 'step_unknown', + eventData: { stepName: 'unknown' }, createdAt: new Date('2024-01-01T00:00:01.000Z'), } as Event; await expect(resumeWorkflow(suspended.session, [alien])).rejects.toThrow( @@ -406,7 +406,7 @@ describe('runWorkflow', () => { // Turbo's first delivery synthesizes `startedAt` from the local clock, // while later (non-turbo) deliveries load the server-canonical `startedAt`. // Replay matching must NOT depend on `startedAt`: correlation IDs come from - // `generateCorrelationId`, keyed off the run-ID-recovered `fixedTimestamp`, not + // `generateUlid`, keyed off the run-ID-recovered `fixedTimestamp`, not // `startedAt`. Here the recorded `add` event uses the createdAt-derived // correlation ID, but `startedAt` is months away — replay must still // regenerate the same ID and consume the completion rather than throwing diff --git a/packages/core/src/workflow.ts b/packages/core/src/workflow.ts index a6e1b0672b..17cf8ea855 100644 --- a/packages/core/src/workflow.ts +++ b/packages/core/src/workflow.ts @@ -15,10 +15,6 @@ import type { Event, WorkflowRun, WorldCapabilities } from '@workflow/world'; import { SPEC_VERSION_SUPPORTS_COMPRESSION } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; -import { - createCorrelationIdGenerator, - isPerKindCorrelationIdsEnabled, -} from './correlation-id.js'; import { EventConsumerResult, EventsConsumer } from './events-consumer.js'; import type { QueueItem } from './global.js'; import { ENOTSUP, WorkflowSuspension } from './global.js'; @@ -155,6 +151,16 @@ export type WorkflowResult = readonly type: 'suspended'; readonly suspension: WorkflowSuspension; readonly session: WorkflowSession; + /** + * Events the replay walked past unclaimed and is still holding, if any. + * Ordinary on a suspension, and only actionable across a run's + * suspensions, so it is reported for telemetry rather than acted on here. + */ + readonly parked?: { + readonly count: number; + readonly eventId: string; + readonly eventType: string; + }; }; /** @@ -238,6 +244,20 @@ function recordResult( }); } else if (span) { applyWorkflowSuspensionToSpan(result.suspension, span); + // Events this pass walked past unclaimed and is still holding. Ordinary on + // a suspension: an out-of-band delivery that landed ahead of the code that + // reads it waits for the pass that reaches that code, and failing here + // would fail exactly the runs that tolerance exists for. The case that is + // not ordinary — the same event still held pass after pass — is a shape + // across these spans, which is why the eventId is on each one and no pass + // tries to rule on it alone. + if (result.parked) { + span.setAttributes({ + ...Attribute.WorkflowParkedEventsCount(result.parked.count), + ...Attribute.WorkflowParkedEventId(result.parked.eventId), + ...Attribute.WorkflowParkedEventType(result.parked.eventType), + }); + } } return result; } @@ -325,14 +345,12 @@ async function createWorkflowSession({ : `http://localhost:${(await getPortLazy()) ?? 3000}` ); - const seed = `${workflowRun.runId}:${workflowRun.workflowName}:${workflowRun.deploymentId}`; - const { context, globalThis: vmGlobalThis, updateTimestamp, } = createContext({ - seed, + seed: `${workflowRun.runId}:${workflowRun.workflowName}:${workflowRun.deploymentId}`, fixedTimestamp, }); @@ -371,14 +389,9 @@ async function createWorkflowSession({ }; const ulid = monotonicFactory(() => vmGlobalThis.Math.random()); - const generateCorrelationId = createCorrelationIdGenerator({ - seed, - fixedTimestamp, - // Correlation IDs must be replay-stable. `startedAt` differs between a - // turbo delivery and a later server-backed replay, so use fixedTimestamp. - positional: () => ulid(fixedTimestamp), - perKind: isPerKindCorrelationIdsEnabled(), - }); + // Correlation IDs must be replay-stable. `startedAt` differs between a turbo + // delivery and a later server-backed replay, so use fixedTimestamp. + const generateUlid = () => ulid(fixedTimestamp); const generateNanoid = nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * vmGlobalThis.Math.random()) ); @@ -393,9 +406,20 @@ async function createWorkflowSession({ // is before any delivery can be registered against it. const deliveryIdleHolder = { current: (): boolean => true }; + // The VM clock only ever moves forward. Consumption order is log order for + // everything whose order the replay decides, but an event the consumer + // parked is delivered after the walk has already passed events written after + // it, and letting its `createdAt` set the clock would make `Date.now()` go + // backwards inside a single replay. + let clock = fixedTimestamp; + const eventsConsumer = new EventsConsumer(events, { onConsumedEvent: (event) => { - updateTimestamp(+event.createdAt); + const at = +event.createdAt; + if (at > clock) { + clock = at; + updateTimestamp(at); + } }, onUnconsumedEvent: (event) => { onWorkflowError( @@ -416,7 +440,7 @@ async function createWorkflowSession({ globalThis: vmGlobalThis, onWorkflowError, eventsConsumer, - generateCorrelationId, + generateUlid, generateNanoid, invocationsQueue: new Map(), // Use getter/setter so the EventsConsumer's getPromiseQueue() always @@ -497,11 +521,11 @@ async function createWorkflowSession({ // Serialization mints stream ids through this symbol, and calls it with no // seed time. `monotonicFactory` returns `encodeTime(lastTime)` on its // increment branch, so one such call latches the *host* wall clock into - // `lastTime` and every id the run mints afterwards carries that timestamp - // instead of `fixedTimestamp` — a value that differs on every replay. - // Binding the seed time here keeps the whole run on one replay-stable clock. + // `lastTime`, and every id the run mints afterwards carries that timestamp + // instead of `fixedTimestamp`, a value that differs on every replay. Binding + // the seed time here keeps the whole run on one replay-stable clock. // @ts-expect-error - `@types/node` says symbol is not valid, but it does work - vmGlobalThis[STABLE_ULID] = () => generateCorrelationId('stream'); + vmGlobalThis[STABLE_ULID] = generateUlid; // Workflow code must import the deterministic `fetch` step from `workflow`. vmGlobalThis.fetch = () => { @@ -1066,12 +1090,34 @@ async function createWorkflowSession({ result = await Promise.race([workflowBody, interruption.promise]); } catch (error) { if (state.type === 'suspended' && error === state.suspension) { - return { type: 'suspended', suspension: state.suspension, session }; + return { + type: 'suspended', + suspension: state.suspension, + session, + // A suspension is not a settling point: the consumer for something + // held may well be registered by the replay that follows this one. + // So it is carried out for the span instead of being judged here. + parked: eventsConsumer.parkedSummary, + }; } return failWorkflow(error); } state = { type: 'completed' }; + // The consumer walks past events whose type carries no ordering claim and + // holds them for a consumer it expects a later `subscribe()` to register. + // The workflow function returning is the point where that expectation is + // settled: nothing more will subscribe, so anything still held was never + // anyone's, and completing here would drop it silently. + const stranded = eventsConsumer.strandedEvent; + if (stranded) { + return failWorkflow( + new ReplayDivergenceError( + `Replay finished without consuming event: eventType=${stranded.eventType}, correlationId=${stranded.correlationId}, eventId=${stranded.eventId}.`, + { eventId: stranded.eventId } + ) + ); + } try { const output = await dehydrateWorkflowReturnValue( result, diff --git a/packages/core/src/workflow/abort-controller.ts b/packages/core/src/workflow/abort-controller.ts index 3d94dee210..4d5d9a08af 100644 --- a/packages/core/src/workflow/abort-controller.ts +++ b/packages/core/src/workflow/abort-controller.ts @@ -111,7 +111,7 @@ export function createCreateAbortController(ctx: WorkflowOrchestratorContext) { readonly [ABORT_HOOK_TOKEN]: string; constructor() { - const id = ctx.generateCorrelationId('abort'); + const id = ctx.generateUlid(); const streamName = getAbortStreamId(id); const hookToken = `abrt_${id}`; @@ -120,10 +120,8 @@ export function createCreateAbortController(ctx: WorkflowOrchestratorContext) { this.signal = new WorkflowAbortSignal(streamName, hookToken); // Register an internal system hook in the invocations queue. - // isSystem prevents token namespace conflicts with user hooks. The id - // draws from its own family, not `hook`, so constructing an abort - // controller does not renumber hooks the workflow creates later. - const correlationId = `hook_${ctx.generateCorrelationId('abortHook')}`; + // isSystem prevents token namespace conflicts with user hooks. + const correlationId = `hook_${ctx.generateUlid()}`; ctx.invocationsQueue.set(correlationId, { type: 'hook', correlationId, diff --git a/packages/core/src/workflow/attribute-dispatcher.ts b/packages/core/src/workflow/attribute-dispatcher.ts index 95ac76dc08..760dee8bbc 100644 --- a/packages/core/src/workflow/attribute-dispatcher.ts +++ b/packages/core/src/workflow/attribute-dispatcher.ts @@ -17,7 +17,7 @@ export function createSetAttributes(ctx: WorkflowOrchestratorContext) { options: { allowReservedAttributes?: boolean } = {} ): Promise { const { promise, resolve } = withResolvers(); - const correlationId = `attr_${ctx.generateCorrelationId('attr')}`; + const correlationId = `attr_${ctx.generateUlid()}`; const queueItem: AttributeInvocationQueueItem = { type: 'attribute', correlationId, diff --git a/packages/core/src/workflow/hook.test.ts b/packages/core/src/workflow/hook.test.ts index fbc2c8063e..bdaa3e5442 100644 --- a/packages/core/src/workflow/hook.test.ts +++ b/packages/core/src/workflow/hook.test.ts @@ -12,7 +12,6 @@ import { aliasSerializationClass, RUN_CLASS_ID, } from '../class-serialization.js'; -import { createCorrelationIdGenerator } from '../correlation-id.js'; import { EventsConsumer } from '../events-consumer.js'; import { WorkflowSuspension } from '../global.js'; import type { WorkflowOrchestratorContext } from '../private.js'; @@ -43,18 +42,13 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { replayPayloadCache: new ReplayPayloadCache(undefined), globalThis: context.globalThis, eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: () => {}, getPromiseQueue: () => Promise.resolve(), }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/workflow/hook.ts b/packages/core/src/workflow/hook.ts index e95998b858..d5d5bed8ec 100644 --- a/packages/core/src/workflow/hook.ts +++ b/packages/core/src/workflow/hook.ts @@ -96,7 +96,7 @@ export function createCreateHook(ctx: WorkflowOrchestratorContext) { } // Generate hook ID and token - const correlationId = `hook_${ctx.generateCorrelationId('hook')}`; + const correlationId = `hook_${ctx.generateUlid()}`; const token = options.token ?? ctx.generateNanoid(); const tokenRetentionUntil = options.experimental_minRetention === undefined diff --git a/packages/core/src/workflow/sleep.test.ts b/packages/core/src/workflow/sleep.test.ts index 5b2d2e6edf..1dd45b52ce 100644 --- a/packages/core/src/workflow/sleep.test.ts +++ b/packages/core/src/workflow/sleep.test.ts @@ -4,7 +4,6 @@ import type { Event } from '@workflow/world'; import * as nanoid from 'nanoid'; import { monotonicFactory } from 'ulid'; import { describe, expect, it, vi } from 'vitest'; -import { createCorrelationIdGenerator } from '../correlation-id.js'; import { EventsConsumer } from '../events-consumer.js'; import { WorkflowSuspension } from '../global.js'; import type { WorkflowOrchestratorContext } from '../private.js'; @@ -28,6 +27,8 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { globalThis: context.globalThis, // ctx.onWorkflowError is accessed via closure — it's defined below on the same object eventsConsumer: new EventsConsumer(events, { + // Fake context: no deliveries are modeled, so the gate is a no-op here. + isDeliveryIdle: () => true, onUnconsumedEvent: (event) => { ctx.onWorkflowError( new ReplayDivergenceError( @@ -39,14 +40,7 @@ function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext { getPromiseQueue: () => Promise.resolve(), }), invocationsQueue: new Map(), - generateCorrelationId: createCorrelationIdGenerator({ - seed: 'test', - fixedTimestamp: workflowStartedAt, - positional: () => ulid(workflowStartedAt), - // The event logs in this file hardcode correlation ids the run-wide - // shared sequence minted, so replay only matches under that scheme. - perKind: false, - }), + generateUlid: () => ulid(workflowStartedAt), generateNanoid: nanoid.customRandom(nanoid.urlAlphabet, 21, (size) => new Uint8Array(size).map(() => 256 * context.globalThis.Math.random()) ), diff --git a/packages/core/src/workflow/sleep.ts b/packages/core/src/workflow/sleep.ts index 5245745e62..c8848d0c3a 100644 --- a/packages/core/src/workflow/sleep.ts +++ b/packages/core/src/workflow/sleep.ts @@ -15,7 +15,7 @@ export function createSleep(ctx: WorkflowOrchestratorContext) { param: StringValue | Date | number ): Promise { const { promise, resolve } = withResolvers(); - const correlationId = `wait_${ctx.generateCorrelationId('wait')}`; + const correlationId = `wait_${ctx.generateUlid()}`; // Calculate the resume time const resumeAt = parseDurationToDate(param); diff --git a/packages/core/tsconfig.json b/packages/core/tsconfig.json index 7749f99278..98892887a9 100644 --- a/packages/core/tsconfig.json +++ b/packages/core/tsconfig.json @@ -5,5 +5,5 @@ "rootDir": "./src" }, "include": ["src"], - "exclude": ["node_modules", "**/*.test.ts", "src/test-support"] + "exclude": ["node_modules", "**/*.test.ts"] } diff --git a/packages/world-local/src/fs.test.ts b/packages/world-local/src/fs.test.ts index 100084a80b..1f647b5bb9 100644 --- a/packages/world-local/src/fs.test.ts +++ b/packages/world-local/src/fs.test.ts @@ -976,6 +976,86 @@ describe('fs utilities', () => { } }); }); + + describe('sort-key cursors', () => { + // Slot-numbered events: the file id carries the sort key, and the + // stored `createdAt` deliberately runs backwards relative to it, the + // way a writer that loses a slot race and bumps produces a higher slot + // with an older timestamp. + const RUN_PREFIX = 'run1-'; + const SLOT_COUNT = 12; + const slotId = (slot: number) => `evnt_${String(slot).padStart(26, '0')}`; + + beforeEach(async () => { + const baseTime = new Date('2024-01-01T00:00:00.000Z').getTime(); + const files: Record = {}; + for (let slot = 1; slot <= SLOT_COUNT; slot++) { + const id = slotId(slot); + files[`${RUN_PREFIX}${id}`] = { + id, + name: `event-${slot}`, + createdAt: new Date(baseTime - ms(`${slot}m`)), + }; + } + await createFilesystem(testDir, files); + }); + + const query = (cursor?: string) => + paginatedFileSystemQuery({ + directory: testDir, + schema: TestItemSchema, + filePrefix: RUN_PREFIX, + getCreatedAt: () => null, + getId: (item: TestItem) => item.id, + getSortKey: (item: TestItem) => item.id, + getSortKeyFromFileId: (fileId: string) => + fileId.slice(RUN_PREFIX.length), + sortOrder: 'asc', + limit: 5, + cursor, + }); + + it('pages through the whole log in slot order', async () => { + const seen: string[] = []; + let cursor: string | undefined; + let hasMore = true; + + while (hasMore) { + const page: PaginatedResponse = await query(cursor); + seen.push(...page.data.map((item) => item.id)); + cursor = page.cursor ?? undefined; + hasMore = page.hasMore; + } + + expect(seen).toEqual( + Array.from({ length: SLOT_COUNT }, (_, index) => slotId(index + 1)) + ); + }); + + it('does not read files the cursor has already passed', async () => { + const firstPage = await query(); + assert(firstPage.cursor, 'expected first page cursor to be defined'); + + const readFile = vi.spyOn(fs, 'readFile'); + const secondPage = await query(firstPage.cursor); + const readIds = readFile.mock.calls.map((call) => + path.basename(String(call[0]), '.json') + ); + readFile.mockRestore(); + + // Only the tail past the cursor is opened. Without the filename-level + // prefilter every page reads every file for the run, which makes + // walking a long event log quadratic. + expect(readIds).toEqual( + Array.from( + { length: SLOT_COUNT - firstPage.data.length }, + (_, index) => + `${RUN_PREFIX}${slotId(firstPage.data.length + index + 1)}` + ) + ); + expect(secondPage.data).toHaveLength(5); + }); + }); }); describe('concurrent writes', () => { diff --git a/packages/world-local/src/fs.ts b/packages/world-local/src/fs.ts index 16f740c4f1..29cc2990f8 100644 --- a/packages/world-local/src/fs.ts +++ b/packages/world-local/src/fs.ts @@ -580,24 +580,70 @@ interface PaginatedFileSystemQueryConfig { cursor?: string; getCreatedAt(filename: string): Date | null; getId?(item: T): string; + /** + * Opt an item out of `createdAt` ordering in favor of a total order carried + * by the item itself. + * + * Slot-numbered events are the case this exists for: the slot is assigned + * at the publish, which is the linearization point, while `createdAt` is + * stamped when the request arrives. A writer that loses a slot race and + * bumps therefore lands at a higher slot with an older `createdAt`, and + * ordering by time would hand back a log whose order contradicts the + * positions the World assigned. Return null to keep the `createdAt` + * ordering (ULID-numbered events, and every other entity). + */ + getSortKey?(item: T): string | null; + /** + * The same key as {@link getSortKey}, read off the file id instead of the + * item, so a sort-key cursor can skip files without opening them. + * + * Without it a sort-key scan has no filename-level prefilter and every page + * loads and parses every file for the run, which makes walking a long event + * log quadratic. Return null when the file id does not carry the key; those + * files are kept and decided by the item-level filter. + */ + getSortKeyFromFileId?(fileId: string): string | null; } -// Cursor format: "timestamp|id" for tie-breaking + +// Cursor formats: +// "timestamp|id" — createdAt order, id for tie-breaking +// "key:" — sort-key order (see getSortKey) +// A run never mixes the two, so a cursor never has to cross formats mid-scan. +export const SORT_KEY_CURSOR_PREFIX = 'key:'; + interface ParsedCursor { timestamp: Date; id: string | null; + sortKey: string | null; } function parseCursor(cursor: string | undefined): ParsedCursor | null { if (!cursor) return null; + if (cursor.startsWith(SORT_KEY_CURSOR_PREFIX)) { + return { + timestamp: new Date(0), + id: null, + sortKey: cursor.slice(SORT_KEY_CURSOR_PREFIX.length), + }; + } + const parts = cursor.split('|'); return { timestamp: new Date(parts[0]), id: parts[1] || null, + sortKey: null, }; } -function createCursor(timestamp: Date, id: string | undefined): string { +function createCursor( + timestamp: Date, + id: string | undefined, + sortKey?: string | null +): string { + if (sortKey) { + return `${SORT_KEY_CURSOR_PREFIX}${sortKey}`; + } return id ? `${timestamp.toISOString()}|${id}` : timestamp.toISOString(); } @@ -616,6 +662,8 @@ export async function paginatedFileSystemQuery( cursor, getCreatedAt, getId, + getSortKey, + getSortKeyFromFileId, } = config; // Validate filePrefix (typically `${runId}-`) so request-derived prefixes @@ -644,7 +692,20 @@ export async function paginatedFileSystemQuery( const parsedCursor = parseCursor(cursor); let candidateFileIds = filteredFileIds; - if (parsedCursor) { + if (parsedCursor?.sortKey && getSortKeyFromFileId) { + // Sort-key cursor: the filename carries the key, so the same strict + // comparison the item-level filter below applies can run here, before any + // file is read. + const cursorSortKey = parsedCursor.sortKey; + candidateFileIds = filteredFileIds.filter((fileId) => { + const key = getSortKeyFromFileId(fileId); + if (key === null) { + return true; + } + const comparison = key.localeCompare(cursorSortKey); + return sortOrder === 'desc' ? comparison < 0 : comparison > 0; + }); + } else if (parsedCursor && !parsedCursor.sortKey) { candidateFileIds = filteredFileIds.filter((fileId) => { const filenameDate = getCreatedAt(`${fileId}.json`); if (filenameDate) { @@ -717,6 +778,23 @@ export async function paginatedFileSystemQuery( for (const item of loadedBatch) { if (!item) continue; + const itemSortKey = getSortKey?.(item) ?? null; + + if (parsedCursor?.sortKey) { + // Sort-key cursor: the key alone is the total order, so there is no + // tie to break. An item without a key cannot be placed relative to + // the cursor at all — that would mean a run mixed the two schemes — + // so keep it and let the comparator below order it. + if (itemSortKey) { + const comparison = itemSortKey.localeCompare(parsedCursor.sortKey); + if (sortOrder === 'desc' ? comparison >= 0 : comparison <= 0) { + continue; + } + } + validItems.push(item); + continue; + } + // Double-check cursor filtering with actual createdAt from JSON // (in case ULID timestamp differs from stored createdAt) if (parsedCursor) { @@ -746,8 +824,18 @@ export async function paginatedFileSystemQuery( } } - // 5. Sort by createdAt (and by ID for tie-breaking if getId is provided) + // 5. Sort by sortKey when the items carry one, else by createdAt (and by ID + // for tie-breaking if getId is provided) validItems.sort((a, b) => { + if (getSortKey) { + const aKey = getSortKey(a); + const bKey = getSortKey(b); + if (aKey !== null && bKey !== null) { + return sortOrder === 'asc' + ? aKey.localeCompare(bKey) + : bKey.localeCompare(aKey); + } + } const aTime = a.createdAt.getTime(); const bTime = b.createdAt.getTime(); const timeComparison = sortOrder === 'asc' ? aTime - bTime : bTime - aTime; @@ -771,7 +859,8 @@ export async function paginatedFileSystemQuery( items.length > 0 ? createCursor( items[items.length - 1].createdAt, - getId?.(items[items.length - 1]) + getId?.(items[items.length - 1]), + getSortKey?.(items[items.length - 1]) ) : null; diff --git a/packages/world-local/src/index.ts b/packages/world-local/src/index.ts index dfa56584df..b89fd67af5 100644 --- a/packages/world-local/src/index.ts +++ b/packages/world-local/src/index.ts @@ -80,6 +80,10 @@ export function createWorld(args?: Partial): LocalWorld { // events-storage.ts `claimHookResume`), so resumeHook()'s parallel fast // path converges on one event in dev exactly as it does on Vercel. hookResumeDedup: true, + // New runs get dense per-run slot event ids. Runs created before this + // keep their ULIDs; the scheme is pinned by a run's own first event id, + // not by this flag, which only says what new runs get. + slotEventIds: true, }, ...queue, ...storage, diff --git a/packages/world-local/src/storage/events-storage.ts b/packages/world-local/src/storage/events-storage.ts index c6d29d1121..fae85024e8 100644 --- a/packages/world-local/src/storage/events-storage.ts +++ b/packages/world-local/src/storage/events-storage.ts @@ -13,12 +13,14 @@ import { import type { AnyEventRequest, CreateEventParams, + CreateEventRequest, Event, EventResult, Hook, HookCreatedEventRequest, PaginatedResponse, PaginationOptions, + ResolveData, SerializedData, Step, Storage, @@ -28,12 +30,15 @@ import type { import { applyAttributeChanges, EventSchema, + eventIdToSlot, + FIRST_EVENT_SLOT, getMaxEventsPerRun, HookSchema, isChildEntityCreationEvent, isHookEventRequiringExistence, isHookLifecycleEventType, isLegacySpecVersion, + isSlotEventId, isStepEventType, isTerminalRunEventType, isTerminalStepStatus, @@ -41,6 +46,7 @@ import { requiresNewerWorld, SPEC_VERSION_CURRENT, StepSchema, + slotToEventId, ulidToDate, validateAttributeChanges, validateUlidTimestamp, @@ -60,6 +66,8 @@ import { readJSON, readJSONWithFallback, resolveWithinBase, + SORT_KEY_CURSOR_PREFIX, + stripTag, taggedPath, write, writeExclusive, @@ -82,6 +90,7 @@ import { reapPendingHookEvents, releaseHookTokenClaimIfOwnedBy, runTerminalMarkerPath, + scanRunEventIds, withHookTokenClaimLock, } from './helpers.js'; import { @@ -166,6 +175,67 @@ const HookResumeClaimSchema = z.object({ eventId: z.string(), payloadDigest: z.string().optional(), }); + +/** + * Whether `event` is the `hook_received` a resume claim stands for. + * + * The claim names the id its writer INTENDED to publish at, drawn from that + * writer's slot allocator before the append. Under slot ids that intent is not + * a reservation: the allocator is per storage instance, so an instance sharing + * the directory can publish an unrelated event at the same position first, and + * the resume then lands somewhere else. An event read back at the claimed id + * therefore has to be identified, not assumed — returning whatever occupies the + * position reports a `run_started` as the resume's own event and silently drops + * the payload. + */ +function isResumeEvent( + event: Event, + claim: z.infer +): boolean { + return ( + event.eventType === 'hook_received' && + event.correlationId === claim.hookId && + // `resumeId` is persisted on every hook_received written through the + // resume path; an event without one predates that and can only be matched + // by position. + (event.resumeId === undefined || event.resumeId === claim.resumeId) + ); +} + +/** + * Finds the event a resume already committed, by the `resumeId` persisted on + * the event itself rather than by the position the claim guessed. + * + * This is the authority the claim's `eventId` only approximates. Reached when + * the claimed position holds nothing (a crash between claim and append) or + * holds an unrelated event (a cross-instance slot collision), so it pays its + * O(run's events) reads on rare paths only. + */ +async function findCommittedResumeEvent( + basedir: string, + runId: string, + claim: z.infer, + tag?: string +): Promise { + const scan = await scanRunEventIds(basedir, runId, tag); + for (const eventId of scan.ids) { + const event = await readJSONWithFallback( + basedir, + 'events', + `${runId}-${eventId}`, + EventSchema, + tag + ); + if ( + event && + event.resumeId === claim.resumeId && + isResumeEvent(event, claim) + ) { + return event; + } + } + return null; +} /** * Whether a token claim held by another `(runId, hookId)` can never become * live again and may therefore be released by a new claimant: @@ -239,6 +309,37 @@ async function readHookRecoveryMarker( * already exists at that exact path — which is the correct * "already-published" semantic. */ +/** + * Log order for a slot-numbered run is slot order, not `createdAt` order. A + * writer stamps `createdAt` when it enters `create()` but only claims its slot + * at publish time, so a writer that loses a slot race and bumps ends up with a + * higher slot and an older timestamp than the writer that beat it. Slot order + * is the one both writers agree on, and it is what makes the log dense and + * position-addressable, so it wins. + * + * Returns `null` for a ULID-numbered run, which falls back to + * `(createdAt, eventId)` — the two never mix within one run. + */ +function eventSortKey(event: Event): string | null { + return isSlotEventId(event.eventId) ? event.eventId : null; +} + +/** + * The same key as {@link eventSortKey}, recovered from an event file's name. + * + * Event files are named `${runId}-${eventId}` plus an optional tag suffix, so + * a run-scoped listing can read the slot without opening the file. That lets a + * sort-key cursor discard the pages it has already returned on the filename + * alone; without it every page of a long log loads and parses every event file + * for the run. + * + * Returns `null` for a ULID-numbered event, which has no slot to compare. + */ +function eventSortKeyFromFileId(runId: string, fileId: string): string | null { + const eventId = stripTag(fileId).slice(runId.length + 1); + return isSlotEventId(eventId) ? eventId : null; +} + async function findExistingHookCreatedEventId( basedir: string, runId: string, @@ -506,6 +607,159 @@ export function createEventsStorage( cachedEventBytes.clear(); cachedPathsByRunId.clear(); totalCachedEventBytes = 0; + runSlotState.clear(); + } + + // ------------------------------------------------------------------ + // Slot allocation + // ------------------------------------------------------------------ + // + // Event ids are per-run positions (`evnt_` + a 26-char zero-padded + // decimal), dense and 1-based, so the count of a run's events and the + // highest id are the same number. That equivalence is what lets a writer + // state its position with a single integer, and it only holds if the World + // never leaves a hole: a slot is claimed by the publish that occupies it, + // never reserved ahead of a write that might still be rejected. + // + // Runs created before slot ids keep their ULIDs for life. A log may not mix + // the two schemes (`events.list` sorts on the id, and they do not + // interleave), so the ids already on disk are the authoritative pin — no + // spec-version negotiation is involved. `null` state below means "this run + // is ULID-numbered". + // + // The map holds the highest slot this instance has seen PUBLISHED for a + // run, never a reservation. A draw reports `published + 1` and leaves the + // entry alone, so a write rejected anywhere between its draw and its + // publish costs nothing: the next writer draws the same position. That is + // what keeps the log dense, and it is not a rare path — a duplicate + // `step_started` from a concurrent replay is rejected on every storm. + // + // The cost is that two in-flight writers hold the same candidate. The + // exclusive publish arbitrates and the loser bumps, which is the same + // mechanism two instances sharing one directory (a test-only configuration + // this backend supports) already rely on. + const runSlotState = new Map(); + + function slotStateKey(runId: string): string { + return tag ? `${runId}.${tag}` : runId; + } + + /** Whether a slot is occupied by a reader-visible event file. */ + async function slotOccupied(runId: string, slot: number): Promise { + const fileId = `${runId}-${slotToEventId(slot)}`; + for (const candidate of tag + ? [ + taggedPath(basedir, 'events', fileId, tag), + taggedPath(basedir, 'events', fileId), + ] + : [taggedPath(basedir, 'events', fileId)]) { + try { + await fs.stat(candidate); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; + } + } + } + return false; + } + + /** + * Draws the candidate slot for `runId`, or null when the run is + * ULID-numbered and should keep minting ULIDs. + * + * `atLeast` re-floors the candidate after a lost publish. The directory scan + * runs once per run per instance (and again on a `rescan`), not once per + * write. + * + * The watermark alone is a lower bound: it only counts publishes this + * instance made or last scanned for, so another instance sharing the + * directory can be ahead of it. The candidate is therefore probed upward + * until it lands on a free position — one `stat` that returns ENOENT in the + * uncontended case. Skipping the probe would be tolerable for an ordinary + * write (the exclusive publish bumps it), but not for the writes that + * record their candidate in a durable claim for other writers to converge + * on: a claim naming an occupied slot never converges. + */ + async function drawEventSlot( + runId: string, + opts?: { rescan?: boolean; atLeast?: number } + ): Promise { + const key = slotStateKey(runId); + let state = runSlotState.get(key); + if (state === undefined || opts?.rescan) { + const scan = await scanRunEventIds(basedir, runId, tag); + if (state === undefined) { + // A run with no events yet is brand new: it starts on slots. A run + // whose visible events are ULIDs stays on ULIDs for life. + state = + scan.count > 0 && !scan.usesSlots + ? null + : { published: scan.maxSlot }; + runSlotState.set(key, state); + } else if (state !== null) { + state.published = Math.max(state.published, scan.maxSlot); + } + } + if (state === null) { + return null; + } + let slot = Math.max( + state.published + 1, + FIRST_EVENT_SLOT, + opts?.atLeast ?? FIRST_EVENT_SLOT + ); + while (await slotOccupied(runId, slot)) { + state.published = Math.max(state.published, slot); + slot += 1; + } + return slot; + } + + /** + * Records that `eventId` is now on disk, so the next draw starts above it. + * + * Only a committed publish moves the watermark. A draw that never published + * leaves no trace, which is the whole reason the log has no holes. + * + * A no-op for ULID-numbered runs, where ids are not positions, and for runs + * this instance has never drawn for — the first draw scans the directory. + */ + function notePublishedSlot(runId: string, eventId: string): void { + const slot = eventIdToSlot(eventId); + if (slot === null) { + return; + } + const state = runSlotState.get(slotStateKey(runId)); + if (state) { + state.published = Math.max(state.published, slot); + } + } + + /** Mints the next event id for `runId` under whichever scheme it uses. */ + async function mintEventId(runId: string): Promise { + const slot = await drawEventSlot(runId); + return slot === null ? `evnt_${monotonicUlid()}` : slotToEventId(slot); + } + + /** + * Mints the key a terminal transition appends under, re-derived at its + * linearization point (after the marker + reap) so it sorts after any + * `hook_received` that legitimately won the promote arbitration. + * + * For slot runs the rescan is the whole mechanism: it floors the candidate + * past anything another instance promoted while this invocation was + * stalled, and the drawn slot dominates by construction. + */ + async function mintDominantEventKey( + runId: string + ): Promise<{ eventId: string; createdAt: Date }> { + const slot = await drawEventSlot(runId, { rescan: true }); + if (slot !== null) { + return { eventId: slotToEventId(slot), createdAt: new Date() }; + } + return mintRunDominantEventKey(basedir, runId, tag); } function cacheEvent( @@ -566,16 +820,47 @@ export function createEventsStorage( } } - async function storeEvent(event: Event): Promise { - const eventPath = taggedPath( - basedir, - 'events', - `${event.runId}-${event.eventId}`, - tag - ); - const serializedEvent = JSON.stringify(event, jsonReplacer, 2); - await write(eventPath, serializedEvent); - rememberStoredEvent(event, eventPath, serializedEvent); + /** + * Publishes a synthetic event (one this call writes in addition to the + * event it was asked for) and returns it under the id it actually landed + * on. + * + * A slot is a position, so the candidate this event was drawn at may have + * been taken by a concurrent writer between the draw and here; the + * exclusive create detects that and the next position is tried. ULID ids + * are globally unique, so a collision there can only be a retry of this + * same event and the write stays an overwrite. + */ + async function storeEvent(event: Event): Promise { + let current = event; + for (let attempt = 0; ; attempt++) { + const eventPath = taggedPath( + basedir, + 'events', + `${current.runId}-${current.eventId}`, + tag + ); + const serializedEvent = JSON.stringify(current, jsonReplacer, 2); + const slot = eventIdToSlot(current.eventId); + if (slot === null) { + await write(eventPath, serializedEvent); + rememberStoredEvent(current, eventPath, serializedEvent); + return current; + } + if (await writeExclusive(eventPath, serializedEvent)) { + notePublishedSlot(current.runId, current.eventId); + rememberStoredEvent(current, eventPath, serializedEvent); + return current; + } + const next = await drawEventSlot(current.runId, { + rescan: attempt > 0 && attempt % 8 === 0, + atLeast: slot + 1, + }); + // `next` is null only for a ULID-numbered run, which the branch above + // already returned for. + assert(next !== null); + current = { ...current, eventId: slotToEventId(next) }; + } } const queryRunEvents = (runId: string, pagination: PaginationOptions) => @@ -589,6 +874,8 @@ export function createEventsStorage( cursor: pagination.cursor, getCreatedAt: getObjectCreatedAt('evnt'), getId: (event) => event.eventId, + getSortKey: eventSortKey, + getSortKeyFromFileId: (fileId) => eventSortKeyFromFileId(runId, fileId), }); // Per-instance in-process mutexes. Two storage instances sharing @@ -610,7 +897,7 @@ export function createEventsStorage( const stepLocks = new Map>(); const hookLocks = new Map>(); - return { + const storage: LocalEventsStorage = { clearCache, async create( runId: string | null, @@ -689,12 +976,25 @@ export function createEventsStorage( return createImpl(); async function createImpl(): Promise { - // Most paths use the freshly-generated candidate eventId. The + // Most paths use the freshly-drawn candidate eventId. The // hook_created dedup-recovery path below may reassign it to // the canonical eventId persisted in the durable token claim // so concurrent / cross-process workers converge on a single - // event in the log. - let eventId = `evnt_${monotonicUlid()}`; + // event in the log; `eventIdPinned` records that, because a pinned + // id must never be bumped past a slot collision (bumping would + // defeat the convergence and duplicate the event). + // + // Drawn below rather than here: slots are positions, so they must be + // drawn in publish order. The resilient-start path writes a synthetic + // `run_created` that has to precede this event in the log, and a slot + // drawn at function entry would sort after it. + let eventId = ''; + let eventIdPinned = false; + // The eventId currently recorded in this resume's `(runId, resumeId)` + // claim, when one was written or read below. An unpinned publish is + // free to land somewhere else, and the claim is the fast path other + // writers read first, so it is corrected once the append commits. + let resumeClaimRecordedId: string | null = null; const now = new Date(); // For run_created events, use client-provided runId or generate one server-side @@ -803,7 +1103,9 @@ export function createEventsStorage( if (created) { // We created the run — also write the run_created event. - const runCreatedEventId = `evnt_${monotonicUlid()}`; + // Drawn before this invocation's own id so it takes the + // earlier slot: it must replay first. + const runCreatedEventId = await mintEventId(effectiveRunId); const runCreatedEvent: Event = { eventType: 'run_created', runId: effectiveRunId, @@ -838,6 +1140,18 @@ export function createEventsStorage( } } + // Draw this event's id now that any synthetic `run_created` above has + // taken the earlier slot. A slot draw is a candidate, not a + // reservation: the many rejections below (a duplicate step_started + // from a concurrent replay, a terminal run, a step already in a + // terminal state) return without publishing, and the position stays + // available to the next writer. + eventId = await mintEventId(effectiveRunId); + + // These events are rejected on a non-existent run to match the + // postgres and vercel worlds, which both surface this as a + // WorkflowRunNotFoundError rather than silently persisting an + // event for a run that was never created. if ( !currentRun && (data.eventType === 'run_failed' || @@ -895,18 +1209,17 @@ export function createEventsStorage( currentRun.status === 'cancelled' ) { // Return existing state (idempotent) - const event: Event = { + const stored = await storeEvent({ ...data, runId: effectiveRunId, eventId, createdAt: now, specVersion: effectiveSpecVersion, - }; - await storeEvent(event); + }); const resolveData = params?.resolveData ?? DEFAULT_RESOLVE_DATA_OPTION; return { - event: stripEventDataRefs(event, resolveData), + event: stripEventDataRefs(stored, resolveData), run: currentRun, ...(currentRun ? { maxEvents: getMaxEventsPerRun() } : {}), }; @@ -1033,13 +1346,22 @@ export function createEventsStorage( !committedClaim.payloadDigest || committedClaim.payloadDigest === params.resumePayloadDigest) ) { - const committedEvent = await readJSONWithFallback( + const atClaimedId = await readJSONWithFallback( basedir, 'events', `${effectiveRunId}-${committedClaim.eventId}`, EventSchema, tag ); + const committedEvent = + atClaimedId && isResumeEvent(atClaimedId, committedClaim) + ? atClaimedId + : await findCommittedResumeEvent( + basedir, + effectiveRunId, + committedClaim, + tag + ); if (committedEvent) { return { event: committedEvent }; } @@ -1115,20 +1437,42 @@ export function createEventsStorage( `hook_received resumeId "${params.resumeId}" already recorded with a different payload` ); } - const existing = await readJSONWithFallback( + const atClaimedId = await readJSONWithFallback( basedir, 'events', `${effectiveRunId}-${claim.eventId}`, EventSchema, tag ); - if (existing) { - return { event: existing }; + if (atClaimedId && isResumeEvent(atClaimedId, claim)) { + return { event: atClaimedId }; + } + // Either nothing is at the claimed position, or something that + // is not this resume is. The claim's `eventId` is only where its + // writer meant to append, so before concluding the resume is + // uncommitted, look for it by the `resumeId` persisted on the + // event. + const committed = await findCommittedResumeEvent( + basedir, + effectiveRunId, + claim, + tag + ); + if (committed) { + return { event: committed }; + } + // The resume really is uncommitted: a crash between the claim + // write and the append. Take over the append. Adopt the claimed + // position when it is still free — under ULIDs it always is, and + // adopting keeps two takers writing the same path so one loses + // the exclusive create instead of publishing a second event. + // When an unrelated event holds it, there is nothing to converge + // on: keep this writer's own id and let the publish bump. + resumeClaimRecordedId = claim.eventId; + if (!atClaimedId) { + eventId = claim.eventId; + eventIdPinned = true; } - // Claim exists but its event is not yet visible (a crash between - // the claim write and the append). Adopt the pinned eventId and - // fall through to (re)write the event idempotently at that path. - eventId = claim.eventId; return null; }; @@ -1142,9 +1486,23 @@ export function createEventsStorage( return converged; } } else { - // Reserve the claim (pinning this candidate eventId) before the + // Reserve the claim (naming this candidate eventId) before the // append. If a concurrent/cross-process writer reserved it first, - // converge on their pinned event instead. + // converge on their event instead. + // + // Under ULIDs the candidate is pinned: the id is globally unique, + // so the only writer that can collide with it is the other writer + // of this same resume, and both must land on the one event. + // + // Under slot ids it cannot be. A slot is a position, not a name: + // another instance's allocator hands out the same number for a + // different event, and refusing to bump would fail this resume's + // append outright. So the claimed id is a hint, the publish is + // free to move, and `converge` identifies the resume's event by + // its persisted `resumeId`. The claim is rewritten with the id + // actually published once the append commits. + eventIdPinned = !isSlotEventId(eventId); + resumeClaimRecordedId = eventId; const won = await writeExclusive( claimPath, JSON.stringify({ @@ -1158,6 +1516,9 @@ export function createEventsStorage( } satisfies z.infer) ); if (!won) { + // Someone else's claim is the durable one now; this writer's + // candidate is not what the claim records. + resumeClaimRecordedId = null; const winner = await readJSON(claimPath, HookResumeClaimSchema); if (winner) { const converged = await converge(winner); @@ -1271,11 +1632,7 @@ export function createEventsStorage( // strictly dominates all visible events of the run guarantees the // terminal event replays last. See mintRunDominantEventKey for // the dominance argument. - const dominantKey = await mintRunDominantEventKey( - basedir, - effectiveRunId, - tag - ); + const dominantKey = await mintDominantEventKey(effectiveRunId); eventId = dominantKey.eventId; event = { ...event, eventId, createdAt: dominantKey.createdAt }; } @@ -1662,13 +2019,17 @@ export function createEventsStorage( ); // Write the synthetic step_created event so replay observes it // (the client step consumer sets hasCreatedEvent only on a - // step_created event). Its eventId is a fresh monotonic ULID. - // Ordering vs. the step_started event row does not affect - // correctness: the step_started consumer is a no-op and only - // step_created flips hasCreatedEvent, so the end state is the - // same whichever sorts first — this matches the resilient - // run_started → run_created precedent in this file. - const stepCreatedEventId = `evnt_${monotonicUlid()}`; + // step_created event). Its id comes from the run's own + // allocator: minting a ULID here would put a second identity + // scheme in a slot-numbered log, and `events.list` cannot + // paginate a mixed log (a ULID id has no sort key, so it lands + // on every page and the cursor eventually repeats). + // + // This publishes into the position the step_started event is + // still only a candidate for, so the synthetic step_created + // sorts ahead of the step_started that triggered it and the + // step_started bumps up one. + const stepCreatedEventId = await mintEventId(effectiveRunId); const stepCreatedEvent: Event = { eventType: 'step_created', runId: effectiveRunId, @@ -1681,15 +2042,7 @@ export function createEventsStorage( input: lazyData.input, }, }; - await writeJSON( - taggedPath( - basedir, - 'events', - `${effectiveRunId}-${stepCreatedEventId}`, - tag - ), - stepCreatedEvent - ); + await storeEvent(stepCreatedEvent); validatedStep = createdStep; stepCreatedLazily = true; } @@ -1983,8 +2336,12 @@ export function createEventsStorage( canonicalEventId = pinned; } - // The canonical ULID also makes converging writes byte-identical. + // Pinned: this id is the convergence point for every writer of + // this hook, so it must not be bumped past a slot collision. A + // collision here means the canonical event is already published, + // which is exactly the duplicate the handler below repairs from. eventId = canonicalEventId; + eventIdPinned = true; const canonicalCreatedAt = ulidToDate(eventId.replace(/^evnt_/, '')) ?? now; event = { @@ -2017,11 +2374,11 @@ export function createEventsStorage( specVersion: effectiveSpecVersion, }; - await storeEvent(conflictEvent); + const storedConflict = await storeEvent(conflictEvent); const resolveData = params?.resolveData ?? DEFAULT_RESOLVE_DATA_OPTION; return { - event: stripEventDataRefs(conflictEvent, resolveData), + event: stripEventDataRefs(storedConflict, resolveData), run, step, hook: undefined, @@ -2253,12 +2610,67 @@ export function createEventsStorage( throw new HookNotFoundError(data.correlationId); } - const compositeKey = `${effectiveRunId}-${eventId}`; - const eventPath = taggedPath(basedir, 'events', compositeKey, tag); + let eventPath = taggedPath( + basedir, + 'events', + `${effectiveRunId}-${eventId}`, + tag + ); // Capture the serialized payload before the write's `await` so the // cached snapshot can't observe a later mutation (see // rememberStoredEvent). - const serializedEvent = JSON.stringify(event, jsonReplacer, 2); + let serializedEvent = JSON.stringify(event, jsonReplacer, 2); + + /** + * Moves this event to the next free slot after a lost publish, and + * reports whether it could. + * + * A slot id is a position in the run's log, not a globally unique + * token, so losing the publish means another writer took the + * position — an ordinary concurrent write. The World's contract is to + * bump and commit rather than reject: `create` must not fail for a + * reason its caller could not have avoided. Bumping is refused for: + * + * - ULID-numbered runs, where ids ARE globally unique and a collision + * really is a duplicate publish that must surface; + * - ids pinned by a durable claim (`hook_created`'s canonical id, + * `hook_received`'s resume claim), which exist precisely so two + * writers converge on ONE event — bumping would publish a second. + * + * A pinned id is only ever read back from a claim its own writer + * recorded before publishing, and that writer bumps only when the + * position it recorded is already occupied. So an adopter that finds + * the claim stale finds the slot taken too: it collides, and the + * collision is the benign-duplicate path this dedup already has. It + * never publishes a second `hook_created` into a free slot. + */ + const bumpEventSlot = async (attempt: number): Promise => { + const current = eventIdToSlot(eventId); + if (eventIdPinned || current === null) { + return false; + } + // Every failure moves this write up by at least one position, so + // this terminates even under heavy contention. Rescan periodically + // so a batch committed by another instance is skipped in one step + // rather than one slot at a time. + const slot = await drawEventSlot(effectiveRunId, { + rescan: attempt > 0 && attempt % 8 === 0, + atLeast: current + 1, + }); + if (slot === null) { + return false; + } + eventId = slotToEventId(slot); + event = { ...event, eventId }; + eventPath = taggedPath( + basedir, + 'events', + `${effectiveRunId}-${eventId}`, + tag + ); + serializedEvent = JSON.stringify(event, jsonReplacer, 2); + return true; + }; // Cross-process terminal-run guard for `hook_received`. A terminal // transition (run_completed / run_failed / run_cancelled) in ANY @@ -2292,73 +2704,90 @@ export function createEventsStorage( // reap has passed necessarily stages after the marker was // committed, so step 3 rejects it. Rejections before step 4 unlink // a file no reader can see. - let eventPublished: boolean; - if (data.eventType === 'hook_received') { - // Step 1: fast path. The marker is the authoritative durable - // signal; the run-state read additionally rejects runs whose - // terminal state was written without a marker (e.g. runs that - // terminated on an older storage version). - const terminalByMarker = await isRunTerminalCommitted( - basedir, - effectiveRunId, - tag - ); - const runNow = terminalByMarker - ? null - : await readJSONWithFallback( - basedir, - 'runs', - effectiveRunId, - WorkflowRunSchema, - tag - ); - if ( - terminalByMarker || - (runNow && isTerminalWorkflowRunStatus(runNow.status)) - ) { - throw new RunExpiredError( - `Workflow run "${effectiveRunId}" is already in a terminal state` - ); - } - - const stagedPath = pendingHookEventPath( - basedir, - effectiveRunId, - eventId, - tag - ); - const staged = await writeExclusive(stagedPath, serializedEvent); - if (!staged) { - // eventId is a freshly generated ULID; its staging path can - // only be occupied by a previous crashed attempt of this very - // event, which never promoted. Surface the same conflict shape - // as a visible-path collision. - throw new EntityConflictError( - `Event "${eventId}" already exists for run "${effectiveRunId}"` + let eventPublished = false; + for (let attempt = 0; ; attempt++) { + if (data.eventType === 'hook_received') { + // Step 1: fast path. The marker is the authoritative durable + // signal; the run-state read additionally rejects runs whose + // terminal state was written without a marker (e.g. runs that + // terminated on an older storage version). + const terminalByMarker = await isRunTerminalCommitted( + basedir, + effectiveRunId, + tag ); - } - try { - if (await isRunTerminalCommitted(basedir, effectiveRunId, tag)) { + const runNow = terminalByMarker + ? null + : await readJSONWithFallback( + basedir, + 'runs', + effectiveRunId, + WorkflowRunSchema, + tag + ); + if ( + terminalByMarker || + (runNow && isTerminalWorkflowRunStatus(runNow.status)) + ) { throw new RunExpiredError( `Workflow run "${effectiveRunId}" is already in a terminal state` ); } - const promoted = await promoteExclusive(stagedPath, eventPath); - if (promoted === 'missing') { - // A terminal transition reaped the staged file between the - // check and the link — the atomic loss of the arbitration. - throw new RunExpiredError( - `Workflow run "${effectiveRunId}" is already in a terminal state` + + const stagedPath = pendingHookEventPath( + basedir, + effectiveRunId, + eventId, + tag + ); + const staged = await writeExclusive(stagedPath, serializedEvent); + if (!staged) { + // The staging path can be occupied by a previous crashed + // attempt of this very event (which never promoted), or, under + // slot ids, by a concurrent writer holding the same position. + // Both are handled the same way: fall through to the bump + // below, which moves off the position when it can and surfaces + // the conflict when it cannot. + if (await bumpEventSlot(attempt)) { + continue; + } + throw new EntityConflictError( + `Event "${eventId}" already exists for run "${effectiveRunId}"` ); } - eventPublished = promoted === 'linked'; - } finally { - // The staged path is not reader-visible; removing it is pure - // cleanup on every outcome (already gone when reaped). - await deleteJSON(stagedPath).catch(() => {}); + try { + if (await isRunTerminalCommitted(basedir, effectiveRunId, tag)) { + throw new RunExpiredError( + `Workflow run "${effectiveRunId}" is already in a terminal state` + ); + } + const promoted = await promoteExclusive(stagedPath, eventPath); + if (promoted === 'missing') { + // A terminal transition reaped the staged file between the + // check and the link — the atomic loss of the arbitration. + throw new RunExpiredError( + `Workflow run "${effectiveRunId}" is already in a terminal state` + ); + } + eventPublished = promoted === 'linked'; + } finally { + // The staged path is not reader-visible; removing it is pure + // cleanup on every outcome (already gone when reaped). + await deleteJSON(stagedPath).catch(() => {}); + } + } else { + eventPublished = await writeExclusive(eventPath, serializedEvent); + } + + if (eventPublished) { + // The position is occupied now, so the next draw for this run + // starts above it. Only a committed publish moves the watermark. + notePublishedSlot(effectiveRunId, eventId); + break; + } + if (!(await bumpEventSlot(attempt))) { + break; } - } else { - eventPublished = await writeExclusive(eventPath, serializedEvent); } if (!eventPublished) { @@ -2391,6 +2820,34 @@ export function createEventsStorage( // replay can serve it without rereading from disk. rememberStoredEvent(event, eventPath, serializedEvent); + // Point the resume claim at where the event actually landed. An + // unpinned publish bumps past occupied slots, so the id the claim + // recorded before the append can be stale; leaving it stale would + // send every later reader of this resume down the `resumeId` scan + // instead of the single read the claim exists to provide. Plain + // overwrite, not exclusive-create: the claim is already this + // writer's, and only the eventId changes. + if ( + data.eventType === 'hook_received' && + params?.resumeId && + resumeClaimRecordedId !== null && + resumeClaimRecordedId !== eventId + ) { + await write( + hookResumeClaimPath(basedir, effectiveRunId, params.resumeId), + JSON.stringify({ + runId: effectiveRunId, + resumeId: params.resumeId, + hookId: data.correlationId, + eventId, + ...(params.resumePayloadDigest + ? { payloadDigest: params.resumePayloadDigest } + : {}), + } satisfies z.infer), + { overwrite: true } + ); + } + // Write the hook entity ONLY now that the event publish has // committed. Doing this earlier (in the `hook_created` // branch above) would mutate an already-committed hook @@ -2551,6 +3008,9 @@ export function createEventsStorage( cursor: params.pagination?.cursor, getCreatedAt: getObjectCreatedAt('evnt'), getId: (event) => event.eventId, + getSortKey: eventSortKey, + getSortKeyFromFileId: (fileId) => + eventSortKeyFromFileId(params.runId, fileId), }); // If resolveData is "none", remove eventData from events @@ -2566,4 +3026,88 @@ export function createEventsStorage( return result; }, }; + + /** + * The report half of bump-and-report: the events sitting on the slots + * between the one the writer asked for and the one its write landed on. + * + * Wrapped around `create` rather than folded into it because `create` has a + * dozen commit points (dedup recovery, hook conflict, lazy step creation) + * and the report is the same at every one of them: read the committed id, + * read back what is below it. + * + * The read is a directory scan, so it only runs when the write actually + * skipped a slot. `hasMore` says the set is a lower bound: another instance + * may hold a lower slot it has not published yet, and a draw whose publish + * was lost leaves one permanently empty. + */ + async function reportSkippedSlots( + result: EventResult, + askedFor: number, + resolveData: ResolveData + ): Promise { + if (!result.event) { + return result; + } + const committedSlot = eventIdToSlot(result.event.eventId); + if ( + committedSlot === null || + askedFor < FIRST_EVENT_SLOT || + committedSlot <= askedFor + 1 + ) { + return result; + } + const span = committedSlot - askedFor - 1; + const page = await storage.list({ + runId: result.event.runId, + pagination: { + cursor: `${SORT_KEY_CURSOR_PREFIX}${slotToEventId(askedFor)}`, + limit: span, + sortOrder: 'asc', + }, + resolveData, + }); + // The cursor is exclusive and the page is in slot order, so a dense log + // yields exactly the skipped slots. A hole lets the page reach past the + // committed slot, which is this writer's own event and anything a later + // writer already published: neither is something it skipped over. + const committedEventId = result.event.eventId; + const events = page.data.filter( + (event) => event.eventId < committedEventId + ); + return { + ...result, + events, + // Deliberately no cursor: the report is a lower bound on what this write + // skipped over, not a page the caller has now read to the end of, so it + // must not advance the caller's read position. + cursor: null, + hasMore: events.length < committedSlot - askedFor - 1, + }; + } + + const create = (async ( + runId: string, + data: CreateEventRequest, + params?: CreateEventParams + ): Promise => { + if (params?.eventCount === undefined) { + return storage.create(runId, data, params); + } + const resolveData = params.resolveData ?? DEFAULT_RESOLVE_DATA_OPTION; + const result = await storage.create(runId, data, params); + // `sinceCursor` and the skipped-slot report share `events`/`cursor`/ + // `hasMore`, and the runtime sends both on the same write. The delta wins: + // the skipped slots all sit above the cursor, so it is a strict superset, + // and it is the only one of the two that advances `cursor`. Narrowing + // `events` to the report while leaving the delta's cursor would tell the + // caller it has read a range it was only handed part of, and the rest + // would never be fetched again. + if (typeof params.sinceCursor === 'string') { + return result; + } + return reportSkippedSlots(result, params.eventCount, resolveData); + }) as LocalEventsStorage['create']; + + return { ...storage, create }; } diff --git a/packages/world-local/src/storage/helpers.ts b/packages/world-local/src/storage/helpers.ts index 605e8b3370..9d599dc670 100644 --- a/packages/world-local/src/storage/helpers.ts +++ b/packages/world-local/src/storage/helpers.ts @@ -2,6 +2,7 @@ import { createHash } from 'node:crypto'; import fs from 'node:fs/promises'; import path from 'node:path'; import { WorkflowWorldError } from '@workflow/errors'; +import { eventIdToSlot } from '@workflow/world'; import { lock } from 'proper-lockfile'; import { decodeTime, monotonicFactory } from 'ulid'; import { z } from 'zod'; @@ -234,27 +235,86 @@ export async function reapPendingHookEvents( * >= every visible event's `createdAt`, which was stamped at that event's * `createImpl()` entry — before its publish, and thus before this call. * Equal-`createdAt` ties fall to the strictly-dominant eventId. + * + * ULID-numbered runs only. A slot-numbered run needs no temporal argument: + * the next slot dominates every allocated one by construction, so its + * terminal transition just draws from the run's slot allocator after the + * reap. Callers pick the branch (see `mintDominantEventKey` in + * events-storage.ts). */ export async function mintRunDominantEventKey( basedir: string, runId: string, tag?: string ): Promise<{ eventId: string; createdAt: Date }> { + const scan = await scanRunEventIds(basedir, runId, tag); + + let ts = Date.now(); + if (scan.maxId) { + try { + const maxTs = decodeTime(scan.maxId.replace(/^evnt_/, '')); + if (ts <= maxTs) { + ts = maxTs + 1; + } + } catch { + // Malformed eventId in the log — fall back to the wall clock. + } + } + return { eventId: `evnt_${monotonicUlid(ts)}`, createdAt: new Date(ts) }; +} + +/** + * What a run's already-published event ids say about its identity scheme. + * + * A run keeps the scheme it was created under for its whole life (a log may + * not mix ULID and slot ids — `events.list` sorts on the id, and the two + * schemes do not interleave), so the ids on disk are the authoritative pin. + * `usesSlots` is false for a run with no events yet; the caller decides what a + * brand-new run gets. + */ +export interface RunEventIdScan { + /** Highest reader-visible event id, or null when the run has no events. */ + maxId: string | null; + /** Whether the run's ids are slot-numbered. */ + usesSlots: boolean; + /** Highest allocated slot, or 0 when the run has none. */ + maxSlot: number; + /** Number of reader-visible events found for the run. */ + count: number; + /** Reader-visible event ids, tag stripped, in directory order. */ + ids: string[]; +} + +/** + * Scans the events directory for one run's ids, honoring tag visibility. + * + * O(all event files), like every other directory-walking read in this + * backend. Callers that run it per write memoize the result and use the + * publish itself to detect when the memo has fallen behind. + */ +export async function scanRunEventIds( + basedir: string, + runId: string, + tag?: string +): Promise { let files: string[] = []; try { files = await fs.readdir(path.join(basedir, 'events')); } catch (error) { // Only ENOENT ("no events directory yet") means there is provably - // nothing visible to dominate. Any other failure would silently mint a - // wall-clock key with no dominance guarantee over an already-accepted - // hook — abort the terminal transition instead; its retry re-runs this - // scan. + // nothing visible. Any other failure would silently report an empty run, + // which would mint a colliding slot / a non-dominant ULID — let the + // caller's retry re-run the scan instead. if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { throw error; } } const prefix = `${runId}-`; - let maxUlid: string | null = null; + const ids: string[] = []; + let maxId: string | null = null; + let maxSlot = 0; + let usesSlots = false; + let count = 0; for (const file of files) { if (!file.startsWith(prefix) || !file.endsWith('.json')) { continue; @@ -266,22 +326,18 @@ export async function mintRunDominantEventKey( continue; } const candidate = stripTag(fileId).slice(prefix.length); - if (!maxUlid || candidate > maxUlid) { - maxUlid = candidate; + count += 1; + ids.push(candidate); + if (!maxId || candidate > maxId) { + maxId = candidate; } - } - let ts = Date.now(); - if (maxUlid) { - try { - const maxTs = decodeTime(maxUlid.replace(/^evnt_/, '')); - if (ts <= maxTs) { - ts = maxTs + 1; - } - } catch { - // Malformed eventId in the log — fall back to the wall clock. + const slot = eventIdToSlot(candidate); + if (slot !== null) { + usesSlots = true; + if (slot > maxSlot) maxSlot = slot; } } - return { eventId: `evnt_${monotonicUlid(ts)}`, createdAt: new Date(ts) }; + return { maxId, usesSlots, maxSlot, count, ids }; } /** 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 48f545033b..1159330c8f 100644 --- a/packages/world-local/src/storage/hook-resume-dedup.test.ts +++ b/packages/world-local/src/storage/hook-resume-dedup.test.ts @@ -4,6 +4,7 @@ import path from 'node:path'; import { SPEC_VERSION_CURRENT, type Storage } from '@workflow/world'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { createHook, createRun, disposeHook } from '../test-helpers.js'; +import { hookResumeClaimPath } from './helpers.js'; import { createStorage } from './index.js'; // When a run carries the `hookResumeInputVersion` marker, the parallel resume @@ -148,6 +149,120 @@ describe('world-local hook_received resume dedup', () => { ).rejects.toThrow(); }); + // A slot allocator is per storage instance, and two instances share the + // directory whenever a dev server serves the hook request from one module + // instance and runs the queue in another. The resume claim names the id its + // writer MEANT to append at, drawn before the append, so a second instance + // can publish an unrelated event at that position first. + describe('when another instance takes the position the claim named', () => { + async function seedStaleAllocator(runId: string) { + // `other` scans the log once, then counts forward in memory. Writing + // through `storage` afterwards fills the positions `other` still thinks + // are free. + const other = createStorage(testDir); + const attr = (from: Storage, key: string) => + from.events.create(runId, { + eventType: 'attr_set', + specVersion: SPEC_VERSION_CURRENT, + correlationId: `attr_${key}`, + eventData: { + changes: [{ key, value: 'x' }], + writer: { type: 'workflow' }, + }, + }); + await attr(other, 'seed'); + await attr(storage, 'ahead_1'); + await attr(storage, 'ahead_2'); + return other; + } + + it('still commits the resume, at the position actually free', async () => { + const { runId, hook } = await setup(); + const other = await seedStaleAllocator(runId); + const payload = new Uint8Array([1, 2, 3]); + + const result = await other.events.create( + runId, + { + eventType: 'hook_received', + specVersion: SPEC_VERSION_CURRENT, + correlationId: hook.hookId, + eventData: { token: hook.token, payload }, + }, + { resumeId: 'resume_1', resumePayloadDigest: 'digest_1' } + ); + + // The reported event must be this resume's own. Returning whatever sits + // at the claimed position reports an `attr_set` as the resume's event + // and drops the payload without an error. + expect(result.event.eventType).toBe('hook_received'); + expect(result.event.resumeId).toBe('resume_1'); + expect(await countHookReceived(runId)).toBe(1); + }); + + it('converges the redelivery on the committed event, not on the occupant', async () => { + const { runId, hook } = await setup(); + const other = await seedStaleAllocator(runId); + const payload = new Uint8Array([1, 2, 3]); + + const first = await other.events.create( + runId, + { + eventType: 'hook_received', + specVersion: SPEC_VERSION_CURRENT, + correlationId: hook.hookId, + eventData: { token: hook.token, payload }, + }, + { resumeId: 'resume_1', resumePayloadDigest: 'digest_1' } + ); + + // Redelivery through the OTHER instance: it reads the claim, which named + // a position the resume did not land at. + const second = await resume(runId, hook, 'resume_1', 'digest_1', payload); + + expect(second.event.eventId).toBe(first.event.eventId); + expect(await countHookReceived(runId)).toBe(1); + }); + + it('converges a redelivery whose claim still names the occupied position', async () => { + const { runId, hook } = await setup(); + const other = await seedStaleAllocator(runId); + const payload = new Uint8Array([1, 2, 3]); + + const first = await other.events.create( + runId, + { + eventType: 'hook_received', + specVersion: SPEC_VERSION_CURRENT, + correlationId: hook.hookId, + eventData: { token: hook.token, payload }, + }, + { resumeId: 'resume_1', resumePayloadDigest: 'digest_1' } + ); + + // Roll the claim back to the position it named before the append, as a + // crash between the append and the claim's correction would leave it. + // That position holds an unrelated event, so a reader that trusts the + // claim reports an `attr_set` as this resume's event: no error, no + // second event, and the payload silently gone. + const claimPath = hookResumeClaimPath(testDir, runId, 'resume_1'); + const claim = JSON.parse(await fs.readFile(claimPath, 'utf8')); + const occupant = (await storage.events.list({ runId })).data.find( + (event) => event.eventType === 'attr_set' + ); + await fs.writeFile( + claimPath, + JSON.stringify({ ...claim, eventId: occupant?.eventId }) + ); + + const second = await resume(runId, hook, 'resume_1', 'digest_1', payload); + + expect(second.event.eventType).toBe('hook_received'); + expect(second.event.eventId).toBe(first.event.eventId); + expect(await countHookReceived(runId)).toBe(1); + }); + }); + it('rejects a reused resumeId + digest that belongs to a DIFFERENT hook', async () => { const { runId, hook } = await setup(); const otherHook = await createHook(storage, runId, { diff --git a/packages/world-local/src/storage/slot-identity.test.ts b/packages/world-local/src/storage/slot-identity.test.ts new file mode 100644 index 0000000000..7ef644e724 --- /dev/null +++ b/packages/world-local/src/storage/slot-identity.test.ts @@ -0,0 +1,469 @@ +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { + EVENT_ID_BODY_LENGTH, + EVENT_ID_PREFIX, + eventIdToSlot, + FIRST_EVENT_SLOT, + SPEC_VERSION_CURRENT, +} from '@workflow/world'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { SORT_KEY_CURSOR_PREFIX } from '../fs.js'; +import { createStorage } from '../storage.js'; +import { monotonicUlid } from './helpers.js'; + +let testDir: string; +let storage: ReturnType; + +beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'wl-slot-')); + storage = createStorage(testDir); +}); + +afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); +}); + +const serialized = (value: unknown) => + ({ data: JSON.stringify(value), encoding: 'json' }) as any; + +function slotId(slot: number): string { + return `${EVENT_ID_PREFIX}${String(slot).padStart(EVENT_ID_BODY_LENGTH, '0')}`; +} + +async function startRun(): Promise { + const created = await storage.events.create('', { + eventType: 'run_created', + specVersion: SPEC_VERSION_CURRENT, + eventData: { + deploymentId: 'dpl_slot', + workflowName: 'slotWorkflow', + input: serialized([]), + }, + } as any); + const { runId } = created.event; + await storage.events.create(runId, { + eventType: 'run_started', + specVersion: SPEC_VERSION_CURRENT, + } as any); + return runId; +} + +async function listEventIds(runId: string): Promise { + const result = await storage.events.list({ + runId, + pagination: { limit: 1000 }, + }); + return result.data.map((event) => event.eventId); +} + +function slotsOf(eventIds: string[]): (number | null)[] { + return eventIds.map((eventId) => eventIdToSlot(eventId)); +} + +describe('slot event ids', () => { + it('numbers a new run densely from the first slot, in log order', async () => { + const runId = await startRun(); + for (let i = 0; i < 5; i++) { + await storage.events.create(runId, { + eventType: 'step_created', + correlationId: `step_${i}`, + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: `step${i}`, input: serialized([]) }, + } as any); + } + + const eventIds = await listEventIds(runId); + // run_created, run_started, then one step_created each. `events.list` + // returns chronological order, so the slots must come out sorted and + // gapless starting at the first slot. + expect(eventIds).toEqual( + eventIds.map((_, i) => slotId(FIRST_EVENT_SLOT + i)) + ); + }); + + it('stays dense when writers race for the same slot', async () => { + const runId = await startRun(); + const width = 20; + await Promise.all( + Array.from({ length: width }, (_, i) => + storage.events.create(runId, { + eventType: 'step_created', + correlationId: `step_${i}`, + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: `step${i}`, input: serialized([]) }, + } as any) + ) + ); + + // Every writer starts from the same view of the log, so all but one lose + // the publish and bump. Bump-and-report means none of them fail, and the + // log they produce is still gapless. + const slots = slotsOf(await listEventIds(runId)); + expect(slots).toEqual( + Array.from({ length: width + 2 }, (_, i) => FIRST_EVENT_SLOT + i) + ); + }); + + it('leaves no hole behind writes that are rejected', async () => { + const runId = await startRun(); + const width = 20; + // Every writer claims the same correlation id, so exactly one + // step_created survives the entity-creation dedup and the rest are + // rejected. A slot + // drawn before the publish and never handed back would be burned by each + // rejection, and allocation only moves forward, so every such hole is + // permanent. + const results = await Promise.allSettled( + Array.from({ length: width }, () => + storage.events.create(runId, { + eventType: 'step_created', + correlationId: 'step_contended', + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: 'contended', input: serialized([]) }, + } as any) + ) + ); + expect(results.filter((r) => r.status === 'fulfilled')).toHaveLength(1); + + // The next write is what exposes a burned slot: it lands right behind the + // winner if every rejection gave its slot back, and `width - 1` positions + // past it if none of them did. + await storage.events.create(runId, { + eventType: 'step_created', + correlationId: 'step_after', + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: 'after', input: serialized([]) }, + } as any); + + const slots = slotsOf(await listEventIds(runId)); + // run_created, run_started, the one step_created that won, and the write + // that followed it. + expect(slots).toEqual([ + FIRST_EVENT_SLOT, + FIRST_EVENT_SLOT + 1, + FIRST_EVENT_SLOT + 2, + FIRST_EVENT_SLOT + 3, + ]); + }); + + it('leaves no hole when a rejected write is overtaken by another', async () => { + const runId = await startRun(); + const width = 8; + for (let i = 0; i < width; i++) { + await storage.events.create(runId, { + eventType: 'step_started', + correlationId: `step_${i}`, + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: `step${i}`, input: serialized([]) }, + } as any); + } + + // Each duplicate names a different step, so they take different per-step + // locks and their draws interleave. This is the shape a step storm + // produces: several replays of one run each re-issuing a step_started the + // winner already published. A slot reserved at the draw and handed back + // only when it is still the highest one drawn cannot survive this — by the + // time a rejection lands, the next writer has drawn past it. + const results = await Promise.allSettled( + Array.from({ length: width }, (_, i) => + storage.events.create(runId, { + eventType: 'step_started', + correlationId: `step_${i}`, + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: `step${i}`, input: serialized([]) }, + } as any) + ) + ); + expect(results.filter((r) => r.status === 'fulfilled')).toHaveLength(0); + + await storage.events.create(runId, { + eventType: 'step_created', + correlationId: 'step_after', + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: 'after', input: serialized([]) }, + } as any); + + // run_created, run_started, a step_created + step_started per step, and + // the write that followed the rejections. + const slots = slotsOf(await listEventIds(runId)); + expect(slots).toEqual( + Array.from({ length: width * 2 + 3 }, (_, i) => FIRST_EVENT_SLOT + i) + ); + }); + + it('orders a terminal event after every event it raced', async () => { + const runId = await startRun(); + await storage.events.create(runId, { + eventType: 'step_created', + correlationId: 'step_a', + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: 'a', input: serialized([]) }, + } as any); + await storage.events.create(runId, { + eventType: 'run_completed', + specVersion: SPEC_VERSION_CURRENT, + eventData: { output: serialized('done') }, + } as any); + + const eventIds = await listEventIds(runId); + expect(eventIds.at(-1)).toBe(slotId(eventIds.length)); + }); + + it('numbers a lazily created step_created from the same allocator', async () => { + const runId = await startRun(); + // A step_started carrying the creation payload with no step_created ahead + // of it makes the World synthesize one. That synthetic event is the only + // event the World writes without a caller asking for it by name, so it is + // the one place a second id scheme can leak into a slot-numbered log. + await storage.events.create(runId, { + eventType: 'step_started', + correlationId: 'step_lazy', + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: 'lazy', input: serialized([]) }, + } as any); + + const eventIds = await listEventIds(runId); + expect(slotsOf(eventIds)).toEqual( + Array.from({ length: eventIds.length }, (_, i) => FIRST_EVENT_SLOT + i) + ); + // A ULID id here has no sort key, so `events.list` would return it on + // every page and the cursor would eventually repeat. + const events = await storage.events.list({ + runId, + pagination: { limit: 1000 }, + }); + // The synthetic step_created takes the lower slot: the step_started that + // triggered it holds a candidate, not a reservation, so publishing the + // step_created first pushes the step_started up one. Replay reads them in + // the order they happened. + expect(events.data.map((event) => event.eventType)).toEqual([ + 'run_created', + 'run_started', + 'step_created', + 'step_started', + ]); + }); + + it('paginates a run whose step_created events were created lazily', async () => { + const runId = await startRun(); + for (let i = 0; i < 6; i++) { + await storage.events.create(runId, { + eventType: 'step_started', + correlationId: `step_lazy_${i}`, + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: `lazy${i}`, input: serialized([]) }, + } as any); + } + + // Walk the log the way the runtime does: one page at a time, asserting the + // cursor always advances. A mixed-scheme log stalls here rather than at + // the id assertion above. + const seenCursors = new Set(); + const walked: string[] = []; + let cursor: string | undefined; + for (let page = 0; page < 20; page++) { + const result = await storage.events.list({ + runId, + pagination: { limit: 3, sortOrder: 'asc', cursor }, + }); + walked.push(...result.data.map((event) => event.eventId)); + if (!result.hasMore) break; + expect(result.cursor).toBeTruthy(); + expect(seenCursors.has(result.cursor as string)).toBe(false); + seenCursors.add(result.cursor as string); + cursor = result.cursor as string; + } + + expect(walked).toEqual(await listEventIds(runId)); + expect(slotsOf(walked)).toEqual( + Array.from({ length: walked.length }, (_, i) => FIRST_EVENT_SLOT + i) + ); + }); + + it('keeps a ULID-numbered run on ULIDs', async () => { + const runId = await startRun(); + // Rewrite the run's log the way it would look had it been created before + // slot ids existed. The scheme is pinned by what is on disk, not by a + // stored flag, so this is the whole of the upgrade path. + const eventsDir = path.join(testDir, 'events'); + const files = (await fs.readdir(eventsDir)).filter((file) => + file.startsWith(`${runId}-`) + ); + files.sort(); + for (const file of files) { + const legacyId = `${EVENT_ID_PREFIX}${monotonicUlid()}`; + const raw = await fs.readFile(path.join(eventsDir, file), 'utf8'); + await fs.writeFile( + path.join(eventsDir, `${runId}-${legacyId}.json`), + raw.replace(/"eventId": "evnt_[^"]+"/, `"eventId": "${legacyId}"`) + ); + await fs.rm(path.join(eventsDir, file)); + } + // The allocator memoizes each run's scheme, so drop the cache the way a + // fresh process would see it. + storage.events.clearCache?.(); + + await storage.events.create(runId, { + eventType: 'step_created', + correlationId: 'step_after_upgrade', + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: 'afterUpgrade', input: serialized([]) }, + } as any); + + const eventIds = await listEventIds(runId); + expect(eventIds).toHaveLength(3); + // No slot ids anywhere: one slot id in a ULID log would sort before every + // ULID (its body starts with ten zeros) and replay out of order. + expect(slotsOf(eventIds)).toEqual([null, null, null]); + }); +}); + +describe('skipped-slot report', () => { + /** Writes `count` step_created events, returning the slots they landed on. */ + async function fill(runId: string, count: number): Promise { + const slots: number[] = []; + for (let i = 0; i < count; i++) { + const result = await storage.events.create(runId, { + eventType: 'step_created', + correlationId: `filler_${i}`, + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: `filler${i}`, input: serialized([]) }, + } as any); + slots.push(eventIdToSlot(result.event.eventId) as number); + } + return slots; + } + + it('hands back the events occupying the slots the write skipped', async () => { + const runId = await startRun(); + const stale = FIRST_EVENT_SLOT + 1; // what the run had after run_started + const filled = await fill(runId, 3); + + // A writer whose loaded log stopped at run_started asks for the slot right + // above it and is bumped past everything written since. + const result = await storage.events.create( + runId, + { + eventType: 'wait_created', + correlationId: 'wait_a', + specVersion: SPEC_VERSION_CURRENT, + eventData: { resumeAt: new Date(0).toISOString() }, + } as any, + { eventCount: stale } + ); + + expect(eventIdToSlot(result.event.eventId)).toBe(stale + filled.length + 1); + expect(result.events?.map((event) => event.eventId)).toEqual( + filled.map(slotId) + ); + expect(result.hasMore).toBe(false); + }); + + it('reports nothing when the write lands on the slot it asked for', async () => { + const runId = await startRun(); + const result = await storage.events.create( + runId, + { + eventType: 'wait_created', + correlationId: 'wait_a', + specVersion: SPEC_VERSION_CURRENT, + eventData: { resumeAt: new Date(0).toISOString() }, + } as any, + { eventCount: FIRST_EVENT_SLOT + 1 } + ); + + expect(eventIdToSlot(result.event.eventId)).toBe(FIRST_EVENT_SLOT + 2); + expect(result.events).toBeUndefined(); + }); + + it('reports nothing when the writer sends no count', async () => { + const runId = await startRun(); + await fill(runId, 2); + const result = await storage.events.create(runId, { + eventType: 'wait_created', + correlationId: 'wait_a', + specVersion: SPEC_VERSION_CURRENT, + eventData: { resumeAt: new Date(0).toISOString() }, + } as any); + + expect(result.events).toBeUndefined(); + }); + + it('lets the sinceCursor delta answer when the writer asks for both', async () => { + // `sinceCursor` and `eventCount` both report through + // `events`/`cursor`/`hasMore`, and the runtime sends both on the same + // write. The delta is a strict superset of the skipped span (the skipped + // slots are all above the cursor) and, unlike the report, it advances + // `cursor`. Returning the narrower set alongside the delta's cursor would + // tell the caller it has read up to the delta end while handing it only + // part of that range, and the events in between would never be fetched + // again. + const runId = await startRun(); + const stale = FIRST_EVENT_SLOT + 1; + const filled = await fill(runId, 3); + + const result = await storage.events.create( + runId, + { + eventType: 'wait_created', + correlationId: 'wait_a', + specVersion: SPEC_VERSION_CURRENT, + eventData: { resumeAt: new Date(0).toISOString() }, + } as any, + { + eventCount: stale, + sinceCursor: `${SORT_KEY_CURSOR_PREFIX}${slotId(stale)}`, + } + ); + + const committed = result.event.eventId; + expect(eventIdToSlot(committed)).toBe(stale + filled.length + 1); + // Everything after the cursor, this write's own event included. + expect(result.events?.map((event) => event.eventId)).toEqual([ + ...filled.map(slotId), + committed, + ]); + expect(result.cursor).toBe(`${SORT_KEY_CURSOR_PREFIX}${committed}`); + expect(result.hasMore).toBe(false); + }); + + it('gives every racing writer the events it was decided without', async () => { + const runId = await startRun(); + const stale = FIRST_EVENT_SLOT + 1; + const width = 8; + + // All eight start from the same view, so seven of them are bumped and each + // one's report covers exactly the slots between `stale` and where it + // landed. Under contention the report can be a lower bound: a writer + // holding a lower slot may not have published yet, which `hasMore` says. + const results = await Promise.all( + Array.from({ length: width }, (_, i) => + storage.events.create( + runId, + { + eventType: 'step_created', + correlationId: `racer_${i}`, + specVersion: SPEC_VERSION_CURRENT, + eventData: { stepName: `racer${i}`, input: serialized([]) }, + } as any, + { eventCount: stale } + ) + ) + ); + + for (const result of results) { + const landed = eventIdToSlot(result.event.eventId) as number; + const reported = result.events ?? []; + const span = landed - stale - 1; + expect(reported.length).toBeLessThanOrEqual(span); + expect(result.hasMore ?? false).toBe(reported.length < span); + for (const event of reported) { + const slot = eventIdToSlot(event.eventId) as number; + expect(slot).toBeGreaterThan(stale); + expect(slot).toBeLessThan(landed); + } + } + }); +}); diff --git a/packages/world-postgres/src/drizzle/migrations/0019_add_event_slots.sql b/packages/world-postgres/src/drizzle/migrations/0019_add_event_slots.sql new file mode 100644 index 0000000000..99b7db3a33 --- /dev/null +++ b/packages/world-postgres/src/drizzle/migrations/0019_add_event_slots.sql @@ -0,0 +1,74 @@ +-- Event ids become per-run slot positions (`evnt_` + a zero-padded decimal), +-- so an id is only unique together with its run. Runs created before this keep +-- their globally-unique ULIDs, which the composite key also admits. +-- +-- LOCKING. Replacing a primary key takes ACCESS EXCLUSIVE on +-- `workflow.workflow_events`, which blocks reads as well as writes, and the +-- migrator runs every pending migration in one transaction, so the lock is held +-- until all of them commit. Building the new key's index under that lock is the +-- part that grows with the table. On an empty or modest table this is +-- instantaneous and needs no thought. +-- +-- On a large existing table, build the index first, outside the migrator, and +-- this migration adopts it instead of building its own: +-- +-- CREATE UNIQUE INDEX CONCURRENTLY "workflow_events_run_id_id_idx" +-- ON "workflow"."workflow_events" ("run_id", "id"); +-- +-- `CONCURRENTLY` cannot appear in this file: Postgres rejects it inside a +-- transaction block. Run it by hand, confirm the index came out valid, then +-- migrate. The branch below picks it up, and the exclusive lock then covers +-- only a catalog update rather than a full build. Adopting an index renames it +-- to the constraint's name, so it ends up as `workflow_events_run_id_id_pk` +-- either way and the two paths leave the same schema behind. +-- Bound the wait for that lock. A pending ACCESS EXCLUSIVE queues ahead of +-- every lock request that arrives after it, so waiting on one long-running +-- reader stalls all traffic to the table for as long as the wait lasts. Ten +-- seconds of that is a blip; an unbounded wait is an outage. Failing instead +-- leaves the migration unapplied and retryable. Raise it by hand for a +-- maintenance window. +-- +-- `SET LOCAL` lasts for the transaction, and the migrator runs every pending +-- migration in one, so a migration that follows this one in the same batch +-- inherits the timeout. +SET LOCAL lock_timeout = '10s';--> statement-breakpoint +ALTER TABLE "workflow"."workflow_events" DROP CONSTRAINT "workflow_events_pkey";--> statement-breakpoint +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + JOIN pg_index i ON i.indexrelid = c.oid + WHERE n.nspname = 'workflow' + AND c.relname = 'workflow_events_run_id_id_idx' + AND c.relkind = 'i' + AND i.indisunique + AND i.indisvalid + ) THEN + ALTER TABLE "workflow"."workflow_events" + ADD CONSTRAINT "workflow_events_run_id_id_pk" + PRIMARY KEY USING INDEX "workflow_events_run_id_id_idx"; + ELSE + -- A `CREATE UNIQUE INDEX CONCURRENTLY` that failed leaves an index of this + -- name behind marked invalid. The branch above rejects it, and the key + -- built here is a different index under a different name, so without this + -- the invalid one would survive the migration: never used by a plan, still + -- maintained on every insert. Dropping it also makes a second attempt at + -- the concurrent build possible without a manual cleanup first. + DROP INDEX IF EXISTS "workflow"."workflow_events_run_id_id_idx"; + ALTER TABLE "workflow"."workflow_events" + ADD CONSTRAINT "workflow_events_run_id_id_pk" PRIMARY KEY ("run_id","id"); + END IF; +END $$;--> statement-breakpoint +-- Redundant once the primary key leads with `run_id`: that index serves every +-- by-run lookup and range scan this one did, so keeping it only costs a second +-- write per event on the table's hottest path. +DROP INDEX IF EXISTS "workflow"."workflow_events_run_id_index";--> statement-breakpoint +-- One row per slot-numbered run. Its absence is the "this run predates slots" +-- signal, so no backfill: existing runs stay on ULIDs for the rest of their +-- lives. A marker only: positions are allocated by the insert that occupies +-- them, read from the event log itself. +CREATE TABLE IF NOT EXISTS "workflow"."workflow_event_slots" ( + "run_id" varchar PRIMARY KEY NOT NULL +); diff --git a/packages/world-postgres/src/drizzle/migrations/meta/_journal.json b/packages/world-postgres/src/drizzle/migrations/meta/_journal.json index 4ce969faa2..7df53dba4e 100644 --- a/packages/world-postgres/src/drizzle/migrations/meta/_journal.json +++ b/packages/world-postgres/src/drizzle/migrations/meta/_journal.json @@ -134,6 +134,13 @@ "when": 1785619990000, "tag": "0018_add_hook_token_retention", "breakpoints": true + }, + { + "idx": 19, + "version": "7", + "when": 1786060800000, + "tag": "0019_add_event_slots", + "breakpoints": true } ] } diff --git a/packages/world-postgres/src/drizzle/schema.ts b/packages/world-postgres/src/drizzle/schema.ts index 578c78cf2d..c12f28e087 100644 --- a/packages/world-postgres/src/drizzle/schema.ts +++ b/packages/world-postgres/src/drizzle/schema.ts @@ -133,7 +133,7 @@ export const runs = schema.table( export const events = schema.table( 'workflow_events', { - eventId: varchar('id').primaryKey(), + eventId: varchar('id').notNull(), eventType: varchar('type').$type().notNull(), correlationId: varchar('correlation_id'), createdAt: timestamp('created_at').defaultNow().notNull(), @@ -153,7 +153,14 @@ export const events = schema.table( > >, (tb) => [ - index().on(tb.runId), + // Event ids are per-run slot positions, so `evnt_…0001` exists once per + // run and is only unique together with the run it belongs to. Runs + // created before slots keep globally-unique ULIDs, which this key also + // admits. + primaryKey({ columns: [tb.runId, tb.eventId] }), + // No standalone index on `runId`: the primary key leads with it, so every + // by-run lookup and range scan is served by that index already. Keeping one + // would cost a second write per event on the table's hottest path. index().on(tb.correlationId), // Runtime-correlated one-shot events must be unique per (run, correlation) // — without @@ -170,6 +177,21 @@ export const events = schema.table( ] ); +/** + * Which runs are slot-numbered. A row exists iff the run is, so its absence is + * exactly the "this run predates slots, keep minting ULIDs" signal — no scan of + * the event log is needed to tell the two schemes apart. + * + * A marker, not a counter. Positions are allocated by the insert that occupies + * them (`MAX(slot) + 1` read from the log inside the INSERT), so nothing is + * handed out ahead of the write that uses it and a write that fails leaves the + * position free for the next one. A counter here would instead burn a position + * per failed write, and every such hole is permanent. + */ +export const eventSlots = schema.table('workflow_event_slots', { + runId: varchar('run_id').primaryKey(), +}); + export const steps = schema.table( 'workflow_steps', { diff --git a/packages/world-postgres/src/index.ts b/packages/world-postgres/src/index.ts index 463ba42fba..dae35f39cc 100644 --- a/packages/world-postgres/src/index.ts +++ b/packages/world-postgres/src/index.ts @@ -64,7 +64,13 @@ export function createWorld( return { specVersion: SPEC_VERSION_CURRENT, - capabilities: { hookRetention: { active: true } }, + capabilities: { + hookRetention: { active: true }, + // New runs get dense per-run slot event ids. Runs created before this + // keep their ULIDs; the scheme is pinned by whether the run owns a slot + // counter, not by this flag, which only says what new runs get. + slotEventIds: true, + }, ...storage, ...streamer, ...queue, diff --git a/packages/world-postgres/src/storage.ts b/packages/world-postgres/src/storage.ts index cecbc51ba6..7d6c10f86d 100644 --- a/packages/world-postgres/src/storage.ts +++ b/packages/world-postgres/src/storage.ts @@ -31,7 +31,11 @@ import type { import { ATTRIBUTE_MAX_PER_RUN, AttributeValidationError, + EVENT_ID_BODY_LENGTH, + EVENT_ID_PREFIX, EventSchema, + eventIdToSlot, + FIRST_EVENT_SLOT, getMaxEventsPerRun, HookSchema, isChildEntityCreationEvent, @@ -44,6 +48,7 @@ import { requiresNewerWorld, SPEC_VERSION_CURRENT, StepSchema, + slotToEventId, stripEventDataRefs, TERMINAL_STEP_STATUSES, TERMINAL_WORKFLOW_RUN_STATUSES, @@ -65,6 +70,7 @@ import { notExists, notInArray, or, + type SQL, sql, } from 'drizzle-orm'; import { monotonicFactory } from 'ulid'; @@ -74,6 +80,245 @@ import { compact } from './util.js'; const DAY_MS = 24 * 60 * 60 * 1000; +/** + * A drizzle handle, either the pool or a transaction. Slot allocation runs on + * whichever one the caller is already inside, so the position an insert takes + * commits or rolls back with the insert itself. + */ +type DrizzleLike = Pick; + +/** Only for legacy (pre-slot) runs; see `allocateEventId`. */ +const legacyEventUlid = monotonicFactory(); + +/** + * How many positions one insert will try before giving up. Reached only when a + * run is taking concurrent writes faster than any of them can commit. + */ +const SLOT_INSERT_MAX_ATTEMPTS = 40; +/** + * Collisions that retry the instant the conflicting writer settles. + * + * `ON CONFLICT DO NOTHING` does not skip an uncommitted conflicting row: the + * unique-index check waits on that writer's transaction and only then reports + * the conflict, so a lost race has already waited for exactly the thing the + * next position depends on. Sleeping on top of that adds latency to a + * suspension flush and buys nothing. + * + * The backoff below covers the shape blocking does not: writers that keep + * arriving while the loop spins, where jittering the herd is the only way the + * loop converges before it exhausts its attempts. + */ +const SLOT_INSERT_IMMEDIATE_ATTEMPTS = 8; +/** Backoff between collisions, so a wide fan-out spreads rather than lockstep. */ +const SLOT_INSERT_BASE_DELAY_MS = 2; +const SLOT_INSERT_MAX_DELAY_MS = 40; + +/** + * Isolation for every transaction an event insert can run inside. + * + * {@link insertEventRow} answers a collision by recomputing the next position + * and inserting again, which only terminates if the retry can see rows + * committed since the transaction began. Under REPEATABLE READ or SERIALIZABLE + * it cannot: every attempt reads the transaction's original snapshot, computes + * the same taken position, and the loop runs to its limit and 503s. READ + * COMMITTED is Postgres' default, so this is a statement of the requirement + * rather than a change, and it keeps a database whose + * `default_transaction_isolation` was raised from turning event writes into + * timeouts. Inserts outside a transaction need nothing: a lone statement takes + * a fresh snapshot at every isolation level. + */ +const SLOT_INSERT_TRANSACTION = { isolationLevel: 'read committed' } as const; + +/** The pg error behind a drizzle wrapper, or an empty shape if there is none. */ +function pgErrorOf(err: unknown): { code?: string; constraint?: string } { + const direct = err as { code?: string; constraint?: string }; + if (direct?.code) { + return direct; + } + return ( + (err as { cause?: { code?: string; constraint?: string } })?.cause ?? {} + ); +} + +/** + * The position a slot-numbered insert takes: one above the highest the run + * already holds, read inside the INSERT that takes it. + * + * Nothing hands out a position ahead of the write that fills it. A writer that + * loses a dedup race, or whose transaction rolls back, leaves the numbering + * untouched, so a log missing a position is missing an *event* rather than + * merely a number. The runtime depends on exactly that: it refuses to replay a + * log with a hole, because a position nothing occupies cannot be told apart + * from an event that never happened. + * + * A counter column would be cheaper and is what this used to be. It cannot + * hold that property: a number handed out before the write lands is a number + * lost whenever the write does not, and the resulting holes are permanent. + * + * The subquery is an index-only read of the primary key's last row for the + * run, not a scan. Ordering is lexicographic, which is the same order as by + * position because every body is zero-padded to a fixed width. + * + * Every numeric parameter is cast explicitly. `substring(text from $n)` with an + * untyped parameter resolves to the *regular expression* overload rather than + * the positional one, which quietly returns NULL for every id and hands every + * writer the first slot. + */ +function nextSlotId(runId: string): SQL { + const bodyFrom = sql.raw(String(EVENT_ID_PREFIX.length + 1)); + const width = sql.raw(String(EVENT_ID_BODY_LENGTH)); + const noEvents = sql.raw(String(FIRST_EVENT_SLOT - 1)); + return sql`${EVENT_ID_PREFIX} || lpad((coalesce((select cast(substring(prev.id from ${bodyFrom}) as bigint) from ${Schema.events} prev where prev.run_id = ${runId} order by prev.id desc limit 1), ${noEvents}) + 1)::text, ${width}, '0')`; +} + +/** + * The id an insert for `runId` should allocate with: a slot expression for a + * slot-numbered run, a fresh ULID for one that predates slots. + * + * A row in `workflow_event_slots` is the marker for the first case. Its + * absence is exactly the "this run predates slots" signal, which is why the + * table is still read even though nothing advances it any more. + * + * A legacy run keeps minting under the original `wevt_` prefix rather than + * moving to `evnt_`: a mid-life prefix change would sort every new event + * before every old one, since `evnt_` < `wevt_`. + */ +async function allocateEventId( + db: DrizzleLike, + runId: string +): Promise> { + const [row] = await db + .select({ runId: Schema.eventSlots.runId }) + .from(Schema.eventSlots) + .where(eq(Schema.eventSlots.runId, runId)) + .limit(1); + return row ? nextSlotId(runId) : `wevt_${legacyEventUlid()}`; +} + +/** + * Inserts one event row, retrying while the position it computed is taken. + * + * The primary-key conflict is absorbed by `ON CONFLICT DO NOTHING` rather than + * raised, so a lost race costs a retry instead of the enclosing transaction — + * an error inside a transaction would poison it, and these inserts run in one. + * Every other unique violation still raises, which is what lets callers + * translate a dedup conflict on `workflow_events_entity_creation_unique`. + * + * Returns `undefined` only for an id that is a plain string (a legacy ULID, or + * the reserved first slot), where a conflict is the caller's answer rather + * than something to retry. + */ +async function insertEventRow( + db: DrizzleLike, + values: Omit & { + eventId: string | SQL; + } +): Promise<{ eventId: string; createdAt: Date } | undefined> { + const runId = values.runId; + const allocates = typeof values.eventId !== 'string'; + for (let attempt = 0; ; attempt++) { + const [row] = await db + .insert(Schema.events) + .values(values as typeof Schema.events.$inferInsert) + .onConflictDoNothing({ + target: [Schema.events.runId, Schema.events.eventId], + }) + .returning({ + eventId: Schema.events.eventId, + createdAt: Schema.events.createdAt, + }); + if (row) { + return row; + } + if (!allocates || attempt >= SLOT_INSERT_MAX_ATTEMPTS) { + if (!allocates) { + return undefined; + } + throw new WorkflowWorldError( + `Could not allocate an event slot for run "${runId}" after ${SLOT_INSERT_MAX_ATTEMPTS} attempts`, + { status: 503 } + ); + } + if (attempt >= SLOT_INSERT_IMMEDIATE_ATTEMPTS) { + const delay = Math.min( + SLOT_INSERT_MAX_DELAY_MS, + SLOT_INSERT_BASE_DELAY_MS * + 2 ** (attempt - SLOT_INSERT_IMMEDIATE_ATTEMPTS) + ); + await new Promise((resolve) => + setTimeout(resolve, Math.random() * delay) + ); + } + } +} + +/** + * Marks a run being created as slot-numbered and returns its first event id. + * + * The row records the scheme and nothing else; positions come from the log + * itself, see {@link nextSlotId}. + * + * `DO NOTHING` on conflict because the arbitration that matters is the event + * insert: two writers racing one run_created both take the first slot, and the + * composite events primary key rejects the loser. + */ +async function openEventSlots(db: DrizzleLike, runId: string): Promise { + await db.insert(Schema.eventSlots).values({ runId }).onConflictDoNothing(); + return slotToEventId(FIRST_EVENT_SLOT); +} + +/** + * The report half of bump-and-report: the events sitting on the slots between + * the one the writer asked for and the one its write actually landed on. + * + * Returns `undefined` when there is nothing to report — the write took the slot + * it asked for, the run is not slot-numbered, or the caller sent a count from a + * log that is already ahead of this write. + * + * The set can be short of the slot span it covers. A position is taken by the + * INSERT that computes it, and that INSERT commits on its own, so at the moment + * this reads the span a concurrent writer holding a lower position may not have + * committed yet. Its row appears shortly after and no position is left behind, + * because a write that fails never took one. `hasMore` says the report is a + * lower bound for now rather than a permanent one, and it is advisory either + * way: the caller's ordinary incremental read still runs. + */ +async function reportSkippedSlots( + db: Drizzle, + runId: string, + committedEventId: string, + askedFor: number, + resolveData: ResolveData +): Promise<{ events: Event[]; hasMore: boolean } | undefined> { + const committedSlot = eventIdToSlot(committedEventId); + if ( + committedSlot === null || + askedFor < FIRST_EVENT_SLOT || + committedSlot <= askedFor + 1 + ) { + return undefined; + } + const rows = await db + .select() + .from(Schema.events) + .where( + and( + eq(Schema.events.runId, runId), + gt(Schema.events.eventId, slotToEventId(askedFor)), + lt(Schema.events.eventId, committedEventId) + ) + ) + .orderBy(Schema.events.eventId); + const events = rows.map((row) => { + row.eventData ||= row.eventDataJson; + return stripEventDataRefs(EventSchema.parse(compact(row)), resolveData); + }); + return { + events, + hasMore: events.length < committedSlot - askedFor - 1, + }; +} + function getHookRetentionLimitMs(): number { const days = Number( process.env.WORKFLOW_POSTGRES_HOOK_RETENTION_LIMIT_DAYS ?? 30 @@ -402,7 +647,7 @@ async function handleLegacyEventPostgres( ); } return insertLegacyEvent(tx); - }) + }, SLOT_INSERT_TRANSACTION) : await insertLegacyEvent(drizzle); const event = EventSchema.parse({ @@ -527,8 +772,18 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { ); } + // The id this call's event took, known only once its insert has + // committed: on a slot-numbered run the position is chosen inside the + // INSERT, so there is nothing to read before it. let eventId: string | undefined; - const getEventId = () => (eventId ??= `wevt_${ulid()}`); + // Lazy, because on a legacy run this mints a ULID and on a slot run it + // reads which of the two schemes applies. Every caller below awaits it + // immediately before its insert. A caller that has already fixed the id + // — run_created, which always takes the first slot — gets that back. + const getEventId = async ( + db: DrizzleLike = drizzle + ): Promise> => + eventId ?? (await allocateEventId(db, effectiveRunId)); // For run_created events, use client-provided runId or generate one server-side let effectiveRunId: string; @@ -646,7 +901,12 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { .returning(); if (inserted) { - const runCreatedEventId = `wevt_${ulid()}`; + // This synthetic run_created is the run's first event, so it + // opens the slot counter the rest of the run allocates from. + const runCreatedEventId = await openEventSlots( + drizzle, + effectiveRunId + ); await drizzle.insert(events).values({ runId: effectiveRunId, eventId: runCreatedEventId, @@ -696,12 +956,14 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { ); } - // Route to legacy handler for pre-event-sourcing runs + // Route to legacy handler for pre-event-sourcing runs. A run this old + // is ULID-numbered by definition, so the id is minted here rather than + // read out of a slot marker the run cannot have. if (isLegacySpecVersion(currentRun.specVersion)) { return handleLegacyEventPostgres( drizzle, effectiveRunId, - getEventId(), + `wevt_${legacyEventUlid()}`, data, currentRun, params @@ -740,23 +1002,24 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { .limit(1); // Create the event (still record it) - const [value] = await drizzle - .insert(Schema.events) - .values({ - runId: effectiveRunId, - eventId: getEventId(), - correlationId: data.correlationId, - eventType: data.eventType, - eventData: 'eventData' in data ? data.eventData : undefined, - specVersion: effectiveSpecVersion, - }) - .returning({ createdAt: Schema.events.createdAt }); + const value = await insertEventRow(drizzle, { + runId: effectiveRunId, + eventId: await getEventId(), + correlationId: data.correlationId, + eventType: data.eventType, + eventData: 'eventData' in data ? data.eventData : undefined, + specVersion: effectiveSpecVersion, + }); + if (!value) { + throw new EntityConflictError( + `run_cancelled for run "${effectiveRunId}" could not be created` + ); + } const result = { ...data, ...value, runId: effectiveRunId, - eventId: getEventId(), }; const parsed = EventSchema.parse(result); const resolveData = params?.resolveData ?? 'all'; @@ -930,6 +1193,10 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { `Workflow run "${effectiveRunId}" already exists` ); } + // Open the run's slot counter. Doing it here, rather than lazily on + // first allocation, is what makes "no row" mean "created before slots + // existed" for the rest of the run's life. + eventId = await openEventSlots(drizzle, effectiveRunId); run = deserializeRunError(compact(runValue)); } @@ -1269,12 +1536,10 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { // step_started. Because this synthetic event is in the same // transaction as the lazy step row and step_started event, we // cannot leave behind only one side of that materialization. - const stepCreatedEventId = `wevt_${ulid()}`; - await tx - .insert(events) - .values({ + try { + await insertEventRow(tx, { runId: effectiveRunId, - eventId: stepCreatedEventId, + eventId: await allocateEventId(tx, effectiveRunId), correlationId: data.correlationId, eventType: 'step_created', eventData: { @@ -1282,8 +1547,18 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { input: lazyData.input, }, specVersion: effectiveSpecVersion, - }) - .onConflictDoNothing(); + }); + } catch (err) { + // A concurrent writer already published this run's + // step_created for the same step. The event exists either way, + // which is all this synthetic write was for. + if ( + pgErrorOf(err).constraint !== + 'workflow_events_entity_creation_unique' + ) { + throw err; + } + } stepCreatedLazily = true; } @@ -1350,31 +1625,27 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { } } - // Allocate the step_started ULID only after the guarded step UPDATE - // has acquired and passed the row lock. Without a sequence, this is - // the local ordering guarantee we can provide: a writer blocked on - // the step row will not carry an older event id into a later insert. - const stepStartedEventId = `wevt_${ulid()}`; - eventId = stepStartedEventId; - const [eventValue] = await tx - .insert(events) - .values({ - runId: effectiveRunId, - eventId: stepStartedEventId, - correlationId: data.correlationId, - eventType: data.eventType, - eventData: storedEventData, - specVersion: effectiveSpecVersion, - }) - .returning({ createdAt: events.createdAt }); + // Allocate the step_started position only after the guarded step + // UPDATE has acquired and passed the row lock, so a writer blocked + // on the step row cannot carry an earlier position into a later + // insert. + const eventValue = await insertEventRow(tx, { + runId: effectiveRunId, + eventId: await allocateEventId(tx, effectiveRunId), + correlationId: data.correlationId, + eventType: data.eventType, + eventData: storedEventData, + specVersion: effectiveSpecVersion, + }); if (!eventValue) { throw new EntityConflictError( - `Event ${stepStartedEventId} could not be created` + `Event for step "${data.correlationId}" could not be created` ); } - return eventValue; - }); + eventId = eventValue.eventId; + return { createdAt: eventValue.createdAt }; + }, SLOT_INSERT_TRANSACTION); } // Handle step_completed event: update step status @@ -1572,25 +1843,22 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { token: eventData.token, conflictingRunId: existingHook.runId, }; - const conflictEventId = getEventId(); - - const [conflictValue] = await drizzle - .insert(events) - .values({ - runId: effectiveRunId, - eventId: conflictEventId, - correlationId: data.correlationId, - eventType: 'hook_conflict', - eventData: conflictEventData, - specVersion: effectiveSpecVersion, - }) - .returning({ createdAt: events.createdAt }); + const conflictValue = await insertEventRow(drizzle, { + runId: effectiveRunId, + eventId: await getEventId(), + correlationId: data.correlationId, + eventType: 'hook_conflict', + eventData: conflictEventData, + specVersion: effectiveSpecVersion, + }); if (!conflictValue) { throw new EntityConflictError( - `Event ${conflictEventId} could not be created` + `hook_conflict for run "${effectiveRunId}" could not be created` ); } + const conflictEventId = conflictValue.eventId; + eventId = conflictEventId; const conflictResult = { eventType: 'hook_conflict' as const, @@ -1690,31 +1958,27 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { ); } - // Allocate the ULID only after the row lock is acquired, + // Allocate the position only after the row lock is acquired, // matching step_started's ordering guarantee: a writer blocked - // on the run row must not carry an older event id into a later + // on the run row must not carry an earlier position into a later // insert. - const hookReceivedEventId = `wevt_${ulid()}`; - eventId = hookReceivedEventId; - const [eventValue] = await tx - .insert(events) - .values({ - runId: effectiveRunId, - eventId: hookReceivedEventId, - correlationId: data.correlationId, - eventType: data.eventType, - eventData: storedEventData, - specVersion: effectiveSpecVersion, - }) - .returning({ createdAt: events.createdAt }); + const eventValue = await insertEventRow(tx, { + runId: effectiveRunId, + eventId: await allocateEventId(tx, effectiveRunId), + correlationId: data.correlationId, + eventType: data.eventType, + eventData: storedEventData, + specVersion: effectiveSpecVersion, + }); if (!eventValue) { throw new EntityConflictError( - `Event ${hookReceivedEventId} could not be created` + `Event for hook "${data.correlationId}" could not be created` ); } - return eventValue; - }); + eventId = eventValue.eventId; + return { createdAt: eventValue.createdAt }; + }, SLOT_INSERT_TRANSACTION); } // Handle wait_created event: create wait entity @@ -1800,17 +2064,18 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { try { if (!value) { - [value] = await drizzle - .insert(events) - .values({ - runId: effectiveRunId, - eventId: getEventId(), - correlationId: data.correlationId, - eventType: data.eventType, - eventData: storedEventData, - specVersion: effectiveSpecVersion, - }) - .returning({ createdAt: events.createdAt }); + const inserted = await insertEventRow(drizzle, { + runId: effectiveRunId, + eventId: await getEventId(), + correlationId: data.correlationId, + eventType: data.eventType, + eventData: storedEventData, + specVersion: effectiveSpecVersion, + }); + if (inserted) { + eventId = inserted.eventId; + value = { createdAt: inserted.createdAt }; + } } } catch (err) { // Translate unique-violation on the correlated-event partial index @@ -1829,10 +2094,7 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { isChildEntityCreationEventType(data.eventType) || (data.eventType === 'attr_set' && data.eventData.writer.type === 'workflow'); - const pgErr = (err as { code?: string; constraint?: string }).code - ? (err as { code?: string; constraint?: string }) - : ((err as { cause?: { code?: string; constraint?: string } }) - .cause ?? {}); + const pgErr = pgErrorOf(err); const pgCode = pgErr.code; const pgConstraint = pgErr.constraint; if ( @@ -1846,16 +2108,16 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { } throw err; } - if (!value) { + if (!value || !eventId) { throw new EntityConflictError( - `Event ${getEventId()} could not be created` + `${data.eventType} for run "${effectiveRunId}" could not be created` ); } const result = { ...data, ...value, runId: effectiveRunId, - eventId: getEventId(), + eventId, ...(storedEventData !== undefined ? { eventData: storedEventData } : {}), @@ -1872,6 +2134,34 @@ export function createEventsStorage(drizzle: Drizzle): Storage['events'] { // For run_started: include all events so the runtime can skip // the initial events.list call and reduce TTFB. let eventPage: PaginatedResponse | undefined; + // The skipped-slot report and the inline delta below share + // `events`/`cursor`/`hasMore`, and the runtime sends both on the same + // write. The delta wins: the skipped slots all sit above the cursor, so + // it is a strict superset, and it is the only one of the two that + // advances `cursor`. Running the report anyway would cost a query whose + // result the delta overwrites. + if ( + params?.eventCount !== undefined && + typeof params.sinceCursor !== 'string' + ) { + const report = await reportSkippedSlots( + drizzle, + effectiveRunId, + parsed.eventId, + params.eventCount, + resolveData + ); + if (report) { + // Deliberately no cursor: the report is a lower bound on what this + // write skipped over, not a page the caller has now read to the end + // of, so it must not advance the caller's read position. + eventPage = { + data: report.events, + cursor: null, + hasMore: report.hasMore, + }; + } + } if (data.eventType === 'run_started' && run && !params?.skipPreload) { const eventRows = await drizzle .select() diff --git a/packages/world-postgres/test/storage.test.ts b/packages/world-postgres/test/storage.test.ts index 62f452b7c9..4882e17c00 100644 --- a/packages/world-postgres/test/storage.test.ts +++ b/packages/world-postgres/test/storage.test.ts @@ -6,11 +6,11 @@ import type { Step, WorkflowRun, } from '@workflow/world'; -import { SPEC_VERSION_CURRENT } from '@workflow/world'; +import { eventIdToSlot, SPEC_VERSION_CURRENT } from '@workflow/world'; import { encode } from 'cbor-x'; import { eq } from 'drizzle-orm'; import { Pool } from 'pg'; -import { decodeTime, ulid } from 'ulid'; +import { ulid } from 'ulid'; import { afterAll, afterEach, @@ -142,7 +142,7 @@ describe('Storage (Postgres integration)', () => { async function truncateTables() { await pool.query( - 'TRUNCATE TABLE workflow.workflow_events, workflow.workflow_steps, workflow.workflow_hooks, workflow.workflow_runs RESTART IDENTITY CASCADE' + 'TRUNCATE TABLE workflow.workflow_events, workflow.workflow_event_slots, workflow.workflow_steps, workflow.workflow_hooks, workflow.workflow_runs RESTART IDENTITY CASCADE' ); } @@ -814,7 +814,7 @@ describe('Storage (Postgres integration)', () => { expect(updated.attempt).toBe(1); // Incremented by step_started }); - it('allocates the step_started event id after the guarded step update', async () => { + it('allocates the step_started slot after the guarded step update', async () => { const stepId = 'step-start-lock'; await createStep(events, testRunId, { stepId, @@ -827,6 +827,14 @@ describe('Storage (Postgres integration)', () => { max: 1, }); const client = await lockPool.connect(); + // The suite's own pool is `max: 1`, so the parked step_started holds + // it for the duration. The overtaking writer needs a connection of + // its own, which is also the shape being tested: two processes. + const otherPool = new Pool({ + connectionString: container.getConnectionUri(), + max: 1, + }); + const otherEvents = createEventsStorage(createClient(otherPool)); try { await client.query('BEGIN'); @@ -841,19 +849,31 @@ describe('Storage (Postgres integration)', () => { }); await new Promise((resolve) => setTimeout(resolve, 50)); - const releasedAt = Date.now(); + // Written while step_started is still parked on the step row lock. + // A writer that drew its slot on entry would already hold a lower + // one than this; drawing after the lock puts it above. + const overtaking = await otherEvents.create(testRunId, { + eventType: 'step_created', + correlationId: 'step-start-lock-overtaker', + eventData: { stepName: 'test-step', input: new Uint8Array([1]) }, + }); await client.query('COMMIT'); const result = await started; - if (!result.event) { - throw new Error('Expected step_started event'); + if (!result.event || !overtaking.event) { + throw new Error('Expected both events'); } - expect( - decodeTime(result.event.eventId.slice('wevt_'.length)) - ).toBeGreaterThanOrEqual(releasedAt); + const startedSlot = eventIdToSlot(result.event.eventId); + const overtakingSlot = eventIdToSlot(overtaking.event.eventId); + expect(startedSlot).not.toBeNull(); + expect(overtakingSlot).not.toBeNull(); + expect(startedSlot as number).toBeGreaterThan( + overtakingSlot as number + ); } finally { client.release(); await lockPool.end(); + await otherPool.end(); } }); @@ -1121,7 +1141,7 @@ describe('Storage (Postgres integration)', () => { const result = await events.create(testRunId, eventData); expect(result.event.runId).toBe(testRunId); - expect(result.event.eventId).toMatch(/^wevt_/); + expect(result.event.eventId).toMatch(/^evnt_0{10}/); expect(result.event.eventType).toBe('step_started'); expect(result.event.correlationId).toBe('corr_123'); expect(result.event.createdAt).toBeInstanceOf(Date); @@ -1146,7 +1166,7 @@ describe('Storage (Postgres integration)', () => { }); expect(result.event.runId).toBe(testRunId); - expect(result.event.eventId).toMatch(/^wevt_/); + expect(result.event.eventId).toMatch(/^evnt_0{10}/); expect(result.event.eventType).toBe('step_failed'); expect(result.event.correlationId).toBe('corr_123_null'); expect(result.event.createdAt).toBeInstanceOf(Date); @@ -1812,6 +1832,243 @@ describe('Storage (Postgres integration)', () => { }); }); + describe('slot event ids', () => { + let testRunId: string; + beforeEach(async () => { + const run = await createRun(events, { + deploymentId: 'deployment-123', + workflowName: 'test-workflow', + input: new Uint8Array(), + }); + testRunId = run.runId; + }); + + it('numbers a run densely from the first slot', async () => { + await updateRun(events, testRunId, 'run_started'); + + const result = await events.list({ + runId: testRunId, + pagination: { sortOrder: 'asc' }, + }); + + expect(result.data.map((e) => eventIdToSlot(e.eventId))).toEqual([1, 2]); + }); + + it('gives concurrent writers distinct, dense slots', async () => { + const writers = 8; + // The suite's own pool is `max: 1`, which would serialize these writes + // and defeat the point. Give each writer a connection so they actually + // contend for the same slot. + const racePool = new Pool({ + connectionString: container.getConnectionUri(), + max: writers, + }); + const raceEvents = createEventsStorage(createClient(racePool)); + + try { + await Promise.all( + Array.from({ length: writers }, (_, i) => + raceEvents.create(testRunId, { + eventType: 'step_created', + correlationId: `slot-step-${i}`, + eventData: { + stepName: 'test-step', + input: new Uint8Array([i]), + }, + }) + ) + ); + } finally { + await racePool.end(); + } + + const result = await events.list({ + runId: testRunId, + pagination: { sortOrder: 'asc' }, + }); + const slots = result.data + .map((e) => eventIdToSlot(e.eventId)) + .sort((a, b) => (a ?? 0) - (b ?? 0)); + + // run_created holds slot 1 and the racing writers take the rest: no + // duplicate (the composite primary key rejects the loser, which retries) + // and no hole (nothing reserves a slot it does not then use), whatever + // order they happen to land in. + expect(slots).toEqual( + Array.from({ length: writers + 1 }, (_, i) => i + 1) + ); + }); + + it('leaves no hole behind writes that are rejected', async () => { + const writers = 8; + const racePool = new Pool({ + connectionString: container.getConnectionUri(), + max: writers, + }); + const raceEvents = createEventsStorage(createClient(racePool)); + + // Every writer claims the same correlation id, so exactly one + // step_created survives the entity-creation unique index and the rest + // are rejected with EntityConflictError. A slot handed out before the + // insert lands would be burned by each of those rejections, and a burned + // slot is a permanent hole: allocation only moves forward. + try { + const results = await Promise.allSettled( + Array.from({ length: writers }, () => + raceEvents.create(testRunId, { + eventType: 'step_created', + correlationId: 'slot-contended-step', + eventData: { + stepName: 'test-step', + input: new Uint8Array([1]), + }, + }) + ) + ); + expect(results.filter((r) => r.status === 'fulfilled')).toHaveLength(1); + } finally { + await racePool.end(); + } + + // The next write is what exposes a burned slot: it lands right behind + // the winner if no rejection consumed a position, and `writers - 1` + // past it if every rejection did. + await events.create(testRunId, { + eventType: 'step_created', + correlationId: 'slot-after-contention', + eventData: { stepName: 'test-step', input: new Uint8Array([1]) }, + }); + + const result = await events.list({ + runId: testRunId, + pagination: { sortOrder: 'asc' }, + }); + const slots = result.data + .map((e) => eventIdToSlot(e.eventId)) + .sort((a, b) => (a ?? 0) - (b ?? 0)); + + // run_created, the one step_created that won, and the write after it. + expect(slots).toEqual([1, 2, 3]); + }); + + it('hands back the events occupying the slots a write skipped', async () => { + await updateRun(events, testRunId, 'run_started'); + // What a writer that loaded the log right after run_started would report. + const stale = 2; + + for (let i = 0; i < 3; i++) { + await events.create(testRunId, { + eventType: 'step_created', + correlationId: `skipped-step-${i}`, + eventData: { stepName: 'test-step', input: new Uint8Array([i]) }, + }); + } + + const result = await events.create( + testRunId, + { + eventType: 'wait_created', + correlationId: 'skipped-wait', + eventData: { resumeAt: new Date('2099-01-01') }, + }, + { eventCount: stale } + ); + + expect(eventIdToSlot(result.event.eventId)).toBe(6); + expect(result.events?.map((e) => eventIdToSlot(e.eventId))).toEqual([ + 3, 4, 5, + ]); + expect(result.events?.map((e) => e.correlationId)).toEqual([ + 'skipped-step-0', + 'skipped-step-1', + 'skipped-step-2', + ]); + expect(result.hasMore).toBe(false); + }); + + it('reports nothing when the write lands on the slot it asked for', async () => { + const result = await events.create( + testRunId, + { + eventType: 'step_created', + correlationId: 'unskipped-step', + eventData: { stepName: 'test-step', input: new Uint8Array() }, + }, + { eventCount: 1 } + ); + + expect(eventIdToSlot(result.event.eventId)).toBe(2); + expect(result.events).toBeUndefined(); + }); + + it('reports nothing when the writer sends no count', async () => { + await updateRun(events, testRunId, 'run_started'); + await events.create(testRunId, { + eventType: 'step_created', + correlationId: 'uncounted-step', + eventData: { stepName: 'test-step', input: new Uint8Array() }, + }); + + const result = await events.create(testRunId, { + eventType: 'wait_created', + correlationId: 'uncounted-wait', + eventData: { resumeAt: new Date('2099-01-01') }, + }); + + expect(result.events).toBeUndefined(); + }); + + it('gives every racing writer the events it was decided without', async () => { + await updateRun(events, testRunId, 'run_started'); + const stale = 2; + const writers = 8; + const racePool = new Pool({ + connectionString: container.getConnectionUri(), + max: writers, + }); + const raceEvents = createEventsStorage(createClient(racePool)); + + let results: Awaited>[]; + try { + results = await Promise.all( + Array.from({ length: writers }, (_, i) => + raceEvents.create( + testRunId, + { + eventType: 'step_created', + correlationId: `race-report-${i}`, + eventData: { + stepName: 'test-step', + input: new Uint8Array([i]), + }, + }, + { eventCount: stale } + ) + ) + ); + } finally { + await racePool.end(); + } + + // Each writer's report covers only the slots between the one it asked + // for and the one it landed on. It can be short of that span: a writer + // holding a lower slot may not have committed its insert yet, which is + // what `hasMore` says. + for (const result of results) { + const landed = eventIdToSlot(result.event.eventId) as number; + const reported = result.events ?? []; + const span = landed - stale - 1; + expect(reported.length).toBeLessThanOrEqual(span); + expect(result.hasMore ?? false).toBe(reported.length < span); + for (const event of reported) { + const slot = eventIdToSlot(event.eventId) as number; + expect(slot).toBeGreaterThan(stale); + expect(slot).toBeLessThan(landed); + } + } + }); + }); + describe('concurrent entity-creation races', () => { let testRunId: string; beforeEach(async () => { diff --git a/packages/world-vercel/src/events-v4.test.ts b/packages/world-vercel/src/events-v4.test.ts index 23ec038eab..161195fe26 100644 --- a/packages/world-vercel/src/events-v4.test.ts +++ b/packages/world-vercel/src/events-v4.test.ts @@ -143,6 +143,116 @@ describe('throwForErrorResponse', () => { /createEvent failed: HTTP 500 plain text oops/ ); }); + + it('reads message and code out of a CBOR body', () => { + // A 412 that carries an event delta answers in CBOR so the delta's + // payloads stay real bytes. Decoding it by content-type is what keeps the + // message and code from being lost to a failed JSON.parse. + try { + throwForErrorResponse( + 412, + { 'content-type': 'application/cbor' }, + encode({ message: 'Event log moved on' }), + 'createEvent', + 'http://x' + ); + expect.unreachable(); + } catch (err) { + expect(PreconditionFailedError.is(err)).toBe(true); + expect((err as PreconditionFailedError).message).toBe( + 'Event log moved on' + ); + } + + try { + throwForErrorResponse( + 404, + { 'content-type': 'application/cbor' }, + encode({ message: 'hook not found', code: 'not_found' }), + 'createEvent', + 'http://x' + ); + expect.unreachable(); + } catch (err) { + expect((err as WorkflowWorldError).code).toBe('not_found'); + } + }); + + it('keeps a CBOR 412 delta whose event payload is real bytes', () => { + const result = new TextEncoder().encode('"done"'); + try { + throwForErrorResponse( + 412, + { 'content-type': 'application/cbor' }, + encode({ + message: 'Event log moved on', + cursor: 'eid:evnt_missing', + events: [ + { + eventId: 'evnt_missing', + runId: 'wrun_1', + eventType: 'step_completed', + correlationId: 'step_0', + specVersion: 5, + createdAt: '2026-06-10T00:00:00.000Z', + eventData: { result }, + }, + ], + }), + 'createEvent', + 'http://x' + ); + expect.unreachable(); + } catch (err) { + const details = (err as PreconditionFailedError).details as { + events: Array<{ eventData: { result: unknown } }>; + cursor?: string; + }; + expect(details.cursor).toBe('eid:evnt_missing'); + // A JSON body would have mangled these bytes and the delta would have + // been refused whole; CBOR round-trips them, so the client can merge. + expect(details.events[0]?.eventData.result).toBeInstanceOf(Uint8Array); + expect( + new TextDecoder().decode( + details.events[0]?.eventData.result as Uint8Array + ) + ).toBe('"done"'); + } + }); + + it('falls back to the default message when a CBOR body will not decode', () => { + // Undecodable bytes must not be appended to the message as mojibake. + const garbage = new Uint8Array([0xff, 0xfe, 0xfd]); + try { + throwForErrorResponse( + 500, + { 'content-type': 'application/cbor' }, + garbage, + 'createEvent', + 'http://x' + ); + expect.unreachable(); + } catch (err) { + expect((err as WorkflowWorldError).message).toBe( + 'v4 createEvent failed: HTTP 500' + ); + } + }); + + it('still parses a JSON body delivered as bytes', () => { + try { + throwForErrorResponse( + 404, + { 'content-type': 'application/json' }, + new TextEncoder().encode('{"message":"hook not found"}'), + 'createEvent', + 'http://x' + ); + expect.unreachable(); + } catch (err) { + expect((err as WorkflowWorldError).message).toBe('hook not found'); + } + }); }); /** @@ -1230,6 +1340,119 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent.assertNoPendingInterceptors(); }); + it('forwards maxSlot in the frame meta', async () => { + const origin = + WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; + const agent = new MockAgent(); + agent.disableNetConnect(); + + let capturedMeta: Record | undefined; + agent + .get(origin) + .intercept({ + path: '/api/v4/runs/wrun_1/events/wait_created', + method: 'POST', + }) + .reply( + 200, + (opts: { body?: unknown }) => { + const bytes = new Uint8Array(opts.body as ArrayBufferLike); + const metaLen = new DataView( + bytes.buffer, + bytes.byteOffset, + bytes.byteLength + ).getUint32(0, false); + capturedMeta = decode(bytes.subarray(4, 4 + metaLen)) as Record< + string, + unknown + >; + return createEventBody({ + eventType: 'wait_created', + specVersion: 5, + correlationId: 'wait_1', + eventData: { resumeAt: CREATED_AT }, + }); + }, + { + headers: { + 'x-wf-event-id': 'evnt_1', + 'x-wf-run-id': 'wrun_1', + 'x-wf-created-at': '2026-06-10T00:00:00.000Z', + }, + } + ); + + await createWorkflowRunEventV4( + { + runId: 'wrun_1', + eventType: 'wait_created', + specVersion: 6, + correlationId: 'wait_1', + maxSlot: 12, + }, + { token: 'test-token', dispatcher: agent } + ); + + expect(capturedMeta?.maxSlot).toBe(12); + agent.assertNoPendingInterceptors(); + }); + + it('omits maxSlot from the frame meta when not set', async () => { + const origin = + WORKFLOW_SERVER_URL_OVERRIDE || 'https://vercel-workflow.com'; + const agent = new MockAgent(); + agent.disableNetConnect(); + + let capturedMeta: Record | undefined; + agent + .get(origin) + .intercept({ + path: '/api/v4/runs/wrun_1/events/wait_created', + method: 'POST', + }) + .reply( + 200, + (opts: { body?: unknown }) => { + const bytes = new Uint8Array(opts.body as ArrayBufferLike); + const metaLen = new DataView( + bytes.buffer, + bytes.byteOffset, + bytes.byteLength + ).getUint32(0, false); + capturedMeta = decode(bytes.subarray(4, 4 + metaLen)) as Record< + string, + unknown + >; + return createEventBody({ + eventType: 'wait_created', + specVersion: 5, + correlationId: 'wait_1', + eventData: { resumeAt: CREATED_AT }, + }); + }, + { + headers: { + 'x-wf-event-id': 'evnt_1', + 'x-wf-run-id': 'wrun_1', + 'x-wf-created-at': '2026-06-10T00:00:00.000Z', + }, + } + ); + + await createWorkflowRunEventV4( + { + runId: 'wrun_1', + eventType: 'wait_created', + specVersion: 5, + correlationId: 'wait_1', + }, + { token: 'test-token', dispatcher: agent } + ); + + expect('maxSlot' in (capturedMeta ?? {})).toBe(false); + agent.assertNoPendingInterceptors(); + }); + it('omits stateEventCount and stateCursor from the frame meta when not set', 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 ab712abf95..f68f35c880 100644 --- a/packages/world-vercel/src/events-v4.ts +++ b/packages/world-vercel/src/events-v4.ts @@ -104,11 +104,13 @@ async function fetchV4( noteEventsTransportOutcome(dispatcher, error), timeoutMs: null, logLabel: opName, + // Read the body as bytes, not text: a CBOR error body (the fence 412 + // carries event payloads back) does not survive a UTF-8 decode. buildError: async (response) => errorFromV4Response( response.status, headersToRecord(response.headers), - await response.text(), + new Uint8Array(await response.arrayBuffer()), opName, url ), @@ -242,6 +244,19 @@ interface CreateEventV4InputBase { * on for the *accepted* path. */ stateCursor?: string; + /** + * Highest event slot the writer had loaded, i.e. the length of its loaded + * log under slot identity. Named `maxSlot` on the wire because the meta + * already carries an unrelated telemetry `eventCount`. + * + * Supersedes the `stateUpdatedAt`/`stateEventCount`/`stateCursor` triple for + * slot-identity runs: with dense positions one integer says everything the + * watermark approximated. The server allocates from the tail regardless, and + * uses this only to report which slots the write skipped over (returned on + * the success response as `events`/`cursor`/`hasMore`). Older servers ignore + * it. + */ + maxSlot?: number; /** Number of consecutive replay divergences resolved by this write. */ replayDivergenceCount?: number; /** Content digest of the serialized resume payload. Forwarded alongside @@ -448,6 +463,7 @@ function buildPostFrameMeta( meta.stateEventCount = input.stateEventCount; } if (input.stateCursor !== undefined) meta.stateCursor = input.stateCursor; + if (input.maxSlot !== undefined) meta.maxSlot = input.maxSlot; if (input.replayDivergenceCount !== undefined) { meta.replayDivergenceCount = input.replayDivergenceCount; } @@ -469,26 +485,25 @@ function buildPostFrameMeta( function errorFromV4Response( statusCode: number, responseHeaders: Record, - errorBody: string, + errorBody: string | Uint8Array, opName: string, url: string ): Error { let message = `v4 ${opName} failed: HTTP ${statusCode}`; let code: string | undefined; let details: unknown; - try { - const json = JSON.parse(errorBody) as { - message?: string; - code?: string; - events?: unknown; - cursor?: unknown; - }; - if (typeof json.message === 'string') message = json.message; - if (typeof json.code === 'string') code = json.code; - if (statusCode === 412) details = decodePreconditionDetails(json); - } catch { - // body wasn't JSON — keep the default message, append raw text below - if (errorBody) message += ` ${errorBody}`; + const { record, text } = parseV4ErrorBody( + errorBody, + readHeader(responseHeaders, 'content-type') + ); + if (record) { + if (typeof record.message === 'string') message = record.message; + if (typeof record.code === 'string') code = record.code; + if (statusCode === 412) details = decodePreconditionDetails(record); + } else if (text) { + // body wasn't a structured object — keep the default message and append + // whatever the server did send + message += ` ${text}`; } const retryAfter = parseRetryAfter( @@ -505,6 +520,55 @@ function errorFromV4Response( }); } +/** The fields `errorFromV4Response` reads off a structured error body. */ +interface V4ErrorBody { + message?: unknown; + code?: unknown; + events?: unknown; + cursor?: unknown; +} + +/** + * Decode an error body into the record the error builder reads, or into the + * raw text to append when it is not structured. + * + * Two encodings reach this. The default is JSON: the v4 request sends no + * `Accept: application/cbor`, so the server's generic error responder + * negotiates JSON. Responses that need to carry event payloads back are + * hand-encoded as CBOR by the server and say so in `content-type`, because + * JSON cannot round-trip a `Uint8Array` (see `hasUnusablePayload`). Reading + * the body as bytes and branching on the header serves both; decoding bytes as + * text first would corrupt CBOR beyond recovery. + */ +function parseV4ErrorBody( + body: string | Uint8Array, + contentType: string | undefined +): { record?: V4ErrorBody; text?: string } { + if (typeof body !== 'string' && contentType?.includes('application/cbor')) { + try { + // cbor-x caches decode state on its input; decode a copy so a shared + // buffer is never mutated under an unrelated reader. + const decoded = decode(body.slice()) as unknown; + if (typeof decoded === 'object' && decoded !== null) { + return { record: decoded as V4ErrorBody }; + } + } catch { + // undecodable CBOR: appending its bytes as text would be noise + } + return {}; + } + const text = typeof body === 'string' ? body : new TextDecoder().decode(body); + try { + const json = JSON.parse(text) as unknown; + if (typeof json === 'object' && json !== null) { + return { record: json as V4ErrorBody }; + } + } catch { + // not JSON either — fall through to the raw text + } + return { text }; +} + /** * Pick the inline event delta off a 412 body. * @@ -520,10 +584,9 @@ function errorFromV4Response( * untrusted-shaped data on a failure path, and the fallback (a full reload) is * always correct. */ -function decodePreconditionDetails(json: { - events?: unknown; - cursor?: unknown; -}): PreconditionFailureDetails | undefined { +function decodePreconditionDetails( + json: V4ErrorBody +): PreconditionFailureDetails | undefined { if (!Array.isArray(json.events) || json.events.length === 0) return undefined; const events: Event[] = []; for (const raw of json.events) { @@ -548,17 +611,19 @@ function decodePreconditionDetails(json: { * Payload fields (input / output / result / error / payload / metadata) are * `Uint8Array` everywhere else in this client — the runtime dehydrates before * writing and rehydrates after reading, and the write path throws on anything - * else. A 412 body is JSON, though: the request carries no - * `Accept: application/cbor`, so resolved bytes serialize to + * else. A JSON 412 body cannot hold that: resolved bytes serialize to * `{"type":"Buffer","data":[…]}` or an index-keyed object depending on the * backend's serializer. `EventSchema` accepts either — its payload fields are * unions that bottom out in `z.any()` — so nothing downstream would flag the - * mangled value; the runtime would hydrate garbage from it instead. + * mangled value; the runtime would hydrate garbage from it instead. A CBOR + * body round-trips the bytes intact and passes this check on its own merits, + * which is why a backend that attaches an event delta to a 412 encodes it that + * way. * * Refusing the delta is one-sided safe: the fallback full reload goes over a * frame-encoded path that returns real bytes. Deltas made only of * payload-less events (waits, hook disposal, attribute writes) keep the fast - * path. + * path whatever the encoding. */ function hasUnusablePayload(candidate: Record): boolean { const eventType = candidate.eventType; @@ -581,7 +646,7 @@ function hasUnusablePayload(candidate: Record): boolean { export function throwForErrorResponse( statusCode: number, responseHeaders: Record, - errorBody: string, + errorBody: string | Uint8Array, opName: string, url: string ): never { diff --git a/packages/world-vercel/src/events.test.ts b/packages/world-vercel/src/events.test.ts index 6ebdd6af97..ab1eb1632a 100644 --- a/packages/world-vercel/src/events.test.ts +++ b/packages/world-vercel/src/events.test.ts @@ -346,6 +346,48 @@ describe('createWorkflowRunEvent precondition snapshot wire fields', () => { agent.assertNoPendingInterceptors(); }); + it('renames eventCount to maxSlot', async () => { + // The runtime sends `eventCount` once a run's own ids are slot-shaped. It + // cannot ride under that name: the v4 meta already has an unrelated + // telemetry `eventCount`, so the backend would read a progress counter as + // a log position. + const agent = mockAgent(); + let capturedMeta: Record | undefined; + + agent + .get(ORIGIN) + .intercept({ + path: '/api/v4/runs/wrun_1/events/run_started', + method: 'POST', + }) + .reply( + 200, + (opts: { body?: unknown }) => { + capturedMeta = decodePostedMeta(opts.body); + return runStartedResponse(); + }, + { + headers: { + 'content-type': V4_FRAME_CONTENT_TYPE, + 'x-wf-event-id': 'evnt_1', + 'x-wf-run-id': 'wrun_1', + 'x-wf-created-at': '2026-06-10T00:00:00.000Z', + 'x-wf-max-events': '10000', + }, + } + ); + + await createWorkflowRunEvent( + 'wrun_1', + { eventType: 'run_started', specVersion: 2 } as AnyEventRequest, + { eventCount: 9 }, + { token: 'test-token', dispatcher: agent } + ); + + expect(capturedMeta?.maxSlot).toBe(9); + agent.assertNoPendingInterceptors(); + }); + it('never sends the snapshot on the legacy v1Compat path', async () => { // Pre-event-sourcing runs have no event log to fence, and the legacy // endpoint has no field for the snapshot: the params are dropped whole. diff --git a/packages/world-vercel/src/events.ts b/packages/world-vercel/src/events.ts index 12e98e49da..d5b981cef1 100644 --- a/packages/world-vercel/src/events.ts +++ b/packages/world-vercel/src/events.ts @@ -606,6 +606,11 @@ async function createWorkflowRunEventInner( stateUpdatedAt: params?.stateUpdatedAt, stateEventCount: params?.stateEventCount, ...(params?.stateCursor ? { stateCursor: params.stateCursor } : {}), + // Slot-identity snapshot. The runtime sends `eventCount` instead of the + // watermark triple once the run's own ids are slot-shaped; it rides as + // `maxSlot` because the v4 meta already has an unrelated telemetry + // `eventCount`. + ...(params?.eventCount !== undefined ? { maxSlot: params.eventCount } : {}), replayDivergenceCount: params?.replayDivergenceCount, occurredAt: params?.occurredAt ?? new Date(), // Opt-in inline-delta: forward the cursor the runtime held before diff --git a/packages/world-vercel/src/index.ts b/packages/world-vercel/src/index.ts index b9a38c3616..e94e98c07e 100644 --- a/packages/world-vercel/src/index.ts +++ b/packages/world-vercel/src/index.ts @@ -1,5 +1,5 @@ import type { World } from '@workflow/world'; -import { SPEC_VERSION_SUPPORTS_COMPRESSION } from '@workflow/world'; +import { SPEC_VERSION_SUPPORTS_SLOT_IDENTITY } from '@workflow/world'; import { createAnalytics } from './analytics.js'; import { createRunId, describeRun } from './create-run-id.js'; import { createGetEncryptionKeyForRun } from './encryption.js'; @@ -30,9 +30,12 @@ export function createWorld(config?: APIConfig): World { config?.projectConfig?.projectId || process.env.VERCEL_PROJECT_ID; return { - // Spec v5 adds client-side zstd/gzip payload compression. The server stores - // those payloads opaquely, and v5 remains a superset of v4 attributes. - specVersion: SPEC_VERSION_SUPPORTS_COMPRESSION, + // Spec v6 adds slot-numbered event ids on top of v5's client-side + // zstd/gzip payload compression. The version is what tells the backend + // which id scheme a run uses: it is stamped on `run_created` and read back + // on every later write, so a run created before v6 keeps its ULIDs even + // though this adapter now asks for slots. + specVersion: SPEC_VERSION_SUPPORTS_SLOT_IDENTITY, capabilities: { hookRetention: { active: true }, // workflow-server enforces the `stateUpdatedAt` optimistic-concurrency @@ -47,6 +50,11 @@ export function createWorld(config?: APIConfig): World { // Vercel deployments are atomic and immutable, so a deployment id names // one fixed build for its whole lifetime. deploymentAffinity: true, + // New runs get dense per-run slot event ids. Runs created before the + // backend adopted them keep their ULIDs; the scheme is pinned by the + // spec version stamped on each run, not by this flag, which only says + // what new runs get. + slotEventIds: true, // NOTE: the backend half of resumeHook()'s parallel fast path — that // the server enforces the `(runId, resumeId)` dedup constraint — is // NO LONGER a static world capability here. It is attested per-lookup by diff --git a/packages/world/src/events.ts b/packages/world/src/events.ts index e397d2c8b6..ba37c947ee 100644 --- a/packages/world/src/events.ts +++ b/packages/world/src/events.ts @@ -814,6 +814,38 @@ export interface CreateEventParams { * authoritative full reload, which is always correct. */ stateCursor?: string; + /** + * How many events the writer held in its loaded log when it decided to write + * this one — equivalently, the slot it expects to land on minus one. + * + * Only meaningful against a World that declares + * `WorldCapabilities.slotEventIds`, where slots are dense and 1-based so a + * count and a position are the same number. Such a World attempts + * `eventCount + 1`, and on contention **bumps** to the next free slot and + * commits there anyway — a stale count never rejects a write. What it does + * instead is report: when the committed slot is higher than the one asked + * for, the events occupying the skipped slots come back on the success + * response in {@link EventResult.events} / `cursor` / `hasMore`, so the + * writer learns exactly what it had not seen. + * + * This supersedes the {@link stateUpdatedAt} / {@link stateEventCount} / + * {@link stateCursor} triple for slot Worlds. That triple approximates a + * position with a ULID-time watermark plus a count of events at or below it, + * which is why a *complete but stale* prefix passes it: every event the + * writer holds is at or below its own watermark, so the count matches and no + * fence fires. A dense position has no such blind spot. Worlds without slots + * ignore this field and keep using the triple. + * + * A batch of writes issued from one snapshot starts from the same + * `eventCount`; they land on consecutive slots in whatever order the World + * serializes them, which is why they can stay a parallel fan-out instead of + * a chain of round-trips. The count a given write sends is the writer's + * position *at that moment*, so it advances mid-batch as reported events are + * folded back into the loaded log: a write issued after a sibling's + * bump-and-report already holds the slots that report named, and asks for a + * slot above them. + */ + eventCount?: number; /** * Timestamp for when the event occurred on the client side. Worlds that * support this can persist it separately from `createdAt`, which represents @@ -945,7 +977,7 @@ export type EventResult = { } & ( | { /** - * Events with data resolved. Three producers populate this: + * Events with data resolved. Four producers populate this: * * - On a `run_started` response: all events up to this point, so the * runtime can skip the initial `events.list` call and reduce TTFB. @@ -958,6 +990,13 @@ export type EventResult = { * log through the canonical `hook_received`, so the lazy hook queue * consumer can skip both the `run_started` write and the initial * `events.list`. + * - On any response from a slot-allocating World (see + * `WorldCapabilities.slotEventIds`) whose committed slot came out + * higher than the one {@link CreateEventParams.eventCount} asked for: + * the events occupying the slots that were skipped over, in slot + * order. This is the "report" half of bump-and-report — the write + * succeeded, and these are the events the writer had not seen when it + * decided to make it. */ events: Event[]; /** Pagination cursor for `events`, matching events.list semantics. */ diff --git a/packages/world/src/index.ts b/packages/world/src/index.ts index 68d0d2de65..50ce1b9a0a 100644 --- a/packages/world/src/index.ts +++ b/packages/world/src/index.ts @@ -111,16 +111,28 @@ export { PaginatedResponseSchema, StructuredErrorSchema, } from './shared.js'; +export { + EVENT_ID_BODY_LENGTH, + EVENT_ID_PREFIX, + eventIdToSlot, + FIRST_EVENT_SLOT, + isSlotBody, + isSlotEventId, + MAX_EVENT_SLOT, + slotToEventId, +} from './slot-identity.js'; export type { SpecVersion } from './spec-version.js'; export { isLegacySpecVersion, requiresNewerWorld, SPEC_VERSION_CURRENT, SPEC_VERSION_LEGACY, + SPEC_VERSION_MAX_SUPPORTED, SPEC_VERSION_SUPPORTS_ATTRIBUTES, SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT, SPEC_VERSION_SUPPORTS_COMPRESSION, SPEC_VERSION_SUPPORTS_EVENT_SOURCING, + SPEC_VERSION_SUPPORTS_SLOT_IDENTITY, } from './spec-version.js'; export type * from './steps.js'; export { diff --git a/packages/world/src/interfaces.ts b/packages/world/src/interfaces.ts index e0184a0d2e..06a49b973e 100644 --- a/packages/world/src/interfaces.ts +++ b/packages/world/src/interfaces.ts @@ -415,6 +415,30 @@ export interface WorldCapabilities { * fail ordinary runs after a version bump. */ deploymentAffinity?: boolean; + + /** + * The World allocates **slot-numbered** event ids: `evnt_` plus the event's + * dense, 1-based position in its run's log, zero-padded to 26 characters + * (see `slot-identity.ts`). Two guarantees come with it, and the runtime + * relies on both: + * + * - **Density.** A run's slots are contiguous from 1, so the number of + * events a reader holds *is* the position of the last one. That is what + * makes {@link CreateEventParams.eventCount} a complete statement of the + * writer's snapshot, where the `stateUpdatedAt` / `stateEventCount` + * watermark pair could only approximate it. + * - **Bump and report.** A create never fails because its requested slot is + * taken. The World advances to the next free slot, commits there, and + * returns the events occupying the slots it skipped over on the success + * response (see {@link EventResult.events}). The writer learns its + * snapshot was stale without the write being rejected. + * + * A run's scheme is pinned by the run, not by this flag: it is readable off + * the shape of the run's own first event id, so a World that turns slots on + * keeps replaying its existing ULID-numbered runs unchanged. The capability + * only says what *new* runs get. + */ + slotEventIds?: boolean; } /** diff --git a/packages/world/src/slot-identity.test.ts b/packages/world/src/slot-identity.test.ts new file mode 100644 index 0000000000..63a7e85d30 --- /dev/null +++ b/packages/world/src/slot-identity.test.ts @@ -0,0 +1,93 @@ +import { ulid } from 'ulid'; +import { describe, expect, it } from 'vitest'; +import { + EVENT_ID_BODY_LENGTH, + EVENT_ID_PREFIX, + eventIdToSlot, + FIRST_EVENT_SLOT, + isSlotBody, + isSlotEventId, + slotToEventId, +} from './slot-identity.js'; +import { ulidToDate, validateUlidTimestamp } from './ulid.js'; + +describe('slotToEventId', () => { + it('mints a fixed-width id whose string order is slot order', () => { + const ids = [1, 2, 9, 10, 99, 100, 1000].map(slotToEventId); + for (const id of ids) { + expect(id).toHaveLength(EVENT_ID_PREFIX.length + EVENT_ID_BODY_LENGTH); + } + expect([...ids].sort()).toEqual(ids); + }); + + it('round-trips through eventIdToSlot', () => { + for (const slot of [FIRST_EVENT_SLOT, 7, 12345, Number.MAX_SAFE_INTEGER]) { + expect(eventIdToSlot(slotToEventId(slot))).toBe(slot); + } + }); + + it('refuses slots it cannot represent exactly', () => { + expect(() => slotToEventId(0)).toThrow(RangeError); + expect(() => slotToEventId(-1)).toThrow(RangeError); + expect(() => slotToEventId(1.5)).toThrow(RangeError); + expect(() => slotToEventId(Number.MAX_SAFE_INTEGER + 2)).toThrow( + RangeError + ); + }); +}); + +describe('isSlotEventId', () => { + it('reads an id however it is prefixed', () => { + const body = String(42).padStart(EVENT_ID_BODY_LENGTH, '0'); + expect(isSlotEventId(`evnt_${body}`)).toBe(true); + expect(isSlotEventId(`wevt_${body}`)).toBe(true); + expect(isSlotEventId(body)).toBe(true); + expect(eventIdToSlot(`wevt_${body}`)).toBe(42); + }); + + it('never mistakes a ULID for a slot', () => { + for (let i = 0; i < 100; i++) { + const id = ulid(); + expect(isSlotBody(id)).toBe(false); + expect(eventIdToSlot(`evnt_${id}`)).toBeNull(); + } + }); + + it('rejects bodies of the wrong shape', () => { + // Right length, but the timestamp region is not all zeros. + expect(isSlotBody('0000000001'.padEnd(EVENT_ID_BODY_LENGTH, '0'))).toBe( + false + ); + // Right prefix of zeros, but a non-digit in the counter region. + expect(isSlotBody('0'.repeat(EVENT_ID_BODY_LENGTH - 1) + 'A')).toBe(false); + // Wrong length. + expect( + isSlotBody('0'.repeat(EVENT_ID_BODY_LENGTH - 1) + '1'.repeat(2)) + ).toBe(false); + expect(isSlotBody('')).toBe(false); + }); +}); + +describe('time is never derived from a slot id', () => { + it('returns null rather than the epoch', () => { + // The trap this guards: `decodeTime` on a slot body succeeds and yields 0. + // A caller that took that at face value would date every event to 1970. + const body = slotToEventId(1).slice(EVENT_ID_PREFIX.length); + expect(ulidToDate(body)).toBeNull(); + expect( + ulidToDate(slotToEventId(999_999).slice(EVENT_ID_PREFIX.length)) + ).toBeNull(); + }); + + it('still decodes a real ULID', () => { + const id = ulid(); + expect(ulidToDate(id)?.getTime()).toBeGreaterThan(0); + }); + + it('fails validation instead of reporting 56 years of drift', () => { + const slotAsRunId = `wrun_${slotToEventId(1).slice(EVENT_ID_PREFIX.length)}`; + expect(validateUlidTimestamp(slotAsRunId, 'wrun_')).toMatch( + /is not a valid ULID/ + ); + }); +}); diff --git a/packages/world/src/slot-identity.ts b/packages/world/src/slot-identity.ts new file mode 100644 index 0000000000..a42283bba3 --- /dev/null +++ b/packages/world/src/slot-identity.ts @@ -0,0 +1,100 @@ +/** + * Slot-based event identity. + * + * An event id is `evnt_` followed by 26 characters. Historically that body was + * a ULID; a World that allocates *slots* instead writes the event's dense + * 1-based position in its run's log, as a zero-padded decimal. + * + * The padding is what makes this a drop-in change rather than a format break. + * Decimal digits are a subset of Crockford base32, and every id is still + * exactly 26 characters, so existing ULID validators accept a slot id, + * lexicographic ordering still matches creation order (fixed width, so string + * order is numeric order), and `eid:` cursors and range fences keep working + * untouched. + * + * The one thing that does *not* survive: a slot id's leading characters are + * zeros, so decoding it as a ULID timestamp yields the Unix epoch. Nothing may + * derive a time from an event id without first ruling out a slot id — see + * {@link isSlotBody} and the guard in `ulidToDate`. + */ + +/** Characters in an event id body, ULID or slot alike. */ +export const EVENT_ID_BODY_LENGTH = 26; + +/** + * Leading zeros a slot body must carry. + * + * This is the discriminator against a ULID: a ULID's first 10 characters + * encode milliseconds since the epoch, and `ulid()` never mints a zero + * timestamp. Requiring the same 10 characters to be `0` therefore separates + * the two schemes with no ambiguity, and caps a slot at 10^16 - 1 — far above + * any run's event count, and reduced further below to stay in safe-integer + * range. + */ +const SLOT_LEADING_ZEROS = 10; + +/** First slot in a run's log. Slots are 1-based and dense. */ +export const FIRST_EVENT_SLOT = 1; + +/** + * Largest representable slot. Bounded by JavaScript's safe-integer range + * rather than by the 16 significant digits the format allows, so a parsed slot + * is always exact. + */ +export const MAX_EVENT_SLOT = Number.MAX_SAFE_INTEGER; + +/** Canonical prefix for event ids. */ +export const EVENT_ID_PREFIX = 'evnt_'; + +/** + * Whether a 26-character event id *body* is a slot rather than a ULID. + * + * Takes the body, not the prefixed id, because the same test applies to event + * ids however they are spelled (`evnt_`, the legacy `wevt_`, or bare). + */ +export function isSlotBody(body: string): boolean { + if (body.length !== EVENT_ID_BODY_LENGTH) return false; + for (let i = 0; i < SLOT_LEADING_ZEROS; i++) { + if (body[i] !== '0') return false; + } + for (let i = SLOT_LEADING_ZEROS; i < EVENT_ID_BODY_LENGTH; i++) { + const code = body.charCodeAt(i); + if (code < 48 || code > 57) return false; + } + return true; +} + +/** Strips a `_` from an event id, if present. */ +function stripEventIdPrefix(eventId: string): string { + const underscore = eventId.indexOf('_'); + return underscore === -1 ? eventId : eventId.slice(underscore + 1); +} + +/** Whether a (possibly prefixed) event id is slot-numbered. */ +export function isSlotEventId(eventId: string): boolean { + return isSlotBody(stripEventIdPrefix(eventId)); +} + +/** + * Formats a slot as a prefixed event id. + * + * @throws if the slot is outside the representable range — a caller that + * overflows must fail loudly rather than mint an id that sorts wrong. + */ +export function slotToEventId(slot: number): string { + if (!Number.isSafeInteger(slot) || slot < FIRST_EVENT_SLOT) { + throw new RangeError(`Invalid event slot: ${slot}`); + } + return `${EVENT_ID_PREFIX}${String(slot).padStart(EVENT_ID_BODY_LENGTH, '0')}`; +} + +/** + * Reads the slot out of a (possibly prefixed) event id, or null when the id is + * not slot-numbered. + */ +export function eventIdToSlot(eventId: string): number | null { + const body = stripEventIdPrefix(eventId); + if (!isSlotBody(body)) return null; + const slot = Number(body); + return Number.isSafeInteger(slot) && slot >= FIRST_EVENT_SLOT ? slot : null; +} diff --git a/packages/world/src/spec-version.test.ts b/packages/world/src/spec-version.test.ts index 880c43180d..da9d9a3f79 100644 --- a/packages/world/src/spec-version.test.ts +++ b/packages/world/src/spec-version.test.ts @@ -4,8 +4,10 @@ import { requiresNewerWorld, SPEC_VERSION_CURRENT, SPEC_VERSION_LEGACY, + SPEC_VERSION_MAX_SUPPORTED, SPEC_VERSION_SUPPORTS_ATTRIBUTES, SPEC_VERSION_SUPPORTS_COMPRESSION, + SPEC_VERSION_SUPPORTS_SLOT_IDENTITY, } from './spec-version.js'; describe('spec version constants', () => { @@ -13,6 +15,19 @@ describe('spec version constants', () => { expect(SPEC_VERSION_CURRENT).toBe(SPEC_VERSION_SUPPORTS_COMPRESSION); expect(SPEC_VERSION_SUPPORTS_COMPRESSION).toBe(5); }); + + it('the readable ceiling is the slot-identity version', () => { + // The default a World stamps and the highest version this SDK can read + // are separate dials. Slot identity is above the default on purpose: only + // a World that actually allocates slots opts into it. + expect(SPEC_VERSION_SUPPORTS_SLOT_IDENTITY).toBe(6); + expect(SPEC_VERSION_MAX_SUPPORTED).toBe( + SPEC_VERSION_SUPPORTS_SLOT_IDENTITY + ); + expect(SPEC_VERSION_MAX_SUPPORTED).toBeGreaterThanOrEqual( + SPEC_VERSION_CURRENT + ); + }); }); describe('requiresNewerWorld', () => { @@ -24,13 +39,20 @@ describe('requiresNewerWorld', () => { expect(requiresNewerWorld(null)).toBe(false); }); - it('rejects runs newer than the current spec version', () => { + it('accepts a slot-identity run even though it is above the default', () => { + // world-vercel stamps this version on the runs it creates. Testing + // against SPEC_VERSION_CURRENT instead of the ceiling would make this SDK + // reject the runs its own adapter just wrote. + expect(requiresNewerWorld(SPEC_VERSION_SUPPORTS_SLOT_IDENTITY)).toBe(false); + }); + + it('rejects runs newer than the highest supported spec version', () => { // This is the contract that protects older SDKs from compressed // payloads they cannot decode: a spec-5 run read by an SDK whose - // SPEC_VERSION_CURRENT is 4 fails this check up front (with - // RunNotSupportedError at the storage layer) instead of failing on - // individual compressed payloads. - expect(requiresNewerWorld(SPEC_VERSION_CURRENT + 1)).toBe(true); + // ceiling is 4 fails this check up front (with RunNotSupportedError at + // the storage layer) instead of failing on individual compressed + // payloads. + expect(requiresNewerWorld(SPEC_VERSION_MAX_SUPPORTED + 1)).toBe(true); }); it('simulates a v4 reader rejecting a compression-era run', () => { diff --git a/packages/world/src/spec-version.ts b/packages/world/src/spec-version.ts index cf516b772b..21112c7553 100644 --- a/packages/world/src/spec-version.ts +++ b/packages/world/src/spec-version.ts @@ -31,13 +31,51 @@ export const SPEC_VERSION_SUPPORTS_ATTRIBUTES = 4 as SpecVersion; */ export const SPEC_VERSION_SUPPORTS_COMPRESSION = 5 as SpecVersion; +/** + * Runs at this spec version get slot-numbered event ids: `evnt_` followed by a + * zero-padded decimal position, dense and contiguous from 1 within one run. + * + * This exists for Worlds that cannot read a run's scheme off its own storage. + * `world-local` and `world-postgres` own the counter that mints the ids, so + * they know per run which scheme it started under. `world-vercel` writes + * through an API whose allocator has to make that decision on each request, + * and the spec version stamped on `run_created` is what carries it. A run + * created before the backend adopted slots stays on ULIDs for its whole life + * because its stamped version is below this one. + */ +export const SPEC_VERSION_SUPPORTS_SLOT_IDENTITY = 6 as SpecVersion; + /** * Current spec version (event-sourced architecture with native attributes * and compressed payloads). + * + * Deliberately NOT bumped for slot-numbered event ids. Slot numbering is a + * property of a run's whole log rather than of an individual event, and it is + * already self-describing: a run's scheme is readable from the shape of its + * own first event id (see `isSlotEventId`), so a World that owns its own id + * allocation needs no version negotiation to pin one. Bumping this constant + * would stamp the new version on every World + * including ones that have not adopted slots yet, which is exactly the + * cross-version breakage the pin exists to avoid. A World that does allocate + * slots declares the higher version itself (see `world-vercel`), and + * `SPEC_VERSION_MAX_SUPPORTED` is what keeps this reader from rejecting the + * runs it produces. */ export const SPEC_VERSION_CURRENT = SPEC_VERSION_SUPPORTS_COMPRESSION as SpecVersion; +/** + * The highest spec version this SDK can read. + * + * Distinct from `SPEC_VERSION_CURRENT`, which is the *default* a World stamps + * on runs it creates. A World may declare a higher version than the default, + * so the "was this run made by a newer SDK?" test has to be against the + * ceiling: comparing against the default would make the SDK reject runs its + * own adapters just created. + */ +export const SPEC_VERSION_MAX_SUPPORTED = + SPEC_VERSION_SUPPORTS_SLOT_IDENTITY as SpecVersion; + /** * Check if a spec version is legacy (<= SPEC_VERSION_LEGACY or undefined). * Legacy runs require different handling - they use direct entity mutation @@ -55,7 +93,7 @@ export function isLegacySpecVersion(v: number | undefined | null): boolean { } /** - * Check if a spec version requires a newer world (> SPEC_VERSION_CURRENT). + * Check if a spec version requires a newer world (> SPEC_VERSION_MAX_SUPPORTED). * This happens when a run was created by a newer SDK version. * * @param v - The spec version number, or undefined/null for legacy runs @@ -63,5 +101,5 @@ export function isLegacySpecVersion(v: number | undefined | null): boolean { */ export function requiresNewerWorld(v: number | undefined | null): boolean { if (v === undefined || v === null) return false; - return v > SPEC_VERSION_CURRENT; + return v > SPEC_VERSION_MAX_SUPPORTED; } diff --git a/packages/world/src/ulid.ts b/packages/world/src/ulid.ts index 1ee1b7b47c..3ba1df2c99 100644 --- a/packages/world/src/ulid.ts +++ b/packages/world/src/ulid.ts @@ -1,5 +1,6 @@ import { decodeTime } from 'ulid'; import { z } from 'zod'; +import { isSlotBody } from './slot-identity.js'; const UlidSchema = z.string().ulid(); @@ -36,8 +37,18 @@ export const DEFAULT_TIMESTAMP_THRESHOLD_MS = /** * Extracts a Date from a ULID string, or null if the string is not a valid ULID. + * + * A slot-numbered event id is syntactically a valid ULID (26 zero-padded + * decimal digits are all Crockford characters) whose timestamp component is + * zero, so decoding one would silently yield the Unix epoch. Slots carry no + * time at all, so this returns null for them and callers fall back to a real + * `createdAt`. See `slot-identity.ts`. */ export function ulidToDate(maybeUlid: string): Date | null { + if (isSlotBody(maybeUlid)) { + return null; + } + const ulid = UlidSchema.safeParse(maybeUlid); if (!ulid.success) { return null; diff --git a/workbench/fastify/.gitignore b/workbench/fastify/.gitignore index 107101d13a..4f99d9f581 100644 --- a/workbench/fastify/.gitignore +++ b/workbench/fastify/.gitignore @@ -25,3 +25,6 @@ vite.config.ts.timestamp-* # Workflows _workflows.ts /.swc + +# Copied from index.html by `copy:index` so nitro can serve it statically +/public/index.html diff --git a/workbench/fastify/package.json b/workbench/fastify/package.json index 48758cad2a..f52cd7dfc9 100644 --- a/workbench/fastify/package.json +++ b/workbench/fastify/package.json @@ -7,7 +7,7 @@ "main": "index.js", "scripts": { "generate:workflows": "node ../scripts/generate-workflows-registry.js", - "predev": "pnpm generate:workflows", + "predev": "pnpm generate:workflows && pnpm copy:index", "copy:index": "mkdir -p public && cp index.html public/index.html", "prebuild": "pnpm generate:workflows && pnpm copy:index", "postbuild": "rm -f public/index.html", diff --git a/workbench/nextjs-turbopack/vercel.json b/workbench/nextjs-turbopack/vercel.json index 22da2a3327..ff2e7944b0 100644 --- a/workbench/nextjs-turbopack/vercel.json +++ b/workbench/nextjs-turbopack/vercel.json @@ -5,8 +5,7 @@ } }, "env": { - "WORKFLOW_PUBLIC_MANIFEST": "1", - "WORKFLOW_PER_KIND_CORRELATION_IDS": "1" + "WORKFLOW_PUBLIC_MANIFEST": "1" }, "regions": [ "iad1",