* feat(world-postgres): retain hook tokens after runs end
* refactor(world-postgres): reuse terminal run statuses
* docs: note Postgres Hook retention support
* fix(world-postgres): expose hook retention deadline
* Fix: Exhaustive `Record<AttributeKey, ...>` in `attribute-panel.tsx` is missing the `tokenRetentionUntil` key that was added to `HookSchema`, causing TS2741 and breaking every Vercel build.
This commit fixes the issue reported at packages/web-shared/src/components/sidebar/attribute-panel.tsx:426
## Bug
Commit `ad58321` added `tokenRetentionUntil: z.coerce.date().optional()` to `HookSchema` in `packages/world/src/hooks.ts:106`. This adds `tokenRetentionUntil` to the inferred `Hook` type.
In `packages/web-shared/src/components/sidebar/attribute-panel.tsx`, `AttributeKey` is a union that includes `keyof Hook`, so `tokenRetentionUntil` becomes a required member of the **exhaustive** `Record<AttributeKey, (value: unknown, context?: DisplayContext) => ...>` object literal `attributeToDisplayFn` (starting at line ~426).
Because the literal had no `tokenRetentionUntil` entry, `tsc` fails:
```
src/components/sidebar/attribute-panel.tsx(426,7): error TS2741:
Property 'tokenRetentionUntil' is missing in type '{ ... }' but required in type
'Record<AttributeKey, (value: unknown, context?: DisplayContext | undefined) => ReactNode>'.
```
This breaks `@workflow/web-shared#build` and therefore every Vercel deployment (17 failing deployments observed, all with this identical error).
## Fix
Added a `tokenRetentionUntil` entry to `attributeToDisplayFn`, placed alongside the other Hook date fields (`lastReceivedAt`, `disposedAt`):
```ts
tokenRetentionUntil: timestampWithTooltipOrNull,
```
`tokenRetentionUntil` is a `Date` field, and `timestampWithTooltipOrNull` (defined at line 402) is the display helper used by all the other surfaced date fields (`createdAt`, `startedAt`, `completedAt`, `retryAfter`, `resumeAt`, `occurredAt`). Given the intent of `ad58321` was to expose the hook retention deadline, surfacing it as a tooltip-annotated timestamp is the consistent choice.
Only `attributeToDisplayFn` is a fully exhaustive `Record<AttributeKey, ...>`; the other maps are `Partial<...>` / `Set`, so no other edits are required.
## Verification
`node_modules` are not installed in this sandbox, so `tsc` could not be executed directly. Verified structurally instead: the newly added `tokenRetentionUntil` entry (line 449) references `timestampWithTooltipOrNull`, which is defined in-file at line 402 and already used by the sibling date entries, so the fix satisfies the missing-key requirement without introducing new type errors.
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: VaguelySerious <mittgfu@gmail.com>
* docs(world-postgres): clarify expired hook rows
* feat(world-postgres): enforce Hook retention limit
* fix(world): remove duplicate Hook retention field
* fix(web-shared): remove duplicate retention renderer
* test(world): remove redundant retention coercion case
---------
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: VaguelySerious <mittgfu@gmail.com>
* feat(core): lazy hook resumption via parallel event write + queue publish (rebased onto #1834 + #3145)
Rebase of #3230 onto current main (267765375 + #1834 resilient resumeHook
+ #3145 event-count-gated replay restart). Reconstructed as a single commit
since `git rebase -i` is unavailable in this environment.
Reconciliation vs the pre-rebase branch:
- Replaces #1834's version-prediction (`supportsQueueHookInput`,
`QUEUE_HOOK_INPUT_MIN_VERSION`) with #3230's capability protocol
(persisted `hookResumeInputVersion` + static `hookResumeDedup`).
- One idempotency protocol: a single `resumeId` + SHA-256 payload digest
per resume, sent to both the direct event write and the queue `hookInput`.
- Two execution tiers: backend+consumer attest dedup -> parallel
`Promise.allSettled(event write, queue publish)`; otherwise plain
sequential (no hookInput/resumeId, event-write errors propagate).
- Consumer re-ensures the `hook_received` event (keyed by resumeId/digest)
after event loading, before replay; skips when already preloaded.
- Preserves #3145: event-count guard, `preconditionReinvocations`,
in-process replay restart, `insertEventByEventId`.
- Removes #1834's resumeId-only test (never released); adds parallel +
consumer-preload + world-local dedup/producer-consumer suites.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(core): read top-level event.resumeId in replay dedup; reconcile unreleased #1834 docs/changeset
- hook.ts: dedup hook_received replay on top-level event.resumeId (the
backend now hoists it to a first-class column), with the legacy nested
eventData.resumeId retained as a deprecated parse-only fallback.
- workflow.test.ts: cover dedup across both top-level and legacy nested forms.
- resume-hook.ts: emit producer recovery telemetry when a transient
event-write failure is swallowed on the parallel path.
- resume-hook.consumer-preload.test.ts: add terminal-run (consume) and
transient-conflict (rethrow/redeliver) re-ensure cases.
- Consolidate the two overlapping changesets into resilient-resume-hook.md
and delete the redundant lazy-hook-resumption.md.
- Docs: return type back to Promise<Hook> (resume-hook.mdx), rewrite the
resilience changelog to the final parallel/deduplicated design, and correct
the WORKFLOW_DISABLE_LAZY_HOOK_RESUME resilience wording.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs,core: rename "Resilient hook resume" → "Lazy hook resume" for consistency
- changelog/index.mdx: update the changelog entry title.
- hook.ts: update the dedup comment label to "Lazy-resume dedup".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: give #3230 its own changeset instead of repurposing #1834's
The lazy-hook-resume work had been folded into #1834's pre-existing
`resilient-resume-hook.md` changeset. Give this PR its own changeset and
delete the superseded #1834 one, whose `resilientResume: true` flag promise
no longer holds (resumeHook() returns plain Promise<Hook>).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: add #3230's own lazy-hook-resumption changeset
Follow-up to 63d877178, which deleted #1834's superseded changeset but did
not stage the replacement. Adds this PR's own changeset.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: tighten lazy-hook-resumption changeset
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: leave #1834's resilient-resume changeset/changelog/docs untouched
Restore #1834's own artifacts that #3230 had rewritten:
- .changeset/resilient-resume-hook.md (restored verbatim)
- docs/.../changelog/resilient-resume.mdx (restored verbatim)
- docs/.../changelog/index.mdx (restored verbatim)
#3230 keeps only its own changeset plus the two docs its code/config genuinely
require: the resumeHook() Promise<Hook> return type (ResumedHook is removed
from the code) and the new WORKFLOW_DISABLE_LAZY_HOOK_RESUME env var.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Reconcile #1834 ResumedHook contract with #3230 parallel resume
Preserve the resilient-resume contract from #1834 on the parallelized
resumeHook() fast path instead of dropping it:
- Restore the `ResumedHook` type (Hook + optional `resilientResume`) and its
exports (`@workflow/core/runtime`, `workflow/api`); resumeHook/resumeHookImpl
return `Promise<ResumedHook>`.
- Set `resilientResume: true` on the swallow-recover branch (transient direct
write failure + successful queue dispatch), absent on the happy/sequential
paths.
- Restore the producer OTEL convention `workflow.hook.resilient_resume` and the
consumer `workflow.hook.resilient_resume_materialized`, wired where the
consumer re-ensures the event.
- Restore the consumer `occurredAt` derivation from the resume ULID so the
materialized hook_received is dated to resume time, not queue-round-trip time.
- Fix the #3230 changeset's contradictory "Still returns Promise<Hook>" line and
update the resilient-resume changelog + resume-hook API reference to the
shipped parallel/dedup behavior.
- Port the #1834 failure-path coverage into resume-hook.parallel.test.ts
(non-retryable event-write rethrow, both-fail prioritizes the queue error,
resilientResume flag + payload delivery on the recovered path).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Address review: drop dead nested resumeId fallback, remove server PR link
- Drop the legacy nested `eventData.resumeId` fallback in the hook consumer.
The nested form was only ever written by unreleased preview builds and is
stripped by `EventSchema` parsing (the `hook_received` eventData schema does
not declare it), so the fallback was dead code. Dedup now keys solely off the
top-level `event.resumeId` column. Repoint the replay dedup test to the
surviving top-level path (it previously exercised the nested form only by
building unparsed Event objects in memory).
- Remove the internal workflow-server PR reference from world-vercel's
capability note (the link 404s outside the org); the note keeps the same
information without the dead link.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Fix Biome lint violations and add Biome CI check
Biome was not configured to respect .gitignore, so ~92% of the 13,355
reported diagnostics came from gitignored build artifacts. Enable VCS
integration (useIgnoreFile), apply safe auto-fixes across the repo, fix
the remaining mechanical errors by hand, downgrade judgment-call a11y /
dangerouslySetInnerHTML rules to warnings, and add a 'biome ci' job to
the Lint workflow so violations block PRs going forward.
* Use an empty changeset (no behavior change, no release needed)
Hooks can carry an optional `resumeContext` mirrored from the run at
creation time. When present, `resumeHook`/`resumeWebhook` resume directly
from it instead of fetching the full run, saving a round trip per resume.
When the context also carries the run's `encryptionPublicKey`, the resume
seals its payload (`encp`) directly to that key. Combined with the sealed
envelope work (#3093-#3096), a default webhook resume then needs neither a
run read nor a cross-deployment run-key lookup: the key is resolved only
when the hook actually stores metadata that must be hydrated symmetrically.
Everything falls back transparently to the full run fetch and symmetric
key when the context (or the public key within it) is absent, so new
clients interoperate with old servers and vice versa.
- world: optional `encryptionPublicKey` on `HookResumeContext`
- world-postgres: `resume_context` column migration
- core: combined fast-path + seal in resume-hook; fast-path control-flow
suite split from the real-serialization crypto suite
- world-vercel: cover the `getEncryptionKeyForRun(runId, { deploymentId })`
overload the fast path relies on
- web-shared: render `resumeContext` in the attribute panel
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat: publish each run's X25519 public key on the run entity
A cross-run writer needs the recipient run's public key to seal a payload
to it. Derive that key at `start()` and stamp it on the run, so a hook
resumption or a forwarded-stream writer can find it on a run fetch it was
already making instead of spending ~350ms on `run-key`.
The key is derived from the per-run key material `getEncryptionKeyForRun()`
already returns, so nothing about key acquisition changes. It is not
secret: the matching private scalar is never stored anywhere, only
re-derived on demand from the deployment's own env seed. Storing it beside
run metadata therefore does not weaken the run's confidentiality.
**Presence is the writer-side gate for sealed envelopes.** A run only
carries a public key if the runtime that created it could also open one —
which holds by construction, since derivation and `encp` dispatch both
live in `@workflow/core`, so any core that can stamp can also open. Runs
are pinned to their creating deployment, so the capability this attests to
is still accurate at resume time. Writers seal iff the field is set and
otherwise fall back to the symmetric path, which makes version skew
degrade gracefully instead of wedging a run.
The field rides on `run_created`, and is mirrored onto the queued
`runInput` so the resilient-start path (server recreates the run from the
queue message when the `run_created` write failed) doesn't silently
produce a run that can't receive sealed writes.
world-vercel's compile-time wire-contract guard caught the new field
before it could be silently dropped on the v4 path, exactly as designed —
routed into the frame meta block as plaintext metadata.
Also adds browser- and VM-safe base64 helpers to `sealed-box.ts`, since
neither `Buffer` nor `btoa` can be assumed in every context that module
runs in. `base64ToBytes` returns undefined on malformed input rather than
throwing, so a corrupt stored key degrades to "no usable public key" and
falls back to the symmetric path instead of crashing a resumption. Both
are cross-validated against `Buffer` in tests.
* review: fix public-key loss on resilient start and lifecycle updates
Two real bugs found in review, both in the local worlds. Neither surfaces
as an error — a run just silently stops accepting sealed cross-run writes
and falls back to the slow symmetric path forever.
**Resilient start dropped the key.** When a `run_started` arrives for a
run that was never created, world-local and world-postgres rebuild the run
from the queued message. Neither copied `encryptionPublicKey` onto the run
row or the synthetic `run_created` event they write. That is precisely the
scenario this field exists to survive. (The equivalent server-side path was
already handled.)
**world-local also wiped the key on every lifecycle transition.** Its
run_started / run_completed / run_failed / run_cancelled handlers rewrite
the whole run document field-by-field, so any field not explicitly listed
is dropped — meaning the key was lost on the *first* `run_started`, not
just on the resilient path. All four rebuild sites now carry it.
world-postgres is safe here by construction because it issues
column-scoped SQL UPDATEs rather than rewriting the row.
**base64 decoding is now strict.** The decoder accepted shapes that
cannot describe a whole number of bytes (`length % 4 === 1`) and ignored
anything after a mid-string `=`, returning a short array instead of
`undefined`. That is worse than throwing: a corrupt stored key looked
*present*, so callers sealed to garbage rather than taking the symmetric
fallback. Now rejects out-of-alphabet characters, bad lengths, misplaced
padding, and non-zero trailing bits — with a round-trip test over every
length 0–48 to make sure the strictness does not overshoot.
* fix: send encryptionPublicKey in the v4 POST frame meta
`splitEventDataForV4` lifted the run's public key into the frame meta and
`events.ts` spread that meta into `CreateEventV4Input`, but
`buildPostFrameMeta` — which copies meta onto the wire field by field — never
forwarded `encryptionPublicKey`, and the field was missing from
`CreateEventV4Input` entirely. Because the meta is applied with a spread,
TypeScript's excess-property check doesn't fire, so the key was computed, put
in the meta, and then silently dropped before the request was sent.
The server therefore never received the key, never stored it on the run
entity, and every cross-run writer fell back to the symmetric envelope. Every
symptom pointed away from the SDK: a deliberately oversized key was accepted
rather than rejected (the field never arrived), the key was absent from the run
row, and `resumeHook()` always chose `encr`.
Add the field to `CreateEventV4Input`, forward it in `buildPostFrameMeta`, and
cover it for both `run_created` and resilient-start `run_started`. Also add a
generic guard asserting that every field the splitter puts in the meta reaches
the wire, so the next omission in this hand-maintained mapping fails a test
instead of silently degrading encryption.
* build: declare typescript (catalog:) in every package that runs tsc
Twenty packages invoke tsc in their build/typecheck scripts without
declaring a typescript dependency, resolving whatever tsc pnpm happens
to leave reachable. That broke locally after the TypeScript 6 upgrade
(#2700): base.json now uses the TS6-only 'types': ['*'] wildcard, and
worktrees carrying pre-upgrade node_modules/.bin/tsc shims (orphaned
typescript@5.9.3 bins that pnpm never refreshes for an undeclared
dependency) fail with TS2688 'Cannot find type definition file for *'.
Declaring 'typescript': 'catalog:' (the convention nest already
follows) makes pnpm own each package's tsc bin, so version upgrades
refresh the shims and this staleness class cannot recur. Packages
without tsc in their scripts are left unchanged.
Full pnpm build: 27/27 tasks green.
* Address review: drop duplicate zod devDep; regenerate lockfile minimally
- packages/world listed zod in both dependencies and devDependencies
(pre-existing on main, surfaced by the devDependencies sort) — keep
the runtime dependency only.
- Regenerate pnpm-lock.yaml from a pristine main baseline with
--lockfile-only (a clean-main run produces zero diff, so main has no
drift). Remaining non-typescript changes are mechanical consequences
of the change itself: typescript is an (optional) peer of several
tooling dependencies, so declaring it in 20 importers creates new
peer-resolution snapshot variants and prunes the now-orphaned old
ones; plus one radix-ui 1.6.1->1.6.2 refresh in docs caused by its
floating 'latest' specifier.
- Validated: pnpm install --frozen-lockfile succeeds; full build 27/27.
* perf(core): lazy inline step start to save a world round-trip per step
The owned-inline runtime path used to write step_created (suspension
handler) and then step_started (executeStep) as two separate world
round-trips for a step it already owns and is about to run inline. This
defers the step_created write: executeStep sends a single step_started
carrying the step input, and the world creates the step on the fly
(materializing the step entity plus a synthetic step_created event so
replay still observes it). Mirrors the existing resilient run_started ->
run_created pattern.
Exactly-one ownership is preserved by the world's atomic create-claim:
the loser of a concurrent lazy step_started gets EntityConflictError,
which executeStep maps to `skipped`, so it never runs the body. A lazy
step_started is only ever sent for a brand-new step (the suspension
handler defers only steps with no prior step_created), so crash recovery
still re-runs a `running` step via the normal non-lazy step_started.
Worlds updated: world-local, world-postgres (implicit create + synthetic
step_created event), world-vercel (routes the input as the v4 frame
payload and threads the server's stepCreated flag). @workflow/world adds
optional `input` to step_started and a `stepCreated` EventResult signal.
Rollout: server-first. The matching workflow-server change must deploy
before this ships; the Vercel world targets a single Vercel-operated
backend (server always >= SDK). For local/postgres the world ships in the
same package as the runtime, so there is no version skew.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(core): materialize deferred step before failing unregistered step on lazy inline path
The lazy inline step-start optimization defers a step's step_created write,
expecting executeStep to materialize the step via a lazy step_started carrying
its input. For an UNREGISTERED step, executeStep bails out before sending that
step_started and writes step_failed directly — but the step entity was never
created, so the world's "step must exist" ordering guard rejects the
step_failed and the run wedges (times out).
This regressed the StepNotRegisteredError e2e tests uniformly across every
framework/world (the ghost step never reached `failed`). Fix: on the lazy path,
send the lazy step_started first to materialize the step (entity + synthetic
step_created, keeping replay correct), then write step_failed. The lazy
step_started's atomic create-claim preserves exactly-one-owner: a concurrent
winner makes ours reject with EntityConflictError → skipped, so the failure is
never written twice.
Adds world-level regression tests (world-local, world-postgres) asserting a
lazy step_started followed by step_failed marks the step failed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(world-local): make duplicate hook_created idempotent
Duplicate processing of the same hook_created — same runId, hookId, and
token, e.g. cross-process replay or queue redelivery — was being recorded
as a hook_conflict in the event log, which then replayed as a self-
conflict HookConflictError.
The fix mirrors the existing step_created duplicate-correlation path:
when the exclusive token claim fails and the existing claim has the same
(runId, hookId), throw EntityConflictError so the runtime's existing
concurrent-replay catch path swallows it. Different runId or hookId
reusing the same token still produces a real hook_conflict.
The persisted token claim already carried hookId; only the read schema
was dropping it. The schema now preserves hookId (marked optional for
backward compatibility with older claim files).
Fixes#2283
* fix(world-postgres): make duplicate hook_created idempotent
world-postgres has the same gap as world-local was just fixed for: the
duplicate-token check in events.create unconditionally writes a
hook_conflict event when an existing hook with the same token is found,
even when the existing hook has the same (runId, hookId) as the
incoming event. The unique partial index on workflow_events does not
catch this because the duplicate path inserts hook_conflict, not
hook_created.
Mirror the world-local fix: when the existing hook's (runId, hookId)
matches the incoming event, throw EntityConflictError so the runtime's
existing concurrent-replay catch path swallows it. Different runId or
hookId reusing the same token still produces a real hook_conflict.
Refs #2283
* test(e2e): add regression test for hook_conflict from same-tick replay race
Regression test for #1665 / #2283. A parent workflow awaits 6 child
workflows with Promise.all; each child does a tiny step and creates one
webhook. Awaited children flatten into the parent run, so all webhook
creations land on the same workflow body. When their step resolutions
align in the same tick the workflow body is re-walked and each pass
submits hook_created with the same deterministic (correlationId, token).
Before the world-side idempotency fix, the world wrote hook_conflict
events for the duplicates and the workflow failed with
HookConflictError. With the fix, duplicates throw EntityConflictError
(swallowed by the suspension handler), no hook_conflict events appear in
the log, and the webhooks resolve normally.
Verified locally against world-local: the test fails reliably (3/3) on
the unfixed code and passes reliably (5/5) on the fixed code.
* test(e2e): rewrite parallelStepsThenWebhookWorkflow to match the actual #1665 repro
The earlier version invoked another 'use workflow' function directly
from inside the parent workflow, which is not a valid child-workflow
invocation (child workflows must be spawned via start()) and didn't
mirror the bug shape on #1665 anyway.
Rewrite the workflow as a single 'use workflow' function that exactly
mirrors Paolo's minimal repro:
await Promise.all([stepA(), stepB()]);
using webhook = createWebhook();
await webhook;
The for-loop runs N independent iterations of that sequence in series,
each disposing its webhook via 'using' before the next, to give the
timing-sensitive race multiple chances to fire.
The race is hard to force deterministically on fast local dev — but
the same (runId, hookId) idempotency invariant is covered
deterministically by the new unit tests in world-local and
world-postgres. This e2e test serves as a higher-level regression net:
its assertions (no hook_conflict event in the log, no
HookConflictError-failed run) are correct whether the race fires or
not, and will catch any future regression on a run that does hit
it.
* fix(world-local,world-postgres): recover crash-orphaned hook claims/rows instead of suppressing the retry
Addresses review feedback on PR #2295.
The original idempotency fix made duplicate same-(runId, hookId)
hook_created submissions throw EntityConflictError so the suspension
handler's concurrent-replay catch path swallows them. But the claim
file (world-local) and hook row (world-postgres) are written before
the durable hook_created event, and the writes are not atomic. A
process / DB interruption between the claim/hook write and the event
write leaves an orphaned claim/hook row; the retry then matched the
same (runId, hookId), threw EntityConflictError, got swallowed, and
the run was permanently left with no hook_created event in the log.
world-local:
- Add a per-(runId, hookId) in-process mutex (withHookLock) mirroring
the existing withStepLock, so two same-tick concurrent calls
serialize on the entity write and the dedup branch never observes
an in-flight winner mid-write.
- In the dedup branch, when the existing claim is for the same
(runId, hookId) we are trying to create, check whether the durable
hook entity actually exists on disk:
- exists → real duplicate: throw EntityConflictError as before.
- missing → orphaned claim from a prior crash: fall through and
complete the partial write (write the hook entity with
overwrite, then emit hook_created via the outer code path).
world-postgres:
- In the dedup branch, when the existing hook row matches the
incoming (runId, hookId), check whether a hook_created event for
this (runId, correlationId) already exists in the event log:
- exists → real duplicate: throw EntityConflictError as before.
- missing → orphaned hook row from a prior crash between hook
INSERT and events INSERT: skip the hook insert (the row is
already there) and let the outer code path emit hook_created,
completing the partial write.
Tests:
- world-local: pre-seed an orphaned token claim with no matching hook
entity, retry hook_created, assert hook entity and hook_created
event both land (no hook_conflict, no EntityConflictError).
- world-postgres: pre-seed an orphaned hook row with no matching
hook_created event, retry, assert hook_created event lands (no
hook_conflict, no EntityConflictError).
Both tests fail on the prior implementation (EntityConflictError
thrown on retry, exact symptom from the review).
* fix(world-local): probe the event log (not the hook entity) to detect duplicate hook_created
Addresses follow-up review on PR #2295.
The previous dedup branch checked whether the durable hook entity
existed on disk. But the hook entity is written before the
`hook_created` event, and the two writes are not atomic, so a crash
between them leaves both the claim file and the hook entity on disk
with no event in the log. The dedup branch then matched on
`(runId, hookId)`, found the hook entity, threw EntityConflictError,
and the suspension handler swallowed the retry — permanently losing
`hook_created` from the event log.
The fix mirrors what the world-postgres branch already does: probe
the run's event log for an existing `hook_created` event for the
same `(runId, correlationId)`. The event is the durable record of a
successful hook creation; the claim file and hook entity are partial-
write artifacts that may exist without the event.
- exists → real duplicate: throw EntityConflictError so the
runtime's concurrent-replay catch path swallows it.
- missing → orphaned partial write (crash at any point before the
event landed): re-write the hook entity (with overwrite: true, in
case a stale partial copy exists) and let the outer code path emit
the hook_created event.
Added a new helper findHookCreatedEvent that runs a filtered
paginatedFileSystemQuery with limit:1 over the run's events.
Regression test "should recover an orphaned hook entity with no
matching hook_created event" added — pre-creates a hook, deletes
just the hook_created event from disk to simulate a crash between
the entity write and the event write, asserts the retry emits a
fresh hook_created event (no hook_conflict, no swallowed
EntityConflictError). I verified this test fails on the prior fix
(throws `EntityConflictError: Hook "hook_orphan_entity_1" already
created`, exactly as pranaygp reported) and passes on this commit.
The previous test ("should recover an orphaned hook token claim
with no matching hook entity") continues to pass — the event-log
probe is a strict superset of the entity probe, since a missing
entity always also implies a missing event.
* fix(world-local): converge same-hook creation across workers via canonical eventId
Addresses follow-up review on PR #2295.
The previous fix made the dedup branch probe the event log to decide
real-duplicate vs orphan-recovery, but the probe and the recovery
write are not a single atomic operation. Two workers sharing a data
directory (or two retries that lose `writeExclusive(constraintPath)`
back to back) could both pass the probe (each observing no
hook_created event yet), both fall through to the recovery write,
and both append a hook_created event with a different eventId —
producing two events in the log for the same (runId, hookId). The
in-process `withHookLock` mutex does not help here because it is
process-local and tag-specific.
The fix persists `eventId` in the durable token claim file (written
by the original `writeExclusive(constraintPath)`). On a same-(runId,
hookId) dedup match, retries adopt that canonical eventId and
rebuild the event with a deterministic createdAt derived from the
eventId (a ULID). The outer event write switches from `writeJSON`
(check-then-write, TOCTOU) to `writeExclusive` (O_CREAT|O_EXCL via
temp-file + hard-link, atomic across processes). Either worker may
win the publish; the other throws EntityConflictError which the
runtime's existing concurrent-replay catch path swallows. Net
result: exactly one hook_created event per logical creation.
Backward compatibility: a claim file written before this commit lacks
`eventId`. Retries that read such a claim fall back to the
event-log probe + fresh-eventId recovery — the legacy behavior that
does not converge across workers but cannot regress for freshly-
written claims after upgrade.
world-postgres already converges across workers via the partial
unique index on workflow_events_entity_creation_unique
(runId+correlationId+eventType for hook/step/wait_created): the
loser's INSERT raises 23505 which is already translated to
EntityConflictError.
Regression tests:
- world-local: `converges same-hook creation across workers to one
event` uses two tagged storage instances sharing one data
directory and fires 25 paired Promise.allSettled hook_created
calls. Expected 25 hook_created events total; before this fix
yielded 50.
- world-postgres: `converges same-hook creation across concurrent
calls to one event` exercises the same shape against the real
Postgres unique index. Already converges; the test is a guard
against future regressions to the catch path.
Verified the world-local test fails on c7b23e1b5 with exactly the
shape pranaygp reported (50 events for 25 logical creations) and
passes on this commit. The earlier orphaned-claim and orphaned-
entity recovery tests also continue to pass.
* fix(world-local): converge legacy hook claims via recovery-marker sidecar; replace tag-proxy test with real subprocess workers
Addresses follow-up review on PR #2295.
Two distinct issues, both flagged by pranaygp as P1:
1. The fallback path for token claims written by versions before
eventId was persisted inline (legacy claims after upgrade) still
permitted the same cross-process corruption the inline fast path
was fixed to prevent. Two processes both reading a legacy claim
each generated their own eventId, landed their
writeExclusive(eventPath) calls at different paths, and appended
two hook_created events for the same (runId, hookId). Existing
persisted claims after a real upgrade are exactly the state the
crash-recovery branch needs to repair, so leaving the legacy path
non-convergent is silent corruption, not backward compatibility.
2. The committed cross-worker convergence test used two tagged
storage instances sharing one directory as a proxy for separate
processes. But tags change the destination filename
(events/wrun_X-evnt_Y.worker-a.json vs ...worker-b.json), so two
tagged workers can each writeExclusive their own event at
different paths and both fulfill. The Map-by-eventId
deduplication in the assertion then masked the duplicate
publication, so the test passed for the wrong reason.
Implementation:
- New HookRecoveryMarkerSchema (`{ eventId, hookId, runId }`) and
HookRecoveryMarkerPath helper. The marker is a sidecar at
hooks/tokens/<hash>.recovery.json, written via writeExclusive so
the first cross-process retry pins its candidate eventId as
canonical; subsequent retries read the marker and adopt that
eventId. Together with the existing writeExclusive(eventPath) in
the outer publish, this gives the legacy-fallback path the same
single-event convergence guarantee as the inline-eventId fast
path.
- pinCanonicalEventIdForLegacyClaim() encapsulates the marker
write-or-read. A stale marker for a different (runId, hookId)
(token-reuse with leaked state) is overwritten best-effort — the
common cross-worker race for the same hook still converges; only
the narrow stale-token-reuse case loses convergence.
- hook_disposed now also deletes the recovery marker when it
deletes the token constraint file, preventing a future legacy
recovery for a recycled token from latching onto a stale eventId.
- The dedup branch unified: existingClaim.eventId for new claims,
pinCanonicalEventIdForLegacyClaim() for legacy ones. Removed the
now-redundant findHookCreatedEvent helper — the
writeExclusive(eventPath) in the outer publish is the
authoritative duplicate-vs-orphan detector.
Tests:
- New test fixture test-fixtures/hook-race-worker.ts (TypeScript,
run via child_process.fork with tsx as execPath — tsx is a
transitive dev dep via vitest). Each subprocess gets its own
createStorage(testDir) so the in-process hookLocks Map cannot
serialize across workers.
- Replaced the tag-proxy test with
"converges same-hook creation across separate OS processes to one
event". Spawns workerCount subprocesses, releases them from a
barrier into the same hook_created, asserts exactly one fulfilled
+ (N-1) rejected with EntityConflictError, and asserts directly
on the raw events.list() result (no Map dedup) that the number of
hook_created entries equals the number of logical creations.
- Added "converges same-hook creation across processes when only a
legacy token claim exists". Same shape, but pre-seeds the legacy
claim format (`{ token, hookId, runId }` with no eventId) before
each race. Verified to FAIL on 7ce66551b (both subprocesses
fulfill, no convergence) and pass on this commit.
- Also verified the new-eventId subprocess test FAILS when the
event write is reverted to writeJSON (TOCTOU), confirming it
exercises the writeExclusive-based cross-process arbitration.
Both prior orphaned-claim / orphaned-entity recovery tests also
continue to pass.
* fix(world-local): per-lifetime recovery markers, restore event-log probe, fix CI tsx resolution
Addresses three P1 review comments on PR #2295.
1. Stale recovery marker leaking across token-reuse lifetimes
(pranaygp):
The previous marker path used `hashToken(token)` so a stale
marker for run A could leak into run B's recovery when the same
token was reused after run A terminated through normal lifecycle.
`deleteAllHooksForRun()` and tagged `world.clear()` deleted the
token constraint and hook entity but NOT the marker sidecar, so
the next legacy claim on the same token entered the stale-marker
overwrite branch and the workers overwrote it non-atomically,
yielding divergent publication.
Fix:
- Marker path now hashes `(token, runId, hookId)` together
(`hookRecoveryMarkerPath` in storage/helpers.ts). Different
lifetimes can never share a marker, so the stale-marker
overwrite branch is removed entirely.
- `hookRecoveryMarkerPath` is moved to helpers.ts and shared
across events-storage.ts, hooks-storage.ts, and index.ts.
- `deleteAllHooksForRun()` and tagged `world.clear()` now also
delete the recovery marker for each hook (disk hygiene; per-
lifetime identity makes leaks no longer corrupting).
- `hook_disposed` now uses the new per-lifetime marker path too.
2. Duplicate `hook_created` event when a legacy claim's event was
already published (VADE bot, also implied by pranaygp's analysis):
Removing the event-log probe from the legacy fallback let a post-
upgrade retry pin a new canonical eventId via the marker and
publish a duplicate event at that path, even when the original
pre-upgrade writer had already successfully published the event
with its own (different) eventId.
Fix:
- Restore `findExistingHookCreatedEventId()` (renamed and made
to return the eventId for clearer semantics).
- Legacy fallback now probes the event log BEFORE pinning the
marker; if a matching `hook_created` event already exists,
throw `EntityConflictError` so the runtime's concurrent-replay
catch path swallows the retry.
- Inline-`eventId` fast path does NOT need the probe — the claim
itself is the durable convergence key.
3. CI failure: tsx not resolvable under pnpm isolated linking
(pranaygp; confirmed by ubuntu/windows unit test 60s timeouts):
The previous test hard-coded `node_modules/.bin/tsx` assuming
tsx would be hoisted there. But tsx was only a transitive peer
dep via vitest, and pnpm's isolated linking does NOT link
transitive peer deps into the workspace bin after a fresh
install — so neither root nor package-local `.bin/tsx` existed
in CI, the subprocess fork never started, and the barrier hung
until vitest killed the test.
Fix:
- Add `tsx` as a direct `devDependency` of `@workflow/world-
local` (pinned to 4.20.6 to match the existing transitive
resolution).
- Resolve via `import.meta.resolve('tsx/package.json')` and read
the `bin` field dynamically, so we adapt to wherever pnpm
links tsx for this package — not a hard-coded layout.
- Lazy-init the resolver (no module-load IIFE) so an absent tsx
fails only the convergence tests, not all 376 tests in the
file.
- Surface a clear error message if resolution fails, calling
out the cause (transitive vs direct deps) for future readers.
Also: harden the barrier helper so `error` events and pre-ready
exits resolve BOTH `readyPromises` and `donePromises`, then
`SIGKILL` siblings. Previously a broken child only resolved
`donePromises`, leaving `Promise.all(readyPromises)` pending
until the per-test timeout (60s in CI).
Regression tests added:
- `legacy claim whose hook_created event was already published does
not append a duplicate event` — pre-seeds a legacy claim AND a
pre-existing `hook_created` event with a different eventId,
asserts the retry throws EntityConflictError and the log still
has exactly the original event.
- `converges legacy claim recovery across run lifetimes after token
reuse` — runs pranaygp's full lifecycle path: race subprocess
workers on run A's legacy claim, terminate run A via
`run_completed` (triggers `deleteAllHooksForRun`), reuse the
token in a legacy claim for run B, race subprocess workers again,
asserts exactly one fulfillment + one `EntityConflictError` per
race and exactly one `hook_created` event per run.
Both new tests verified to fail on 2c673e436 (after rebuilding):
the published-event test throws via duplicate publish instead of
EntityConflictError, the token-reuse test sees both run B workers
fulfill (2 events instead of 1).
The existing orphaned-claim and orphaned-entity recovery tests also
continue to pass.
CI loop confirmed to be repaired locally by spawning subprocesses
via the new resolver and intentionally breaking the worker fixture
to verify the helper fails fast (~500ms) instead of hanging at the
barrier.
* fix(world-local): defer hook entity write until event publish commits
Addresses karthikscale3's P1 review comment on PR #2295.
The dedup-recovery path used to write the hook entity BEFORE the
outer event publish proved whether the attempt was repairing a
missing event or just colliding with an already-published
`hook_created`. For already-committed duplicates, the event write
then throws `EntityConflictError`, but the hook entity had
already been overwritten with the retry's payload — leaving the
durable hook entity and the event log inconsistent (e.g. the
entity reflects the retry's metadata while the event still
carries the original).
karthikscale3 reproduced this on the prior head by creating
`hook_created` with metadata `{ v: "a" }`, then retrying the
same `(runId, hookId, token)` with metadata `{ v: "b" }` and
`isWebhook: false`: the retry threw `EntityConflictError` but
`hooks.get()` returned the retry's payload.
Fix: defer the hook entity write until AFTER the outer
`writeExclusive(eventPath)` commits. The branch now only
captures the entity-to-write and its overwrite options; the
actual write happens immediately after the event publish in the
shared trailing block. A retry that ends in
`EntityConflictError` (the event was already published) now
leaves the entity untouched.
The first-writer happy path and all recovery paths (orphaned-
claim, orphaned-entity, cross-worker convergence, legacy claim,
token-reuse across lifetimes) are unaffected — they all reach
the event publish successfully, then the entity write runs as
before.
Regression test `does not mutate an already-committed hook
entity when a duplicate hook_created retry collides` added to
world-local: runs karthikscale3's exact scenario and asserts the
persisted entity still carries the original metadata and
isWebhook. Verified to fail on the prior commit (persisted
metadata = 0xbb instead of 0xaa) and pass on this commit after
rebuilding.
Parallel guard test `does not mutate an already-committed hook
entity when a duplicate hook_created retry collides` added to
world-postgres. Postgres already protected this via
`onConflictDoNothing()` on the hook INSERT, but the test guards
against a future regression that adds an UPDATE/UPSERT to the
dedup path.
* refactor(world-local): per-instance in-process locks; drop tsx subprocess test plumbing
You were right that the tsx subprocess machinery was overkill for a
storage-level convergence test. Replaced with a simple two-instance
in-process test that exercises the same cross-process semantics
without spawning anything.
The trick: `stepLocks` and `hookLocks` were module-level Maps shared
by all `createEventsStorage` calls in the same process. Move them
inside the function so each `createStorage(dir)` call gets its own
lock map. Two storage instances sharing one data directory then
behave exactly like two separate OS processes:
- independent in-process `hookLocks` Maps (no in-process
serialization between them), and
- a shared filesystem (so the on-disk `writeExclusive` claim /
marker / event publish primitives are the only thing arbitrating
convergence).
This is also a real architectural improvement — the global lock map
was always a leaky abstraction that made unit-test simulation of
the cross-process path awkward.
Changes:
- `stepLocks` and `hookLocks` moved from module scope into
`createEventsStorage`. `withStepLock` and `withHookLock` wrappers
collapsed into direct `withInProcessLock(map, key, fn)` calls at
the two call sites that need them.
- The three convergence regression tests in `storage.test.ts` now
use `const workerA = createStorage(testDir); const workerB =
createStorage(testDir);` and race `Promise.allSettled` of
`events.create` from both — no subprocess, no IPC, no barrier
helper, no `raceHookCreatedAcrossProcesses`. Same assertions
(exactly one fulfillment + N-1 `EntityConflictError` per race,
raw `events.list()` shows exactly one `hook_created` per
logical creation — no Map dedup) so the regression catches are
identical.
- Removed: `tsx` devDep, `test-fixtures/hook-race-worker.ts`,
`HOOK_RACE_WORKER` / `resolveTsxLoaderUrl` / `TSX_BIN` /
`raceHookCreatedAcrossProcesses` and the
`fork`/`fileURLToPath` imports they pulled in.
Verified (after rebuilding world-local):
- All 379 tests pass on macOS in ~1s (was ~6.7s with subprocesses).
- Convergence tests confirmed to still catch the bugs: temporarily
reverted the `eventId = canonicalEventId` adoption → both workers
fulfilled (2 events instead of 1). Temporarily reverted the
legacy-claim marker pin → same: both workers fulfilled.
- No subprocess machinery means no Windows-specific quirks
(cli.mjs shebang, .cmd wrappers, .bin hoisting under pnpm
isolated linking, etc.) that produced the Windows CI 60s
timeouts.
- World-postgres still has its own parallel guard test for the
karthikscale3 "no-mutate-on-duplicate" regression; that one
exercises real DB concurrency and is unaffected by this change.
Full repo `pnpm test` (43 packages) and the
`parallelStepsThenWebhookWorkflow` e2e test against world-local
both green.
* fix(world-local): repair event-first hook orphans from the persisted event; skip #1665 e2e on world-postgres
- A crash between the hook_created event publish and the deferred hook
entity write left the event committed with the entity missing and
unrepairable (retries threw EntityConflictError without materializing
the entity). Retries now rebuild the entity from the PERSISTED event's
payload — never the retry's eventData — via a race-safe writeExclusive,
on both the canonical-eventId collision path and the legacy-claim
probe path.
- Skip parallelStepsThenWebhookWorkflow e2e on world-postgres: the
same-tick replay pattern surfaces a separate pre-existing step_started
ordering bug there (#2331).
---------
Co-authored-by: Peter Wielander <peter.wielander@vercel.com>
github-actions[bot]