diff --git a/codex-rs/app-server-daemon/src/backend/windows.rs b/codex-rs/app-server-daemon/src/backend/windows.rs index 3b8b91ae47..53603afe2b 100644 --- a/codex-rs/app-server-daemon/src/backend/windows.rs +++ b/codex-rs/app-server-daemon/src/backend/windows.rs @@ -3,7 +3,9 @@ //! Managed servers must not elevate ordinary clients sharing the account's socket. //! Installer jobs contain extraction processes when an update is cancelled. //! Detached launches stop the launcher's original stdio handles from propagating. +//! Launch probes distinguish job restrictions from other failures without running the binary. +use std::fmt; use std::io; use std::os::windows::io::AsRawHandle; use std::os::windows::io::FromRawHandle; @@ -15,6 +17,7 @@ use std::process::Stdio; use anyhow::Context; use anyhow::Result; +use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED; use windows_sys::Win32::Foundation::ERROR_INVALID_HANDLE; use windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER; use windows_sys::Win32::Foundation::ERROR_LOCK_VIOLATION; @@ -113,24 +116,52 @@ pub(crate) fn ensure_not_elevated() -> Result<()> { Ok(()) } +/// A launch that only fails when asked to leave the launcher's Windows job. +/// Automatic CLI startup may use its embedded server; lifecycle operations +/// must still return this error before stopping an existing daemon. +#[derive(Debug)] +pub struct DetachedLaunchRestricted; + +impl fmt::Display for DetachedLaunchRestricted { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str( + "this Windows launcher prevents background processes from outliving it (for example, cargo run); build and run codex.exe directly to use the background server", + ) + } +} + // Check that breakaway launch is permitted before stopping an existing daemon. // An outer system job may remain attached; membership alone does not establish // whether it will terminate the daemon. Suspend the probe before cleanup. pub(crate) fn ensure_detached_launch(executable: &Path) -> Result<()> { - let mut child = Command::new(executable) - .creation_flags(CREATE_SUSPENDED | DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB) + let mut command = Command::new(executable); + command .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) - .spawn() - .context("cannot launch detached daemon; existing daemon was not stopped")?; + .creation_flags(CREATE_SUSPENDED | DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB); + let (mut child, launch_result) = match command.spawn() { + Ok(child) => (child, Ok(())), + Err(err) if err.raw_os_error() == Some(ERROR_ACCESS_DENIED as i32) => { + // Access denied can also mean the file cannot be executed. Only + // classify a job restriction if removing breakaway makes it work. + // This diagnostic child stays suspended and is always reaped. + command.creation_flags(CREATE_SUSPENDED | DETACHED_PROCESS); + let child = match command.spawn() { + Ok(child) => child, + Err(_) => return Err(err).context("cannot launch detached daemon"), + }; + (child, Err(err).context(DetachedLaunchRestricted)) + } + Err(err) => return Err(err).context("cannot launch detached daemon"), + }; child .kill() .context("failed to terminate suspended launch probe")?; child .wait() .context("failed to reap suspended launch probe")?; - Ok(()) + launch_result } pub(super) struct Process(OwnedHandle); diff --git a/codex-rs/app-server-daemon/src/backend/windows_tests.rs b/codex-rs/app-server-daemon/src/backend/windows_tests.rs index ed9044bd18..1910b2c96f 100644 --- a/codex-rs/app-server-daemon/src/backend/windows_tests.rs +++ b/codex-rs/app-server-daemon/src/backend/windows_tests.rs @@ -151,7 +151,17 @@ fn detached_launch_preflight_rejects_restrictive_job() { 0 ); // A new job does not permit breakaway. Reject before any lifecycle mutation. - assert!(super::ensure_detached_launch(&executable).is_err()); + let err = super::ensure_detached_launch(&executable).expect_err("breakaway forbidden"); + assert!( + err.is::(), + "expected a classified job restriction: {err:#}" + ); + // File access failures must still fail normally, even in a restricted + // job: a CLI must not hide corrupt or inaccessible installations. + let temp = tempfile::tempdir().expect("test directory"); + let err = + super::ensure_detached_launch(temp.path()).expect_err("directory is not an executable"); + assert!(!err.is::(), "{err:#}"); return; } let output = std::process::Command::new(executable) diff --git a/codex-rs/app-server-daemon/src/lib.rs b/codex-rs/app-server-daemon/src/lib.rs index 755a0ebd82..54ca92b6c1 100644 --- a/codex-rs/app-server-daemon/src/lib.rs +++ b/codex-rs/app-server-daemon/src/lib.rs @@ -2,6 +2,8 @@ mod backend; #[cfg(windows)] +pub use backend::windows::DetachedLaunchRestricted; +#[cfg(windows)] use backend::windows::try_lock_file; mod client; mod install_lock; diff --git a/codex-rs/cli/tests/daemon_startup.rs b/codex-rs/cli/tests/daemon_startup.rs index 828ccf1d11..be8752836e 100644 --- a/codex-rs/cli/tests/daemon_startup.rs +++ b/codex-rs/cli/tests/daemon_startup.rs @@ -59,6 +59,28 @@ async fn daemon_auto_start_preserves_bedrock_onboarding() -> Result<()> { daemon_startup("bedrock").await } +#[tokio::test] +#[cfg(windows)] +async fn restrictive_launcher_uses_embedded_if_daemon_cannot_start() -> Result<()> { + const CHILD: &str = "CODEX_TEST_RESTRICTIVE_DAEMON_START"; + if std::env::var_os(CHILD).is_some() { + // The PTY creates its own breakaway-permitted job. Add Cargo's + // restriction *inside* the PTY so it is the CLI's innermost job. + let job = codex_utils_pty::JobObject::create_without_breakaway()?; + let codex = codex_utils_cargo_bin::cargo_bin("codex")?; + let mut command = tokio::process::Command::new(codex); + command + .arg("--no-alt-screen") + .stdin(std::process::Stdio::inherit()) + .stdout(std::process::Stdio::inherit()) + .stderr(std::process::Stdio::inherit()); + let status = job.spawn_contained(&mut command)?.wait().await?; + ensure!(status.success(), "CLI exited: {status}"); + return Ok(()); + } + daemon_startup("restrictive-job").await +} + #[tokio::test] #[cfg(unix)] async fn bedrock_onboarding_leaves_a_running_daemon_untouched() -> Result<()> { @@ -129,26 +151,29 @@ async fn daemon_startup(command: &str) -> Result<()> { env.insert("TERM".into(), "xterm-256color".into()); let mut args = vec!["--no-alt-screen".to_string()]; let mut steps: VecDeque<(&str, &[u8])> = VecDeque::new(); - if matches!(command, "start" | "bedrock-running") || mismatch { + if matches!(command, "start" | "bedrock-running" | "restrictive-job") || mismatch { // A selected package with a stopped daemon avoids installing a release. let managed = home .path() - .join("packages/app-server-daemon/current/bin/codex"); + .join("packages/app-server-daemon/current/bin") + .join(if cfg!(windows) { "codex.exe" } else { "codex" }); fs::create_dir_all(home.path().join("packages/app-server-daemon/current/bin"))?; // Hard links change the executable's ctime and invalidate Rosetta's translation cache. #[cfg(unix)] std::os::unix::fs::symlink(&codex, &managed)?; #[cfg(not(unix))] fs::hard_link(&codex, &managed).or_else(|_| fs::copy(&codex, &managed).map(|_| ()))?; - fs::create_dir(home.path().join("app-server-daemon"))?; - fs::write( - home.path().join("app-server-daemon/settings.json"), - if persisted { - r#"{"shutdownGraceSeconds":0,"updater":{"autoUpdateEnabled":false},"featureOverrides":{"api_key_model_discovery":true}}"# - } else { - r#"{"shutdownGraceSeconds":0,"updater":{"autoUpdateEnabled":false}}"# - }, - )?; + if command != "restrictive-job" { + fs::create_dir(home.path().join("app-server-daemon"))?; + fs::write( + home.path().join("app-server-daemon/settings.json"), + if persisted { + r#"{"shutdownGraceSeconds":0,"updater":{"autoUpdateEnabled":false},"featureOverrides":{"api_key_model_discovery":true}}"# + } else { + r#"{"shutdownGraceSeconds":0,"updater":{"autoUpdateEnabled":false}}"# + }, + )?; + } } let pid_file = home.path().join("app-server-daemon/daemon.pid"); let result = async { @@ -172,6 +197,9 @@ async fn daemon_startup(command: &str) -> Result<()> { // The draft header is visible before the session's command composer is ready. steps.push_back(("GPT-5.6-Terra", b"/status\r")); "Server:Localbackgroundserver" + } else if command == "restrictive-job" { + steps.push_back(("GPT-5.6-Terra", b"\x14")); + "Runningwithoutthesharedbackgroundserver:thisWindowslauncher" } else if mismatch { args.extend(if persisted { ["--disable".into(), "api_key_model_discovery".into()] @@ -201,8 +229,19 @@ async fn daemon_startup(command: &str) -> Result<()> { steps.push_back(("GPT-5.6-Terra", b"\x14")); "Runningwithoutthesharedbackgroundserver:--strict-config" }; + let program = if cfg!(windows) && command == "restrictive-job" { + env.insert("CODEX_TEST_RESTRICTIVE_DAEMON_START".into(), "1".into()); + args = vec![ + "--exact".into(), + "restrictive_launcher_uses_embedded_if_daemon_cannot_start".into(), + "--nocapture".into(), + ]; + std::env::current_exe()? + } else { + codex.clone() + }; let spawned = codex_utils_pty::spawn_pty_process( - &codex.to_string_lossy(), + &program.to_string_lossy(), &args, &workspace_path, &env, @@ -214,7 +253,7 @@ async fn daemon_startup(command: &str) -> Result<()> { codex_utils_pty::ChildFds::Inherited(&[]), ) .await?; - let exit = spawned.exit_rx; + let mut exit = spawned.exit_rx; let session = spawned.session; let writer = session.writer_sender(); let mut stdout = spawned.stdout_rx; @@ -247,6 +286,16 @@ async fn daemon_startup(command: &str) -> Result<()> { .chars() .filter(|c| !c.is_whitespace()) .collect(); + if command == "restrictive-job" + && text.contains("starttheWindowsdaemonfromanon-elevatedterminal") + { + // Elevated runners must still reject startup before the job probe. + ensure!(!pid_file.exists()); + let status = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), &mut exit) + .await??; + ensure!(status != 0, "elevated daemon launch must fail"); + return Ok::<_, anyhow::Error>(()); + } if let Some((ready, input)) = steps.front() && text.contains(ready) { @@ -276,8 +325,15 @@ async fn daemon_startup(command: &str) -> Result<()> { ensure!(home.path().join("app-server-daemon/daemon.pid").exists()); } else if let Some(existing_daemon) = &existing_daemon { ensure!(fs::read(&pid_file)? == *existing_daemon); - } else if bedrock_onboarding { + } else if bedrock_onboarding || command == "restrictive-job" { ensure!(!pid_file.exists()); + if command == "restrictive-job" { + let contents = screen.screen().contents(); + let warning = contents.lines() + .find(|line| line.contains("Running without the shared background server:")) + .context("missing rendered fallback warning")?; + insta::assert_snapshot!("restrictive_launcher_warning", warning.trim()); + } } return Ok::<_, anyhow::Error>(()); } diff --git a/codex-rs/cli/tests/snapshots/daemon_startup__restrictive_launcher_warning.snap b/codex-rs/cli/tests/snapshots/daemon_startup__restrictive_launcher_warning.snap new file mode 100644 index 0000000000..ae68754df2 --- /dev/null +++ b/codex-rs/cli/tests/snapshots/daemon_startup__restrictive_launcher_warning.snap @@ -0,0 +1,6 @@ +--- +source: cli/tests/daemon_startup.rs +assertion_line: 325 +expression: warning.trim() +--- +⚠ Running without the shared background server: this Windows launcher requires embedded mode. diff --git a/codex-rs/tui/src/daemon_startup.rs b/codex-rs/tui/src/daemon_startup.rs index 9ce454fc9e..ba6de532fb 100644 --- a/codex-rs/tui/src/daemon_startup.rs +++ b/codex-rs/tui/src/daemon_startup.rs @@ -1,6 +1,7 @@ //! Local daemon launch policy. Explicit embedded launches never discover or start a daemon; //! optional attachment may fall back to embedded mode, while automatic launches -//! require a compatible shared server and a successful connection. +//! require a compatible shared server and a successful connection, except when +//! the Windows launcher forbids detaching a missing server. use super::*; use std::collections::BTreeMap; diff --git a/codex-rs/tui/src/startup_orchestration.rs b/codex-rs/tui/src/startup_orchestration.rs index cbba29ac3f..1305e61c6c 100644 --- a/codex-rs/tui/src/startup_orchestration.rs +++ b/codex-rs/tui/src/startup_orchestration.rs @@ -531,18 +531,31 @@ pub(super) async fn run_main_inner( crossterm::terminal::disable_raw_mode()?; let result = codex_app_server_daemon::start_with_features(&daemon_features).await; daemon_telemetry::record_start(&config, &result).await; - result.map_err(|err| { - std::io::Error::other(format!("{err:#}\n{}", daemon_startup::FAILURE_HINT)) - }) + match result { + Ok(output) => Ok(Some(output)), + #[cfg(windows)] + Err(err) if err.is::() => { + Ok(None) + } + Err(err) => Err(std::io::Error::other(format!( + "{err:#}\n{}", + daemon_startup::FAILURE_HINT + ))), + } }) .await?; - managed_daemon = output.backend.is_some(); - app_server_target = AppServerTarget::LocalDaemon { - endpoint: RemoteAppServerEndpoint::UnixSocket { - socket_path: AbsolutePathBuf::from_absolute_path_checked(output.socket_path)?, - }, - allow_embedded_fallback: false, - }; + if let Some(output) = output { + managed_daemon = output.backend.is_some(); + app_server_target = AppServerTarget::LocalDaemon { + endpoint: RemoteAppServerEndpoint::UnixSocket { + socket_path: AbsolutePathBuf::from_absolute_path_checked(output.socket_path)?, + }, + allow_embedded_fallback: false, + }; + } else { + app_server_target = AppServerTarget::Embedded; + daemon_exclusion = Some("this Windows launcher"); + } } // The overview must inspect the shared server's agents regardless of local settings. let compatibility_warning = if cli.agents_overview {