fix(team): kill all members on team done/blocked - #611
Conversation
…ked` Previously these commands only updated the config file status — the team-lead and other agents kept running in an infinite idle loop. Now both commands call killTeamMembers() to terminate all worker processes after setting status, reusing the same killWorkersByName() pattern from disbandTeam().
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the team management commands by ensuring that all active worker processes associated with a team are properly terminated when the team's status is marked as 'done' or 'blocked'. This prevents orphaned processes and addresses a bug where team-leads could remain active after their intended lifecycle, improving resource management and system stability. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a fix to ensure that when a team is marked as 'done' or 'blocked', all its member processes are terminated. This is achieved by adding a new killTeamMembers function in team-manager.ts and calling it from the done and blocked commands in team.ts. The changes are logical and effectively address the issue of lingering processes. I've suggested a small performance improvement to parallelize the worker termination process.
| for (const member of config.members) { | ||
| try { | ||
| await killWorkersByName(member); | ||
| } catch { | ||
| // Best-effort — continue with other members | ||
| } | ||
| } |
There was a problem hiding this comment.
For improved efficiency, you can parallelize killing the team members' workers using Promise.all. This will initiate all kill operations concurrently rather than sequentially, preventing a delay in killing one worker from blocking the others.
| for (const member of config.members) { | |
| try { | |
| await killWorkersByName(member); | |
| } catch { | |
| // Best-effort — continue with other members | |
| } | |
| } | |
| await Promise.all( | |
| config.members.map((member) => | |
| killWorkersByName(member).catch(() => { | |
| // Best-effort — continue with other members | |
| }), | |
| ), | |
| ); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f076c51e95
ℹ️ 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".
|
|
||
| for (const member of config.members) { | ||
| try { | ||
| await killWorkersByName(member); |
There was a problem hiding this comment.
Scope member shutdown to the target team
Calling killWorkersByName(member) here can terminate workers from other teams because team memberships store generic role names (e.g. engineer, reviewer) and killWorkersByName matches globally on w.role === agentName || w.id === agentName rather than filtering by w.team; after this commit, running genie team done <name> or blocked <name> will now kill same-role agents in unrelated active teams.
Useful? React with 👍 / 👎.
Summary
genie team done <name>andgenie team blocked <name>now kill all team member processes after setting statuskillWorkersByName()fromdisbandTeam()— same mechanism, without deleting worktree/configChanges
src/lib/team-manager.ts: AddedkillTeamMembers()— iterates team members and kills each viakillWorkersByName()src/term-commands/team.ts:doneandblockedhandlers now callkillTeamMembers()aftersetTeamStatus()Test plan
bun run typecheckpassesbun run lintpassesbun test— 736 tests passbun run build— builds successfullygenie team done <name>sets status todoneAND kills all agent processesgenie team blocked <name>sets status toblockedAND kills all agent processes