mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Fall back to embedded mode under restrictive Windows launchers (#48491)
## Why
Windows launchers such as `cargo run` can prevent background processes from outliving them, causing automatic daemon startup to fail and blocking the CLI from opening.
## What changed
- Classify detached launch restrictions by retrying an access-denied launch probe without the job breakaway flag. Keep both probes suspended and terminate and reap successful probes.
- Use the embedded server with a visible warning when automatic startup encounters this restriction. Preserve errors for other launch failures, including inaccessible executables and elevated startup.
- Keep explicit daemon lifecycle operations failing on the restriction, with guidance to build and run `codex.exe` directly.
## Testing
Add Windows coverage for restriction classification and executable access failures, plus a CLI integration test and warning snapshot for embedded fallback under a restrictive job. The integration test also checks that elevated startup still fails and no daemon PID file is created.
GitOrigin-RevId: fc7c46e0f5f07ca0aba12b535a0e372021bdf46a
(cherry picked from commit a6bd19261c)
This commit is contained in:
committed by
Ian MacLeod
parent
bbb398d186
commit
9804fabd68
@@ -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);
|
||||
|
||||
@@ -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::<super::DetachedLaunchRestricted>(),
|
||||
"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::<super::DetachedLaunchRestricted>(), "{err:#}");
|
||||
return;
|
||||
}
|
||||
let output = std::process::Command::new(executable)
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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>(());
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
@@ -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;
|
||||
|
||||
@@ -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::<codex_app_server_daemon::DetachedLaunchRestricted>() => {
|
||||
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 {
|
||||
|
||||
Reference in New Issue
Block a user