From 48eca1505e823615088394ef10b47acf87c0fe19 Mon Sep 17 00:00:00 2001 From: obchain <167975049+obchain@users.noreply.github.com> Date: Tue, 23 Jun 2026 00:44:36 +0530 Subject: [PATCH] feat(shell): add hide_window option to suppress Windows console window (#3747) Co-authored-by: Steven Enamakel --- .env.example | 4 + .../agent/harness/session/builder/factory.rs | 5 +- src/openhuman/agent/host_runtime.rs | 117 +++++++++++++++--- src/openhuman/channels/runtime/startup.rs | 6 +- src/openhuman/config/mod.rs | 2 +- .../config/schema/load/env_overlay.rs | 26 ++++ src/openhuman/config/schema/load_tests.rs | 51 ++++++++ src/openhuman/config/schema/mod.rs | 4 +- src/openhuman/config/schema/runtime.rs | 38 ++++++ src/openhuman/config/schema/types.rs | 4 + src/openhuman/inference/local/mod.rs | 5 +- tests/inference_agent_raw_coverage_e2e.rs | 47 ++++--- 12 files changed, 264 insertions(+), 45 deletions(-) diff --git a/.env.example b/.env.example index 10aeb641d..64fde524c 100644 --- a/.env.example +++ b/.env.example @@ -123,6 +123,10 @@ OPENHUMAN_BROWSER_ALLOW_ALL=0 OPENHUMAN_LOG_PROMPTS=0 # [optional] Enable reasoning mode OPENHUMAN_REASONING_ENABLED= +# [optional] Windows only: suppress the console window that flashes when the +# shell tool spawns child processes (CREATE_NO_WINDOW). Default: false. +# Equivalent TOML: [shell] hide_window = true +OPENHUMAN_SHELL_HIDE_WINDOW= # --------------------------------------------------------------------------- # Web search diff --git a/src/openhuman/agent/harness/session/builder/factory.rs b/src/openhuman/agent/harness/session/builder/factory.rs index f3d0d2680..916be0001 100644 --- a/src/openhuman/agent/harness/session/builder/factory.rs +++ b/src/openhuman/agent/harness/session/builder/factory.rs @@ -315,8 +315,9 @@ impl Agent { "[profiles] applying per-profile session gate" ); } - let runtime: Arc = - Arc::from(host_runtime::create_runtime(&config.runtime)?); + let runtime: Arc = Arc::from( + host_runtime::create_runtime(&config.runtime, config.shell.hide_window)?, + ); let security = Arc::new(SecurityPolicy::from_config( &config.autonomy, &config.workspace_dir, diff --git a/src/openhuman/agent/host_runtime.rs b/src/openhuman/agent/host_runtime.rs index d7c5ba022..51c5d15bd 100644 --- a/src/openhuman/agent/host_runtime.rs +++ b/src/openhuman/agent/host_runtime.rs @@ -23,17 +23,24 @@ pub trait RuntimeAdapter: Send + Sync { ) -> anyhow::Result; } -pub struct NativeRuntime; - -impl Default for NativeRuntime { - fn default() -> Self { - Self::new() - } +#[derive(Default)] +pub struct NativeRuntime { + /// When true, shell-family child processes are spawned with the Windows + /// `CREATE_NO_WINDOW` flag so no console window flashes. Sourced from + /// `[shell] hide_window` (#3727/#3728). No effect on macOS/Linux. + hide_window: bool, } impl NativeRuntime { + /// A native runtime with default behaviour (no window suppression). pub const fn new() -> Self { - Self + Self { hide_window: false } + } + + /// A native runtime that suppresses the Windows console window for spawned + /// shell child processes when `hide_window` is true. + pub const fn with_hide_window(hide_window: bool) -> Self { + Self { hide_window } } } @@ -101,10 +108,35 @@ impl RuntimeAdapter for NativeRuntime { // (#3353, Fix 2) crate::openhuman::config::ensure_usable_cwd(workspace_dir)?; cmd.current_dir(workspace_dir); + // Optionally suppress the Windows console window for this child process + // (`[shell] hide_window`). No-op when disabled and on non-Windows hosts. + maybe_hide_window(&mut cmd, self.hide_window); Ok(cmd) } } +/// Suppress the Windows console window for a shell child process when `hide` is +/// set, by applying the `CREATE_NO_WINDOW` creation flag (`0x08000000`). No-op +/// when `hide` is false, and on non-Windows hosts the flag does not exist so this +/// does nothing regardless. Delegates to the shared [`apply_no_window`] helper so +/// there is a single source of truth for the flag (#3727/#3728). +fn maybe_hide_window(cmd: &mut tokio::process::Command, hide: bool) { + tracing::trace!( + hide_window = hide, + windows = cfg!(windows), + "[agent][runtime] hide_window evaluated for shell child process" + ); + if !hide { + return; + } + crate::openhuman::inference::local::process_util::apply_no_window(cmd); + #[cfg(windows)] + tracing::debug!( + creation_flags = "0x08000000", + "[agent][runtime] applied CREATE_NO_WINDOW to shell child process" + ); +} + /// Locate a `bash` binary once (cached — this is hit on every shell call) for /// the `pipefail` wrapper in [`NativeRuntime::build_shell_command`]. Returns /// `None` on hosts without bash (e.g. minimal containers), where we fall back @@ -195,9 +227,15 @@ impl RuntimeAdapter for DockerRuntime { } } -pub fn create_runtime(config: &RuntimeConfig) -> anyhow::Result> { +/// Build the runtime adapter for the configured `kind`. `hide_window` comes +/// from `[shell] hide_window` and only affects the native runtime on Windows +/// (the docker runtime spawns the `docker` client, out of scope here). +pub fn create_runtime( + config: &RuntimeConfig, + hide_window: bool, +) -> anyhow::Result> { match config.kind.as_str() { - "native" => Ok(Box::new(NativeRuntime::new())), + "native" => Ok(Box::new(NativeRuntime::with_hide_window(hide_window))), "docker" => Ok(Box::new(DockerRuntime::new(config.docker.clone()))), other => anyhow::bail!("Unsupported runtime kind: {other}"), } @@ -285,26 +323,67 @@ mod tests { #[test] fn create_runtime_supports_native_and_docker_and_rejects_unknown() { - let native = create_runtime(&RuntimeConfig::default()).unwrap(); + let native = create_runtime(&RuntimeConfig::default(), false).unwrap(); assert_eq!(native.name(), "native"); - let docker = create_runtime(&RuntimeConfig { - kind: "docker".into(), - docker: DockerRuntimeConfig::default(), - ..RuntimeConfig::default() - }) + let docker = create_runtime( + &RuntimeConfig { + kind: "docker".into(), + docker: DockerRuntimeConfig::default(), + ..RuntimeConfig::default() + }, + false, + ) .unwrap(); assert_eq!(docker.name(), "docker"); - let err = create_runtime(&RuntimeConfig { - kind: "vm".into(), - ..RuntimeConfig::default() - }) + let err = create_runtime( + &RuntimeConfig { + kind: "vm".into(), + ..RuntimeConfig::default() + }, + false, + ) .err() .unwrap(); assert!(err.to_string().contains("Unsupported runtime kind: vm")); } + /// `[shell] hide_window` plumbs through `create_runtime` into the native + /// adapter, and a hide-window native runtime still builds a usable shell + /// command on every platform (the `CREATE_NO_WINDOW` flag is Windows-only + /// and applied without disturbing the command on macOS/Linux). + #[test] + fn native_runtime_with_hide_window_still_builds_shell_command() { + let native = create_runtime(&RuntimeConfig::default(), true).unwrap(); + assert_eq!(native.name(), "native"); + + let runtime = NativeRuntime::with_hide_window(true); + let command = runtime + .build_shell_command("echo hi", Path::new("/tmp")) + .expect("hide_window should not break command construction"); + assert_eq!(command.as_std().get_current_dir(), Some(Path::new("/tmp"))); + + // The program/args are identical with and without the flag — hiding the + // window must not alter what is executed. + let plain = NativeRuntime::with_hide_window(false) + .build_shell_command("echo hi", Path::new("/tmp")) + .unwrap(); + assert_eq!(command.as_std().get_program(), plain.as_std().get_program()); + } + + /// `maybe_hide_window` is a no-op when disabled (and on non-Windows hosts + /// even when enabled), and must never panic. + #[test] + fn maybe_hide_window_is_safe_no_op() { + let mut disabled = tokio::process::Command::new("echo"); + maybe_hide_window(&mut disabled, false); + let mut enabled = tokio::process::Command::new("echo"); + maybe_hide_window(&mut enabled, true); + assert_eq!(disabled.as_std().get_program(), "echo"); + assert_eq!(enabled.as_std().get_program(), "echo"); + } + /// Regression: a failed stage in a pipeline must surface as a non-zero exit /// (pipefail), so the harness records the call as failed and the /// repeated-failure circuit breaker can trip — rather than `… | tail` diff --git a/src/openhuman/channels/runtime/startup.rs b/src/openhuman/channels/runtime/startup.rs index e0d3b96b5..77843dc93 100644 --- a/src/openhuman/channels/runtime/startup.rs +++ b/src/openhuman/channels/runtime/startup.rs @@ -253,8 +253,10 @@ pub async fn start_channels(mut config: Config) -> Result<()> { tracing::warn!("Provider warmup failed (non-fatal): {e}"); } - let runtime: Arc = - Arc::from(host_runtime::create_runtime(&config.runtime)?); + let runtime: Arc = Arc::from(host_runtime::create_runtime( + &config.runtime, + config.shell.hide_window, + )?); // Create the agent's action sandbox + default projects home and register the // projects dir as a ReadWrite trusted root. Shared with the always-run // `bootstrap_core_runtime` boot so a fresh install gets these dirs even with diff --git a/src/openhuman/config/mod.rs b/src/openhuman/config/mod.rs index f4ef99921..f89ba9e3a 100644 --- a/src/openhuman/config/mod.rs +++ b/src/openhuman/config/mod.rs @@ -44,7 +44,7 @@ pub use schema::{ ReflectionSource, ReliabilityConfig, ResourceLimitsConfig, RuntimeConfig, SandboxBackend, SandboxConfig, SchedulerConfig, SchedulerGateConfig, SchedulerGateMode, ScreenIntelligenceConfig, SearchConfig, SearchEngine, SearchEngineCredentials, SearxngConfig, - SecretsConfig, SecurityConfig, SlackConfig, StorageConfig, StorageProviderConfig, + SecretsConfig, SecurityConfig, ShellConfig, SlackConfig, StorageConfig, StorageProviderConfig, StorageProviderSection, StreamMode, TeamModelConfig, TelegramConfig, UpdateConfig, UpdateRestartStrategy, VoiceActivationMode, VoiceServerConfig, WebSearchConfig, WebhookConfig, DEFAULT_CLOUD_LLM_MODEL, DEFAULT_MEMORY_SYNC_INTERVAL_SECS, DEFAULT_MODEL, diff --git a/src/openhuman/config/schema/load/env_overlay.rs b/src/openhuman/config/schema/load/env_overlay.rs index 779f7ae27..c30bc4f1c 100644 --- a/src/openhuman/config/schema/load/env_overlay.rs +++ b/src/openhuman/config/schema/load/env_overlay.rs @@ -118,6 +118,32 @@ impl Config { } } + if let Some(flag) = env.get_any(&["OPENHUMAN_SHELL_HIDE_WINDOW", "SHELL_HIDE_WINDOW"]) { + let normalized = flag.trim().to_ascii_lowercase(); + match normalized.as_str() { + "1" | "true" | "yes" | "on" => { + self.shell.hide_window = true; + tracing::debug!( + value = %flag, + "[config][shell] OPENHUMAN_SHELL_HIDE_WINDOW applied: hide_window=true" + ); + } + "0" | "false" | "no" | "off" => { + self.shell.hide_window = false; + tracing::debug!( + value = %flag, + "[config][shell] OPENHUMAN_SHELL_HIDE_WINDOW applied: hide_window=false" + ); + } + _ => tracing::warn!( + value = %flag, + "[config][shell] OPENHUMAN_SHELL_HIDE_WINDOW unrecognized value ignored; \ + keeping current hide_window={}", + self.shell.hide_window + ), + } + } + self.apply_search_env(env); self.apply_proxy_env(env); self.apply_runtime_env(env); diff --git a/src/openhuman/config/schema/load_tests.rs b/src/openhuman/config/schema/load_tests.rs index 03ab70dfe..5e145854d 100644 --- a/src/openhuman/config/schema/load_tests.rs +++ b/src/openhuman/config/schema/load_tests.rs @@ -187,6 +187,57 @@ fn apply_env_overrides_reasoning_enabled_parses_truthy_falsy() { } } +#[test] +fn apply_env_overrides_shell_hide_window_parses_truthy_falsy() { + let _g = env_lock(); + clear_env(&["OPENHUMAN_SHELL_HIDE_WINDOW", "SHELL_HIDE_WINDOW"]); + let mut cfg = Config::default(); + assert!(!cfg.shell.hide_window, "default should be off"); + + unsafe { + std::env::set_var("OPENHUMAN_SHELL_HIDE_WINDOW", "on"); + } + cfg.apply_env_overrides(); + assert!(cfg.shell.hide_window); + + unsafe { + std::env::set_var("OPENHUMAN_SHELL_HIDE_WINDOW", "false"); + } + cfg.apply_env_overrides(); + assert!(!cfg.shell.hide_window); + + // The unprefixed alias `SHELL_HIDE_WINDOW` is honored too. + unsafe { + std::env::remove_var("OPENHUMAN_SHELL_HIDE_WINDOW"); + std::env::set_var("SHELL_HIDE_WINDOW", "on"); + } + cfg.apply_env_overrides(); + assert!(cfg.shell.hide_window, "alias should set hide_window"); + + // The namespaced var takes precedence over the alias when both are set. + unsafe { + std::env::set_var("OPENHUMAN_SHELL_HIDE_WINDOW", "off"); + std::env::set_var("SHELL_HIDE_WINDOW", "on"); + } + cfg.apply_env_overrides(); + assert!( + !cfg.shell.hide_window, + "OPENHUMAN_-prefixed var should win over the alias" + ); + + // Unknown value leaves the field unchanged. + cfg.shell.hide_window = true; + unsafe { + std::env::set_var("OPENHUMAN_SHELL_HIDE_WINDOW", "maybe"); + std::env::remove_var("SHELL_HIDE_WINDOW"); + } + cfg.apply_env_overrides(); + assert!(cfg.shell.hide_window); + unsafe { + std::env::remove_var("OPENHUMAN_SHELL_HIDE_WINDOW"); + } +} + #[test] fn apply_env_overrides_web_search_limits_only() { let _g = env_lock(); diff --git a/src/openhuman/config/schema/mod.rs b/src/openhuman/config/schema/mod.rs index 1b2ae4117..d42397792 100644 --- a/src/openhuman/config/schema/mod.rs +++ b/src/openhuman/config/schema/mod.rs @@ -74,7 +74,9 @@ pub use proxy::{ ProxyConfig, ProxyScope, }; pub use routes::{EmbeddingRouteConfig, ModelRouteConfig}; -pub use runtime::{DockerRuntimeConfig, ReliabilityConfig, RuntimeConfig, SchedulerConfig}; +pub use runtime::{ + DockerRuntimeConfig, ReliabilityConfig, RuntimeConfig, SchedulerConfig, ShellConfig, +}; pub use runtime_python::RuntimePythonConfig; pub use scheduler_gate::{SchedulerGateConfig, SchedulerGateMode}; pub use storage_memory::{ diff --git a/src/openhuman/config/schema/runtime.rs b/src/openhuman/config/schema/runtime.rs index 86e996dae..378717b6f 100644 --- a/src/openhuman/config/schema/runtime.rs +++ b/src/openhuman/config/schema/runtime.rs @@ -35,6 +35,19 @@ pub struct DockerRuntimeConfig { pub allowed_workspace_roots: Vec, } +/// `[shell]` — behaviour of the shell-family tools (`shell`, `node_exec`, +/// `npm_exec`, monitor) when they spawn child processes. +#[derive(Debug, Clone, Default, Serialize, Deserialize, JsonSchema)] +#[serde(default)] +pub struct ShellConfig { + /// On Windows, suppress the console window that briefly flashes for every + /// child process the shell tool spawns by passing `CREATE_NO_WINDOW` + /// (`0x08000000`) in the process creation flags. No-op on macOS/Linux. + /// Defaults to `false` for backward compatibility. + #[serde(default)] + pub hide_window: bool, +} + fn default_true() -> bool { defaults::default_true() } @@ -175,3 +188,28 @@ impl Default for SchedulerConfig { } } } + +#[cfg(test)] +mod shell_config_tests { + use super::ShellConfig; + + #[test] + fn shell_config_defaults_hide_window_off() { + // Backward compatibility: absent `[shell]` section must not change + // behaviour, so `hide_window` defaults to false. + assert!(!ShellConfig::default().hide_window); + } + + #[test] + fn shell_config_parses_hide_window_from_toml() { + let cfg: ShellConfig = toml::from_str("hide_window = true").unwrap(); + assert!(cfg.hide_window); + } + + #[test] + fn shell_config_empty_table_keeps_default() { + // An empty `[shell]` table relies on `#[serde(default)]` for the field. + let cfg: ShellConfig = toml::from_str("").unwrap(); + assert!(!cfg.hide_window); + } +} diff --git a/src/openhuman/config/schema/types.rs b/src/openhuman/config/schema/types.rs index 70cda75cc..c8ba71ecf 100644 --- a/src/openhuman/config/schema/types.rs +++ b/src/openhuman/config/schema/types.rs @@ -120,6 +120,9 @@ pub struct Config { #[serde(default)] pub runtime: RuntimeConfig, + #[serde(default)] + pub shell: ShellConfig, + #[serde(default)] pub screen_intelligence: ScreenIntelligenceConfig, @@ -707,6 +710,7 @@ impl Default for Config { autonomy: AutonomyConfig::default(), sandbox: SandboxConfig::default(), runtime: RuntimeConfig::default(), + shell: ShellConfig::default(), screen_intelligence: ScreenIntelligenceConfig::default(), autocomplete: AutocompleteConfig::default(), reliability: ReliabilityConfig::default(), diff --git a/src/openhuman/inference/local/mod.rs b/src/openhuman/inference/local/mod.rs index 0891f04d0..1ae22b92a 100644 --- a/src/openhuman/inference/local/mod.rs +++ b/src/openhuman/inference/local/mod.rs @@ -35,7 +35,10 @@ pub(crate) mod install_whisper; pub(crate) mod lm_studio; pub(crate) mod model_requirements; mod ollama; -mod process_util; +// `pub(crate)` so the shared `apply_no_window` helper can be reused from the +// agent shell runtime (`agent::host_runtime`) — single source of truth for the +// Windows `CREATE_NO_WINDOW` flag (#3727/#3728). +pub(crate) mod process_util; pub mod profile; pub(crate) mod provider; pub(crate) use model_requirements::{evaluate_context, ContextEligibility, MIN_CONTEXT_TOKENS}; diff --git a/tests/inference_agent_raw_coverage_e2e.rs b/tests/inference_agent_raw_coverage_e2e.rs index 19c39c9a9..d9ba0dd04 100644 --- a/tests/inference_agent_raw_coverage_e2e.rs +++ b/tests/inference_agent_raw_coverage_e2e.rs @@ -3899,36 +3899,45 @@ fn agent_dispatchers_and_host_runtime_cover_public_edge_paths() { assert!(provider_messages[2].content.contains("call-1")); assert_eq!(provider_messages[3].content, "done"); - let native_runtime = create_runtime(&RuntimeConfig { - kind: "native".into(), - ..Default::default() - }) + let native_runtime = create_runtime( + &RuntimeConfig { + kind: "native".into(), + ..Default::default() + }, + false, + ) .expect("native runtime"); assert_eq!(native_runtime.name(), "native"); assert!(native_runtime.has_shell_access()); - let docker_runtime = create_runtime(&RuntimeConfig { - kind: "docker".into(), - docker: DockerRuntimeConfig { - image: "alpine:coverage".into(), - network: "none".into(), - mount_workspace: false, - read_only_rootfs: false, - memory_limit_mb: Some(128), - cpu_limit: None, + let docker_runtime = create_runtime( + &RuntimeConfig { + kind: "docker".into(), + docker: DockerRuntimeConfig { + image: "alpine:coverage".into(), + network: "none".into(), + mount_workspace: false, + read_only_rootfs: false, + memory_limit_mb: Some(128), + cpu_limit: None, + ..Default::default() + }, ..Default::default() }, - ..Default::default() - }) + false, + ) .expect("docker runtime"); assert_eq!(docker_runtime.name(), "docker"); assert!(!docker_runtime.has_filesystem_access()); assert_eq!(docker_runtime.memory_budget(), 128); - let unsupported = match create_runtime(&RuntimeConfig { - kind: "wasm".into(), - ..Default::default() - }) { + let unsupported = match create_runtime( + &RuntimeConfig { + kind: "wasm".into(), + ..Default::default() + }, + false, + ) { Ok(runtime) => panic!( "unsupported runtime unexpectedly created: {}", runtime.name()