Skip to content

feat: replace tree-kill with shared implementation#600

Merged
exKAZUu merged 18 commits intomainfrom
feat/replace-tree-kill-with-internal
Mar 1, 2026
Merged

feat: replace tree-kill with shared implementation#600
exKAZUu merged 18 commits intomainfrom
feat/replace-tree-kill-with-internal

Conversation

@exKAZUu
Copy link
Member

@exKAZUu exKAZUu commented Mar 1, 2026

Summary

  • add a native process-tree killer implementation in shared-lib-node
  • replace tree-kill package usage in shared-lib-node and wb
  • add a new wb tree-kill <pid> [signal] command
  • add real process-kill tests without mocks in both packages

Verification

  • yarn check-for-ai
  • yarn workspace @willbooster/shared-lib-node test test/treeKill.test.ts
  • yarn workspace @willbooster/wb test test/treeKill.test.ts

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, 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 centralizes process tree killing logic within the monorepo by implementing a custom, cross-platform treeKill utility in shared-lib-node. This change eliminates an external dependency, improves control over process management, and integrates the new functionality into the wb CLI, providing a consistent and robust way to terminate processes and their descendants across the project.

Highlights

  • Native Process Tree Killer: A new native process-tree killer implementation has been added to the shared-lib-node package, replacing the external tree-kill dependency.
  • Dependency Removal: The external tree-kill package has been removed from both shared-lib-node and wb packages, reducing external dependencies.
  • New CLI Command: A new wb tree-kill <pid> [signal] command has been introduced, allowing direct process tree termination via the wb CLI.
  • Enhanced Testing: Real process-kill tests, without mocks, have been added to both shared-lib-node and wb to ensure the reliability of the new tree-killing functionality.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • packages/shared-lib-node/package.json
    • Removed the tree-kill dependency.
  • packages/shared-lib-node/src/index.ts
    • Exported the new treeKill function.
  • packages/shared-lib-node/src/spawn.ts
    • Updated the import of treeKill to use the internal implementation.
    • Modified the treeKill call to void and added a .catch block, reflecting its new asynchronous nature.
  • packages/shared-lib-node/src/treeKill.ts
    • Added a new file containing the native treeKill implementation, supporting both Windows (via taskkill) and Unix-like systems (via ps and process.kill).
    • Included helper functions for collecting descendant PIDs, running commands, and handling process-not-found errors.
  • packages/shared-lib-node/test/treeKill.test.ts
    • Added a new test file with real process-kill tests to verify the treeKill functionality, including tests for parent and descendant process termination and handling of non-existent PIDs.
  • packages/wb/package.json
    • Removed the tree-kill dependency.
  • packages/wb/src/commands/treeKill.ts
    • Added a new command file defining the wb tree-kill command, which utilizes the treeKill function from shared-lib-node.
  • packages/wb/src/index.ts
    • Updated the import of treeKill to use the internal implementation from shared-lib-node.
    • Integrated the new treeKillCommand into the yargs CLI builder.
    • Modified the treeKill call in the signal handler to void to correctly handle the asynchronous function.
  • packages/wb/test/treeKill.test.ts
    • Added a new test file to verify the wb tree-kill command's functionality, ensuring it correctly terminates process trees.
  • yarn.lock
    • Removed entries related to the tree-kill package.
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.

Footnotes

  1. 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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、tree-kill 依存関係をネイティブ実装に置き換えるもので、依存関係を減らし、より多くの制御を可能にする素晴らしい改善です。新しい実装は、WindowsとUnixライクシステム向けのプラットフォーム固有のロジックでうまく構成されています。wb tree-kill コマンドとそれに対応するテストの追加も、価値のある貢献です。コードの明確さと堅牢性を向上させるために、いくつかの提案があります。

exKAZUu and others added 3 commits March 1, 2026 19:11
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the external tree-kill dependency with a custom, in-house treeKill implementation within the shared-lib-node package, which is then integrated into the spawnAsync utility and exposed as a new wb tree-kill command. The custom treeKill function handles process tree termination for both Windows and other operating systems, including error handling for non-existent processes. Review comments emphasized the importance of awaiting the treeKill function in signal handlers to ensure proper cleanup before process.exit() is called, and suggested a more concise approach for managing child process PIDs within the collectDescendantPids function.

exKAZUu and others added 2 commits March 1, 2026 21:12
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、外部のtree-killパッケージをshared-lib-node内のネイティブ実装に置き換えるものです。これにより、依存関係が削減され、プロセスの終了処理が改善されます。新しいtreeKillの実装は、WindowsとUnix系システムの両方に対応しており、堅牢なテストも追加されています。また、wb tree-killという新しいCLIコマンドも導入されています。全体的に、これは素晴らしい改善ですが、いくつかの点でさらに改善できる可能性があります。特に、テストコードの重複や、エラーハンドリングの一貫性についてコメントしました。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、tree-kill パッケージを独自の共有実装に置き換えるもので、全体的に良い変更だと思います。これにより、外部依存が減り、コードベースの制御性が向上します。新しい treeKill の実装とテストは堅牢に見えますが、パフォーマンスと堅牢性をさらに向上させるための提案をいくつか追加しました。

exKAZUu and others added 2 commits March 1, 2026 21:36
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、tree-kill パッケージを独自の共有実装に置き換えるもので、素晴らしい改善です。コードは全体的に非常によく書かれており、特にWindowsとUnix系OSの両方に対応したネイティブな実装や、実際のプロセスを使った堅牢なテストが追加されている点は高く評価できます。いくつかの軽微な改善点(コードの重複排除や互換性の向上)を提案しますが、全体としては質の高い変更です。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、tree-kill 依存関係を独自のネイティブ実装に置き換えるもので、依存関係を減らすための素晴らしい改善です。実装は概ね堅牢で、Windowsを別途処理したり、外部コマンドにタイムアウトを追加したりするなど、優れたプラクティスが採用されています。しかし、プロセスのキル順序に重大な問題が1点と、保守性と型安全性を向上させるための中程度の問題が2点見つかりました。これらの点についてコメントしています。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

tree-killパッケージをカスタム実装に置き換えることで、依存関係を削減するという素晴らしい改善ですね。新しい実装は、Windowsとその他のプラットフォームを個別に処理するなど、よく構造化されています。また、新しいwb tree-killコマンドの追加や、包括的なテストも評価できます。
レビューでは、コードの可読性向上とエラーハンドリングの堅牢性に関するいくつかの提案をさせていただきました。全体として、これは素晴らしい貢献です。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、外部の tree-kill パッケージを独自の共有実装に置き換えるものです。これにより、新しい wb tree-kill コマンドが追加され、テストも拡充されています。全体的に良い変更ですが、いくつか改善点があります。主な懸念点は、新しい treeKill の実装が同期的であり、イベントループをブロックする可能性があることです。これを非同期に修正することを強く推奨します。また、コードの可読性を向上させるための小さなリファクタリングも提案しました。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、tree-kill 依存関係をネイティブ実装に置き換えるもので、外部依存を減らす素晴らしい取り組みです。実装はWindowsと非Windowsプラットフォームの両方をカバーし、包括的なテストも含まれており、よく構造化されています。また、wb tree-kill コマンドの追加も便利なユーティリティです。spawn.tspackages/wb/src/index.ts でシャットダウン時のエラー処理や再入防止の変更が加えられており、堅牢性が向上しています。新しい treeKill.ts の実装について、コードの明確性と正確性を向上させるための提案がいくつかあります。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、tree-kill パッケージを独自のネイティブ実装に置き換えるもので、依存関係を減らすための良い変更です。実装はWindowsとUnix系システムをカバーしており、クロスプラットフォームに対応しています。また、新しい wb tree-kill コマンドと充実したテストが追加されています。コードは全体的に質が高いですが、treeKill.ts の実装において、コードの可読性をさらに向上させるための提案が1点あります。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

Declining the async-conversion suggestion for in this PR. We intentionally keep synchronous in this codebase to keep shutdown/signal paths deterministic and avoid reintroducing Promise-handling complexity at call sites ( cleanup and signal handling). We already bounded command execution with timeouts, and this PR’s direction is a non-async API. If we later observe real event-loop blocking in production, we can add a separate API without changing current behavior.

@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

Declining the async-conversion suggestion for treeKill in this PR.

We intentionally keep treeKill synchronous in this codebase to keep shutdown/signal paths deterministic and avoid reintroducing Promise-handling complexity at call sites (spawn cleanup and wb signal handling). We already bound command execution with timeouts, and this PR direction is a non-async API.

If we later observe real event-loop blocking in production, we can add a separate treeKillAsync API without changing current behavior.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、tree-killパッケージをネイティブ実装に置き換えるもので、依存関係の削減と制御の向上という点で素晴らしい改善です。新しい実装はよく構造化されており、WindowsやPOSIXライクなシステムなど、さまざまなプラットフォームをカバーしています。また、堅牢なエラーハンドリングと包括的なテストも含まれています。コードの明確性を向上させるために、冗長なチェックを削除する軽微な提案が1点あります。全体として、これは質の高い貢献です。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、tree-kill パッケージを shared-lib-node 内のネイティブ実装に置き換えるものです。この新しい実装は shared-lib-nodewb の両方で利用されます。また、新しい wb tree-kill コマンドと、それを検証するための堅牢なテストが追加されています。全体として、プロセスキル操作のエラーハンドリングが追加されるなど、堅牢性が向上しており、よく構造化された変更です。コードの正確性と可読性の観点から、2点改善提案があります。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu
Copy link
Member Author

exKAZUu commented Mar 1, 2026

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

このプルリクエストは、外部パッケージ tree-kill を独自の共有実装に置き換えるもので、素晴らしい改善です。新しい treeKill の実装は、WindowsとPOSIX系OSの両方をサポートしており、堅牢なエラーハンドリングも含まれています。また、新しい wb tree-kill コマンドの追加や、モックを使わない実践的なテストが追加されている点も高く評価できます。コード全体として品質が高く、1点だけ可読性向上のための軽微なリファクタリング提案があります。

Co-authored-by: WillBooster (Codex CLI) <agent@willbooster.com>
@exKAZUu exKAZUu enabled auto-merge (squash) March 1, 2026 15:26
@exKAZUu exKAZUu merged commit 5b86762 into main Mar 1, 2026
7 checks passed
@exKAZUu exKAZUu deleted the feat/replace-tree-kill-with-internal branch March 1, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant