From ae09e7bc947e39a97e41591e35740bbeb98a00ec Mon Sep 17 00:00:00 2001 From: David Viejo Date: Mon, 21 Sep 2026 22:57:24 +0200 Subject: [PATCH 1/6] fix(windows): preserve provider launch environment and test native shims --- CHANGELOG.md | 5 + CONTRIBUTING.md | 5 + docs/how-to/test-platform-compatibility.md | 50 +++++ src/process.rs | 219 ++++++++++++++++++++- src/ssh.rs | 3 + 5 files changed, 278 insertions(+), 4 deletions(-) create mode 100644 docs/how-to/test-platform-compatibility.md diff --git a/CHANGELOG.md b/CHANGELOG.md index cad1b32..5b63191 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ Versioning and Keep a Changelog conventions. ### Changed +- Preserve Windows system and profile environment variables when launching providers, + without inheriting unrelated credentials. Hide provider and cleanup console windows. + Add native process fixtures for arguments, stdin, failures, cancellation, and + Windows Claude/Codex/OpenCode `.cmd` wrappers. + - Corrected the minimum supported Rust version to 1.88 to match locked dependencies, with an all-targets/all-features compiler check in CI. - Crate archive paths are root-anchored so nested third-party README/license diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 87f8dd3..980a3b7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,3 +47,8 @@ Keep pull requests focused and describe: By contributing, you agree that your contributions are licensed under the project's MIT OR Apache-2.0 terms. + +## Platform compatibility + +See [Linux and Windows runtime testing](docs/how-to/test-platform-compatibility.md) +for native launch fixtures and the separate authenticated-provider acceptance checks. diff --git a/docs/how-to/test-platform-compatibility.md b/docs/how-to/test-platform-compatibility.md new file mode 100644 index 0000000..18dbfc5 --- /dev/null +++ b/docs/how-to/test-platform-compatibility.md @@ -0,0 +1,50 @@ +# Test Linux and Windows runtime compatibility + +Run native process tests on the execution host, not only on the desktop client. +The runtime launches the provider CLI on that host. A Windows Fleet client +connected to a Linux execution target does not exercise native Windows agents. + +## Credential-free launch tests + +Install stable Rust and run: + +```sh +cargo test --locked --lib process::tests -- --nocapture +cargo test --locked --all-features +``` + +The process tests compile a small native executable in a directory containing +spaces. They verify JSON and Unicode arguments, stdin, explicit environment +values, nonzero exit status, and cancellation. Windows additionally launches +`claude.cmd`, `codex.cmd`, and `opencode.cmd` fixture wrappers. These are launcher +fixtures, not actual authenticated provider turns. No provider installation or +model credentials are needed. The existing CI matrix runs these tests on Linux, +macOS, and Windows. + +The SDK preserves an explicit environment allowlist. On Windows this includes +system paths and profile/configuration directories, using case-insensitive key +matching. Unrelated API keys are not inherited. Explicit command environment +values still override inherited values. Windows child launches and taskkill +cleanup suppress console windows. Rust owns native command argument escaping; +callers must supply separate program and argument values. + +## Real provider acceptance + +Before claiming full platform support, run each installed provider through the +SDK on Linux and Windows with the user's own login. Verify tool start/completion, +approvals, cancellation with descendant processes, session resume, missing or +expired authentication, and long-running development servers. Exercise both +native executable and npm shim installations on Windows. Run equivalent tests +through Fleet after its adapter correctly maps the SDK's launch capabilities. +A successful cross-compilation or fixture run does not certify these journeys. + +## Current limits + +- SSH password authentication from a Windows execution host is explicitly + unsupported; the askpass helper currently requires Unix. +- Process-tree cleanup uses Unix process groups or Windows `taskkill /T /F`. + This is lifecycle management, not a security sandbox. +- Sandbox backend support must be checked independently; never bypass a requested + sandbox to make a platform test pass. +- Windows OS-service installation is an application responsibility, outside + this library's provider launch contract. diff --git a/src/process.rs b/src/process.rs index d8ce233..e89b091 100644 --- a/src/process.rs +++ b/src/process.rs @@ -1,5 +1,5 @@ use std::collections::VecDeque; -use std::ffi::OsString; +use std::ffi::OsStr; use std::process::Stdio; use tokio::io::AsyncReadExt; @@ -32,13 +32,45 @@ const SAFE_ENVIRONMENT: &[&str] = &[ "SSL_CERT_DIR", ]; +// Windows tools locate their configuration and system executables through these +// variables. Keep this allowlist explicit: inheriting the entire environment +// would also expose unrelated API keys to providers and their tools. +const WINDOWS_SAFE_ENVIRONMENT: &[&str] = &[ + "SYSTEMROOT", + "SYSTEMDRIVE", + "WINDIR", + "COMSPEC", + "PATHEXT", + "USERPROFILE", + "HOMEDRIVE", + "HOMEPATH", + "APPDATA", + "LOCALAPPDATA", + "PROGRAMDATA", + "PROGRAMFILES", + "PROGRAMFILES(X86)", +]; + +fn allowed_environment_key(name: &OsStr, windows: bool) -> bool { + let Some(name) = name.to_str() else { + return false; + }; + if windows { + SAFE_ENVIRONMENT + .iter() + .chain(WINDOWS_SAFE_ENVIRONMENT) + .any(|allowed| name.eq_ignore_ascii_case(allowed)) + } else { + SAFE_ENVIRONMENT.contains(&name) + } +} + pub(crate) fn spawn(spec: &CommandSpec, cwd: &std::path::Path) -> std::io::Result { let mut command = Command::new(&spec.program); command.args(&spec.args).current_dir(cwd); if spec.clear_environment { - let preserved = SAFE_ENVIRONMENT - .iter() - .filter_map(|name| std::env::var_os(name).map(|value| (OsString::from(name), value))); + let preserved = + std::env::vars_os().filter(|(name, _)| allowed_environment_key(name, cfg!(windows))); command.env_clear().envs(preserved); } command.envs(&spec.environment); @@ -49,6 +81,8 @@ pub(crate) fn spawn(spec: &CommandSpec, cwd: &std::path::Path) -> std::io::Resul .kill_on_drop(true); #[cfg(unix)] command.process_group(0); + #[cfg(windows)] + command.creation_flags(0x0800_0000); // CREATE_NO_WINDOW for desktop hosts. command.spawn() } @@ -93,7 +127,9 @@ impl ProcessTreeGuard { } #[cfg(windows)] if let Some(process_id) = self.process_id.take() { + use std::os::windows::process::CommandExt; let _ = std::process::Command::new("taskkill") + .creation_flags(0x0800_0000) .args(["/PID", &process_id.to_string(), "/T", "/F"]) .stdin(Stdio::null()) .stdout(Stdio::null()) @@ -129,3 +165,178 @@ where } Ok(String::from_utf8_lossy(&tail.into_iter().collect::>()).into_owned()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::time::Duration; + use tokio::io::AsyncWriteExt; + + #[test] + fn windows_environment_preserves_system_and_profile_but_not_credentials() { + for name in [ + "Path", + "SystemRoot", + "ComSpec", + "PATHEXT", + "USERPROFILE", + "AppData", + "LOCALAPPDATA", + ] { + assert!( + allowed_environment_key(OsStr::new(name), true), + "missing {name}" + ); + } + for name in [ + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "AWS_SECRET_ACCESS_KEY", + "FLEET_TOKEN", + "UNRELATED_VARIABLE", + ] { + assert!( + !allowed_environment_key(OsStr::new(name), true), + "leaked {name}" + ); + assert!( + !allowed_environment_key(OsStr::new(name), false), + "leaked {name}" + ); + } + assert!(allowed_environment_key(OsStr::new("PATH"), false)); + assert!(!allowed_environment_key(OsStr::new("Path"), false)); + assert!(!allowed_environment_key(OsStr::new("APPDATA"), false)); + } + + // Build a native fixture rather than requiring Node, Bash, or a real model + // login. This exercises the same launcher on Windows, Linux, and macOS. + fn native_fixture(root: &std::path::Path) -> std::path::PathBuf { + let source = root.join("fixture.rs"); + std::fs::write(&source, r#" + use std::io::Read; + fn main() { + let args: Vec<_> = std::env::args().skip(1).collect(); + if args.first().map(String::as_str) == Some("--exit") { + eprintln!("fixture failure"); + std::process::exit(17); + } + if args.first().map(String::as_str) == Some("--wait") { + std::thread::sleep(std::time::Duration::from_secs(60)); + return; + } + for arg in args { println!("ARG={arg:?}"); } + let mut input = String::new(); + std::io::stdin().read_to_string(&mut input).expect("stdin"); + println!("STDIN={input:?}"); + println!("OVERLAY={}", std::env::var("SDK_LAUNCH_TEST").expect("explicit environment")); + #[cfg(windows)] + for key in ["SystemRoot", "USERPROFILE", "APPDATA", "LOCALAPPDATA"] { + assert!(std::env::var_os(key).is_some(), "missing {key}"); + } + } + "#).expect("write native fixture"); + let binary = root.join(if cfg!(windows) { + "provider fixture.exe" + } else { + "provider fixture" + }); + let result = std::process::Command::new("rustc") + .args(["--edition=2021", "--crate-name=provider_fixture"]) + .arg(source) + .arg("-o") + .arg(&binary) + .output() + .expect("run rustc"); + assert!( + result.status.success(), + "fixture compilation: {}", + String::from_utf8_lossy(&result.stderr) + ); + binary + } + + async fn assert_launch(program: &std::path::Path, cwd: &std::path::Path) { + let args = [ + "app-server", + "--config", + r#"{"mcpServers":{"test":{"url":"http://127.0.0.1:1234/mcp"}}}"#, + "project with spaces", + "héllo", + ]; + let mut spec = CommandSpec::new(program); + spec.args = args.iter().map(std::ffi::OsString::from).collect(); + spec.environment + .insert("SDK_LAUNCH_TEST".into(), "explicit".into()); + let mut child = spawn(&spec, cwd).expect("launch provider fixture"); + let mut guard = ProcessTreeGuard::for_child(&child); + let mut stdin = child.stdin.take().expect("piped stdin"); + stdin + .write_all(b"{\"id\":1}\n") + .await + .expect("write protocol input"); + drop(stdin); + let output = tokio::time::timeout(Duration::from_secs(10), child.wait_with_output()) + .await + .expect("provider deadline") + .expect("provider output"); + guard.disarm(); + assert!( + output.status.success(), + "provider stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let text = String::from_utf8(output.stdout).expect("UTF-8 output"); + for arg in args { + assert!( + text.contains(&format!("ARG={arg:?}")), + "argument changed: {arg:?}: {text}" + ); + } + assert!(text.contains(&format!("STDIN={:?}", "{\"id\":1}\n"))); + assert!(text.contains("OVERLAY=explicit")); + } + + #[tokio::test] + async fn native_provider_launch_preserves_arguments_stdin_and_exit_status() { + let root = tempfile::Builder::new() + .prefix("sdk provider space ") + .tempdir() + .expect("fixture directory"); + let binary = native_fixture(root.path()); + assert_launch(&binary, root.path()).await; + + // npm-installed provider CLIs on Windows are frequently .cmd shims. + // Rust owns batch-file escaping; do not interpolate a shell command. + #[cfg(windows)] + for name in ["claude", "codex", "opencode"] { + let shim = root.path().join(format!("{name}.cmd")); + std::fs::write( + &shim, + format!("@echo off\r\n\"{}\" %*\r\n", binary.display()), + ) + .expect("write cmd shim"); + assert_launch(&shim, root.path()).await; + } + + let mut spec = CommandSpec::new(&binary); + spec.args = vec!["--exit".into()]; + let child = spawn(&spec, root.path()).expect("failure fixture"); + let output = tokio::time::timeout(Duration::from_secs(10), child.wait_with_output()) + .await + .expect("exit deadline") + .expect("failure output"); + assert_eq!(output.status.code(), Some(17)); + assert!(String::from_utf8_lossy(&output.stderr).contains("fixture failure")); + + spec.args = vec!["--wait".into()]; + let mut child = spawn(&spec, root.path()).expect("cancellation fixture"); + let mut guard = ProcessTreeGuard::for_child(&child); + guard.terminate(); + let status = tokio::time::timeout(Duration::from_secs(10), child.wait()) + .await + .expect("termination deadline") + .expect("termination status"); + assert!(!status.success()); + } +} diff --git a/src/ssh.rs b/src/ssh.rs index 6768b8c..1fb7f3b 100644 --- a/src/ssh.rs +++ b/src/ssh.rs @@ -2,7 +2,9 @@ use std::collections::BTreeMap; use std::ffi::{OsStr, OsString}; +#[cfg(unix)] use std::fs::OpenOptions; +#[cfg(unix)] use std::io::Write; use std::path::{Path, PathBuf}; use std::pin::Pin; @@ -27,6 +29,7 @@ use crate::{ const STDERR_CAPTURE_BYTES: usize = 32 * 1024; const VERSION_PROBE_TIMEOUT: Duration = Duration::from_secs(8); static NEXT_PROCESS_ID: AtomicU64 = AtomicU64::new(1); +#[cfg(unix)] static NEXT_ASKPASS_ID: AtomicU64 = AtomicU64::new(1); const ASKPASS_PASSWORD_ENV: &str = "TEMPS_AGENT_RUNTIME_SSH_PASSWORD"; const LOGIN_ENVIRONMENT_MARKER: &[u8] = b"\0TEMPS_AGENT_RUNTIME_LOGIN_ENV\0"; From b6b7cac70105685f2dff0e4ed228c70aa56fe575 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Tue, 22 Sep 2026 00:23:04 +0200 Subject: [PATCH 2/6] fix: accept intentional attached provider shutdown after turn completion --- docs/how-to/manage-background-processes.md | 8 ++++++++ src/runtime.rs | 14 ++++++++++++-- tests/opencode_serve.rs | 5 +++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/docs/how-to/manage-background-processes.md b/docs/how-to/manage-background-processes.md index ffb50d4..ea7fbfc 100644 --- a/docs/how-to/manage-background-processes.md +++ b/docs/how-to/manage-background-processes.md @@ -107,3 +107,11 @@ For agent-launched descendants, see [Keep tool processes running](keep-tool-processes-running.md). For durable projection rules, see [Persist command and tool execution](persist-command-execution.md). + +### Provider server shutdown + +For attached providers such as `opencode serve`, the runtime stops the server +after a terminal protocol event. A signal or nonzero exit caused by that +intentional shutdown does not turn a completed turn into a failure. Provider +errors and an unexpected stream closure still fail the turn; cancellation +continues to abort the session before terminating its process. diff --git a/src/runtime.rs b/src/runtime.rs index 5353849..769c65b 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -2409,6 +2409,7 @@ impl AgentRuntime { } }; let mut lines = BufReader::new(reader).lines(); + let mut protocol_completed = false; loop { let line = tokio::select! { _ = request.cancellation.cancelled() => { @@ -2510,6 +2511,7 @@ impl AgentRuntime { } } if output.terminal { + protocol_completed = true; stdin.take(); if attached { // A stdio provider is read to end-of-output because @@ -2537,10 +2539,18 @@ impl AgentRuntime { .map_err(|source| RuntimeError::Transport { provider, source })?; match tokio::time::timeout(ATTACHED_SHUTDOWN_GRACE, process.wait()).await { Ok(status) => { - status.map_err(|source| RuntimeError::Transport { provider, source })? + let status = status.map_err(|source| RuntimeError::Transport { provider, source })?; + // An intentional server shutdown can exit by signal (Unix) + // or a nonzero termination code (Windows). Only a terminal + // protocol event makes that expected; EOF alone is not success. + if protocol_completed { + TransportExitStatus { success: true, code: None } + } else { + status + } } Err(_) => TransportExitStatus { - success: true, + success: protocol_completed, code: None, }, } diff --git a/tests/opencode_serve.rs b/tests/opencode_serve.rs index d1def42..77868da 100644 --- a/tests/opencode_serve.rs +++ b/tests/opencode_serve.rs @@ -227,8 +227,9 @@ impl TransportProcessControl for Control { std::future::pending::<()>().await; } Ok(TransportExitStatus { - success: true, - code: Some(0), + // Real servers stopped by the runtime exit by signal/nonzero. + success: false, + code: None, }) } From eee2866885bdd684b2aa46fffdd5e76fef99be6c Mon Sep 17 00:00:00 2001 From: David Viejo Date: Tue, 22 Sep 2026 03:37:30 +0200 Subject: [PATCH 3/6] fix: resolve Windows launchers and probe OpenCode health [skip ci] --- docs/how-to/test-platform-compatibility.md | 5 +++ src/process.rs | 40 +++++++++++++++++++++- src/providers/opencode_http.rs | 10 +++--- tests/opencode_serve.rs | 8 +++-- 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/docs/how-to/test-platform-compatibility.md b/docs/how-to/test-platform-compatibility.md index 18dbfc5..a64fd22 100644 --- a/docs/how-to/test-platform-compatibility.md +++ b/docs/how-to/test-platform-compatibility.md @@ -48,3 +48,8 @@ A successful cross-compilation or fixture run does not certify these journeys. sandbox to make a platform test pass. - Windows OS-service installation is an application responsibility, outside this library's provider launch contract. + +Real CLI checks now also cover extensionless npm command names on Windows, +resolved against the child PATH before spawning. OpenCode serve readiness uses +`/global/health` and requires a healthy JSON response; `/app` is a web UI route +in current versions and is not an API readiness check. Each probe is bounded. diff --git a/src/process.rs b/src/process.rs index e89b091..e1692a4 100644 --- a/src/process.rs +++ b/src/process.rs @@ -65,8 +65,37 @@ fn allowed_environment_key(name: &OsStr, windows: bool) -> bool { } } +#[cfg(windows)] +fn windows_program(spec: &CommandSpec) -> std::path::PathBuf { + // CreateProcess only searches .exe for a bare name. npm installs .cmd + // launchers; resolve those against the effective child PATH before letting + // Rust handle batch-file argument escaping. + if spec.program.components().count() != 1 || spec.program.extension().is_some() { + return spec.program.clone(); + } + let path = spec.environment.iter() + .find(|(key, _)| key.to_string_lossy().eq_ignore_ascii_case("PATH")) + .map(|(_, value)| value.clone()) + .or_else(|| std::env::var_os("PATH")); + if let Some(path) = path { + for directory in std::env::split_paths(&path) { + for extension in ["exe", "cmd", "bat", "com"] { + let candidate = directory.join(&spec.program).with_extension(extension); + if candidate.is_file() { + return candidate; + } + } + } + } + spec.program.clone() +} + pub(crate) fn spawn(spec: &CommandSpec, cwd: &std::path::Path) -> std::io::Result { - let mut command = Command::new(&spec.program); + #[cfg(windows)] + let program = windows_program(spec); + #[cfg(not(windows))] + let program = spec.program.clone(); + let mut command = Command::new(program); command.args(&spec.args).current_dir(cwd); if spec.clear_environment { let preserved = @@ -266,6 +295,14 @@ mod tests { ]; let mut spec = CommandSpec::new(program); spec.args = args.iter().map(std::ffi::OsString::from).collect(); + // Exercise bare-name discovery with a per-command PATH, without + // mutating the test process environment or borrowing installed CLIs. + #[cfg(windows)] + if program.components().count() == 1 { + let inherited = std::env::var_os("PATH").unwrap_or_default(); + let paths = std::iter::once(cwd.to_path_buf()).chain(std::env::split_paths(&inherited)); + spec.environment.insert("Path".into(), std::env::join_paths(paths).unwrap()); + } spec.environment .insert("SDK_LAUNCH_TEST".into(), "explicit".into()); let mut child = spawn(&spec, cwd).expect("launch provider fixture"); @@ -317,6 +354,7 @@ mod tests { ) .expect("write cmd shim"); assert_launch(&shim, root.path()).await; + assert_launch(std::path::Path::new(name), root.path()).await; } let mut spec = CommandSpec::new(&binary); diff --git a/src/providers/opencode_http.rs b/src/providers/opencode_http.rs index 473dc70..600ca51 100644 --- a/src/providers/opencode_http.rs +++ b/src/providers/opencode_http.rs @@ -179,8 +179,8 @@ async fn run_bridge(port: u16, incoming: DuplexStream, outgoing: DuplexStream) { /// Poll a cheap, always-safe endpoint until the server answers. async fn wait_until_ready(port: u16) -> std::result::Result<(), String> { for attempt in 0..READINESS_ATTEMPTS { - match perform(port, "GET", "/app", None).await { - Ok((status, _)) if status < 500 => return Ok(()), + match tokio::time::timeout(Duration::from_secs(1), perform(port, "GET", "/global/health", None)).await { + Ok(Ok((200, body))) if body.get("healthy").and_then(Value::as_bool) == Some(true) => return Ok(()), // Connection refused while the server is still binding its port is // expected for the first attempts. _ => {} @@ -650,8 +650,10 @@ mod tests { 1\r\n\n\r\n" } else if request.starts_with("POST") { b"HTTP/1.1 200 OK\r\nContent-Length: 14\r\n\r\n{\"ok\":\"yes\"}\r\n" + } else if request.starts_with("GET /global/health ") { + b"HTTP/1.1 200 OK\r\nContent-Length: 16\r\n\r\n{\"healthy\":true}" } else { - b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}" + b"HTTP/1.1 404 Not Found\r\nContent-Length: 2\r\n\r\n{}" }; let _ = socket.write_all(response).await; let _ = socket.flush().await; @@ -725,7 +727,7 @@ mod tests { return; } let _ = socket - .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}") + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 16\r\n\r\n{\"healthy\":true}") .await; } }); diff --git a/tests/opencode_serve.rs b/tests/opencode_serve.rs index 77868da..8d7e4eb 100644 --- a/tests/opencode_serve.rs +++ b/tests/opencode_serve.rs @@ -1,7 +1,7 @@ //! Served-OpenCode coverage against a scripted `opencode serve`. //! //! The fixture is a real HTTP server bound to the very port the adapter -//! reserved for the turn, speaking the real protocol: `/app` for readiness, +//! reserved for the turn, speaking the real protocol: `/global/health` for readiness, //! `/session` to open one, `/event` as a chunked Server-Sent Events stream, //! `/session/{id}/message` to prompt, and //! `/session/{id}/permissions/{id}` to answer a permission. Nothing here @@ -287,7 +287,11 @@ async fn handle(mut socket: TcpStream, script: Script, requests: ArcOpenCode UI".to_string() + } else if path.starts_with("/session?") || path == "/session" { json!({"id": "session-fixture", "title": "Fixture session"}).to_string() } else { json!({}).to_string() From a781c295d1aa759e7c57e6df508d933f3dccfe11 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Tue, 22 Sep 2026 03:41:12 +0200 Subject: [PATCH 4/6] fix: forward Fleet instructions to Codex app-server [skip ci] --- docs/how-to/test-platform-compatibility.md | 4 ++++ src/providers/codex.rs | 1 + src/providers/codex_app_server.rs | 3 +++ tests/codex_app_server.rs | 16 ++++++++++++++++ 4 files changed, 24 insertions(+) diff --git a/docs/how-to/test-platform-compatibility.md b/docs/how-to/test-platform-compatibility.md index a64fd22..3f12eb5 100644 --- a/docs/how-to/test-platform-compatibility.md +++ b/docs/how-to/test-platform-compatibility.md @@ -53,3 +53,7 @@ Real CLI checks now also cover extensionless npm command names on Windows, resolved against the child PATH before spawning. OpenCode serve readiness uses `/global/health` and requires a healthy JSON response; `/app` is a web UI route in current versions and is not an API readiness check. Each probe is bounded. + +Fleet launch-context checks include additional developer instructions on new +and resumed Codex app-server threads; this preserves Fleet model identity +instructions without rejecting the first turn before it reaches the provider. diff --git a/src/providers/codex.rs b/src/providers/codex.rs index e472680..5145a09 100644 --- a/src/providers/codex.rs +++ b/src/providers/codex.rs @@ -658,6 +658,7 @@ impl AgentAdapter for Codex { fn launch_context_capabilities(&self) -> LaunchContextCapabilities { LaunchContextCapabilities { + system_prompt_append: self.app_server_mode(), stdio_mcp: true, http_mcp: true, ..LaunchContextCapabilities::default() diff --git a/src/providers/codex_app_server.rs b/src/providers/codex_app_server.rs index cfc4baa..d31a35c 100644 --- a/src/providers/codex_app_server.rs +++ b/src/providers/codex_app_server.rs @@ -145,6 +145,9 @@ pub(super) fn prepare_turn(request: &TurnRequest, state: &mut AdapterState) -> R "sandbox": sandbox, "approvalPolicy": approval, }); + if let Some(instructions) = request.launch_context.system_prompt_append.as_deref() { + thread_params["developerInstructions"] = json!(instructions); + } if let Some(model) = request.model.as_deref() { thread_params["model"] = json!(model); } diff --git a/tests/codex_app_server.rs b/tests/codex_app_server.rs index 82cbd90..9525d8c 100644 --- a/tests/codex_app_server.rs +++ b/tests/codex_app_server.rs @@ -694,3 +694,19 @@ async fn a_resumed_thread_reuses_its_identifier_without_restarting_the_session() "a resumed thread is not a new session" ); } + +#[tokio::test] +async fn developer_instructions_reach_new_and_resumed_threads() { + for resumed in [false, true] { + let transport = AppServer::new(Script::Approval); + let runtime = runtime(transport.clone()); + let responder = Responder::new(ApprovalDecision::Allow, None); + let mut request = request(); + let instructions = "Fleet identity\nKeep \"quoted\" instructions intact."; + request.launch_context.system_prompt_append = Some(instructions.into()); + if resumed { request.session_id = Some("thread-fixture".into()); } + runtime.run(request, &Collector::default(), Some(&responder)).await.unwrap(); + let method = if resumed { "thread/resume" } else { "thread/start" }; + assert_eq!(transport.method_frame(method).unwrap()["params"]["developerInstructions"], instructions); + } +} From 4632bf590c9d6cf5d799e6859bc6234e91688a44 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Wed, 23 Sep 2026 11:48:48 +0200 Subject: [PATCH 5/6] style: format Windows compatibility changes --- src/process.rs | 7 +++++-- src/providers/opencode_http.rs | 11 +++++++++-- src/runtime.rs | 8 ++++++-- tests/codex_app_server.rs | 20 ++++++++++++++++---- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/process.rs b/src/process.rs index e1692a4..e1be9ec 100644 --- a/src/process.rs +++ b/src/process.rs @@ -73,7 +73,9 @@ fn windows_program(spec: &CommandSpec) -> std::path::PathBuf { if spec.program.components().count() != 1 || spec.program.extension().is_some() { return spec.program.clone(); } - let path = spec.environment.iter() + let path = spec + .environment + .iter() .find(|(key, _)| key.to_string_lossy().eq_ignore_ascii_case("PATH")) .map(|(_, value)| value.clone()) .or_else(|| std::env::var_os("PATH")); @@ -301,7 +303,8 @@ mod tests { if program.components().count() == 1 { let inherited = std::env::var_os("PATH").unwrap_or_default(); let paths = std::iter::once(cwd.to_path_buf()).chain(std::env::split_paths(&inherited)); - spec.environment.insert("Path".into(), std::env::join_paths(paths).unwrap()); + spec.environment + .insert("Path".into(), std::env::join_paths(paths).unwrap()); } spec.environment .insert("SDK_LAUNCH_TEST".into(), "explicit".into()); diff --git a/src/providers/opencode_http.rs b/src/providers/opencode_http.rs index 600ca51..fd288c3 100644 --- a/src/providers/opencode_http.rs +++ b/src/providers/opencode_http.rs @@ -179,8 +179,15 @@ async fn run_bridge(port: u16, incoming: DuplexStream, outgoing: DuplexStream) { /// Poll a cheap, always-safe endpoint until the server answers. async fn wait_until_ready(port: u16) -> std::result::Result<(), String> { for attempt in 0..READINESS_ATTEMPTS { - match tokio::time::timeout(Duration::from_secs(1), perform(port, "GET", "/global/health", None)).await { - Ok(Ok((200, body))) if body.get("healthy").and_then(Value::as_bool) == Some(true) => return Ok(()), + match tokio::time::timeout( + Duration::from_secs(1), + perform(port, "GET", "/global/health", None), + ) + .await + { + Ok(Ok((200, body))) if body.get("healthy").and_then(Value::as_bool) == Some(true) => { + return Ok(()) + } // Connection refused while the server is still binding its port is // expected for the first attempts. _ => {} diff --git a/src/runtime.rs b/src/runtime.rs index 769c65b..5495e68 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -2539,12 +2539,16 @@ impl AgentRuntime { .map_err(|source| RuntimeError::Transport { provider, source })?; match tokio::time::timeout(ATTACHED_SHUTDOWN_GRACE, process.wait()).await { Ok(status) => { - let status = status.map_err(|source| RuntimeError::Transport { provider, source })?; + let status = + status.map_err(|source| RuntimeError::Transport { provider, source })?; // An intentional server shutdown can exit by signal (Unix) // or a nonzero termination code (Windows). Only a terminal // protocol event makes that expected; EOF alone is not success. if protocol_completed { - TransportExitStatus { success: true, code: None } + TransportExitStatus { + success: true, + code: None, + } } else { status } diff --git a/tests/codex_app_server.rs b/tests/codex_app_server.rs index 9525d8c..b65b0b8 100644 --- a/tests/codex_app_server.rs +++ b/tests/codex_app_server.rs @@ -704,9 +704,21 @@ async fn developer_instructions_reach_new_and_resumed_threads() { let mut request = request(); let instructions = "Fleet identity\nKeep \"quoted\" instructions intact."; request.launch_context.system_prompt_append = Some(instructions.into()); - if resumed { request.session_id = Some("thread-fixture".into()); } - runtime.run(request, &Collector::default(), Some(&responder)).await.unwrap(); - let method = if resumed { "thread/resume" } else { "thread/start" }; - assert_eq!(transport.method_frame(method).unwrap()["params"]["developerInstructions"], instructions); + if resumed { + request.session_id = Some("thread-fixture".into()); + } + runtime + .run(request, &Collector::default(), Some(&responder)) + .await + .unwrap(); + let method = if resumed { + "thread/resume" + } else { + "thread/start" + }; + assert_eq!( + transport.method_frame(method).unwrap()["params"]["developerInstructions"], + instructions + ); } } From 8c1f251bc6049e7ba6a855e201df2a729cf74fc4 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Wed, 23 Sep 2026 11:53:23 +0200 Subject: [PATCH 6/6] fix: preserve early command rejection diagnostics --- src/process.rs | 13 +++++++ src/runtime.rs | 97 +++++++++++++++++++++++++++++--------------------- 2 files changed, 70 insertions(+), 40 deletions(-) diff --git a/src/process.rs b/src/process.rs index e1be9ec..1fb5836 100644 --- a/src/process.rs +++ b/src/process.rs @@ -294,6 +294,10 @@ mod tests { r#"{"mcpServers":{"test":{"url":"http://127.0.0.1:1234/mcp"}}}"#, "project with spaces", "héllo", + "a&b|ce^f", + "%PATH% !SDK_LAUNCH_TEST!", + "a\"b & echo injected", + "trailing slash\\", ]; let mut spec = CommandSpec::new(program); spec.args = args.iter().map(std::ffi::OsString::from).collect(); @@ -358,6 +362,15 @@ mod tests { .expect("write cmd shim"); assert_launch(&shim, root.path()).await; assert_launch(std::path::Path::new(name), root.path()).await; + // Batch files cannot represent literal newlines safely. Rust must + // reject them before starting the shell instead of interpreting + // the remainder as another command. + for argument in ["line\nnext", "line\rnext"] { + let mut invalid = CommandSpec::new(&shim); + invalid.args.push(argument.into()); + let error = spawn(&invalid, root.path()).expect_err("reject batch newline"); + assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); + } } let mut spec = CommandSpec::new(&binary); diff --git a/src/runtime.rs b/src/runtime.rs index 5495e68..9146764 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1136,36 +1136,26 @@ impl AgentRuntime { working_directory, }) .await?; - if let Some(initial) = initial_stdin { - let Some(mut stdin) = process.take_stdin() else { - let _ = process.terminate().await; - return Err(extension_transport_error( - self.transport.name(), - operation, - "the extension command did not expose stdin", - false, - )); - }; - stdin.write_all(&initial).await.map_err(|error| { - extension_transport_error( - self.transport.name(), - operation, - format!("could not write extension content: {error}"), - true, - ) - })?; - stdin.flush().await.map_err(|error| { - extension_transport_error( - self.transport.name(), - operation, - format!("could not flush extension content: {error}"), - true, - ) - })?; - drop(stdin); - } else { - drop(process.take_stdin()); - } + // A rejecting command may close stdin before reading the content. + // Keep its exit status and stderr authoritative rather than returning + // a timing-dependent broken-pipe error and hiding the rejection. + let stdin = process.take_stdin(); + let write_input = async move { + if let Some(initial) = initial_stdin { + let Some(mut stdin) = stdin else { + return Err("the extension command did not expose stdin".to_string()); + }; + stdin + .write_all(&initial) + .await + .map_err(|error| format!("could not write extension content: {error}"))?; + stdin + .flush() + .await + .map_err(|error| format!("could not flush extension content: {error}"))?; + } + Ok::<_, String>(()) + }; let Some(stdout) = process.take_stdout() else { let _ = process.terminate().await; return Err(extension_transport_error( @@ -1179,16 +1169,24 @@ impl AgentRuntime { .take_stderr() .map(|stderr| tokio::spawn(crate::process::bounded_stderr(stderr, STDERR_TAIL_BYTES))); let run = async { - let mut output = Vec::new(); - stdout - .take((max_bytes + 1) as u64) - .read_to_end(&mut output) - .await - .map_err(|error| error.to_string())?; - if output.len() > max_bytes { - return Err(format!("command output exceeded {max_bytes} bytes")); - } + let read_output = async { + let mut output = Vec::new(); + stdout + .take((max_bytes + 1) as u64) + .read_to_end(&mut output) + .await + .map_err(|error| error.to_string())?; + if output.len() > max_bytes { + return Err(format!("command output exceeded {max_bytes} bytes")); + } + Ok::<_, String>(output) + }; + let (output, input_result) = tokio::join!(read_output, write_input); + let output = output?; let status = process.wait().await.map_err(|error| error.message)?; + if status.success { + input_result?; + } Ok::<_, String>((output, status)) }; let result = tokio::time::timeout(timeout, run).await; @@ -3264,10 +3262,29 @@ mod tests { .expect_err("a project-controlled symlink must not redirect skill writes"); assert_eq!(error.kind, TransportErrorKind::ProcessControlFailed); - assert!(error.message.contains("symlink component")); + assert!(error.message.contains("symlink component"), "{error:?}"); assert!(!outside.path().join("skills/review/SKILL.md").exists()); } + #[cfg(unix)] + #[tokio::test] + async fn extension_rejection_preserves_stderr_when_stdin_is_not_consumed() { + let workspace = tempfile::tempdir().unwrap(); + let runtime = AgentRuntime::builder().build().unwrap(); + let mut command = CommandSpec::new("/bin/sh"); + command.args = vec![ + "-c".into(), + "printf 'specific rejection' >&2; exit 78".into(), + ]; + command.initial_stdin = Some(vec![b'x'; 1024 * 1024]); + let error = runtime + .execute_extension_command(command, workspace.path().into(), "test_rejection") + .await + .expect_err("the command rejects input"); + assert!(error.message.contains("specific rejection"), "{error:?}"); + assert!(!error.retryable); + } + #[test] fn launch_context_requires_explicit_mcp_secret_sources() { let mut request = TurnRequest::new(Provider::Claude, ".", "test");