Uh oh!
There was an error while loading. Please reload this page.
Replace hook.hasConflict with hook.getConflict() returning the conflicting Run - #2373
Conversation
…ull>)
hasConflict's boolean didn't expose WHICH run owns the token, so the
duplicate run couldn't act on the conflict. getConflict resolves with
null once registration commits, or with a Run handle for the conflicting
run — letting the workflow return/log the owner's runId, inspect its
status, await its result, or cancel it and continue, all in code.
The workflow-mode create-hook module exposes the bundle's compiled Run
class (durable step-proxy methods) on a well-known symbol so the host-
side hook consumer can construct the conflicting run inside the VM.
Contexts without the class (plain unit tests) fall back to a { runId }
object, which is also the documented v4 shape (no native Run
serialization in v4).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>🦋 Changeset detectedLatest commit: 0aa3cb0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
This PR replaces the hook conflict detection API from hook.hasConflict: Promise<boolean> to hook.getConflict: Promise<Run | null>, so a duplicate run can learn which run currently owns a conflicting hook token (and optionally act on it via the returned Run handle). It also updates the runtime, tests, e2e workflows, and v4/v5 docs to reflect the new API.
Changes:
- Replace
hasConflictplumbing withgetConflict, including resolving to a conflictingRunhandle and reusing the same instance across awaits. - Expose the workflow-bundle
Runclass via a newWORKFLOW_RUN_CLASSsymbol so the hook consumer can construct durableRunhandles inside workflow code. - Update workbench workflows, unit/e2e tests, docs (v4 + v5), and changesets for the renamed API.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| workbench/nextjs-turbopack/app/workflows/definitions.ts | Renames e2e workflow entries from hookHasConflict* to hookGetConflict*. |
| workbench/example/workflows/99_e2e.ts | Updates example workflows to use hook.getConflict and return conflict run metadata. |
| packages/core/src/workflow/hook.ts | Implements getConflict resolution, constructs conflicting Run via WORKFLOW_RUN_CLASS. |
| packages/core/src/workflow/hook.test.ts | Updates unit tests to assert getConflict semantics and stable instance identity. |
| packages/core/src/workflow/create-hook.ts | Registers workflow-bundle Run class on globalThis[WORKFLOW_RUN_CLASS]. |
| packages/core/src/workflow.test.ts | Updates workflow runner tests for suspension + getConflict behavior. |
| packages/core/src/symbols.ts | Adds WORKFLOW_RUN_CLASS symbol definition and rationale. |
| packages/core/src/runtime/suspension-handler.ts | Updates comment/semantics references from hasConflict to getConflict. |
| packages/core/src/runtime/suspension-handler.test.ts | Updates tests and descriptions to the new getConflict naming. |
| packages/core/src/runtime.ts | Updates inline-execution gating comments to reference hook.getConflict. |
| packages/core/src/global.ts | Updates hook invocation queue item comment to reflect getConflict. |
| packages/core/src/create-hook.ts | Public Hook interface now exposes getConflict and updated docs/examples. |
| packages/core/e2e/e2e.test.ts | Renames e2e tests and asserts conflict returns owner runId + durable status. |
| docs/content/docs/v5/how-it-works/event-sourcing.mdx | Updates hook conflict behavior description to reference getConflict. |
| docs/content/docs/v5/foundations/hooks.mdx | Updates conflict-checking guidance and examples to use getConflict. |
| docs/content/docs/v5/api-reference/workflow/create-webhook.mdx | Updates webhook return shape docs to getConflict. |
| docs/content/docs/v5/api-reference/workflow/create-hook.mdx | Updates API reference + examples from hasConflict to getConflict. |
| docs/content/docs/v4/how-it-works/event-sourcing.mdx | Updates v4 event-sourcing docs to reference getConflict. |
| docs/content/docs/v4/foundations/hooks.mdx | Updates v4 conflict-checking docs to getConflict ({ runId } guidance). |
| docs/content/docs/v4/api-reference/workflow/create-webhook.mdx | Updates v4 webhook docs to getConflict returning { runId }/null. |
| docs/content/docs/v4/api-reference/workflow/create-hook.mdx | Updates v4 hook API reference to getConflict returning { runId }/null. |
| .changeset/hook-has-conflict.md | Removes changeset for the superseded hasConflict API. |
| .changeset/hook-get-conflict.md | Adds changeset for getConflict. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // executes inside the VM, so `Run` here is the plugin-compiled variant | ||
| // whose methods are durable step proxies. The host-side consumer uses it | ||
| // to construct the conflicting run resolved by `hook.getConflict`. | ||
| (globalThis as any)[WORKFLOW_RUN_CLASS] ??= Run; |
There was a problem hiding this comment.
Fixed in 919bde5: the registration is now guarded on the workflow runtime being present (WORKFLOW_CREATE_HOOK is installed on the VM globalThis before the bundle is evaluated), so importing this module outside the VM neither mutates the host global nor exposes the host-side Run (whose methods are not step proxies). Verified via the hookGetConflict e2e suite against a real bundle.
| * Resolves with the conflicting {@link Run} if another active hook | ||
| * already owns this hook's token, or `null` once the hook has been | ||
| * registered and is ready to receive payloads. |
There was a problem hiding this comment.
Already addressed in dec869d (this review appears to have run against the initial commit): createConflictingRun() no longer falls back to a { runId }-shaped object. When a real Run cannot be constructed — including hook_conflict events without conflictingRunId — getConflict() rejects with HookConflictError instead, and the JSDoc documents exactly that.
| * ``` | ||
| */ | ||
| readonly hasConflict: Promise<boolean>; | ||
| readonly getConflict: Promise<Run<unknown> | null>; |
There was a problem hiding this comment.
Already addressed in dec869d: the runtime now strictly honors Promise<Run<unknown> | null>. The { runId } fallback was removed — degenerate cases (legacy events without conflictingRunId, contexts without the bundle Run class) reject with HookConflictError rather than resolving with a value that lacks Run accessors, so the public type no longer needs widening.
| ``` | ||
| Calling `createHook()` on its own does not register the hook — registration is only committed when the workflow suspends. Awaiting `hook.hasConflict` suspends the workflow to commit the hook registration, then resolves with `false` once the hook is registered and ready to receive payloads, or `true` if another active hook already owns the same token (see [`HookConflictError`](/docs/errors/hook-conflict)). | ||
| Calling `createHook()` on its own does not register the hook — registration is only committed when the workflow suspends. Awaiting `hook.getConflict` suspends the workflow to commit the hook registration, then resolves with `null` once the hook is registered and ready to receive payloads, or with a `Run` handle for the run that owns the token if another active hook already claimed it (see [`HookConflictError`](/docs/errors/hook-conflict)). The conflicting run's accessors are durable steps, so the workflow can inspect `await conflict.status`, wait on `await conflict.returnValue`, or cancel the owner with `await conflict.cancel()` — see [Run idempotency](/docs/foundations/idempotency#run-idempotency) for these strategies. |
There was a problem hiding this comment.
Clarified in 919bde5: the hooks docs (v4 + v5) now state that for hook_conflict events persisted by older worlds without the owning run ID, getConflict() rejects with HookConflictError instead of resolving with an incomplete handle — so when it does resolve with a conflict, the durable accessors are always available.
| "workflow": minor | ||
| --- | ||
| Add `hook.getConflict`, a promise that suspends the workflow to commit hook registration and resolves with the conflicting `Run` when another active hook owns the token (or `null` once the hook is registered), without waiting for hook payload data. |
There was a problem hiding this comment.
that doesn't matter because we didn't ship a new release with the previousl documented API so it's a fast follow and change
There was a problem hiding this comment.
Updated in 919bde5: the changeset now explicitly calls out that this replaces hook.hasConflict and includes the migration (await hook.hasConflict → const conflict = await hook.getConflict(), branch on conflict !== null).
getConflict's contract is Promise<Run | null>. In the degenerate cases
where a real Run cannot be constructed — a hook_conflict event persisted
by an old world without conflictingRunId, or a context that never loaded
the workflow-mode create-hook module — reject with HookConflictError
instead of resolving with a { runId }-shaped impostor.
Test harnesses now register the Run class on the (VM) globalThis like
real bundles do.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TooTallNate
left a comment
There was a problem hiding this comment.
Request Changes — the feature is right, but the WORKFLOW_RUN_CLASS global duplicates infrastructure the serialization pipeline already provides
First: the API direction is a clear improvement. A Run handle beats the boolean (hasConflict told you that, not who), code-driven conflict strategy beats a fixed policy enum, and the dec869d9e decision to reject with HookConflictError rather than resolve a { runId }-shaped impostor keeps the Run contract honest. The determinism story checks out too: both fast-paths settle through promiseQueue, conflictRunRef is shared so repeated awaits observe one instance, replay reconstructs the handle from the same event-log conflictingRunId, and the bundle-compiled Run's accessors are step proxies (I verified the compiled output — Object.defineProperty(Run.prototype, ...) with __step_Run$... proxies), so await conflict.status / conflict.cancel() from workflow code are durable. 1156 unit tests pass locally; the red express E2E lane is failing errorWorkflowNested/errorWorkflowCrossFile (not hook tests) — baseline flake.
The change I'm requesting: use the class registry instead of a bespoke global
The (globalThis as any)[WORKFLOW_RUN_CLASS] ??= Run registration in workflow-mode create-hook.ts re-implements, ad hoc, something the serialization pipeline already does systematically. I built workbench/example from this branch and inspected the compiled flow bundle:
(function(__wf_cls,__wf_id){var__wf_sym=Symbol.for("workflow-class-registry"), ...
})(Run,"class//./packages/core/dist/runtime/run//Run");The SWC plugin already registers the workflow-compiled Run in globalThis[WORKFLOW_CLASS_REGISTRY] at bundle evaluation — because Run carries WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE. This is the exact channel a start()-returned Run already uses to cross from host to VM today: the Instance reducer emits { classId, data: { runId, resilientStart } }, and getClassRevivers(global).Instance looks the class up via getSerializationClass(classId, global) against the VM's global and calls its WORKFLOW_DESERIALIZE.
So the class this PR smuggles through a new symbol is already sitting in the VM's registry when the hook consumer runs. The only missing piece is the lookup key — the plugin derives it from the module path (class//./packages/core/dist/runtime/run//Run in the workbench; different prefix under npm-installed layouts / other bundlers), so it can't be hardcoded.
Proposal (~15 lines total):
- Register
Rununder a stable, well-known classId alongside the plugin's path-derived one.registerSerializationClasscan't be called twice (its non-configurabledefineProperty(cls, 'classId')throws once the plugin IIFE has run), so add a tiny alias helper toclass-serialization.ts:
/** Register an additional stable id for an already-registered class * (skips the classId defineProperty). */exportfunctionaliasSerializationClass(classId: string,cls: Function){getRegistry().set(classId,cls);}Call it at module scope of
runtime/run.ts(e.g.aliasSerializationClass('class//workflow//Run', Run)). Note what this buys over the symbol approach: per-context discipline for free. Evaluated inside the VM, it registers the VM's step-proxyRunon the VM's registry; evaluated on the host, it registers the hostRunon the host's registry. Each global gets its own correct class — which means theif (globalThis[WORKFLOW_CREATE_HOOK])guard and the??=(both of which exist purely to keep the hostRunfrom leaking into the wrong global) simply disappear, along with the module-scope conditional increate-hook.tsand the new symbol insymbols.ts.The consumer constructs through the pipeline's own hook instead of
new RunClass(id):
functioncreateConflictingRun(ctx,conflictingRunId){if(typeofconflictingRunId!=='string')returnnull;constRunClass=getSerializationClass(RUN_STABLE_CLASS_ID,ctx.globalThis);if(!RunClass)returnnull;return(RunClassasany)[WORKFLOW_DESERIALIZE]({runId: conflictingRunId});}Same degenerate-case semantics you already have (registry miss → null → awaiters reject with HookConflictError), but the construction takes the identical code path a serialized Run instance takes everywhere else. class-serialization.ts is deliberately dependency-light ("separate from private.ts to avoid pulling in Node.js-only dependencies"), so importing it from workflow/hook.ts is safe.
Side benefit worth banking: a stable classId hardens Run serialization generally. Today a Run persisted in the event log (e.g. a start()-in-workflow handle in a step result) embeds the path-derived id, which is only safe because version-skew protection pins replay to the same deployment whose bundles agree on the path. A stable alias removes that coupling — any future cross-deployment replay (or a bundler change mid-release) stops being able to break Run hydration with "Class not found".
I considered the fuller "interact with the serialization pipeline" variant — making getConflict() resolution a step so the standard step-return serde does everything — and I think your current design is better: piggybacking on hook_created/hook_conflict costs zero extra events and zero queue hops, where a step-based resolution would add one of each per call. The registry lookup keeps that property while deleting the bespoke channel.
Smaller notes
- The v4 docs describe a
{ runId }return that no released 4.x SDK has (andgetConflictitself isn't onstable). Same sequencing flag as #2015/#2011 — the v4 pages keep widening the documented-but-unreleased surface; fine if the backport chain is imminent, worth holding otherwise. e56e6fbc2→5b3b88b0b's getter→method change (hook.getConflict()) is the right call — a property that allocates and enqueues new promise resolvers on every access was the same traphasConflicthad; a method at least signals effects.- Changeset correctly replaces the
hasConflictone and is scoped to@workflow/core+workflow(minor) — and sincehasConflictshipped in no stable release and at most one beta, replacing rather than deprecating is fine.
Happy to re-review quickly once the registry switch is in — everything else here is ready.
… registry Replace the bespoke WORKFLOW_RUN_CLASS global with the registry the serialization pipeline already uses to revive Run instances: - The SWC plugin already auto-registers the workflow bundle's compiled Run in globalThis[workflow-class-registry], but under a path-derived classId the host cannot know statically. The workflow-mode create-hook module now aliases it under a stable id (class//workflow//Run) via a new aliasSerializationClass() helper (a plain registry entry — registerSerializationClass cannot be reused since the plugin's IIFE already defined the non-configurable classId property). - createConflictingRun() looks the class up with getSerializationClass(RUN_CLASS_ID, ctx.globalThis) and constructs through its WORKFLOW_DESERIALIZE hook, exactly as the Instance reviver would for a serialized Run crossing from a step into the workflow. - Because the registry is keyed per-global, no environment guard is needed: a stray host-side import registers the host Run on the host registry, which is the correct class for that context. The WORKFLOW_CREATE_HOOK guard, the ??=, and the WORKFLOW_RUN_CLASS symbol are all deleted. Verified: 1156 core unit tests; compiled workbench bundle contains the stable alias alongside the plugin's path-derived registration with zero WORKFLOW_RUN_CLASS references; all 5 hookGetConflict e2e tests pass against a local nextjs-turbopack dev server, including conflict resolution reading conflict.status through a durable step.
TooTallNate
left a comment
There was a problem hiding this comment.
Approve — the registry switch from my previous review is now implemented (0aa3cb0)
I implemented the change I requested (after discussing with Nathan), so this approval covers Pranay's feature plus that refactor — Pranay, please give 0aa3cb0 a once-over since it rewrites the class-handoff mechanism you authored.
What changed:
WORKFLOW_RUN_CLASSis gone (symbol, the??=registration, and theWORKFLOW_CREATE_HOOKguard). The workflow-modecreate-hookmodule now aliases the bundle'sRuninto the existing serialization class registry under a stable id (class//workflow//Run) via a newaliasSerializationClass()helper — a plain registry entry, sinceregisterSerializationClass's non-configurabledefineProperty(classId)would throw on a class the SWC plugin already registered.createConflictingRunresolves through the pipeline:getSerializationClass(RUN_CLASS_ID, ctx.globalThis)+ construction via the class'sWORKFLOW_DESERIALIZEhook — the identical path theInstancereviver takes for a serializedRuncrossing from a step into the workflow. Degenerate-case semantics unchanged: registry miss or missing deserialize hook →null→ awaiters reject withHookConflictError.- No environment guard needed: the registry is per-global by construction, so a stray host-side import of the module registers the host
Runon the host registry — correct for that context.
Verification:
- 1156 core unit tests pass;
hook.test.ts/workflow.test.tsmocks updated to register through the registry (the VM-string test now mirrors the real bundle shape, including theWORKFLOW_DESERIALIZEhook) - Rebuilt
workbench/example: compiled flow bundle contains the stable alias alongside the plugin's path-derived registration, with zeroWORKFLOW_RUN_CLASSreferences - All 5
hookGetConflict*e2e tests pass against a localnextjs-turbopackdev server, including the conflict test that resolves the actual owner and readsawait conflict.statusthrough a durable step — the full host → VM-registry →WORKFLOW_DESERIALIZE→ step-proxy chain - The two biome complexity findings in
hook.tsare pre-existing on this branch (identical count before/after)
Remaining notes from my previous review stand but don't block: the v4 docs describe a surface not yet on stable (recurring sequencing flag), and the stable classId incidentally hardens Run event-log serialization against future cross-deployment replay.
Backport PR opened against |
Summary
Follow-up to #2015.
hook.hasConflict's boolean told the workflow that a conflict exists but not which run owns the token, so the duplicate run couldn't act on it. This replaces it withhook.getConflict(), a promise that resolves with:nullonce the hook registration is committed (hook_createdrecorded), orRunhandle for the conflicting run when another active hook owns the token.Like
hasConflict, awaitinggetConflict()suspends the workflow to commit the hook registration without waiting for payload data. But on conflict, the workflow now has the active owner in hand and can choose its own idempotency strategy in code:This is deliberately code-driven rather than a fixed set of ID-reuse policies (cf. Temporal's
WorkflowIdReusePolicy/WorkflowIdConflictPolicy).Implementation
create-hookmodule exposes the bundle's compiledRunclass on a well-known global symbol (WORKFLOW_RUN_CLASS). The bundle'sRunis the SWC-plugin-compiled variant whose accessors (status,returnValue,cancel(), …) are durable step proxies — safe and deterministic inside workflow code. The host-side hook event consumer constructs the conflictingRunfrom thehook_conflictevent'sconflictingRunId(Expose conflicting run id on hook conflicts #2012) using that class.getConflictnever resolves with anything but a realRunornull. In the degenerate cases where aRuncannot be constructed — ahook_conflictevent persisted by an old world withoutconflictingRunId, or a context that never loaded the workflow-modecreate-hookmodule — it rejects withHookConflictError(which still carries the token) instead of resolving with a{ runId }-shaped impostor. Note: the{ runId }shape remains the documented v4 return value, since v4 has no nativeRunserialization — v4 docs direct users togetRun(conflict.runId)inside a step.await hook.getConflict()calls observe the sameRuninstance; both fast-paths settle throughpromiseQueueto keep resolution order aligned with the event log.HookConflictError.Docs
Updated
createHook()/createWebhook()references, hooks foundations, and event-sourcing pages (v4 + v5; v5 documents theRunreturn, v4 documents{ runId }). A follow-up to #2011 adds the full conflict-handling strategy guide to the run-idempotency docs.Validation
cd packages/core && pnpm typecheck && pnpm vitest run src/(1155 tests)pnpm test:docsnextjs-turbopack: allhookGetConflict*tests pass, including the conflict test asserting the duplicate run resolves the actual owner'srunIdand readsawait conflict.status === 'running'via a durable step.