chore: rolling promotion dev -> main - #615
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().
fix(team): kill all members on team done/blocked
execSync calls used stdio: 'inherit' which dumps bun install output into CC's protocol stream, corrupting it and crashing the session. Changed to ['pipe', 'pipe', 'inherit'] so only stderr is forwarded (for progress messages) while stdout is captured silently. Closes #612
fix: pipe stdout in smart-install hook to prevent session crashes
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughVersion bumps applied across multiple plugin manifests and packages (3.260316.16 → 3.260316.17). New team member cleanup function added to team management module with file locking for status updates, integrated into team command handlers to kill members when marking teams as done or blocked. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 performs a routine rolling promotion from the 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 appears to be a rolling promotion from dev to main, including version bumps and a few functional changes. Notably, the team done and team blocked commands now also terminate all associated worker processes.
My review has identified a couple of areas for improvement:
- A change to
execSyncoptions insmart-install.jsappears to unintentionally suppress output during installation, which could negatively impact user experience and debuggability. - The new
killTeamMembersfunction could benefit from logging errors instead of swallowing them, which would aid in future debugging.
Please see the detailed comments for specifics.
| execSync('powershell -c "irm bun.com/install.ps1 | iex"', { stdio: ['pipe', 'pipe', 'inherit'], shell: true }); | ||
| } else { | ||
| execSync('curl -fsSL https://bun.com/install | bash', { stdio: 'inherit', shell: true }); | ||
| execSync('curl -fsSL https://bun.com/install | bash', { stdio: ['pipe', 'pipe', 'inherit'], shell: true }); |
There was a problem hiding this comment.
The change from stdio: 'inherit' to stdio: ['pipe', 'pipe', 'inherit'] for execSync will suppress the standard output of the installation commands. The output is piped but not captured or displayed, which will hide installation progress from the user. This can make it seem like the installation is hanging and complicates debugging.
For interactive installation scripts like this, showing the output is crucial for user experience. I recommend reverting this change to restore the previous behavior.
This feedback also applies to the other execSync calls modified in this file (lines 162 and 390).
| execSync('powershell -c "irm bun.com/install.ps1 | iex"', { stdio: ['pipe', 'pipe', 'inherit'], shell: true }); | |
| } else { | |
| execSync('curl -fsSL https://bun.com/install | bash', { stdio: 'inherit', shell: true }); | |
| execSync('curl -fsSL https://bun.com/install | bash', { stdio: ['pipe', 'pipe', 'inherit'], shell: true }); | |
| execSync('powershell -c "irm bun.com/install.ps1 | iex"', { stdio: 'inherit', shell: true }); | |
| } else { | |
| execSync('curl -fsSL https://bun.com/install | bash', { stdio: 'inherit', shell: true }); |
| } catch { | ||
| // Best-effort — continue with other members | ||
| } |
There was a problem hiding this comment.
While the "best-effort" approach of continuing on failure is reasonable here, completely swallowing errors in the empty catch block can make debugging difficult if killWorkersByName fails for an unexpected reason.
It would be beneficial to log the error to provide visibility into potential issues.
} catch (error) {
console.error(`Failed to kill workers for member "${member}":`, error);
// 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: 953a7839c7
ℹ️ 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.
Restrict done/blocked worker kill to the selected team
killTeamMembers now drives team done/team blocked, but it kills by member name only (killWorkersByName(member)), and that matcher filters on role/id without any team check. In a normal multi-team setup where both teams have roles like engineer, marking team A as done/blocked will also terminate team B's workers with the same role, interrupting unrelated work. Please scope the worker match to teamName before killing/unregistering.
Useful? React with 👍 / 👎.
Rewrote team-lead AGENTS.md using prompt-optimizer patterns: - promptMode: append → system (replaces CC default prompt entirely) - XML-tagged blocks: <mission>, <principles>, <tool_usage>, <lifecycle>, <heartbeat>, <commands_reference>, <constraints> - No role prompting — direct mission with motivation - Tool usage instructions included (Bash, Read, Write, Edit, Grep, Glob) - Focused on orchestration, not general assistance
…ook-stdio feat: team-lead prompt rewrite — system mode with XML behavioral blocks
Rolling Promotion PR
Auto-maintained rolling promotion PR from
devtomain.Process:
ready-to-mergeadded when all checks passSummary by CodeRabbit
Chores
Bug Fixes
New Features