-
Notifications
You must be signed in to change notification settings - Fork 401
Description
Describe the bug
The command line configuration is not respected by the TokioChildProcess.
I think this bug is introduced by #156, which replace the tokio::process::Child with use process_wrap::tokio::{TokioChildWrapper, TokioCommandWrap};
To Reproduce
Steps to reproduce the behavior:
let transport = TokioChildProcess::new(Command::new(&command).configure(|cmd| {
cmd.args(&args);
#[cfg(windows)]
cmd.creation_flags(windows::Win32::System::Threading::CREATE_NO_WINDOW.0);
// we want to ensure the MCP service is killed when our process exits
cmd.kill_on_drop(true);
if let Some(env) = &env {
for (key, value) in env {
cmd.env(key, value);
}
}
}))?;
let client = ().serve(transport).await?;
The creation_flags is not respected, and on Windows, the mcp creates new window regardless.
The process-wrap seems to have a different API for creation_flags, but users are not able to pass this flag into the TokioChildProcess::new: https://crates.io/crates/process-wrap/6.0.0 document reads:
you can see the doc explicitly mentioned
This is a shim to allow setting Windows process creation flags with this API, as otherwise they'd be overwritten.
or maybe I just don't know the correct way to do it? @humbertoyusta maybe you can shed some lights on is this expected?
It seems that if I want to enable this in the rmcp library, that means the rmcp library needs to depend on window lib, which seems to be pulling lots of unnecessary dependencies. but if we don't pull it here, but instead ask the users to set those flags, then we probably should not use process-wrap or we should probably provide a different API.
Expected behavior
tokio::process api should be respected.
Why do we need this
we are creating a desktop app in both windows and macos, the desktop is end-user facing, and pop up the terminal windows is quite scary. that's why we choose to hide it. but we found we cannot suppress the terminal window anymore even if we explicitly set the flag.
Logs
If applicable, add logs to help explain your problem.
Additional context
Add any other context about the problem here.
