Conversation
…ent invocation A concurrent replay that already created a step under the same correlation id used to be tolerated with an info log regardless of what it created. When the two replays bound the id to different step calls, the step body ran with the winner's arguments and the loser's branch received its result: silent cross-wiring when both calls were the same step function. Read the persisted step on the 409 and compare its name and (decrypted) input with ours; a mismatch is a non-deterministic replay and now fails the run as CORRUPTED_EVENT_LOG instead of continuing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 7e6492e The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
Contributor
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (105 failed)redis (20 failed):
turso (85 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain in duplicate-step conflict handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Backports log-order correlation-ID fixes to stable and adds validation for conflicting duplicate step_created events.
Changes:
- Pins correlation-ID draws and delivery ordering to the event log.
- Detects conflicting duplicate steps and routes corruption to terminal failure.
- Adds regression tests and release changesets.
File summaries
| File | Reviewed change |
|---|---|
packages/core/src/workflow.ts |
Adds seeded ULID generation and draw counting. |
packages/core/src/storm-log-sweep.test.ts |
Updates mint-count test coverage. |
packages/core/src/storm-log-replay.test.ts |
Updates replay draw-count coverage. |
packages/core/src/step-delivery-ordering.test.ts |
Tests parked delivery ordering. |
packages/core/src/runtime/suspension-handler.ts |
Invokes duplicate-step verification; placeholder conflicts still lack name validation. |
packages/core/src/runtime/suspension-handler.test.ts |
Tests duplicate-step conflict handling. |
packages/core/src/runtime/step-create-conflict.ts |
Compares duplicate step names and inputs; placeholder detection uses marker text matching. |
packages/core/src/runtime.ts |
Routes corruption errors to terminal run failure. |
packages/core/src/private.ts |
Implements log-order quiescence; the awaitEarlierDeliveries JSDoc is detached from its declaration. |
packages/core/src/log-order-draws.test.ts |
Tests prefix-stable draw ordering. |
packages/core/src/delivery-barrier-dispenser.test.ts |
Adds mint-count test parity. |
packages/core/src/delivery-barrier-coverage.test.ts |
Tests barrier idle reachability and termination. |
.changeset/step-created-conflict-guard.md |
Documents the duplicate-step guard release. |
.changeset/quiet-donkeys-repeat.md |
Documents delivery-ordering fixes. |
.changeset/log-order-draws.md |
Documents log-ordered draws. |
Review details
Suppressed comments (1)
packages/core/src/private.ts:437
- The detailed JSDoc immediately above this block describes
awaitEarlierDeliveries, but these newly inserted helper declarations now sit between that comment and the exported function. As a result the ordering contract is detached fromawaitEarlierDeliveriesin generated docs/tooltips and the function appears undocumented; move the helpers or move the JSDoc next to its declaration.
/**
- Files reviewed: 15/15 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
475
to
+479
| if (EntityConflictError.is(err)) { | ||
| runtimeLogger.info('Step already exists, continuing', { | ||
| workflowRunId: runId, | ||
| // A concurrent handler wrote this step first. Benign when it | ||
| // wrote the same invocation; a different name or input under | ||
| // this id is a non-deterministic replay and fails the run. | ||
| await verifyDuplicateStepCreate({ |
Comment on lines
+125
to
+129
| // The winner recorded the placeholder `finalizeUnserializableStep` | ||
| // writes when a step's arguments refuse to serialize. That branch is | ||
| // about to fail the step with the serialization error, which is the | ||
| // outcome the user should see; do not mask it with a corruption report. | ||
| if (isUnserializablePlaceholder(a)) return 'incomparable'; |
VaguelySerious
approved these changes
Sep 11, 2026
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #3406 and #3700 to
stable, plus a new guard that turns the silent variant of the same bug into a run failure.Why
A production run on
workflow@4.8.5completed successfully while one branch of aPromise.allSettledfan-out wrote another branch's step result to its own row. The event log shows how: the fan-out's 14findCurrentStatusStepcreates were minted by two concurrent invocations (a queue-woken replay and the original invocation continuing after its inline step), each drawing the run's ordinal correlation ids in a different order and each losing 6-8 of itsstep_createdwrites to the other as duplicates. The step body ran with the first writer's arguments; a later replay bound the same id to a different branch and handed that branch the result.This is exactly the draw-order dependence #3700 fixes on
main.stablehas #3554 (delivery retirement pinned to log order) but not #3700 (draw order pinned to log order), so 4.8.5 through 4.8.8 are all affected. It stayed silent because the step consumer only checks the step name on an incoming event, and the suspension handler swallows the 409 on a duplicatestep_createdwith an info log; when both branches call the same step function nothing notices.What's in here
gatesOnextraction; prerequisite for [core] Pin correlation-id draw order to event-log order #3700'sprivate.tschanges). This is the same commit as the open backport [core] Keep step results ordered behind waits parked on unread hook payloads (#3406) (backport to stable) #3718, cherry-picked here because [core] Pin correlation-id draw order to event-log order #3700 doesn't apply without it. If [core] Keep step results ordered behind waits parked on unread hook payloads (#3406) (backport to stable) #3718 merges first this PR rebases trivially; otherwise [core] Keep step results ordered behind waits parked on unread hook payloads (#3406) (backport to stable) #3718 can be closed as included.workflow.tsonstablehas drifted frommain(noonDuplicateEvent, no clock guard,generateUlidinline on the context), so themintCountplumbing was rewritten against thestableshape.stablehadvmGlobalThis[STABLE_ULID] = ulid(the raw factory, unseeded). It now installs the counting,startedAt-seeded generator, which is what [core] Pin correlation-id draw order to event-log order #3700 relies on for quiescence and is also [core] Derive correlation ids from per-kind sequences (opt-in) #3301's fix for a stream id minted during dehydration latching the host wall clock into the id sequence.test-support/orchestrator-context.tsdoesn't exist onstable(it came with [core] Ignore duplicate events per event class instead of failing the run #3381); the four affected suites here each define their own context and received the samemintCountaddition.log-order-draws.test.tsusesrunWorkflowand ports unchanged.patch, notminor: onstablethis is a bug fix, and aworkflowminor would force a major on@workflow/ai's peer range.runtime-tuning.mdxdocs hunk targets the v5 docs tree, whichstabledoesn't have;stablehas no runtime env-var reference page to addWORKFLOW_LOG_ORDER_DRAWSto, so it's documented in the changeset only.verifyDuplicateStepCreate(runtime/step-create-conflict.ts). On a 409 forstep_created, read the persisted step and compare its name and its decrypted input bytes with ours. Same invocation: continue as before. Different name or arguments: throwCorruptedEventLogError, whichruntime.tsnow routes to the normal terminal path so the run fails asCORRUPTED_EVENT_LOGinstead of redelivering or continuing with the wrong result. Best-effort by design: if the persisted step can't be read or compared, it logs and continues, so the check can't turn a transient read failure into a failed run. A concurrent serialization-failure placeholder is left to its ownstep_failed.Testing
packages/core: 20 tests insuspension-handler.test.ts+log-order-draws.test.tspass, including 7 new guard tests (same input, different input, different name, encrypted compare by plaintext, unreadable step, missing input, placeholder).@workflow/coresuite: 866 passed, 7 failed. The 7 areserialization.test.ts > DOMException serializationand fail identically on an untouchedorigin/stablecheckout with Node 22.18, so they are unrelated to this change.tsc --noEmitclean.🤖 Generated with Claude Code