Skip to content

fix(stop): a stop at a tool boundary stops the turn - #382

Merged
nissa-seru merged 2 commits into
anima-research:mainfrom
nissa-seru:fix/stop-at-tool-boundary
Oct 17, 2026
Merged

nissa-seru merged 2 commits into
anima-research:mainfrom
nissa-seru:fix/stop-at-tool-boundary

Conversation

@nissa-seru

@nissa-seru nissa-seru commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

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() and abortInference() 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 ready until the boundary resumes its stream. In between, the boundary awaits buildStoredToolResultContent, 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 from streaming and waiting_for_tools. At ready it 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.
  • The framework's abortInference() returned false, as if nothing were in flight, and the turn resumed.
  • A boundary over budget restarted the turn after the stop, with a new stream and request. A quiesce's abandon reads the stream from the agent's state and cancels it, but the boundary still restarted the turn, and a budget restart runs while the host is quiesced.

The budget restart's own call to cancelStream() at ready had 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 a ready agent's stream directly before calling cancelStream() (stopRun, src/modules/subagent-module.ts:1101–1121 at 3874469).

The change

  • Agent.cancelStream() cancels the stream of whichever state holds one ('stream' in state: streaming, waiting for tools, or ready), as abandonActiveTurns already reads it. Agent.abortInference() and stop()'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:

    • The agent's state is still the ready object the boundary began from. cancelStream(), stop() and abortInference() set the agent idle, and an ephemeral run's disposal cancels before it deregisters, so each replaces it.
    • The framework hasn't cancelled the round's stream. A quiesce's abandon records its cancel and cancels the stream, and leaves the agent's state to driveStream, which resets it once it reads the stream's 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_dropped for 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

  • A stop at a tool boundary stops. The turn ends with the stop's own terminal event, the one a stop while the round's tools run already gets, instead of going on. The framework's abortInference() returns true there.
  • One tool:result_dropped per 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. Its agentStatus is the agent's status at the drop: idle after a host's stop, and ready for a quiesce's abandon the boundary sees before driveStream has reset the agent.
  • What a dropped round leaves. Neither its assistant content (its text, any thinking and its tool_use) nor its results reach the window. A result that spilled keeps its workspace file, and the images its results carried stay in the agent's image ledger until the ledger evicts them, or until stop() or an ephemeral run's disposal discards the ledger. Nothing stored names either.
  • The order of stored rows. A round's assistant row and its results now land together, after the spill's workspace write, so while the write runs neither is in the window. A message sent during the write is deferred, as before, and lands after them.
  • connectome-host: a search of its src and web at 3874469 found no references to tool:result_dropped, abortInference() or the tool-result process event. A test using its stopRun cancel sequence now ends cleanly. On main, the boundary resumed onto the stream stopRun had 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 for cancelStream() alone.

What each part is for

This change exists so that a stop at a tool boundary stops the turn.

  • 'stream' in state in Agent.cancelStream(), Agent.abortInference() and stop()'s loop. Each of the three listed the statuses it would cancel, and none listed ready. Asking whether the state holds a stream is how abandonActiveTurns already reads it, and it leaves no list to fall behind a new status. The alternative is adding ready to 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.
  • The check after the boundary's await. Without it, the boundary resumes or restarts a stopped turn (9 tests fail without it).
    • Its first arm compares the agent's state with the ready object 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.
    • Its second arm reads the framework's record of a cancel, for a quiesce's abandon whose aborted reaches driveStream after the boundary has checked. Without it, the boundary is right only when driveStream wins that race (the two late-aborted tests). The check accepts any recorded kind rather than naming quiesce_abandoned; the boundary records its own kinds only after the check.
    • That arm counts a record only when the stream is no longer waiting for its tool results. Without that condition, a record left under a reused key would drop a live stream's round (the seeded-record test). The record supplies the framework's cancellation marker. The state check establishes that the current stream is no longer waiting for tools; it does not establish the record's ownership. Without the record check, this arm would instead drop any round whose stream had stopped waiting, regardless of reason.
  • Moving the assistant-row write after the check. Keeping the old pre-await write alongside the new drop check would leave a stopped round's assistant content without its results. The cost is moving an existing write (9 tests fail without the move). On 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_dropped and 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).
  • The moved tool-result-guard test. Its stop() relied on this gap. Starting stop() on inference:completed keeps it a test of shutdown's retry, which it still pins.
  • StoppableMembrane and 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.ts holds the round's spill write open, so the agent reads ready, then stops the turn and lets the write finish:

  • a host's cancelStream(), a host that cancels the stream itself first as stopRun does, and abortInference(): the stop cancels the stream, the round isn't resumed and no other request goes out, one tool:result_dropped names the round's call, and neither the round's tool_use nor its result is in the window. After cancelStream() 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, one tool:result_dropped names the call, no other request goes out, and the image-ledger map is empty;
  • a quiesce's abandon at a boundary over budget, through membrane itself over a scripted provider: membrane delivers aborted and driveStream resets the agent before the write returns, and then there's no inference:stream_restarted, no restart request, and no round in the window;
  • a quiesce's abandon whose stream delivers aborted only after the boundary has decided, so the agent still reads ready when the boundary checks it, under budget and over it: one tool:result_dropped with agentStatus: 'ready', no resume or restart, and no round in the window;
  • a round of two calls, each result spilling, after thinking and text: stopped with cancelStream(), nothing of the round is stored, and one tool:result_dropped names the result whose arrival made the round ready;
  • an ephemeral run whose idle watchdog disposes it at the boundary: the disposal cancels the stream, the round isn't resumed or stored, and the run's image ledger goes with it;
  • a boundary nobody stops: while the write runs, neither of the round's rows is in the window; then both are, adjacent, with a message sent meanwhile after them;
  • the round of two calls, nobody stopping: its thinking, text and two calls are stored in order, with both results right after;
  • a cancel recorded under the stream's key that wasn't this stream's, as a record from an earlier agent of the same name would be: the stream is still waiting for its results, and the round is stored and resumed;
  • a boundary over budget that nobody stops restarts the turn, and the restart cancels the stream it replaces.

Except in the first quiesce case and the stopRun case, the stream is the test mock's, made to refuse results once cancelled, as membrane's YieldingStreamImpl does at done. In the late-aborted cases it also holds its aborted back until the test releases it. The tests assert neither inference:exhausted nor inference:aborted for 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" 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 guard's accept has queued its failed edit. That keeps what it tests: it passes on main and with this change, and it still fails with shutdown's retry of queued edits removed.

Checks

At this head merged with main 2482625, on Linux, with CI's steps on Node 24.22.0 and 20.20.2:

  • The typecheck is clean, npm test passes 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 main 2482625 counts 2,411 in 152, so the only difference is this PR's 13 tests. On Node 24.22.0, --test-force-exit ended history-module after 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's src/agent.ts and src/framework.ts, 10 of the 13 tests fail. Three pass there too: a budget restart nobody stops, since main's workaround already cancelled the replaced stream; a round of two calls nobody stops, which main also stores whole once its write is done; and a cancel record that wasn't the stream's, since main reads no record there.

  • Against this PR's first commit, 19f6587, only the two quiesce cases whose aborted comes late fail, at their assertions.

  • Each piece taken out fails tests of its own:

    • the 'stream' in state test in cancelStream() (6 cases), in abortInference() (1) and in stop()'s loop (1);
    • the whole check after the await (9), its recorded-cancel arm (the 2 late-aborted cases), and the stream's confirmation of a record (the case with a record that wasn't the stream's);
    • storing the round after the check rather than before the await (9);
    • the restart's own cancel (1);
    • and, from the first commit's run on d5e6feb1, shutdown's retry of queued edits (4 tool-result-guard tests, 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 6e24dbe passes 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

  • Stops before the turn's stream exists (its hooks, 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.
  • The round in flight at a stop isn't stored, whether the stop lands while its tools run or at the boundary. Keeping it would change what a stop keeps, which is outside this fix.
  • The 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 the complete emission (src/membrane.ts:3914, :4259) after their await stream.requestToolExecution(…) (:3671, :4158). The framework's only provideToolResults call is immediately followed by setStreaming (src/framework.ts:9125–9129). These tests do not exercise that guard.
  • A cancel record left over from an earlier stream. The framework records its own cancels under the agent's name and the stream's number, and a new agent of the same name numbers its streams from 1 again (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.
  • Not run against a live provider or connectome-host. The window needs a result over the inline cap and a workspace to spill to, and the tests hold that write open.

Neighbours

A scan of 73 open pull requests at 05:18 UTC on October 17 identified these four neighbours. The scan used git merge-tree against this head merged with main 2482625:

Written by Milo-2653 (Claude Opus 5.5). Text reviewed and posted by Fiona-2757 (GPT-6).

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>
@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5 Tier: apex

[Medium impact] The reviewed changes appear safe to merge; no new blocking issue was found.

Summary

Stops now cancel streams parked at tool boundaries. The boundary checks whether the turn stopped after preparing its results, before storing or resuming the round.

  • Stops at the tool-result boundary now end the turn.

Reviews (2) · Last reviewed commit: "fix(stop): the boundary also drops a rou..." · Reviewed by Greptile

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() queues aborted and wakes the iterator synchronously (src/yielding-stream.ts:179-195);
  • driveStream is parked on next(), since neither its loop head (:12357-12360) nor its tool-calls case 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, because turn_endedandbudget_restart` are set later by this same boundary.
  • Or have abandonActiveTurns call agent.cancelStream() after recording the marker, so the agent goes idle synchronously. The quiesce_abandoned case's reset() then has nothing left to do. This changes the abandon for the streaming and waiting_for_tools states 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 one tool:result_dropped goes 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.addAssistantResponse at :8871, toolResultGuard.storeResults at :8877) are synchronous, and nothing is awaited from the check at :8850 through provideToolResults/setStreaming at :9112-9116. So no deferred flush, abort note or other write can land between the two rows.
  • On main the tool_use was stored before the spill's await. A stop in the window then left a tool_use with no result, and for stopRun the [inference-failed] note sat between them. That can no longer happen.
  • On the drop path, pendingAssistantBlocks is cleaned up by driveStream's finally (:13717-13720), whose ownsPhysicalStream stays 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 finally flushes 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 aborted case: inference:exhausted, the [inference-failed] … Stream aborted: user note, settled as exhausted.
  • The boundary emits one tool:result_dropped for each.
  • The framework's abortInference() also emits inference:aborted and returns true. That's the same asymmetry it has at streaming/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's src/agent.ts and src/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 with tsc on the dependency floors (@animalabs/membrane 0.5.87, @animalabs/context-manager 0.14.1, @animalabs/chronicle 0.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-guard 124, host-quiesce 33, ephemeral-settle 8, physical-window-restart 12, stop-with-host-operations 58, present-while-acting 57, provider-waits 90, code-execution 50, inference-failure-bounds 30, operator-surgery 24, same-round-think-policy 19, silent-heartbeat-rows 12, tool-batch-hook 9, tool-presentation 7, tool-wrapper-prose-guard-integration 18, and the new file.
    • Full npm test: 2,338 of 2,339 tests in 164 suites. The one failure is history-module.test.js, whose process died with SIGSEGV in 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.state returns _state itself (src/agent.ts:256). Every writer of _state was checked, and none replaces a live ready object during the await except a stop: a stream start requires idle, and framework inference is serialized behind this boundary.
  • The new tests do what they say: they hold the real spill write open and assert ready before stopping. The two real-membrane cases exercise membrane 0.5.87's own cancel()/aborted rather than the mock's.
  • The moved tool-result-guard test still tests shutdown's retry: it begins stop() on inference:completed, after the guard has queued its failed edit.
  • The description's membrane citations are accurate: 0.5.87 emits complete only at src/membrane.ts:3914/:4259, after requestToolExecution returns (:3671, :4158). So the complete guard's own spill await (src/framework.ts:12750-12755) can't meet a waiting_for_tools agent, and leaving it alone is fine.
  • connectome-host at current main: the callers of cancelStream() are the TUI's stop, the web UI's interrupt and the subagent stopRun (src/tui.ts:2419, src/modules/web-ui-module.ts:1875, src/modules/subagent-module.ts:1117-1119). None reads tool:result_dropped or abortInference(). stopRun's direct cancel followed by cancelStream() is now a double cancel, which membrane ignores once the stream is done (src/yielding-stream.ts:180-182).
  • Neighbours: #172 is currently CONFLICTING with main regardless 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-sol at xhigh (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's src, the merge-with-main runs 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>
@nissa-seru

Copy link
Copy Markdown
Contributor Author

Replying to this review.

@Anarchid All four are fixed in 6e24dbe, one commit on 19f6587. R1-1 uses your first suggested fix with one added condition, and the fixes for R1-2 to R1-4 are text and tests. We'll still merge it no earlier than 12:00 UTC on October 17, once our pre-merge checks pass on its merge with the then-current main, unless you ask for changes before then.

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 (src/framework.ts:8860-8863).

  • Any recorded kind counts, not only quiesce_abandoned. At that point any record is another caller's cancel, since this boundary records its own turn_ended and budget_restart only below the check. The stops that record nothing, such as stop()'s loop and a disposal, replace the agent's state, and the check that the state is still the boundary's ready object catches them.
  • The stream's own state is the condition we added. The framework keys its records by the agent's name and the stream's number, and a new agent of the same name numbers its streams from 1 again. Without the condition, a record left under the key by an earlier stream would drop a live stream's round. At this boundary a live stream is still waiting for its results, so such a record drops nothing.
  • Three tests cover it. In two, the stream delivers the abandon's aborted only after the boundary has decided, one under budget and one over it. Without the record check, the boundary resumes the round under budget, and over budget restarts the turn during the quiesce. The third seeds a record under the stream's key and fails if the check reads the record alone.
  • One case remains. driveStream reads a record by the same key when a stream is aborted or fails, so there a leftover record can still be read as a later stream's. Keying the records by the stream itself would close that, in a change this pull request doesn't make. The description's Not covered section names it.

R1-2. The comment now says the images stay until the ledger evicts them, or until stop() or an ephemeral run's disposal discards the ledger. The tests check three stop paths: after cancelStream() the image is still in the agent's ledger, after stop() there are no ledgers, and after the disposal the run's ledger is gone. The changelog no longer mentions the ledger, which is the framework's own cache, and keeps to what a host or a reader of the store sees. That includes the spilled result's workspace file, which stays and which nothing stored names. The cancelStream() test now reads that file back.

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 cancelStream() test checks that the round's text is absent from the window, and the stopped two-call round below, which has a thinking block, leaves none of its blocks there.

R1-4. Two tests run a round of two calls, each result spilling, after thinking and text. Stopped with cancelStream(), the round leaves nothing in the window, and one tool:result_dropped names the result whose arrival made the round ready. With nobody stopping it, the window holds the thinking, the text and both calls in order, with both results right after.

Checks. At 6e24dbe merged with main 2482625, on Linux with Node 24.22.0 and 20.20.2, the typecheck is clean, CI's steps pass with 2,424 tests, and the per-file runs report no failures; the description gives their counts. Against main's source, 10 of the 13 tests in test/stop-at-tool-boundary.test.ts fail, and against 19f6587's, only the two late-aborted quiesce cases do. Each piece of the change, taken out, fails tests of its own.

Written by Milo-2653 (Claude Opus 5.5). Text reviewed and posted by Fiona-2757 (GPT-6).

— Fiona-2757

nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 17, 2026
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 17, 2026
@nissa-seru nissa-seru added the no-greptile For maintainers and the team: Greptile skips this PR. Not for general use. label Oct 17, 2026
@nissa-seru
nissa-seru merged commit d439613 into anima-research:main Oct 17, 2026
8 checks passed
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 17, 2026
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.
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 18, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

greptile no-greptile For maintainers and the team: Greptile skips this PR. Not for general use.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants