Skip to content

feat(stop): use agent.cancel() to stop the event loop - #3506

Merged
Unshure merged 6 commits into
strands-agents:mainfrom
Unshure:agent-tasks/stop-tool-use-cancel
Oct 2, 2026
Merged

Unshure merged 6 commits into
strands-agents:mainfrom
Unshure:agent-tasks/stop-tool-use-cancel

Conversation

@Unshure

@Unshure Unshure commented Jul 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Proposal to unify the different agent cancellation strategies within the sdk around agent.cancel. This will be used for moving the stop tool out of experimental, and used to stop the bidi agent.

@Unshure
Unshure requested a review from a team as a code owner July 27, 2026 19:21
@Unshure
Unshure requested a review from mehtarac July 27, 2026 19:21
@Unshure
Unshure marked this pull request as draft July 27, 2026 19:30
Adds design doc 0017-deferred-cancel.md proposing an `afterCurrentTools`
option on agent.cancel() that defers the abort signal until after the
current tool batch completes. This enables the stop tool to use the
cancel API while preserving cooperative semantics.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Unshure
Unshure force-pushed the agent-tasks/stop-tool-use-cancel branch from 4dfca1e to c5ea838 Compare July 27, 2026 19:33
@github-actions github-actions Bot added size/m area-agent Related to the agent class or general agent questions area-tool Tool behavior/api enhancement New feature or request strands-running labels Jul 27, 2026
Comment thread team/designs/0017-deferred-cancel.md Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Comment

The design is well-structured and follows the template — the Problem/Current State is accurate (I verified stop_event_loop at event_loop.py:842 and the TS WeakSet + AfterToolsEvent.endTurn marker pattern both exist as described), and it weighs real alternatives. The main blockers are a PR-framing mismatch and a couple of open decisions the doc leaves unresolved.

Review themes
  • PR framing vs. content: The title (feat(stop):) and description describe completed Python/TS code changes with passing test counts (16/16, 17/17), but the actual diff is a single Proposed design doc. Per team/designs/README.md, a design is merged first, then implemented in a separate PR that references it. Recommend reframing this as docs(design): (proposal only) so reviewers and git history aren't misled.
  • Unresolved decisions: The composability policy is listed as a goal but never chosen, and the TS implementation path is left as an "actually, a simpler way…" aside rather than a committed decision. Both should be resolved before approval (inline).
  • Core API decision: Overloading cancel() for a cooperative stop sits in tension with the doc's own argument that cancel() is an aggressive abort — worth a stronger justification against the dedicated-method alternative (inline).
  • Scope gap: Behavior under the concurrent tool executor isn't addressed; examples assume sequential (inline).
  • Nits: future date (2026-07-27) and Issue: TBD.

Solid proposal overall — resolving the open policy/decision questions would make it straightforward to approve.

@mehtarac

Copy link
Copy Markdown
Member

@strandly-the-agent do a fresh review of this PR

@mehtarac mehtarac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this ready for review? just curious

strandly-the-agent

This comment was marked as off-topic.

Unshure and others added 4 commits September 29, 2026 10:39
Revise the proposal for a unified cancellation mechanism in the SDK, introducing an optional deferred cancellation flag on the existing `agent.cancel()` method. This change aims to simplify the cancellation process and improve the handling of tool execution termination.
Consolidate the mechanism for stopping the agent loop by adding an 'after_current_tools' option to the 'cancel()' method. This change allows for deferred cancellation until the current tool batch completes, streamlining the cancellation process.
@strands-agents strands-agents deleted a comment from github-actions Bot Sep 29, 2026
@strands-agents strands-agents deleted a comment from github-actions Bot Sep 29, 2026
@strands-agents strands-agents deleted a comment from github-actions Bot Sep 29, 2026
@strands-agents strands-agents deleted a comment from github-actions Bot Sep 29, 2026
@Unshure
Unshure marked this pull request as ready for review September 29, 2026 19:32
Comment thread team/designs/0019-deferred-cancel.md
Comment thread team/designs/0019-deferred-cancel.md
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Comment

Fresh review as requested (@mehtarac). The doc has improved substantially since the last pass — it's tighter, retitled to Unified Agent Cancellation, and the earlier open items are resolved: the composability policy is now decided (last-write-wins, with rationale), the TS implementation path commits to a choice (option (a) preferred), and the PR description no longer claims completed code. I also confirmed the proposal is backward-compatible — today's cancel() takes no args in both SDKs (agent.py:604, agent.ts:1045), so adding optional params doesn't break existing callers.

Remaining items (all minor)
  • Template header: **Status** and **Issue** fields were dropped — only **Date** remains, and the date is a future value (2026-07-27). (inline)
  • Cross-SDK signature shape: Python takes message positionally while TS uses an options bag; for a doc about a unified API this divergence is worth either aligning or justifying, plus a TS example in the DX section. (inline)
  • Nit: filename slug (0019-deferred-cancel.md) no longer matches the new title.

Strong proposal — cleaning up the header fields and the cross-SDK signature note would make this ready to approve.

Comment thread team/designs/0019-deferred-cancel.md
Comment thread team/designs/0019-deferred-cancel.md
Comment thread team/designs/0019-deferred-cancel.md
Comment thread team/designs/0019-deferred-cancel.md
@Unshure
Unshure merged commit 5e82809 into strands-agents:main Oct 2, 2026
37 of 38 checks passed
@Unshure

Unshure commented Oct 2, 2026

Copy link
Copy Markdown
Member Author

@strandly-the-agent can you help create a pull request for this design?

@strandly-the-agent

Copy link
Copy Markdown
Contributor

@Unshure done — two draft PRs, one per SDK (per the repo's one-sub-project-per-PR rule):

One thing worth your call before merging, flagged in both bodies: agent-as-tool maps a sub-agent's cancelled result to an error tool result (_agent_as_tool.py:252, agent-as-tool.ts:215), so a sub-agent that ends via stop now reports an error to its parent where it previously returned its content. That's the stop-reason-override question liramon2 raised; I left it as-is rather than guess.

Other scope notes
  • message becomes the final assistant message on every cancel exit (streaming, pre-tools, post-batch) in both SDKs; omitted keeps today's behaviour. Last write wins.
  • Python LocalAgent protocol and BidiAgent.cancel() are unchanged (Agent widens compatibly); bidi is the follow-up the design mentions.
  • My independent review subagent timed out, so the PRs carry my self-review only — please read with that in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-agent Related to the agent class or general agent questions area-tool Tool behavior/api enhancement New feature or request size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants