From 392953f2874dc1ce19a97995852da0ff4d3a7429 Mon Sep 17 00:00:00 2001 From: Peter Wielander Date: Wed, 12 Aug 2026 18:44:35 -0700 Subject: [PATCH 1/6] [core] Drop pre-slot event ids, the preconditionGuard capability, and resilient dispatch by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three removals that stand on their own, split out of the v5 API switch. **Pre-slot event ids.** `maxEventSlot`, `findEventSlotGap`, and the step executor's slot observer went through a lenient decoder that answered "no position" for an id that is not a slot. That leniency *was* the pre-slot support, and it is the wrong shape now: a write whose `eventCount` is absent is indistinguishable, to a World, from one that honestly loaded nothing. They call `requireEventSlot` and throw. Skew protection is what makes it safe on Vercel — a run executes on the deployment that created it, so a build carrying this never replays a run created before slot ids. **The `preconditionGuard` capability.** Every World is now assumed to be able to refuse a stale replay-context write, so the three behaviors that keyed on the flag apply unconditionally: the per-step inline event-log delta stays enabled while the run has an open hook, an inline step's `step_started` claim is awaited before the body runs, and resilient dispatch stops consulting it. Before this only world-vercel declared it, so for world-local and world-postgres this is a behavior change rather than a no-op — they now pay the await-then-run claim while a hook is open, and get the inline delta in that same case. **`WORKFLOW_RESILIENT_STEP_DISPATCH`.** Off by default, `=1` to opt in. The publish races the create's verdict: a World that refuses the `step_created` sends the runtime back to replay while the payload-carrying message is already out, and nothing orders the refusal before the consumer's redelivery re-ensure. It was gated on the capability that no longer exists, and was already off for world-vercel, which declared it. `@workflow/world-sim` moves to slot ids with the runtime, since it drives the real one. The book's mint-ordered count goes 35/6/6 to 38/3/3 and append-only stays 41/0/0: four of the six reds staged a read missing an event the log already held, which under slots is a gap the runtime re-reads past. Also deletes `.changeset/windows-preload-timeout.md`, committed here by mistake. Co-Authored-By: Claude Opus 5 --- .../drop-precondition-guard-capability.md | 6 + .changeset/require-slot-event-ids.md | 6 + .changeset/resilient-step-dispatch-off.md | 5 + .changeset/windows-preload-timeout.md | 5 - .github/workflows/world-sim.yml | 10 +- .../precondition-failed-error.mdx | 2 +- .../docs/v5/configuration/runtime-tuning.mdx | 16 +- docs/content/worlds/v5/building-a-world.mdx | 5 +- packages/core/src/retained-vm-loop.test.ts | 3 +- packages/core/src/runtime.test.ts | 70 +++-- packages/core/src/runtime.ts | 69 ++--- packages/core/src/runtime/constants.ts | 12 +- packages/core/src/runtime/helpers.test.ts | 147 ++++------- packages/core/src/runtime/helpers.ts | 76 +++--- .../resume-hook.consumer-preload.test.ts | 4 +- .../runtime/resume-latency.runtime.test.ts | 7 +- packages/core/src/runtime/step-executor.ts | 23 +- .../src/runtime/suspension-handler.test.ts | 90 ++----- .../core/src/runtime/suspension-handler.ts | 25 +- .../runtime/wait-completion-replay.test.ts | 16 +- packages/world-sim/DESIGN.md | 155 ++++++----- packages/world-sim/src/ids.test.ts | 8 +- packages/world-sim/src/ids.ts | 44 +--- packages/world-sim/src/invariants.test.ts | 9 +- packages/world-sim/src/replay.test.ts | 3 +- packages/world-sim/src/scenario.ts | 2 +- packages/world-sim/src/store.test.ts | 35 +-- packages/world-sim/src/store.ts | 244 ++++++++++++++---- packages/world-sim/src/world.ts | 54 ++-- packages/world-vercel/src/index.ts | 11 - packages/world/src/index.ts | 1 + packages/world/src/interfaces.ts | 26 -- packages/world/src/slot-identity.ts | 24 ++ workbench/sim-world/README.md | 10 +- .../in-flight-before-decision-counted.ts | 57 ++-- .../scenarios/in-flight-before-decision.ts | 9 +- 36 files changed, 639 insertions(+), 650 deletions(-) create mode 100644 .changeset/drop-precondition-guard-capability.md create mode 100644 .changeset/require-slot-event-ids.md create mode 100644 .changeset/resilient-step-dispatch-off.md delete mode 100644 .changeset/windows-preload-timeout.md diff --git a/.changeset/drop-precondition-guard-capability.md b/.changeset/drop-precondition-guard-capability.md new file mode 100644 index 0000000000..3a71e9ece7 --- /dev/null +++ b/.changeset/drop-precondition-guard-capability.md @@ -0,0 +1,6 @@ +--- +'@workflow/world': patch +'@workflow/core': patch +--- + +Remove the `preconditionGuard` World capability. Every World is now assumed to be able to reject a stale replay-context write, so the behaviors that keyed on the flag apply everywhere. diff --git a/.changeset/require-slot-event-ids.md b/.changeset/require-slot-event-ids.md new file mode 100644 index 0000000000..bcb8974eb4 --- /dev/null +++ b/.changeset/require-slot-event-ids.md @@ -0,0 +1,6 @@ +--- +'@workflow/world': patch +'@workflow/core': patch +--- + +Require every event id the runtime reads to be a log position. `requireEventSlot` replaces the lenient decode that returned "no position" for an id that is not a slot. diff --git a/.changeset/resilient-step-dispatch-off.md b/.changeset/resilient-step-dispatch-off.md new file mode 100644 index 0000000000..0f5e4ea322 --- /dev/null +++ b/.changeset/resilient-step-dispatch-off.md @@ -0,0 +1,5 @@ +--- +'@workflow/core': patch +--- + +Turn resilient step dispatch off by default. Set `WORKFLOW_RESILIENT_STEP_DISPATCH=1` to opt back in. diff --git a/.changeset/windows-preload-timeout.md b/.changeset/windows-preload-timeout.md deleted file mode 100644 index 2665fe54af..0000000000 --- a/.changeset/windows-preload-timeout.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@workflow/world-local': patch ---- - -Raise the timeout on the world-local complete-preload test so it stops failing on the Windows CI runner. diff --git a/.github/workflows/world-sim.yml b/.github/workflows/world-sim.yml index df62b815f7..8587ef4282 100644 --- a/.github/workflows/world-sim.yml +++ b/.github/workflows/world-sim.yml @@ -3,20 +3,20 @@ name: World Sim # Plays the deterministic scenario book (`workbench/sim-world`) against the # runtime in this commit, once per log world, and publishes the two summaries. # -# This lane never blocks a merge, by design. Six scenarios in the book fail on +# This lane never blocks a merge, by design. Three scenarios in the book fail on # purpose: each one is a reproduction of a corruption the runtime can still # produce, stating the outcome its own durable log implies, and staying red # until the runtime gets there. A gate that goes red on every PR is a gate # everyone learns to ignore, so the job publishes numbers instead of verdicts — # and the number to watch is in the comment, not the check mark. # -# mint-ordered (production): 35 passed, 6 failed, 6 violations +# mint-ordered (production): 38 passed, 3 failed, 3 violations # append-only: 41 passed, 0 failed, 0 violations # -# A seventh red is a regression. Five means something got fixed and a scenario +# A fourth red is a regression. Two means something got fixed and a scenario # is ready to retire. The append-only column is the measurement the pair exists -# for: it says which of the six would close if event positions were assigned at -# commit instead of at the handler's mint. +# for: it says which of the three would close if event positions were assigned +# at commit instead of at the handler's mint. on: push: diff --git a/docs/content/docs/v5/api-reference/workflow-errors/precondition-failed-error.mdx b/docs/content/docs/v5/api-reference/workflow-errors/precondition-failed-error.mdx index fc4a994e4f..d1d1208735 100644 --- a/docs/content/docs/v5/api-reference/workflow-errors/precondition-failed-error.mdx +++ b/docs/content/docs/v5/api-reference/workflow-errors/precondition-failed-error.mdx @@ -10,7 +10,7 @@ related: `PreconditionFailedError` is thrown by world implementations when an event creation is rejected because the client's event-log snapshot is stale: the log already held more events than the position the creation named. It corresponds to HTTP 412 Precondition Failed semantics. -This only occurs against a world that fences on that position (`capabilities.preconditionGuard` — see [Stale-write rejection](/docs/configuration/runtime-tuning#stale-write-rejection)); event creations that carry no position are never rejected with this error. +This only occurs against a world that fences on that position (see [Stale-write rejection](/docs/configuration/runtime-tuning#stale-write-rejection)); event creations that carry no position are never rejected with this error. A world rejects only on evidence and accepts the creation whenever it cannot decide, so this error always means the snapshot really was stale — but not receiving it does not prove the snapshot was current. diff --git a/docs/content/docs/v5/configuration/runtime-tuning.mdx b/docs/content/docs/v5/configuration/runtime-tuning.mdx index 352a53337f..ac040caab9 100644 --- a/docs/content/docs/v5/configuration/runtime-tuning.mdx +++ b/docs/content/docs/v5/configuration/runtime-tuning.mdx @@ -73,22 +73,22 @@ For example, a workflow can run a 10-minute inline step even with `WORKFLOW_REPL ### `WORKFLOW_RESILIENT_STEP_DISPATCH` -- Default: enabled +- Default: disabled - When a suspension hands newly created steps to the queue, the runtime publishes each step's execution message in parallel with its `step_created` event write instead of sequencing them, cutting a round trip per dispatched step. The message also carries the serialized step input (`stepInput`), so a transient `step_created` write failure (429 / 5xx / transport) still executes the step — the queue consumer idempotently re-ensures the event before running it, converging with the producer's write on the step's correlation ID. This mirrors resilient start (`runInput`) and the lazy hook resume (`hookInput`). -- The runtime falls back to the sequential create-then-publish dispatch automatically when the step input is too large to inline on the queue message, when the run's queue transport cannot carry binary payloads (pre-CBOR spec versions), or — on the `node` VM engine, whose suspension writes are replay-context writes — when the World can [reject a write as stale](#stale-write-rejection) (`capabilities.preconditionGuard`; the Vercel World declares it): a rejected `step_created` must not be materializable through the queue side-channel, and only sequencing the publish after the create gives the message a happens-after edge over the create's verdict. The `quickjs` engine's suspension writes are not replay-context writes, so it uses resilient dispatch against every World. +- It is off by default because the publish races the create's verdict. A World that [rejects the `step_created` as stale](#stale-write-rejection) sends the runtime back to replay from a corrected log, but the message carrying the payload is already out, so the consumer can materialize a step the World refused. Nothing orders the create's refusal before the consumer's redelivery re-ensure, so the sequential path is the only one that gives the message a happens-after edge over the create's verdict. +- Even when enabled, the runtime falls back to the sequential create-then-publish dispatch when the step input is too large to inline on the queue message, or when the run's queue transport cannot carry binary payloads (pre-CBOR spec versions). - Producer-side recoveries are reported on the suspension span as `workflow.step.resilient_dispatch_recovered`; a consumer that materialized the event reports `workflow.step.resilient_dispatch_materialized`. -- Set `0` to force the sequential dispatch as a kill switch. +- Set `1` to enable it. ### Stale-write rejection -- Not a variable: this is what a World declaring `capabilities.preconditionGuard` does, and what the runtime does about it. +- Not a variable: this is what a World that fences on the replayed-from position does, and what the runtime does about it. The runtime assumes any World may. - A replay-context event creation names the position it replayed from (`eventCount`, the number of events the replay had loaded), and a World that fences on it rejects the creation with 412 ([`PreconditionFailedError`](/docs/api-reference/workflow-errors/precondition-failed-error)) when the log already held more than that. The position is derived from the run's event IDs, so it is sent only for a run whose World numbers events by position; a run on the older ID scheme, and any caller with no loaded log to be stale against, sends none and is never rejected. - On rejection the runtime restarts the replay in the same invocation from a corrected event log, and falls back to a re-invocation with a fresh replay once the restart budget is spent. The rejected write is never retried as-is: a replay working from a corrected log derives different events, so only a fresh replay may write again. -- Against a fencing World the runtime also keeps the per-step event-log delta optimization (consuming the delta returned by a step's terminal write instead of issuing an extra `events.list` per step) active while the run has an open hook. Without a fence, an open hook disables it. -- While a hook is open on a fencing World, inline steps take the await-then-run path even when optimistic inline start is enabled: the step's `step_started` claim is awaited before the body runs, so a claim rejected as stale never executes user code. -- Worlds that do not fence ignore the position and must not declare the capability, so the dependent optimizations stay off against them. +- The runtime keeps the per-step event-log delta optimization (consuming the delta returned by a step's terminal write instead of issuing an extra `events.list` per step) active while the run has an open hook, because a `hook_received` missed by the delta window is what a fence rejects. +- While a hook is open, inline steps take the await-then-run path even when optimistic inline start is enabled: the step's `step_started` claim is awaited before the body runs, so a claim rejected as stale never executes user code. - A fence only ever rejects on evidence, and fails open in every other case: a World that cannot decide must accept the write. A rejection therefore always means the position really was stale, but the absence of one does not prove it was current. -- The Vercel World declares the capability. It does not fence a run that uses [slot-numbered event IDs](/docs/how-it-works/event-sourcing#event-ids), because such a run has no position to reject: the World assigns each event its slot at commit time and reports back the slots the write skipped over. +- The Vercel World does not fence a run that uses [slot-numbered event IDs](/docs/how-it-works/event-sourcing#event-ids), because such a run has no position to reject: the World assigns each event its slot at commit time and reports back the slots the write skipped over. ### `WORKFLOW_SLOT_GAP_CHECK` diff --git a/docs/content/worlds/v5/building-a-world.mdx b/docs/content/worlds/v5/building-a-world.mdx index 757ac23a7a..3207705379 100644 --- a/docs/content/worlds/v5/building-a-world.mdx +++ b/docs/content/worlds/v5/building-a-world.mdx @@ -37,7 +37,6 @@ interface WorldCapabilities { active: boolean; }; slotEventIds?: boolean; - preconditionGuard?: boolean; } interface World extends Storage, Queue, Streamer { @@ -49,7 +48,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, `slotEventIds` when it allocates [slot-numbered event IDs](#event-id-allocation), and `preconditionGuard` when it can [reject a stale write](#optional-rejecting-a-stale-write). 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, and `slotEventIds` when it allocates [slot-numbered event IDs](#event-id-allocation). 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 @@ -132,7 +131,7 @@ Two rules make this safe: A rejection may optionally carry the events the caller was missing, as `{ events, cursor }` on the error's `details`. Only include them when you can prove the set is complete — that those events fully account for the discrepancy and are not truncated — and that every one of them belongs to the run being written. The runtime merges them straight into the replay's event log, so anything else there is worse than no delta at all. Otherwise omit them, and the runtime performs a full reload instead. -Declare `capabilities.preconditionGuard` if your World can refuse a write this way. The runtime reads it as "a write can come back refused", not as a promise that any particular one will be, and three behaviors key on it: the per-step event-log delta optimization stays enabled while the run has an open hook, an inline step's `step_started` claim is awaited before the body runs, and a `step_created` publish is sequenced after the create on the `node` VM engine. A World that accepts `eventCount` and ignores it must leave the capability unset — sending a position is not the same as one being enforced. +The runtime assumes any World may refuse a write this way, and shapes three behaviors around it: the per-step event-log delta optimization stays enabled while the run has an open hook, an inline step's `step_started` claim is awaited before the body runs, and a `step_created` publish is sequenced after the create on the `node` VM engine. A World that accepts `eventCount` and ignores it pays those costs without the benefit, which is another reason to prefer reporting the skipped events over rejecting. ## Queue Interface diff --git a/packages/core/src/retained-vm-loop.test.ts b/packages/core/src/retained-vm-loop.test.ts index 39ff8f64e6..b7a88a979f 100644 --- a/packages/core/src/retained-vm-loop.test.ts +++ b/packages/core/src/retained-vm-loop.test.ts @@ -2,6 +2,7 @@ import { PreconditionFailedError } from '@workflow/errors'; import { type Event, SPEC_VERSION_CURRENT, + slotToEventId, type WorkflowRun, } from '@workflow/world'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -225,7 +226,7 @@ async function drive( return { run, events }; } const event = { - eventId: `e-${++seq}`, + eventId: slotToEventId(++seq), runId, createdAt: new Date(), ...data, diff --git a/packages/core/src/runtime.test.ts b/packages/core/src/runtime.test.ts index 108b07d870..18bf614eea 100644 --- a/packages/core/src/runtime.test.ts +++ b/packages/core/src/runtime.test.ts @@ -105,7 +105,7 @@ async function runWorkflowHandlerWithEvents( } const event = { - eventId: `event-${createdEvents.length}`, + eventId: slotToEventId(createdEvents.length), runId: workflowRun.runId, createdAt: new Date(), ...data, @@ -393,7 +393,7 @@ describe('workflowEntrypoint replay guards', () => { createdEvents.push(data); return { event: { - eventId: `event-${createdEvents.length}`, + eventId: slotToEventId(createdEvents.length), runId: 'wrun_schema_validation', createdAt: new Date(), ...data, @@ -492,7 +492,7 @@ describe('workflowEntrypoint replay guards', () => { ? { run: workflowRun } : { event: { - eventId: `event-${createdEvents.length}`, + eventId: slotToEventId(createdEvents.length), runId: workflowRun.runId, createdAt: new Date(), ...data, @@ -590,7 +590,7 @@ describe('workflowEntrypoint replay guards', () => { ? { run: workflowRun } : { event: { - eventId: `event-${createdEvents.length}`, + eventId: slotToEventId(createdEvents.length), runId: workflowRun.runId, createdAt: new Date(), ...data, @@ -670,7 +670,7 @@ describe('workflowEntrypoint replay guards', () => { createdEvents.push(data); return { event: { - eventId: `event-${createdEvents.length}`, + eventId: slotToEventId(createdEvents.length), runId: 'wrun_parse', createdAt: new Date(), ...data, @@ -755,7 +755,7 @@ describe('workflowEntrypoint replay guards', () => { }; const events: Event[] = [ { - eventId: 'event-foreign-failed', + eventId: slotToEventId(1), runId: 'wrun_other', eventType: 'run_failed', eventData: { @@ -798,7 +798,7 @@ describe('workflowEntrypoint replay guards', () => { const events: Event[] = [ { - eventId: 'event-0', + eventId: slotToEventId(1), runId: workflowRun.runId, eventType: 'wait_created', correlationId: 'wait_01HK153X00VFKAJV9XFN9JXXRS', @@ -808,7 +808,7 @@ describe('workflowEntrypoint replay guards', () => { createdAt: new Date('2024-01-01T00:00:00.000Z'), }, { - eventId: 'event-1', + eventId: slotToEventId(2), runId: workflowRun.runId, eventType: 'wait_completed', correlationId: 'wait_01HK153X00VFKAJV9XFN9JXXRS', @@ -841,7 +841,7 @@ describe('workflowEntrypoint replay guards', () => { expect(queueCalls.map((c) => c.message)).toContainEqual( expect.objectContaining({ replayDivergence: { - eventId: 'event-0', + eventId: slotToEventId(1), count: 1, }, }) @@ -958,7 +958,7 @@ describe('workflowEntrypoint replay guards', () => { // matches no hook' below). const events: Event[] = [ { - eventId: 'event-0', + eventId: slotToEventId(1), runId: workflowRun.runId, eventType: 'hook_created', correlationId: 'hook_01HK153X00VFKAJV9XFN9JXXRS', @@ -989,7 +989,7 @@ describe('workflowEntrypoint replay guards', () => { ); expect(queueCalls.map((c) => c.message)).toContainEqual( expect.objectContaining({ - replayDivergence: { eventId: 'event-0', count: 1 }, + replayDivergence: { eventId: slotToEventId(1), count: 1 }, }) ); }); @@ -1018,7 +1018,7 @@ describe('workflowEntrypoint replay guards', () => { // the run. const events: Event[] = [ { - eventId: 'event-0', + eventId: slotToEventId(1), runId: workflowRun.runId, eventType: 'hook_received', correlationId: 'hook_01HK153X00VFKAJV9XFN9JXXRS', @@ -1166,7 +1166,7 @@ describe('workflowEntrypoint replay guards', () => { createdEvents.push(data); return { event: { - eventId: `event-${createdEvents.length}`, + eventId: slotToEventId(createdEvents.length), runId: workflowRun.runId, createdAt: new Date(), ...data, @@ -1276,7 +1276,7 @@ describe('workflowEntrypoint replay guards', () => { createdEvents.push(data); return { event: { - eventId: `event-${createdEvents.length}`, + eventId: slotToEventId(createdEvents.length), runId: workflowRun.runId, createdAt: new Date(), ...data, @@ -1441,7 +1441,7 @@ describe('workflowEntrypoint step-dispatch ack ordering', () => { const recordEvent = (data: any): Event => { eventSeq += 1; const created = { - eventId: `event-${eventSeq}`, + eventId: slotToEventId(eventSeq), runId: workflowRun.runId, createdAt: new Date(), ...data, @@ -1699,7 +1699,7 @@ describe('workflowEntrypoint step-dispatch ack ordering', () => { const rec = (data: any): Event => { seq += 1; const e = { - eventId: `e-${seq}`, + eventId: slotToEventId(seq), runId: workflowRun.runId, createdAt: new Date(), ...data, @@ -1851,12 +1851,13 @@ describe('workflowEntrypoint resilient step consumption (stepInput re-ensure)', deploymentId: 'test-deployment', }; - let eventSeq = 0; + // Slot 1 is taken by the seeded event below, so recorded events start at 2. + let eventSeq = 1; const durableEvents: Event[] = [ // An unrelated pending step: keeps the run un-replayable so the handler // returns right after the background step completes. { - eventId: 'event-other', + eventId: slotToEventId(1), runId: opts.runId, createdAt: new Date(), eventType: 'step_created', @@ -1868,7 +1869,7 @@ describe('workflowEntrypoint resilient step consumption (stepInput re-ensure)', const recordEvent = (data: any): Event => { eventSeq += 1; const created = { - eventId: `event-${eventSeq}`, + eventId: slotToEventId(eventSeq), runId: opts.runId, createdAt: new Date(), ...data, @@ -2203,7 +2204,7 @@ describe('workflowEntrypoint turbo mode', () => { const rec = (data: any): Event => { seq += 1; const e = { - eventId: `e-${seq}`, + eventId: slotToEventId(seq), runId, createdAt: new Date(), ...data, @@ -2512,7 +2513,7 @@ describe('workflowEntrypoint inline-delta gate with open hooks', () => { * World capabilities to declare. Absent by default — capability-gated * fast paths must fail closed without them. */ - capabilities?: { preconditionGuard?: boolean; maxConcurrency?: boolean }; + capabilities?: { maxConcurrency?: boolean }; /** Workflow source to run (defaults to hookAndStepWorkflow). */ source?: string; /** @@ -2653,15 +2654,14 @@ describe('workflowEntrypoint inline-delta gate with open hooks', () => { return call?.[2] as { sinceCursor?: string } | undefined; } - it('requests the inline delta despite the open hook when the World enforces the precondition guard', async () => { + it('requests the inline delta despite the open hook', async () => { const { res, eventsCreate } = await driveDeltaGate( - 'wrun_delta_gate_guard_on', - { capabilities: { preconditionGuard: true } } + 'wrun_delta_gate_guard_on' ); expect(res.status).toBe(204); - // The suspension created a hook (left open) and one lazy inline step — - // with an enforced guard, a hook_received missed by the delta window is - // fenced by the outside-event marker, so the fast path stays active. + // The suspension created a hook (left open) and one lazy inline step. A + // hook_received missed by the delta window is fenced by the outside-event + // marker, so the fast path stays active. expect(stepCompletedParams(eventsCreate)?.sinceCursor).toBe( 'cursor_delta_gate' ); @@ -2672,16 +2672,6 @@ describe('workflowEntrypoint inline-delta gate with open hooks', () => { ); }); - it('does not request the inline delta with an open hook when no World fence exists', async () => { - const { res, eventsCreate } = await driveDeltaGate( - 'wrun_delta_gate_guard_off' - ); - expect(res.status).toBe(204); - // Without the guard there is no fence for a hook_received landing in the - // delta window, so the conservative gate keeps the fetch path. - expect(stepCompletedParams(eventsCreate)?.sinceCursor).toBeUndefined(); - }); - it('restarts the replay in-process and still completes the run when a stale lazy claim is rejected by the guard (interleaved hook_received)', async () => { // Simulates the interleaving the fence exists for: after step A's // terminal write, an out-of-band hook_received bumps the run's marker; @@ -2690,7 +2680,6 @@ describe('workflowEntrypoint inline-delta gate with open hooks', () => { const { res, eventsCreate, queueMock } = await driveDeltaGate( 'wrun_delta_gate_stale_claim', { - capabilities: { preconditionGuard: true }, source: hookAndTwoStepWorkflow, rejectClaimOnce: { stepName: 'deltaGateStepB', @@ -2752,7 +2741,6 @@ describe('workflowEntrypoint inline-delta gate with open hooks', () => { const { res, eventsCreate } = await driveDeltaGate( 'wrun_delta_gate_stale_claim_optimistic', { - capabilities: { preconditionGuard: true }, source: hookAndTwoStepWorkflow, rejectClaimOnce: { stepName: 'deltaGateStepB', @@ -2838,7 +2826,7 @@ describe('workflowEntrypoint latency telemetry (ttfs / stso)', () => { const rec = (data: any): Event => { seq += 1; const e = { - eventId: `e-${seq}`, + eventId: slotToEventId(seq), runId, createdAt: new Date(), ...data, @@ -3122,7 +3110,7 @@ describe('workflowEntrypoint latency telemetry (ttfs / stso)', () => { const attrOccurredAt = new Date(runCreatedAtMs + 7_000); const attrEvent = { ...attrCreates[0], - eventId: 'e-attr-1', + eventId: slotToEventId(1), runId, createdAt: attrOccurredAt, occurredAt: attrOccurredAt, diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index 0f2db2550d..abdb1272a2 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -1325,14 +1325,13 @@ export function workflowEntrypoint( appendEventLog(eventLog, delta); eventLog = { ...eventLog, type: 'ready' }; } else { - // MUST be a full, cursor-less reload. The cursor filters - // by lexicographic event id while a hole is defined by - // ULID *time*: an event in the same millisecond sorts - // either side of the cursor depending on its random - // component, and an event minted in an earlier - // millisecond but committed later always sorts below it. - // An incremental load therefore heals the hole only by - // luck. + // MUST be a full, cursor-less reload. The cursor lists + // forward from the highest event id this replay holds, + // and what a stale snapshot is missing sits below it: a + // slot allocated inside a concurrent commit takes a + // position this replay had already read past. An + // incremental load starts above the hole and never + // returns it. eventLog = { type: 'loadAll' }; // The corrected log inserts the missing events BELOW the // length already scanned for payload prewarming, shifting @@ -3688,9 +3687,8 @@ export function workflowEntrypoint( // `wait_completed` landing after the delta snapshot // does not bump the outside-event marker, so nothing // fences a replay from the stale delta. - // - No open (or this-suspension-created) hook — UNLESS - // the World declares it fences stale writes - // (`capabilities.preconditionGuard`). The + // - An open (or this-suspension-created) hook is fine, + // because a World fences a stale write. The // delta snapshots the log at the step_completed // write but is consumed on the next replay, so an // out-of-band `hook_received` landing in that window @@ -3699,8 +3697,8 @@ export function workflowEntrypoint( // it. That staleness is qualitatively the same // read-to-write race the fetch path already has (an // event can land right after `events.list` returns - // and before the suspension's writes); on a fencing - // World it is also fenced: `hook_received` bumps the + // and before the suspension's writes); it is also + // fenced: `hook_received` bumps the // per-run outside-event marker, so every durable // write the stale replay attempts is rejected with // 412 — its guarded suspension creates (retried over @@ -3714,9 +3712,7 @@ export function workflowEntrypoint( // by THIS suspension are inside the delta (their // `hook_created` lands before the step-terminal // write), so only their `hook_received` responses - // are subject to the same fenced window. Without a - // fencing World there is nothing to reject a stale - // write, so keep the conservative gate. + // are subject to the same fenced window. // - With no hook or wait open at all, the only // out-of-band writer is cancellation, which is safe // to observe one iteration late. See @@ -3726,13 +3722,6 @@ export function workflowEntrypoint( // own events and the per-write delta would be partial, so // the delta is not requested (the gate below is false for // multi-step) and the next iteration does a normal fetch. - // Whether the World fences a stale write. Sending a - // snapshot is not the same as it being enforced: a - // World that does not declare the capability ignores - // what it is sent, so nothing rejects. - const guardEnforced = - world.capabilities?.preconditionGuard === true; - const requestInlineDelta = typeof eventLog.cursor === 'string' && err.stepCount === 1 && @@ -3741,32 +3730,24 @@ export function workflowEntrypoint( lazyInlineSteps.length === 1 && ownedRecoverySteps.length === 0 && !suspensionResult.waitTimeout && - !openHookWaitState.openWait && - (guardEnforced || - (err.hookCount === 0 && - !openHookWaitState.openHook)); + !openHookWaitState.openWait; // Stale-sensitive batch: a hook is open in the run (or // was created by this suspension, so its hook_received // can land any moment) — an out-of-band event can make - // the view this batch was scheduled from stale. With - // the guard in force, the fence rejects a stale - // claim's durable writes — but it cannot un-run a step - // BODY that optimistic start began before the claim - // settled. Suppress optimistic start for these batches - // (take await-then-run) so a 412-fenced step never - // executes user code at all: the fence then covers - // side effects, not just the event log. Costs one - // claim round-trip per step while a hook is open, only - // on guard-enforcing deployments. Without the guard - // nothing 412s, so suppression would buy nothing — - // stale-view exposure there is the pre-existing - // optimistic-start contract (idempotent side effects). + // the view this batch was scheduled from stale. The + // fence rejects a stale claim's durable writes, but it + // cannot un-run a step BODY that optimistic start began + // before the claim settled. Suppress optimistic start + // for these batches (take await-then-run) so a + // 412-fenced step never executes user code at all: the + // fence then covers side effects, not just the event + // log. Costs one claim round-trip per step while a hook + // is open. const suppressOptimisticStart = - guardEnforced && - (openHookWaitState.openHook || - err.hookCount > 0 || - suspensionResult.hasHookEvents); + openHookWaitState.openHook || + err.hookCount > 0 || + suspensionResult.hasHookEvents; // Turbo mode forces optimistic inline start for this // batch — but only while the run is still "clean" (a pure diff --git a/packages/core/src/runtime/constants.ts b/packages/core/src/runtime/constants.ts index a6b71eb839..f9315c50cc 100644 --- a/packages/core/src/runtime/constants.ts +++ b/packages/core/src/runtime/constants.ts @@ -240,11 +240,17 @@ export const MAX_RESILIENT_STEP_INPUT_BYTES = 128 * 1024; * event if the direct write failed transiently. Mirrors the resilient start * (`runInput`) and resilient hook resume (`hookInput`) patterns. * - * **On by default.** Disable via `WORKFLOW_RESILIENT_STEP_DISPATCH=0` to - * restore the sequential create-then-queue dispatch. + * **Off by default.** Enable via `WORKFLOW_RESILIENT_STEP_DISPATCH=1`. + * + * The queue publish races the create's verdict. A World that refuses the + * `step_created` as stale sends the runtime back to replay from a corrected + * log, but the message carrying the payload is already out, so the consumer + * can materialize a step the World refused. Nothing orders the create's + * refusal before the consumer's redelivery re-ensure, so enabling this trades + * that window for the latency the parallel publish saves. */ export function isResilientStepDispatchEnabled(): boolean { - return process.env.WORKFLOW_RESILIENT_STEP_DISPATCH !== '0'; + return process.env.WORKFLOW_RESILIENT_STEP_DISPATCH === '1'; } const warnedMaxEventsValues = new Set(); diff --git a/packages/core/src/runtime/helpers.test.ts b/packages/core/src/runtime/helpers.test.ts index 15b84ba5e0..30588d38bf 100644 --- a/packages/core/src/runtime/helpers.test.ts +++ b/packages/core/src/runtime/helpers.test.ts @@ -1,7 +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'; import { @@ -589,14 +588,8 @@ describe('loadWorkflowRunEvents', () => { }); }); -const makeUlidEvent = (time: number): Event => - ({ - eventId: `evnt_${ulid(time)}`, - runId: 'wrun_mockidnumber0001', - eventType: 'step_created', - correlationId: 'step_mock', - createdAt: new Date(time), - }) as unknown as Event; +/** An id from the scheme slots replaced: a ULID, which carries no position. */ +const UNPOSITIONED_EVENT_ID = 'evnt_01HF7YATRRC3M0F1K9Q2J8XW5B'; describe('slotSnapshotParams', () => { it('sends the highest slot the loaded log occupies', () => { @@ -627,35 +620,35 @@ describe('slotSnapshotParams', () => { expect(slotSnapshotParams([])).toEqual({}); }); - it('sends nothing for a run whose events are not slot-numbered', () => { - expect(slotSnapshotParams([makeUlidEvent(1_700_000_000_000)])).toEqual({}); - }); - - it('sends nothing when one event of the log is not a slot', () => { - // A log may not mix the two schemes. If it somehow does, the slot reading - // is meaningless, and a count derived from part of the log would understate - // the writer's position in a way the World cannot detect. + it('throws when any event of the log carries no slot', () => { + // Skipping the id instead would understate the writer's position, and the + // World cannot tell an understated position from an honest one: it would + // hand back the same events on every create for the rest of the run. const events = [ makeEvent(slotToEventId(1)), - makeUlidEvent(1_700_000_000_000), + makeEvent(UNPOSITIONED_EVENT_ID), ]; - expect(slotSnapshotParams(events)).toEqual({}); + expect(() => slotSnapshotParams(events)).toThrow(UNPOSITIONED_EVENT_ID); }); }); describe('maxEventSlot', () => { - it('is undefined for a log with no slot ids', () => { + it('is undefined for an empty log', () => { expect(maxEventSlot([])).toBeUndefined(); - expect(maxEventSlot([makeUlidEvent(1_700_000_000_000)])).toBeUndefined(); + }); + + it('throws rather than ignoring an id that carries no slot', () => { + expect(() => maxEventSlot([makeEvent(UNPOSITIONED_EVENT_ID)])).toThrow( + UNPOSITIONED_EVENT_ID + ); }); }); /** * 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. + * World's density counter: it reports a hole only where the log proves one. */ describe('findEventSlotGap', () => { const slotLog = (...slots: number[]) => @@ -703,20 +696,17 @@ describe('findEventSlotGap', () => { }); }); - it('says nothing about a log it cannot read as slots', () => { + it('says nothing about an empty log', () => { 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(); + }); + + it('throws on a log whose ids carry no position', () => { + // The check is entirely positional. An id it cannot read is a log it + // cannot judge, and passing the run as dense would be a verdict it never + // reached. + expect(() => + findEventSlotGap([...slotLog(1, 2), makeEvent(UNPOSITIONED_EVENT_ID)]) + ).toThrow(UNPOSITIONED_EVENT_ID); }); }); @@ -811,80 +801,39 @@ describe('mergeReportedEvents', () => { 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); - const second = makeUlidEvent(1_700_000_001_000); - const third = makeUlidEvent(1_700_000_002_000); - const target = [first]; + const target = [makeEvent(slotToEventId(1))]; - appendUniqueEvents(target, [second, third]); - - expect(target.map((e) => e.eventId)).toEqual([ - first.eventId, - second.eventId, - third.eventId, + appendUniqueEvents(target, [ + makeEvent(slotToEventId(2)), + makeEvent(slotToEventId(3)), ]); + + expect(target.map((e) => e.eventId)).toEqual([1, 2, 3].map(slotToEventId)); }); it('preserves the order the World returned, never re-sorting by event id', () => { - // A World's canonical order is its own: world-local orders by - // `(createdAt, eventId)` and re-mints keys so the two diverge, so an - // id-ordered re-sort here would produce an order no load would return. - const older = makeUlidEvent(1_700_000_000_000); - const newer = makeUlidEvent(1_700_000_002_000); - const middle = makeUlidEvent(1_700_000_001_000); - const target = [older, newer]; - - appendUniqueEvents(target, [middle]); - - expect(target.map((e) => e.eventId)).toEqual([ - older.eventId, - newer.eventId, - middle.eventId, - ]); + // Unlike mergeReportedEvents, this appends a page the World handed back as + // a unit. Its order is the World's answer, and a re-sort here would produce + // an order no load would return. + const target = [makeEvent(slotToEventId(1)), makeEvent(slotToEventId(3))]; + + appendUniqueEvents(target, [makeEvent(slotToEventId(2))]); + + expect(target.map((e) => e.eventId)).toEqual([1, 3, 2].map(slotToEventId)); }); it('deduplicates by event id', () => { - const first = makeUlidEvent(1_700_000_000_000); - const second = makeUlidEvent(1_700_000_001_000); + const first = makeEvent(slotToEventId(1)); + const second = makeEvent(slotToEventId(2)); const target = [first]; appendUniqueEvents(target, [first, second, second]); - expect(target.map((e) => e.eventId)).toEqual([ - first.eventId, - second.eventId, - ]); - }); - - it('keeps a same-millisecond pair in receipt order', () => { - const time = 1_700_000_000_000; - const a = makeEvent(`evnt_${ulid(time).slice(0, 10)}AAAAAAAAAAAAAAAA`); - const b = makeEvent(`evnt_${ulid(time).slice(0, 10)}ZZZZZZZZZZZZZZZZ`); - const target = [b]; - - appendUniqueEvents(target, [a]); - - expect(target.map((e) => e.eventId)).toEqual([b.eventId, a.eventId]); + expect(target.map((e) => e.eventId)).toEqual([1, 2].map(slotToEventId)); }); it('leaves the snapshot correct even when the merge is not id-ordered', () => { @@ -901,7 +850,7 @@ describe('appendUniqueEvents', () => { }); describe('preconditionEventDelta', () => { - // The run every `makeUlidEvent` belongs to. + // The run every fixture event below belongs to. const RUN_ID = 'wrun_mockidnumber0001'; const delta = (details: unknown) => preconditionEventDelta( @@ -910,7 +859,7 @@ describe('preconditionEventDelta', () => { ); it('returns the decoded events and cursor a World attached to the 412', () => { - const event = makeUlidEvent(1_700_000_000_000); + const event = makeEvent(slotToEventId(1)); expect(delta({ events: [event], cursor: 'eid:next' })).toEqual({ events: [event], @@ -919,7 +868,7 @@ describe('preconditionEventDelta', () => { }); it('returns a null cursor when the World sent events without one', () => { - const event = makeUlidEvent(1_700_000_000_000); + const event = makeEvent(slotToEventId(1)); expect(delta({ events: [event] })).toEqual({ events: [event], @@ -941,9 +890,9 @@ describe('preconditionEventDelta', () => { // The delta is merged straight into the replay's log, so a foreign event // there produces a corrupt log rather than a corrected one: the replay // consumes a correlation id for an event this run does not have. - const mine = makeUlidEvent(1_700_000_000_000); + const mine = makeEvent(slotToEventId(1)); const theirs = { - ...makeUlidEvent(1_700_000_001_000), + ...makeEvent(slotToEventId(2)), runId: 'wrun_someotherrun001', } as Event; diff --git a/packages/core/src/runtime/helpers.ts b/packages/core/src/runtime/helpers.ts index be1d867655..03c08f88c3 100644 --- a/packages/core/src/runtime/helpers.ts +++ b/packages/core/src/runtime/helpers.ts @@ -14,11 +14,11 @@ import type { World, } from '@workflow/world'; import { - eventIdToSlot, FIRST_EVENT_SLOT, getQueueTopicPrefix, HealthCheckPayloadSchema, HOOK_RESUME_INPUT_VERSION, + requireEventSlot, resolveQueueNamespace, SPEC_VERSION_CURRENT, SPEC_VERSION_LEGACY, @@ -489,15 +489,11 @@ function recordRequestedEventCursor( * same array. The set is updated alongside `target`. * * Events are appended in the order the World returned them, and are not - * re-sorted: a World's canonical order is its own, and the runtime cannot - * reproduce it from event ids alone. That is true even though the id schemes in - * this repo happen to sort correctly today — a slot-numbered run orders by id - * everywhere, and `world-local` falls back to `(createdAt, eventId)` only for a - * run minted before slots, where it re-mints keys and the two orders diverge. - * Every append source is already in canonical order - * relative to the tail (a cursor-delimited page, or a write-response delta), so - * receipt order is the order to keep. Nothing downstream may assume the tail is - * the newest event — see {@link maxEventSlot}. + * re-sorted. Every append source is already in canonical order relative to the + * tail (a cursor-delimited page, or a write-response delta), so receipt order is + * the order to keep, and re-sorting here would only cost a pass over the log. + * Nothing downstream may assume the tail is the newest event — see + * {@link maxEventSlot}. */ export function appendUniqueEvents( target: Event[], @@ -528,13 +524,10 @@ export function appendUniqueEvents( * land in `eventId` order — a plain `push` would place a late-committing * earlier event after events that sort before it, corrupting replay. * - * Lexicographic string order matches commit order under both id schemes, which - * is why this needs no slot gate the way {@link mergeReportedEvents} does: a - * slot id is a fixed-width zero-padded position, and a ULID is monotonic by - * construction. What differs is the guarantee. On a slot run the id order *is* - * the World's canonical order, while on a ULID run it is the runtime's best - * reconstruction of it, which is enough for a single splice into a page that - * was already loaded in that order. + * Lexicographic string order is the log's order: a slot id is a fixed-width + * zero-padded position, so comparing the strings compares the positions. This + * needs no parse of its own for that reason, and the comparison is exact rather + * than a reconstruction. */ export function insertEventByEventId(target: Event[], event: Event): void { // Linear scan from the end: the spliced event is almost always the newest @@ -774,10 +767,8 @@ export function isSlotGapCheckEnabled(): boolean { * * 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. + * after events they precede. Sorting by id restores the World's canonical order + * rather than guessing at it: a slot id is that order, written down. */ export function mergeReportedEvents( target: Event[], @@ -786,7 +777,7 @@ export function mergeReportedEvents( const before = target.length; appendUniqueEvents(target, events); const added = target.length - before; - if (added > 0 && maxEventSlot(target) !== undefined) { + if (added > 0) { target.sort((a, b) => a.eventId < b.eventId ? -1 : a.eventId > b.eventId ? 1 : 0 ); @@ -845,9 +836,7 @@ export function absorbSkippedSlotReport( } /** - * 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 highest slot the loaded log occupies, or `undefined` for an empty 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 @@ -860,14 +849,16 @@ export function absorbSkippedSlotReport( * * 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. + * + * @throws if any event id carries no slot. Every World the runtime replays + * against numbers events by slot, so an id that does not is a broken log rather + * than an older one, and a maximum derived by skipping it would understate the + * log to every write that reads it. */ -export function maxEventSlot(events: Event[]): number | undefined { +export function maxEventSlot(events: readonly Event[]): number | undefined { let max: number | undefined; for (const event of events) { - const slot = eventIdToSlot(event.eventId); - if (slot === null) { - return undefined; - } + const slot = requireEventSlot(event.eventId); if (max === undefined || slot > max) { max = slot; } @@ -888,8 +879,8 @@ export interface EventSlotGap { /** * 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 + * 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, @@ -906,8 +897,10 @@ export interface EventSlotGap { * 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. + * Returns `undefined` for an empty log, which has no density to check. + * + * @throws if any event id carries no slot, for the reason {@link maxEventSlot} + * gives. */ export function findEventSlotGap( events: readonly Event[] @@ -915,10 +908,7 @@ export function findEventSlotGap( const occupied = new Set(); let maxSlot = 0; for (const event of events) { - const slot = eventIdToSlot(event.eventId); - if (slot === null) { - return undefined; - } + const slot = requireEventSlot(event.eventId); occupied.add(slot); if (slot > maxSlot) { maxSlot = slot; @@ -1014,7 +1004,7 @@ export async function settleEventSlotGap( * why {@link slotSnapshotParams} takes the maximum rather than the count. * * Its own object rather than a bare number so a call site cannot half-send it, - * and so the empty case (a run that is not slot-numbered) spreads to nothing. + * and so the empty case spreads to nothing. */ export interface SlotSnapshotParams { eventCount?: number; @@ -1023,15 +1013,17 @@ export interface SlotSnapshotParams { /** * Build the slot snapshot to attach to a replay-context event creation. * - * Empty for a run that is not slot-numbered: there is no position to name, and - * a World that numbers by ULID has nothing to compare against. + * Empty for an empty log, which is the state a `run_created` write is issued + * from: there is no position held yet to name. * * The maximum rather than the length, for the reason {@link maxEventSlot} * gives: a partially-read log holds fewer events than its highest position, and * counting those would make the write claim to have seen less than it has, so * the World would report the same events back on every attempt. */ -export function slotSnapshotParams(events: Event[]): SlotSnapshotParams { +export function slotSnapshotParams( + events: readonly Event[] +): SlotSnapshotParams { const eventCount = maxEventSlot(events); return eventCount === undefined ? {} : { eventCount }; } 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 52ee1ca227..6cf8d69322 100644 --- a/packages/core/src/runtime/resume-hook.consumer-preload.test.ts +++ b/packages/core/src/runtime/resume-hook.consumer-preload.test.ts @@ -26,6 +26,7 @@ import { type CreateEventRequest, type Event, SPEC_VERSION_CURRENT, + slotToEventId, type WorkflowRun, type World, } from '@workflow/world'; @@ -182,7 +183,6 @@ async function runResumeConsumerScenario(options: { updatedAt: startedAt, }; - const hostUlid = monotonicFactory(); let eventIndex = 0; const event = (data: CreateEventRequest): Event => { const t = +startedAt + ++eventIndex * 100; @@ -190,7 +190,7 @@ async function runResumeConsumerScenario(options: { ...data, specVersion: data.specVersion ?? SPEC_VERSION_CURRENT, runId, - eventId: `evnt_${hostUlid(t)}`, + eventId: slotToEventId(eventIndex), createdAt: new Date(t), } as Event; }; diff --git a/packages/core/src/runtime/resume-latency.runtime.test.ts b/packages/core/src/runtime/resume-latency.runtime.test.ts index 278d131008..f6c0d14a92 100644 --- a/packages/core/src/runtime/resume-latency.runtime.test.ts +++ b/packages/core/src/runtime/resume-latency.runtime.test.ts @@ -30,6 +30,7 @@ import { type Event, type HookResumeTiming, SPEC_VERSION_CURRENT, + slotToEventId, type WorkflowInvokePayload, type WorkflowRun, type World, @@ -342,7 +343,6 @@ async function runScenario(options: ScenarioOptions = {}) { updatedAt: startedAt, }; - const hostUlid = monotonicFactory(); let eventIndex = 0; const event = (data: CreateEventRequest): Event => { const t = +startedAt + ++eventIndex * 100; @@ -350,7 +350,7 @@ async function runScenario(options: ScenarioOptions = {}) { ...data, specVersion: data.specVersion ?? SPEC_VERSION_CURRENT, runId, - eventId: `evnt_${hostUlid(t)}`, + eventId: slotToEventId(eventIndex), createdAt: new Date(t), } as Event; }; @@ -1060,7 +1060,6 @@ async function runScenarioWithoutPreload() { updatedAt: startedAt, }; - const hostUlid = monotonicFactory(); let eventIndex = 0; const event = (data: CreateEventRequest): Event => { const t = +startedAt + ++eventIndex * 100; @@ -1068,7 +1067,7 @@ async function runScenarioWithoutPreload() { ...data, specVersion: data.specVersion ?? SPEC_VERSION_CURRENT, runId, - eventId: `evnt_${hostUlid(t)}`, + eventId: slotToEventId(eventIndex), createdAt: new Date(t), } as Event; }; diff --git a/packages/core/src/runtime/step-executor.ts b/packages/core/src/runtime/step-executor.ts index 390e315889..63295800ce 100644 --- a/packages/core/src/runtime/step-executor.ts +++ b/packages/core/src/runtime/step-executor.ts @@ -23,7 +23,7 @@ import type { World, } from '@workflow/world'; import { - eventIdToSlot, + requireEventSlot, SPEC_VERSION_CURRENT, SPEC_VERSION_SUPPORTS_COMPRESSION, } from '@workflow/world'; @@ -162,8 +162,8 @@ export interface StepExecutorParams { * A World that fences rejects a stale claim with `PreconditionFailedError` * (412); executeStep does NOT translate that rejection (re-claiming in place * would still commit the stale schedule), so it propagates for the caller to - * abandon the batch and restart its replay. Undefined for a run that is not - * slot-numbered, or a caller with nothing loaded. + * abandon the batch and restart its replay. Undefined for a caller with + * nothing loaded. */ slotSnapshot?: SlotSnapshotParams; /** @@ -313,11 +313,22 @@ export async function executeStep( let knownSlot = params.slotSnapshot?.eventCount; const observeSlot = (result: { event?: Event; events?: Event[] }): void => { if (knownSlot === undefined) { + // The caller scheduled this step without naming a position, so there is + // no snapshot to advance and the writes below send none. Not the same as + // a run without positions: every run has them, this executor just was not + // told which one it started from. return; } - const committed = result.event ? eventIdToSlot(result.event.eventId) : null; - for (const slot of [committed, maxEventSlot(result.events ?? []) ?? null]) { - if (slot !== null && slot > knownSlot) { + const observed: number[] = []; + if (result.event) { + observed.push(requireEventSlot(result.event.eventId)); + } + const reported = maxEventSlot(result.events ?? []); + if (reported !== undefined) { + observed.push(reported); + } + for (const slot of observed) { + if (slot > knownSlot) { knownSlot = slot; } } diff --git a/packages/core/src/runtime/suspension-handler.test.ts b/packages/core/src/runtime/suspension-handler.test.ts index 1d6bd7327e..d34814500b 100644 --- a/packages/core/src/runtime/suspension-handler.test.ts +++ b/packages/core/src/runtime/suspension-handler.test.ts @@ -12,7 +12,7 @@ import { type WorkflowRun, type World, } from '@workflow/world'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { WorkflowSuspension } from '../global.js'; import { maxEventSlot, stepDispatchIdempotencyKey } from './helpers.js'; import { ReplayRecoveryReporter } from './replay-recovery-reporter.js'; @@ -631,6 +631,21 @@ describe('handleSuspension', () => { describe('resilient step dispatch', () => { const queueName = '__wkf_workflow_test-workflow' as ValidQueueName; + // Opt-in feature, so every test that expects a publish has to ask for it. + // The default-off case is covered by its own test below, which unsets this. + let previousFlag: string | undefined; + beforeEach(() => { + previousFlag = process.env.WORKFLOW_RESILIENT_STEP_DISPATCH; + process.env.WORKFLOW_RESILIENT_STEP_DISPATCH = '1'; + }); + afterEach(() => { + if (previousFlag === undefined) { + delete process.env.WORKFLOW_RESILIENT_STEP_DISPATCH; + } else { + process.env.WORKFLOW_RESILIENT_STEP_DISPATCH = previousFlag; + } + }); + /** A run whose queue transport supports binary payloads (CBOR). */ const cborRun: WorkflowRun = { ...run, specVersion: SPEC_VERSION_CURRENT }; @@ -774,51 +789,13 @@ describe('resilient step dispatch', () => { ).rejects.toThrow('bad request'); }); - it('falls back to create-only when the world enforces the precondition guard', async () => { - const { world, eventsCreate, queue } = createQueueWorld({ - capabilities: { preconditionGuard: true }, - }); - - const result = await handleSuspension({ - suspension: new WorkflowSuspension(fourStepsPending(), globalThis), - world, - run: cborRun, - stepDispatch: stepDispatch(), - }); - - // The guarded create can be 412-rejected; a payload-carrying message - // would let the consumer materialize the rejected step. Sequential path: - // create here, caller dispatches. - expect(queue).not.toHaveBeenCalled(); - expect(eventsCreate).toHaveBeenCalledWith( - run.runId, - expect.objectContaining({ - eventType: 'step_created', - correlationId: 's4', - }), - expect.anything() - ); - expect(result.queuedStepCorrelationIds.size).toBe(0); - expect(result.createdStepCorrelationIds).toContain('s4'); - }); - - it('stays sequential under an enforced guard regardless of other capabilities', async () => { - // The guard gate is deliberately not liftable by backend-side revocation - // bookkeeping: nothing orders a slow guarded create's eventual 412 before - // the consumer's redelivery re-ensure, so no capability may re-enable the - // payload-carrying publish while creates are guarded. - const { world, queue } = createQueueWorld({ - capabilities: { - preconditionGuard: true, - // Unknown/extra capability flags must not lift the gate. - ...({ resilientStepDispatch: true } as Record), - } as World['capabilities'], - }); + it('falls back to create-only when the run predates the CBOR queue transport', async () => { + const { world, queue } = createQueueWorld(); const result = await handleSuspension({ suspension: new WorkflowSuspension(fourStepsPending(), globalThis), world, - run: cborRun, + run: { ...run, specVersion: 2 }, stepDispatch: stepDispatch(), }); @@ -826,13 +803,14 @@ describe('resilient step dispatch', () => { expect(result.queuedStepCorrelationIds.size).toBe(0); }); - it('falls back to create-only when the run predates the CBOR queue transport', async () => { + it('falls back to create-only when WORKFLOW_RESILIENT_STEP_DISPATCH is unset', async () => { + delete process.env.WORKFLOW_RESILIENT_STEP_DISPATCH; const { world, queue } = createQueueWorld(); const result = await handleSuspension({ suspension: new WorkflowSuspension(fourStepsPending(), globalThis), world, - run: { ...run, specVersion: 2 }, + run: cborRun, stepDispatch: stepDispatch(), }); @@ -840,30 +818,6 @@ describe('resilient step dispatch', () => { expect(result.queuedStepCorrelationIds.size).toBe(0); }); - it('falls back to create-only when WORKFLOW_RESILIENT_STEP_DISPATCH=0', async () => { - const prev = process.env.WORKFLOW_RESILIENT_STEP_DISPATCH; - process.env.WORKFLOW_RESILIENT_STEP_DISPATCH = '0'; - try { - const { world, queue } = createQueueWorld(); - - const result = await handleSuspension({ - suspension: new WorkflowSuspension(fourStepsPending(), globalThis), - world, - run: cborRun, - stepDispatch: stepDispatch(), - }); - - expect(queue).not.toHaveBeenCalled(); - expect(result.queuedStepCorrelationIds.size).toBe(0); - } finally { - if (prev === undefined) { - delete process.env.WORKFLOW_RESILIENT_STEP_DISPATCH; - } else { - process.env.WORKFLOW_RESILIENT_STEP_DISPATCH = prev; - } - } - }); - it('never queues from here when no stepDispatch is provided (terminal drain)', async () => { const { world, queue } = createQueueWorld(); diff --git a/packages/core/src/runtime/suspension-handler.ts b/packages/core/src/runtime/suspension-handler.ts index 713b0a1157..7855ca0a95 100644 --- a/packages/core/src/runtime/suspension-handler.ts +++ b/packages/core/src/runtime/suspension-handler.ts @@ -677,26 +677,23 @@ export async function handleSuspension({ // // - The caller provided a dispatch target (`stepDispatch`) — terminal // drains and other create-only callers never queue. - // - The feature is enabled (`WORKFLOW_RESILIENT_STEP_DISPATCH` opt-out). - // - The World does not fence stale writes - // (`capabilities.preconditionGuard`). A guard-enforcing - // backend can reject the step_created as stale (412) and the caller then - // restarts the replay — but a queue message carrying the payload would - // already be out, letting the consumer materialize a step the guard - // rejected. This gate is deliberately NOT liftable by backend-side - // revocation bookkeeping: nothing orders a slow create's eventual 412 - // (which is when the backend learns the dispatch is poisoned) before the - // consumer's redelivery re-ensure, and a best-effort marker that fails - // open cannot carry a correctness property. The sequential path is the - // only thing that gives the message a happens-after edge over its - // create's guard verdict. + // - The feature is enabled (`WORKFLOW_RESILIENT_STEP_DISPATCH` opt-in). + // It is off by default because a World may reject the `step_created` as + // stale (412) and the caller then restarts the replay — but a queue + // message carrying the payload would already be out, letting the consumer + // materialize a step the World refused. Nothing orders a slow create's + // eventual 412 (which is when the backend learns the dispatch is + // poisoned) before the consumer's redelivery re-ensure, so no + // backend-side revocation bookkeeping can close that window: a + // best-effort marker that fails open cannot carry a correctness property. + // The sequential path is the only thing that gives the message a + // happens-after edge over its create's verdict. // - The run's queue transport preserves binary payloads (CBOR, // specVersion >= 3): `stepInput.input` is the serialized (possibly // encrypted) input bytes, which the JSON transport would mangle. const resilientDispatchEligible = stepDispatch !== undefined && isResilientStepDispatchEnabled() && - world.capabilities?.preconditionGuard !== true && (run.specVersion ?? 0) >= SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT; // The trace carrier for resilient step dispatches, resolved at most once per diff --git a/packages/core/src/runtime/wait-completion-replay.test.ts b/packages/core/src/runtime/wait-completion-replay.test.ts index 8da3149a36..2666f95a37 100644 --- a/packages/core/src/runtime/wait-completion-replay.test.ts +++ b/packages/core/src/runtime/wait-completion-replay.test.ts @@ -81,12 +81,6 @@ 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); @@ -131,9 +125,7 @@ async function runStaleWaitReplayScenario(options: { ...data, specVersion: data.specVersion ?? SPEC_VERSION_CURRENT, runId, - eventId: options.slotEventIds - ? slotToEventId(eventIndex) - : `evt_${eventIndex.toString().padStart(3, '0')}`, + eventId: slotToEventId(eventIndex), createdAt, }) as Event; @@ -691,9 +683,8 @@ 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 + it('asks for the delta and the skipped slots at once', async () => { + // 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 @@ -702,7 +693,6 @@ describe('workflow handler wait completion replay', () => { const result = await runStaleWaitReplayScenario({ includePreloadedCursor: true, returnInlineDelta: true, - slotEventIds: true, }); const waitWrite = result.createEvent.mock.calls.find( diff --git a/packages/world-sim/DESIGN.md b/packages/world-sim/DESIGN.md index 0da704e759..893fb03fd7 100644 --- a/packages/world-sim/DESIGN.md +++ b/packages/world-sim/DESIGN.md @@ -297,15 +297,15 @@ how a scenario can be run one flag apart from its neighbour. `countGuard` **follows the fence** unless a spec says otherwise, because a World that fences arms both halves — see below. -**`preconditionGuard`** models `WorldCapabilities.preconditionGuard`: reject a -replay-context write whose snapshot predates the newest externally-originated -event. In the SDK the capability is declared by **world-vercel only** -(`packages/world-vercel/src/index.ts`); `world-local` and `world-postgres` -declare neither it nor `maxConcurrency`. It no longer describes what world-vercel -does to a run, though: a slot-identity run has no snapshot to reject, because the -World allocates the event's position at commit time and reports the positions the -write skipped over. What the sim's fence still covers is the 412 *reception* path -the runtime keeps for Worlds that do fence, and the predicate itself. +**`preconditionGuard`** rejects a replay-context write whose snapshot predates +the newest externally-originated event. It is a store option here and not a +World capability: the runtime assumes any World may refuse a stale write, so a +scenario can change what the store does about one but never what the runtime +expects. It does not describe what world-vercel does to a run either — a +slot-identity run has no snapshot to reject, because the World allocates the +event's position at commit time and reports the positions the write skipped +over. What the sim's fence covers is the 412 *reception* path the runtime keeps +for Worlds that do fence, and the predicate itself. Its predicate is narrower than the bug class, and the reason is its *shape*, not the event type it watches. The marker advances on `hook_received` **or** @@ -452,7 +452,7 @@ interface ScenarioSpec { verifyReplay?: boolean; // default on for runs reaching completed/failed expect?: { status?: ScenarioOutcome; output?: unknown }; // output: deep equality limits?: ScenarioLimits; - preconditionGuard?: boolean; // advertise + enforce the optimistic-concurrency fence + preconditionGuard?: boolean; // enforce the optimistic-concurrency fence countGuard?: boolean; // also enforce its count half appendOnlyLog?: boolean; // position at commit, not at mint; see §5 } @@ -655,7 +655,7 @@ Measured on branch `sim-world`. **Scenarios** — `pnpm sim` in `workbench/sim-world`: ``` -41 scenario(s): 35 passed, 6 failed, 6 consistency violation(s) +41 scenario(s): 38 passed, 3 failed, 3 consistency violation(s) ``` And the same book against an append-only log (`pnpm sim --append-only`): @@ -664,8 +664,8 @@ And the same book against an append-only log (`pnpm sim --append-only`): 41 scenario(s): 41 passed, 0 failed, 0 consistency violation(s) ``` -Both numbers are the intended steady state; see "The six" below for which of -the six violations that second line closes on the merits and which close +Both numbers are the intended steady state; see "The three" below for which of +the three violations that second line closes on the merits and which close because the correct answer itself changes. There was a seventh red until recently, `unclaimed-payload-under-fork`, and it @@ -677,20 +677,20 @@ mistake, and agreed. Only the log disagreeing with itself caught it. #3406 fixed the delivery-barrier ordering and it is now green in both worlds; the scenario stays as that fix's regression test. -With the fence forced off (`pnpm sim --no-fence`), violations go to **8** +With the fence forced off (`pnpm sim --no-fence`), violations go to **5** mint-ordered and stay at **0** append-only — see §5. -Replay verification across the book: **33 `ok`, 6 `MISMATCH`, 2 `skipped`** +Replay verification across the book: **36 `ok`, 3 `MISMATCH`, 2 `skipped`** (skipped where the run did not reach a terminal status). -`run.ts` exits non-zero, and that is the intended steady state. The six +`run.ts` exits non-zero, and that is the intended steady state. The three failures are reproductions of corruptions the runtime can still produce; each states the outcome its own durable log implies and fails until the runtime gets there, so the failure line names both sides (`expected "afterSlow:doc-26", got "afterFast:doc-26"`). -**The number is the thing to watch: six today.** A seventh is a regression; -five means something got fixed and a scenario is ready to retire. +**The number is the thing to watch: three today.** A fourth is a regression; +two means something got fixed and a scenario is ready to retire. That makes the book a poor plain CI gate, which is what `--report-only` is for: it prints every failure and exits 0, so a job can *publish* the book's current @@ -698,54 +698,48 @@ state rather than block on it. `--summary-file` writes one collapsed `
` — a visible line carrying the count and a green or orange dot, the whole table behind it — for a PR comment or `$GITHUB_STEP_SUMMARY`, and `--detail-file` writes the full colour-free trace as an artifact to read when a -number moves. Deliberately nothing above the fold but the count: six are red on -purpose, so a comment that leads with the failures leads with the part that is +number moves. Deliberately nothing above the fold but the count: three are red +on purpose, so a comment that leads with the failures leads with the part that is not news, and grows a wall of text on exactly the PRs that changed nothing. The workbench's `pnpm test` is `--report-only --summary-file`, so a recursive `pnpm -r test` stays green and still says what happened; `pnpm sim` stays strict, so running it by hand fails loudly. -### The six +### The three -The `fix` column names the specific change that closes the scenario. `shown -green by` is the stronger claim: a *passing* scenario that is this one with that -fix armed, same workflow and same tempo, one flag apart. Where it says "none -yet", the fix is identified by argument but nothing in the book proves it. - -| scenario | mechanism | fix | shown green by | -|---|---|---|---| -| `stale-read-step-count-fork` (doc-23) | `withholdNextEvent(1)` + `deliverHook`; hook at `#7`, `wait_completed` at `#8`, no-hook branch at `#9` | `preconditionGuard` — the withheld `hook_received` is the newest out-of-band write and the orchestrator's snapshot predates the sleep, so the watermark fires | `stale-read-step-count-fork-fenced` (doc-24) | -| `stale-read-equal-step-counts` (doc-25) | same fault on a fork whose branches emit one step each | `preconditionGuard`, for the same reason | none yet | -| `step-vs-step-fork` (doc-26) | two of the run's own `step_completed` events, one delivery | `countGuard`. **Not** `preconditionGuard`: the withheld completion is a hole in the middle of the log, which moves no high-water mark (§5) | none yet | -| `step-vs-step-fork-fenced` (doc-27) | same, `preconditionGuard: true`, zero rejections | `countGuard`. This row *is* the proof that the watermark half does not fix doc-26 | none yet | -| `in-flight-before-decision` (doc-29) | `beginHookDelivery`, committed before the decision is written | `countGuard` | `in-flight-before-decision-counted` (doc-30) | -| `in-flight-after-decision` (doc-31) | `beginHookDelivery`, committed after the decision | none in the SDK. Needs an append-tail fence — `assertSlotAboveTail`, `vercel/workflow-server#692` | — | +| scenario | mechanism | fix | +|---|---|---| +| `in-flight-before-decision` (doc-29) | `beginHookDelivery`, committed before the decision is written | none in the SDK. The hole is a live reservation, so re-reading finds it still empty | +| `in-flight-before-decision-counted` (doc-30) | same tempo, count half of the fence armed | same. Mint-ordered the write never reaches the fence | +| `in-flight-after-decision` (doc-31) | `beginHookDelivery`, committed after the decision | none in the SDK. Needs an append-tail fence — `assertSlotAboveTail`, `vercel/workflow-server#692` | Those handles are `ScenarioSpec.id`, and they select: `pnpm sim in-flight-after-decision` plays one row of this table. -So: **two** of the six have their fix demonstrated by a paired green scenario, -**three** have a fix identified but unproven here, and **one** has no fix at all. -Writing the three missing pairs is the obvious next increment. - -Note what the `fix` column does *not* mean. `countGuard` closing doc-29 is a -statement about a fencing World's *predicate*, and nothing more. It is not a -statement about world-vercel, which does not fence a slot-identity run at all: -positions there are assigned at commit, so a write that named a stale one still -commits and comes back carrying the events it skipped over. That is the -append-only column below, reached by a different mechanism. Read the `fix` -column as "which predicate would have caught this fault", and read the -append-only column for what production actually does about it. - -**The append-only log closes all six, in two different senses — and the split -is four and two, not three and three.** Four (doc-23, doc-25, doc-26, doc-27) -close on the merits, with the book asking them exactly what it asked before: the -reordering was the fault, and once positions are assigned at commit the withheld -read degrades from a hole to a truncation, which the fence can see. The -remaining two (doc-29, doc-31) close because the branch the run ends on changes. -A hook that commits after the timeout genuinely *is* after it when the tail is -the only place a write can land, so the log records the timeout first and the -run that settled is the run the log describes. +**There used to be six, and slot-numbered event ids closed half of them.** The +four that closed — `stale-read-step-count-fork` (doc-23), +`stale-read-equal-step-counts` (doc-25), `step-vs-step-fork` (doc-26), +`step-vs-step-fork-fenced` (doc-27) — all staged a *read* that was missing an +event the log already held. Under ULIDs that read was indistinguishable from a +complete one, and the fence was the only thing that could have caught it, which +is why their `fix` column used to name a predicate. Under slot ids a missing +event is a gap in a numbered sequence, so the runtime sees it without asking +anyone: it re-reads, gets the full log, and decides the fork the way the log +records it. The fence never has to fire. Those four are now regression tests for +the gap audit rather than open reproductions, and doc-24's pairing with doc-23 +is now a pairing between two green scenarios. + +What is left is the family the audit cannot repair by re-reading, because the +position really is empty at the moment of the read: a writer has reserved it and +has not committed. Mint-ordered, doc-29 and doc-30 now fail *loudly* rather than +silently — the replay refuses a log it cannot follow instead of following it +into the wrong branch — which is a better outcome than the divergence they used +to produce, and still a failure. + +**The append-only log closes all three, and by construction rather than by +catching anything.** Nothing reserves a position, so no read can see a hole, and +a hook that commits after the timeout genuinely *is* after it. The log records +the timeout first and the run that settled is the run the log describes. **No expectation is restated per world, and there is no mechanism to.** The first cut of this had one — an `expectAppendOnly` field on three scenarios, @@ -761,43 +755,44 @@ report the branch in the trace. That costs nothing, because the expectations were never what caught the fault. The load-bearing assertion is the invariant: **the log a run wrote must be a log the runtime can replay back into that same run**. It is world-independent, on by -default (`verifyReplay`), and it is what all six reds trip. Measured, not +default (`verifyReplay`), and it is what all three reds trip. Measured, not assumed: strip every `expect` in the book and the violation counts do not move -— 6 mint-ordered, 0 append-only, the same six by name. (Pass/fail does move by +— 3 mint-ordered, 0 append-only, the same three by name. (Pass/fail does move by one, and only for a bookkeeping reason: `hook-never-arrives` expects `stalled`, and a stall's reason is reported as a problem unless the scenario said it was expecting one.) That also removes the one place where the flag's scoreboard rested on a judgement about what the right answer *is* rather than on something the harness checks on its own. -doc-30 is worth a line because it was the third `expectAppendOnly` and is *not* -one of the six — mint-ordered it already passes, since `countGuard` catches -there what the watermark half misses. Its branch moves under the flag for the -same reason its uncounted twin's does, so the old pinned output would have -turned a green scenario red. What made it distinct from doc-29 was never the -branch anyway; it is that the count half of the fence fires at all. That is now -asserted directly, matched on the guard's own message and true in both worlds — -and it fails if `countGuard` is turned off, which is the check that a bare -`rejections().length > 0` would have missed, since doc-29 rejects too. +doc-30 is worth a line because it was the third `expectAppendOnly`. Its branch +moves under the flag for the same reason its uncounted twin's does, so the old +pinned output would have turned a scenario red for ending on the world's answer +rather than on a fault. What makes it distinct from doc-29 was never the branch +anyway; it is that the fence fires at all. That is asserted directly, matched on +`PreconditionFailedError` rather than on "something was rejected", since doc-29 +rejects too. The assertion is scoped to the append-only world, because +mint-ordered the write never reaches the fence — the reservation ahead of it +makes the log unreadable first. Two details worth keeping: - Hook delivery participates. `beginHookDelivery` still reserves a position at the handler boundary; under the flag the reservation stops being binding and the write re-mints at the tail if anything overtook it (`positionAtCommit`). -- doc-30's 412 still fires, and now saves nothing. The count guard counts events - at or below the caller's watermark, the watermark is a millisecond, and a hook - committing after the timeout within the same virtual millisecond is still "at - or below" it. The reload finds nothing to correct and the run settles anyway. - That false positive is the standing cost of the count half of the fence once - the log is append-only, and doc-30's trace is where to see it. - -Four of the six are hook-driven and two deliberately are not — the pair proves -the corruption needs no out-of-band event type. All the pure hook-timing -scenarios pass: placing a hook precisely is what works. What fails is a hook -that is durable in the log but absent from the read the live pass decided on. - -The last row is the only one with no fix in the SDK: the hole opens *after* the +- doc-30's 412 still fires, and now saves nothing. The hook commits after the + timeout and therefore sorts after it, so the reload finds nothing to correct + and the run settles anyway. That false positive is the standing cost of the + fence once the log is append-only, and doc-30's trace is where to see it. + +All three are hook-driven, and the two that were not (doc-26 and doc-27, two of +the run's own `step_completed` events) are the ones the gap audit closed — so +the book no longer holds an open reproduction that needs no out-of-band event +type. All the pure hook-timing scenarios pass: placing a hook precisely is what +works. What fails is a hook whose position is spoken for but whose write has not +landed when the live pass reads. + +doc-31, the last row, is the one that no fence placed anywhere in the write path +could reach: the hole opens *after* the write that should have fenced it, in the quiescent gap between deliveries where the run makes no writes and so meets no checks. `assertSlotAboveTail` in `vercel/workflow-server#692` is the append-tail fence for it. diff --git a/packages/world-sim/src/ids.test.ts b/packages/world-sim/src/ids.test.ts index 1fcd36a9a8..645aecc5ce 100644 --- a/packages/world-sim/src/ids.test.ts +++ b/packages/world-sim/src/ids.test.ts @@ -19,10 +19,10 @@ describe('deterministic id factory', () => { it('sorts by (time, mint order), which is what event-log ordering relies on', () => { let now = 1_704_067_200_000; const ids = createIdFactory(() => now); - const a = ids.eventId(); - const b = ids.eventId(); + const a = ids.messageId(); + const b = ids.messageId(); now += 1; - const c = ids.eventId(); + const c = ids.messageId(); expect(a < b).toBe(true); expect(b < c).toBe(true); }); @@ -30,7 +30,7 @@ describe('deterministic id factory', () => { it('is a pure function of (clock, counter)', () => { const build = () => { const ids = createIdFactory(() => 1_704_067_200_000); - return [ids.runId(), ids.eventId(), ids.messageId()]; + return [ids.runId(), ids.messageId()]; }; expect(build()).toEqual(build()); }); diff --git a/packages/world-sim/src/ids.ts b/packages/world-sim/src/ids.ts index f7a62b227f..0a26e47b87 100644 --- a/packages/world-sim/src/ids.ts +++ b/packages/world-sim/src/ids.ts @@ -3,16 +3,20 @@ * * Every ID the simulation hands out is a function of (virtual time, a * per-scenario counter) — never of `Math.random()` or the host clock. Two - * runs of the same scenario produce byte-identical run IDs, event IDs and - * message IDs, which is what makes an event-stream dump usable as a golden - * file. + * runs of the same scenario produce byte-identical run IDs and message IDs, + * which is what makes an event-stream dump usable as a golden file. * - * IDs still have to be *real* ULIDs: `@workflow/world` validates run IDs with - * `z.string().ulid()` and decodes their embedded timestamp (both to reject - * clock-skewed clients and to seed the workflow VM's fixed clock), so the - * encoding below is the standard Crockford base32 layout — 10 timestamp + * Run and message IDs have to be *real* ULIDs: `@workflow/world` validates run + * IDs with `z.string().ulid()` and decodes their embedded timestamp (both to + * reject clock-skewed clients and to seed the workflow VM's fixed clock), so + * the encoding below is the standard Crockford base32 layout — 10 timestamp * characters followed by 16 characters of "randomness" that we fill from the * counter instead. + * + * Event IDs are not minted here at all. They are the event's position in its + * run's log (`@workflow/world`'s `slotToEventId`), which only the store can + * assign because only the store knows how much of the log is already spoken + * for. See `SimStore.mintEvent`. */ const CROCKFORD = '0123456789ABCDEFGHJKMNPQRSTVWXYZ'; @@ -28,29 +32,6 @@ function encodeBase32(value: number, length: number): string { return out; } -/** - * Decode the mint time back out of an id, prefixed (`evnt_01H…`) or bare. - * - * Both concurrency guards compare *ULID times*, never row timestamps: the - * client's snapshot is the ULID time of the newest event it loaded, and the - * server's marker is the ULID time of the newest out-of-band event. Anything - * that has to reason about the log's order therefore reads it back out of the - * id, which is why this lives beside the minting and not beside a caller. - * - * Returns `+Infinity` for an id whose time field is not base32 — an id that - * cannot be placed sorts after everything rather than silently landing at 0. - */ -export function ulidTimeOf(id: string): number { - const ulid = id.includes('_') ? id.slice(id.indexOf('_') + 1) : id; - let time = 0; - for (const char of ulid.slice(0, 10)) { - const digit = CROCKFORD.indexOf(char); - if (digit === -1) return Number.POSITIVE_INFINITY; - time = time * 32 + digit; - } - return time; -} - /** * A monotonic ULID source. * @@ -63,8 +44,6 @@ export function ulidTimeOf(id: string): number { export interface IdFactory { /** Mint a bare ULID stamped with the current virtual time. */ ulid(): string; - /** Mint `evnt_`. */ - eventId(): string; /** Mint `wrun_`. */ runId(): string; /** Mint a monotonically increasing message id. */ @@ -92,7 +71,6 @@ export function createIdFactory(now: () => number): IdFactory { return { ulid, - eventId: () => `evnt_${ulid()}`, runId: () => `wrun_${ulid()}`, messageId: () => `msg_${ulid()}`, count: () => counter, diff --git a/packages/world-sim/src/invariants.test.ts b/packages/world-sim/src/invariants.test.ts index 1e9eb79745..bef2dcb778 100644 --- a/packages/world-sim/src/invariants.test.ts +++ b/packages/world-sim/src/invariants.test.ts @@ -1,4 +1,9 @@ -import type { Event, Step, WorkflowRun } from '@workflow/world'; +import { + type Event, + type Step, + slotToEventId, + type WorkflowRun, +} from '@workflow/world'; import { describe, expect, it } from 'vitest'; import { checkInvariants, type InvariantInput } from './invariants.js'; @@ -25,7 +30,7 @@ function event(partial: Partial & Pick): Event { counter++; return { runId: RUN, - eventId: `evnt_${String(counter).padStart(4, '0')}`, + eventId: slotToEventId(counter), createdAt: new Date(BASE.getTime() + counter), specVersion: 5, ...partial, diff --git a/packages/world-sim/src/replay.test.ts b/packages/world-sim/src/replay.test.ts index bc4a56ff73..b199fc78ab 100644 --- a/packages/world-sim/src/replay.test.ts +++ b/packages/world-sim/src/replay.test.ts @@ -10,6 +10,7 @@ import { getWorld } from '@workflow/core/runtime'; import { type Event, SPEC_VERSION_CURRENT, + slotToEventId, type WorkflowRun, } from '@workflow/world'; import { describe, expect, it } from 'vitest'; @@ -26,7 +27,7 @@ function event(partial: Partial & Pick): Event { counter++; return { runId: RUN, - eventId: `evnt_${String(counter).padStart(4, '0')}`, + eventId: slotToEventId(counter), createdAt: new Date(AT.getTime() + counter), specVersion: SPEC_VERSION_CURRENT, ...partial, diff --git a/packages/world-sim/src/scenario.ts b/packages/world-sim/src/scenario.ts index b1566027ac..449fb52c9e 100644 --- a/packages/world-sim/src/scenario.ts +++ b/packages/world-sim/src/scenario.ts @@ -324,7 +324,7 @@ export async function runScenario( // It has to be an out-of-band writer. An inline step's `step_completed` // held the same way would stall the orchestrator that should misread the // log, because the runtime awaits its promise before deciding anything. - const position = world.reservePosition(); + const position = world.reservePosition(runId); return { eventId: position.eventId, async commit() { diff --git a/packages/world-sim/src/store.test.ts b/packages/world-sim/src/store.test.ts index 9041627967..944dea5435 100644 --- a/packages/world-sim/src/store.test.ts +++ b/packages/world-sim/src/store.test.ts @@ -3,11 +3,12 @@ import { HookNotFoundError, RunExpiredError, } from '@workflow/errors'; -import { SPEC_VERSION_CURRENT } from '@workflow/world'; +import { requireEventSlot, SPEC_VERSION_CURRENT } from '@workflow/world'; import { beforeEach, describe, expect, it } from 'vitest'; import { createIdFactory } from './ids.js'; import { createSimStore, + type LoadedSnapshot, type MintedEvent, type SimStore, type SimStoreOptions, @@ -16,6 +17,18 @@ import { const SPEC = SPEC_VERSION_CURRENT; +/** + * The snapshot of a caller that loaded the run's whole log. The store is driven + * directly here, so there is no facade tracking reads to derive one from. + */ +function loadedAll(store: SimStore): LoadedSnapshot { + const events = store.allEvents(RUN); + return { + maxSlot: Math.max(...events.map((e) => requireEventSlot(e.eventId))), + count: events.length, + }; +} + function setup(options?: Omit) { let now = 1_704_067_200_000; const store = createSimStore({ @@ -424,10 +437,7 @@ describe('sim store', () => { }); guarded.tick(10); - const snapshot = { - updatedAt: guarded.nowMs(), - count: guarded.store.allEvents(RUN).length, - }; + const snapshot = loadedAll(guarded.store); guarded.tick(10); // An out-of-band resume: no snapshot, so it advances the marker. await guarded.store.events.create(RUN, { @@ -450,7 +460,7 @@ describe('sim store', () => { ) ).rejects.toThrow(/out of band/); - // An up-to-date snapshot passes — an equal timestamp must not livelock. + // An up-to-date snapshot passes — an equal watermark must not livelock. await expect( guarded.store.events.create( RUN, @@ -460,12 +470,7 @@ describe('sim store', () => { correlationId: 'step_1', eventData: { stepName: 'step//./w//s', input: new Uint8Array() }, }, - { - snapshot: { - updatedAt: guarded.nowMs(), - count: guarded.store.allEvents(RUN).length, - }, - } + { snapshot: loadedAll(guarded.store) } ) ).resolves.toBeTruthy(); }); @@ -488,7 +493,7 @@ describe('sim store', () => { it('is off by default: a held write lands behind one committed sooner', async () => { await createRun(store, RUN); // The handler boundary takes a position; the write is then held. - const minted = store.mintEvent(); + const minted = store.mintEvent(RUN); tick(10); const overtook = await store.events.create(RUN, hook); const held = await store.events.create(RUN, step, heldAt(minted)); @@ -505,7 +510,7 @@ describe('sim store', () => { it('re-mints a write that was overtaken while it was held', async () => { const world = setup({ appendOnlyLog: true }); await createRun(world.store, RUN); - const minted = world.store.mintEvent(); + const minted = world.store.mintEvent(RUN); world.tick(10); const overtook = await world.store.events.create(RUN, hook); const held = await world.store.events.create(RUN, step, heldAt(minted)); @@ -523,7 +528,7 @@ describe('sim store', () => { it('leaves an uncontended write at the position it minted', async () => { const world = setup({ appendOnlyLog: true }); await createRun(world.store, RUN); - const minted = world.store.mintEvent(); + const minted = world.store.mintEvent(RUN); world.tick(10); const uncontended = await world.store.events.create( RUN, diff --git a/packages/world-sim/src/store.ts b/packages/world-sim/src/store.ts index abde8a4193..2f499abd83 100644 --- a/packages/world-sim/src/store.ts +++ b/packages/world-sim/src/store.ts @@ -43,14 +43,16 @@ import { type PaginatedResponse, type PaginationOptions, type ResolveData, + requireEventSlot, SPEC_VERSION_CURRENT, type Step, type Storage, + slotToEventId, stripEventDataRefs, type Wait, type WorkflowRun, } from '@workflow/world'; -import { type IdFactory, ulidTimeOf } from './ids.js'; +import type { IdFactory } from './ids.js'; /** Per-run event ceiling reported on run responses, mirroring the other worlds. */ const MAX_EVENTS_PER_RUN = 25_000; @@ -67,20 +69,21 @@ const DEFAULT_PAGE_LIMIT = 20; const RUN_EVENT_INDEX_WINDOW = 16; /** - * A log position, minted before the write that will occupy it commits. + * A log position, taken before the write that will occupy it commits. * - * The event id *is* the log's sort key, and the sim mints it the way - * workflow-server does: in the request handler (`EventId.make()`), not in - * storage — DynamoDB does not generate ids. Everything downstream follows from - * that one fact. A write that is minted and then takes a while to commit keeps - * the earlier position it was given, so the log can gain an event *behind* a - * position a reader has already seen. That is the hole no high-water mark can - * detect, and reproducing it is the reason minting is separable from appending. + * The event id *is* the position: `evnt_` followed by the event's 1-based slot + * in its run's log. This store hands the slot out in the request handler rather + * than at the append, which is the shape a World has whenever it cannot ask + * storage to allocate — and everything downstream follows from that one fact. A + * write that takes a slot and then takes a while to commit keeps the earlier + * slot it was given, so the log can gain an event *behind* a position a reader + * has already seen. That is the hole no high-water mark can detect, and + * reproducing it is the reason taking a position is separable from appending. * - * `createdAt` is the mint instant, not the commit instant: in production it is - * decoded back out of the ULID, so the two can never disagree. Entity rows + * `createdAt` is the mint instant, not the commit instant, matching a World + * that derives the row's timestamp at the handler boundary. Entity rows * (step/run/hook timestamps) still use the commit instant, because those are - * written by the transaction rather than derived from the id. + * written by the transaction rather than carried with the position. */ export interface MintedEvent { eventId: string; @@ -103,13 +106,14 @@ interface SimCreateParams { * come from a replay context at all (an out-of-band writer, or a store driven * directly by a unit test). * - * Reconstructed by the world facade from the pages the writer read, because - * the runtime no longer states it: `@workflow/core` describes its snapshot as - * a slot count, and the sim mints ULIDs, so there is nothing on the wire for - * the fence to read. The reconstruction is the same derivation the client - * used to make — the newest loaded position, and how many events sit at or - * below it — which is what lets the fence spot a hole *behind* the watermark - * that no comparison against the watermark alone can see. + * Reconstructed by the world facade from the pages the writer read rather + * than taken off the wire, because the wire carries only half of it: the + * runtime states the highest slot it holds, and the fence also wants how many + * events it loaded at or below that slot. The reconstruction is the same + * derivation the client made — the newest loaded position, and how many + * events sit at or below it — which is what lets the fence spot a hole + * *behind* the watermark that no comparison against the watermark alone can + * see. * * See `SimStoreOptions.preconditionGuard` and `SimWorldOptions.countGuard`. */ @@ -118,9 +122,9 @@ interface SimCreateParams { /** What a replay-context writer had loaded when it decided to write. */ export interface LoadedSnapshot { - /** ULID time of the newest loaded event. */ - updatedAt: number; - /** How many loaded events sit at or below {@link updatedAt}. */ + /** Slot of the newest loaded event. */ + maxSlot: number; + /** How many loaded events sit at or below {@link maxSlot}. */ count: number; } @@ -142,10 +146,10 @@ interface RunEventIndex { */ function countRecordedAtOrBelow( index: RunEventIndex, - updatedAt: number + maxSlot: number ): number | null { const above = index.recentEventIds.filter( - (id) => ulidTimeOf(id) > updatedAt + (id) => requireEventSlot(id) > maxSlot ).length; const pruned = index.total > index.recentEventIds.length; if (pruned && above === index.recentEventIds.length) return null; @@ -246,13 +250,13 @@ export interface SimStore extends Storage { */ seedFromLog(log: readonly Event[]): void; /** - * Mint the next log position, without writing anything. + * Take the run's next log position, without writing anything. * * The world facade calls this at the handler boundary — before any hold can * fire — so a write held mid-flight already owns the position it will * eventually occupy. See {@link MintedEvent}. */ - mintEvent(): MintedEvent; + mintEvent(runId: string): MintedEvent; /** * Hide the *next* event appended from the following `reads` event-log reads. * @@ -414,10 +418,28 @@ export function createSimStore(options: SimStoreOptions): SimStore { /** hookIds that have been explicitly disposed; disposal is permanent. */ const disposedHooks = new Set(); /** - * Per run: ULID time of the newest externally-originated event. Only read - * when `preconditionGuard` is on. See `SimCreateParams.snapshot`. + * Per run: slot of the newest externally-originated event. Only read when + * `preconditionGuard` is on. See `SimCreateParams.snapshot`. */ const externalWriteMarker = new Map(); + /** + * Per run: the highest slot handed out, committed or merely spoken for. + * + * Separate from the committed log because a position is taken at the handler + * boundary: between `mintEvent` and the append, the slot exists and belongs + * to nobody. A write that never commits gives its slot back (see + * `releaseSlot`); a position reserved and then abandoned out of band leaves + * it empty for good. + */ + const highestSlot = new Map(); + /** + * Per run: positions handed out and given back, still unoccupied. + * + * Reused before the range grows, so the log stays dense. What makes that + * safe is that a slot only lands here once its create has returned: nothing + * is still holding it, and nothing ever will. + */ + const freeSlots = new Map(); /** * Per run: the tail of the log, for the count guard. Records *every* event, * replay-origin included — the corruption it guards against is one replay @@ -460,8 +482,69 @@ export function createSimStore(options: SimStoreOptions): SimStore { const waitKey = (runId: string, correlationId: string) => `${runId}:${correlationId}`; - function mintEvent(): MintedEvent { - return { eventId: ids.eventId(), createdAt: new Date(nowMs()) }; + /** Highest slot committed to a run's log, or 0 for a log with no events. */ + function committedSlot(runId: string): number { + let max = 0; + for (const event of events) { + if (event.runId !== runId) continue; + const slot = requireEventSlot(event.eventId); + if (slot > max) max = slot; + } + return max; + } + + function mintEvent(runId: string): MintedEvent { + let slot: number; + const free = appendOnlyLog ? undefined : freeSlots.get(runId); + if (free?.length) { + free.sort((a, b) => a - b); + slot = free.shift() as number; + } else { + slot = (highestSlot.get(runId) ?? 0) + 1; + highestSlot.set(runId, slot); + } + return { eventId: slotToEventId(slot), createdAt: new Date(nowMs()) }; + } + + /** + * Give a slot back when nothing committed at it. + * + * A hole in the log is corruption as far as the runtime is concerned, so a + * create that appends nothing must not consume a position. Two kinds do: one + * the store rejects outright, and one it accepts as a no-op (a second + * `run_started` for a run already started, say). Production reaches the same + * place from the other side, by allocating inside the transaction, after the + * validation, so a write it refuses never had a slot to lose. Reproducing + * *that* difference is not what this store is for: the fault it stages is + * two writes taking positions in one order and committing in another, and a + * rejection leaving a permanent hole would sit on top of every one of those + * scenarios as a second, unrelated corruption. + * + * A slot at the top of the range is dropped rather than recycled, because a + * range that never grew is not a hole to fill. Anything below it goes on the + * free list, since concurrent writers mean the rejected position is not + * always the newest one. + */ + function releaseSlot(runId: string, position: MintedEvent): void { + // Under `appendOnlyLog` the position is decided at the append, which keeps + // the mark on the committed tail; a reservation nothing used was never + // counted in the first place. + if (appendOnlyLog) return; + const occupied = events.some( + (e) => e.runId === runId && e.eventId === position.eventId + ); + if (occupied) return; + const free = freeSlots.get(runId) ?? []; + free.push(requireEventSlot(position.eventId)); + let highest = highestSlot.get(runId) ?? 0; + let index = free.indexOf(highest); + while (index !== -1) { + free.splice(index, 1); + highest--; + index = free.indexOf(highest); + } + highestSlot.set(runId, highest); + freeSlots.set(runId, free); } function recordInIndex(event: Event): void { @@ -483,25 +566,38 @@ export function createSimStore(options: SimStoreOptions): SimStore { /** * The position an event actually commits at. * - * Only `appendOnlyLog` can move one. A write that is still the newest - * position when it arrives keeps the id it was already handed out under, so - * uncontended history is unchanged; one that was overtaken while it was held - * re-mints and takes the tail. + * Only `appendOnlyLog` can move one, and there it moves every write that is + * not already landing on the run's next free slot: the position is whatever + * follows the newest *committed* event, decided here rather than at the + * boundary. A write that was never overtaken is already there, so uncontended + * history is unchanged; one that was overtaken while it was held gives up the + * slot it reserved and takes the tail. * - * Compared as plain strings: the ids are fixed-width ULIDs whose lexical - * order *is* `(createdAt, eventId)` order, because the time field is the - * `createdAt` millisecond and the suffix is a monotonic counter. See - * `createIdFactory`. + * Recomputing rather than comparing also keeps the log dense. A slot the + * boundary handed out and nothing committed at is a permanent hole in the + * default mode; under `appendOnlyLog` nothing consumes a slot until it + * commits, so no reservation can leave one behind. */ function positionAtCommit(event: Event): Event { if (!appendOnlyLog) return event; - const tail = events[events.length - 1]; - if (!tail || event.eventId > tail.eventId) return event; - return { ...event, ...mintEvent() }; + const next = committedSlot(event.runId) + 1; + if (requireEventSlot(event.eventId) === next) return event; + return { + ...event, + eventId: slotToEventId(next), + createdAt: new Date(nowMs()), + }; } function append(incoming: Event): Event { const event = positionAtCommit(incoming); + if (appendOnlyLog) { + // The commit decided the position, so the allocator follows the log + // rather than the other way round. A write that reserved a slot and + // then committed *below* it would otherwise leave the mark above the + // tail, and the next mint would skip the difference. + highestSlot.set(event.runId, requireEventSlot(event.eventId)); + } events.push(event); recordInIndex(event); if (armedWithhold !== undefined) { @@ -797,10 +893,18 @@ export function createSimStore(options: SimStoreOptions): SimStore { } } - async function create( + /** + * Append one event, or refuse to. + * + * `held` carries the position this call is holding out to the wrapper below, + * which hands it back when the call throws. It is a parameter rather than a + * closure variable because two creates can be in flight at once. + */ + async function commitEvent( runIdArg: string | null, data: AnyEventRequest, - params?: CreateEventParams + params: CreateEventParams | undefined, + held: { runId?: string; position?: MintedEvent } ): Promise { // Commit time, for the entity rows the transaction writes. The *event's* // timestamp comes from its minted position instead — see `MintedEvent`. @@ -808,11 +912,6 @@ export function createSimStore(options: SimStoreOptions): SimStore { const internal = params as | (CreateEventParams & SimCreateParams) | undefined; - // Reassigned only by the two paths that write a *synthetic* event ahead of - // the requested one: the synthetic takes the position minted at the - // boundary (production mints it first, for exactly this ordering) and the - // requested event re-mints so it still sorts after. - let position = internal?.minted ?? mintEvent(); const resolveData: ResolveData = params?.resolveData ?? 'all'; const specVersion = data.specVersion ?? SPEC_VERSION_CURRENT; @@ -825,6 +924,14 @@ export function createSimStore(options: SimStoreOptions): SimStore { runId = runIdArg; } + // Reassigned only by the two paths that write a *synthetic* event ahead of + // the requested one: the synthetic takes the position taken at the + // boundary (which is the earlier one, for exactly this ordering) and the + // requested event takes a fresh one so it still sorts after. + let position = internal?.minted ?? mintEvent(runId); + held.runId = runId; + held.position = position; + let currentRun = runs.get(runId); // ---- Resilient start --------------------------------------------------- @@ -854,7 +961,8 @@ export function createSimStore(options: SimStoreOptions): SimStore { append(synthetic); // The synthetic took the boundary-minted position, so the `run_started` // row built below needs a fresh one to sort after it. - position = mintEvent(); + position = mintEvent(runId); + held.position = position; } } @@ -874,7 +982,7 @@ export function createSimStore(options: SimStoreOptions): SimStore { const snapshot = internal?.snapshot; if (options.preconditionGuard && snapshot) { const marker = externalWriteMarker.get(runId); - if (marker !== undefined && snapshot.updatedAt < marker) { + if (marker !== undefined && snapshot.maxSlot < marker) { throw new PreconditionFailedError( `Run "${runId}" changed out of band since the caller's snapshot` ); @@ -890,7 +998,7 @@ export function createSimStore(options: SimStoreOptions): SimStore { if (options.countGuard) { const index = runEventIndex.get(runId); const recorded = index - ? countRecordedAtOrBelow(index, snapshot.updatedAt) + ? countRecordedAtOrBelow(index, snapshot.maxSlot) : null; if (recorded !== null && recorded > snapshot.count) { throw new PreconditionFailedError( @@ -1160,7 +1268,8 @@ export function createSimStore(options: SimStoreOptions): SimStore { // an early position for either. No scenario needs that yet; the writes // that race for position in practice are the completions. const { input: _dropped, ...rest } = data.eventData; - position = mintEvent(); + position = mintEvent(runId); + held.position = position; event = { ...event, ...position, eventData: rest } as Event; } @@ -1180,10 +1289,10 @@ export function createSimStore(options: SimStoreOptions): SimStore { // Two details are load-bearing, both copied from workflow-server's // `recordOutsideEvent`: // - // - The mark is the event's *own* position time, not the commit instant. It - // has to be the same derivation as a caller's watermark (the position - // time of its newest loaded event) or a caller holding exactly this event - // would compare as older and 412 forever. + // - The mark is the event's *own* slot, not the commit instant. It has to + // be the same derivation as a caller's watermark (the slot of its newest + // loaded event) or a caller holding exactly this event would compare as + // older and 412 forever. // - The write is forward-only. Concurrent out-of-band events can commit out // of position order — the whole subject of these scenarios — and letting a // late-committing older event drag the mark backwards would silently @@ -1198,7 +1307,7 @@ export function createSimStore(options: SimStoreOptions): SimStore { const previous = externalWriteMarker.get(runId) ?? 0; externalWriteMarker.set( runId, - Math.max(previous, event.createdAt.getTime()) + Math.max(previous, requireEventSlot(event.eventId)) ); } @@ -1254,6 +1363,19 @@ export function createSimStore(options: SimStoreOptions): SimStore { return deltaPage ? { ...result, ...deltaPage } : result; } + async function create( + runIdArg: string | null, + data: AnyEventRequest, + params?: CreateEventParams + ): Promise { + const held: { runId?: string; position?: MintedEvent } = {}; + try { + return await commitEvent(runIdArg, data, params, held); + } finally { + if (held.runId && held.position) releaseSlot(held.runId, held.position); + } + } + const storage: SimStore = { runs: { async get(id: string, params?: { resolveData?: ResolveData }) { @@ -1424,6 +1546,14 @@ export function createSimStore(options: SimStoreOptions): SimStore { const seeded = clone(event) as Event; events.push(seeded); recordInIndex(seeded); + // A cold start inherits the log's positions, so the next write has to + // continue them. Taking the maximum rather than counting: the log a + // scenario seeds is whatever the previous process committed, holes + // included, and re-issuing a slot it already used would be worse than + // leaving the hole. + const slot = requireEventSlot(seeded.eventId); + const highest = highestSlot.get(seeded.runId) ?? 0; + if (slot > highest) highestSlot.set(seeded.runId, slot); // The event's own position time is the only clock a seeded row can // have: the live one belongs to whenever this world was built. applyEvent(seeded, seeded.createdAt); diff --git a/packages/world-sim/src/world.ts b/packages/world-sim/src/world.ts index 0938126a29..2599e6e654 100644 --- a/packages/world-sim/src/world.ts +++ b/packages/world-sim/src/world.ts @@ -30,11 +30,12 @@ import { type Event, getQueueTopicPrefix, type QueuePayload, + requireEventSlot, SPEC_VERSION_CURRENT, type World, } from '@workflow/world'; import { createVirtualClock, type VirtualClock } from './clock.js'; -import { createIdFactory, type IdFactory, ulidTimeOf } from './ids.js'; +import { createIdFactory, type IdFactory } from './ids.js'; import { createSimQueue, type DirectHandler, type SimQueue } from './queue.js'; import { createSimStore, @@ -99,10 +100,10 @@ export interface SimWorldOptions { * log is append-only and no read can be contradicted by a later one. See * `SimStoreOptions.appendOnlyLog` for what that buys and what it costs. * - * The boundary mint still happens — `reservePosition` and everything a - * scenario hangs off it work unchanged. It just stops being binding: a held - * write that nothing overtook keeps the position it reserved, and one that was - * overtaken re-mints when it lands. + * The boundary reservation still happens — `reservePosition` and everything + * a scenario hangs off it work unchanged. It just stops being binding: a held + * write that nothing overtook keeps the position it reserved, and one that + * was overtaken takes the tail when it lands. */ appendOnlyLog?: boolean; } @@ -129,15 +130,16 @@ export interface SimWorld extends World { */ asExternal(fn: () => Promise): Promise; /** - * Take a log position now, to be used by a write that happens later. + * Take a log position in `runId` now, to be used by a write that happens + * later. * * The scenario's own calls are not call points (see `fireWatches`), so a script - * cannot hold *itself* between minting and committing the way it holds a - * writer. This pair is how it states the same thing directly: reserve the + * cannot hold *itself* between taking a position and committing the way it + * holds a writer. This pair is how it states the same thing directly: reserve the * position, do whatever should observe the log without it, then run the write * inside `withReservedPosition` so it lands where it was reserved. */ - reservePosition(): MintedEvent; + reservePosition(runId: string): MintedEvent; /** Run `fn` with the next `events.create` taking `position` instead of minting. */ withReservedPosition( position: MintedEvent, @@ -608,12 +610,12 @@ export function createSimWorld(options: SimWorldOptions = {}): SimWorld { if (!runId) return undefined; const set = loadedEvents()?.get(runId); if (!set || set.size === 0) return undefined; - let updatedAt = 0; + let maxSlot = 0; for (const eventId of set) { - const at = ulidTimeOf(eventId); - if (at > updatedAt) updatedAt = at; + const slot = requireEventSlot(eventId); + if (slot > maxSlot) maxSlot = slot; } - return { updatedAt, count: set.size }; + return { maxSlot, count: set.size }; } /** Wrap one world method so it becomes a call point. */ @@ -652,7 +654,7 @@ export function createSimWorld(options: SimWorldOptions = {}): SimWorld { if (call === 'events.create') { // A reserved position wins: it belongs to a write whose handler was // entered earlier and is only now reaching storage. - const minted = reservedPosition ?? store.mintEvent(); + const minted = reservedPosition ?? store.mintEvent(args[0] as string); reservedPosition = undefined; const params = (args[2] ?? {}) as Record; callArgs = [ @@ -661,12 +663,12 @@ export function createSimWorld(options: SimWorldOptions = {}): SimWorld { { ...params, minted, - // The snapshot the fence reads. Reconstructed rather than taken off - // the wire: the runtime states its position as a slot count, and - // this store mints ULIDs, so there is nothing on the wire a ULID - // fence can compare. What the facade tracks is the same array the - // client is holding (see `loadedEvents`), so the pair it derives is - // the pair the client would have sent. + // The snapshot the fence reads. Reconstructed rather than taken + // off the wire, because the wire carries only the writer's highest + // slot and the count guard also wants how many events it loaded at + // or below it. What the facade tracks is the same array the client + // is holding (see `loadedEvents`), so the pair it derives is the + // pair the client would have sent. // // Attached whenever the fence is armed, not just for the count // half: `SimCreateParams.snapshot` is also what marks a write as @@ -777,11 +779,11 @@ export function createSimWorld(options: SimWorldOptions = {}): SimWorld { const world: SimWorld = { specVersion: SPEC_VERSION_CURRENT, - capabilities: { - // Only advertise the fence when the store is actually enforcing it — a - // runtime fast path gated on this capability must never run without one. - ...(options.preconditionGuard ? { preconditionGuard: true } : {}), - }, + // Whether the fence is armed is a store option, not a capability: the + // runtime assumes every World may reject a stale write, so a scenario can + // only change what the store does about one, never what the runtime + // expects. See `SimStoreOptions.preconditionGuard`. + capabilities: {}, getDeploymentId: intercept('getDeploymentId', () => simQueue.getDeploymentId() ), @@ -848,7 +850,7 @@ export function createSimWorld(options: SimWorldOptions = {}): SimWorld { async asExternal(fn) { return externalCtx.run(true, fn); }, - reservePosition: () => store.mintEvent(), + reservePosition: (runId) => store.mintEvent(runId), async withReservedPosition(position, fn) { reservedPosition = position; try { diff --git a/packages/world-vercel/src/index.ts b/packages/world-vercel/src/index.ts index f1fb2cbe53..87efba4029 100644 --- a/packages/world-vercel/src/index.ts +++ b/packages/world-vercel/src/index.ts @@ -38,17 +38,6 @@ export function createWorld(config?: APIConfig): World { specVersion: SPEC_VERSION_SUPPORTS_SLOT_IDENTITY, capabilities: { hookRetention: { active: true }, - // The backend rejects a stale create with 412 (PreconditionFailedError) - // rather than committing it. - // - // No write this adapter now sends can be rejected that way: the backend - // evaluates the fence only for a create carrying a ULID-era snapshot, - // and this SDK sends none. A run created here is v6, where staleness is - // handled by allocating the write above the contention and reporting - // back the slots it skipped. The capability is kept because the runtime - // reads it as "a write can be refused", and the choices keyed on it stay - // the conservative ones while the slot path carries the load. - preconditionGuard: true, // Vercel Queues supports maxConcurrency-limited consumers, which // WORKFLOW_SEQUENTIAL_REPLAYS=1 uses for per-run `maxConcurrency: 1` // flow topics (see queue.ts and @workflow/builders). diff --git a/packages/world/src/index.ts b/packages/world/src/index.ts index 4a0d932599..a8325cc44c 100644 --- a/packages/world/src/index.ts +++ b/packages/world/src/index.ts @@ -121,6 +121,7 @@ export { isSlotBody, isSlotEventId, MAX_EVENT_SLOT, + requireEventSlot, slotToEventId, } from './slot-identity.js'; export type { SpecVersion } from './spec-version.js'; diff --git a/packages/world/src/interfaces.ts b/packages/world/src/interfaces.ts index 8d62f746dc..d895b88416 100644 --- a/packages/world/src/interfaces.ts +++ b/packages/world/src/interfaces.ts @@ -173,14 +173,6 @@ export interface Storage { ): Promise<(WorkflowRun | WorkflowRunWithoutData | null)[]>; }; - /** - * Lists canonical workflow storage records. - * - * @remarks Observability and inspection usage of this method is - * deprecated. Use `world.analytics?.runs.list()` for plan-aware - * observability queries. This storage API remains available for - * operational and payload-bearing callers. - */ list( params: ListWorkflowRunsParams & { resolveData: 'none' } ): Promise>; @@ -342,24 +334,6 @@ export interface WorldCapabilities { active: boolean; }; - /** - * The World fences a stale replay-context write: an event creation whose - * snapshot is behind what the run has already recorded is rejected with a - * `PreconditionFailedError` (412) rather than committed. The runtime's - * response is to abandon the write and replay from a corrected log. - * - * Runtime optimizations that are only safe behind that fence read this - * capability, so a World that accepts a snapshot and ignores it must leave - * this unset: sending a snapshot is not the same as one being enforced, and - * enabling those optimizations with nothing behind them makes a stale replay - * commit. - * - * Orthogonal to {@link slotEventIds}, which never rejects — it commits above - * the contention and reports back what the writer missed. A World may - * declare either, both, or neither. - */ - preconditionGuard?: boolean; - /** * The World's queue supports `maxConcurrency`-limited consumption — in * particular the per-run flow topics consumed with `maxConcurrency: 1` diff --git a/packages/world/src/slot-identity.ts b/packages/world/src/slot-identity.ts index a42283bba3..29fdb98616 100644 --- a/packages/world/src/slot-identity.ts +++ b/packages/world/src/slot-identity.ts @@ -98,3 +98,27 @@ export function eventIdToSlot(eventId: string): number | null { const slot = Number(body); return Number.isSafeInteger(slot) && slot >= FIRST_EVENT_SLOT ? slot : null; } + +/** + * Reads the slot out of an event id, for a caller that has no answer without + * one. + * + * Separate from {@link eventIdToSlot} because the two failures are different + * problems. A caller that can act on either scheme asks the question and takes + * `null` as an answer; a caller whose whole computation is positional (a + * precondition snapshot, a density audit) has no correct behaviour to fall back + * on, and silently skipping the id would make it report a position it never + * verified. Throwing names the id instead. + * + * @throws if the id is not slot-numbered, i.e. the World minting it does not + * allocate slots. + */ +export function requireEventSlot(eventId: string): number { + const slot = eventIdToSlot(eventId); + if (slot === null) { + throw new Error( + `Event id is not slot-numbered: ${eventId}. This World allocates event positions the runtime cannot read.` + ); + } + return slot; +} diff --git a/workbench/sim-world/README.md b/workbench/sim-world/README.md index ea48165e1d..2998a18f1b 100644 --- a/workbench/sim-world/README.md +++ b/workbench/sim-world/README.md @@ -165,20 +165,18 @@ Two of these are measurements rather than conveniences. **`--append-only`** moves every event's position from its handler's mint to its commit, which is the one change that makes a stale read impossible: the log can be behind, never wrong. Running with and without it is how you tell which of -the reds that change would actually close. Today: **35 pass / 6 violations** +the reds that change would actually close. Today: **38 pass / 3 violations** mint-ordered, **41 pass / 0 violations** append-only. -The one red it does *not* close is `unclaimed-payload-under-fork`, and that is -the point of it: no log position is wrong there, the runtime hands two -resolutions to the workflow in the order the log did not record. It is the only -scenario in the book that is red in both worlds. +It closes all three, and by construction rather than by catching anything: with +nothing reserving a position ahead of a commit, no read can see a hole. **`--no-fence`** turns the fence off everywhere, asking whether anything relies on it. It is a diagnostic, not a world — **read the violation count, not the pass count**, because a scenario whose whole point is that the guard fired asserts exactly that and fails by design when you disarm it (`in-flight-before-decision-counted` is the one that does this today). -Measured: **6 → 8** violations mint-ordered, so it is load-bearing there; +Measured: **3 → 5** violations mint-ordered, so it is load-bearing there; **0 → 0** append-only, so it is dead weight once positions are assigned at commit. diff --git a/workbench/sim-world/scenarios/in-flight-before-decision-counted.ts b/workbench/sim-world/scenarios/in-flight-before-decision-counted.ts index 827a8aa660..e04a5309bc 100644 --- a/workbench/sim-world/scenarios/in-flight-before-decision-counted.ts +++ b/workbench/sim-world/scenarios/in-flight-before-decision-counted.ts @@ -17,14 +17,19 @@ export const scenario: ScenarioSpec = { '`@workflow/core` puts on every replay-context create; what differs is ' + 'what a World does with it. This sim rejects on it, which is why the flag ' + 'below is the default and the twin above has to switch it off. ' + - 'Under an append-only log the 412 still fires and now saves nothing: the ' + - 'count is taken at or below the caller’s watermark, and the watermark ' + - 'is a millisecond, so a hook that commits after the timeout inside the ' + - 'same virtual millisecond still counts as "at or below" it. The reload ' + - 'sees the hook behind the timeout in log order, re-decides the same way, ' + - 'and settles — a restart with nothing to correct. That is what the ' + - 'fence costs once the log is append-only: false positives at millisecond ' + - 'granularity, in exchange for a hole that can no longer open.', + 'Which half fires is no longer the point, though, and that is what slot ' + + 'positions changed. A watermark used to be a millisecond, so two writes ' + + 'inside one virtual millisecond compared equal and only the count could ' + + 'separate them; a watermark that is a slot is strictly ordered, so the ' + + 'watermark half now rejects this on its own. What the scenario still ' + + 'asserts is that the fence fires at all. ' + + 'Under an append-only log the 412 still fires and saves nothing: the ' + + 'reload sees the hook behind the timeout in log order, re-decides the ' + + 'same way, and settles — a restart with nothing to correct. ' + + 'Mint-ordered there is no fence to reach: the receiver holds a position ' + + 'ahead of everything the orchestrator writes, so the log has a hole in it ' + + 'while the write is in flight, and the next replay refuses the log ' + + 'outright rather than following it into the wrong branch.', workflow: 'stepCountForkWorkflow', input: ['doc-30'], preconditionGuard: true, @@ -39,23 +44,25 @@ export const scenario: ScenarioSpec = { await hook.commit(); await wf.release(); - // The point of the scenario, and the part that is true in both worlds: the - // count half of the fence fires where the watermark half did not. What the - // reload then decides is a different question and belongs to the world — - // mint-ordered it corrects the branch, append-only it re-confirms it — so - // that half is left to the trace. Asserting it here is what forced this - // scenario to carry two expectations, and it was never what distinguished - // it from its uncounted twin. - // Matched on the count half's own message, not on "something was - // rejected". The twin rejects too — its writes hit `RunExpiredError` once - // the corrupted branch has run — so a bare `rejections().length > 0` would - // hold there as well and assert nothing about the guard. - sim.check( - 'the count guard fenced the write the watermark let through', - sim.world - .rejections() - .some((r) => r.message.includes('at or below the caller')) - ); + // Matched on the error, not on "something was rejected". The twin rejects + // too — its writes hit `RunExpiredError` once the corrupted branch has run + // — so a bare `rejections().length > 0` would hold there as well and + // assert nothing about the fence. + // + // Only asserted under an append-only log, because only there does the + // write reach the fence. Mint-ordered, the receiver's reserved position is + // binding, so the log carries a hole for as long as the write is in + // flight, and the replay that reads it refuses the log before any write of + // its own is checked. That refusal is the violation the trace reports; a + // check here would restate it as a second failure. + if (sim.appendOnlyLog) { + sim.check( + 'the fence rejected the write', + sim.world + .rejections() + .some((r) => r.errorName === 'PreconditionFailedError') + ); + } }, // The rejection and the reload show up in the trace as `!!` lines. Whichever // branch the reload lands on, it is the one the durable log implies — so diff --git a/workbench/sim-world/scenarios/in-flight-before-decision.ts b/workbench/sim-world/scenarios/in-flight-before-decision.ts index 23259f7015..14764fc576 100644 --- a/workbench/sim-world/scenarios/in-flight-before-decision.ts +++ b/workbench/sim-world/scenarios/in-flight-before-decision.ts @@ -13,10 +13,11 @@ export const scenario: ScenarioSpec = { 'The receiver commits while the orchestrator is held at the produced ' + 'point of C, so by the time C is checked the hole has closed and the log ' + 'holds an event the writer never loaded. The watermark guard is on and ' + - 'passes anyway, by construction: the marker moves to the ULID time of the ' + - "hook, which sorts at or below the writer's own snapshot, so " + - '`snapshot.updatedAt < marker` is false. It corrupts — the same corruption as ' + - 'the doc-23 pair, reached without a stale read. ' + + 'never gets to speak: the log has a hole in it from the moment the ' + + 'receiver takes its position, so the next replay refuses the log before ' + + 'any write of its own is checked. What the fence would have caught, the ' + + 'gap audit catches earlier and more bluntly — the run fails rather than ' + + 'following a log it cannot follow into the wrong branch. ' + 'Under an append-only log there is no position to be spoken for: the hook ' + 'commits after the timeout and therefore sorts after it, the log says the ' + 'timeout won, and the settle branch the run took is the one the log ' + From d8eb897d25558a0c2bfab20acf47270c9e98f5fc Mon Sep 17 00:00:00 2001 From: Peter Wielander Date: Wed, 12 Aug 2026 18:48:33 -0700 Subject: [PATCH 2/6] [world-vercel] Call the v5 event API Every `events` read and write goes to `/api/v5/...`. The wire frame is unchanged, so the file keeps its name and its schemas: what the route selects is server behavior, not a format. On v5 the backend no longer writes the Step and Wait rows on the event path, so a v5 run reads back with no steps through `steps.list` until the read model is rebuilt from events. `runs.cancel` stays on `/v4/runs/cancel`: there is no v5 mirror of it. `WORKFLOW_EVENTS_TRANSPORT=ws` reaches v5 too, decided on the backend side. Its endpoint stays versioned on its own, which is about the frame envelope rather than which write path a frame reaches. `WORKFLOW_SERVER_URL_OVERRIDE` is pinned to the branch serving /v5 for the life of this PR and must be reverted to '' before merge. Co-Authored-By: Claude Opus 5 --- .changeset/world-vercel-v5-events.md | 5 ++ .gitignore | 4 ++ .../world-vercel/src/events-v4-ws.test.ts | 4 +- packages/world-vercel/src/events-v4.test.ts | 46 ++++++++--------- packages/world-vercel/src/events-v4.ts | 39 +++++++++++---- packages/world-vercel/src/events.test.ts | 50 +++++++++---------- .../src/trace-propagation.test.ts | 4 +- packages/world-vercel/src/utils.ts | 5 +- .../world-vercel/src/ws-transport-enabled.ts | 7 +++ packages/world-vercel/src/ws-transport.ts | 4 +- 10 files changed, 104 insertions(+), 64 deletions(-) create mode 100644 .changeset/world-vercel-v5-events.md diff --git a/.changeset/world-vercel-v5-events.md b/.changeset/world-vercel-v5-events.md new file mode 100644 index 0000000000..9b1e4a9fcf --- /dev/null +++ b/.changeset/world-vercel-v5-events.md @@ -0,0 +1,5 @@ +--- +'@workflow/world-vercel': patch +--- + +Send event reads and writes to the Vercel World's v5 event API. diff --git a/.gitignore b/.gitignore index a3f813e31a..15c38c08b0 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,10 @@ workbench/nextjs-*/public/.well-known/workflow workbench/sveltekit/static/.well-known/workflow +# E2E diagnostics sidecar files (written to the repo root by writeDiagnosticsSidecar() +# in packages/core/e2e/utils.ts during e2e runs) +e2e-diagnostics-*.json + # Event log race repro output (written to the repo root by the harness and by # scripts/event-log-race-repro-local.sh) event-log-race-repro-results.json diff --git a/packages/world-vercel/src/events-v4-ws.test.ts b/packages/world-vercel/src/events-v4-ws.test.ts index 7efa0a2361..4ac8df5809 100644 --- a/packages/world-vercel/src/events-v4-ws.test.ts +++ b/packages/world-vercel/src/events-v4-ws.test.ts @@ -129,7 +129,7 @@ describe('transport gate', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/step_completed', + path: '/api/v5/runs/wrun_1/events/step_completed', method: 'POST', }) .reply(200, materializedBody(), { @@ -183,7 +183,7 @@ describe('createWorkflowRunEventV4 over ws', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/step_completed', + path: '/api/v5/runs/wrun_1/events/step_completed', method: 'POST', }) .reply(200, materializedBody(), { diff --git a/packages/world-vercel/src/events-v4.test.ts b/packages/world-vercel/src/events-v4.test.ts index 120ab1a384..1d57eff0c8 100644 --- a/packages/world-vercel/src/events-v4.test.ts +++ b/packages/world-vercel/src/events-v4.test.ts @@ -293,7 +293,7 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?returnAll=true', + path: '/api/v5/runs/wrun_1/events?returnAll=true', method: 'GET', }) .reply(200, frames, { @@ -327,7 +327,7 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?returnAll=true', + path: '/api/v5/runs/wrun_1/events?returnAll=true', method: 'GET', }) .reply( @@ -381,7 +381,7 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?returnAll=true', + path: '/api/v5/runs/wrun_1/events?returnAll=true', method: 'GET', }) .reply(200, frames, { @@ -412,7 +412,7 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?returnAll=true', + path: '/api/v5/runs/wrun_1/events?returnAll=true', method: 'GET', }) .reply(200, frames, { @@ -455,7 +455,7 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?limit=500', + path: '/api/v5/runs/wrun_1/events?limit=500', method: 'GET', }) .reply(200, frames, { @@ -480,7 +480,7 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?returnAll=true', + path: '/api/v5/runs/wrun_1/events?returnAll=true', method: 'GET', }) .reply( @@ -504,7 +504,7 @@ describe('getWorkflowRunEventsV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?returnAll=true&cursor=eid%3Aevnt_1', + path: '/api/v5/runs/wrun_1/events?returnAll=true&cursor=eid%3Aevnt_1', method: 'GET', }) .reply( @@ -579,7 +579,7 @@ describe('getEventsByCorrelationIdV4 over HTTP', () => { .intercept({ path: (path) => { requestedPaths.push(path); - return path.startsWith('/api/v4/events?'); + return path.startsWith('/api/v5/events?'); }, method: 'GET', }) @@ -643,7 +643,7 @@ describe('getEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/evnt_1?remoteRefBehavior=resolve', + path: '/api/v5/runs/wrun_1/events/evnt_1?remoteRefBehavior=resolve', method: 'GET', }) .reply(200, frames, { @@ -685,7 +685,7 @@ describe('v4 transport uses global fetch (observability)', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?returnAll=true', + path: '/api/v5/runs/wrun_1/events?returnAll=true', method: 'GET', }) .reply(200, encodeFrame({ _end: 1, hasMore: false }, new Uint8Array(0)), { @@ -704,7 +704,7 @@ describe('v4 transport uses global fetch (observability)', () => { expect(fetchSpy).toHaveBeenCalledTimes(1); const [calledUrl, calledInit] = fetchSpy.mock.calls[0]; - expect(String(calledUrl)).toContain('/api/v4/runs/wrun_1/events'); + expect(String(calledUrl)).toContain('/api/v5/runs/wrun_1/events'); agent.assertNoPendingInterceptors(); // Cache-busting header must be set so Next.js fetch memoization / Data @@ -727,7 +727,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { .intercept({ // The event type rides in the URL purely as an observability hint // (access logs / traces); the frame meta stays authoritative. - path: '/api/v4/runs/wrun_1/events/step_completed', + path: '/api/v5/runs/wrun_1/events/step_completed', method: 'POST', }) .reply( @@ -784,7 +784,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/hook_created', + path: '/api/v5/runs/wrun_1/events/hook_created', method: 'POST', }) .reply( @@ -828,7 +828,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_started', + path: '/api/v5/runs/wrun_1/events/run_started', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -899,7 +899,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_started', + path: '/api/v5/runs/wrun_1/events/run_started', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -936,7 +936,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_started', + path: '/api/v5/runs/wrun_1/events/run_started', method: 'POST', headers: (headers) => headers.accept === '*/*', }) @@ -995,7 +995,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/wait_created', + path: '/api/v5/runs/wrun_1/events/wait_created', method: 'POST', }) .reply( @@ -1056,7 +1056,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/wait_created', + path: '/api/v5/runs/wrun_1/events/wait_created', method: 'POST', }) .reply( @@ -1108,7 +1108,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/wait_created', + path: '/api/v5/runs/wrun_1/events/wait_created', method: 'POST', }) .reply( @@ -1171,7 +1171,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/wait_created', + path: '/api/v5/runs/wrun_1/events/wait_created', method: 'POST', }) .reply( @@ -1226,7 +1226,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/wait_created', + path: '/api/v5/runs/wrun_1/events/wait_created', method: 'POST', }) .reply( @@ -1283,7 +1283,7 @@ describe('createWorkflowRunEventV4 over HTTP', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/wait_created', + path: '/api/v5/runs/wrun_1/events/wait_created', method: 'POST', }) .reply( @@ -1368,7 +1368,7 @@ describe('v4 POST frame meta forwards every field the splitter produces', () => agent .get(origin) .intercept({ - path: `/api/v4/runs/wrun_1/events/${data.eventType}`, + path: `/api/v5/runs/wrun_1/events/${data.eventType}`, method: 'POST', }) .reply( diff --git a/packages/world-vercel/src/events-v4.ts b/packages/world-vercel/src/events-v4.ts index e8de126e6b..7510a0fb4d 100644 --- a/packages/world-vercel/src/events-v4.ts +++ b/packages/world-vercel/src/events-v4.ts @@ -120,6 +120,27 @@ async function fetchV4( const EVENT_ID_HEADER = 'x-wf-event-id'; const MAX_EVENTS_HEADER = 'x-wf-max-events'; +/** + * The route version every event request in this file is sent to. + * + * Distinct from the *frame* version, which is still v4 and is what the rest of + * this file is named after: `/v5` serves the same wire format, validated by the + * same schemas, so nothing about encoding or decoding changes with it. What + * changes is the backend's own bookkeeping behind the route. Under `/v4` an + * event write also writes the Step or Wait row that arbitrated it, and the + * write's atomicity came from a conditional update on that row; under `/v5` the + * event's position in its run's log is the arbitration (see + * `SPEC_VERSION_SUPPORTS_SLOT_IDENTITY`), so the row is no longer on the write + * path. + * + * The consequence a reader of this file should know: rows are still the backend's + * read model for `steps.list`, and `/v5` stops filling them. A run created by + * this adapter reports its steps through the event log, which is what the + * runtime replays from, and reports nothing through the row projection that + * `wf inspect` and the dashboard read. + */ +const EVENTS_ROUTE_VERSION = 'v5'; + interface CreateEventV4InputBase { // runId is required even for run_created, because the payload is keyed under the runId runId: string; @@ -639,7 +660,7 @@ export function throwForErrorResponse( } /** - * POST /api/v4/runs/:runId/events/:eventType + * POST /api/v5/runs/:runId/events/:eventType * * Sends the full request as a single v4 frame and validates the materialized * CBOR response. @@ -670,7 +691,7 @@ async function postWorkflowRunEventV4( input.payload ?? new Uint8Array(0) ); - const url = `${baseUrl}/v4/runs/${encodeURIComponent(input.runId)}/events/${encodeURIComponent(input.eventType)}`; + const url = `${baseUrl}/${EVENTS_ROUTE_VERSION}/runs/${encodeURIComponent(input.runId)}/events/${encodeURIComponent(input.eventType)}`; return fetchV4( url, { method: 'POST', headers, body: frame }, @@ -917,7 +938,7 @@ export type HookReceivedPreloadV4Result = }; /** - * POST /api/v4/runs/:runId/events/hook_received with the v4-frame `Accept`, + * POST /api/v5/runs/:runId/events/hook_received with the v4-frame `Accept`, * consuming either response mode. * * A server that supports the lazy-hook replay stream answers the consumer's @@ -970,7 +991,7 @@ function readHeader( } /** - * GET /api/v4/runs/:runId/events/:eventId + * GET /api/v5/runs/:runId/events/:eventId * * Returns one validated event. The wire format is identical to a single LIST * frame so the server can stream the payload back without buffering. @@ -984,7 +1005,7 @@ export async function getEventV4( const { baseUrl, headers } = await getHttpConfig(config); const url = - `${baseUrl}/v4/runs/${encodeURIComponent(runId)}/events/${encodeURIComponent(eventId)}` + + `${baseUrl}/${EVENTS_ROUTE_VERSION}/runs/${encodeURIComponent(runId)}/events/${encodeURIComponent(eventId)}` + `?remoteRefBehavior=${remoteRefBehavior}`; const response = await fetchV4( url, @@ -1113,7 +1134,7 @@ function paginationToQuery(params: ListEventsV4Params): string { } /** - * GET /api/v4/runs/:runId/events + * GET /api/v5/runs/:runId/events * * Parses the binary-frame stream into validated events plus the pagination * cursor from the sentinel frame. @@ -1133,7 +1154,7 @@ export async function getWorkflowRunEventsV4( while (true) { const url = - `${baseUrl}/v4/runs/${encodeURIComponent(runId)}/events` + + `${baseUrl}/${EVENTS_ROUTE_VERSION}/runs/${encodeURIComponent(runId)}/events` + paginationToQuery({ ...params, cursor }); try { const page = await consumeListFrameStream( @@ -1159,7 +1180,7 @@ export async function getWorkflowRunEventsV4( } /** - * GET /api/v4/events?correlationId=...&runId=... + * GET /api/v5/events?correlationId=...&runId=... * * Same frame stream as getWorkflowRunEventsV4 but selected by correlation id * instead of run id alone. Used by the storage adapter's @@ -1183,7 +1204,7 @@ export async function getEventsByCorrelationIdV4( sp.set('correlationId', correlationId); sp.set('runId', runId); appendListParams(sp, params); - const url = `${baseUrl}/v4/events?${sp.toString()}`; + const url = `${baseUrl}/${EVENTS_ROUTE_VERSION}/events?${sp.toString()}`; const events: Event[] = []; const page = await consumeListFrameStream( url, diff --git a/packages/world-vercel/src/events.test.ts b/packages/world-vercel/src/events.test.ts index 5fea9675a4..68df853faa 100644 --- a/packages/world-vercel/src/events.test.ts +++ b/packages/world-vercel/src/events.test.ts @@ -193,7 +193,7 @@ describe('createWorkflowRunEvent slot snapshot wire fields', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_started', + path: '/api/v5/runs/wrun_1/events/run_started', method: 'POST', }) .reply( @@ -235,7 +235,7 @@ describe('createWorkflowRunEvent slot snapshot wire fields', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_started', + path: '/api/v5/runs/wrun_1/events/run_started', method: 'POST', }) .reply( @@ -354,7 +354,7 @@ describe('createWorkflowRunEvent result contract', () => { agent .get(ORIGIN) .intercept({ - path: `/api/v4/runs/wrun_1/events/${eventType}`, + path: `/api/v5/runs/wrun_1/events/${eventType}`, method: 'POST', }) .reply(200, createEventBody(data as AnyEventRequest, response), { @@ -385,7 +385,7 @@ async function postStepStartedMeta( agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/step_started', + path: '/api/v5/runs/wrun_1/events/step_started', method: 'POST', }) .reply( @@ -469,7 +469,7 @@ describe('createWorkflowRunEvent replayDivergenceCount wire field', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_completed', + path: '/api/v5/runs/wrun_1/events/run_completed', method: 'POST', }) .reply( @@ -784,7 +784,7 @@ describe('createWorkflowRunEvent response coercion', () => { agent .get(ORIGIN) .intercept({ - path: `/api/v4/runs/${taggedRunId}/events/run_created`, + path: `/api/v5/runs/${taggedRunId}/events/run_created`, method: 'POST', }) .reply( @@ -847,7 +847,7 @@ describe('createWorkflowRunEvent response coercion', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_started', + path: '/api/v5/runs/wrun_1/events/run_started', method: 'POST', }) .reply( @@ -892,7 +892,7 @@ describe('createWorkflowRunEvent response coercion', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_started', + path: '/api/v5/runs/wrun_1/events/run_started', method: 'POST', }) .reply( @@ -956,7 +956,7 @@ describe('createWorkflowRunEvent response coercion', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/run_started', + path: '/api/v5/runs/wrun_1/events/run_started', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -1051,7 +1051,7 @@ describe('createWorkflowRunEvent response coercion', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/wait_created', + path: '/api/v5/runs/wrun_1/events/wait_created', method: 'POST', }) .reply( @@ -1109,7 +1109,7 @@ describe('createWorkflowRunEvent resolveData', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/step_completed', + path: '/api/v5/runs/wrun_1/events/step_completed', method: 'POST', }) .reply( @@ -1193,7 +1193,7 @@ describe('getWorkflowRunEvents remoteRefBehavior mapping', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events', + path: '/api/v5/runs/wrun_1/events', method: 'GET', query: { returnAll: 'true', remoteRefBehavior: 'lazy' }, }) @@ -1227,7 +1227,7 @@ describe('getWorkflowRunEvents remoteRefBehavior mapping', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events', + path: '/api/v5/runs/wrun_1/events', method: 'GET', query: { returnAll: 'true', remoteRefBehavior: 'resolve' }, }) @@ -1253,7 +1253,7 @@ describe('getWorkflowRunEvents remoteRefBehavior mapping', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events', + path: '/api/v5/runs/wrun_1/events', method: 'GET', query: { returnAll: 'true', remoteRefBehavior: 'resolve' }, }) @@ -1288,7 +1288,7 @@ describe('getWorkflowRunEvents remoteRefBehavior mapping', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events', + path: '/api/v5/runs/wrun_1/events', method: 'GET', query: { returnAll: 'true', remoteRefBehavior: 'resolve' }, }) @@ -1341,7 +1341,7 @@ describe('getWorkflowRunEvents legacy structured-error compatibility', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events', + path: '/api/v5/runs/wrun_1/events', method: 'GET', query: { returnAll: 'true', remoteRefBehavior: 'resolve' }, }) @@ -1410,7 +1410,7 @@ describe('getWorkflowRunEvents hasMore mapping', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events', + path: '/api/v5/runs/wrun_1/events', method: 'GET', // These tests omit the limit and use the default resolveData // ('all' → resolve); match both translated query params. @@ -1499,7 +1499,7 @@ describe('getWorkflowRunEvents by correlation id is scoped to the run', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/events', + path: '/api/v5/events', method: 'GET', query: { correlationId: 'step_001', @@ -1625,7 +1625,7 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { .intercept({ // The headers matcher proves the frame Accept was sent — an // unmatched request would leave the interceptor pending. - path: '/api/v4/runs/wrun_1/events/hook_received', + path: '/api/v5/runs/wrun_1/events/hook_received', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -1699,7 +1699,7 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/hook_received', + path: '/api/v5/runs/wrun_1/events/hook_received', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -1761,7 +1761,7 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/hook_received', + path: '/api/v5/runs/wrun_1/events/hook_received', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -1769,7 +1769,7 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/hook_received', + path: '/api/v5/runs/wrun_1/events/hook_received', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -1800,7 +1800,7 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/hook_received', + path: '/api/v5/runs/wrun_1/events/hook_received', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -1847,7 +1847,7 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/hook_received', + path: '/api/v5/runs/wrun_1/events/hook_received', method: 'POST', headers: { accept: V4_FRAME_CONTENT_TYPE }, }) @@ -1884,7 +1884,7 @@ describe('createWorkflowRunEvent hook_received replay preload', () => { agent .get(ORIGIN) .intercept({ - path: '/api/v4/runs/wrun_1/events/hook_received', + path: '/api/v5/runs/wrun_1/events/hook_received', method: 'POST', }) .reply( diff --git a/packages/world-vercel/src/trace-propagation.test.ts b/packages/world-vercel/src/trace-propagation.test.ts index c071d59960..9ac26b222d 100644 --- a/packages/world-vercel/src/trace-propagation.test.ts +++ b/packages/world-vercel/src/trace-propagation.test.ts @@ -155,7 +155,7 @@ describe('v4 event requests (fetchV4) trace propagation', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events?returnAll=true', + path: '/api/v5/runs/wrun_1/events?returnAll=true', method: 'GET', }) .reply(200, encodeFrame({ _end: 1, hasMore: false }, new Uint8Array(0)), { @@ -213,7 +213,7 @@ describe('v4 event requests (fetchV4) trace propagation', () => { agent .get(origin) .intercept({ - path: '/api/v4/runs/wrun_1/events/hook_received', + path: '/api/v5/runs/wrun_1/events/hook_received', method: 'POST', }) .reply(200, encodeFrame({ _end: 1, hasMore: false }, new Uint8Array(0)), { diff --git a/packages/world-vercel/src/utils.ts b/packages/world-vercel/src/utils.ts index b9d46fae48..5adf9b6c56 100644 --- a/packages/world-vercel/src/utils.ts +++ b/packages/world-vercel/src/utils.ts @@ -32,7 +32,10 @@ import { version } from './version.js'; * `main` — rewritten by external CI for branch-deployment testing. * Prefer `VERCEL_WORKFLOW_SERVER_URL` for deployment-time configuration. */ -export const WORKFLOW_SERVER_URL_OVERRIDE = ''; +// TEMPORARY — must be reverted to '' before this PR merges. The /v5 event +// routes this adapter now calls only exist on the backend branch below. +export const WORKFLOW_SERVER_URL_OVERRIDE = + 'https://workflow-server-git-peter-no-step-wait-materialization.vercel.sh'; /** * HTTP methods that are safe to transparently re-issue inside the adapter. diff --git a/packages/world-vercel/src/ws-transport-enabled.ts b/packages/world-vercel/src/ws-transport-enabled.ts index ed370ee56f..2938c67d11 100644 --- a/packages/world-vercel/src/ws-transport-enabled.ts +++ b/packages/world-vercel/src/ws-transport-enabled.ts @@ -13,6 +13,13 @@ * streamed, sentinel-terminated multi-frame response doesn't map onto a single * WS message. * + * The socket carries the same frames to the same route as the HTTP branch. Its + * endpoint is versioned on its own (`/websockets/v1`, see `toEventsWsUrl`), and + * each frame is forwarded into the newest events route — the one + * `EVENTS_ROUTE_VERSION` in `events-v4.ts` names for the HTTP branch. Which + * route is not a thing the client picks per transport, and should not become + * one: a run may write over either and must get the same semantics from both. + * * **Known gap: WS writes open no client span.** The upgrade carries W3C trace * context (see `resolveUpgradeHeaders`), so server spans still join the caller's * trace, but the HTTP branch's `instrumentedFetch` also opens an OTEL CLIENT diff --git a/packages/world-vercel/src/ws-transport.ts b/packages/world-vercel/src/ws-transport.ts index d612e98dbf..891395747b 100644 --- a/packages/world-vercel/src/ws-transport.ts +++ b/packages/world-vercel/src/ws-transport.ts @@ -651,9 +651,9 @@ export function resetWsEventsTransportsForTest(): void { } /** - * Derive the WS endpoint URL for one run from the http(s) base URL used for v4 + * Derive the WS endpoint URL for one run from the http(s) base URL used for the * REST calls. `/websockets/v1/runs/:runId` is versioned independently of the - * `v4` REST API these frames are forwarded into (server `docs/ws-protocol.md`). + * REST API these frames are forwarded into (server `docs/ws-protocol.md`). * Scoped to one run because one client instance only ever drives one run, so * `runId` belongs on the connection rather than on every frame. */ From 3ba9ea08a6a1d602dc6beb0225e0d3daffb1c56d Mon Sep 17 00:00:00 2001 From: Peter Wielander Date: Wed, 12 Aug 2026 18:48:46 -0700 Subject: [PATCH 3/6] [core] State the attempt on step_started; read steps back where they now live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retry ceiling is decided against a step's attempt number, and that number came back from the World: the step row carried a counter its start patch incremented. A World that stores only the log has no such counter, and the attempt is on no event unless the writer puts it there — so every attempt reported 1, no step ever exhausted its budget, and any failing step retried forever. The writer already knows the number (it is what the pre-body ceiling is enforced against), so it states it. Omitted when the caller supplied none, so a World that counts for itself is not overwritten by a guess. The e2e step assertions move off the storage listing. `--withData` was forcing it, for two reasons that both changed: `attempt` is now on the analytics listing, and a step's error is a payload no metadata listing ever carries — that one reads the `step_failed` event, which holds it on every World. Co-Authored-By: Claude Opus 5 --- .changeset/step-started-attempt.md | 5 ++ packages/core/e2e/e2e.test.ts | 46 +++++---------- packages/core/e2e/utils.ts | 44 +++++++++++++- .../core/src/runtime/step-executor.test.ts | 58 +++++++++++++++++++ packages/core/src/runtime/step-executor.ts | 22 ++++++- 5 files changed, 141 insertions(+), 34 deletions(-) create mode 100644 .changeset/step-started-attempt.md diff --git a/.changeset/step-started-attempt.md b/.changeset/step-started-attempt.md new file mode 100644 index 0000000000..c7cd4d2816 --- /dev/null +++ b/.changeset/step-started-attempt.md @@ -0,0 +1,5 @@ +--- +'@workflow/core': patch +--- + +State the attempt number on `step_started`, so a World that keeps no step row can still report which attempt a step is on. diff --git a/packages/core/e2e/e2e.test.ts b/packages/core/e2e/e2e.test.ts index 18b0650099..13d66a9065 100644 --- a/packages/core/e2e/e2e.test.ts +++ b/packages/core/e2e/e2e.test.ts @@ -45,6 +45,7 @@ import { isLocalDeployment, setupRunTracking, setupWorld, + stepFailedError, trackRun, writeDiagnosticsSidecar, } from './utils'; @@ -1314,20 +1315,10 @@ describe('e2e', () => { expect(result.stack).not.toContain('99_e2e.ts'); } - // Verify step failed via CLI (--withData needed to resolve errorRef) - const { json: steps } = await cliInspectJson( - `steps --runId ${run.runId} --withData` - ); - const failedStep = steps.find((s: any) => - s.stepName.includes('errorStepFn') - ); - expect(failedStep.status).toBe('failed'); - // The CLI hydrates `step.error` from the serialization pipeline. - // Errors thrown from steps are wrapped in `FatalError` by the - // step executor, which serializes via the Instance reducer - // (`{ classId, data }`); the CLI surfaces unregistered class - // instances as placeholders with the original `data` payload. - const errorData = failedStep.error.data ?? failedStep.error; + // Verify the step failed, and that its error survived the write. + // The error comes from the event log rather than the step listing + // — see `stepFailedError`. + const errorData = await stepFailedError(run.runId, 'errorStepFn'); expect(errorData.message).toContain('Step error message'); // Step error stack should contain the original step function name @@ -1376,17 +1367,11 @@ describe('e2e', () => { expect(result.stack).not.toContain('helpers.ts'); } - // Verify step failed via CLI - same stack info available there too (--withData needed to resolve errorRef) - const { json: steps } = await cliInspectJson( - `steps --runId ${run.runId} --withData` - ); - const failedStep = steps.find((s: any) => - s.stepName.includes('stepThatThrowsFromHelper') + // Same stack info is available on the durable event too. + const errorData = await stepFailedError( + run.runId, + 'stepThatThrowsFromHelper' ); - expect(failedStep.status).toBe('failed'); - // See note above: serialized step errors arrive as Instance refs - // when the FatalError class isn't registered in this process. - const errorData = failedStep.error.data ?? failedStep.error; if (hasNestedStepStackFrames()) { expect(errorData.stack).toContain('throwErrorFromStep'); } @@ -1418,12 +1403,11 @@ describe('e2e', () => { expect(result.finalAttempt).toBe(3); - // --withData forces the storage-backed listing: the analytics - // listing may omit the attempt column entirely (it is optional in - // the analytics schema), so only the durable step entity can be - // asserted on. Poll because rows for a just-finished run can lag. + // The analytics listing reports `attempt` as the number of starts + // the log holds, which is the number the step row's counter held. + // Poll because rows for a just-finished run can lag. const steps = await cliInspectJsonUntil( - `steps --runId ${run.runId} --withData`, + `steps --runId ${run.runId}`, (json) => json.some( (s: any) => @@ -1455,10 +1439,8 @@ describe('e2e', () => { // (which inspect the value inside the SWC-instrumented workflow). // Here we only assert step lifecycle behavior. - // --withData forces the storage-backed listing — see the - // retry-success test above. const steps = await cliInspectJsonUntil( - `steps --runId ${run.runId} --withData`, + `steps --runId ${run.runId}`, (json) => json.some( (s: any) => diff --git a/packages/core/e2e/utils.ts b/packages/core/e2e/utils.ts index 910dd1f94b..4aac10c4ff 100644 --- a/packages/core/e2e/utils.ts +++ b/packages/core/e2e/utils.ts @@ -5,7 +5,7 @@ import { setTimeout as sleep } from 'node:timers/promises'; import { fileURLToPath } from 'node:url'; import { createWorkflowUrl } from '@workflow/utils'; import { createWorld as createVercelTestWorld } from '@workflow/world-vercel'; -import { onTestFailed } from 'vitest'; +import { expect, onTestFailed } from 'vitest'; import { getTrustedSourcesHeaders } from '../../../scripts/trusted-sources-headers.mjs'; import type { Run } from '../src/runtime'; import { getWorld, setWorld } from '../src/runtime'; @@ -876,3 +876,45 @@ export const cliInspectJsonUntil = async ( await new Promise((resolve) => setTimeout(resolve, intervalMs)); } }; + +/** + * The resolved error payload of the `step_failed` event for the step whose + * name contains `stepName`. + * + * Read from the event log rather than the step listing. A step's error is a + * payload, and the metadata-only analytics listing that backs `inspect steps` + * does not carry one; against a World that materializes no step rows there is + * no second listing to fall back to. The event log holds every payload on + * every World, so this is the one place the assertion can be made everywhere. + * + * Polls, because the write lands slightly after the run settles. + */ +export const stepFailedError = async ( + runId: string, + stepName: string + // biome-ignore lint/suspicious/noExplicitAny: raw CLI JSON +): Promise => { + // biome-ignore lint/suspicious/noExplicitAny: raw CLI JSON + const isMatch = (event: any): boolean => + event.eventType === 'step_failed' && + typeof event.eventData?.stepName === 'string' && + event.eventData.stepName.includes(stepName); + + const events = await cliInspectJsonUntil( + `events --run ${runId} --withData`, + // biome-ignore lint/suspicious/noExplicitAny: raw CLI JSON + (json: any[]) => json.some(isMatch) + ); + // biome-ignore lint/suspicious/noExplicitAny: raw CLI JSON + const failed = (events as any[]).find(isMatch); + expect( + failed, + `no step_failed event for a step named "${stepName}"` + ).toBeDefined(); + // Errors thrown from steps are wrapped in `FatalError` by the step executor, + // which serializes via the Instance reducer (`{ classId, data }`); the CLI + // surfaces unregistered class instances as placeholders carrying the + // original `data` payload. + const error = failed.eventData.error; + return error?.data ?? error; +}; diff --git a/packages/core/src/runtime/step-executor.test.ts b/packages/core/src/runtime/step-executor.test.ts index ad80d8e1b7..a1ca74882f 100644 --- a/packages/core/src/runtime/step-executor.test.ts +++ b/packages/core/src/runtime/step-executor.test.ts @@ -247,3 +247,61 @@ describe('executeStep — compute instance stamping', () => { } }); }); + +describe('executeStep — attempt stamping', () => { + afterEach(() => { + counter += 1; + }); + + it('states the attempt on step_started', async () => { + // A World that stores only the log cannot count attempts for itself: the + // number is on no event unless the writer puts it there, and it is the + // number the retry ceiling is decided against. See `attemptStamp`. + const world = makeWorld(); + const stepName = uniqueStepName(); + const { runId, stepId } = await setupRunningStep({ + world, + stepName, + onBody: () => {}, + }); + + await executeStep({ + world, + workflowRunId: runId, + workflowName: 'wf', + workflowStartedAt: Date.now(), + stepId, + stepName, + authoritativeAttempt: 3, + }); + + const [started] = await eventsFor(world, runId, stepId, 'step_started'); + expect( + (started?.eventData as { attempt?: number } | undefined)?.attempt + ).toBe(3); + }); + + it('states no attempt when the caller supplied none', async () => { + // A World that counts for itself must not have its count overwritten by a + // guess, so the field is omitted rather than defaulted. + const world = makeWorld(); + const stepName = uniqueStepName(); + const { runId, stepId } = await setupRunningStep({ + world, + stepName, + onBody: () => {}, + }); + + await executeStep({ + world, + workflowRunId: runId, + workflowName: 'wf', + workflowStartedAt: Date.now(), + stepId, + stepName, + }); + + const [started] = await eventsFor(world, runId, stepId, 'step_started'); + expect(started?.eventData).not.toHaveProperty('attempt'); + }); +}); diff --git a/packages/core/src/runtime/step-executor.ts b/packages/core/src/runtime/step-executor.ts index 63295800ce..d14f355df5 100644 --- a/packages/core/src/runtime/step-executor.ts +++ b/packages/core/src/runtime/step-executor.ts @@ -603,6 +603,24 @@ export async function executeStep( const startEventParams: CreateEventParams = { computeInstanceId: COMPUTE_INSTANCE_ID, }; + // Which attempt this start is, stated by the writer. + // + // A World that keeps a step row can own this itself, by incrementing a + // counter as it applies the start, and one that stores only the log + // cannot: the attempt is not on any event unless someone puts it there, + // and the run's log is not a cheap thing to count on a step's hot path. + // So the writer states it. It already knows the number — the retry + // ceiling below is enforced against exactly this value before the body + // runs (see StepExecutorParams.authoritativeAttempt) — and stating it + // costs one integer on a frame that is already being sent. + // + // Omitted rather than defaulted when the caller did not supply one: a + // World that counts for itself must not have its own count overwritten + // by a guess, and one that does not read back `1` anyway. + const attemptStamp = + params.authoritativeAttempt !== undefined + ? { attempt: params.authoritativeAttempt } + : {}; // `Date.now()` taken immediately before the `step_started` create is // issued (either path below) — anchors RSFS's end point. See // StepLatencyEventData.rsfs and the call sites below. @@ -658,6 +676,7 @@ export async function executeStep( stepName, workflowName, input: params.lazyStepInput, + ...attemptStamp, // Inline-ownership stamp — see StepExecutorParams.ownerMessageId. ...(params.ownerMessageId !== undefined ? { ownerMessageId: params.ownerMessageId } @@ -720,9 +739,10 @@ export async function executeStep( stepName, workflowName, input: params.lazyStepInput, + ...attemptStamp, ...ownershipStamp, } - : { stepName, ...ownershipStamp }, + : { stepName, ...attemptStamp, ...ownershipStamp }, }, // Guard the claim — see StepExecutorParams.slotSnapshot. A // stale (412) rejection is intentionally NOT translated by From 99c641afaed8663738215220cc4449a28f25d9c1 Mon Sep 17 00:00:00 2001 From: Peter Wielander Date: Wed, 12 Aug 2026 18:48:58 -0700 Subject: [PATCH 4/6] [core] Ignore a step event behind the step's result; show the log that corrupted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two invocations can run one step: a queue-dispatched step has no claim to lose, its message can be delivered twice, and both deliveries start a step that already exists. When the loser's body then fails behind the winner's result, the `step_retrying` it records repeats no class, so no consumer wants it and the replay was calling it divergence — failing a run that had finished. Measured rather than argued: with the backend's inline claim fenced and this reverted, `event-log-race-repro` corrupts 12 of 12 storm runs; with it, 0. It changes one deliberate expectation — a start behind a completion that repeats no class used to be divergence. The harness now prints a failing attempt's event-log tail. It reported `CORRUPTED_EVENT_LOG` and nothing else: the message the runtime failed on does not survive into the run row, and the observability tables are not populated for a preview backend, so which two writes collided had to be inferred from the shape of the code. Inferring it is how two diagnoses went wrong, and the tail is what showed the remaining failure is an infinite retry loop rather than the corruption it was assumed to be. Co-Authored-By: Claude Opus 5 --- .changeset/straggler-after-step-outcome.md | 5 + .../core/e2e/event-log-race-repro.test.ts | 56 ++++++++++ packages/core/src/events-consumer.test.ts | 105 ++++++++++++++++-- packages/core/src/events-consumer.ts | 54 ++++++++- 4 files changed, 209 insertions(+), 11 deletions(-) create mode 100644 .changeset/straggler-after-step-outcome.md diff --git a/.changeset/straggler-after-step-outcome.md b/.changeset/straggler-after-step-outcome.md new file mode 100644 index 0000000000..b3a082d1e6 --- /dev/null +++ b/.changeset/straggler-after-step-outcome.md @@ -0,0 +1,5 @@ +--- +'@workflow/core': patch +--- + +Ignore a `step_started` or `step_retrying` written behind a step's recorded result instead of failing the run with `CORRUPTED_EVENT_LOG`. A losing attempt that outlives the winning one produces exactly that log, and neither writer was wrong. diff --git a/packages/core/e2e/event-log-race-repro.test.ts b/packages/core/e2e/event-log-race-repro.test.ts index b577df7b5c..a00a39c80f 100644 --- a/packages/core/e2e/event-log-race-repro.test.ts +++ b/packages/core/e2e/event-log-race-repro.test.ts @@ -132,6 +132,16 @@ interface ReproRunResult { /** Diagnostics from the driver: how much racing pressure the attempt actually * applied. A run that corrupts with `stragglers: 0` in every round would mean * the step-count amplifier was not the trigger. */ + /** + * The tail of the run's event log, for an attempt that did not complete. + * + * Without it this job is signature-blind: `errorCode` says + * `CORRUPTED_EVENT_LOG` and the message the runtime failed on does not + * survive into the run row, so a reader is left inferring which two writes + * collided from the shape of the code. The corruption is always an ordering + * between events, so the ordering is the evidence. + */ + timeline?: string[]; pressure?: { resumesSent: number; resumesFailed: number; @@ -392,6 +402,50 @@ function validateStormReturn(value: unknown): { return { stragglers }; } +/** + * The last {@link TIMELINE_TAIL} events of a run, as `slot type corr#short`. + * + * Correlation ids are shortened to their last six characters: the corruption + * is about which events share one, and the full ULID makes a timeline + * unreadable at a glance without telling the reader anything more. + * + * Best-effort. This runs only for an attempt that already failed, so a listing + * error must not replace the outcome being reported with a transport one. + */ +const TIMELINE_TAIL = 40; + +async function readTimeline( + world: Awaited>, + runId: string +): Promise { + try { + const events: string[] = []; + let cursor: string | null | undefined; + do { + const page = await world.events.list({ + runId, + pagination: { cursor: cursor ?? undefined, sortOrder: 'asc' }, + }); + for (const event of page.data) { + const corr = event.correlationId; + events.push( + `${event.eventId.replace(/^evnt_0*/, '')} ${event.eventType}` + + (corr ? ` ${corr.slice(-6)}` : '') + ); + } + cursor = page.hasMore ? page.cursor : undefined; + } while (cursor); + return events.length > TIMELINE_TAIL + ? [ + `… ${events.length - TIMELINE_TAIL} earlier`, + ...events.slice(-TIMELINE_TAIL), + ] + : events; + } catch { + return undefined; + } +} + async function pollTerminalRun( run: Run, startedAt: number, @@ -439,6 +493,7 @@ async function pollTerminalRun( errorMessage: failure.error?.message, errorName: failure.error?.name, durationMs: Date.now() - startedAt, + timeline: await readTimeline(world, run.runId), }; } @@ -460,6 +515,7 @@ async function pollTerminalRun( outcome: 'stuck', status: lastStatus, durationMs: Date.now() - startedAt, + timeline: await readTimeline(world, run.runId), }; } diff --git a/packages/core/src/events-consumer.test.ts b/packages/core/src/events-consumer.test.ts index 0406b181fd..b7051003c0 100644 --- a/packages/core/src/events-consumer.test.ts +++ b/packages/core/src/events-consumer.test.ts @@ -841,6 +841,90 @@ describe('EventsConsumer', () => { expect(onDuplicateEvent).toHaveBeenCalledWith(events[3], 'step_started'); }); + it('skips a step_retrying written behind the step result', async () => { + // The straggler that repeats no class: a losing attempt outlived the + // winning one, so its retry is the step's FIRST `step_retrying` and it + // lands after the completion. No consumer can want it — the step is + // over — and treating it as divergence fails a run that finished. + const corr = 'step_R'; + const events = [ + realEvent('step_created', corr), + realEvent('step_started', corr), + realEvent('step_completed', corr), + realEvent('step_retrying', corr), + ]; + const onUnconsumedEvent = vi.fn(); + const onDuplicateEvent = vi.fn(); + const consumer = consumerFor(events, { + onUnconsumedEvent, + onDuplicateEvent, + }); + + consumer.subscribe(entityConsumer(corr, 'step_completed')); + await waitPastDeferredCheck(); + + expect(consumer.eventIndex).toBe(events.length); + expect(onUnconsumedEvent).not.toHaveBeenCalled(); + expect(onDuplicateEvent).toHaveBeenCalledWith( + events[3], + 'step_completed' + ); + }); + + it('skips a step_retrying written behind a step failure', async () => { + const corr = 'step_RF'; + const events = [ + realEvent('step_created', corr), + realEvent('step_started', corr), + realEvent('step_failed', corr), + realEvent('step_retrying', corr), + ]; + const onUnconsumedEvent = vi.fn(); + const onDuplicateEvent = vi.fn(); + const consumer = consumerFor(events, { + onUnconsumedEvent, + onDuplicateEvent, + }); + + consumer.subscribe(entityConsumer(corr, 'step_failed')); + await waitPastDeferredCheck(); + + expect(onUnconsumedEvent).not.toHaveBeenCalled(); + expect(onDuplicateEvent).toHaveBeenCalledWith(events[3], 'step_failed'); + }); + + it('still reports an unconsumed step_retrying for a step with no result', async () => { + // The rule keys on the outcome being decided, not on the type. A retry + // nobody wants for a step still running is the divergence it always was. + const corr = 'step_LIVE'; + const events = [ + realEvent('step_created', 'step_OTHER'), + realEvent('step_retrying', corr), + ]; + const onUnconsumedEvent = vi.fn(); + const consumer = consumerFor(events, { onUnconsumedEvent }); + + consumer.subscribe(entityConsumer('step_OTHER', 'step_completed')); + await vi.waitFor(() => expect(onUnconsumedEvent).toHaveBeenCalled()); + expect(onUnconsumedEvent).toHaveBeenCalledWith(events[1]); + }); + + it('does not let one step result silence a straggler for another step', async () => { + const corr = 'step_ONE'; + const events = [ + realEvent('step_created', corr), + realEvent('step_started', corr), + realEvent('step_completed', corr), + realEvent('step_retrying', 'step_TWO'), + ]; + const onUnconsumedEvent = vi.fn(); + const consumer = consumerFor(events, { onUnconsumedEvent }); + + consumer.subscribe(entityConsumer(corr, 'step_completed')); + await vi.waitFor(() => expect(onUnconsumedEvent).toHaveBeenCalled()); + expect(onUnconsumedEvent).toHaveBeenCalledWith(events[3]); + }); + it('skips a step_created that repeats a class already in the log', async () => { // Classes are tracked independently, so a completed step still has a // recorded step_created and a second one is ignorable. @@ -952,10 +1036,12 @@ describe('EventsConsumer', () => { }); }); - it('does not let one class suppress another for the same entity', async () => { - // The step's outcome is in the log but its first attempt never wrote a - // step_started, so this one is not a repeat of anything and divergence - // is the right answer. + it('skips a start behind the result even when it repeats no class', async () => { + // The step's outcome is in the log and this start repeats nothing, so + // the class rule alone would call it divergence. It is not: an attempt + // that began while another was finishing writes exactly this, and + // neither writer was wrong. What settles it is that the outcome is + // already decided, which no later event for this step can change. const corr = 'step_A'; const events = [ realEvent('step_created', corr), @@ -972,11 +1058,12 @@ describe('EventsConsumer', () => { consumer.subscribe(entityConsumer(corr, 'step_completed')); await waitPastDeferredCheck(); - expect(consumer.eventIndex).toBe(2); - expect(onDuplicateEvent).not.toHaveBeenCalled(); - await vi.waitFor(() => { - expect(onUnconsumedEvent).toHaveBeenCalledWith(events[2]); - }); + expect(consumer.eventIndex).toBe(events.length); + expect(onUnconsumedEvent).not.toHaveBeenCalled(); + expect(onDuplicateEvent).toHaveBeenCalledWith( + events[2], + 'step_completed' + ); }); it('does not track hook deliveries, whose consumers subscribe lazily', async () => { diff --git a/packages/core/src/events-consumer.ts b/packages/core/src/events-consumer.ts index a6686006b9..b6ab05cf28 100644 --- a/packages/core/src/events-consumer.ts +++ b/packages/core/src/events-consumer.ts @@ -1,4 +1,9 @@ -import { type Event, entityEventClass, envNumber } from '@workflow/world'; +import { + type Event, + entityEventClass, + envNumber, + isStepEventType, +} from '@workflow/world'; import { eventsLogger } from './logger.js'; /** @@ -352,7 +357,9 @@ export class EventsConsumer { // for the same entity it is a straggler from a concurrent replay: // step over it in this pass rather than paying the deferred window for // a consumer that cannot come (see `firstEventTypeOfClass`). - const firstType = this.firstEventTypeOfClass(currentEvent); + const firstType = + this.firstEventTypeOfClass(currentEvent) ?? + this.outcomeAlreadyDecided(currentEvent); if (firstType !== undefined) { this.skipDuplicateEvent(currentEvent, firstType); continue; @@ -540,6 +547,49 @@ export class EventsConsumer { return key === undefined ? undefined : this.seenEventClasses.get(key); } + /** + * The terminal event that already settled this step, for a step-lifecycle + * event that arrives behind it. + * + * {@link firstEventTypeOfClass} catches a straggler that repeats a class, + * which covers a second `step_created` or a second `step_started`. It cannot + * catch the first `step_retrying` of a step whose result is already + * recorded, because that event repeats no class: nothing consumed a + * `step_retrying` for this step, so the class was never recorded, and no + * consumer will ever want one — the step is over. + * + * That event exists whenever a losing attempt outlives the winning one. Two + * invocations run the same step, one commits `step_completed`, the other's + * body then fails and records the retry it would have taken. + * + * A step claimed inline has exactly one owner, so that pair cannot arise + * there — the World tells one writer it created the step and the other that + * it did not. A queue-dispatched step has no claim to lose: its message can + * be delivered twice, both deliveries start a step that exists, and nothing + * before the fact can decide between two attempts that are both legitimate. + * A World that settles the step in the same write as the event refuses the + * loser's *terminal* write, which is enough when the completion is visible; + * one that stores only the log can refuse only what the writer skipped over, + * and the completion is invisible to a writer whose own start already sits + * above it. Measured: with the inline claim fenced and this skip reverted, + * `event-log-race-repro` still corrupts 12 of 12 storm runs. + * + * Skipping is as deterministic as the class rule it extends, and for the + * same reason: the outcome was decided at a fixed position that every replay + * reads, and correlation ids are per body position, so no consumer can + * appear later wanting this. What the skip protects is the difference + * between a run that reports its own corruption and a run that finishes. + */ + private outcomeAlreadyDecided(event: Event): Event['eventType'] | undefined { + if (!isStepEventType(event.eventType)) return undefined; + if (entityEventClass(event.eventType) === 'step_terminal') { + // A second outcome is a repeat of the class, which the caller already + // checked. Reaching here means it is the first, and it is wanted. + return undefined; + } + return this.seenEventClasses.get(`step_terminal:${event.correlationId}`); + } + /** Steps the walk over a repeat of an already-consumed class. */ private skipDuplicateEvent(event: Event, firstType: Event['eventType']) { this.eventIndex++; From 8ef177d2188c7f9d93b87589dc97b54ce06e867c Mon Sep 17 00:00:00 2001 From: Peter Wielander Date: Wed, 12 Aug 2026 18:49:45 -0700 Subject: [PATCH 5/6] Stop tracking a generated e2e diagnostics sidecar `writeDiagnosticsSidecar` writes it into the repo root on every e2e run, and the gitignore rule added alongside its sibling does not apply to a file already tracked. Committed here by mistake, from running the suite locally. Co-Authored-By: Claude Opus 5 --- e2e-diagnostics-nextjs-turbopack-vercel.json | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 e2e-diagnostics-nextjs-turbopack-vercel.json diff --git a/e2e-diagnostics-nextjs-turbopack-vercel.json b/e2e-diagnostics-nextjs-turbopack-vercel.json deleted file mode 100644 index cdd3a1a3c2..0000000000 --- a/e2e-diagnostics-nextjs-turbopack-vercel.json +++ /dev/null @@ -1,8 +0,0 @@ -[ - { - "testName": "WorkflowNotRegisteredError fails the run when workflow does not exist", - "runId": "wrun_41KV8YMWKC00RAQCRH320GSWR4", - "timestamp": "2026-06-16T19:31:07.845Z", - "dashboardUrl": "https://vercel.com/vercel-labs/example-nextjs-workflow-turbopack/observability/workflows/runs/wrun_41KV8YMWKC00RAQCRH320GSWR4?environment=preview" - } -] From 9af6f63a5ddf52d05dc5148219f42d5023e555ef Mon Sep 17 00:00:00 2001 From: Peter Wielander Date: Wed, 12 Aug 2026 19:17:38 -0700 Subject: [PATCH 6/6] [core] Carry step input, attempt, and log position on dispatch messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Step row answered three questions at execution time that nothing answers on a World that materializes no step rows (the v5 event path): 1. **The input.** v4's step_started PATCH returned the row, whose inputRef was stored at step_created; the executor hydrates the body from that response. A bare start against the no-row path synthesizes its response from the event just committed, which carries no payload — so every queue-dispatched step hydrated `undefined` and failed with devalue's "Invalid input" before user code ran, retried, and failed again on the next delivery, forever. 2. **The attempt.** The row's counter was the retry-ceiling authority. With it gone the only cheap signal was the queue delivery count, which resets to 1 on every fresh publish — and every retry after step_retrying IS a fresh publish, so the ceiling's fast gate never opened, the writer stamped attempt 1 on every start, and a failing step never exhausted its budget. This is what turned the race-repro storms into 12/12 stuck-at-running. 3. **The start fence.** The no-row write path reads a write's refusal out of the slots it skipped, which requires the writer to name its log position — and the queued executor had none to name, so a start behind the step's recorded outcome was told it won and re-ran the body. The dispatcher — a replay that holds the loaded log — knows all three, so the step dispatch message now carries them: `stepInput` (the same serialized bytes step_created stored, when binary, within the resilient size bound, and on a CBOR transport), and a new `stepContext` with the 1-based attempt this dispatch asks for plus the dispatcher's slot snapshot. The background executor takes max(delivery count, declared attempt) as its ceiling input (still verified against the recorded start count before failing the step), seeds its writes with the snapshot, and hydrates from the response, then the message, then a step_created read-back — and refuses to run a body with no input at all rather than handing undefined to the deserializer. Every field is optional and advisory: old messages and old consumers behave exactly as before. Co-Authored-By: Claude Fable 5 --- .changeset/step-dispatch-context.md | 6 + packages/core/src/runtime.ts | 117 ++++++++-- .../core/src/runtime/step-executor.test.ts | 201 ++++++++++++++++++ packages/core/src/runtime/step-executor.ts | 103 ++++++++- .../src/runtime/suspension-handler.test.ts | 85 ++++++++ .../core/src/runtime/suspension-handler.ts | 51 ++++- packages/world/src/queue.ts | 49 +++++ 7 files changed, 587 insertions(+), 25 deletions(-) create mode 100644 .changeset/step-dispatch-context.md diff --git a/.changeset/step-dispatch-context.md b/.changeset/step-dispatch-context.md new file mode 100644 index 0000000000..4fce4ce4db --- /dev/null +++ b/.changeset/step-dispatch-context.md @@ -0,0 +1,6 @@ +--- +'@workflow/world': patch +'workflow': patch +--- + +Carry step input, attempt number, and log position on step dispatch messages so queued steps hydrate their input and stop retrying past maxRetries on Worlds without step rows diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index abdb1272a2..1752430b9c 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -677,6 +677,7 @@ export function workflowEntrypoint( runInput, hookInput, stepInput, + stepContext, hookResumeTiming, } = WorkflowInvokePayloadSchema.parse(message_); @@ -1594,6 +1595,12 @@ export function workflowEntrypoint( ...(await replayMessage()), stepId: incomingStepId, stepName: incomingStepName, + // Carry the dispatcher's input and context verbatim + // — the re-routed delivery still has to hydrate and + // bound retries, and this message's copy is the + // only one it will see. + ...(stepInput !== undefined ? { stepInput } : {}), + ...(stepContext !== undefined ? { stepContext } : {}), // Carry the resume timing verbatim so the re-routed // hop stays inside `step_dispatch` rather than // vanishing from the TTR decomposition. @@ -1606,18 +1613,27 @@ export function workflowEntrypoint( ? +bgRun.startedAt : Date.now(); - // Retry ceiling for a backgrounded step. `metadata.attempt` - // (the queue delivery count) is a cheap upper bound, but it - // over-counts: a ThrottleError / TooEarlyError — or any - // redelivery that never ran the body — still advances it, so - // trusting it directly could fail a step as "exceeded max - // retries" before the body ever ran (a user-visible - // regression under transient backend pressure). Use it only - // as a fast gate: while it is at or under the ceiling the - // step cannot be exhausted, so proceed without touching the - // log. Only once it crosses the ceiling do we load the full - // event log and derive the authoritative attempt from the - // recorded `step_started` count — scoped to the lifecycle + // Retry ceiling for a backgrounded step, from the two + // cheap signals a queued delivery carries, verified + // against the log once they say the budget is spent. + // + // `metadata.attempt` (the queue delivery count) is an + // upper bound on THIS message's deliveries, but it + // over-counts within a message — a ThrottleError / + // TooEarlyError redelivery that never ran the body + // still advances it — and it resets to 1 on every + // fresh publish, so it never sees the attempts behind + // a step re-dispatched after `step_retrying`. The + // dispatcher's `stepContext.attempt` covers exactly + // that half: it is the recorded start total in the log + // the re-dispatching replay held, plus one for this + // dispatch. Neither is authoritative alone, so take + // the max as the fast gate: while it is at or under + // the ceiling the step cannot be exhausted, so proceed + // without touching the log. Only once it crosses the + // ceiling do we load the full event log and derive the + // authoritative attempt from the recorded + // `step_started` count — scoped to the lifecycle // attempt total (bare starts plus the largest single // owner's starts): throttle/too-early redeliveries write // no start at all, racing invocations' one-off stamped @@ -1629,11 +1645,14 @@ export function workflowEntrypoint( // retries trips the combined ceiling. This still bounds // timeouts, which write no error for the post-body guard // to catch. - let bgAuthoritativeAttempt = metadata.attempt; + let bgAuthoritativeAttempt = Math.max( + metadata.attempt, + stepContext?.attempt ?? 1 + ); const bgMaxRetries = getStepFunction(incomingStepName)?.maxRetries ?? DEFAULT_STEP_MAX_RETRIES; - if (metadata.attempt > bgMaxRetries + 1) { + if (bgAuthoritativeAttempt > bgMaxRetries + 1) { const loaded = await loadWorkflowRunEvents(runId); bgAuthoritativeAttempt = countStepStartedEvents( @@ -1678,10 +1697,30 @@ export function workflowEntrypoint( stepId: incomingStepId, stepName: incomingStepName, runSpecVersion: bgRun.specVersion, - // Retry ceiling: the queue delivery count as a fast - // gate, verified against the recorded step_started + // Retry ceiling: the delivery count and the + // dispatcher's declared attempt as a fast gate, + // verified against the recorded step_started // count once it crosses the ceiling (see above). authoritativeAttempt: bgAuthoritativeAttempt, + // Input fallback for a World whose bare + // step_started response carries none (no step + // rows): the same serialized bytes the + // dispatcher wrote into step_created. + ...(stepInput !== undefined + ? { dispatchedStepInput: stepInput.input } + : {}), + // The dispatcher's log position, so this + // execution's writes name what their scheduling + // was decided against — a bare start naming no + // position gets no duplicate gate from a World + // that reads refusals out of the log. + ...(stepContext?.eventCount !== undefined + ? { + slotSnapshot: { + eventCount: stepContext.eventCount, + }, + } + : {}), ...(bgResumeTracking ? { resumeTracking: bgResumeTracking } : {}), @@ -3539,6 +3578,27 @@ export function workflowEntrypoint( handOffResumeTiming = false; resumeTracking = undefined; } + // What this dispatcher knows and the executing + // invocation cannot cheaply learn: the serialized + // input (a World without step rows answers a bare + // start without one), which attempt this dispatch + // asks for (the queue delivery count resets on + // every fresh publish, so a step re-dispatched + // after step_retrying would report attempt 1 + // forever), and this replay's log position (a bare + // start naming no position gets no duplicate gate + // from a World that reads refusals out of the + // log). See StepDispatchContextSchema. + const dispatchInput = + suspensionResult.stepDispatchInputs.get( + step.correlationId + ); + const dispatchAttempt = + countStepStartedEvents( + eventLog.events, + step.correlationId, + { type: 'totalAttempts' } + ) + 1; dispatches.push( queueMessage( world, @@ -3552,6 +3612,13 @@ export function workflowEntrypoint( ...(stepResumeTiming ? { hookResumeTiming: stepResumeTiming } : {}), + ...(dispatchInput + ? { stepInput: { input: dispatchInput } } + : {}), + stepContext: { + attempt: dispatchAttempt, + ...slotSnapshot(), + }, }, { // Step-identity-scoped: dedupes against every @@ -4048,6 +4115,7 @@ export function workflowEntrypoint( const toRetry: { step: (typeof inlineExecutions)[number]; delaySeconds: number; + nextAttempt?: number; }[] = []; let anyPendingOps = false; // A throttled inline step delays redelivery of THIS @@ -4076,6 +4144,7 @@ export function workflowEntrypoint( toRetry.push({ step: s, delaySeconds: r.timeoutSeconds, + nextAttempt: r.nextAttempt, }); } else if (r.type === 'throttled') { throttleTimeout = Math.max( @@ -4118,7 +4187,7 @@ export function workflowEntrypoint( if (toRetry.length > 0) { const retryTraceCarrier = await nextTraceCarrier(); await Promise.all( - toRetry.map(({ step, delaySeconds }) => + toRetry.map(({ step, delaySeconds, nextAttempt }) => queueMessage( world, getWorkflowQueueName(workflowName, namespace), @@ -4128,6 +4197,20 @@ export function workflowEntrypoint( stepName: step.stepName, traceCarrier: retryTraceCarrier, requestedAt: new Date(), + // What this dispatcher knows that a fresh + // delivery cannot learn for itself — see + // StepDispatchContextSchema. No stepInput: + // a `retry` result implies the step exists + // (its start succeeded), so the executor's + // step_created read-back covers hydration + // on a World whose bare-start response + // carries none. + stepContext: { + ...(nextAttempt !== undefined + ? { attempt: nextAttempt } + : {}), + ...slotSnapshot(), + }, }, { delaySeconds, diff --git a/packages/core/src/runtime/step-executor.test.ts b/packages/core/src/runtime/step-executor.test.ts index a1ca74882f..ceda2fa718 100644 --- a/packages/core/src/runtime/step-executor.test.ts +++ b/packages/core/src/runtime/step-executor.test.ts @@ -305,3 +305,204 @@ describe('executeStep — attempt stamping', () => { expect(started?.eventData).not.toHaveProperty('attempt'); }); }); + +// A World that materializes no step rows answers a bare `step_started` from +// the event just committed, which carries no input — v4's response was the +// row, whose inputRef was stored at step_created. The executor must then +// hydrate from the dispatch message's bytes, or failing that, read the +// step_created event back. Simulated here by stripping `input` from the +// start response of a materializing world. +function withInputlessStartResponses(world: World): World { + const events: World['events'] = { + ...world.events, + create: (async (runId, data, params) => { + const result = await world.events.create(runId, data as never, params); + if ( + (data as { eventType?: string }).eventType === 'step_started' && + (result as { step?: { input?: unknown } }).step + ) { + const { input: _input, ...step } = ( + result as { step: { input?: unknown } & Record } + ).step; + return { ...result, step }; + } + return result; + }) as World['events']['create'], + }; + return { ...world, events }; +} + +async function setupStepWithArg(opts: { + world: World; + stepName: string; + arg: string; + onBody: (arg: unknown) => void; +}): Promise<{ runId: string; stepId: string }> { + const { world, stepName, arg, onBody } = opts; + const runInput = await dehydrateStepArguments([], 'run', undefined); + const created = await world.events.create(null, { + eventType: 'run_created', + specVersion: SPEC_VERSION_CURRENT, + eventData: { + deploymentId: 'dpl_test', + workflowName: 'wf', + input: runInput, + }, + }); + const runId = created.run!.runId; + await world.events.create(runId, { + eventType: 'run_started', + specVersion: SPEC_VERSION_CURRENT, + eventData: {}, + } as never); + + const stepId = 'step_input_1'; + const stepInput = await dehydrateStepArguments( + { args: [arg], closureVars: undefined, thisVal: undefined }, + runId, + undefined + ); + await world.events.create(runId, { + eventType: 'step_created', + specVersion: SPEC_VERSION_CURRENT, + correlationId: stepId, + eventData: { stepName, input: stepInput }, + }); + + registerStepFunction(stepName, async (bodyArg: unknown) => { + onBody(bodyArg); + return 'ok'; + }); + + return { runId, stepId }; +} + +describe('executeStep — step input fallbacks (Worlds without step rows)', () => { + afterEach(() => { + counter += 1; + }); + + it('hydrates from the dispatch message when the start response has no input', async () => { + const world = withInputlessStartResponses(makeWorld()); + const stepName = uniqueStepName(); + const seen: unknown[] = []; + const { runId, stepId } = await setupStepWithArg({ + world, + stepName, + arg: 'from-message', + onBody: (arg) => seen.push(arg), + }); + const dispatchedStepInput = await dehydrateStepArguments( + { args: ['from-message'], closureVars: undefined, thisVal: undefined }, + runId, + undefined + ); + + const listSpy = vi.spyOn(world.events, 'listByCorrelationId'); + const result = await executeStep({ + world, + workflowRunId: runId, + workflowName: 'wf', + workflowStartedAt: Date.now(), + stepId, + stepName, + dispatchedStepInput: dispatchedStepInput as never, + }); + + expect(result.type).toBe('completed'); + expect(seen).toEqual(['from-message']); + // The message bytes answered; no log read-back was needed. + expect(listSpy).not.toHaveBeenCalled(); + }); + + it('reads the step_created event back when neither the response nor the message carries input', async () => { + const world = withInputlessStartResponses(makeWorld()); + const stepName = uniqueStepName(); + const seen: unknown[] = []; + const { runId, stepId } = await setupStepWithArg({ + world, + stepName, + arg: 'from-log', + onBody: (arg) => seen.push(arg), + }); + + const listSpy = vi.spyOn(world.events, 'listByCorrelationId'); + const result = await executeStep({ + world, + workflowRunId: runId, + workflowName: 'wf', + workflowStartedAt: Date.now(), + stepId, + stepName, + }); + + expect(result.type).toBe('completed'); + expect(seen).toEqual(['from-log']); + expect(listSpy).toHaveBeenCalled(); + }); + + it('does not run the body when no input source exists at all', async () => { + const world = withInputlessStartResponses(makeWorld()); + const stepName = uniqueStepName(); + const seen: unknown[] = []; + const { runId, stepId } = await setupStepWithArg({ + world, + stepName, + arg: 'unreachable', + onBody: (arg) => seen.push(arg), + }); + + // The read-back finds no step_created either (e.g. not yet visible). + vi.spyOn(world.events, 'listByCorrelationId').mockResolvedValue({ + data: [], + cursor: null, + hasMore: false, + }); + + const result = await executeStep({ + world, + workflowRunId: runId, + workflowName: 'wf', + workflowStartedAt: Date.now(), + stepId, + stepName, + }); + + // The missing input is an error for the retry path (bounded by the + // attempt ceiling), never a body execution with undefined arguments. + expect(result.type).toBe('retry'); + expect(seen).toEqual([]); + }); + + it('prefers the input the start response carries', async () => { + // Worlds with step rows answer the start from the row; the message bytes + // must not displace that. + const world = makeWorld(); + const stepName = uniqueStepName(); + const seen: unknown[] = []; + const { runId, stepId } = await setupStepWithArg({ + world, + stepName, + arg: 'from-row', + onBody: (arg) => seen.push(arg), + }); + const dispatchedStepInput = await dehydrateStepArguments( + { args: ['from-message'], closureVars: undefined, thisVal: undefined }, + runId, + undefined + ); + + const result = await executeStep({ + world, + workflowRunId: runId, + workflowName: 'wf', + workflowStartedAt: Date.now(), + stepId, + stepName, + dispatchedStepInput: dispatchedStepInput as never, + }); + + expect(result.type).toBe('completed'); + expect(seen).toEqual(['from-row']); + }); +}); diff --git a/packages/core/src/runtime/step-executor.ts b/packages/core/src/runtime/step-executor.ts index d14f355df5..ea8979e5c0 100644 --- a/packages/core/src/runtime/step-executor.ts +++ b/packages/core/src/runtime/step-executor.ts @@ -122,6 +122,20 @@ export interface StepExecutorParams { * carries no payload (the legacy contract). */ lazyStepInput?: SerializedData; + /** + * The step's serialized input as carried on the queue message that + * dispatched this execution (`stepInput.input`) — the same bytes the + * dispatcher wrote into the step's `step_created` event. Used ONLY as a + * local hydration source when the `step_started` response returns a step + * without `input`: a World that materializes no step rows has no row to + * answer a bare start from, and the start event itself carries no payload. + * Never sent on the wire by this executor — a `step_started` carrying an + * input is a create-claim (see {@link lazyStepInput}), which a bare start + * of an existing step must not become. When neither the response nor this + * field has the input, the executor falls back to reading the step's + * `step_created` event from the log. + */ + dispatchedStepInput?: SerializedData; /** * Inline step ownership: the queue message ID of the invocation this * executeStep call runs in (from the queue handler's meta). When set, the @@ -260,7 +274,19 @@ export type StepExecutionResult = inlineDelta?: InlineEventDelta; } | { type: 'failed' } - | { type: 'retry'; timeoutSeconds: number } + | { + type: 'retry'; + timeoutSeconds: number; + /** + * The attempt number the retry dispatch should ask for + * (`stepContext.attempt` on the queue message): the attempt that just + * burned plus one, or the same attempt again when the body never ran + * (a `TooEarly` start rejection). Undefined when this executor was not + * told which attempt it ran — the dispatch then carries none and the + * consumer falls back to the delivery count. + */ + nextAttempt?: number; + } | { type: 'skipped' } | { type: 'gone' } | { type: 'throttled'; timeoutSeconds: number }; @@ -272,6 +298,46 @@ export type StepExecutionResult = * Does NOT queue workflow continuation messages — the caller decides what to do next. * Used by the combined workflow handler for step execution. */ +/** + * The step's serialized input, read back from its `step_created` event. + * + * Last-resort input source for a bare start against a World that + * materializes no step rows: the `step_started` response has no row to + * answer from, and the dispatch message may not have carried the bytes (an + * old producer, an oversized or non-binary input, an owned-recovery re-run, + * a JSON queue transport). The `step_created` event holds the input on every + * World, and it is the earliest event of its correlation id, so the first + * ascending page almost always answers; the loop covers a correlation id + * whose page one is retry churn. + * + * Returns `undefined` when no `step_created` exists (the caller decides what + * that means). Transient read failures propagate — the queue redelivers and + * a later attempt converges, the same policy as every other pre-body read. + */ +async function fetchStepCreatedInput( + world: World, + runId: string, + stepId: string +): Promise { + let cursor: string | undefined; + for (;;) { + const page = await world.events.listByCorrelationId({ + runId, + correlationId: stepId, + resolveData: 'all', + pagination: { sortOrder: 'asc', ...(cursor ? { cursor } : {}) }, + }); + const created = page.data.find((e) => e.eventType === 'step_created'); + if (created) { + return 'eventData' in created + ? (created.eventData as { input?: unknown } | undefined)?.input + : undefined; + } + if (!page.hasMore || !page.cursor) return undefined; + cursor = page.cursor; + } +} + export async function executeStep( params: StepExecutorParams ): Promise { @@ -564,7 +630,12 @@ export async function executeStep( stepId, timeoutSeconds, }); - return { type: 'retry', timeoutSeconds }; + // The body never ran, so the re-dispatch asks for this same attempt. + return { + type: 'retry', + timeoutSeconds, + nextAttempt: params.authoritativeAttempt, + }; } return undefined; }; @@ -890,13 +961,36 @@ export async function executeStep( // Use the provided encryption key when available, otherwise resolve // through the memoized accessor declared at the top of this trace. const encryptionKey = params.encryptionKey ?? (await getEncryptionKey()); + // The bytes the body's arguments hydrate from. A World with step rows + // answers the start from the row, which carries the input stored at + // `step_created`; one that stores only the log answers a bare start + // with no input at all. The dispatch message is the first fallback + // (the same bytes the dispatcher wrote into `step_created`), and a + // message that carried none reads that event back from the log. + let stepInputSource: unknown = step.input; + if (stepInputSource === undefined) { + stepInputSource = params.dispatchedStepInput; + } + if (stepInputSource === undefined) { + stepInputSource = await trace('step.fetch_input', {}, () => + fetchStepCreatedInput(world, workflowRunId, stepId) + ); + } + if (stepInputSource === undefined) { + // Without this, the undefined falls through to the deserializer, + // which reports it as an opaque "Invalid input". + throw new WorkflowRuntimeError( + `Step "${stepId}" has no input: the step_started response, the ` + + 'dispatch message, and the step_created event all carried none' + ); + } const hydratedInput = await trace( 'step.hydrate', {}, async (hydrateSpan) => { const startTime = Date.now(); const hydrated = await hydrateStepArguments( - step.input, + stepInputSource, workflowRunId, encryptionKey, ops, @@ -1402,7 +1496,8 @@ export async function executeStep( ...Attribute.StepRetryWillRetry(true), }); - return { type: 'retry', timeoutSeconds }; + // This attempt burned, so the retry dispatch asks for the next one. + return { type: 'retry', timeoutSeconds, nextAttempt: step.attempt + 1 }; } // Create step_completed event outside the step execution failure path: diff --git a/packages/core/src/runtime/suspension-handler.test.ts b/packages/core/src/runtime/suspension-handler.test.ts index d34814500b..80e5deb3ee 100644 --- a/packages/core/src/runtime/suspension-handler.test.ts +++ b/packages/core/src/runtime/suspension-handler.test.ts @@ -7,6 +7,7 @@ import { import type { Event } from '@workflow/world'; import { SPEC_VERSION_CURRENT, + SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT, slotToEventId, type ValidQueueName, type WorkflowRun, @@ -912,3 +913,87 @@ describe('retainedStepInputsSafe (serialization passivity gate)', () => { expect(result.lazyInlineSteps).toHaveLength(1); }); }); + +describe('step dispatch inputs', () => { + // The serialized input of every eager step created by a pass rides the + // caller's dispatch messages (`stepInput`), so an executing invocation can + // hydrate the body even when its World's bare `step_started` response + // carries no input (no step rows). See + // SuspensionHandlerResult.stepDispatchInputs. + const pendingStepWithHookAwaiter = () => + new Map([ + [ + 'step_eager', + { + type: 'step' as const, + correlationId: 'step_eager', + stepName: 'eagerStep', + args: ['payload'], + }, + ], + // The conflict awaiter suppresses lazy-inline designation, so the step + // above gets an eager step_created — the shape whose dispatch message + // needs the input. + [ + 'hook_awaited', + { + type: 'hook' as const, + correlationId: 'hook_awaited', + token: 'claim-token', + hasConflictAwaiter: true, + }, + ], + ]); + + it('returns the serialized input of each eagerly created step', async () => { + const eventsCreate = vi.fn().mockImplementation(async (_runId, event) => ({ + event, + })); + const world = createWorld(eventsCreate); + + const result = await handleSuspension({ + suspension: new WorkflowSuspension( + pendingStepWithHookAwaiter() as never, + globalThis + ), + world, + run: { + ...run, + specVersion: SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT, + }, + }); + + const input = result.stepDispatchInputs.get('step_eager'); + expect(input).toBeInstanceOf(Uint8Array); + // The bytes are exactly what the step_created event stored, so a consumer + // hydrating from the message sees what a log read-back would return. + const createdCall = eventsCreate.mock.calls.find( + ([, event]) => event.eventType === 'step_created' + ); + expect(createdCall?.[1]?.eventData?.input).toBe(input); + }); + + it('returns nothing for a run whose queue transport mangles binary payloads', async () => { + const eventsCreate = vi.fn().mockImplementation(async (_runId, event) => ({ + event, + })); + const world = createWorld(eventsCreate); + + const result = await handleSuspension({ + suspension: new WorkflowSuspension( + pendingStepWithHookAwaiter() as never, + globalThis + ), + world, + run: { ...run, specVersion: 2 }, + }); + + expect(result.stepDispatchInputs.size).toBe(0); + // The step_created write itself is unaffected. + expect( + eventsCreate.mock.calls.some( + ([, event]) => event.eventType === 'step_created' + ) + ).toBe(true); + }); +}); diff --git a/packages/core/src/runtime/suspension-handler.ts b/packages/core/src/runtime/suspension-handler.ts index 7855ca0a95..cda7e744c8 100644 --- a/packages/core/src/runtime/suspension-handler.ts +++ b/packages/core/src/runtime/suspension-handler.ts @@ -151,6 +151,21 @@ export interface SuspensionHandlerResult { stepName: string; dehydratedInput: SerializedData; }>; + /** + * The serialized input of each step whose `step_created` this pass wrote + * (eager, non-lazy steps only), for the caller to attach to that step's + * execution queue message as `stepInput`. The executing invocation hydrates + * the step body's arguments from it when the World's `step_started` + * response carries no input — a World that materializes no step rows has no + * row to answer from, and the event a bare start commits holds none. + * + * Only inputs the queue message can carry: binary (the JSON transport would + * mangle the bytes; requires the run's CBOR transport) and at most + * `MAX_RESILIENT_STEP_INPUT_BYTES` (the same inline-cost bound as resilient + * dispatch). A step missing from this map is dispatched without input and + * the executor falls back to reading its `step_created` event. + */ + stepDispatchInputs: Map; /** * The soonest pending wait, if any: seconds until it elapses and the * correlationId of the wait that produced that timeout. The @@ -672,6 +687,17 @@ export async function handleSuspension({ // write). Reported to the caller so its dispatch pass skips them. const queuedStepCorrelationIds = new Set(); + // Serialized inputs of the eager steps created this pass, for the caller's + // dispatch pass to carry on each step's queue message. See + // SuspensionHandlerResult.stepDispatchInputs. + const stepDispatchInputs = new Map(); + + // Whether the run's queue transport preserves binary payloads (CBOR, + // specVersion >= 3). `stepInput.input` is the serialized (possibly + // encrypted) input bytes, which the JSON transport would mangle. + const queueTransportPreservesBinary = + (run.specVersion ?? 0) >= SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT; + // Resilient step dispatch eligibility, shared by every step op below (the // per-step input-size check is applied inside the op). All must hold: // @@ -688,13 +714,12 @@ export async function handleSuspension({ // best-effort marker that fails open cannot carry a correctness property. // The sequential path is the only thing that gives the message a // happens-after edge over its create's verdict. - // - The run's queue transport preserves binary payloads (CBOR, - // specVersion >= 3): `stepInput.input` is the serialized (possibly - // encrypted) input bytes, which the JSON transport would mangle. + // - The run's queue transport preserves binary payloads + // (`queueTransportPreservesBinary` above). const resilientDispatchEligible = stepDispatch !== undefined && isResilientStepDispatchEnabled() && - (run.specVersion ?? 0) >= SPEC_VERSION_SUPPORTS_CBOR_QUEUE_TRANSPORT; + queueTransportPreservesBinary; // The trace carrier for resilient step dispatches, resolved at most once per // suspension (the per-step ops run concurrently and share it). @@ -748,6 +773,16 @@ export async function handleSuspension({ }); return; } + // Keep the bytes for the caller's dispatch pass when the queue + // message can carry them — see + // SuspensionHandlerResult.stepDispatchInputs. + if ( + queueTransportPreservesBinary && + dehydratedInput instanceof Uint8Array && + dehydratedInput.byteLength <= MAX_RESILIENT_STEP_INPUT_BYTES + ) { + stepDispatchInputs.set(queueItem.correlationId, dehydratedInput); + } const stepEvent: CreateEventRequest = { eventType: 'step_created' as const, specVersion: SPEC_VERSION_CURRENT, @@ -786,6 +821,13 @@ export async function handleSuspension({ traceCarrier, requestedAt: new Date(), stepInput: { input: dehydratedInput }, + // A step created this pass is brand-new, so this dispatch + // asks for its first attempt; the position is this + // suspension's own snapshot. See StepDispatchContextSchema. + stepContext: { + attempt: 1, + ...(eventLog ? slotSnapshotParams(eventLog.events) : {}), + }, }, // Same key as the caller's dispatch pass and any concurrent // handler's — redundant publishes for this step dedupe. The @@ -1020,6 +1062,7 @@ export async function handleSuspension({ pendingSteps: stepItems, createdStepCorrelationIds, queuedStepCorrelationIds, + stepDispatchInputs, lazyInlineSteps, // On hook conflict the caller re-invokes immediately and never reads // the wait timeout, so don't report one. diff --git a/packages/world/src/queue.ts b/packages/world/src/queue.ts index 3e11d9ce00..a6eb7443b8 100644 --- a/packages/world/src/queue.ts +++ b/packages/world/src/queue.ts @@ -263,6 +263,48 @@ export const HookResumeTimingSchema = z.object({ }); export type HookResumeTiming = z.infer; +/** + * What the dispatcher knew about a step when it published its execution + * message ({@link WorkflowInvokePayload.stepId}). + * + * The Step row used to answer both questions at execution time: its counter + * was the attempt number, and its conditional write refused a start behind + * the step's outcome. A World that materializes no step rows has neither, + * and the executing invocation cannot answer them for itself — it runs from + * a queued delivery without loading the log, and the queue's own delivery + * count resets on every fresh publish, so a step re-dispatched after + * `step_retrying` would report attempt 1 forever. The dispatcher is a replay + * that HAS the log, so it states what it saw. + * + * Both fields are advisory and the object is optional: an old producer omits + * it (the consumer falls back to the delivery count, today's behavior), an + * old consumer ignores it, and workflow-server never reads it. A stale + * dispatcher understates both — safe in both cases: an understated `attempt` + * grants a retry too many (the ceiling still verifies against the log before + * failing the step), an understated `eventCount` only widens the span the + * World reports back. + */ +export const StepDispatchContextSchema = z.object({ + /** + * Which attempt the dispatched execution is, 1-based: the step's recorded + * `step_started` total in the dispatcher's replayed log, plus one for the + * attempt this message asks for. The consumer takes the max of this and + * the queue delivery count as its retry-ceiling input. + */ + attempt: z.number().int().positive().optional(), + /** + * The highest slot the dispatcher's loaded log occupied when it published + * this message — the same snapshot its own writes carry (see + * `CreateEventParams.eventCount`). Seeds the executor's writes so a World + * that gates duplicates from the log can see what this execution's claim + * skipped over, and refuse a start behind the step's recorded outcome. + * Omitted when the dispatcher had nothing loaded or the run's events are + * not slot-numbered. + */ + eventCount: z.number().int().positive().optional(), +}); +export type StepDispatchContext = z.infer; + export const WorkflowInvokePayloadSchema = z.object({ runId: z.string(), traceCarrier: TraceCarrierSchema.optional(), @@ -306,6 +348,13 @@ export const WorkflowInvokePayloadSchema = z.object({ * `step_created` event exists (keyed by `stepId`) before executing the step. */ stepInput: StepDispatchInputSchema.optional(), + /** + * Dispatcher-known step context, only present alongside `stepId`: the + * attempt number this dispatch asks for and the dispatcher's log position. + * See {@link StepDispatchContextSchema} for why the executing invocation + * cannot derive either for itself on a World without step rows. + */ + stepContext: StepDispatchContextSchema.optional(), /** * Hook-resume TTR timing. Present on both `resumeHook()` dispatch paths * (unlike `hookInput`, which only rides the parallel fast path), and