Uh oh!
There was an error while loading. Please reload this page.
Add hook.hasConflict for early hook conflict detection - #2015
Merged
Conversation
🦋 Changeset detectedLatest commit: 2342dae 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 |
Contributor
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
workflow with 1 step💻 Local Development
▲ Production (Vercel)
workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
❌ Some benchmark jobs failed:
Check the workflow run for details. |
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new hook.ready promise to let workflow code wait for hook registration (hook_created) independently of receiving payloads, enabling deterministic token claiming for idempotency patterns (and consistent conflict handling via HookConflictError).
Changes:
- Extend the public
Hookinterface withready: Promise<void>and implement it in the workflow VM hook runtime. - Resolve/reject
readywaiters based onhook_created/hook_conflict, including suspension behavior when awaiting readiness before registration is recorded. - Add tests and update v4/v5 documentation to describe
hook.ready(and webhook readiness via the sharedHookinterface).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/workflow/hook.ts | Implements hook.ready tracking/queuing and resolves/rejects it on hook_created/hook_conflict, including suspension when needed. |
| packages/core/src/workflow/hook.test.ts | Adds unit tests covering ready resolution, suspension behavior, conflict rejection, and ensuring payload consumption is unchanged. |
| packages/core/src/workflow.test.ts | Adds integration tests verifying suspension, resolution, and conflict behavior when hook.ready is used inside runWorkflow. |
| packages/core/src/create-hook.ts | Extends the public Hook interface with documented ready: Promise<void>. |
| docs/content/docs/v5/how-it-works/event-sourcing.mdx | Documents that hook.ready rejects on hook conflicts in addition to payload awaiting. |
| docs/content/docs/v5/foundations/hooks.mdx | Adds a section explaining when/how to use hook.ready to wait for registration. |
| docs/content/docs/v5/api-reference/workflow/create-webhook.mdx | Notes ready availability on returned webhook objects and how to use it before sharing the URL. |
| docs/content/docs/v5/api-reference/workflow/create-hook.mdx | Documents hook.ready and adds a registration-focused example. |
| docs/content/docs/v4/how-it-works/event-sourcing.mdx | Mirrors v5 event-sourcing updates for hook.ready conflict semantics. |
| docs/content/docs/v4/foundations/hooks.mdx | Mirrors v5 “Waiting for Hook Registration” guidance. |
| docs/content/docs/v4/api-reference/workflow/create-webhook.mdx | Mirrors v5 webhook ready documentation note. |
| docs/content/docs/v4/api-reference/workflow/create-hook.mdx | Mirrors v5 hook.ready API docs and example. |
| .changeset/hook-ready.md | Declares minor bumps for @workflow/core and workflow to ship the new API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
pranaygp
commented
May 18, 2026
Uh oh!
There was an error while loading. Please reload this page.
pranaygpforce-pushed
the
pranaygp/codex/hook-ready
branch
from
May 18, 2026 23:52
b07bab5 to
4b6edc9CompareUh oh!
There was an error while loading. Please reload this page.
pranaygp
commented
May 19, 2026
Uh oh!
There was an error while loading. Please reload this page.
…hook-ready * origin-https/main: (119 commits) fix: settle aborted parallel steps before completing abortParallelWorkflow (#2244) Add native v4 workflow attribute events (#2226) Version Packages (beta) (#2326) [ci] Fix flaky windows unit tests (#2359) Capture Vercel runtime logs when e2e Vercel Prod lanes fail (#2356) Fix e2e failure reporting under vitest 4 and preserve fetch error causes (#2355) [core] Fix process crash from rejected waitUntil promises (#2336) [core] Remove duplicate `waitUntil` for suspension handler async operations (#2345) Prevent local tests from hanging (#2338) feat(core): add optional namespace for queue topic prefix (#2305) [web-shared] Show precise durations in the new trace viewer (#2335) Validate unique workflow step IDs at build time (#2018) Move run attributes into their own detail card (#2327) [core] Forward-port stream reconnect to getReadable level (#2318) [docs] Add "Step executed multiple times" error page (#2310) Fix flickering on the detail panel when navigating the trace viewer (#2325) Version Packages (beta) (#2254) Patch vulnerable package dependencies (#2301) Deprecate DurableAgent and update it with WorkflowAgent in v5 docs (#2285) [codex] Fix partial world-local exclusive writes (#2296) ... # Conflicts: # packages/core/src/runtime.ts # packages/core/src/runtime/suspension-handler.ts
- hook.hasConflict resolves true when the token is owned by another active hook, false once registration is committed — no throw, so workflows can branch on conflicts early. Awaiting it suspends the workflow to commit the hook registration (createHook alone does not). - Chain the already-created fast-path through promiseQueue so resolution order matches event-log order (review feedback). - Skip inline step execution when a suspension has an awaited hook creation so the hasConflict continuation can advance independently of step execution (review feedback). - Update unit tests, e2e tests, workbench workflows, and v4/v5 docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
TooTallNate
approved these changes
Jun 11, 2026
TooTallNate
left a comment
Member
There was a problem hiding this comment.
Approve — determinism-safe design, strong test coverage; one docs-sequencing flag
I built @workflow/core from this branch and ran the full unit suite locally (1146 tests, matching the PR's claimed validation) plus the three new test files individually (116 tests). All green.
The semantics hold up under determinism scrutiny
This touches the workflow VM's most replay-sensitive machinery, so I traced the settlement paths carefully:
- Every
hasConflictsettlement goes throughctx.promiseQueue— both fast-paths (hasCreated/ conflict-already-known) and the event-driven paths (hook_created→ resolvefalse,hook_conflict→ resolvetrue). Resolution order always matches event-log order, which is the invariant that keeps replay deterministic. - The
hook_conflictconsumer drains payload andhasConflictwaiters synchronously (deferring actual settlement throughpromiseQueue), so the null-event handler can't see them and raise a spurious suspension — same pattern the existing payload path uses. - The suspension trigger condition (
hasConflictPromises.length > 0 && !hasCreated && !hasConflictin the null-event handler, plus theeventLogEmptyfast-path increateHasConflictPromise) correctly mirrors the payload-awaiter pattern for both orderings of "awaited before/after log exhaustion." Both are unit-tested. hasAwaitedHookCreationonly affects scheduling, never event-log content — it decides which invocation executes steps (queue-all +timeoutSeconds: 0re-invoke instead of inline), so there's no replay-divergence surface. The reasoning in the runtime.ts comment is exactly right: running a step inline would serialize work the workflow expressed as parallel (hook.hasConflict.then(() => stepB())racingawait stepA()).- The
EntityConflictErrorpath returninghasAwaitedHookCreation: trueis correct and subtle: on a crash-replay where the hook event already exists, the awaiter still needs an immediate re-invocation to consumehook_createdand resolve — silently swallowing it would hang the awaiter until something else woke the run. - The e2e parallelism proof (
hookHasConflictThenStepParallelWorkflow: 100ms stepB triggered via thehasConflictcontinuation must complete while 10s stepA is still running, asserted via step timestamps) tests the actual claim end-to-end rather than just the mechanism.
My earlier docs comments were both addressed (the create-hook example now explicitly points to awaiting the hook/iterating for payload data).
One sequencing flag: the v4 docs (should-fix before merge, or fast-follow the backport)
hasConflict does not exist on stable — I checked origin/stable:packages/core/src/create-hook.ts and there are zero references. But this PR adds the API to the v4 docs (docs/content/docs/v4/...), and docs deploy from main. The moment this merges, the live v4 documentation advertises a property that no released 4.x SDK has — and it'll stay wrong until a backport merges to stableand ships in a 4.x release.
Two clean options:
- Drop the v4 docs pages from this PR and let the backport PR carry them (the backport flow cherry-picks
docs/content/normally, so they'd land together with the code onstable— though note they'd then need a separate main-side commit since v4 docs render from main), or - Keep them but treat the
stablebackport + 4.x release as an immediate follow-up so the docs-reality gap is measured in days.
Given the changeset is minor (correct for a new API) and this is a self-contained feature — squarely in the auto-backport criteria — option 2 is probably fine, but it should be a deliberate choice, not an accident.
Non-blocking notes
get hasConflict()returns a fresh promise per property access (hook.hasConflict !== hook.hasConflict). Each settles deterministically throughpromiseQueue, so it's correctness-neutral, but identity-unstable promises from a property (vs a method) can surprise — e.g. registering two waiters from what reads like one. Caching the promise per hook after first access would be friendlier and would also make thehasConflictAwaiterflag-set happen exactly once. Fine as-is.- The local
hasConflictboolean insidecreateCreateHooknow shadows the public property name; a rename (sawConflictEvent?) would help the next reader.
CI
The two red E2E jobs are baseline flakes unrelated to this PR: astro fails AbortController > abortFromStepWorkflow (the abort area #2244 just touched on main) and hono fails resilient start: addTenWorkflow ... run_created returns 500 (known flake). No hook test failed anywhere. The required check needs a re-run to go green before merge.
Nice work — this is a well-reasoned addition in a part of the codebase where it's very easy to be subtly wrong.
State both resolution values explicitly (true = token already owned, false = registered) instead of a parenthetical that only described the false case.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pranaygp
commented
Jun 11, 2026
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| expect.anything() | ||
| ); | ||
| expect(result.hasAwaitedHookCreation).toBe(true); | ||
| expect(result.timeoutSeconds).toBeUndefined(); |
Contributor
There was a problem hiding this comment.
Test assertions check non-existent result.timeoutSeconds property, making toBeUndefined() assertions pass trivially without verifying actual behavior.
Contributor
No backport to This is a new minor feature adding the To override, re-run the Backport to stable workflow manually via |
Contributor
Backport PR opened against |
This was referenced Jun 11, 2026
Merged
This was referenced Jun 12, 2026
6 tasks
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 freeto 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.
Summary
Adds
hook.hasConflict, a lazyPromise<boolean>getter that lets workflow code commit hook registration and check for token conflicts without waiting for hook payload data.Calling
createHook()alone does not register the hook — registration is only committed when the workflow suspends. Awaitinghook.hasConflictsuspends the workflow to commit the registration, then resolves withfalseoncehook_createdis recorded, ortrueif another active hook already owns the token. Unlike awaiting the hook payload (which rejects withHookConflictErroron conflict),hasConflictnever throws, so workflows can branch on conflicts early:This gives workflows a way to claim a deterministic hook token for idempotency before running duplicate-sensitive work, and is a prerequisite for #2011, where the idempotency docs/examples should use
await hook.hasConflictinstead of requiring consumers to wait for or resume hook data just to claim the token.Changes
Hook.hasConflict: Promise<boolean>to the public hook interface.hasConflictwaiters withfalseonhook_createdandtrueonhook_conflict; both fast-paths settle throughpromiseQueueso resolution order matches event-log order.hook.hasConflictis awaited before hook creation is recorded.hasConflictcontinuation (e.g.hook.hasConflict.then(() => stepB())) can advance in parallel with other steps instead of only after they finish.hook.hasConflictin the v4 and v5 hook/webhook docs and event-sourcing pages, including the suspension-commits-registration semantics.true).Validation
cd packages/core && pnpm typecheck && pnpm vitest run src/(1146 tests)nextjs-turbopackdev server:pnpm vitest run packages/core/e2e/e2e.test.ts -t 'hook'(20 tests, including the new parallel-continuation test)Docs Preview
Changed pages on the docs preview deployment:
createHook()API referencecreateWebhook()API reference