feat(kernel): add durable run cancellation - #142
Conversation
Session-Id: 01a06343-9355-7393-95d7-d2fb2d972c73
|
Warning Review limit reachedNext included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (25)
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Essentials by visiting https://app.coderabbit.ai/settings/billing. Comment |
…ease_conflict This edits a test that judges this branch's own behaviour, which AGENTS.md rails against. It is therefore its own commit, touching nothing else, and the taxonomy call was made by the lead (relayflow-lead-0903), not by this branch. Flagging it for the independent signoff to re-derive rather than inherit. The test is #142's own — "cancels over the real socket and rejects the lease holder after closure", added by main in feat(kernel): add durable run cancellation. It cancels a run over the socket, then has the lease holder complete the step, and asserted the refusal carried `lease_conflict`. What this branch changed is which of two refusals fires first, not whether the completion is refused. `step.complete` now runs the `ensure_mutable` admission gate before `completion_worker`: ensure_mutable(&engine, ¶ms.run_id)?; // -> run_terminal let worker_id = hub .completion_worker(connection_id, &key) .map_err(protocol_conflict)?; // -> lease_conflict Each code has exactly one producer in the tree (server/protocol.rs:58 and :46), so the ordering fully determines which is returned. Unchanged by this commit, and still asserted by the same test: the completion is refused, exactly one run.cancel.requested entry exists, and exactly one run.completed entry exists carrying completionReason "canceled". Only the error code moved. The lead's reasoning for preferring run_terminal: lease_conflict tells a worker "someone else holds your lease", which is false here -- nobody holds it, the run is over -- and it invites a retry that terminality does not. Checking "can this run accept mutations at all?" before "who holds this lease?" is also the correct precedence: the cheaper, more general, fail-closed question first. The rejected alternative was weakening ensure_mutable so lease_conflict still won. That trades a correct guard for a stale expectation. Mutation-verified, both directions, on the rebased tree at 512723c. RED (before this commit): $ ./node_modules/.bin/vitest run FAIL tests/live-kernel.test.ts > ... > cancels over the real socket and rejects the lease holder after closure AssertionError: expected JournalProtocolError: run_terminal: run 0... { code: '...' } to match object { code: 'lease_conflict' } - Object { - "code": "lease_conflict", + JournalProtocolError { + "code": "run_terminal", Tests 2 failed | 409 passed | 3 skipped (414) GREEN (after): $ ./node_modules/.bin/vitest run Tests 1 failed | 410 passed | 3 skipped (414) The one remaining failure is the pre-existing `JournalClient wire conformance` failure, which is independent: it survives moving ensure_mutable after completion_worker, whereas this test does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
PROTOCOL CHANGE. This edits main's test to match a deliberate behaviour change
in this branch, so it is its own commit, touching nothing else. The call was the
lead's (relayflow-lead-0903), not this branch's. The signoff should re-derive it
rather than inherit it.
This is main's test -- "JournalClient wire conformance against live relayflowd".
It started a deterministic run, which completes immediately, then called
event.emit and stream.append on that terminal run and asserted both SUCCEED:
expect((await client.eventEmit(run_id, 'unmatched', {ok:true})).matched).toBe(0);
expect((await client.streamAppend(run_id, 'results', {answer:4})).offset).toBe(0);
bdd598c admits every mutating verb through `ensure_mutable`, so this branch
refuses both with run_terminal. Unlike the step.complete change in 269fcc6, this
is not an error-code taxonomy move: observable behaviour on a shipped verb pair
goes from accepted to refused.
It does not remove a working capability. It removes a way to corrupt a journal
that main reports as success. On main, stream.append against a terminal run
journals stream.appended AFTER run.completed, and this branch's state.rs fold
guard rejects exactly that -- so main produces journals the daemon cannot fold
on resume. Same data directory, both binaries:
MAIN resume -> exit=0 {"status":"completed","completion_reason":"success"}
BRANCH resume -> exit=1 Error: fold run journal
Caused by: journal entry 5 appears after terminal run.completed
The realistic shape is worse than that synthetic one, and shows main is already
self-inconsistent. A worker holds an llm lease; the run is cancelled out from
under it; the worker then does what a live worker does:
late step.complete -> THROW [lease_conflict] <- main already refuses this
late stream.append -> OK {"offset":0} <- and corrupts the journal
late event.emit -> OK {"matched":0}
>>> entries AFTER terminal run.completed: ["stream.appended"]
Main already holds "a terminal run accepts no step completions"
(engine/remote.rs:44, from #142). This branch extends the same rule to the other
mutating verbs, which is what the words already meant.
Checked before changing anything, rather than assuming: no product code calls
either verb (only the JournalClient method definitions); no doc sanctions a
post-terminal append -- kernel/DESIGN.md:384-385 says event.emit "satisfies
wait.event", which a terminal run has none of, and stream.append "journals
stream.appended", which is the corruption; and the one in-repo live use
(crash_resume/llm.rs) is against a parked run and is unaffected.
The test keeps full wire coverage of both verbs. Their success paths move to the
parked llm run, which is the only state in which appending to a run's journal is
meaningful, and the terminal case now asserts the refusal -- matching
crash_resume/protocol_admission.rs, which is left alone because it pins the
invariant correctly.
$ ./node_modules/.bin/tsc -p tsconfig.tests.json
(clean)
$ RELAYFLOWD_BIN=<this worktree's own build> ./node_modules/.bin/vitest run
Test Files 23 passed | 1 skipped (24)
Tests 411 passed | 3 skipped (414)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* kernel: dispatch runnable steps in parallel Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): drive complete parallel dispatch batches Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): preserve parallel assignment lifecycle Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): close parallel dispatch admission gaps Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): reject forged completion pins Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): canonicalize workspace surfaces across kernel/SDK/socket Second P1 in PR #137 review: `/mount/repo` and `/mount/./repo` were admitted concurrently. `parallel::SurfaceIdentity::Opaque("workspace:...")` compared the raw string; workspace surfaces bypassed the canonical path identity used for external surfaces, letting alias forms conflict-check as disjoint. Repair: - `SurfaceIdentity::External` → `SurfaceIdentity::Path{kind, namespace, components}` with `PathSurfaceKind::{Workspace,External}`. Workspace and external surfaces now share the canonical path identity but do not cross-collide. - Rename `spec::external_surface_identity` → `spec::path_surface_identity`; callers of the workspace surface use the same canonicalizer. - SDK `isCanonicalExternalSurface` → `isCanonicalPathSurface`; workspace entries now reject empty/./.. components with the same error class as external surfaces. - New `kernel/relayflowd/tests/crash_resume/workspace_identity.rs` covers alias refusal and canonical subtree serialization over a real socket. - 33 new SDK validate cases pin workspace canonicalization; 9 kernel parallel_tests cases (ancestor/descendant/sibling for both surface kinds) still green. Evidence: $ cargo test -p relayflowd-core --lib machine::parallel test result: ok. 9 passed; 0 failed; ... $ cargo test -p relayflowd --test crash_resume workspace_aliases test result: ok. 1 passed; 0 failed; ... finished in 0.89s $ ./node_modules/.bin/vitest run tests/validate.test.ts Test Files 1 passed (1) Tests 48 passed (48) Session-Id: 6cae47a0-1263-4c8b-bfaa-bd5ffc72e08e Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): preserve terminal-slash surface compatibility Session-Id: 01a0667b-bd7e-73c1-8e14-e3e9d13d136e Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): one spelling per surface, refusing the terminal slash Reverts 83db98b's accept-and-normalize and restores 53bfee0's strict rule for BOTH workspace and external surfaces. An independent signoff at 83db98b found a P0: exactly-once effects can double-fire. 83db98b widened the *external* accept set the same way it widened workspace -- its own test diff moved "/provider/item/" from reject to accept -- but added `workspace_surfaces_equal` only. The exactly-once ledger key is a raw SQL string: PRIMARY KEY (step_id, idempotency_key, surface_path) relayflowd-journal/src/lib.rs:48, append.rs:162 `idempotency_key = sha256(run_id || step_id)` (machine.rs:396) and `step_id` are both constant across attempts, so `surface_path` is the only variable in that key -- and it had two legal spellings. Executed against the real SqliteJournal at 83db98b: attempt1 '/provider/item' deduped = false attempt2 '/provider/item/' deduped = false effect_count = 2 ; confirmed_effect_count = 1 `deduped=false` means "you owe the provider call" (engine/effects.rs:17-24), so one logical effect fires twice. At the parent commit it failed closed at effects.rs:127. Accept-and-normalize only holds if EVERY identity comparison routes through the same normalization. 83db98b reached fifteen workspace comparison sites and got all fifteen right; it missed the sixteenth, which happens to be the one guarding exactly-once. Uniform reject needs no such completeness: one surface has exactly one spelling, and a non-canonical one never enters the system. Two facts make the strict rule the house rule rather than a new constraint: testdata/hello-agent.flow.yaml already authored `surface: repo`, so the ladder fixture was the outlier; and 53bfee0's own contract test `workspace_mounts_and_worktrees_must_have_one_canonical_spelling` already asserted `/mount/repo/` is refused, which accept-and-normalize contradicted. RED (before this change, with the tests restored to the strict contract): $ cargo test -p relayflowd-core --lib spec::tests::external_surface ---- spec::tests::external_surface_paths_must_have_one_canonical_spelling stdout ---- panicked at relayflowd-core/src/spec/tests.rs:146:9: accepted non-canonical surface "/provider/item/" test result: FAILED. 0 passed; 1 failed GREEN: $ cargo test --workspace 22 + 31 + 1 + 1 + 4 + 3 + 37 + 5 + 18 passed; 0 failed $ ./node_modules/.bin/vitest run 259 passed (260); 1 pre-existing live-kernel failure, unchanged Fixtures: testdata/hello-ladder.flow.yaml authors `surface: repo`, and its canonical JSON and sha256 are regenerated through the SDK compiler rather than hand-edited. The canonical diff is one character; the hash moves ecccd7b2..de095a29 -> 57cac294..f6d57944, and spec_parity confirms kernel and SDK still agree byte-for-byte. `workspace_surfaces_equal` is kept across its 18 call sites. Under a single spelling it is equivalent to string equality for valid surfaces, but it still compares parsed identities and so fails closed when either side does not parse -- defense in depth at the pin/declaration seam for exactly the bug class above. Removing it would be an 18-site change for no safety gain. A non-canonical spelling is no longer a surface at all, so `external_surface_contains("/provider/item/", "/provider/item")` now fails closed rather than resolving to the canonical form, and the two ancestor/descendant conflict cases that exercised the terminal slash are dropped as unreachable states rather than restated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * test(sdk): a late completion after cancel reports run_terminal, not lease_conflict This edits a test that judges this branch's own behaviour, which AGENTS.md rails against. It is therefore its own commit, touching nothing else, and the taxonomy call was made by the lead (relayflow-lead-0903), not by this branch. Flagging it for the independent signoff to re-derive rather than inherit. The test is #142's own — "cancels over the real socket and rejects the lease holder after closure", added by main in feat(kernel): add durable run cancellation. It cancels a run over the socket, then has the lease holder complete the step, and asserted the refusal carried `lease_conflict`. What this branch changed is which of two refusals fires first, not whether the completion is refused. `step.complete` now runs the `ensure_mutable` admission gate before `completion_worker`: ensure_mutable(&engine, ¶ms.run_id)?; // -> run_terminal let worker_id = hub .completion_worker(connection_id, &key) .map_err(protocol_conflict)?; // -> lease_conflict Each code has exactly one producer in the tree (server/protocol.rs:58 and :46), so the ordering fully determines which is returned. Unchanged by this commit, and still asserted by the same test: the completion is refused, exactly one run.cancel.requested entry exists, and exactly one run.completed entry exists carrying completionReason "canceled". Only the error code moved. The lead's reasoning for preferring run_terminal: lease_conflict tells a worker "someone else holds your lease", which is false here -- nobody holds it, the run is over -- and it invites a retry that terminality does not. Checking "can this run accept mutations at all?" before "who holds this lease?" is also the correct precedence: the cheaper, more general, fail-closed question first. The rejected alternative was weakening ensure_mutable so lease_conflict still won. That trades a correct guard for a stale expectation. Mutation-verified, both directions, on the rebased tree at 512723c. RED (before this commit): $ ./node_modules/.bin/vitest run FAIL tests/live-kernel.test.ts > ... > cancels over the real socket and rejects the lease holder after closure AssertionError: expected JournalProtocolError: run_terminal: run 0... { code: '...' } to match object { code: 'lease_conflict' } - Object { - "code": "lease_conflict", + JournalProtocolError { + "code": "run_terminal", Tests 2 failed | 409 passed | 3 skipped (414) GREEN (after): $ ./node_modules/.bin/vitest run Tests 1 failed | 410 passed | 3 skipped (414) The one remaining failure is the pre-existing `JournalClient wire conformance` failure, which is independent: it survives moving ensure_mutable after completion_worker, whereas this test does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * docs(reviews): record the PR #137 rebase onto 512723c Supersedes an unmerged first pass of this report that targeted 990093b. Every command is pinned to a literal SHA rather than the origin/main ref, which moved twice during the task. This rebase produced ZERO conflicts, which is the risk rather than the result: on #139's rebase a line that reverted a lowering auto-merged silently. Every hunk was therefore audited by reading. 512723c adds #138, which touches four files this branch also edits (spec.rs, spec/tests.rs, validate.ts, validate.test.ts) and, critically, moves timeoutMs to deterministic-only in TWO independent places: the step-fields allowlist and compileStep's base spread. Getting one right and missing the other yields a spec that validates but lowers wrong, and validateSpec cannot see it. Both halves are byte-identical to 512723c and both were re-proved behaviourally through compileYaml + toKernelSpec: a deterministic step lowers to timeout_ms, llm and agent are refused at the allowlist. #136's `output` line survives in both verb lists. Artifact survival, both directions. All 15 of #138's blobs hashed before and after: 11 identical including compile.ts and step-fields.ts; the 4 that moved are the 4 this branch edits and each is a pure addition. Every line of #138 content absent afterwards was enumerated: a first pass with plain diff reported 14, of which 7 were false positives from re-indentation and one rustfmt attribute rewrap; whitespace-insensitively 7 remain, all attributed and none authored by #138. In the other direction, a whole-tree set-diff of the branch's own change set before against after reports exactly three deltas across 41 files, the same three deliberate resolutions as the first pass and nothing else. The branch's own gate is proved where it lives rather than where it is convenient: a canonical spec compiled through the SDK, its lowered kernel spec then mutated and submitted over a real socket with the SDK out of the path. The kernel refuses all five non-canonical forms across both surface kinds, and accepts the canonical control. Gates: tsc --noEmit, tsc -p tsconfig.type-tests.json (a gate #138 added that the brief's list predates), and tsc -p tsconfig.tests.json all pass; cargo test --workspace is 130 passed, 0 failed; vitest is 410 passed with one failure, the pre-existing wire-conformance one. Rust test names set-difference to exactly the union of both parents, 130 executed against 130 expected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * docs(reviews): correct the wire-conformance attribution — it is this branch's An earlier version of this report called the vitest wire-conformance failure "pre-existing". That was wrong. It was observed at 83db98b, which is #137's own head and already carries bdd598c; "pre-existing at the branch parent" is not "pre-existing on main", and I repeated the stronger claim without testing it. Determined empirically instead. A throwaway worktree at 512723c with its own kernel build and RELAYFLOWD_BIN pinned to it runs tests/live-kernel.test.ts at 21 passed (21), wire conformance included. The same file fails on this branch. The failure is the branch's. It is also not an error-code taxonomy change, which is why no assertion was touched. bdd598c gates six verbs with ensure_mutable, two of them event.emit and stream.append. The wire-conformance test starts a deterministic run, which completes immediately, then calls both on it and asserts they SUCCEED (matched === 0, offset === 0). Main accepts them; this branch refuses them. The observable protocol behaviour changed from accepted to refused. The refusal is load-bearing rather than gratuitous, and the obvious fix is the wrong one. On main, stream.append against a terminal run writes a stream.appended entry after run.completed. The branch's state.rs fold guard rejects exactly that, so a journal main produces is one this branch cannot load: the same data directory resumes clean under the main binary and fails under the branch binary with "journal entry 5 appears after terminal run.completed". Relaxing ensure_mutable to turn the test green would let the daemon write journals it then cannot resume, which is a durability bug and worse than a red test. The rebase also surfaced a direct contradiction: the branch's own protocol_admission test asserts stream.append and event.emit return run_terminal and leave the journal unchanged, while main's wire-conformance test asserts they succeed. Both are in the tree, both were written deliberately, and they cannot both pass. Because protocol_admission pins the error code, an accept-and-ignore compromise would require editing that gate too. Three options are laid out in the report. None is implemented; the branch is unchanged and the push is on hold pending the lead's call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * test(sdk): a terminal run refuses event.emit and stream.append PROTOCOL CHANGE. This edits main's test to match a deliberate behaviour change in this branch, so it is its own commit, touching nothing else. The call was the lead's (relayflow-lead-0903), not this branch's. The signoff should re-derive it rather than inherit it. This is main's test -- "JournalClient wire conformance against live relayflowd". It started a deterministic run, which completes immediately, then called event.emit and stream.append on that terminal run and asserted both SUCCEED: expect((await client.eventEmit(run_id, 'unmatched', {ok:true})).matched).toBe(0); expect((await client.streamAppend(run_id, 'results', {answer:4})).offset).toBe(0); bdd598c admits every mutating verb through `ensure_mutable`, so this branch refuses both with run_terminal. Unlike the step.complete change in 269fcc6, this is not an error-code taxonomy move: observable behaviour on a shipped verb pair goes from accepted to refused. It does not remove a working capability. It removes a way to corrupt a journal that main reports as success. On main, stream.append against a terminal run journals stream.appended AFTER run.completed, and this branch's state.rs fold guard rejects exactly that -- so main produces journals the daemon cannot fold on resume. Same data directory, both binaries: MAIN resume -> exit=0 {"status":"completed","completion_reason":"success"} BRANCH resume -> exit=1 Error: fold run journal Caused by: journal entry 5 appears after terminal run.completed The realistic shape is worse than that synthetic one, and shows main is already self-inconsistent. A worker holds an llm lease; the run is cancelled out from under it; the worker then does what a live worker does: late step.complete -> THROW [lease_conflict] <- main already refuses this late stream.append -> OK {"offset":0} <- and corrupts the journal late event.emit -> OK {"matched":0} >>> entries AFTER terminal run.completed: ["stream.appended"] Main already holds "a terminal run accepts no step completions" (engine/remote.rs:44, from #142). This branch extends the same rule to the other mutating verbs, which is what the words already meant. Checked before changing anything, rather than assuming: no product code calls either verb (only the JournalClient method definitions); no doc sanctions a post-terminal append -- kernel/DESIGN.md:384-385 says event.emit "satisfies wait.event", which a terminal run has none of, and stream.append "journals stream.appended", which is the corruption; and the one in-repo live use (crash_resume/llm.rs) is against a parked run and is unaffected. The test keeps full wire coverage of both verbs. Their success paths move to the parked llm run, which is the only state in which appending to a run's journal is meaningful, and the terminal case now asserts the refusal -- matching crash_resume/protocol_admission.rs, which is left alone because it pins the invariant correctly. $ ./node_modules/.bin/tsc -p tsconfig.tests.json (clean) $ RELAYFLOWD_BIN=<this worktree's own build> ./node_modules/.bin/vitest run Test Files 23 passed | 1 skipped (24) Tests 411 passed | 3 skipped (414) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * docs(reviews): record the option-1 resolution and the assumption test behind it The lead chose option 1 and asked that the assumption behind it be tested rather than inherited: is there any legitimate late stream.append or event.emit on a terminal run, out-of-band async completion in particular. Checked four ways and the assumption holds. No product code calls either verb — only the JournalClient definitions, a loopback mock, and tests. No doc sanctions a post-terminal append: DESIGN.md says event.emit satisfies a wait, which a terminal run has none of, and stream.append journals stream.appended, which is the corruption itself. The one live in-repo use appends against a parked run and is unaffected. The async shape was executed rather than reasoned about, and it cuts against main: a worker holding an llm lease on a run cancelled out from under it has its late step.complete refused with lease_conflict while its late stream.append is accepted in the same moment, journalling after run.completed and producing a journal the branch cannot fold. Main already holds the rule for step completions at engine/remote.rs:44; this branch extends it to the remaining mutating verbs. Adds the PR-body Protocol change section, and records a CI finding: the two contradicting tests both passed because neither runs in flows CI. cargo test never runs at all, and CI's vitest names four files, of which live-kernel.test.ts is not one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * docs(reviews): lead the Protocol change section with main's self-inconsistency The strongest argument for the change is not the resume demonstration, it is that main already holds the rule and fails to apply it uniformly: a worker whose run was cancelled has its late step.complete refused with lease_conflict and its late stream.append accepted, in the same moment. This PR extends main's own rule rather than imposing a new one. Reordered so a reviewer meets that first, with the resume corruption as the reason it matters and DESIGN.md:384-385 as corroboration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 --------- Co-authored-by: kjgbot <kjgbot@agentrelay.dev>
Summary
run.cancel.requestedintent and terminal canceled factVerification
Literal commands and captured output, including the mutation failure/pass pair and the disclosed parallel-suite timeout, are committed in the evidence report.
Final gates include Rust workspace clippy/tests, the focused cancellation crash/race tests, typed full-stack socket cancellation, and the 239-test SDK suite with serial file execution.