fix(core): pre-check deployment affinity before the lazy resume write - #3374
Conversation
The lazy hook fast path (#3345) hoisted the consumer's hook_received write above the deployment-affinity guard (#2960), so a misrouted lazy resume wrote its event before the guard could re-route the delivery. Stamp the run's pinned deployment on the resume message (hookInput.deploymentId, from the producer's resume context) and, on the consumer, compare it against the ambient deployment id immediately before the fast path: a match continues with no run fetch, a mismatch fetches the authoritative run and hands it to the existing guard — which keeps sole ownership of re-route/fail policy and remains the authoritative protection before replay and step execution. The re-routed message preserves the complete hookInput (it may hold the only copy of the resume payload). Older messages without the field, and worlds without deployment affinity, are unchanged: they skip the pre-check and rely on the authoritative guard, the pre-guard write staying convergent per (runId, resumeId). Fixes the misrouted-lazy-resume unit test broken by the #2960/#3345 ordering: a modern misrouted resume now re-routes with zero event writes, asserted for both hook_received and run_started. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 3a6dc5b The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
TooTallNate
left a comment
There was a problem hiding this comment.
Reviewed at 3a6dc5b (based on current main, 0 behind). This is exactly the right resolution to the #2960 ↔ #3345 interaction — the "carry the pin on the message" option, implemented with the guard kept authoritative.
Verified locally:
- The red test on main (
re-routes a misrouted lazy hook resume with its payload intact) passes on this branch, and the full core suite is green: 90 files, 1945 passed / 3 expected fail. - Provenance of the stamp is correct — this was my main worry.
hookInput.deploymentIdcomes from the hook's resume context, which derives fromrun.deploymentId(the immutable pin), not the producer's ambient id — it's the same field the producer already trusts for encryption-key derivation (getEncryptionKeyForRun(runId, { deploymentId })). Since a run's pin never changes, the stamp can't go stale. - The pre-check is advisory-only and can't make a wrong decision on its own: a mismatch only pays one
runs.getand hands the authoritative run to the existingguardDeploymentAffinity, which re-verifies againstrun.deploymentId— so even a hypothetically wrong stamp costs a fetch, never a false re-route. A match,getDeploymentId()failure, or an older message all fall through to the post-setup guard, which remains the protection before any replay or step execution. - Correctly capability-gated: without
deploymentAffinity, version-tagged ids (dpl_local@<version>) would make every local resume pay a spurious run fetch — the gate mirrors the guard's own eligibility. And the test assertingrunsGetis never called on a correctly routed modern message pins down the zero-cost claim. - The re-enqueue payload spreads the complete
hookInput(stamp included), so the re-routed message re-checks for free on arrival at the pinned deployment, and the payload — potentially the only copy — survives the hop. - The rewritten comment block on the flow-replay guard does what I asked on #3345: the "writes nothing" invariant is now scoped deliberately (restored for modern messages, consciously waived for older ones whose idempotent
(runId, resumeId)write stays convergent) instead of being silently false. - Single producer publish site, and it's the one stamped ✓. Changeset:
@workflow/worldminor for the schema field — thank you for applying the convention unprompted.
CI: the workbench-python-workflow deployment failure appears on every currently-open PR (3368/3374/3375) — baseline infra breakage, not this change.
Fast turnaround on a main breakage, with the cheap path staying free and the authoritative path staying authoritative. Approving.
What
Lazy hook resume messages now carry the run's pinned deployment (
hookInput.deploymentId, stamped by the producer from its resume context). Before the consumer's hoistedhook_receivedwrite, the runtime compares it against the ambient deployment id: a match continues with no run fetch; a mismatch fetches the authoritative run and hands it to the existing deployment-affinity guard, re-enqueuing with the completehookInput(payload included). The field is optional — older messages skip the pre-check and rely on the authoritative guard as before.Why
#3345 hoisted the consumer's
hook_receivedwrite above #2960's deployment-affinity guard, so a misrouted lazy resume wrote its event before the guard could re-route (the misrouted-lazy-resume unit test fails on main). With the pre-check, a misrouted modern resume re-routes with zero event writes, while correctly routed resumes pay no extra fetch or latency.🤖 Generated with Claude Code