fix(core)#20941: reap orphaned descendant processes on PTY abort by manavmax · Pull Request #21124 · google-gemini/gemini-cli

manavmax · 2026-03-04T17:21:33Z

Summary

Previously, killProcessGroup would only call pty.kill() when a PTY was provided, which only terminates the PTY session leader (e.g., the bash wrapper). Nested background child processes spawned by the command would survive as orphans, silently consuming CPU/memory and eventually causing OS-level resource exhaustion. This fix ensures complete process tree cleanup on PTY abort across both Windows and Unix.

Details

The fix adds descendant process reaping after pty.kill() on both platforms:

Windows: After pty.kill(), it now also invokes taskkill /pid /f /t to terminate the entire process tree, including descendants.
Unix: When the initial process group kill fails and falls back to pty.kill(), it now also attempts process.kill(-pid, SIGKILL) to reap any orphaned descendant processes.

Two new regression tests were added, and one existing test was updated to validate the new behaviour.

Related Issues

Fixes #20941

How to Validate

npm test -w @google/gemini-cli-core -- src/utils/process-utils.test.ts

All 9 tests pass, including 2 new regression tests:

should also invoke taskkill on Windows when pty is provided to reap descendants
should attempt process group kill on Unix after pty fallback to reap orphaned descendants

### Pre-Merge Checklist

[ ✔] Updated relevant documentation and README (if needed)
[ ✔] Added/updated tests (if needed)
Noted breaking changes (if any)
[ ✔] Validated on required platforms/methods:
- [ ✔] MacOS
- [ ✔] npm run
- [ ] npx
- [ ] Docker
- [ ] Podman
- [ ] Seatbelt
- [ ] Windows
- [ ] npm run
- [ ] npx
- [ ] Docker
- [ ] Linux
- [ ] npm run
- [ ] npx
- [ ] Docker

gemini-code-assist · 2026-03-04T17:24:56Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 addresses a critical issue where descendant processes spawned by a command within a PTY session could become orphaned and continue consuming system resources after the PTY session leader was terminated. The changes introduce robust process tree cleanup mechanisms for both Windows and Unix environments, ensuring that all child processes are properly terminated upon PTY abort, thereby preventing resource exhaustion.

Highlights

Windows Process Cleanup: Implemented taskkill /pid /f /t after pty.kill() to ensure complete termination of process trees, including orphaned descendants, on Windows.
Unix Process Cleanup: Added a fallback mechanism to attempt process.kill(-pid, SIGKILL) after pty.kill() to reap orphaned descendant processes when the initial group kill fails on Unix.
Enhanced Test Coverage: Enhanced the test suite with two new regression tests and an update to an existing test to validate the new process reaping logic on both Windows and Unix.

Changelog

packages/core/src/utils/process-utils.test.ts
- Updated an existing test case description and assertions for Windows PTY kill.
- Added a new test case to verify taskkill invocation on Windows for descendant reaping.
- Added a new test case to verify process group kill attempt on Unix after PTY fallback.
packages/core/src/utils/process-utils.ts
- Implemented taskkill invocation on Windows after pty.kill() to terminate orphaned descendant processes.
- Added a process.kill(-pid, SIGKILL) attempt on Unix after pty.kill() fallback to reap orphaned descendant processes.

Activity

Updated relevant documentation and README.
Added/updated tests.
Validated on MacOS (npm run).

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request addresses an important issue of orphaned descendant processes on PTY abort by ensuring a more robust process tree cleanup on both Windows and Unix. The changes involve adding taskkill on Windows and a fallback process group kill on Unix. The accompanying tests have been updated, but I've identified a duplicated test case that should be removed. Additionally, I've suggested a refactoring in the Windows process killing logic to eliminate code duplication and improve maintainability. Overall, this is a valuable fix for resource leak prevention.

manavmax · 2026-03-06T13:35:09Z

@google-gemini , @gemini-code-assist PTAL

gemini-code-assist

Code Review

This pull request modifies the killProcessGroup utility to prevent orphaned descendant processes when a PTY is aborted. For Windows, it ensures taskkill /f /t is always called to terminate the process tree, even when a PTY object is present. For Unix, in the fallback scenario where an initial process group kill fails, it adds a second attempt to kill the process group after the PTY's own kill method is invoked. New tests are added to verify these changes on both platforms. The changes appear to correctly implement the described logic for preventing orphaned processes. No high or critical severity issues were found during this review.

- Add vi.unstubAllEnvs() to afterEach to prevent env var test leakage - Split SystemRoot test into two hermetic vi.stubEnv variants: one with SystemRoot set (absolute path) and one without (bare binary) - Add taskkill spawn error fallback test (ENOENT -> pty.kill()) - Add Unix orphan group SIGKILL regression test after pty fallback - Replace forbidden as-any casts with as-unknown-as-ChildProcess - On Unix pty fallback path, also call process.kill(-pid, SIGKILL) to reap orphaned descendant processes, closing the same gap as google-gemini#21124 Addresses gemini-code-assist review comments on PR google-gemini#20945.

manavmax · 2026-03-09T11:30:34Z

@mrpmohiburrahman , @spencer426
PTAL , resolved all the issues.
Please rerun CI checks and review the PR.
Thanks!

manavmax · 2026-03-09T21:36:27Z

Update: Feedback Addressed

I have pushed a new commit that addresses all points raised in the latest review. I have also verified these changes locally with unit tests and linting to ensure alignment with the project’s architectural standards.

Key Changes:

Architectural Alignment: Switched from direct child_process.spawn usage to the standardized spawnAsync utility in packages/core. This ensures consistent shell operation handling and better error reporting.
Correct Process Cleanup Order: Re-verified and solidified the logic to ensure the process group kill signal (-pid) is sent before the PTY session leader is terminated on Unix/macOS. This correctly reaps orphaned grandchild processes as requested.
Async/Await Consistency: Converted ShellExecutionService.kill() to be asynchronous and ensured all call sites in the CLI (including background shell dismissal and app cleanup) properly await the termination. This fixes the race conditions identified in the review.

Test Robustness:
Added vi.restoreAllMocks() in afterEach to prevent test pollution.
Moved mock initializations into beforeEach for better isolation.
Fixed linting errors by replacing any casts with proper Vitest Mock and MockInstance types.

Verification:
packages/core/src/utils/process-utils.test.ts
passes all 8 tests.
npm run lint and npm run typecheck pass for all modified files.

Thanks again for the detailed feedback! CC: @spencer426 @mrpmohiburrahman

spencer426 · 2026-03-10T13:55:48Z

Looks like there are some merge conflicts please resolve.

Previously, killProcessGroup would only call pty.kill() when a PTY was provided, which only terminates the PTY session leader (e.g., the bash wrapper). Nested background child processes spawned by the command would survive as orphans, silently consuming CPU/memory and eventually causing OS-level resource exhaustion. On Windows: After pty.kill(), now also invokes taskkill /pid /f /t to terminate the entire process tree including descendants. On Unix: When the initial process group kill fails and falls back to pty.kill(), now also attempts process.kill(-pid, SIGKILL) to reap any orphaned descendant processes. Fixes google-gemini#20941

…lignment

manavmax · 2026-03-10T16:54:29Z

Final Update: Conflicts Resolved & Code Verified

I have resolved the merge conflict in shellExecutionService.ts. I have successfully merged the new destroy() cleanup logic from main with the required await killProcessGroup logic to ensure robust process termination on macOS/Unix.

Summary of Re-Verification:

Resolved Conflicts: Manually merged the kill method to include both the await logic for process groups and the new destroy() PTY cleanup.
Fixed Linting: Corrected a missing await in shellExecutionService.test.ts that surfaced after making the termination logic asynchronous.
Full Validation Pass: Verified locally that packages/core passes all unit tests, linting, and typecheck.
Zero Regressions: Re-confirmed that the process kill order remains correct (Group signal -> PTY termination) to prevent orphaned processes.

The PR is now fully up-to-date with main and should be ready for final approval.

CC: @spencer426 @mrpmohiburrahman

…am merge The upstream merge introduced a new spawn options object to spawnAsync which caused the taskkill assertion to fail because it now passes options as a third argument instead of none. Update the assertion to match the new call signature. Also add missing await to ShellExecutionService.kill() call in the PTY destroy test to ensure proper async execution.

manavmax · 2026-03-10T22:59:10Z

Hi @spencer426 @mrpmohiburrahman , Cleaned up all the issue.
Could you please rerun the CI checks and review the PR?
Thank you!

spencer426 · 2026-03-11T00:13:53Z

@manavmax Looks like there's still a linting issue.

Fix indentation in union type definitions and empty callback formatting to pass CI prettier checks.

Resolved merge conflict in AppContainer.tsx by adopting the upstream Promise.all pattern for killing background shells.

manavmax · 2026-03-11T08:25:24Z

Hi @spencer426, all issues have been addressed:

Merge conflict resolved:
packages/cli/src/ui/AppContainer.tsx— Adopted the upstream Promise.all pattern for killing background shells.

Previous fixes (from earlier commits):
Test assertion fix — Updated shellExecutionService.test.ts to match the new spawnAsync call signature after the upstream merge.
Prettier formatting fix — Fixed indentation and empty callback formatting in shellExecutionService.ts.

Verification:
✅ Prettier passes on all files
✅ ESLint passes across all workspaces
✅ Core tests: 68/68 pass (process-utils + shellExecutionService)
✅ CLI tests: 107/107 pass (AppContainer)
✅ No merge conflicts remain

Ready for review. Could you please rerun the CI checks? Thank you for your patience!

spencer426

LGTM

manavmax · 2026-03-11T15:57:32Z

Thank you @spencer426 for guiding me throughout all of this.

…TY abort (google-gemini#21124) Co-authored-by: Spencer <spencertang@google.com>

) Co-authored-by: Spencer <spencertang@google.com>

…TY abort (google-gemini#21124) Co-authored-by: Spencer <spencertang@google.com>

manavmax requested a review from a team as a code owner March 4, 2026 17:21

gemini-code-assist Bot reviewed Mar 4, 2026

View reviewed changes

Comment thread packages/core/src/utils/process-utils.test.ts Outdated

gemini-cli Bot added priority/p2 Important but can be addressed in a future release. area/core Issues related to User Interface, OS Support, Core Functionality help wanted We will accept PRs from all issues marked as "help wanted". Thanks for your support! labels Mar 4, 2026

gemini-code-assist Bot reviewed Mar 6, 2026

View reviewed changes

spencer426 self-requested a review March 7, 2026 04:56

mrpmohiburrahman reviewed Mar 7, 2026

View reviewed changes

Comment thread packages/core/src/utils/process-utils.ts Outdated

github-actions Bot mentioned this pull request Mar 8, 2026

📊 AI CLI 工具社区动态日报 2026-03-08 duanyytop/agents-radar#95

Closed