Repository navigation
fix(stop): a stop at a tool boundary stops the turn - #382
Conversation
Once a round's last tool result is in, the agent reads `ready` until the
boundary resumes its stream, and in between the boundary awaits
buildStoredToolResultContent, which writes a result over the inline cap
to the workspace, in the mount's turn. A stop in that window didn't stop:
- Agent.cancelStream(), which every connectome-host stop calls, cancelled
only from `streaming` and `waiting_for_tools`. At `ready` it set the
agent idle and left the stream running, and after its write the
boundary resumed the stream and the turn went on.
- stop() cancelled only agents at those two statuses, so the turn resumed
and completed during shutdown.
- The framework's abortInference() returned false at `ready`, and the
turn resumed.
- A boundary over budget restarted the turn after the stop, a quiesce's
abandon included, and a budget restart runs while the host is quiesced.
The budget restart's own cancelStream() had met this already: its comment
recorded the parked stream, and it cancelled the live stream itself.
The change:
- cancelStream(), Agent.abortInference() and stop()'s cancel loop read
the stream from whichever state holds one ('stream' in state), as
abandonActiveTurns already does, instead of listing statuses.
- After its await, the boundary goes on only if the agent's state is
still the `ready` object it began from. Every stop replaces it:
cancelStream() sets the agent idle, a quiesce's abandon has driveStream
reset it, and an ephemeral run's disposal cancels before it
deregisters. Otherwise the boundary logs, emits one tool:result_dropped
for the result it was handling, as the drop after the batch hook does,
and returns, without resuming, restarting, settling an endTurn or
draining deferred writes.
- The round's tool_use row is stored after that check, with its results,
rather than before the await. A stopped round is then not stored at
all, as when a stop comes while its tools run, and the window never
holds a tool_use without its result. Storing the results after a stop
would race driveStream's teardown, whose deferred flush can land
messages between the two. A result that spilled keeps its workspace
file, and its images stay in the agent's image ledger until the ledger
evicts them.
- The budget restart's workaround becomes agent.cancelStream() after its
mark.
tool-result-guard's "shutdown retries edits queued by its terminating
stream" began stop() inside the submission of the round's results, while
the agent read `ready`, so it relied on stop() passing over that agent:
the stream went on to complete and accept the batch while stop() waited.
It now begins stop() on the turn's inference:completed, after the accept
has queued its failed edit. That keeps what it tests: it passes on main
and with this change, and fails without shutdown's retry.
Tests: test/stop-at-tool-boundary.test.ts holds the round's spill write
open and stops the turn with a host's cancelStream(), connectome-host's
stopRun sequence, stop(), abortInference(), a quiesce's abandon at a
boundary over budget, and an ephemeral run's disposal. It also checks a
round nobody stops, and a budget restart cancelling the stream it
replaces. Against main's src, 7 of its 8 cases fail, each at its own
assertion; the restart case passes there as well.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
[Medium impact] The reviewed changes appear safe to merge; no new blocking issue was found. SummaryStops now cancel streams parked at tool boundaries. The boundary checks whether the turn stopped after preparing its results, before storing or resuming the round.
Reviews (2) · Last reviewed commit: "fix(stop): the boundary also drops a rou..." · Reviewed by Greptile |
Anarchid
left a comment
There was a problem hiding this comment.
Nightly dual review · round R1 at 19f6587 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: APPROVE (the formal review event is left to the maintainer). Finding ids R1-n are stable; cite them in fix commits.
Reviewed at 19f6587 against main e6a0317 (merge-base 91e70ed; main's later commits don't touch src/agent.ts or src/framework.ts)
Verdict
Approve. No blockers, no majors, four minors.
The fix does what it says. At a tool boundary, every stop now cancels the stream and replaces the agent's ready state before the boundary can resume. The boundary checks that state after its last await, and nothing is awaited between that check and the store or resume, so a stopped round can't be resumed, restarted or half-stored. The minors are a written-down invariant for the quiesce path, two prose corrections and one missing test. None of them needs to hold the merge.
No earlier review of ours exists on this PR, so this is round 1 and there are no prior findings to carry over.
Blockers
None.
Major
None.
Minor
R1-1. The boundary catches a quiesce's abandon only because driveStream wins a microtask race, and nothing writes that down.
abandonActiveTurns cancels the stream object and leaves the agent's ready state in place (src/framework.ts:4683-4689). The state is replaced only when driveStream's quiesce_abandoned case runs agent.reset() (:13364-13374). So the new check at :8850-8851 sees an abandon only if that reset lands before the boundary's continuation runs. Today it always does:
- membrane 0.5.87's
cancel()queuesabortedand wakes the iterator synchronously (src/yielding-stream.ts:179-195); - driveStream is parked on
next(), since neither its loop head (:12357-12360) nor itstool-callscase awaits anything; - the boundary resumes only when the spill's I/O completes.
I forced the worst ordering I could reach: abandonActiveTurns() called synchronously inside the spill's own completion, before the boundary's continuation. The round was still dropped, with and without a budget restart pending (one tool:result_dropped, no inference:stream_restarted, one request, nothing of the round stored). So quiesce's real callers, an API call or its drain timer, can't reach this.
It is still a hidden dependency. If someone later adds an await to driveStream's loop head or its tool-calls case, the quiesce case this PR closes opens again. The boundary would store the round. Over budget, it would overwrite quiesce_abandoned with budget_restart (:9079) and queue a restart through the quiesce. Otherwise, it would call provideToolResults on a done stream, which throws in the event loop.
Fix (either one):
- Make the check deterministic: also drop when
this.frameworkCancelledStreams.get(\${agent.name}:${agent.streamId}`) === 'quiesce_abandoned'. At that point it's the only marker that can exist, becauseturn_endedandbudget_restart` are set later by this same boundary. - Or have
abandonActiveTurnscallagent.cancelStream()after recording the marker, so the agent goes idle synchronously. Thequiesce_abandonedcase'sreset()then has nothing left to do. This changes the abandon for thestreamingandwaiting_for_toolsstates too, so it's the wider of the two changes.
At the least, say in the comment at :8836-8849 that the quiesce case depends on this ordering.
R1-2. The image-ledger sentence is wrong for two of the stops it covers.
The changelog (changelog.d/stop-at-tool-boundary.fixed.md:15-16) and the boundary comment (src/framework.ts:8846-8847) say a dropped round's images "stay in the agent's image ledger until the ledger evicts them". That holds for cancelStream(), abortInference() and a quiesce's abandon. It doesn't hold for the other two:
stop()clears every ledger before it cancels streams (:2722).- An ephemeral run's disposal deletes its agent's ledger (
:6141).
In both, the images go with the ledger at once. The boundary's retains (:14496-14511) land in a ledger object that is no longer in the map. That's harmless, but the sentence promises otherwise. Fix: qualify it, e.g. "…until the ledger evicts them, or until stop() or an ephemeral run's disposal discards the ledger."
R1-3. A dropped round loses more than its tool_use and results.
The changelog (:13-15) and the comment at :8842-8844 say "neither its tool_use nor its results reach the window". But the blocks held back are the round's whole assistant turn: its preamble text and thinking as well as its tool_use (:8867, which is what driveStream's tool-calls case puts into pendingAssistantBlocks). I stopped a round whose response was "Fetching." plus a fetch call. The window afterwards held the user's message and the turn's [inference-failed] note, and no "Fetching.", even though a host would have streamed that text to the user. This matches what a stop already does while the round's tools run. Fix: one clause in the changelog's "What a reader of the store sees", e.g. "the round's assistant blocks (its text, thinking and tool_use) and its results", so nobody reads the missing preamble as a bug.
R1-4. No test covers a round with more than one result.
The description and the changelog both say: "The round's earlier results go with its state, without events of their own." Every new test has a one-call round. I checked a two-call round, both calls spilling, both ways:
- Stopped with
cancelStream(): neither call's rows reach the window, and exactly onetool:result_droppedgoes out, naming the call whose result arrived last (call_fetch2). - Nobody stops: the window holds
text,tool_use:a,tool_use:b,tool_result:a,tool_result:b, adjacent.
Both outcomes are right. Fix: a test pinning them would keep that sentence true; holdAtToolBoundary needs only a second tool_use in the first scripted response.
Answers to the open questions
Is the stored round consistent after a stop at the boundary? Yes.
- A stopped round stores nothing: no tool_use, no results, no preamble.
- An unstopped round stores its tool_use row, then its results row. The calls between them (
agent.addAssistantResponseat:8871,toolResultGuard.storeResultsat:8877) are synchronous, and nothing is awaited from the check at:8850throughprovideToolResults/setStreamingat:9112-9116. So no deferred flush, abort note or other write can land between the two rows. - On
mainthe tool_use was stored before the spill's await. A stop in the window then left a tool_use with no result, and forstopRunthe[inference-failed]note sat between them. That can no longer happen. - On the drop path,
pendingAssistantBlocksis cleaned up by driveStream'sfinally(:13717-13720), whoseownsPhysicalStreamstays true because no new stream can start for the agent while the event loop awaits this boundary. For an ephemeral run, the disposal cleans it up (:6132). - The tool-result guard never staged the batch, so its
abandon('aborted')has nothing to settle. - A message sent during the write is still deferred, and driveStream's
finallyflushes it after the stop. I checked this: the message landed after the[inference-failed]note. The agent's next turn then runs and stores normally.
Do stop(), cancelStream() and abortInference() behave the same at the boundary? They do in what they store and in whether the turn goes on. In traces they match each other exactly as they already did while tools run:
- All three cancel the stream and set the agent idle synchronously (
src/agent.ts:1426-1429,:1488-1491,src/framework.ts:2735-2736). - All three reach driveStream's unmarked
abortedcase:inference:exhausted, the[inference-failed] … Stream aborted: usernote, settled asexhausted. - The boundary emits one
tool:result_droppedfor each. - The framework's
abortInference()also emitsinference:abortedand returnstrue. That's the same asymmetry it has atstreaming/waiting_for_tools, and it's #172's to resolve. stop()also closes admission and waits for the stream handles to settle.
Does a budget restart still happen when it should? Yes. With nobody stopping, the identity check passes, the marker is set before agent.cancelStream() (:9079-9081), and that call now cancels the ready stream itself. That's equivalent to the removed direct cancel() plus the old idle-only cancelStream(). The restart branch always has a stream, since both overBudget and overPhysical require currentState.stream. The new restart test and physical-window-restart (12 of 12) pass.
The description's account of main is accurate, with one nuance. cancelStream(), stop() and abortInference() did pass over a ready agent, and the turn resumed or restarted. A host that cancelled the stream itself first (connectome-host's stopRun) didn't see the turn go on. The resume threw Cannot provide tool results: stream is not waiting for tools (status: done), which I reproduced with this PR's tests against main's src, and the round was left stored around the abort note, as the description says. A quiesce's abandon on main did reset the agent, but the boundary then restarted the turn if it was over budget, or threw on the resume if it wasn't.
Verified fine
- Red-first: the new tests against
main'ssrc/agent.tsandsrc/framework.ts: 7 of 8 fail, and the budget-restart-nobody-stops case passes on both, as the description says. - Against what will run it: this PR merged into current
main(e6a03170) builds withtscon the dependency floors (@animalabs/membrane0.5.87,@animalabs/context-manager0.14.1,@animalabs/chronicle0.4.0).test/stop-at-tool-boundary.test.ts: 8 of 8.- Each in its own process, 559 of 559 in 16 neighbouring files:
tool-result-guard124,host-quiesce33,ephemeral-settle8,physical-window-restart12,stop-with-host-operations58,present-while-acting57,provider-waits90,code-execution50,inference-failure-bounds30,operator-surgery24,same-round-think-policy19,silent-heartbeat-rows12,tool-batch-hook9,tool-presentation7,tool-wrapper-prose-guard-integration18, and the new file. - Full
npm test: 2,338 of 2,339 tests in 164 suites. The one failure ishistory-module.test.js, whose process died withSIGSEGVin the parallel run. Run on its own it passes 49 of 49, and it doesn't touch the code this PR changes.
- The identity check is sound:
Agent.statereturns_stateitself (src/agent.ts:256). Every writer of_statewas checked, and none replaces a livereadyobject during the await except a stop: a stream start requiresidle, and framework inference is serialized behind this boundary. - The new tests do what they say: they hold the real spill write open and assert
readybefore stopping. The two real-membrane cases exercise membrane 0.5.87's owncancel()/abortedrather than the mock's. - The moved
tool-result-guardtest still tests shutdown's retry: it beginsstop()oninference:completed, after the guard has queued its failed edit. - The description's membrane citations are accurate: 0.5.87 emits
completeonly atsrc/membrane.ts:3914/:4259, afterrequestToolExecutionreturns (:3671,:4158). So thecompleteguard's own spill await (src/framework.ts:12750-12755) can't meet awaiting_for_toolsagent, and leaving it alone is fine. - connectome-host at current
main: the callers ofcancelStream()are the TUI's stop, the web UI's interrupt and the subagentstopRun(src/tui.ts:2419,src/modules/web-ui-module.ts:1875,src/modules/subagent-module.ts:1117-1119). None readstool:result_droppedorabortInference().stopRun's direct cancel followed bycancelStream()is now a double cancel, which membrane ignores once the stream isdone(src/yielding-stream.ts:180-182). - Neighbours: #172 is currently
CONFLICTINGwithmainregardless of this PR. #286 touches the drain block below the new check. The description already says how whichever lands second should rebase.
Method
Two reviewers, each pass verified by the other before anything was adopted.
-
Codex
gpt-5.6-solatxhigh(read-only sandbox, no network, no dependencies installed): about 8 minutes, 158 commands, 4.52M input tokens (4.31M cached) and 28.7k output tokens (18.7k reasoning). It read the diff,src/, membrane 0.5.87's source and connectome-host at the PR's cited commit. It reported two findings:- The quiesce ordering, rated major. Claude had found it independently. A probe forcing the race couldn't reach the failure from real callers, so it is downgraded here to a minor hardening note (R1-1).
- The ledger prose (R1-2), which only Sol caught.
-
Claude (Opus 5.5): found R1-3 and R1-4. Ran the tests (red-first against
main'ssrc, the merge-with-mainruns above) and wrote throwaway probes for:- a deferred message across a stop;
- the next turn after a stop;
- two-call rounds, stopped and not;
- the per-stop terminal traces;
- abandon fired inside the spill's completion.
None of the probes is part of this PR.
Sol couldn't run anything. Neither reviewer ran against a live provider or a live connectome-host.
… cancelled; texts and tests for what a dropped round leaves Answers the review of 19f6587 (R1-1 to R1-4). R1-1: a quiesce's abandon records its cancel, cancels the stream and leaves the agent's `ready` state to driveStream, which resets it once it reads the stream's `aborted`. The boundary's identity check saw the abandon only because that reset landed before the spill's write returned. The boundary now also drops the round when the framework has recorded a cancel of the stream and the stream is no longer waiting for tools. Any recorded kind counts, since the kinds this boundary sets itself, `turn_ended` and `budget_restart`, are set below the check. The stream's own state is what makes the record this stream's: records are keyed by the agent's name and the stream's number, which a new agent of the same name starts again from 1, so one left under the key by an earlier stream sits beside a stream that is still waiting, and drops nothing. R1-2: a dropped round's images stay in the agent's image ledger until the ledger evicts them, or until stop() or an ephemeral run's disposal discards the ledger. The comment says so. The changelog no longer describes the ledger, and keeps the spilled result's workspace file, which nothing stored names. R1-3: what a dropped round doesn't store is its whole assistant content, its text and any thinking as well as its tool_use. The changelog and the comment say so. R1-4 and the rest, in test/stop-at-tool-boundary.test.ts: - a quiesce's abandon whose stream delivers `aborted` only after the boundary has decided, under budget and over it (the test mock's stream, with its `aborted` held back); - a round of two calls with thinking and text, stopped with cancelStream() and with nobody stopping it; - a cancel recorded under the stream's key that wasn't this stream's; - after a stop, the round's text isn't in the window; after cancelStream() the spilled result's workspace file is still there and the result's image is still in the agent's image ledger, and stop() and a disposal discard the ledger. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Replying to this review. @Anarchid All four are fixed in R1-1. The boundary now also drops the round when the framework has recorded a cancel of the stream and the stream is no longer waiting for tools (
R1-2. The comment now says the images stay until the ledger evicts them, or until R1-3. The changelog, the comment and the description now say that the framework stores neither a dropped round's assistant content (its text, any thinking and its tool_use) nor its results. The R1-4. Two tests run a round of two calls, each result spilling, after thinking and text. Stopped with Checks. At Written by Milo-2653 (Claude Opus 5.5). Text reviewed and posted by Fiona-2757 (GPT-6). — Fiona-2757 |
One conflict, in the tool boundary's budget restart. anima-research#382 removed the direct currentState.stream.cancel() and its comment there, since agent.cancelStream() now cancels the stream at `ready` itself. This branch had added its end recorder's call after that direct cancel. The resolution takes main's side and keeps the recorder's call inside the `if (currentState.stream)` block, after the framework's cancel marker and before agent.cancelStream(): the recorder counts and logs the stream from its own closures and reads nothing the cancel changes, and its write can't skip the cancel, since it catches what it throws (bb11216). Everything else merged without conflict.
…rd isn't a stop, and abortInference traces once Answers our internal review of the merge and the routing. A stop whose own log write fails. The stop terminals, settleDeliberateCancel() and settleFrameworkCancel(), let a failed inference-log write throw. The throw left the terminal partway and reached driveStream's catch, which sent the stop down the failure path: inference:failed and inference:exhausted. A framework stop's terminal writes before it traces, so a shutdown or a quiesce abandon whose write failed was never traced as inference:aborted at all. Each terminal now catches its write's failure and says so on the console, as the writes at a boundary end and in finally do, so the stop stays a stop. logStream has already marked the rounds' usage logged, so they stay counted but unlogged, as when finally's own write fails. Marking them after the write would carry them twice wherever a store records an entry and then throws. A budget restart's caller record. The merge records a caller's cancel whenever cancelStream() cancels a stream, and anima-research#382's cancelStream() cancels at a tool boundary too. So a budget restart's own cancelStream(), which runs at a tool boundary, now leaves a reasonless caller record. Our must-fix's message says nothing is recorded there, which was true before anima-research#382. The error case let that record decide unless the framework had marked a shutdown or a quiesce abandon, so a restart whose stream reports its cancel as an error ended as a caller's stop: the [turn-interrupted] note, inference:aborted, and an ephemeral run's caller rejected. In the error case the caller record now decides only when the framework marked nothing, as after the loop. An endTurn cancels the stream object directly and leaves no record. abortInference at a tool boundary. framework.abortInference() emits its own inference:aborted only when no stream will report the abort. It kept anima-research#172's test, streaming or waiting for tools, while anima-research#382's Agent.abortInference() cancels a `ready` agent's stream too. So an abort at a tool boundary was traced twice. It now asks what Agent asks: whether the state holds a stream. Tests: - end-turn-usage: a caller's stop, a shutdown and an operator quiesce after a finished round each get one entry, the stop's own, carrying the round, when the stream reports the cancel as an error and when it just closes. That reaches the four call sites the aborted-shape tests don't. - end-turn-usage: a caller's stop, a shutdown and a quiesce after a finished round whose own entry's write throws each trace inference:aborted once and no failure, and the console says so once. - end-turn-usage: a budget restart whose stream reports its cancel as an error, and one whose stream just closes, trace no inference:aborted and write no [turn-interrupted] note. - stop-at-tool-boundary: abortInference at a tool boundary traces one inference:aborted, carrying its reason. - tool-result-guard: the stream that waits after reporting a caller's cancel now waits until the run's caller has deregistered the agent, within 5 s, where it waited a fixed 50 ms. A loaded run could take longer to deregister, and the test then passed without the terminal's own batch abandon. - cancel-surfaces-as-error: the aborted-with-reason-error test waits for the turn's terminal trace, where it slept a fixed 100 ms before asserting inference:exhausted had come. RoundThenEnd takes how a cancel ends it: aborted, as membrane reports one, an error event, or just closing. residentWith() takes the agent's limits. Comments, and anima-research#207's fragment: a stop that reaches its terminal writes its own entry. A stop whose stream was superseded, or whose agent was replaced or disposed, before its terminal arrives never reaches one: driveStream drops the event, as on main. The comment after the loop says the framework's record decides first, which it does. The record is taken after the caller record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TL;DR: A stop that arrives at a tool boundary now stops the turn. That is the moment between a round's last tool result and the stream's resume, while the framework stores the round. Before,
Agent.cancelStream(),stop()andabortInference()passed over the stream there, and the boundary then resumed the turn, or restarted it when it was over budget.We'll merge this not before 2026-10-17 12:00 UTC, once our pre-merge checks pass on its merge with the then-current
main, unless you ask for changes before then.The gap
Once a round's last tool result is in, the agent reads
readyuntil the boundary resumes its stream. In between, the boundary awaitsbuildStoredToolResultContent, which writes a result over the inline cap to the workspace. That write goes through the workspace's mount turn, so it can wait on other work there. A stop in that window didn't stop:Agent.cancelStream(), which every connectome-host stop calls (the TUI's Esc, the web UI's interrupt, a subagent's cancel and zombie reclaim), cancelled only fromstreamingandwaiting_for_tools. Atreadyit set the agent idle and left the stream running. After its write, the boundary resumed the stream (inference:stream_resumed) and set the agent streaming again, so the turn went on.stop()cancelled only agents at those two statuses, so the turn resumed and completed during shutdown.abortInference()returnedfalse, as if nothing were in flight, and the turn resumed.The budget restart's own call to
cancelStream()atreadyhad met this already: its comment recorded the parked stream, and it cancelled the live stream itself. connectome-host works around it too: its subagent stop cancels areadyagent's stream directly before callingcancelStream()(stopRun,src/modules/subagent-module.ts:1101–1121at3874469).The change
Agent.cancelStream()cancels the stream of whichever state holds one ('stream' in state: streaming, waiting for tools, or ready), asabandonActiveTurnsalready reads it.Agent.abortInference()andstop()'s cancel loop use the same test, so no list of statuses is left to go stale.After its await, the boundary goes on only if the round's turn is still live. Two checks decide it:
readyobject the boundary began from.cancelStream(),stop()andabortInference()set the agent idle, and an ephemeral run's disposal cancels before it deregisters, so each replaces it.aborted. That reset can come after the boundary's check, so the boundary also reads the recorded cancel. It counts the record only when this stream is no longer waiting for its tool results.If either says the turn was stopped, the boundary logs a line, emits
tool:result_droppedfor the result it was handling, as the drop after the batch hook does, and returns. It doesn't resume, restart, settle an endTurn, or drain deferred writes.The boundary stores the round's assistant row (its text, any thinking and its tool_use) after that check, with its results, rather than before the await. So a stopped round isn't stored at all, as when a stop comes while the round's tools run (the stop's teardown deletes the held blocks), and the window never holds the tool_use without its results.
The budget restart's workaround becomes
agent.cancelStream()after its mark, and the comment that described this gap goes.What hosts and readers of the store will notice
abortInference()returnstruethere.tool:result_droppedper stop at a boundary, for the result the boundary was handling. It's the existing trace with its meaning unchanged: a result that reached no model because its turn changed. The round's earlier results go with its state, without events of their own, as at the batch hook's drop. ItsagentStatusis the agent's status at the drop:idleafter a host's stop, andreadyfor a quiesce's abandon the boundary sees before driveStream has reset the agent.stop()or an ephemeral run's disposal discards the ledger. Nothing stored names either.srcandwebat3874469found no references totool:result_dropped,abortInference()or the tool-result process event. A test using itsstopRuncancel sequence now ends cleanly. Onmain, the boundary resumed onto the streamstopRunhad just cancelled, and the resume threw ("Cannot provide tool results: stream is not waiting for tools (status: done)"). The round was stored anyway, with the turn's[inference-failed]note between its tool_use and its result. Now its own cancel ends the stream, and the boundary drops the round as it does forcancelStream()alone.What each part is for
This change exists so that a stop at a tool boundary stops the turn.
'stream' in stateinAgent.cancelStream(),Agent.abortInference()andstop()'s loop. Each of the three listed the statuses it would cancel, and none listedready. Asking whether the state holds a stream is howabandonActiveTurnsalready reads it, and it leaves no list to fall behind a new status. The alternative is addingreadyto each list, leaving three lists to keep in step. Without the change, the stop doesn't reach the stream: taking it out of each fails 6, 1 and 1 tests.readyobject the boundary began from, as the drop after the batch hook compares the agent's pending call. One comparison catches every stop that replaces the state:cancelStream(),stop(),abortInference()and an ephemeral run's disposal.abortedreaches driveStream after the boundary has checked. Without it, the boundary is right only when driveStream wins that race (the two late-abortedtests). The check accepts any recorded kind rather than namingquiesce_abandoned; the boundary records its own kinds only after the check.main, the earlier write also lets a teardown's note land between the assistant row and its results, as the connectome-host note above describes.tool:result_droppedand a log line at the drop. This reuses the existing trace type and emits a warning, as the drop after the batch hook does.agent.cancelStream()in the budget restart. It replaces the restart's direct cancel of the stream, a workaround for this gap, and its comment, so there's one way to cancel the stream (1 test fails with no cancel there).tool-result-guardtest. Itsstop()relied on this gap. Startingstop()oninference:completedkeeps it a test of shutdown's retry, which it still pins.StoppableMembraneand the scripted membrane in the tests. The shared test mock accepts results after a cancel, which membrane refuses. The wrapper enforces that post-cancel refusal in these tests, and two cases run through membrane itself.Left out: the boundary cancelling the stream itself, which would make it a second owner of cancellation; storing a stopped round whose tools ran, which would change what a stop keeps; and keying cancel records by the stream (see Not covered).
Tests
test/stop-at-tool-boundary.test.tsholds the round's spill write open, so the agent readsready, then stops the turn and lets the write finish:cancelStream(), a host that cancels the stream itself first asstopRundoes, andabortInference(): the stop cancels the stream, the round isn't resumed and no other request goes out, onetool:result_droppednames the round's call, and neither the round's tool_use nor its result is in the window. AftercancelStream()the round's text isn't either, the spilled result's workspace file is still there, and the result's image is still in the agent's image ledger;stop(): the stream is cancelled, the boundary does not resume it, onetool:result_droppednames the call, no other request goes out, and the image-ledger map is empty;abortedand driveStream resets the agent before the write returns, and then there's noinference:stream_restarted, no restart request, and no round in the window;abortedonly after the boundary has decided, so the agent still readsreadywhen the boundary checks it, under budget and over it: onetool:result_droppedwithagentStatus: 'ready', no resume or restart, and no round in the window;cancelStream(), nothing of the round is stored, and onetool:result_droppednames the result whose arrival made the round ready;Except in the first quiesce case and the
stopRuncase, the stream is the test mock's, made to refuse results once cancelled, as membrane'sYieldingStreamImpldoes atdone. In the late-abortedcases it also holds itsabortedback until the test releases it. The tests assert neitherinference:exhaustednorinference:abortedfor the stop, since #172 changes which one a stop gets.test/tool-result-guard.test.ts's "storage recovery: shutdown retries edits queued by its terminating stream" beganstop()inside the submission of the round's results, while the agent readready. So it relied onstop()passing over that agent: the stream went on to complete and accept the batch whilestop()waited. It now beginsstop()on the turn'sinference:completed, after the guard's accept has queued its failed edit. That keeps what it tests: it passes onmainand with this change, and it still fails with shutdown's retry of queued edits removed.Checks
At this head merged with
main2482625, on Linux, with CI's steps on Node 24.22.0 and 20.20.2:The typecheck is clean,
npm testpasses 2,424 of 2,424, and the three files CI runs unforced pass 105 of 105, on both Nodes.Each test file in its own process passes on both Nodes. On Node 20.20.2 that is 2,424 tests in 153 files, and
main2482625counts 2,411 in 152, so the only difference is this PR's 13 tests. On Node 24.22.0,--test-force-exitendedhistory-moduleafter 37 of its 49 tests, all passing, so that run counted 2,412; the single run of all files counted all 2,424 on both Nodes.Red-first: against
main'ssrc/agent.tsandsrc/framework.ts, 10 of the 13 tests fail. Three pass there too: a budget restart nobody stops, sincemain's workaround already cancelled the replaced stream; a round of two calls nobody stops, whichmainalso stores whole once its write is done; and a cancel record that wasn't the stream's, sincemainreads no record there.Against this PR's first commit,
19f6587, only the two quiesce cases whoseabortedcomes late fail, at their assertions.Each piece taken out fails tests of its own:
'stream' in statetest incancelStream()(6 cases), inabortInference()(1) and instop()'s loop (1);abortedcases), and the stream's confirmation of a record (the case with a record that wasn't the stream's);d5e6feb1, shutdown's retry of queued edits (4tool-result-guardtests, the moved one among them).The ledger checks fail when what they check changes: with
stop()keeping the image ledgers, or a disposal keeping its run's, the test that reads that ledger fails.GitHub's CI run at
6e24dbepasses all four Build & Test legs (Ubuntu and macOS, each on Node 20 and 24) and the packed-release install check, and the changelog check passes.Not covered
gatherContext, the compile), or while the tool-result guard waits to retry a refusal after its stream has completed, are outside this fix. Those get their own pull request, after this one and fix: a cancelled stream is a deliberate stop, not a failure; a shutdown is neither (supersedes #134) #172.complete-case guard, for a stream that completes while the agent still waits for tools, awaits the same spill and remains unchanged. The reason for leaving it comes from inspection of membrane 0.5.87, this package's floor: its two runners place thecompleteemission (src/membrane.ts:3914,:4259) after theirawait stream.requestToolExecution(…)(:3671,:4158). The framework's onlyprovideToolResultscall is immediately followed bysetStreaming(src/framework.ts:9125–9129). These tests do not exercise that guard.src/agent.ts:120,:1265). So a record that outlives its stream can sit under a key a later stream takes, and driveStream reads the record by that key when the later stream is aborted or fails (src/framework.ts:13283,:13366). The boundary here counts a record only when this stream is no longer waiting for tools, so an old record alone does not drop a round still waiting for its results (the seeded-record test above). Keying the records by the stream itself would close it everywhere. That would be a change of its own, which this pull request doesn't make.Neighbours
A scan of 73 open pull requests at 05:18 UTC on October 17 identified these four neighbours. The scan used
git merge-treeagainst this head merged withmain2482625:abortedcase records: a stop rather than a failed inference. Stops atreadynow reach that case. The two conflict incancelStream()andstop()'s loop. At9178180, fix: a cancelled stream is a deliberate stop, not a failure; a shutdown is neither (supersedes #134) #172 records a cancel only forstreamingandwaiting_for_tools; this PR uses'stream' in stateinstead.agent.cancelStream(). The conflict is between fix(usage): a tool-ended turn completes, and every stream counts its usage once #207's added line and this PR's replacement cancel.cancelStream()'s status checks in atry/finally. The conflict is between feat(lifecycle): add irreversible resident retirement primitive #116's wrapper and this PR's single'stream' in statecheck.Written by Milo-2653 (Claude Opus 5.5). Text reviewed and posted by Fiona-2757 (GPT-6).