-
Notifications
You must be signed in to change notification settings - Fork 2.3k
reuse the cancellation token in the agent level #3599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
there are a few tools removed from this - I assume that is by design? (ie not needed). Not sure how to validate functionality (as not aware of subagents) |
Hi @michaelneale yes, the sub_recipe_execution_tool are removed as they are renamed to sub_agent_execution_tool. So they are not long required. In the PR I used tokio::select! start multiple operations async, when the cancellation token is cancelled, the other operations (the tasks executions) will be cancelled. One of the test cases : I tried to kick off a recipe with 2 subrecipes (goose run --recipe sub_recipe) running. If I enters CRLT+C, the processes of running sub recipes will be terminated. Also it does not have warning/error message as specified in the issue, because if it is cancelled, the taskExecuteTracker won't send notification to the channel. One thing I forgot to mention is: the cancellation token is passed through to a few modules up to the tasks.rs. I am going to have a follow up PR to make this better. |
crates/goose/src/agents/agent.rs
Outdated
| tool_monitor, | ||
| router_tool_selector: Mutex::new(None), | ||
| scheduler_service: Mutex::new(None), | ||
| current_cancellation_token: Arc::new(Mutex::new(None)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this? can't we just use the existing token and just pass it on, instead of this being a property on the agent? I know agents aren't re-entrant, but still
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we invoke the sub_agent_execute_tool in the dispatch_tool_call function, we need to pass cancellation_token so that the tool can reuse the cancellation token. The dispatch_tool_call function is not only triggered by reply directly, it can be trigged by reply -> handle_approval_tool_requests -> dispatch_tool_call.
I can try to pass the token via those functions, but I feel it is a bit verbose to pass it around in those functions
| self.cancellation_token | ||
| .as_ref() | ||
| .is_some_and(|t| t.is_cancelled()) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably put this in some util function and use everywhere but we can do that in something separate
* 'main' of github.com:block/goose: docs: use installer component for tutorial, add updating extensions section (#3608) fix: show token alert popover during agent responses and agent failure cases (#3536) reuse the cancellation token in the agent level (#3599) Docs: Move MongoDB tutorial to MCP section (#3602) docs: desktop recipe format (#3594) Fix model display name not being updated immediately after leaving settings (#3587) Added option to summarize the chat when an error is triggered (#3598)
…ry-resiliancy * 'main' of github.com:block/goose: fix: loading shared sessions (#3607) docs: use installer component for tutorial, add updating extensions section (#3608) fix: show token alert popover during agent responses and agent failure cases (#3536) reuse the cancellation token in the agent level (#3599) Docs: Move MongoDB tutorial to MCP section (#3602) docs: desktop recipe format (#3594) Fix model display name not being updated immediately after leaving settings (#3587) Added option to summarize the chat when an error is triggered (#3598) Remove mcp_macros and unused types (#3581) fix: add fallback id to messages if none provided (#3584) feat: migrate ErrorData from internal mcp crates to rmcp version (#3586)
* main: feat: subagent turn count, simple agent loop (#3597) feat: subagent independent extension manager (#3596) Improve session history loading resiliency (#3588) Added logging and changed default route case to not redirect to home when there is an invalid route (#3610) fix: chat sidebar layout overlapping content occasionally (#3590) fix: loading shared sessions (#3607) docs: use installer component for tutorial, add updating extensions section (#3608) fix: show token alert popover during agent responses and agent failure cases (#3536) reuse the cancellation token in the agent level (#3599) Docs: Move MongoDB tutorial to MCP section (#3602) docs: desktop recipe format (#3594) Fix model display name not being updated immediately after leaving settings (#3587) Added option to summarize the chat when an error is triggered (#3598) Remove mcp_macros and unused types (#3581) fix: add fallback id to messages if none provided (#3584) feat: migrate ErrorData from internal mcp crates to rmcp version (#3586) fix: adjust subrecipe description to allow running tests (#3585) Scenario tests (#3430) feat: migrate JsonRpcMessage/Request/Response/Error/Notification from internal mcp crates to rmcp versions (#3564) Restore recipe parameters functionality (#3530)
…enhance * 'main' of github.com:block/goose: docs: use installer component for tutorial, add updating extensions section (#3608) fix: show token alert popover during agent responses and agent failure cases (#3536) reuse the cancellation token in the agent level (#3599) Docs: Move MongoDB tutorial to MCP section (#3602) docs: desktop recipe format (#3594) Fix model display name not being updated immediately after leaving settings (#3587) Added option to summarize the chat when an error is triggered (#3598)
* main: (28 commits) fix: multi-tool calls in streamed openai-compatible responses (#3609) feat: subagent turn count, simple agent loop (#3597) feat: subagent independent extension manager (#3596) Improve session history loading resiliency (#3588) Added logging and changed default route case to not redirect to home when there is an invalid route (#3610) fix: chat sidebar layout overlapping content occasionally (#3590) fix: loading shared sessions (#3607) docs: use installer component for tutorial, add updating extensions section (#3608) fix: show token alert popover during agent responses and agent failure cases (#3536) reuse the cancellation token in the agent level (#3599) Docs: Move MongoDB tutorial to MCP section (#3602) docs: desktop recipe format (#3594) Fix model display name not being updated immediately after leaving settings (#3587) Added option to summarize the chat when an error is triggered (#3598) Remove mcp_macros and unused types (#3581) fix: add fallback id to messages if none provided (#3584) feat: migrate ErrorData from internal mcp crates to rmcp version (#3586) fix: adjust subrecipe description to allow running tests (#3585) Scenario tests (#3430) feat: migrate JsonRpcMessage/Request/Response/Error/Notification from internal mcp crates to rmcp versions (#3564) ...
* main: Goose security updates (#3579) fix: multi-tool calls in streamed openai-compatible responses (#3609) feat: subagent turn count, simple agent loop (#3597) feat: subagent independent extension manager (#3596) Improve session history loading resiliency (#3588) Added logging and changed default route case to not redirect to home when there is an invalid route (#3610) fix: chat sidebar layout overlapping content occasionally (#3590) fix: loading shared sessions (#3607) docs: use installer component for tutorial, add updating extensions section (#3608) fix: show token alert popover during agent responses and agent failure cases (#3536) reuse the cancellation token in the agent level (#3599) Docs: Move MongoDB tutorial to MCP section (#3602)
Signed-off-by: Adam Tarantino <[email protected]>
Handle CLI CTRL+C gracefully to cancel the tasks
fixes issue #3553
Close #3550 as now we can reuse the cancellation token created in the agent reply