Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sealed-log-noop-shared-predicate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@workflow/web-shared': patch
---

Render sealed log positions (`noop` events) as the log rows they are: shown in event lists, excluded from span geometry and trace duration, since a seal's timestamp belongs to whichever reader wrote it rather than to the run.
9 changes: 9 additions & 0 deletions .changeset/sealed-log-spec-seven.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@workflow/world': minor
'@workflow/world-vercel': minor
'@workflow/world-local': minor
'@workflow/world-postgres': minor
'@workflow/core': minor
---

New runs are created with the sealed-log event identity (specVersion 7). A sealed-log run's event positions are assigned by the backend before each write commits, so concurrent writers never contend for a position. A position whose writer dies is closed by the backend with a `noop` event; replay steps over those without delivering them or advancing the deterministic clock. Set `WORKFLOW_SEALED_LOG=0` to put a deployment back on the previous scheme. Every runtime reads sealed logs either way, and a run's version is fixed at creation, so the setting only affects new runs.
10 changes: 10 additions & 0 deletions docs/content/docs/v5/configuration/runtime-tuning.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,16 @@ For example, a workflow can run a 10-minute inline step even with `WORKFLOW_REPL
- The check trades one failure for another. Most holes stand for an event that never happened, and replaying past those is correct. A hole standing for an event that did happen looks identical, and replaying past that one produces a run whose result is silently wrong. Failing is the recoverable side of that trade.
- Set `0` to replay across holes instead.

### `WORKFLOW_SEALED_LOG`

- Default: enabled
- New runs are created at the sealed-log spec version, in which the World's backend assigns each event its position *before* the write commits rather than letting concurrent writers race for one. Concurrent writes then never contend for a position, which is what makes a wide fan-out cheap.
- The price of assigning positions in advance is that a writer which claims one and then dies leaves a position no writer will ever fill. The backend closes such a position by writing a `noop` event into it once it can prove the position was abandoned, so a reader still sees the dense log it needs. Replay steps over a `noop` without delivering it to the workflow and without advancing the deterministic clock — its timestamp belongs to whichever reader sealed it, not to the run.
- Set `0` to put a deployment back on the previous scheme, where each position is allocated by the write that occupies it. Use this as the kill switch if position assignment turns out to be at fault for event-log problems.
- Existing runs are unaffected either way. A run's spec version is stamped once, at creation, and read from the run for the rest of its life, so flipping this changes only what *new* runs get, and a run in flight keeps the scheme it started on. Every build reads sealed logs regardless of the setting.
- A run created at the sealed-log version can only be replayed by a reader that knows to skip `noop` events. That is every runtime on this release train, but a runtime that pins its own accepted spec range separately — the Python runtime, for one — has to have caught up before it can read these runs. Switch this off in an environment where it has not.
- Only the Vercel World seals. The Local and Postgres Worlds allocate each position at the commit that occupies it, so they cannot leave a hole and never write a `noop`; the setting still moves the version they stamp, so the fleet stays on one spec.

### `WORKFLOW_PRECONDITION_MAX_INPROCESS_RESTARTS`

- Default: `3`
Expand Down
20 changes: 20 additions & 0 deletions docs/content/docs/v5/how-it-works/event-sourcing.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,12 @@ Events are categorized by the entity type they affect. Each event contains metad
| `wait_created` | Creates a new wait in `waiting` state. Contains the timestamp when the wait should complete. |
| `wait_completed` | Transitions the wait to `completed` state when the delay period has elapsed. |

### System Events

| Event | Description |
|-------|-------------|

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review: Nit

Good addition. Its twin was missed: docs/content/docs/v5/api-reference/workflow-runtime/world/storage.mdx:123-130 has an "Event Types" table with the same Run/Attribute/Step/Hook/Wait breakdown and no System row. That page documents events.list returning Event[], so a World author reads a closed set there that no longer matches EventTypeSchema.

| `noop` | Seals an abandoned log position (specVersion 7 and above). Written only by the backend, never by workflow code — the create endpoints reject it. See [Sealed positions](#sealed-positions-noop-events). |

## Terminal States

Terminal states represent the end of an entity's lifecycle. Once an entity reaches a terminal state, no further events can transition it to another state.
Expand Down Expand Up @@ -263,6 +269,20 @@ Both kinds of skip are logged at `debug`, so neither reaches the console unless

The observability UI greys out the events it can identify this way, with the reason on hover. Its set is narrower than the runtime's: it reads the log without consumer state, and a consumer for an entity that is still open legitimately claims a repeat — each retry of a step writes another `step_started`. So it marks a repeat only once no consumer can remain for it: past a terminal event for the same entity, or a second `run_started`, of which the log records one per run. On a partial view of the log — one page of a paginated list, or search results — it marks nothing, since which copy came first is a property of the whole log.

## Sealed Positions (noop events)

Runs at specVersion 7 and above live in a *sealed log*: the backend hands each write its position from a per-run sequencer **before** the write commits, so concurrent writers hold distinct positions and never race one another for a slot. This is how new runs are created by default; [`WORKFLOW_SEALED_LOG=0`](/docs/configuration/runtime-tuning#workflow_sealed_log) puts a deployment back on the previous scheme. Every runtime *reads* a sealed log regardless, and a run's version is fixed at creation, so changing the setting never affects a run already in flight. The trade is that a writer can claim a position and then die — a crashed process, a cancelled transaction — leaving a hole that no writer will ever fill, and a hole reads exactly like an event the reader failed to load.

The backend restores the dense log at read time by **sealing** such positions: once a hole is provably abandoned (bounded by the commit time of later positions — positions are handed out in order, so a committed later position proves how long the hole has been open), the backend writes a `noop` event into it. A `noop` occupies its position — length-based completeness checks, cursors, and pagination all count it — and means nothing:

- It is **never offered to any consumer** during replay. The walk steps over it in the same synchronous pass that delivers the events around it, so its presence cannot perturb delivery order, promise scheduling, or which branch of a `Promise.all` resumes first.
- It **never advances the deterministic clock**. A `noop`'s `createdAt` is the *sealer's* wall clock — it can even postdate events at higher positions — and letting it feed the replay clock would make a log containing a seal replay differently from one whose hole was filled by its original writer. Same rule, and same mechanism, as skipped duplicates above.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review: Note

The invariants listed here hold for replay, but "means nothing" is not yet true of the latency telemetry, where a seal reads as run activity and silently suppresses a measurement. Two sites, both confirmed by running them:

TTFS and RSFSruntime.ts:2927 allowlists three types:

invocationStartedClean ??= eventLog.events.every(
  (e) => e.eventType === 'run_created' || e.eventType === 'run_started' || e.eventType === 'attr_set'
);

A seal before the first step flips this to false, and RSFS shares TTFS's eligibility exactly, so both go unreported.

STSOruntime/step-latency.ts:249 keys on the log's last event:

const lastEvent = events[events.length - 1];
if (... && (lastEvent.eventType === 'step_completed' || lastEvent.eventType === 'step_failed'))

A trailing seal makes prevStepEndMs undefined. I reproduced this with a back-to-back step pair: measured at T0+500 without the seal, undefined with a seal appended.

Neither is a corner case — a seal at the tail is precisely what a fanout whose last writer died leaves behind. The consequence is that rolling spec 7 out quietly shrinks the TTFS/STSO sample population, which reads as a data change rather than a seal artifact when you are watching those numbers for regressions. noop should be admitted in both places for the same reason attr_set already is: it is not something the run did.

(Same class, lower stakes: runtime/quickjs-entrypoint.ts:104 isFirstInvocation uses the same two-type every, so a seal in the preload costs the first-invocation fast path a redundant fetch.)

- Its `correlationId` is `noop_` followed by the sealed position's zero-padded digits — deterministic, so any two sealers racing for the same hole mint the identical event, and recognizable at a glance in the log.

A sealed position races its original writer at the same uniqueness fence as every other write, and losing that race is the good outcome: the real event landed first, and readers get it instead. A live writer that gets sealed over simply re-derives a fresh position and commits there — the same recovery as losing any other write race — so sealing can cost a retry, never a wrong log.

`noop` is not user-creatable: it does not exist in the create schemas, and backends reject it on every create endpoint. Only a backend's own read path writes one.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review: Note

"backends reject it on every create endpoint" is stronger than what ships. CreateEventSchema is never .parse()d anywhere in the repo — it is consumed only as a type, via AnyEventRequest, which is what events.create takes in world-local, world-postgres, world-vercel and world-sim. So the exclusion is a compile-time guarantee for SDK callers, not a runtime rejection by the World.

This PR's own tests demonstrate it: both world-postgres/test/spec.test.ts and world-local/src/storage/slot-identity.test.ts create a noop through world.events.create(runId, {eventType: 'noop', ...} as any) and assert it succeeds. That is deliberate (they are testing tolerance), and it is also the counterexample to the sentence above.

Two ways to make them agree: narrow the claim to the backends that validate on the wire, or add a runtime guard on the create path so the sentence is enforceable. The second is worth considering on its own merits — a noop accepted from a caller burns a slot the caller never allocated, which is exactly the failure the sealer exists to repair.


## Event Correlation

Events use a `correlationId` to link related events together. For step, hook, and wait events, the correlation ID identifies the specific entity instance:
Expand Down
8 changes: 8 additions & 0 deletions docs/content/worlds/v5/building-a-world.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,14 @@ Two properties have to hold, and both are about what a reader can conclude from

Allocate the position **at the commit**, in the same operation that appends the event. That is what makes a reader's log a *prefix* of the run's log rather than a prefix with a hole in it: nothing can land behind a position a reader has already passed. Handing a position out earlier — in a request handler, say — and committing later breaks the property every replay depends on, and is the one case where you may need [a stale-write rejection](#optional-rejecting-a-stale-write) to compensate.

#### Optional: pre-assigned positions and `noop` sealing

Spec version 7 legitimizes one alternative to allocate-at-commit, for Worlds whose store makes commit-time allocation a contention bottleneck: hand positions out from a per-run atomic counter **before** the commit, and restore density at read time. Pre-assignment means concurrent writers hold distinct positions and never race for one — but a writer that claims a position and dies leaves a permanent hole. A World that allocates this way MUST therefore **seal** provably abandoned positions by writing a `noop` event into them (racing the original writer at the same uniqueness fence — losing that race means the real event landed, which is success), and MUST NOT return a page with an interior hole: return the dense prefix below the hole and let the caller's next page pick up past it once the position resolves to an event or a seal.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review: Note

This is the right place for the contract, and it is missing the half that the SDK enforces most harshly. The paragraph tells a pre-assigning World to "return the dense prefix below the hole and let the caller's next page pick up past it" but says nothing about what hasMore must be when that prefix is empty.

The runtime gives no latitude there. loadWorkflowRunEvents runs two guards per page (runtime/helpers.ts):

if (requestedCursors.has(cursor)) throw eventPaginationContractError(runId, 'repeated a cursor');
// and, on the request side:
if (requestedCursors.has(cursor)) throw eventPaginationContractError(runId, 'did not advance');

Both raise WORLD_CONTRACT_ERROR, which is non-retryable and whose hint asks the user to file a bug with the runId. A World that truncates at a hole, makes no progress, and still reports hasMore: true with the cursor the caller just sent therefore fails the run outright — and "made no progress because the hole is still young" is the normal steady state under sustained fanout, not an edge.

Since this section is the contract a World author implements against, it should state the rule: a truncated page that returned nothing must report hasMore: false and let the caller come back, rather than re-offering the same cursor. Worth adding a world-testing conformance case too — the suite currently has nothing that exercises a sealed or truncated page, so a World author has no way to check either half of this new contract.


The runtime's side of the contract: it skips `noop` events during replay — never delivered to a consumer, never advancing the deterministic clock — so a sealed log replays identically to one whose holes were filled by their writers. `noop` is not user-creatable and is never sent to `events.create()`; only your own read path may write one. Worlds that allocate at the commit (a synchronous counter, a unique-constraint append) keep perfect density by construction and never need any of this — `world-local` and `world-postgres` never seal, and a World that allocates at the commit is spec-7 compliant with no work.

Note the version a World stamps comes from `mintedSpecVersion()`: 7 by default, and the slot-identity version when [`WORKFLOW_SEALED_LOG=0`](/docs/configuration/runtime-tuning#workflow_sealed_log) switches it off. Declare `mintedSpecVersion()` rather than a literal so your World moves with the fleet, and note that a run created at spec 7 may be read by a runtime other than the one that created it — the reader has to understand `noop` before anything stamps 7 in that environment.

`events.create()` params carry `eventCount`: how many events the writer held in the log it replayed from, which is the position it expects to land on minus one. Attempt `eventCount + 1`. When that position is taken, **do not reject the write**. Advance to the next free position, commit there, and return the events occupying the positions you skipped over on the success response, in `events` with a matching `cursor` and `hasMore`. The writer merges them into its own log and replays once, rather than paying a second round trip to discover it was behind. A caller with a stale count is the normal case for a fan-out, and rejecting it would serialize writes the runtime issues in parallel.

### Optional: Rejecting a Stale Write
Expand Down
161 changes: 161 additions & 0 deletions packages/core/src/events-consumer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1167,3 +1167,164 @@ describe('EventsConsumer', () => {
});
});
});

describe('sealed-log noop events (specVersion 7)', () => {
function logEvent(eventType: Event['eventType'], id: string): Event {
return createMockEvent({ id, eventId: id, eventType } as Partial<Event>);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review: Note

These are good tests and they are also, after d30f7e5a0d, close to the whole of the coverage. Unpinning e2e from the server preview is correct for merge, but it means nothing in CI now sees a noop end to end: world-local and world-postgres never seal, so the only paths that produce one are unit tests — and the schema half of those does not run at all (see my note on packages/world/src/events.test.ts).

That is the structural reason the QuickJS clock bug survived a fully green e2e matrix including every quickjs lane: no lane can produce the event that would expose it.

Two cheap ways to close it, either is enough:

  • a world-testing conformance case that stores a seal and replays across it, which runs against every World and both engines; or
  • keep the server-preview pin alive on a scratch branch and record the result here, so the sealed path is exercised at least once before this lands.

function consumerFor(ids: string[]) {
const seen: string[] = [];
const callback = (event: Event | null) => {
if (event && ids.includes(event.id) && !seen.includes(event.id)) {
seen.push(event.id);
return EventConsumerResult.Consumed;
}
return EventConsumerResult.NotConsumed;
};
return { seen, callback };
}

it('steps over a noop without offering it to any consumer', async () => {
// The backend sealed an abandoned slot between two real events. The walk
// must pass through it as if the position never had a writer: both real
// events land, nothing is reported unconsumed, and the callback is never
// even offered the noop.
const noop = logEvent('noop' as Event['eventType'], 'noop-1');
const before = logEvent('wait_created', 'wait-1');
const after = logEvent('wait_completed', 'wait-2');
const onUnconsumedEvent = vi.fn();
const offered: (string | null)[] = [];
const consumer = new EventsConsumer([before, noop, after], {
onUnconsumedEvent,
getPromiseQueue: () => Promise.resolve(),
isDeliveryIdle: () => true,
});
const reals = consumerFor(['wait-1', 'wait-2']);
consumer.subscribe((event) => {
offered.push(event === null ? null : event.id);
return reals.callback(event);
});

await vi.waitFor(() => {
expect(reals.seen).toEqual(['wait-1', 'wait-2']);
});
expect(consumer.eventIndex).toBe(3);
expect(onUnconsumedEvent).not.toHaveBeenCalled();
expect(offered).not.toContain('noop-1');
});

it('never advances the deterministic clock off a noop', async () => {
// A noop's createdAt is the SEALER's wall clock — it can postdate every
// real event around it. Letting it reach onConsumedEvent would leak that
// timestamp into replay Date.now() and diverge from a log whose hole was
// filled by the real writer instead.
const noop = createMockEvent({
id: 'noop-1',
eventId: 'noop-1',
eventType: 'noop',
createdAt: new Date(Date.now() + 60_000),
} as Partial<Event>);
const real = logEvent('wait_created', 'wait-1');
const onConsumedEvent = vi.fn();
const consumer = new EventsConsumer([noop, real], {
...defaultOptions,
onConsumedEvent,
});
const reals = consumerFor(['wait-1']);
consumer.subscribe(reals.callback);

await vi.waitFor(() => {
expect(reals.seen).toEqual(['wait-1']);
});
expect(onConsumedEvent).toHaveBeenCalledTimes(1);
expect(onConsumedEvent).toHaveBeenCalledWith(real);
});

it('is scheduling-neutral: same offers, same tick, as the log without noops', async () => {
// The skip is a synchronous `continue` inside the walk pass — it consumes
// no extra micro- or macrotask. This pins that: a log with noops
// interleaved at the head, middle, and tail is fully consumed after the
// SAME single tick as its noop-free twin, and the sequence of events
// offered to consumers is byte-for-byte identical. Deterministic
// scheduling is what keeps replay ULID draws (and therefore correlation
// ids) stable across branches racing in Promise.all.
async function offersAfterOneTick(events: Event[]) {
const offered: (string | null)[] = [];
const consumer = new EventsConsumer(events, {
onUnconsumedEvent: vi.fn(),
getPromiseQueue: () => Promise.resolve(),
isDeliveryIdle: () => true,
});
consumer.subscribe((event) => {
offered.push(event === null ? null : event.id);
return event === null
? EventConsumerResult.NotConsumed
: EventConsumerResult.Consumed;
});
// subscribe() schedules exactly one nextTick; the walk drains
// synchronously inside it. One tick must therefore finish either log.
await waitForNextTick();
return { offered, index: consumer.eventIndex, total: events.length };
}

const clean = await offersAfterOneTick([
logEvent('wait_created', 'w1'),
logEvent('wait_completed', 'w2'),
]);
const sealed = await offersAfterOneTick([
logEvent('noop' as Event['eventType'], 'n0'),
logEvent('wait_created', 'w1'),
logEvent('noop' as Event['eventType'], 'n1'),
logEvent('noop' as Event['eventType'], 'n2'),
logEvent('wait_completed', 'w2'),
logEvent('noop' as Event['eventType'], 'n3'),
]);

expect(clean.index).toBe(clean.total);
expect(sealed.index).toBe(sealed.total);
// Identical offer sequences — the noops were never offered at all, and
// both logs finished inside the same single tick.
expect(sealed.offered).toEqual(clean.offered);
});

it('consumes an all-noop log to the end without divergence', async () => {
const onUnconsumedEvent = vi.fn();
const consumer = new EventsConsumer(
[
logEvent('noop' as Event['eventType'], 'n1'),
logEvent('noop' as Event['eventType'], 'n2'),
],
{
onUnconsumedEvent,
getPromiseQueue: () => Promise.resolve(),
isDeliveryIdle: () => true,
}
);
consumer.subscribe(() => EventConsumerResult.NotConsumed);

await vi.waitFor(() => {
expect(consumer.eventIndex).toBe(2);
});
expect(onUnconsumedEvent).not.toHaveBeenCalled();
});

it('handles a log that ends on a noop', async () => {
const real = logEvent('wait_created', 'wait-1');
const noop = logEvent('noop' as Event['eventType'], 'noop-1');
const onUnconsumedEvent = vi.fn();
const consumer = new EventsConsumer([real, noop], {
onUnconsumedEvent,
getPromiseQueue: () => Promise.resolve(),
isDeliveryIdle: () => true,
});
const reals = consumerFor(['wait-1']);
consumer.subscribe(reals.callback);

await vi.waitFor(() => {
expect(reals.seen).toEqual(['wait-1']);
});
expect(consumer.eventIndex).toBe(2);
expect(onUnconsumedEvent).not.toHaveBeenCalled();
});
});
29 changes: 28 additions & 1 deletion packages/core/src/events-consumer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { type Event, entityEventClass, envNumber } from '@workflow/world';
import {
type Event,
entityEventClass,
envNumber,
isSealedNoopEvent,
} from '@workflow/world';
import { eventsLogger } from './logger.js';

/**
Expand Down Expand Up @@ -331,6 +336,10 @@ export class EventsConsumer {
// event's by the index it holds.
this.drainParked();
const currentEvent = this.events[this.eventIndex] ?? null;
if (currentEvent !== null && isSealedNoopEvent(currentEvent)) {
this.skipSealedNoop(currentEvent);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review: Blocking

Second issue in this file, from the other direction: handleEndOfLog decides whether a parked event is orphaned by looking at the log's last row, and a trailing seal defeats it.

// :598
const last = this.events.at(-1);
if (!last || !TERMINAL_EVENT_TYPES.has(last.eventType)) {
  // "A later replay is still expected" -> debug log, return

TERMINAL_EVENT_TYPES is {run_completed, run_failed, run_cancelled}. With a noop as the last element, a parked event on an already terminated run stops reaching scheduleUnconsumedCheck(..., false) and is downgraded to a debug line. That swaps a loud ReplayDivergenceError for a silent stall — the run just stops, holding an event no consumer will ever come for.

The "a seal always has a committed position above it" argument does not rescue this, because this.events is a loaded prefix, not the whole log. A page truncated below a young hole ends on whatever sits under that hole, and if that position was itself sealed earlier, at(-1) is a noop. Incremental appends can land the same way.

The shape was clearly on your mind — events-consumer.test.ts adds 'handles a log that ends on a noop' — but this reader of at(-1) was not audited alongside it. findLast((e) => e.eventType !== 'noop') fixes it, and that test is one assertion away from covering it.

continue;
}
const consumed = this.offer(currentEvent);
if (consumed) {
this.eventIndex++;
Expand Down Expand Up @@ -540,6 +549,24 @@ export class EventsConsumer {
return key === undefined ? undefined : this.seenEventClasses.get(key);
}

/**
* Steps the walk over a sealed-log `noop` (specVersion >= 7): the World's
* backend wrote it to occupy a slot whose writer allocated the position and
* died, so the log's density arithmetic holds. It is invisible to the
* workflow: no consumer is offered it, no event class is recorded, and —
* exactly as with {@link skipDuplicateEvent} — the deterministic clock does
* not advance, so a log that happens to contain one produces the same
* timestamps as a log that does not. (Its `createdAt` is the seal time,
* which can even postdate later slots' events; letting it touch the clock
* would leak the sealer's wall clock into replay.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review: Blocking

The clock rule this docstring establishes is implemented for the node:vm engine only. The QuickJS engine has its own event loop — processEvents in packages/core/src/runtime/quickjs-runtime.ts — and at line 1808 it calls:

advanceClock(+event.createdAt);

for every row in the log, before the if (!cid) continue guard and before the switch (event.eventType). A noop carries a correlationId (noop_<position>) so it passes the guard, matches no case, and is otherwise inert — but it has already moved the clock. grep -c noop packages/core/src/runtime/quickjs-runtime.ts is 0 on this head.

Reproduced with the existing runQuickJSWorkflow harness: two logs for the same run, identical except position 3 is a writer-filled attr_set (in-order clock) vs a noop sealed an hour later, with the sleep's wait_completed above it.

- noopSealed:    "2025-01-01T00:00:11.000Z"   (expected)
+ noopSealed:    "2025-01-01T01:00:00.000Z"   (actual)
  writerFilled:  "2025-01-01T00:00:11.000Z"

Date.now() after the wait jumps to seal time, and because the replay clock is monotonic every subsequent read in the run is poisoned. Concretely that means: the same log replays with different timestamps on the two engines, and a replay taken before the seal (log truncated at the hole) disagrees with one taken after. Both engines ship and both run on every E2E matrix row.

Worth noting the same reasoning is already applied correctly one layer up — packages/web-shared/src/lib/sealed-events.ts excludes noops from span geometry and latestKnownTime for exactly this reason. The replay clock needs the same treatment. The events-consumer.test.ts and step-delivery-ordering.test.ts coverage added here is good but is node:vm-only, so it cannot catch this; a QuickJS case belongs alongside the deterministic replay clock suite in quickjs-runtime.test.ts.

*/
private skipSealedNoop(event: Event) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review: Blocking

The QuickJS engine still advances the deterministic clock on a noop. This skip covers the node:vm engine only.

EventsConsumer is one of two replay paths. The node:vm engine feeds its clock from onConsumedEvent (workflow.ts:417), so bypassing offer() here correctly starves it. The QuickJS engine has its own loop over the raw event array:

// runtime/quickjs-runtime.ts:1802
for (const event of events) {
  advanceClock(+event.createdAt);   // <- before any filtering
  const cid = event.correlationId;
  if (!cid) continue;
  switch (event.eventType) { /* no noop arm, no default */ }

Two things make this bite rather than glance:

  • advanceClock is Math.max (:1391), so the seal's timestamp ratchets the VM clock forward permanently, and a seal's createdAt is exactly the value the PR says "can even postdate events at higher positions".
  • noop carries correlationId = noop_<slot>, so the if (!cid) continue guard does not skip it.

Reproduced against this branch. Log = run_created@1, wait_created@2, noop@3 (sealed at 01:00:00Z), wait_completed@4 (00:00:11Z), against the workflow from the existing deterministic replay clock suite:

expected 1735693200000 not to be 1735693200000   // == +SEAL_AT

The noop-free twin returns endTime === +waitCompletedAt; the sealed log returns the seal time. So Date.now() inside the workflow jumps an hour, and the same log replays differently before and after the hole is sealed — the exact divergence skipSealedNoop exists to prevent.

The fix is a continue on noop ahead of advanceClock in processEvents. Worth pinning with a QuickJS twin of the 'never advances the deterministic clock off a noop' test added here — that test is what makes the invariant real on node:vm, and its absence on the other engine is why this got through.

this.eventIndex++;
eventsLogger.debug('Skipping sealed-log noop event', {
eventId: event.eventId,
});
}

/** Steps the walk over a repeat of an already-consumed class. */
private skipDuplicateEvent(event: Event, firstType: Event['eventType']) {
this.eventIndex++;
Expand Down
Loading
Loading