Skip to content

fix(loop): cancelled runs skip unstarted tools, every message gets events, every TurnStart is paired (#243) - #244

Merged
yuanhao merged 5 commits into
mainfrom
fix/loop-cancel-and-events
Oct 6, 2026
Merged

fix(loop): cancelled runs skip unstarted tools, every message gets events, every TurnStart is paired (#243)#244
yuanhao merged 5 commits into
mainfrom
fix/loop-cancel-and-events

Conversation

@yuanhao

@yuanhao yuanhao commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Closes #243. Stacked on #242, which adds the hook-order test that this PR updates. Merge #242 first; this PR's base then moves to main.

The three fixes

  1. A cancelled run no longer executes tool calls it hadn't started (the important one).
    • Before: a run cancelled while the model's response arrived still ran its tool calls, and the cancel was only seen at the next turn.
    • Now: execute_single_tool checks the run's cancel token before middleware, and again after middleware (which can await). An unstarted call is answered through unexecuted_tool_call with Tool call not run: the run was cancelled., so ToolExecutionStart/End stay paired and the transcript stays valid. The run then ends with [Agent stopped: cancelled] as before. A tool already running still gets the cancel via its ToolContext.
  2. Every appended message has its events.
    • Before: a final failure or cancel before any provider output appended its assistant message silently.
    • Now: it gets MessageStart + MessageEnd (AttemptEvents::is_silent → announce). This includes a run aborted during a retry's backoff. Retried attempts that failed before output still get only ProviderRetry. A provider that returns Ok without streaming anything is announced the same way.
  3. Every TurnStart has a TurnEnd. A turn stopped by an execution limit or by on_before_turn returning false now sends TurnEnd: the history's last message (the stop marker, for a limit) and no tool results.

Evidence

tests/hook_order_test.rs pins the timelines, so each behaviour change shows as a diff in this PR:

  • turn_limit: + TurnEnd.
  • provider_error_before_any_output / cancelled_before_any_output: + MessageStart/MessageEnd.
  • cancelled_before_tools_start: the tool no longer runs, and its result is an error.
  • New cases: stopped_by_on_before_turn and provider_streams_nothing.

Other tests and docs updated for the new behaviour:

  • agent_loop_test: cancelling during the backoff now shows the announced Aborted message after retry:1/4.
  • retry_safe_test: the terminal-abort pattern now ends with [Error, Aborted]. The filter releases the aborted message (renamed the_filter_and_an_abort_release_only_the_aborted_message).
  • docs/concepts/retry.md and messages-events.md described the old behaviour and are corrected. CHANGELOG Unreleased → Fixed.

For downstream consumers

These are new events, not changed ones:

  • an extra MessageStart/MessageEnd pair for failures and aborts that streamed nothing;
  • an extra TurnEnd for turns stopped early.

yoyo's prompt.rs uses MessageEnd only to flush text and counts TurnStart (unchanged). yoyo's full suite runs against the release as usual.

Checks

  • cargo test --all-features: all 56 test binaries pass.
  • clippy with --all-features and --no-default-features.
  • cargo doc -Dwarnings.
  • fmt.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG

yuanhao and others added 4 commits October 6, 2026 18:44
@yuanhao @claude
test: pin the order of existing hooks and events on every run path (#241
cc6b0ef 
 PR 0)

Records each hook with the number of events sent when it fired, so the
timeline is exact. Covers a plain answer, a tool call, a refusal, a
provider error, a cancel while streaming, a cancel while tools run, a turn
limit, and a rejected input. No behaviour change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
@yuanhao @claude
test(hook order): pin retries, hook chains, a sourced tool, turn-hook…
d131ef9 
… notes, continue_loop and errors before output

Adds the cases the review found missing: the turn hook runs once per
provider attempt; filters and middleware run in installation order (a
Modify feeds the next, a Deny stops the chain, a Warn passes); a sourced
tool goes through middleware; a turn hook's note lands on the latest user
message; continue_loop runs no input filters; a provider error or cancel
before any output leaves a message with no MessageStart/MessageEnd; a
cancel after streaming started ends the message as Aborted. Renames
cancelled_while_tools_run to cancelled_before_tools_start and corrects the
notes on current behaviour. The original eight timelines are unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
@yuanhao @claude
fix(loop): skip tool calls of a cancelled run, announce every appende…
c9c72f3 
…d message, pair every TurnStart (#243)

- A tool call reached after the run was cancelled (before middleware, or
  while middleware awaited) is answered with an error result and never
  runs. It used to run, with the cancel seen only at the next turn.
- A final failure or cancel before any output is announced with
  MessageStart/MessageEnd, so every message in the history has its events
  (including a run aborted during a retry's backoff).
- A turn stopped by an execution limit or by on_before_turn returning
  false sends TurnEnd.

Updates the pinned timelines in tests/hook_order_test.rs (each change is a
diff there), adds cases for on_before_turn stopping a run and for a provider
that streams nothing, and updates two retry_safe tests and the docs that
described the old behaviour.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
@yuanhao @claude
fix(loop): review fixes for #243
f402f95 
- Done without Start: the forwarder now sends MessageStart first, so
  every message in the history has both events.
- Doc comments of push_stop_marker and unexecuted_tool_call were split
  from their functions by the new close_turn / cancelled_tool_call.
- retry_safe_events rustdoc still described the old abort behaviour.
- Comments on the forwarder JoinError fallback and on close_turn with an
  empty history.
- Tests: a cancel while middleware decides (covers the post-middleware
  check), a cancel between Sequential tools and between Batched tools
  (second call answered, marker once), and Done without Start.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
@yuanhao
yuanhao changed the base branch from feat/extension-hook-order to main October 6, 2026 20:22
@yuanhao
Merge main (with #242 squashed) into fix/loop-cancel-and-events
973bd87 
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG

# Conflicts:
#	tests/hook_order_test.rs
@yuanhao
yuanhao merged commit b4619bb into main Oct 6, 2026
14 checks passed
@yuanhao
yuanhao deleted the fix/loop-cancel-and-events branch October 6, 2026 20:27
This was referenced Oct 6, 2026
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Loop: tools run after cancel, limit-stopped turns stay open, messages appended without events

1 participant

@yuanhao