Repository navigation
fix(sdk): refuse a reordered serial resume instead of re-running a completed step - #685
Merged
Merged
Conversation
…mpleted step Serial step ids are <kind>-<N> from one counter assigned in call order, which inside Promise.all is result-arrival order. A resume replays completed results in a different order, so branches can take each other's ordinals. A same-kind swap with a different spec is already refused by the kernel (run_admission_conflict). A kind swap is not: run-3 and llm-3 are different admission keys, so a resumed f.run that lands on a completed f.llm's ordinal ran again and the resume exited 0. An f.human swap applied a recorded answer to a different question. On a re-dispatched attempt the root driver now hands the body the root's journaled step index. A serial call whose ordinal is journaled under a different kind, or an f.human whose wait id was asked a different question or recipient, is refused as serial_replay_order_mismatch before anything is admitted. The refusal is latched against author catch. Read-only: no step id changes, no new journal writes, first attempts read nothing, parallel roots are untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
[High impact] The PR appears safe to merge; no new actionable issue was found.
|
A step declared before a serial_replay_order_mismatch and awaited after it still started: each operation's start check only asked whether done() had been called. Every operation now re-checks the refusal latch when it starts, and the refusal closes queued worker admissions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nals; share the replay latch Adversarial review of the serial replay guard: - A parked f.human holds its ordinal before its answer is lowered into the step index. The root driver now adds the root's wait.human ids to the snapshot a re-dispatched body is checked against. - An unfinished f.dispatch holds its ordinal from its child's first step. Any dispatch-N--* record now claims ordinal N for dispatch. - Helper, MCP and plugin effects now index their completion on the root like every other child, so their ordinals are visible to a resume. - The refusal latch is shared by the root and its dispatched children, so a child still running stops admitting, and f.on and f.memory check it before acting. - Drop the queued-slot stops added in the previous commit: in serial mode the deterministic runner has no queue, and the body-failure path already closes worker slots. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ex failed effects Final verification of the serial replay guard: - A replayed f.hook verdict restores the counter to the watermark it recorded, which also counts ordinals other branches took while the hook ran. Jumping past one handed that branch's completed step a fresh ordinal and re-ran it. A restore that skips a journaled ordinal this attempt has not claimed is now refused as serial_replay_order_mismatch. - MCP and plugin calls that fail with a diagnostic are indexed on the root like successful ones, so a caught failure's ordinal is guarded. - Tests for helper and plugin indexing, which no suite covered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dependent
CI (linux-x64) failed three of the new live tests:
- The MCP call log was `${TMPDIR ?? '.'}/...`: with TMPDIR unset it was a
relative path the mock server resolved against its own cwd. It now
lives at an absolute path under the fixture root.
- The parked-human case could park before the slow branch wrote its
effect. The parking branch now waits for that effect, so the journal
holds both before the run stops.
- Teardown removed the fixture while a daemon was still flushing
(ENOTEMPTY). Each fixture now SIGKILLs its daemon and waits for it to
exit (a reaped or zombie process) before the directory is removed.
The original order no longer rests on a 1.5s model delay either: a
"slow" answer is held until the other branch has taken its next ordinal
and signalled `go`, so a loaded host cannot invert it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A resumed serial authored run could re-execute a completed step and still exit 0. This PR adds a fail-closed guard. It does not change any step id. Everything is in the SDK.
Mechanism
Serial step ids are
<kind>-<N>. One ordinal counter is shared by every kind and assigned in call order (authored-flow-executor.tsnextId). InsidePromise.all, call order follows result-arrival order: each branch's next call is made when its previous step resolves. On resume, completedf.llm/f.agentresults replay from the journal almost instantly. If the original run's results arrived in a different order (model latency), the branches take each other's ordinals.run-3re-requested with the other branch's command): the kernel's admission key is bound to the spec hash, so the resume fails withrun_admission_conflict. That is fail-closed but spurious. It is the intermittent failure inresumes 'llm' branches without replaying completed effects.run-3andllm-3hash to different admission keys. A resumedf.runthat lands on the ordinal a completedf.llmholds gets a fresh key and runs again. Repro onmain: effectsLbecameLLZ, the model was called 4 times instead of 3, andflows resumeexited 0 withcompletionReason: success. The result verifier only checks that ordinals are contiguous, and a swap keeps them contiguous.f.humanswap: the answer is keyed by wait id alone, and the lowered spec embeds the answer, not the question. A reordered resume handedhuman-3's recorded "yes" (given to "Ship the right release?") to "Delete the left bucket?", then re-asked the right question ashuman-4.The fix
dispatch.attempt > 1), the root driver (authored-root.ts) reads the root'sauthored-stepsindex and itswait.humanids once. It hands that snapshot to the body; dispatched children receive it too.authored-serial-ordinals.ts):<kind>-<N>step;f.human, from itswait.humanid;f.dispatch, from anydispatch-N--*record of its child.complete-Nmarker. Anf.humanis refused when its journaled wait asked a different question or recipient.authored-helper-effect.ts,authored-mcp.ts,authored-plugin-effect.ts). Their ordinals are visible. These are the samerelayflows.authored-step.v1records other steps already write. Flows that use these effects therefore get one more index record per call on a first run. Otherwise the guard only reads.f.hookverdict restores the counter to theafterStepit recorded. That watermark also counts ordinals other branches took while the hook ran, so jumping past one gave that branch's completed step a fresh ordinal and re-ran it. A restore that would skip a journaled ordinal this attempt hasn't claimed is now refused, naming the hook, the ordinal and the journaled step.f.onandf.memorybefore they act. So an authorcatchcannot get anything else admitted: not a step saved before the refusal and awaited after it, not a child flow that is still running, not an activity, not a memory write.serial_replay_order_mismatch, exit 1. The message names the ordinal, the journaled step and kind, and the attempted kind, e.g.ordinal 3 is journaled as llm step "llm-3", but this resume reached it with run.The guard does not make a reordered resume succeed. It now fails closed: the root fails as
step_failed, like today'srun_admission_conflict, instead of re-running effects. Fix 2 makes it succeed.Pre-existing caveats, also on
main:hook-Nmarker (run_admission_conflict, measured on unmodifiedmain). Resuming past a hook that ran its own steps therefore already fails. The watermark guard now refuses that skip itself, a change of refusal kind that doesn't affect any working path. This needs its own fix.unsettled_derived_workon 2.0.55 and onmain. I didn't reproduce it here.Changed questions are refused. An
f.humanwhose question or recipient text differs from the one it parked on is refused on every resume, for example a question built from the time of day. That is the same as anf.runwhose command changes between attempts. It is documented indocs/SURFACE.md.Gaps that remain for fix 2
f.run, helper, MCP and plugin steps are indexed only at completion. An ordinal whose only holder was one of these, still in flight at the crash, is invisible to the guard. In that window a different-kind call can take the ordinal before a later ordinal is refused.f.hookverdicts are keyed by step id plus hook name, not input. Two same-name hooks with different inputs can exchange verdicts. Closing that needs a hook-record format change.f.llm/f.agentthat started before the refusal and is still waiting for a worker slot (--agent-capacity) is admitted when the slot frees. Its own ordinal passed the guard. The body-failure path closes the worker slots once the body settles. I removed the eager stop added in 8ec5a9b because it had no test, and in serial mode the deterministic runner has no queue to stop.Impact
This affects hosted and self-hosted runs alike: both go through the same root driver. Exposed: serial roots, meaning any body with
f.llm,f.agent,f.dispatch,f.human, helpers,budget, imports and so on, that runs effects as continuations insidePromise.all. Resume after a lost sandbox is the worst case, because replays return instantly while the original run waited on the model. Not exposed: parallel-eligible roots (causal-frontier ids), and serial bodies made only off.run, whose children share one in-order daemon connection and so return in call order.Follow-up: fix 2, stable per-branch ids (identityVersion 3)
identityVersion: 3. Version 2 and legacy roots keepkind-Nplus this guard, so no journaled id ever changes.<kind>-after-<sha256(sorted causal predecessors)>-<ordinal within that frontier>. This is the parallel scheme generalized to every kind. The serial lifecycle already records each call's causal predecessors (today the index recordsafter: ["llm-1"]onrun-3). The ordinal counts calls declared against the same frontier, in synchronous declaration order, so result-arrival order never participates.kind-Nwith this guard. In those cases the nondeterminism is the flow's own, and it is refused rather than repaired.f.humanwaits and hook records to a digest of the question or input. Index steps at admission, not completion.Tests
All on Node v22.23.4. "Fails on revert" means: that one fix was reverted on its own, the CLI rebuilt, the test run and its failure recorded, then the file restored byte for byte (the diff sha matched before and after) and the test re-run green.
ok:true, effectsLLZok:trueok:truehuman-4llm_cli_unresolvedrun-1admittedLre-ran (effectsLL); question re-asked ashuman-4, exit 3Lre-ran (CLL); refused only at ordinal 4dispatch-1--run-1claims ordinal 1; prefix parsingLre-ran (LL); refused only at ordinal 4CDL)subscription.opensentrun-3run-2, hook-1, run-4, complete-5, re-runningbhook-1marker conflict, not by this guardThese pin existing behaviour and pass with or without the guard:
run_admission_conflict;Cross-version:
RELAYFLOWS_PREVIOUS_CLI). On a live A/B againstmain, measured at ae2bf8f (first run plus in-order resume, all journals normalized; the later commits add only reads to this llm+run flow), both builds produce the same two variants. The second variant is concurrent branches appending two index records in the opposite order, whichmainalso does (1 run in 20).RELAYFLOWS_PREVIOUS_CLI=<2.0.50 cli>over the parallel suites (authored-parallel-agents,-body,-eligibility,-llm,-run-live): 74 passed, 0 skipped. With 2.0.55 as the previous CLI, four of those gated cases fail by design, because they assert 2.0.50-only behaviour: an older CLI refusing a v3 root, and legacy root metadata withoutidentityVersion.Suites at 28b349b: the PR, parallel, lifecycle and authority files (
serial-replay-identity,authored-serial-ordinals,authored-human,f-memory,authored-effect-index,authored-plugin-effect,mcp-lifecycle,mcp,authored-hooks,authored-root,authored-step-index,human-live,authored-activity,authored-parallel-*,authored-flow-lifecycle-executor,daemon-lifecycle,daemon-lifecycle-live,authored-node-authority,authored-surface-authority): 23 files, 383 passed, 8 skipped (theRELAYFLOWS_PREVIOUS_CLI-gated cases). The one failure in that run was the live hook test making the fixture manifest's network preflight call toapi.github.com; it now runs offline and passes 3/3.npm run typecheckandtypecheck:testsare clean.Earlier suites (at 3c95275):
serial-replay-identity,authored-serial-ordinals,authored-human,f-memory: green.authored-flow-lifecycle-executor,daemon-lifecycle,mcp-lifecycle) and authority (authored-node-authority,authored-surface-authority) suites, plusauthored-root: green.npm test(full SDK gate: 4777 tests): 39 failed. 36 of them fail identically on unmodifiedmainon this macOS host:hosted-base-snapshot,hosted-extension-isolation("requires Linux"),hosted-extension-protocol,event-await-cli,det-withhold-live,babysitter-native-extension. The other three (authored-rootsource-authority,wrapper-exit-drainlease abort,live-kerneltwenty-six-step) timed out while other jobs had the host at a load average of 48–66. Re-run in isolation on this branch, those three files pass 93/93, twice.npm run typecheck,npm run typecheck:tests: clean.No gate files are edited.
🤖 Generated with Claude Code
Agent Relay sessions
claudesessionaf1462f7· opened viagh pr create· last active 2026-10-26claudesession28cd20e0· contributor · rangh prcommands · last active 2026-10-26