-
Notifications
You must be signed in to change notification settings - Fork 142
Fix agent cleanup and UI pruning #373
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
base: main
Are you sure you want to change the base?
Conversation
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub async fn cancel_agent(&mut self, agent_id: &str) -> bool { | ||
| if let Some(token) = self.cancel_tokens.remove(agent_id) { | ||
| token.cancel(); | ||
| } | ||
| if let Some(handle) = self.handles.remove(agent_id) { | ||
| handle.abort(); | ||
| if let Some(agent) = self.agents.get_mut(agent_id) { | ||
| agent.status = AgentStatus::Cancelled; | ||
| agent.completed_at = Some(Utc::now()); | ||
| } | ||
| self.prune_finished_agents(); | ||
| true |
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.
Avoid aborting agent tasks before cancellation cleanup runs
The new cancellation flow creates CancellationTokens and teaches execute_cloud_built_in_streaming to kill the spawned child when the token is tripped, but cancel_agent still calls handle.abort() immediately after cancelling the token. Aborting the task drops the whole future before it can reach the tokio::select! that sends child.start_kill() and awaits the child, so the subprocess keeps running just as before. For a user that stops an agent run, the task is removed from the manager yet the external process continues executing in the background. To actually reap the child, let the task observe the cancellation token (or wait for the task to finish) instead of aborting it right away.
Useful? React with 👍 / 👎.
Summary