Skip to content

feat(kernel): add durable run cancellation - #142

Merged
kjgbot merged 1 commit into
mainfrom
feat/v2-run-cancel
Sep 3, 2026
Merged

feat(kernel): add durable run cancellation#142
kjgbot merged 1 commit into
mainfrom
feat/v2-run-cancel

Conversation

@kjgbot

@kjgbot kjgbot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add durable run.cancel.requested intent and terminal canceled fact
  • close active leases and waits, reject late completion, and make repeat cancellation idempotent
  • preserve cancellation across SIGKILL/resume and linearize cancellation against worker completion
  • expose cancellation through the real Unix socket, CLI, and typed SDK client

Verification

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.

Session-Id: 01a06343-9355-7393-95d7-d2fb2d972c73
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 55 minutes.

Check out review usage here.

View limit details

Limit 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.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: edc198a6-2ce7-440d-a5e6-7ae8b5dac770

📥 Commits

Reviewing files that changed from the base of the PR and between a0d42ff and 01eb0e3.

📒 Files selected for processing (25)
  • kernel/DESIGN.md
  • kernel/relayflowd-core/src/entry.rs
  • kernel/relayflowd-core/src/lib.rs
  • kernel/relayflowd-core/src/machine.rs
  • kernel/relayflowd-core/src/machine/cancel.rs
  • kernel/relayflowd-core/src/machine/recovery.rs
  • kernel/relayflowd-core/src/machine/tests.rs
  • kernel/relayflowd-core/src/state.rs
  • kernel/relayflowd/src/engine.rs
  • kernel/relayflowd/src/engine/remote.rs
  • kernel/relayflowd/src/lib.rs
  • kernel/relayflowd/src/main.rs
  • kernel/relayflowd/src/server.rs
  • kernel/relayflowd/src/server/cancel.rs
  • kernel/relayflowd/src/server/client.rs
  • kernel/relayflowd/src/server/session.rs
  • kernel/relayflowd/tests/crash_resume.rs
  • kernel/relayflowd/tests/crash_resume/concurrency.rs
  • ops/reviews/20260902-v2-run-cancel-evidence.md
  • sdk/src/index.ts
  • sdk/src/journal-client.ts
  • sdk/src/protocol.ts
  • sdk/tests/journal-client-loopback.ts
  • sdk/tests/journal-client.test.ts
  • sdk/tests/live-kernel.test.ts

Note

🎁 Summarized by CodeRabbit Free

Your 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 @coderabbitai help to get the list of available commands.

@kjgbot
kjgbot merged commit 3bcba0e into main Sep 3, 2026
2 checks passed
kjgbot pushed a commit that referenced this pull request Sep 3, 2026
…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, &params.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
kjgbot pushed a commit that referenced this pull request Sep 3, 2026
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
kjgbot added a commit that referenced this pull request Sep 3, 2026
* 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, &params.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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant