Cleanup zombie processes for child process client

[humbertoyusta]

Ensure child processes kill all subprocesses on drop (for transport-child-process)

Motivation and Context

Starting a tokio child processes, in several mcp servers, like using uv some-mcp-server in MacOS, will create subprocesses that will not be cleaned when the first process is killed with the .kill_on_drop(true)

Using a JobObject in Windows and a Process Group in Unix, killing the child process will also clean all subprocesses, avoiding resource leaking

How Has This Been Tested?

We (refact.ai) use this library for integrating mcp servers with our client, we recently switched from another library, so it's not in production yet.

Scenarios about starting servers like with uv were tested to avoid resource leaking, also fakeish scenarios like sh -c "npx some-mcp-server" were tested, causing subprocess leaking before, not now

Breaking Changes

TokioChildProcess::new(command) takes now tokio::process::Command entirely, instead of &mut tokio::process::Command

• The reasion for this, is that the wrapping library used for process group and job object, requires taking the whole command instead of &mut ref
• Example tests have been updated accordingly

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• [?] My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not needed)
• I have added or updated documentation as needed

[4t145]

Thank you for PR, I will have a look on this!

[4t145]

Looks good!

Is there a way to provide a chained call api to create a cmd? Just want to give out a more easy to use interface.

And it will be great if you can update the document like README.

[humbertoyusta]

Thanks for the review,

I updated README and the other example that was missing.

I couldn't find a way to provide an inline chained way, due to the fact that most std/tokio::process::Command methods like args(), envs() take and return &mut Command instead of Command, so if you chain it in one line, there's no way to get the Command again.

I checked what process-wrap does for this, but they just use command in non-inline way, or create their own structs directly.

TokioChildProcess::new({
let mut cmd = tokio::process::Command::new("npx");
cmd.arg("mcp-server-git")
cmd
})

will workfor inline way, but this is just the same as doing it before, I coulnd't find a clean way to chain it inline

[Copilot]

Pull Request Overview

This PR updates the TokioChildProcess API to consume a full tokio::process::Command (instead of a mutable reference) and integrates the process_wrap crate to ensure that child processes cleanup their subprocesses (using a JobObject on Windows and a Process Group on Unix). The changes span multiple examples, tests, and the core transport implementation.

• Updated TokioChildProcess::new to take ownership of the command and chain additional configurations.
• Adjusted various examples and tests to use the new command initialization pattern.
• Introduced a ChildWithCleanup wrapper that utilizes process_wrap for proper child process termination.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
examples/simple-chat-client/src/config.rs	Updated command creation to comply with the new API
examples/simple-chat-client/Cargo.toml	Changed rmcp dependency from git to local path
examples/rig-integration/src/config/mcp.rs	Updated command initialization per new API
examples/clients/src/std_io.rs	Refactored command creation and updated commented examples
examples/clients/src/everything_stdio.rs	Modified command creation for consistency
examples/clients/src/collection.rs	Updated command creation; note change in argument value
docs/readme/README.zh-cn.md	Refactored example to show new command initialization
crates/rmcp/tests/test_with_python.rs	Updated test command creation
crates/rmcp/tests/test_with_js.rs	Updated test command creation
crates/rmcp/src/transport/child_process.rs	Refactored transport module to wrap child process with cleanup
crates/rmcp/src/lib.rs	Updated documentation to reflect the new API
crates/rmcp/Cargo.toml	Added process-wrap dependency and updated transport features
README.md	Updated quick-start example to use new command pattern

Comments suppressed due to low confidence (1)

examples/clients/src/collection.rs:22

• [nitpick] Verify that changing the argument from 'mcp-server-git' to 'mcp-client-git' is intentional and consistent with the intended behavior of the example.

cmd.arg("mcp-client-git");

[Copilot]

[nitpick] Consider logging or handling errors from 'start_kill()' in the Drop implementation to aid in debugging potential cleanup issues.

[4t145]

How about create a command wrapper in rmcp? It could be something like this.

#[derive(Debug)]pubstructCommand{tokio: tokio::process::Command,}implFrom<tokio::process::Command>forCommand{fnfrom(tokio: tokio::process::Command) -> Self{Self{ tokio }}}implCommand{pubfnnew<S:AsRef<OsStr>>(program:S) -> Command{Self::from(tokio::process::Command::new(program))}pubfnarg<S:AsRef<OsStr>>(mutself,arg:S) -> Command{self.tokio.arg(arg);self}pubfnargs<I,S>(mutself,args:I) -> CommandwhereI:IntoIterator<Item = S>,S:AsRef<OsStr>,{self.tokio.args(args);self}pubfnenv<K,V>(mutself,key:K,val:V) -> CommandwhereK:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.env(key, val);self}pubfnenvs<I,K,V>(mutself,vars:I) -> CommandwhereI:IntoIterator<Item = (K,V)>,K:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.envs(vars);self}pubfninto_child_process(self) -> std::io::Result<TokioChildProcess>{TokioChildProcess::new(self.tokio)}pubfninto_tokio(self) -> tokio::process::Command{self.tokio}pubfnas_tokio_mut(&mutself) -> &mut tokio::process::Command{&mutself.tokio}pubfnas_tokio(&self) -> &tokio::process::Command{&self.tokio}}

And we can use it like this

let transport = Command::new("node").arg("tests/test_with_js/server.js").into_child_process()?;let client = ().serve(transport).await?;

Will it be confusing for there are too many Command type?

[jokemanfire]

How about add a event listener , ant use wait_pid to recycle the children process ?I think we only need to focus on the possibility of zombie like child processes during the runtime of the main process.

[humbertoyusta]

One option for inline could be to modify examples to do like:

TokioChildProcess::new(tokio::process::Command::new("node").map(
|cmd| { cmd.arg("tests/test_with_js/client.js"); cmd }))

Another could be for people who use tap crate, using tap_mut, but it's more or less the same, we could provide that trait to command, but it would need to be imported and it's not much less verbose

use rmcp::transport::child_process::CommandTapMut;TokioChildProcess::new(tokio::process::Command::new("node").tap_mut(
|cmd| { cmd.arg("tests/test_with_js/client.js");}))

Or we could do a wrapper to command like proposed before, but there will be several Command structs then, not sure which is the best approach.

[4t145]

@humbertoyusta The first approach looks great. I am okay with it.

[humbertoyusta]

Actually .map() didn't work, I thought Command::new() was giving result but it's not, so I needed to add a trait for tokio::process::Command.

I still think it's better than a full Command wrapper, but if you think otherwise let me know.

[4t145]

@humbertoyusta I am ok with the ext trait.

Looks there's confict and ci didn't pass. So could you merge it with the main branch, for I just made a lot of changes, and fix the formatting and test ci? Thanks for your patience!

[4t145]

It's okey with the ci message, I can rewrite it when merge.

[4t145]

And thanks for your work!

[humbertoyusta]

Ensure child processes kill all subprocesses on drop (for transport-child-process)

Motivation and Context

Starting a tokio child processes, in several mcp servers, like using uv some-mcp-server in MacOS, will create subprocesses that will not be cleaned when the first process is killed with the .kill_on_drop(true)

Using a JobObject in Windows and a Process Group in Unix, killing the child process will also clean all subprocesses, avoiding resource leaking

How Has This Been Tested?

We (refact.ai) use this library for integrating mcp servers with our client, we recently switched from another library, so it's not in production yet.

Scenarios about starting servers like with uv were tested to avoid resource leaking, also fakeish scenarios like sh -c "npx some-mcp-server" were tested, causing subprocess leaking before, not now

Breaking Changes

TokioChildProcess::new(command) takes now tokio::process::Command entirely, instead of &mut tokio::process::Command

• The reasion for this, is that the wrapping library used for process group and job object, requires taking the whole command instead of &mut ref
• Example tests have been updated accordingly

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• [?] My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not needed)
• I have added or updated documentation as needed

[4t145]

Thank you for PR, I will have a look on this!

[4t145]

Looks good!

Is there a way to provide a chained call api to create a cmd? Just want to give out a more easy to use interface.

And it will be great if you can update the document like README.

[humbertoyusta]

Thanks for the review,

I updated README and the other example that was missing.

I couldn't find a way to provide an inline chained way, due to the fact that most std/tokio::process::Command methods like args(), envs() take and return &mut Command instead of Command, so if you chain it in one line, there's no way to get the Command again.

I checked what process-wrap does for this, but they just use command in non-inline way, or create their own structs directly.

TokioChildProcess::new({
let mut cmd = tokio::process::Command::new("npx");
cmd.arg("mcp-server-git")
cmd
})

will workfor inline way, but this is just the same as doing it before, I coulnd't find a clean way to chain it inline

[Copilot]

Pull Request Overview

This PR updates the TokioChildProcess API to consume a full tokio::process::Command (instead of a mutable reference) and integrates the process_wrap crate to ensure that child processes cleanup their subprocesses (using a JobObject on Windows and a Process Group on Unix). The changes span multiple examples, tests, and the core transport implementation.

• Updated TokioChildProcess::new to take ownership of the command and chain additional configurations.
• Adjusted various examples and tests to use the new command initialization pattern.
• Introduced a ChildWithCleanup wrapper that utilizes process_wrap for proper child process termination.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
examples/simple-chat-client/src/config.rs	Updated command creation to comply with the new API
examples/simple-chat-client/Cargo.toml	Changed rmcp dependency from git to local path
examples/rig-integration/src/config/mcp.rs	Updated command initialization per new API
examples/clients/src/std_io.rs	Refactored command creation and updated commented examples
examples/clients/src/everything_stdio.rs	Modified command creation for consistency
examples/clients/src/collection.rs	Updated command creation; note change in argument value
docs/readme/README.zh-cn.md	Refactored example to show new command initialization
crates/rmcp/tests/test_with_python.rs	Updated test command creation
crates/rmcp/tests/test_with_js.rs	Updated test command creation
crates/rmcp/src/transport/child_process.rs	Refactored transport module to wrap child process with cleanup
crates/rmcp/src/lib.rs	Updated documentation to reflect the new API
crates/rmcp/Cargo.toml	Added process-wrap dependency and updated transport features
README.md	Updated quick-start example to use new command pattern

Comments suppressed due to low confidence (1)

examples/clients/src/collection.rs:22

• [nitpick] Verify that changing the argument from 'mcp-server-git' to 'mcp-client-git' is intentional and consistent with the intended behavior of the example.

cmd.arg("mcp-client-git");

[Copilot]

[nitpick] Consider logging or handling errors from 'start_kill()' in the Drop implementation to aid in debugging potential cleanup issues.

[4t145]

How about create a command wrapper in rmcp? It could be something like this.

#[derive(Debug)]pubstructCommand{tokio: tokio::process::Command,}implFrom<tokio::process::Command>forCommand{fnfrom(tokio: tokio::process::Command) -> Self{Self{ tokio }}}implCommand{pubfnnew<S:AsRef<OsStr>>(program:S) -> Command{Self::from(tokio::process::Command::new(program))}pubfnarg<S:AsRef<OsStr>>(mutself,arg:S) -> Command{self.tokio.arg(arg);self}pubfnargs<I,S>(mutself,args:I) -> CommandwhereI:IntoIterator<Item = S>,S:AsRef<OsStr>,{self.tokio.args(args);self}pubfnenv<K,V>(mutself,key:K,val:V) -> CommandwhereK:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.env(key, val);self}pubfnenvs<I,K,V>(mutself,vars:I) -> CommandwhereI:IntoIterator<Item = (K,V)>,K:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.envs(vars);self}pubfninto_child_process(self) -> std::io::Result<TokioChildProcess>{TokioChildProcess::new(self.tokio)}pubfninto_tokio(self) -> tokio::process::Command{self.tokio}pubfnas_tokio_mut(&mutself) -> &mut tokio::process::Command{&mutself.tokio}pubfnas_tokio(&self) -> &tokio::process::Command{&self.tokio}}

And we can use it like this

let transport = Command::new("node").arg("tests/test_with_js/server.js").into_child_process()?;let client = ().serve(transport).await?;

Will it be confusing for there are too many Command type?

[jokemanfire]

How about add a event listener , ant use wait_pid to recycle the children process ?I think we only need to focus on the possibility of zombie like child processes during the runtime of the main process.

[humbertoyusta]

One option for inline could be to modify examples to do like:

TokioChildProcess::new(tokio::process::Command::new("node").map(
|cmd| { cmd.arg("tests/test_with_js/client.js"); cmd }))

Another could be for people who use tap crate, using tap_mut, but it's more or less the same, we could provide that trait to command, but it would need to be imported and it's not much less verbose

use rmcp::transport::child_process::CommandTapMut;TokioChildProcess::new(tokio::process::Command::new("node").tap_mut(
|cmd| { cmd.arg("tests/test_with_js/client.js");}))

Or we could do a wrapper to command like proposed before, but there will be several Command structs then, not sure which is the best approach.

[4t145]

@humbertoyusta The first approach looks great. I am okay with it.

[humbertoyusta]

Actually .map() didn't work, I thought Command::new() was giving result but it's not, so I needed to add a trait for tokio::process::Command.

I still think it's better than a full Command wrapper, but if you think otherwise let me know.

[4t145]

@humbertoyusta I am ok with the ext trait.

Looks there's confict and ci didn't pass. So could you merge it with the main branch, for I just made a lot of changes, and fix the formatting and test ci? Thanks for your patience!

[4t145]

It's okey with the ci message, I can rewrite it when merge.

[4t145]

And thanks for your work!

[humbertoyusta]

Ensure child processes kill all subprocesses on drop (for transport-child-process)

Motivation and Context

Starting a tokio child processes, in several mcp servers, like using uv some-mcp-server in MacOS, will create subprocesses that will not be cleaned when the first process is killed with the .kill_on_drop(true)

Using a JobObject in Windows and a Process Group in Unix, killing the child process will also clean all subprocesses, avoiding resource leaking

How Has This Been Tested?

We (refact.ai) use this library for integrating mcp servers with our client, we recently switched from another library, so it's not in production yet.

Scenarios about starting servers like with uv were tested to avoid resource leaking, also fakeish scenarios like sh -c "npx some-mcp-server" were tested, causing subprocess leaking before, not now

Breaking Changes

TokioChildProcess::new(command) takes now tokio::process::Command entirely, instead of &mut tokio::process::Command

• The reasion for this, is that the wrapping library used for process group and job object, requires taking the whole command instead of &mut ref
• Example tests have been updated accordingly

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• [?] My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not needed)
• I have added or updated documentation as needed

[4t145]

Thank you for PR, I will have a look on this!

[4t145]

Looks good!

Is there a way to provide a chained call api to create a cmd? Just want to give out a more easy to use interface.

And it will be great if you can update the document like README.

[humbertoyusta]

Thanks for the review,

I updated README and the other example that was missing.

I couldn't find a way to provide an inline chained way, due to the fact that most std/tokio::process::Command methods like args(), envs() take and return &mut Command instead of Command, so if you chain it in one line, there's no way to get the Command again.

I checked what process-wrap does for this, but they just use command in non-inline way, or create their own structs directly.

TokioChildProcess::new({
let mut cmd = tokio::process::Command::new("npx");
cmd.arg("mcp-server-git")
cmd
})

will workfor inline way, but this is just the same as doing it before, I coulnd't find a clean way to chain it inline

[Copilot]

Pull Request Overview

This PR updates the TokioChildProcess API to consume a full tokio::process::Command (instead of a mutable reference) and integrates the process_wrap crate to ensure that child processes cleanup their subprocesses (using a JobObject on Windows and a Process Group on Unix). The changes span multiple examples, tests, and the core transport implementation.

• Updated TokioChildProcess::new to take ownership of the command and chain additional configurations.
• Adjusted various examples and tests to use the new command initialization pattern.
• Introduced a ChildWithCleanup wrapper that utilizes process_wrap for proper child process termination.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
examples/simple-chat-client/src/config.rs	Updated command creation to comply with the new API
examples/simple-chat-client/Cargo.toml	Changed rmcp dependency from git to local path
examples/rig-integration/src/config/mcp.rs	Updated command initialization per new API
examples/clients/src/std_io.rs	Refactored command creation and updated commented examples
examples/clients/src/everything_stdio.rs	Modified command creation for consistency
examples/clients/src/collection.rs	Updated command creation; note change in argument value
docs/readme/README.zh-cn.md	Refactored example to show new command initialization
crates/rmcp/tests/test_with_python.rs	Updated test command creation
crates/rmcp/tests/test_with_js.rs	Updated test command creation
crates/rmcp/src/transport/child_process.rs	Refactored transport module to wrap child process with cleanup
crates/rmcp/src/lib.rs	Updated documentation to reflect the new API
crates/rmcp/Cargo.toml	Added process-wrap dependency and updated transport features
README.md	Updated quick-start example to use new command pattern

Comments suppressed due to low confidence (1)

examples/clients/src/collection.rs:22

• [nitpick] Verify that changing the argument from 'mcp-server-git' to 'mcp-client-git' is intentional and consistent with the intended behavior of the example.

cmd.arg("mcp-client-git");

[Copilot]

[nitpick] Consider logging or handling errors from 'start_kill()' in the Drop implementation to aid in debugging potential cleanup issues.

[4t145]

How about create a command wrapper in rmcp? It could be something like this.

#[derive(Debug)]pubstructCommand{tokio: tokio::process::Command,}implFrom<tokio::process::Command>forCommand{fnfrom(tokio: tokio::process::Command) -> Self{Self{ tokio }}}implCommand{pubfnnew<S:AsRef<OsStr>>(program:S) -> Command{Self::from(tokio::process::Command::new(program))}pubfnarg<S:AsRef<OsStr>>(mutself,arg:S) -> Command{self.tokio.arg(arg);self}pubfnargs<I,S>(mutself,args:I) -> CommandwhereI:IntoIterator<Item = S>,S:AsRef<OsStr>,{self.tokio.args(args);self}pubfnenv<K,V>(mutself,key:K,val:V) -> CommandwhereK:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.env(key, val);self}pubfnenvs<I,K,V>(mutself,vars:I) -> CommandwhereI:IntoIterator<Item = (K,V)>,K:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.envs(vars);self}pubfninto_child_process(self) -> std::io::Result<TokioChildProcess>{TokioChildProcess::new(self.tokio)}pubfninto_tokio(self) -> tokio::process::Command{self.tokio}pubfnas_tokio_mut(&mutself) -> &mut tokio::process::Command{&mutself.tokio}pubfnas_tokio(&self) -> &tokio::process::Command{&self.tokio}}

And we can use it like this

let transport = Command::new("node").arg("tests/test_with_js/server.js").into_child_process()?;let client = ().serve(transport).await?;

Will it be confusing for there are too many Command type?

[jokemanfire]

How about add a event listener , ant use wait_pid to recycle the children process ?I think we only need to focus on the possibility of zombie like child processes during the runtime of the main process.

[humbertoyusta]

One option for inline could be to modify examples to do like:

TokioChildProcess::new(tokio::process::Command::new("node").map(
|cmd| { cmd.arg("tests/test_with_js/client.js"); cmd }))

Another could be for people who use tap crate, using tap_mut, but it's more or less the same, we could provide that trait to command, but it would need to be imported and it's not much less verbose

use rmcp::transport::child_process::CommandTapMut;TokioChildProcess::new(tokio::process::Command::new("node").tap_mut(
|cmd| { cmd.arg("tests/test_with_js/client.js");}))

Or we could do a wrapper to command like proposed before, but there will be several Command structs then, not sure which is the best approach.

[4t145]

@humbertoyusta The first approach looks great. I am okay with it.

[humbertoyusta]

Actually .map() didn't work, I thought Command::new() was giving result but it's not, so I needed to add a trait for tokio::process::Command.

I still think it's better than a full Command wrapper, but if you think otherwise let me know.

[4t145]

@humbertoyusta I am ok with the ext trait.

Looks there's confict and ci didn't pass. So could you merge it with the main branch, for I just made a lot of changes, and fix the formatting and test ci? Thanks for your patience!

[4t145]

It's okey with the ci message, I can rewrite it when merge.

[4t145]

And thanks for your work!

[humbertoyusta]

Ensure child processes kill all subprocesses on drop (for transport-child-process)

Motivation and Context

Starting a tokio child processes, in several mcp servers, like using uv some-mcp-server in MacOS, will create subprocesses that will not be cleaned when the first process is killed with the .kill_on_drop(true)

Using a JobObject in Windows and a Process Group in Unix, killing the child process will also clean all subprocesses, avoiding resource leaking

How Has This Been Tested?

We (refact.ai) use this library for integrating mcp servers with our client, we recently switched from another library, so it's not in production yet.

Scenarios about starting servers like with uv were tested to avoid resource leaking, also fakeish scenarios like sh -c "npx some-mcp-server" were tested, causing subprocess leaking before, not now

Breaking Changes

TokioChildProcess::new(command) takes now tokio::process::Command entirely, instead of &mut tokio::process::Command

• The reasion for this, is that the wrapping library used for process group and job object, requires taking the whole command instead of &mut ref
• Example tests have been updated accordingly

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• [?] My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not needed)
• I have added or updated documentation as needed

[4t145]

Thank you for PR, I will have a look on this!

[4t145]

Looks good!

Is there a way to provide a chained call api to create a cmd? Just want to give out a more easy to use interface.

And it will be great if you can update the document like README.

[humbertoyusta]

Thanks for the review,

I updated README and the other example that was missing.

I couldn't find a way to provide an inline chained way, due to the fact that most std/tokio::process::Command methods like args(), envs() take and return &mut Command instead of Command, so if you chain it in one line, there's no way to get the Command again.

I checked what process-wrap does for this, but they just use command in non-inline way, or create their own structs directly.

TokioChildProcess::new({
let mut cmd = tokio::process::Command::new("npx");
cmd.arg("mcp-server-git")
cmd
})

will workfor inline way, but this is just the same as doing it before, I coulnd't find a clean way to chain it inline

[Copilot]

Pull Request Overview

This PR updates the TokioChildProcess API to consume a full tokio::process::Command (instead of a mutable reference) and integrates the process_wrap crate to ensure that child processes cleanup their subprocesses (using a JobObject on Windows and a Process Group on Unix). The changes span multiple examples, tests, and the core transport implementation.

• Updated TokioChildProcess::new to take ownership of the command and chain additional configurations.
• Adjusted various examples and tests to use the new command initialization pattern.
• Introduced a ChildWithCleanup wrapper that utilizes process_wrap for proper child process termination.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
examples/simple-chat-client/src/config.rs	Updated command creation to comply with the new API
examples/simple-chat-client/Cargo.toml	Changed rmcp dependency from git to local path
examples/rig-integration/src/config/mcp.rs	Updated command initialization per new API
examples/clients/src/std_io.rs	Refactored command creation and updated commented examples
examples/clients/src/everything_stdio.rs	Modified command creation for consistency
examples/clients/src/collection.rs	Updated command creation; note change in argument value
docs/readme/README.zh-cn.md	Refactored example to show new command initialization
crates/rmcp/tests/test_with_python.rs	Updated test command creation
crates/rmcp/tests/test_with_js.rs	Updated test command creation
crates/rmcp/src/transport/child_process.rs	Refactored transport module to wrap child process with cleanup
crates/rmcp/src/lib.rs	Updated documentation to reflect the new API
crates/rmcp/Cargo.toml	Added process-wrap dependency and updated transport features
README.md	Updated quick-start example to use new command pattern

Comments suppressed due to low confidence (1)

examples/clients/src/collection.rs:22

• [nitpick] Verify that changing the argument from 'mcp-server-git' to 'mcp-client-git' is intentional and consistent with the intended behavior of the example.

cmd.arg("mcp-client-git");

[Copilot]

[nitpick] Consider logging or handling errors from 'start_kill()' in the Drop implementation to aid in debugging potential cleanup issues.

[4t145]

How about create a command wrapper in rmcp? It could be something like this.

#[derive(Debug)]pubstructCommand{tokio: tokio::process::Command,}implFrom<tokio::process::Command>forCommand{fnfrom(tokio: tokio::process::Command) -> Self{Self{ tokio }}}implCommand{pubfnnew<S:AsRef<OsStr>>(program:S) -> Command{Self::from(tokio::process::Command::new(program))}pubfnarg<S:AsRef<OsStr>>(mutself,arg:S) -> Command{self.tokio.arg(arg);self}pubfnargs<I,S>(mutself,args:I) -> CommandwhereI:IntoIterator<Item = S>,S:AsRef<OsStr>,{self.tokio.args(args);self}pubfnenv<K,V>(mutself,key:K,val:V) -> CommandwhereK:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.env(key, val);self}pubfnenvs<I,K,V>(mutself,vars:I) -> CommandwhereI:IntoIterator<Item = (K,V)>,K:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.envs(vars);self}pubfninto_child_process(self) -> std::io::Result<TokioChildProcess>{TokioChildProcess::new(self.tokio)}pubfninto_tokio(self) -> tokio::process::Command{self.tokio}pubfnas_tokio_mut(&mutself) -> &mut tokio::process::Command{&mutself.tokio}pubfnas_tokio(&self) -> &tokio::process::Command{&self.tokio}}

And we can use it like this

let transport = Command::new("node").arg("tests/test_with_js/server.js").into_child_process()?;let client = ().serve(transport).await?;

Will it be confusing for there are too many Command type?

[jokemanfire]

How about add a event listener , ant use wait_pid to recycle the children process ?I think we only need to focus on the possibility of zombie like child processes during the runtime of the main process.

[humbertoyusta]

One option for inline could be to modify examples to do like:

TokioChildProcess::new(tokio::process::Command::new("node").map(
|cmd| { cmd.arg("tests/test_with_js/client.js"); cmd }))

Another could be for people who use tap crate, using tap_mut, but it's more or less the same, we could provide that trait to command, but it would need to be imported and it's not much less verbose

use rmcp::transport::child_process::CommandTapMut;TokioChildProcess::new(tokio::process::Command::new("node").tap_mut(
|cmd| { cmd.arg("tests/test_with_js/client.js");}))

Or we could do a wrapper to command like proposed before, but there will be several Command structs then, not sure which is the best approach.

[4t145]

@humbertoyusta The first approach looks great. I am okay with it.

[humbertoyusta]

Actually .map() didn't work, I thought Command::new() was giving result but it's not, so I needed to add a trait for tokio::process::Command.

I still think it's better than a full Command wrapper, but if you think otherwise let me know.

[4t145]

@humbertoyusta I am ok with the ext trait.

Looks there's confict and ci didn't pass. So could you merge it with the main branch, for I just made a lot of changes, and fix the formatting and test ci? Thanks for your patience!

[4t145]

It's okey with the ci message, I can rewrite it when merge.

[4t145]

And thanks for your work!

[humbertoyusta]

Ensure child processes kill all subprocesses on drop (for transport-child-process)

Motivation and Context

Starting a tokio child processes, in several mcp servers, like using uv some-mcp-server in MacOS, will create subprocesses that will not be cleaned when the first process is killed with the .kill_on_drop(true)

Using a JobObject in Windows and a Process Group in Unix, killing the child process will also clean all subprocesses, avoiding resource leaking

How Has This Been Tested?

We (refact.ai) use this library for integrating mcp servers with our client, we recently switched from another library, so it's not in production yet.

Scenarios about starting servers like with uv were tested to avoid resource leaking, also fakeish scenarios like sh -c "npx some-mcp-server" were tested, causing subprocess leaking before, not now

Breaking Changes

TokioChildProcess::new(command) takes now tokio::process::Command entirely, instead of &mut tokio::process::Command

• The reasion for this, is that the wrapping library used for process group and job object, requires taking the whole command instead of &mut ref
• Example tests have been updated accordingly

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• [?] My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not needed)
• I have added or updated documentation as needed

[4t145]

Thank you for PR, I will have a look on this!

[4t145]

Looks good!

Is there a way to provide a chained call api to create a cmd? Just want to give out a more easy to use interface.

And it will be great if you can update the document like README.

[humbertoyusta]

Thanks for the review,

I updated README and the other example that was missing.

I couldn't find a way to provide an inline chained way, due to the fact that most std/tokio::process::Command methods like args(), envs() take and return &mut Command instead of Command, so if you chain it in one line, there's no way to get the Command again.

I checked what process-wrap does for this, but they just use command in non-inline way, or create their own structs directly.

TokioChildProcess::new({
let mut cmd = tokio::process::Command::new("npx");
cmd.arg("mcp-server-git")
cmd
})

will workfor inline way, but this is just the same as doing it before, I coulnd't find a clean way to chain it inline

[Copilot]

Pull Request Overview

This PR updates the TokioChildProcess API to consume a full tokio::process::Command (instead of a mutable reference) and integrates the process_wrap crate to ensure that child processes cleanup their subprocesses (using a JobObject on Windows and a Process Group on Unix). The changes span multiple examples, tests, and the core transport implementation.

• Updated TokioChildProcess::new to take ownership of the command and chain additional configurations.
• Adjusted various examples and tests to use the new command initialization pattern.
• Introduced a ChildWithCleanup wrapper that utilizes process_wrap for proper child process termination.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
examples/simple-chat-client/src/config.rs	Updated command creation to comply with the new API
examples/simple-chat-client/Cargo.toml	Changed rmcp dependency from git to local path
examples/rig-integration/src/config/mcp.rs	Updated command initialization per new API
examples/clients/src/std_io.rs	Refactored command creation and updated commented examples
examples/clients/src/everything_stdio.rs	Modified command creation for consistency
examples/clients/src/collection.rs	Updated command creation; note change in argument value
docs/readme/README.zh-cn.md	Refactored example to show new command initialization
crates/rmcp/tests/test_with_python.rs	Updated test command creation
crates/rmcp/tests/test_with_js.rs	Updated test command creation
crates/rmcp/src/transport/child_process.rs	Refactored transport module to wrap child process with cleanup
crates/rmcp/src/lib.rs	Updated documentation to reflect the new API
crates/rmcp/Cargo.toml	Added process-wrap dependency and updated transport features
README.md	Updated quick-start example to use new command pattern

Comments suppressed due to low confidence (1)

examples/clients/src/collection.rs:22

• [nitpick] Verify that changing the argument from 'mcp-server-git' to 'mcp-client-git' is intentional and consistent with the intended behavior of the example.

cmd.arg("mcp-client-git");

[Copilot]

[nitpick] Consider logging or handling errors from 'start_kill()' in the Drop implementation to aid in debugging potential cleanup issues.

[4t145]

How about create a command wrapper in rmcp? It could be something like this.

#[derive(Debug)]pubstructCommand{tokio: tokio::process::Command,}implFrom<tokio::process::Command>forCommand{fnfrom(tokio: tokio::process::Command) -> Self{Self{ tokio }}}implCommand{pubfnnew<S:AsRef<OsStr>>(program:S) -> Command{Self::from(tokio::process::Command::new(program))}pubfnarg<S:AsRef<OsStr>>(mutself,arg:S) -> Command{self.tokio.arg(arg);self}pubfnargs<I,S>(mutself,args:I) -> CommandwhereI:IntoIterator<Item = S>,S:AsRef<OsStr>,{self.tokio.args(args);self}pubfnenv<K,V>(mutself,key:K,val:V) -> CommandwhereK:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.env(key, val);self}pubfnenvs<I,K,V>(mutself,vars:I) -> CommandwhereI:IntoIterator<Item = (K,V)>,K:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.envs(vars);self}pubfninto_child_process(self) -> std::io::Result<TokioChildProcess>{TokioChildProcess::new(self.tokio)}pubfninto_tokio(self) -> tokio::process::Command{self.tokio}pubfnas_tokio_mut(&mutself) -> &mut tokio::process::Command{&mutself.tokio}pubfnas_tokio(&self) -> &tokio::process::Command{&self.tokio}}

And we can use it like this

let transport = Command::new("node").arg("tests/test_with_js/server.js").into_child_process()?;let client = ().serve(transport).await?;

Will it be confusing for there are too many Command type?

[jokemanfire]

How about add a event listener , ant use wait_pid to recycle the children process ?I think we only need to focus on the possibility of zombie like child processes during the runtime of the main process.

[humbertoyusta]

One option for inline could be to modify examples to do like:

TokioChildProcess::new(tokio::process::Command::new("node").map(
|cmd| { cmd.arg("tests/test_with_js/client.js"); cmd }))

Another could be for people who use tap crate, using tap_mut, but it's more or less the same, we could provide that trait to command, but it would need to be imported and it's not much less verbose

use rmcp::transport::child_process::CommandTapMut;TokioChildProcess::new(tokio::process::Command::new("node").tap_mut(
|cmd| { cmd.arg("tests/test_with_js/client.js");}))

Or we could do a wrapper to command like proposed before, but there will be several Command structs then, not sure which is the best approach.

[4t145]

@humbertoyusta The first approach looks great. I am okay with it.

[humbertoyusta]

Actually .map() didn't work, I thought Command::new() was giving result but it's not, so I needed to add a trait for tokio::process::Command.

I still think it's better than a full Command wrapper, but if you think otherwise let me know.

[4t145]

@humbertoyusta I am ok with the ext trait.

Looks there's confict and ci didn't pass. So could you merge it with the main branch, for I just made a lot of changes, and fix the formatting and test ci? Thanks for your patience!

[4t145]

It's okey with the ci message, I can rewrite it when merge.

[4t145]

And thanks for your work!

[humbertoyusta]

Ensure child processes kill all subprocesses on drop (for transport-child-process)

Motivation and Context

Starting a tokio child processes, in several mcp servers, like using uv some-mcp-server in MacOS, will create subprocesses that will not be cleaned when the first process is killed with the .kill_on_drop(true)

Using a JobObject in Windows and a Process Group in Unix, killing the child process will also clean all subprocesses, avoiding resource leaking

How Has This Been Tested?

We (refact.ai) use this library for integrating mcp servers with our client, we recently switched from another library, so it's not in production yet.

Scenarios about starting servers like with uv were tested to avoid resource leaking, also fakeish scenarios like sh -c "npx some-mcp-server" were tested, causing subprocess leaking before, not now

Breaking Changes

TokioChildProcess::new(command) takes now tokio::process::Command entirely, instead of &mut tokio::process::Command

• The reasion for this, is that the wrapping library used for process group and job object, requires taking the whole command instead of &mut ref
• Example tests have been updated accordingly

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• [?] My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not needed)
• I have added or updated documentation as needed

[4t145]

Thank you for PR, I will have a look on this!

[4t145]

Looks good!

Is there a way to provide a chained call api to create a cmd? Just want to give out a more easy to use interface.

And it will be great if you can update the document like README.

[humbertoyusta]

Thanks for the review,

I updated README and the other example that was missing.

I couldn't find a way to provide an inline chained way, due to the fact that most std/tokio::process::Command methods like args(), envs() take and return &mut Command instead of Command, so if you chain it in one line, there's no way to get the Command again.

I checked what process-wrap does for this, but they just use command in non-inline way, or create their own structs directly.

TokioChildProcess::new({
let mut cmd = tokio::process::Command::new("npx");
cmd.arg("mcp-server-git")
cmd
})

will workfor inline way, but this is just the same as doing it before, I coulnd't find a clean way to chain it inline

[Copilot]

Pull Request Overview

This PR updates the TokioChildProcess API to consume a full tokio::process::Command (instead of a mutable reference) and integrates the process_wrap crate to ensure that child processes cleanup their subprocesses (using a JobObject on Windows and a Process Group on Unix). The changes span multiple examples, tests, and the core transport implementation.

• Updated TokioChildProcess::new to take ownership of the command and chain additional configurations.
• Adjusted various examples and tests to use the new command initialization pattern.
• Introduced a ChildWithCleanup wrapper that utilizes process_wrap for proper child process termination.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
examples/simple-chat-client/src/config.rs	Updated command creation to comply with the new API
examples/simple-chat-client/Cargo.toml	Changed rmcp dependency from git to local path
examples/rig-integration/src/config/mcp.rs	Updated command initialization per new API
examples/clients/src/std_io.rs	Refactored command creation and updated commented examples
examples/clients/src/everything_stdio.rs	Modified command creation for consistency
examples/clients/src/collection.rs	Updated command creation; note change in argument value
docs/readme/README.zh-cn.md	Refactored example to show new command initialization
crates/rmcp/tests/test_with_python.rs	Updated test command creation
crates/rmcp/tests/test_with_js.rs	Updated test command creation
crates/rmcp/src/transport/child_process.rs	Refactored transport module to wrap child process with cleanup
crates/rmcp/src/lib.rs	Updated documentation to reflect the new API
crates/rmcp/Cargo.toml	Added process-wrap dependency and updated transport features
README.md	Updated quick-start example to use new command pattern

Comments suppressed due to low confidence (1)

examples/clients/src/collection.rs:22

• [nitpick] Verify that changing the argument from 'mcp-server-git' to 'mcp-client-git' is intentional and consistent with the intended behavior of the example.

cmd.arg("mcp-client-git");

[Copilot]

[nitpick] Consider logging or handling errors from 'start_kill()' in the Drop implementation to aid in debugging potential cleanup issues.

[4t145]

How about create a command wrapper in rmcp? It could be something like this.

#[derive(Debug)]pubstructCommand{tokio: tokio::process::Command,}implFrom<tokio::process::Command>forCommand{fnfrom(tokio: tokio::process::Command) -> Self{Self{ tokio }}}implCommand{pubfnnew<S:AsRef<OsStr>>(program:S) -> Command{Self::from(tokio::process::Command::new(program))}pubfnarg<S:AsRef<OsStr>>(mutself,arg:S) -> Command{self.tokio.arg(arg);self}pubfnargs<I,S>(mutself,args:I) -> CommandwhereI:IntoIterator<Item = S>,S:AsRef<OsStr>,{self.tokio.args(args);self}pubfnenv<K,V>(mutself,key:K,val:V) -> CommandwhereK:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.env(key, val);self}pubfnenvs<I,K,V>(mutself,vars:I) -> CommandwhereI:IntoIterator<Item = (K,V)>,K:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.envs(vars);self}pubfninto_child_process(self) -> std::io::Result<TokioChildProcess>{TokioChildProcess::new(self.tokio)}pubfninto_tokio(self) -> tokio::process::Command{self.tokio}pubfnas_tokio_mut(&mutself) -> &mut tokio::process::Command{&mutself.tokio}pubfnas_tokio(&self) -> &tokio::process::Command{&self.tokio}}

And we can use it like this

let transport = Command::new("node").arg("tests/test_with_js/server.js").into_child_process()?;let client = ().serve(transport).await?;

Will it be confusing for there are too many Command type?

[jokemanfire]

How about add a event listener , ant use wait_pid to recycle the children process ?I think we only need to focus on the possibility of zombie like child processes during the runtime of the main process.

[humbertoyusta]

One option for inline could be to modify examples to do like:

TokioChildProcess::new(tokio::process::Command::new("node").map(
|cmd| { cmd.arg("tests/test_with_js/client.js"); cmd }))

Another could be for people who use tap crate, using tap_mut, but it's more or less the same, we could provide that trait to command, but it would need to be imported and it's not much less verbose

use rmcp::transport::child_process::CommandTapMut;TokioChildProcess::new(tokio::process::Command::new("node").tap_mut(
|cmd| { cmd.arg("tests/test_with_js/client.js");}))

Or we could do a wrapper to command like proposed before, but there will be several Command structs then, not sure which is the best approach.

[4t145]

@humbertoyusta The first approach looks great. I am okay with it.

[humbertoyusta]

Actually .map() didn't work, I thought Command::new() was giving result but it's not, so I needed to add a trait for tokio::process::Command.

I still think it's better than a full Command wrapper, but if you think otherwise let me know.

[4t145]

@humbertoyusta I am ok with the ext trait.

Looks there's confict and ci didn't pass. So could you merge it with the main branch, for I just made a lot of changes, and fix the formatting and test ci? Thanks for your patience!

[4t145]

It's okey with the ci message, I can rewrite it when merge.

[4t145]

And thanks for your work!

[humbertoyusta]

Ensure child processes kill all subprocesses on drop (for transport-child-process)

Motivation and Context

Starting a tokio child processes, in several mcp servers, like using uv some-mcp-server in MacOS, will create subprocesses that will not be cleaned when the first process is killed with the .kill_on_drop(true)

Using a JobObject in Windows and a Process Group in Unix, killing the child process will also clean all subprocesses, avoiding resource leaking

How Has This Been Tested?

We (refact.ai) use this library for integrating mcp servers with our client, we recently switched from another library, so it's not in production yet.

Scenarios about starting servers like with uv were tested to avoid resource leaking, also fakeish scenarios like sh -c "npx some-mcp-server" were tested, causing subprocess leaking before, not now

Breaking Changes

TokioChildProcess::new(command) takes now tokio::process::Command entirely, instead of &mut tokio::process::Command

• The reasion for this, is that the wrapping library used for process group and job object, requires taking the whole command instead of &mut ref
• Example tests have been updated accordingly

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• [?] My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not needed)
• I have added or updated documentation as needed

[4t145]

Thank you for PR, I will have a look on this!

[4t145]

Looks good!

Is there a way to provide a chained call api to create a cmd? Just want to give out a more easy to use interface.

And it will be great if you can update the document like README.

[humbertoyusta]

Thanks for the review,

I updated README and the other example that was missing.

I couldn't find a way to provide an inline chained way, due to the fact that most std/tokio::process::Command methods like args(), envs() take and return &mut Command instead of Command, so if you chain it in one line, there's no way to get the Command again.

I checked what process-wrap does for this, but they just use command in non-inline way, or create their own structs directly.

TokioChildProcess::new({
let mut cmd = tokio::process::Command::new("npx");
cmd.arg("mcp-server-git")
cmd
})

will workfor inline way, but this is just the same as doing it before, I coulnd't find a clean way to chain it inline

[Copilot]

Pull Request Overview

This PR updates the TokioChildProcess API to consume a full tokio::process::Command (instead of a mutable reference) and integrates the process_wrap crate to ensure that child processes cleanup their subprocesses (using a JobObject on Windows and a Process Group on Unix). The changes span multiple examples, tests, and the core transport implementation.

• Updated TokioChildProcess::new to take ownership of the command and chain additional configurations.
• Adjusted various examples and tests to use the new command initialization pattern.
• Introduced a ChildWithCleanup wrapper that utilizes process_wrap for proper child process termination.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
examples/simple-chat-client/src/config.rs	Updated command creation to comply with the new API
examples/simple-chat-client/Cargo.toml	Changed rmcp dependency from git to local path
examples/rig-integration/src/config/mcp.rs	Updated command initialization per new API
examples/clients/src/std_io.rs	Refactored command creation and updated commented examples
examples/clients/src/everything_stdio.rs	Modified command creation for consistency
examples/clients/src/collection.rs	Updated command creation; note change in argument value
docs/readme/README.zh-cn.md	Refactored example to show new command initialization
crates/rmcp/tests/test_with_python.rs	Updated test command creation
crates/rmcp/tests/test_with_js.rs	Updated test command creation
crates/rmcp/src/transport/child_process.rs	Refactored transport module to wrap child process with cleanup
crates/rmcp/src/lib.rs	Updated documentation to reflect the new API
crates/rmcp/Cargo.toml	Added process-wrap dependency and updated transport features
README.md	Updated quick-start example to use new command pattern

Comments suppressed due to low confidence (1)

examples/clients/src/collection.rs:22

• [nitpick] Verify that changing the argument from 'mcp-server-git' to 'mcp-client-git' is intentional and consistent with the intended behavior of the example.

cmd.arg("mcp-client-git");

[Copilot]

[nitpick] Consider logging or handling errors from 'start_kill()' in the Drop implementation to aid in debugging potential cleanup issues.

[4t145]

How about create a command wrapper in rmcp? It could be something like this.

#[derive(Debug)]pubstructCommand{tokio: tokio::process::Command,}implFrom<tokio::process::Command>forCommand{fnfrom(tokio: tokio::process::Command) -> Self{Self{ tokio }}}implCommand{pubfnnew<S:AsRef<OsStr>>(program:S) -> Command{Self::from(tokio::process::Command::new(program))}pubfnarg<S:AsRef<OsStr>>(mutself,arg:S) -> Command{self.tokio.arg(arg);self}pubfnargs<I,S>(mutself,args:I) -> CommandwhereI:IntoIterator<Item = S>,S:AsRef<OsStr>,{self.tokio.args(args);self}pubfnenv<K,V>(mutself,key:K,val:V) -> CommandwhereK:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.env(key, val);self}pubfnenvs<I,K,V>(mutself,vars:I) -> CommandwhereI:IntoIterator<Item = (K,V)>,K:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.envs(vars);self}pubfninto_child_process(self) -> std::io::Result<TokioChildProcess>{TokioChildProcess::new(self.tokio)}pubfninto_tokio(self) -> tokio::process::Command{self.tokio}pubfnas_tokio_mut(&mutself) -> &mut tokio::process::Command{&mutself.tokio}pubfnas_tokio(&self) -> &tokio::process::Command{&self.tokio}}

And we can use it like this

let transport = Command::new("node").arg("tests/test_with_js/server.js").into_child_process()?;let client = ().serve(transport).await?;

Will it be confusing for there are too many Command type?

[jokemanfire]

How about add a event listener , ant use wait_pid to recycle the children process ?I think we only need to focus on the possibility of zombie like child processes during the runtime of the main process.

[humbertoyusta]

One option for inline could be to modify examples to do like:

TokioChildProcess::new(tokio::process::Command::new("node").map(
|cmd| { cmd.arg("tests/test_with_js/client.js"); cmd }))

Another could be for people who use tap crate, using tap_mut, but it's more or less the same, we could provide that trait to command, but it would need to be imported and it's not much less verbose

use rmcp::transport::child_process::CommandTapMut;TokioChildProcess::new(tokio::process::Command::new("node").tap_mut(
|cmd| { cmd.arg("tests/test_with_js/client.js");}))

Or we could do a wrapper to command like proposed before, but there will be several Command structs then, not sure which is the best approach.

[4t145]

@humbertoyusta The first approach looks great. I am okay with it.

[humbertoyusta]

Actually .map() didn't work, I thought Command::new() was giving result but it's not, so I needed to add a trait for tokio::process::Command.

I still think it's better than a full Command wrapper, but if you think otherwise let me know.

[4t145]

@humbertoyusta I am ok with the ext trait.

Looks there's confict and ci didn't pass. So could you merge it with the main branch, for I just made a lot of changes, and fix the formatting and test ci? Thanks for your patience!

[4t145]

It's okey with the ci message, I can rewrite it when merge.

[4t145]

And thanks for your work!

[humbertoyusta]

Ensure child processes kill all subprocesses on drop (for transport-child-process)

Motivation and Context

Starting a tokio child processes, in several mcp servers, like using uv some-mcp-server in MacOS, will create subprocesses that will not be cleaned when the first process is killed with the .kill_on_drop(true)

Using a JobObject in Windows and a Process Group in Unix, killing the child process will also clean all subprocesses, avoiding resource leaking

How Has This Been Tested?

We (refact.ai) use this library for integrating mcp servers with our client, we recently switched from another library, so it's not in production yet.

Scenarios about starting servers like with uv were tested to avoid resource leaking, also fakeish scenarios like sh -c "npx some-mcp-server" were tested, causing subprocess leaking before, not now

Breaking Changes

TokioChildProcess::new(command) takes now tokio::process::Command entirely, instead of &mut tokio::process::Command

• The reasion for this, is that the wrapping library used for process group and job object, requires taking the whole command instead of &mut ref
• Example tests have been updated accordingly

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• [?] My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling (not needed)
• I have added or updated documentation as needed

[4t145]

Thank you for PR, I will have a look on this!

[4t145]

Looks good!

Is there a way to provide a chained call api to create a cmd? Just want to give out a more easy to use interface.

And it will be great if you can update the document like README.

[humbertoyusta]

Thanks for the review,

I updated README and the other example that was missing.

I couldn't find a way to provide an inline chained way, due to the fact that most std/tokio::process::Command methods like args(), envs() take and return &mut Command instead of Command, so if you chain it in one line, there's no way to get the Command again.

I checked what process-wrap does for this, but they just use command in non-inline way, or create their own structs directly.

TokioChildProcess::new({
let mut cmd = tokio::process::Command::new("npx");
cmd.arg("mcp-server-git")
cmd
})

will workfor inline way, but this is just the same as doing it before, I coulnd't find a clean way to chain it inline

[Copilot]

Pull Request Overview

This PR updates the TokioChildProcess API to consume a full tokio::process::Command (instead of a mutable reference) and integrates the process_wrap crate to ensure that child processes cleanup their subprocesses (using a JobObject on Windows and a Process Group on Unix). The changes span multiple examples, tests, and the core transport implementation.

• Updated TokioChildProcess::new to take ownership of the command and chain additional configurations.
• Adjusted various examples and tests to use the new command initialization pattern.
• Introduced a ChildWithCleanup wrapper that utilizes process_wrap for proper child process termination.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
examples/simple-chat-client/src/config.rs	Updated command creation to comply with the new API
examples/simple-chat-client/Cargo.toml	Changed rmcp dependency from git to local path
examples/rig-integration/src/config/mcp.rs	Updated command initialization per new API
examples/clients/src/std_io.rs	Refactored command creation and updated commented examples
examples/clients/src/everything_stdio.rs	Modified command creation for consistency
examples/clients/src/collection.rs	Updated command creation; note change in argument value
docs/readme/README.zh-cn.md	Refactored example to show new command initialization
crates/rmcp/tests/test_with_python.rs	Updated test command creation
crates/rmcp/tests/test_with_js.rs	Updated test command creation
crates/rmcp/src/transport/child_process.rs	Refactored transport module to wrap child process with cleanup
crates/rmcp/src/lib.rs	Updated documentation to reflect the new API
crates/rmcp/Cargo.toml	Added process-wrap dependency and updated transport features
README.md	Updated quick-start example to use new command pattern

Comments suppressed due to low confidence (1)

examples/clients/src/collection.rs:22

• [nitpick] Verify that changing the argument from 'mcp-server-git' to 'mcp-client-git' is intentional and consistent with the intended behavior of the example.

cmd.arg("mcp-client-git");

[Copilot]

[nitpick] Consider logging or handling errors from 'start_kill()' in the Drop implementation to aid in debugging potential cleanup issues.

[4t145]

How about create a command wrapper in rmcp? It could be something like this.

#[derive(Debug)]pubstructCommand{tokio: tokio::process::Command,}implFrom<tokio::process::Command>forCommand{fnfrom(tokio: tokio::process::Command) -> Self{Self{ tokio }}}implCommand{pubfnnew<S:AsRef<OsStr>>(program:S) -> Command{Self::from(tokio::process::Command::new(program))}pubfnarg<S:AsRef<OsStr>>(mutself,arg:S) -> Command{self.tokio.arg(arg);self}pubfnargs<I,S>(mutself,args:I) -> CommandwhereI:IntoIterator<Item = S>,S:AsRef<OsStr>,{self.tokio.args(args);self}pubfnenv<K,V>(mutself,key:K,val:V) -> CommandwhereK:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.env(key, val);self}pubfnenvs<I,K,V>(mutself,vars:I) -> CommandwhereI:IntoIterator<Item = (K,V)>,K:AsRef<OsStr>,V:AsRef<OsStr>,{self.tokio.envs(vars);self}pubfninto_child_process(self) -> std::io::Result<TokioChildProcess>{TokioChildProcess::new(self.tokio)}pubfninto_tokio(self) -> tokio::process::Command{self.tokio}pubfnas_tokio_mut(&mutself) -> &mut tokio::process::Command{&mutself.tokio}pubfnas_tokio(&self) -> &tokio::process::Command{&self.tokio}}

And we can use it like this

let transport = Command::new("node").arg("tests/test_with_js/server.js").into_child_process()?;let client = ().serve(transport).await?;

Will it be confusing for there are too many Command type?

[jokemanfire]

How about add a event listener , ant use wait_pid to recycle the children process ?I think we only need to focus on the possibility of zombie like child processes during the runtime of the main process.

[humbertoyusta]

One option for inline could be to modify examples to do like:

TokioChildProcess::new(tokio::process::Command::new("node").map(
|cmd| { cmd.arg("tests/test_with_js/client.js"); cmd }))

Another could be for people who use tap crate, using tap_mut, but it's more or less the same, we could provide that trait to command, but it would need to be imported and it's not much less verbose

use rmcp::transport::child_process::CommandTapMut;TokioChildProcess::new(tokio::process::Command::new("node").tap_mut(
|cmd| { cmd.arg("tests/test_with_js/client.js");}))

Or we could do a wrapper to command like proposed before, but there will be several Command structs then, not sure which is the best approach.

[4t145]

@humbertoyusta The first approach looks great. I am okay with it.

[humbertoyusta]

Actually .map() didn't work, I thought Command::new() was giving result but it's not, so I needed to add a trait for tokio::process::Command.

I still think it's better than a full Command wrapper, but if you think otherwise let me know.

[4t145]

@humbertoyusta I am ok with the ext trait.

Looks there's confict and ci didn't pass. So could you merge it with the main branch, for I just made a lot of changes, and fix the formatting and test ci? Thanks for your patience!

[4t145]

It's okey with the ci message, I can rewrite it when merge.

[4t145]

And thanks for your work!