World-side incrementing event ID (specVersion 6) - #3389
Conversation
Event ids become the event's dense 1-based position in its run's log: `evnt_` followed by the slot as a 26-character zero-padded decimal. The padding keeps every existing ULID validator, lexicographic sort key, range fence and `eid:` cursor working untouched, since decimal digits are a subset of Crockford base32 and the width is unchanged. A slot id decodes as a ULID timestamp of zero, so `ulidToDate` refuses one rather than dating the event to 1970. The scheme is self-describing and pinned per run: world-local reads it off the log, world-postgres off the presence of a `workflow_event_slots` row. Runs created before this keep their ULIDs for the rest of their lives, and a log never mixes the two.
A writer now sends the highest slot its loaded log occupies. When the World bumps the write past that slot, it hands back the events sitting on the slots it skipped, in the existing EventResult.events/hasMore fields. The count is the max slot, not the array length: a slot is claimed by the write that occupies it, so an allocation whose insert then fails leaves a permanent hole, and a length would make every later write ask below it and be handed the same events forever. The report is additive and advisory. It does not advance the cursor and does not suppress the ordinary incremental read, so a report that is short (a lower slot whose writer has not committed yet) is self-healing rather than a source of dropped events. hasMore says the set is a lower bound. Client side, the merge happens in the suspension handler's single write funnel so the rest of the flush batch asks for a slot above what was just learned. Merging re-sorts to slot order, which invalidates the payload prewarm scan position, hence the resetScan.
The consumer walked the log strictly in index order and declared replay divergence as soon as the head event was claimed by nobody. Only replay-origin events (run_created, step_created, wait_created, hook_created, ...) carry that ordering claim: their position is the replay's own decision record. Deliveries that arrive from outside the replay (hook_received, step_completed, wait_completed, ...) can legally land at a slot a concurrent writer did not see, so they are now parked and offered to a consumer registered later, under the log index they originally held so delivery barriers keep their ordering. Divergence is still declared when an ordered event cannot be consumed, when the loaded log is already terminal, and when the workflow function returns with an event still parked.
…n's log Every entity family now draws correlation ids from its own sequence, so a replay that disagrees about one sleep() no longer renames every step after it. A run started under the run-wide shared sequence has to keep replaying under it, and rather than pin that with a version or a fleet-wide flag, the run says so itself: a kind's first draw is exactly deriveBody(seed, kind), so one exact match anywhere in the log identifies the scheme. Old runs stay on the shared sequence for as long as they live, with no quiet window. WORKFLOW_PER_KIND_CORRELATION_IDS becomes an override rather than an opt-in: 1 forces per-kind, 0 forces the shared sequence, unset lets each run decide.
Bump-and-report tells a writer what it missed, but only after the write is durable. That is enough for an out-of-band event landing beside a replay's decision, and not enough for one that would have changed it: by the time the stale replay learns it was stale, its step_created is already committed at a correlation id the fresh replay will draw for a different step. A replay-context create now names the correlation ids it is blocked on: queue entries whose creation event is already in the log, so a resolution for them could have been committed unseen. Entries the same suspension is about to create are excluded (their ids were minted by this replay), as are disposed hooks and hooks the suspension aborts itself. A slot-allocating World reads the slots above the writer's eventCount before committing, and rejects with 412 — unseen tail attached — when one of them resolves an awaited id. Everything else stays bump-and-report. The existing precondition recovery path handles the rejection. The set rides on the slot branch only, so ULID-numbered runs and non-slot Worlds are untouched. WORKFLOW_AWAITED_RESOLUTION_FENCE=0 disables it. Measured on the local step-storm repro against world-postgres: 8/18 corrupted at step 4, 1/54 with the fence, at a step-storm p50 of 60s -> 91s.
…writes Two writes in world-local treated an event id as a name rather than a position, which slot ids broke. A `step_started` that creates its step lazily minted a ULID for the synthetic `step_created`, putting two identity schemes in one log. `events.list` cannot paginate that: a ULID id has no sort key, so it lands on every page and the cursor eventually repeats (WORLD_CONTRACT_ERROR). It now draws from the run's own allocator. A lazy hook resume pinned the slot its claim named and refused to bump, so an unrelated event published at that position by another storage instance made the resume either fail its append or, worse, converge onto the occupant and report an `attr_set` as the resume's own event: no error, no second event, payload dropped. The claimed id is now a hint, the append is free to move, convergence identifies the event by its persisted `resumeId`, and the claim is corrected once the append commits.
🦋 Changeset detectedLatest commit: 54958ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📊 Workflow Benchmarkscommit Backend:
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 170410ms → this run 196961ms (Δ +26551ms, +16%) 📜 Previous results (3)c9fae7bTue, 11 Aug 2026 02:04:02 GMT · run logs
944522dTue, 11 Aug 2026 01:04:59 GMT · run logs
3d9c294Tue, 11 Aug 2026 00:21:16 GMT · run logs
ℹ️ Metric definitions & methodologyThe collapsed STSO distribution section above buckets every step gap of the sequential-steps run (not a sampled window), split by whether the step ending the gap ran inline — in the same warm process as the step before it, so the gap is pure framework overhead — or after a queue-hop — the first step of a fresh process, which pays queue dispatch, client reinit and event-log replay. Bars overlay the two runs: Best/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) · SO: stream overhead (end-to-end write+consume time beyond the modelled generation window) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) · stream overhead (text): writer streams 300 variable-length text token deltas paced at 100/s for 3s (a haiku-size LLM's token throughput) while a parallel reader drains the whole stream; SO is the end-to-end write+consume time beyond the 3s generation window (overhead/backpressure) · stream overhead (structured): same workload as stream overhead (text), but each delta is an AI-SDK-style structured object ({ type: 'text-delta', id, text }) instead of a raw string, so the SO gap vs the text scenario is the added serialization cost 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · SO 250/500/1000 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
🧪 E2E Test Results❌ Some tests failed ❌ Failed E2E Tests💻 Local Development (1 failed)nextjs-webpack-stable-node (1 failed):
E2E Test SummarySummary
Details by Category✅ ▲ Vercel Production
❌ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ vercel-multi-region
|
VaguelySerious
left a comment
VaguelySerious
left a comment
There was a problem hiding this comment.
AI review: no blocking issues
| // which id scheme a run uses: it is stamped on `run_created` and read back | ||
| // on every later write, so a run created before v6 keeps its ULIDs even | ||
| // though this adapter now asks for slots. | ||
| specVersion: SPEC_VERSION_SUPPORTS_SLOT_IDENTITY, |
There was a problem hiding this comment.
AI Review: Note
specVersion and capabilities.slotEventIds are both declared statically here, so every new run this adapter starts asks for slot identity and there is no path by which it can be told not to. The block right below documents the opposite choice for the hook-resume fast path, and the reason given applies here word for word: attesting per lookup lets a backend "drop new resumes to the sequential path immediately, without a redeploy of this adapter."
Slot identity is the larger of the two commitments, and it is the one with no escape hatch. A backend that stops accepting slot-numbered runs, for a defect or during a staged rollout, cannot express that to a deployed adapter, and skew protection means the adapter cannot be turned down either: runs keep executing on the deployment that created them. Whatever the backend does in that state, this side has already decided.
The client is otherwise well set up to degrade, which is what makes the static declaration look like the weak link rather than the design. preconditionSnapshotParams keys off the observed ids (maxEventSlot(events)) and falls back to the stateUpdatedAt/stateEventCount/stateCursor triple when the ids it holds are not slots, so a run whose backend numbered it with ULIDs works end to end from a client that asked for slots. The gap is only in how the request is made: run_created names a version and takes an answer of yes or an error, with no third outcome.
Not blocking on this PR, since a backend that degrades rather than refuses makes it moot, but the asymmetry with hookResumeDedupVersion is worth resolving deliberately rather than by omission.
There was a problem hiding this comment.
(AI) The escape clause this note leans on ("a backend that degrades rather than refuses makes it moot") did not hold when it was written: the backend answered 400 on a spec-6 run_created while its slot-identity gate was off, so the request had no third outcome on either side.
That is now fixed on the backend rather than here. A run that asks for slot identity while the gate is shut is admitted and stamped one spec version down, ULID-numbered, through a one-directional clamp applied at the only two places the request can decide a run's identity: the run row write and the resilient-start path that has to pick an id scheme before a run row exists. It never raises a version that did not ask for slots, and every later write still resolves the scheme from the persisted run.specVersion, never from the request.
On why this side stays static rather than moving to a per-lookup attestation like hookResumeDedupVersion: the two decisions are not the same shape. Hook resume is decided per lookup, so an attestation carried on the lookup arrives in time to change the answer. Identity is decided once, at run_created, and pinned by the run row for the run's life. An attestation would arrive after the only moment it could matter. What has to be able to change without a redeploy of this adapter is the answer to the request, and that now can: flipping the gate stops new slot-numbered runs immediately, existing runs keep replaying under the scheme stamped on them, and this adapter reads the scheme off the ids the log returns rather than off what it asked for.
There was a problem hiding this comment.
(AI) Correcting the mechanism in my previous reply. The backend does not stamp a degraded version. The admission gate was removed instead, so a spec-6 run_created is always accepted and identity is decided by the run's declared spec version alone, on the same reasoning that rules out a SPEC_VERSION_MAX_SUPPORTED: refusing at admission rolls nothing back, and only fails writes from clients that were already told the version is supported.
That removes the 400, so the request no longer has an outcome this adapter cannot handle. It does not answer the part of the note about a staged rollout or a defect. With the gate gone there is no backend-side way to stop numbering new runs by slot, and there is no switch on this side either, so the escape hatch this note asks for exists nowhere today. Worth stating rather than leaving implied: the only hold on a slot-identity rollout right now is the choice to ship or not ship this adapter version.
| // backend adopted them keep their ULIDs; the scheme is pinned by the | ||
| // spec version stamped on each run, not by this flag, which only says | ||
| // what new runs get. | ||
| slotEventIds: true, |
There was a problem hiding this comment.
AI Review: Note
The prod lanes fail stepWinsRaceWorkflow on this branch, on the workflow's own internal clock rather than on a test timeout:
E2E Vercel Prod Tests (astro - node):expected 5788 to be less than 5000E2E Vercel Prod Tests (hono - node):expected 5825 to be less than 5000
Two lanes overshooting a 5s bound by 16% within 40ms of each other is not the shape of a timeout flake, and I checked this is not one of the known rotating failures: main's recent runs pass this test (6539ms and 6449ms of harness wall clock, assertion green, so main's internal duration is under 5000), and where main's prod lanes do fail they fail different tests on 120-180s timeouts. The base branch's completed runs failed only on Vercel deployment errors, not on this test. Earlier runs on this branch failed other tests, not this one.
The Event Timeline printed for the failing run localizes the cost to one gap:
+0.0s run_created
+0.0s run_started
+0.1s wait_created
+4.3s step_created (delayMsStep)
+4.3s step_started
+5.4s step_completed
+5.7s run_completed
The workflow races a 1s step against a 10s sleep and returns endTime - startTime measured inside the VM, which the replay clock advances from each consumed event's createdAt. So the assertion is measuring roughly step_completed.createdAt minus run start, and ~800ms of the overshoot sits between wait_created and step_created.
What I cannot do is attribute it. This branch points e2e at a preview backend through WORKFLOW_SERVER_URL_OVERRIDE while main and the base branch run against production, and preview capacity is a competing explanation for several hundred milliseconds at exactly that boundary. I looked at the obvious client-side candidate and do not think it is the cause: settleEventSlotGap only sleeps when it has already found a hole, and under an allocator that computes a position inside the write that fills it, a hole below the log maximum should not be observable in the first place (a writer can only claim N+1 after seeing N committed).
The check that would settle it is a prod-lane run with the override removed once the backend side is deployed, which is the release order anyway. Worth doing before merge rather than reading the red lane as noise: if ~800ms per suspension boundary is the cost of server-side allocation, that is a number to know now, and if it is preview latency the lane goes green and this closes itself.
There was a problem hiding this comment.
(AI) The next run on this branch (4beedf94d3, same preview backend, same WORKFLOW_SERVER_URL_OVERRIDE) passed all 26 E2E Vercel Prod Tests lanes, astro-node and hono-node included. So the overshoot did not reproduce, which defeats the "two lanes within 40ms of each other is not the shape of a flake" argument. It does not prove there is no per-boundary cost; it says the cost, if any, is not deterministic at that boundary, and one green run is weak evidence against a ~16% margin.
One correction to the dismissal of the client-side candidate. settleEventSlotGap does not only sleep when a hole is real: its own doc says a hole can be transient, because two writers can collide and the one that retried past the other can commit first, leaving the lower slot briefly missing. When it fires it costs 25 + 50 + 100 = 175ms of sleep plus three full loadWorkflowRunEvents reloads, which against a preview backend is in the right order of magnitude for the gap in that timeline. I could not confirm that window is actually reachable given how the allocator reads before it writes, so this stays a candidate rather than a conclusion, but it should not have been ruled out on the grounds that a hole below the log maximum is unobservable.
Cheap way to separate the two if a lane goes red again: one run with WORKFLOW_SLOT_GAP_CHECK=0. If the overshoot survives, the gap check is not it. Agreed on the release-order check either way: prod lanes with the override removed, once the backend side is deployed.
# Conflicts: # packages/core/src/private.ts
TooTallNate
left a comment
TooTallNate
left a comment
There was a problem hiding this comment.
Reviewed at 7495e71 (base current main). All suites green locally: core 2047 passed / 3 expected fail, world 108, world-local 540, world-vercel 482; root build + typecheck clean. I reviewed the client-claim predecessor (#3305) in depth, so this focused on the redesign — which resolves every structural concern I raised there.
The order-tolerant consumer is the riskiest piece and it held up:
- All three preservation claims are implemented, not just asserted: the clock takes a max in
workflow.ts'sonConsumedEvent(monotone, never rewinds on late consumption); a parked event is offered witheventIndexswapped to its original log index in a try/finally so its delivery barrier lands where the log put it (this is what keeps cross-replay delivery ordering deterministic — the barrier system orders by index, not by claim time); and the null sentinel path drains parked first but is never suppressed. - The park decision inherits the same deferred grace window as the old unconsumed check, the
events[eventIndex] !== currentEventguard closes the append-drain race, one-shot resolution dedup catches the decidable double-resolution case, and end-of-log with a terminal tail correctly converts a stranded park into divergence. The allowlist-not-complement choice (unknown types keep strict behavior) is the right default. - The honest tradeoff documentation on
attr_set/step_completed(divergence surfaces atstrandedEventrather than at the offending event, because guessing the other way fails healthy runs) is the kind of comment that will save someone a week.
The gap check answers the transient-hole question before I could ask it: settleEventSlotGap re-reads up to 3× with backoff — covering exactly the mid-commit and paged-read windows — with a full reload (a cursor-anchored read starts past the hole and can never see it fill), and only a settled hole is fatal. The maxSlot-not-count reasoning for partial reads, and folding only complete (hasMore !== true) reports into the snapshot, are both correct and both documented at the point of use.
The correlation-scheme detection fixes the #3305 caveat I flagged: reading the scheme off the run's own log via the first-draw fingerprint (the first id of any kind is exactly deriveBody(seed, kind)) means in-flight runs on self-hosted worlds keep replaying under the scheme that minted their ids — no version pinning, no fleet split, and the WORKFLOW_PER_KIND_CORRELATION_IDS=0 footgun is documented.
Mode detection off the id shape (one event settles the whole log) cleanly unifies the two stamping paths — the Vercel adapter's spec-6 declaration and the local/postgres worlds' slotEventIds capability. And the Postgres migration's lock note (pre-build CREATE UNIQUE INDEX CONCURRENTLY, migration adopts it) plus its dedicated changeset is operator-grade work.
Four asks:
- Changeset bump:
@workflow/worldgains real API surface — theslotEventIdscapability field, themaxSlotcreate param,SPEC_VERSION_SUPPORTS_SLOT_IDENTITY/SPEC_VERSION_MAX_SUPPORTED— that should beminor, notpatch, per the convention we've been applying. - Deploy sequencing deserves a hard statement: the Vercel adapter stamps spec 6 unconditionally, and a backend that accepts the stamp without allocating slots would mint ULIDs into a run whose stamp says slots — poisoning its mode signal for life. The backend half must be live everywhere before this ships; put that in the rollout plan explicitly.
- Revert
WORKFLOW_SERVER_URL_OVERRIDEbefore merge (disclosed; the redNo Test Overridescheck is doing its job). - Close or explicitly supersede #3305 so the record shows which design won and why.
CI triage: the Docs Code Samples failure is baseline — I reproduced the identical builders/README.md line-39 failure (MyBuilder undefined) on an unmodified checkout of main, so someone should fix that sample separately. nextjs-webpack canary quickjs is the long-standing HMR flake; python-workbench deploy is the known infra baseline.
The measurement table is the best part of the PR: same repro, four configurations, isolating #3406's contribution from this design's, with the fence's removal justified by 0/36 rather than by argument. That's how a fence should die. Approving.
Correlation ids go back to one monotonic ULID sequence per run, drawn through `ctx.generateUlid()`. The per-kind module, its `CorrelationIdKind` families, the run-log scheme detection, and the `WORKFLOW_PER_KIND_CORRELATION_IDS` override are gone, along with the workbench opt-in that set it. Per-kind sequences narrowed a replay divergence to the kind that actually disagreed, rather than renaming every entity after the extra draw. They never removed the divergence: two replays that disagree about how many steps ran still mint different ids for the next step. The determinism hole that produced those disagreements was in delivery-barrier ordering and is fixed on its own, so nothing here depends on the narrowing, and carrying two id schemes means every run has to be replayed under the one that minted it for the rest of its life. Keeps the separate fix that came with them: `STABLE_ULID` stays bound to the run's seed time, so a stream id minted during serialization cannot latch the host wall clock into the sequence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The backend that serves spec v6 is on main now, so the adapter goes back to resolving its URL the normal way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Event Log Race ReproNo event-log regressions in the latest repro job. Run History
Latest Scenario Breakdown
|
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
The events consumer walks the log synchronously, but the resolutions that walk triggers do not resolve synchronously: a step result hydrates in the host, resolves from a detached continuation behind `awaitEarlierDeliveries`, and only then does VM code run far enough to subscribe the consumer for the next event. The walk therefore routinely sits on an ordered event that nobody has claimed yet while the workflow is mid-flight on its way to claiming it. The deferred unconsumed-event check resolved that with a fixed `DEFERRED_CHECK_DELAY_MS` after the promise queue drained, which is a bet that every delivery lands inside the window. Replaying a batch of N parallel step results loses it: the queue drains with N-1 still on the detached path, and the check raises `ReplayDivergenceError` against a log the same replay goes on to reproduce exactly. Measured on the event-log race repro against world-postgres, on identical event logs: 0 of 114 runs corrupted at a 100ms window, 34 of 42 at 10ms, with the no-parallel-delivery control clean at both. `hasParkedCommittedDelivery` already documents this hazard for the suspension path, where `scheduleWhenIdle` guards it by polling. Export that predicate as `isDeliveryIdle`, thread it into `EventsConsumer`, and poll it before starting the delay timer. Termination is inherited: it counts only deliveries that resolve on their own, so nothing can gate its own retirement, and a genuinely orphaned event reaches the check on the first poll.
| ELSE | ||
| ALTER TABLE "workflow"."workflow_events" | ||
| ADD CONSTRAINT "workflow_events_run_id_id_pk" PRIMARY KEY ("run_id","id"); |
There was a problem hiding this comment.
A failed CREATE UNIQUE INDEX CONCURRENTLY does leaves an invalid index named workflow_events_run_id_id_idx on the table. Should that be cleaned up?
LLM suggests dropping it explicitly in this branch so the two paths really do leave the same schema behind:
ELSE
DROP INDEX IF EXISTS "workflow"."workflow_events_run_id_id_idx";
ALTER TABLE "workflow"."workflow_events"
ADD CONSTRAINT "workflow_events_run_id_id_pk" PRIMARY KEY ("run_id","id");Also maybe add SET lock_timeout before the migrator runs?
There was a problem hiding this comment.
(AI) Both applied in c9fae7b8f7.
The invalid index is dropped in the ELSE branch, before the key is built, with the reason next to it: the IF branch rejects an invalid index, and the key built in ELSE is a different index under a different name, so without the drop the invalid one survives the migration. Never used by a plan, still maintained on every insert. It also makes a second attempt at the concurrent build possible without a manual cleanup first.
SET LOCAL lock_timeout = '10s' is now the first statement. Note it applies to the rest of the batch, not just this migration: the migrator runs every pending migration in one transaction, so a migration that follows this one inherits the timeout. That is written down above the statement.
| for (let attempt = 0; ; attempt++) { | ||
| const [row] = await db | ||
| .insert(Schema.events) | ||
| .values(values as typeof Schema.events.$inferInsert) | ||
| .onConflictDoNothing({ | ||
| target: [Schema.events.runId, Schema.events.eventId], | ||
| }) | ||
| .returning({ | ||
| eventId: Schema.events.eventId, | ||
| createdAt: Schema.events.createdAt, | ||
| }); | ||
| if (row) { | ||
| return row; | ||
| } | ||
| if (!allocates || attempt >= SLOT_INSERT_MAX_ATTEMPTS) { | ||
| if (!allocates) { | ||
| return undefined; | ||
| } | ||
| throw new WorkflowWorldError( | ||
| `Could not allocate an event slot for run "${runId}" after ${SLOT_INSERT_MAX_ATTEMPTS} attempts`, | ||
| { status: 503 } | ||
| ); | ||
| } | ||
| const delay = Math.min( | ||
| SLOT_INSERT_MAX_DELAY_MS, | ||
| SLOT_INSERT_BASE_DELAY_MS * 2 ** attempt | ||
| ); | ||
| await new Promise((resolve) => setTimeout(resolve, Math.random() * delay)); |
There was a problem hiding this comment.
Since the correctness here depends on READ COMMITTED isolation level, we should probably be explicit (hopefully there's a way to set that in drizzle connections)
INSERT … ON CONFLICT DO NOTHING blocks on an uncommitted conflicting row so I dont think we need a sleep after that.
Suggest skipping the sleep for the first several attempts and keeping the backoff only as a late-stage safety valve.
There was a problem hiding this comment.
(AI) Both applied in c9fae7b8f7.
Isolation is now explicit: drizzle.transaction(fn, { isolationLevel: 'read committed' }) on all three transaction sites that can run a slot insert. The dependency is real and was implicit before. Under REPEATABLE READ every retry re-reads MAX(slot) from the same snapshot, so it recomputes the same losing position forever until the attempt budget runs out.
On the sleep: correct, and the code now says why. ON CONFLICT DO NOTHING does not skip an uncommitted conflicting row, it waits on the other transaction and then reports the conflict, so by the time an attempt fails the competitor has already committed and the next MAX(slot)+1 sees it. The first 8 attempts retry with no delay and the exponential backoff starts after that as a late-stage valve for a wide fan-out. That matters for latency: a suspension flush writes its batch concurrently, so the sleep was on the critical path of the flush, not on a rare contended edge.
| export function detectPerKindCorrelationIds( | ||
| seed: string, | ||
| correlationIds: Iterable<string | undefined> | ||
| ): boolean { | ||
| const firstDraws = new Set( | ||
| CORRELATION_ID_KINDS.map((kind) => deriveBody(seed, kind)) | ||
| ); | ||
| let sawCorrelationId = false; | ||
| for (const correlationId of correlationIds) { | ||
| if (!correlationId) { | ||
| continue; | ||
| } | ||
| sawCorrelationId = true; | ||
| if (firstDraws.has(correlationId.slice(-BODY_CHARS))) { | ||
| return true; | ||
| } | ||
| } | ||
| return !sawCorrelationId; |
There was a problem hiding this comment.
Could loading all correlation ids get expensive if we do it for this check? I suppose we we load all of them anyway?
Random thought: should this / could the scheme be encoded in the prefix rather than use parsing. The mismatched case will loop over all correlation ids in the run before deciding it's not per kind id scheme.
There was a problem hiding this comment.
We load all of them anyway, yeah. We haven't yet thought about how to do anything with partial event logs, hence also our assumption that you'll OOM if your log becomes very long
| let candidateFileIds = filteredFileIds; | ||
|
|
||
| if (parsedCursor) { | ||
| if (parsedCursor && !parsedCursor.sortKey) { |
There was a problem hiding this comment.
Note: may not be a big deal for world-local
Sort-key cursors skip the filename prefilter, making pagination O(n²).
&& !parsedCursor.sortKey means every event file for the run gets loaded and parsed, on every page, and then filtered in memory.
There was a problem hiding this comment.
(AI) Fixed in c9fae7b8f7. It was a big enough deal to fix: an event file's name is ${runId}-${eventId}, so for a slot-numbered run the key is right there in the filename and there was no reason to open the file to learn it.
paginatedFileSystemQuery takes a getSortKeyFromFileId alongside getSortKey, applying the same strict comparison the item-level filter applies, and keeping any file whose name does not yield a key so a mixed-scheme directory still decides on the item. Both run-scoped event listings pass it.
Covered by two tests in fs.test.ts: one pages a slot-numbered log to the end and asserts the full slot order, the other spies on readFile and asserts page two opens only the files past the cursor. The second fails without the prefilter (it reads all 12 files instead of 7).
| 'step_started', | ||
| 'step_retrying', | ||
| 'step_completed', | ||
| 'step_failed', |
There was a problem hiding this comment.
Why do we need to park step events?
IIUC step events would have a claimant ready, no?
There is no window in which the replay knows about step_abc but has nothing registered to consume its events.
There was a problem hiding this comment.
This is true... Looking into it
There was a problem hiding this comment.
(AI) You're right, and they're gone as of 54958aeae6.
The code argument, for the record: step() subscribes its consumer before the step's first event can exist; step_created is ordered, so the walk cannot pass it unless a consumer takes it; the consumer stays subscribed until step_completed/step_failed; and after that the World refuses any further write for that step (EntityConflictError on a terminal step status). So there is no window, exactly as you said.
I wanted a measurement too, since the argument only proves parking is unnecessary, not that nothing depended on it. With the four step types removed, the local Postgres repro ran 32 step-storm attempts and produced 13 divergences. Every one of them was on step_created (12) or wait_created (1), both ordered types that this change does not touch. Not one landed on step_started, step_retrying, step_completed or step_failed — which is the direct test, because with parking removed any step event reaching the head unclaimed would surface as a divergence on that type. Zero corrupted logs, unchanged from before.
Two notes on what I did not change:
ONE_SHOT_EVENT_TYPESis now justwait_completed.park()is the only reader of the resolutions it records and it rejects non-parkable types before looking, so thestep_completed/step_failedentries had become dead weight on a hot path.wait_completedstays parkable even though the same argument covers it (the sleep consumer lives from thesleep()call towait_completed). A wait can also be force-completed out of band by the stop-sleeps API, a shape the repro does not exercise at all, and narrowing tolerance is the direction that turns healthy runs intoReplayDivergenceError. Happy to drop it too if you'd rather; I just didn't want to widen the change on an argument I hadn't measured.
Drop the hedged framing that described positional event IDs as one backend-dependent option: v5 ships with the world allocating a dense per-run slot for every event, so the docs describe that single shape. Document event parking in corrupted-event-log, and add an event ID allocation contract (uniqueness, density, the eventCount bump-and-report response) to the World authoring guide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Defaulting the option to always-idle is exactly the pre-gate behaviour, so a construction site could opt a whole replay path back out of the gate without saying so. Every test that drives a consumer with no orchestrator context now passes `() => true` and states why at the call site. Also exports MIN_DEFERRED_CHECK_DELAY_MS so tests asking for the shortest legal delay do not hardcode a number the floor would clamp up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # packages/core/src/events-consumer.test.ts # packages/core/src/events-consumer.ts # packages/core/src/unconsumed-check-delivery-idle.test.ts # packages/core/src/workflow.ts
…efilter - `0019_add_event_slots.sql` drops an invalid `workflow_events_run_id_id_idx` left behind by a failed `CREATE UNIQUE INDEX CONCURRENTLY` before building the key itself, so both branches leave the same schema, and bounds the exclusive-lock wait with `SET LOCAL lock_timeout = '10s'`. - `world-postgres` pins the slot-insert transactions to READ COMMITTED, which is what makes the recomputed `MAX(slot)+1` see a committed competitor, and skips the backoff sleep for the first attempts: `ON CONFLICT DO NOTHING` blocks on an uncommitted conflicting row, so an early sleep only adds latency to a suspension flush. - `world-local` gains a filename-level prefilter for sort-key cursors, so paging a slot-numbered event log no longer loads and parses every event file for the run on every page.
A step's consumer is subscribed by `step()` before the step's first event can exist, `step_created` is ordered so the walk cannot pass it unclaimed, and the consumer stays subscribed until `step_completed`/`step_failed`, after which the World refuses further writes for that step. So no step lifecycle event can reach the walk head with nothing to consume it, and parking them only deferred reports of what is divergence either way. Trims ONE_SHOT_EVENT_TYPES to match: `park()` is the only reader of the resolutions it records, and it rejects non-parkable types before looking, so `step_completed`/`step_failed` entries there were dead weight.
shalabhc
left a comment
shalabhc
left a comment
There was a problem hiding this comment.
Excited about this change. Not just for correctness but I think it unlocks more optimizations.
|
No backport to This is a large architectural change that introduces a new spec version (6) with World-side slot-allocated event IDs, a new To override, re-run the Backport to stable workflow manually via |
3 participants
Alternative to #3305. Same goal, but with World-side allocated event ids, so a suspension flush stays a parallel fan-out of writes rather than N chained round-trips.
This should fix
CORRUPTED_EVENT_LOGby guaranteeing that the event log stays append only, and that decisions on the client side are made with correct prefixes only.Depends on #3406 (merged), which is the other half of the result below: it closes a determinism hole in the delivery-barrier ordering that this PR's earlier revisions were papering over with a World-side fence. That fence is gone (see "What this PR does not do").
1. Event ids become slots
evnt_+String(slot).padStart(26, '0'), first slot 1, dense and contiguous per run, allocated World-side. The padding is to make it a valid Crockford base32 and pass validation for the old schema. To make this work, we had to stop relying on ULID as a timestamp.isSlotId()guards that:ulidToDateandvalidateUlidTimestamprefuse rather than return epoch 0.A run is pinned to the scheme stamped on its own
run_created, so an existing run keeps replaying under ULIDs no matter what the writing client supports. Slot identity isspecVersion6;SPEC_VERSION_CURRENTstays 5 until every World can allocate slots.2.
eventCounton write, skipped slots on the success responseA writer sends how many events it had loaded. When the World has to place the write above that, it returns the events occupying the skipped slots in the existing
events/cursorfields, and the runtime feeds them intopendingInlineDelta. The client merges and replays once instead of discovering it was behind on a later round-trip. The World does not refuse a write for being behind.Because slots are dense, a client can tell a complete log from a truncated one by its length alone.
WORKFLOW_SLOT_GAP_CHECK(default on) makes replay refuse a log whose slots are not contiguous, rather than replaying a hole as if it were the end of the log.3. Order-tolerant
EventsConsumerThe consumer walked strictly in index order and declared divergence the moment the head event was claimed by nobody. Only replay-origin events need to match in log order, because their order is the replay's decision record. Deliveries do not.
So: keep
eventIndexas the ordered walk pointer, add aparkedlist. An unconsumed head event of a parkable type (hook_received,wait_completed,step_started,step_retrying,step_completed,step_failed,attr_set,hook_conflict,run_cancelled) moves toparkedand the walk continues.parkeddrains in order on everysubscribe()and before thenullsentinel. Divergence is declared only when an ordered event cannot be consumed, or whenparkedis still non-empty at a terminal state.Three things this had to preserve: the deterministic clock takes a max rather than rewinding when a parked event is consumed late; a late-consumed event registers its delivery barrier under its original log index, not the current one; and parked events never suppress the sentinel that triggers suspension.
Kill switches
WORKFLOW_PRECONDITION_GUARD=0restores the pre-slot watermark guard behaviour.WORKFLOW_SLOT_GAP_CHECK=0stops replay refusing a non-contiguous log.WORKFLOW_DEFERRED_CHECK_DELAY_MStunes the deferred-delivery re-check.Docs Preview
corrupted-event-logWORKFLOW_SLOT_GAP_CHECK)