mirror of
https://github.com/openai/codex.git
synced 2026-10-14 16:38:43 +00:00
Suppress console windows for local Windows MCP servers (#48238)
## What changed Expose Windows process creation flags on the shared command wrapper and use `CREATE_NO_WINDOW` when launching local stdio MCP servers, including fallback launches without Job Object containment. Preserve `CREATE_SUSPENDED` when assigning the server to a Job Object before execution. ## Testing Add a Windows regression test covering direct launches and launches through `cmd.exe`. Verify that the server has no console, initializes successfully, returns tools, and exits after shutdown. GitOrigin-RevId: 84475e82999d91a431e09327d4f5639b59c01427
This commit is contained in:
committed by
Ian MacLeod
parent
5c3671c1a0
commit
bd5b3b7a20
@@ -1003,6 +1003,15 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
std::fs::write(pid_file, std::process::id().to_string())?;
|
||||
}
|
||||
#[cfg(windows)]
|
||||
if let Ok(console_state_file) = std::env::var("MCP_TEST_CONSOLE_STATE_FILE") {
|
||||
#[link(name = "kernel32")]
|
||||
unsafe extern "system" {
|
||||
fn GetConsoleWindow() -> *mut std::ffi::c_void;
|
||||
}
|
||||
let has_console = unsafe { !GetConsoleWindow().is_null() };
|
||||
std::fs::write(console_state_file, has_console.to_string())?;
|
||||
}
|
||||
#[cfg(windows)]
|
||||
if let Ok(marker_file) = std::env::var("MCP_TEST_BREAKAWAY_DENIED_FILE") {
|
||||
use std::os::windows::process::CommandExt;
|
||||
|
||||
|
||||
@@ -214,6 +214,11 @@ impl StdioServerLauncher for LocalStdioServerLauncher {
|
||||
|
||||
// Local private implementation.
|
||||
|
||||
#[cfg(windows)]
|
||||
const CREATE_NO_WINDOW: u32 = 0x0800_0000;
|
||||
#[cfg(windows)]
|
||||
const CREATE_SUSPENDED: u32 = 0x0000_0004;
|
||||
|
||||
#[cfg(unix)]
|
||||
const PROCESS_GROUP_TERM_GRACE_PERIOD: Duration = Duration::from_secs(2);
|
||||
|
||||
@@ -288,6 +293,8 @@ impl LocalStdioServerLauncher {
|
||||
// handles and needs a handle allowlist in the shared spawn backend.
|
||||
#[cfg(unix)]
|
||||
command.descriptor_policy(DescriptorPolicy::Explicit);
|
||||
#[cfg(windows)]
|
||||
command.creation_flags(CREATE_NO_WINDOW);
|
||||
command
|
||||
};
|
||||
#[cfg(windows)]
|
||||
@@ -298,6 +305,8 @@ impl LocalStdioServerLauncher {
|
||||
let job = match codex_utils_pty::JobObject::create_without_breakaway() {
|
||||
Ok(job) => {
|
||||
command.prepare_suspended_spawn(&job);
|
||||
// The helper replaces creation flags; retain suspension and suppress the console.
|
||||
command.creation_flags(CREATE_SUSPENDED | CREATE_NO_WINDOW);
|
||||
Some(job)
|
||||
}
|
||||
Err(error) => {
|
||||
|
||||
@@ -134,6 +134,90 @@ async fn stdio_message_limits_preserve_legacy_local_compatibility() -> anyhow::R
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn local_stdio_initializes_without_a_console_directly_and_through_cmd() -> anyhow::Result<()>
|
||||
{
|
||||
let server = codex_utils_cargo_bin::cargo_bin("test_stdio_server")?;
|
||||
let temp_dir = tempfile::tempdir()?;
|
||||
let console_state_file = temp_dir.path().join("console-state");
|
||||
let server_pid_file = temp_dir.path().join("server.pid");
|
||||
std::fs::write(
|
||||
temp_dir.path().join("launch.cmd"),
|
||||
"@echo off\r\n\"%MCP_TEST_SERVER_BINARY%\"\r\n",
|
||||
)?;
|
||||
let env = HashMap::from([
|
||||
(
|
||||
OsString::from("MCP_TEST_CONSOLE_STATE_FILE"),
|
||||
console_state_file.clone().into_os_string(),
|
||||
),
|
||||
(
|
||||
OsString::from("MCP_TEST_PID_FILE"),
|
||||
server_pid_file.clone().into_os_string(),
|
||||
),
|
||||
(
|
||||
OsString::from("MCP_TEST_SERVER_BINARY"),
|
||||
server.clone().into(),
|
||||
),
|
||||
]);
|
||||
|
||||
for (program, args) in [
|
||||
(server.into(), Vec::new()),
|
||||
(
|
||||
OsString::from("cmd.exe"),
|
||||
["/d", "/s", "/c", "call", ".\\launch.cmd"]
|
||||
.map(OsString::from)
|
||||
.to_vec(),
|
||||
),
|
||||
] {
|
||||
let client = RmcpClient::new_stdio_client(
|
||||
program,
|
||||
args,
|
||||
Some(env.clone()),
|
||||
&[],
|
||||
Some(temp_dir.path().to_string_lossy().into_owned()),
|
||||
Arc::new(LocalStdioServerLauncher::new(temp_dir.path().to_path_buf())),
|
||||
)
|
||||
.await?;
|
||||
client
|
||||
.initialize(
|
||||
InitializeRequestParams::new(
|
||||
ClientCapabilities::default(),
|
||||
Implementation::new("stdio-console-test", "1.0.0"),
|
||||
)
|
||||
.with_protocol_version(ProtocolVersion::V_2025_06_18),
|
||||
Some(Duration::from_secs(10)),
|
||||
Box::new(|_, _| {
|
||||
async {
|
||||
Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
meta: None,
|
||||
})
|
||||
}
|
||||
.boxed()
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
let console_state = std::fs::read_to_string(&console_state_file)?;
|
||||
let server_pid = std::fs::read_to_string(&server_pid_file)?
|
||||
.trim()
|
||||
.parse::<u32>()?;
|
||||
let server_process = open_process_for_wait(server_pid)?;
|
||||
let tools = client
|
||||
.list_tools(/*params*/ None, Some(Duration::from_secs(10)))
|
||||
.await;
|
||||
client.shutdown().await;
|
||||
wait_for_process_exit(&server_process)?;
|
||||
|
||||
pretty_assertions::assert_eq!(console_state, "false");
|
||||
assert!(!tools?.tools.is_empty());
|
||||
std::fs::remove_file(&console_state_file)?;
|
||||
std::fs::remove_file(&server_pid_file)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn local_stdio_shutdown_terminates_descendants_after_server_exit() -> anyhow::Result<()> {
|
||||
|
||||
@@ -233,6 +233,12 @@ impl Command {
|
||||
self
|
||||
}
|
||||
|
||||
/// Set the Windows process creation flags, replacing any previously selected flags.
|
||||
#[cfg(windows)]
|
||||
pub fn creation_flags(&mut self, flags: u32) -> &mut Self {
|
||||
self.inner.creation_flags(flags);
|
||||
self
|
||||
}
|
||||
/// Preserve Job Object assignment before the child begins executing on Windows.
|
||||
#[cfg(windows)]
|
||||
pub fn prepare_suspended_spawn(&mut self, job: &crate::JobObject) {
|
||||
|
||||
Reference in New Issue
Block a user