From cee49e9c717d84a0a4b5a4d21066d3b6a0540292 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:03:41 -0600 Subject: [PATCH 1/7] feat(service): drain active work before restart --- CHANGELOG.md | 4 + README.md | 10 +- docs/USAGE.md | 17 +++ src/cli/render/help.rs | 15 ++- src/cli/service.rs | 5 +- src/cli/upgrade.rs | 9 +- src/service/manage.rs | 188 ++++++++++++++++++++++++++++++++- src/service/mod.rs | 20 +++- tests/daemon_runtime_tests.rs | 8 +- tests/service_command_tests.rs | 116 ++++++++++++++++++++ 10 files changed, 375 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f1ac956..264d41ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to OCM are documented here. ## Unreleased +### Changed + +- Make `ocm service restart ` request an indefinite OpenClaw active-work drain before restart when restart-handoff protocol v1 is available, require explicit `--force` for interrupting legacy or unhealthy gateways, and return immediately for self-restarts so the requesting turn can finish. + ## 0.2.30 - 2026-07-23 ### Added diff --git a/README.md b/README.md index 64a8c4cb..ad0fb23b 100644 --- a/README.md +++ b/README.md @@ -251,10 +251,16 @@ OCM negotiates fresh-process restart support only when it executes an `openclaw.mjs` entrypoint directly or through OCM's managed Node.js toolchain, so the gateway PID is the process OCM owns. `ocm service status ` reports `protocol v1` when OpenClaw can hand restart intent back to OCM atomically. +With that protocol, `ocm service restart ` asks OpenClaw to drain active +work before handing the process restart back to OCM. If the drain is still in +progress after OCM's observation window, the command returns a pending warning +and leaves the gateway running; it does not escalate a slow drain into a kill. + Package-manager, shell, host-Node, and other wrapper-backed bindings run in legacy compatibility mode without OCM's native service identity or detached -respawn; use `ocm service restart ` or bind a directly invoked OpenClaw -runtime for gateway-initiated fresh-process restarts. +respawn. Bind a directly invoked OpenClaw runtime for gateway-aware restarts, or +use `ocm service restart --force` as an explicit recovery action that +bypasses active-work draining. ## Why not just run OpenClaw directly? diff --git a/docs/USAGE.md b/docs/USAGE.md index 16cec904..b5bc4b13 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -303,6 +303,23 @@ ocm service stop mira ocm service restart mira ``` +Normal restart is gateway-aware when `ocm service status mira` reports restart +handoff `protocol v1`: OpenClaw drains active work and then hands the fresh +process restart back to OCM. A restart can therefore remain pending after the +command returns. OCM leaves the existing gateway running while it drains +instead of force-stopping it. + +Use the forced path only when the gateway is unhealthy or its binding cannot +negotiate the restart handoff: + +```bash +ocm service restart mira --force +``` + +Forced restart bypasses OpenClaw's active-work drain and can interrupt an +in-flight turn. OpenClaw restart recovery may resume supported sessions, but a +forced restart is intentionally the recovery path rather than the default. + ### Remove the service ```bash diff --git a/src/cli/render/help.rs b/src/cli/render/help.rs index 2424c74f..ef877fde 100644 --- a/src/cli/render/help.rs +++ b/src/cli/render/help.rs @@ -2125,9 +2125,15 @@ pub fn service_command_help(cmd: &str, action: &str) -> Option { ), "restart" => render_leaf( "Restart an env under the background service", - "Restart one env under the shared OCM background service.", - vec![format!("{cmd} service restart [--raw] [--json]")], + "Ask OpenClaw to drain active work before restarting one env under the shared OCM background service.", + vec![format!( + "{cmd} service restart [--force] [--raw] [--json]" + )], &[ + ( + "--force", + "Bypass OpenClaw active-work draining and restart the supervised child directly", + ), ( "--raw", "Force plain line output instead of the TTY receipt view", @@ -2135,7 +2141,10 @@ pub fn service_command_help(cmd: &str, action: &str) -> Option { ("--json", "Print the action summary as JSON"), ], vec![format!("{cmd} service restart mira")], - &[], + &[ + "The default gateway-aware restart can remain pending while active work drains.", + "Use --force only for recovery when the gateway cannot accept a graceful restart request.", + ], ), "uninstall" => render_leaf( "Disable an env in the background service", diff --git a/src/cli/service.rs b/src/cli/service.rs index 9f18d977..cbcf8048 100644 --- a/src/cli/service.rs +++ b/src/cli/service.rs @@ -105,12 +105,15 @@ impl Cli { pub(super) fn handle_service_restart(&self, args: Vec) -> Result { let (args, json_flag, profile) = self.consume_human_output_flags(args, "service restart")?; + let (args, force) = Self::consume_flag(args, "--force"); let Some(name) = args.first() else { return Err("service restart requires ".to_string()); }; Self::assert_no_extra_args(&args[1..])?; - let summary = self.service_service().restart(name)?; + let summary = self + .service_service() + .restart_with_options(name, crate::service::ServiceRestartOptions { force })?; if json_flag { self.print_json(&summary)?; return Ok(0); diff --git a/src/cli/upgrade.rs b/src/cli/upgrade.rs index 690305b5..a9ebc33e 100644 --- a/src/cli/upgrade.rs +++ b/src/cli/upgrade.rs @@ -2677,9 +2677,12 @@ impl Cli { } if service.running { - let restart = self - .with_progress(format!("Restarting service for {env_name}"), || { - self.service_service().restart_locked(env_name) + let restart = + self.with_progress(format!("Restarting service for {env_name}"), || { + self.service_service().restart_locked_with_options( + env_name, + crate::service::ServiceRestartOptions { force: true }, + ) })?; let note = join_optional_warnings( join_warnings(&restart.warnings), diff --git a/src/service/manage.rs b/src/service/manage.rs index b7b07db0..1bcdbef6 100644 --- a/src/service/manage.rs +++ b/src/service/manage.rs @@ -1,13 +1,19 @@ use std::collections::BTreeMap; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; use std::thread::sleep; use std::time::{Duration, Instant}; +#[cfg(unix)] +use std::os::unix::process::CommandExt; + use serde::Serialize; use super::inspect::ServiceSummary; use super::service_backend_support_error; -use crate::env::EnvironmentService; +use crate::env::{EnvironmentService, ResolvedExecution}; +use crate::infra::shell::{build_openclaw_dev_source_env, build_openclaw_env}; +use crate::managed_node::apply_path_prepend_to_environment; use crate::store::{restore_environment_service_policy, set_environment_service_policy}; use crate::supervisor::{SupervisorService, sync_supervisor_if_present}; @@ -31,6 +37,11 @@ pub struct ServiceActionSummary { pub type ServiceInstallSummary = ServiceActionSummary; +#[derive(Clone, Copy, Debug, Default)] +pub struct ServiceRestartOptions { + pub force: bool, +} + #[derive(Clone, Copy)] enum ServiceSupervisorPolicy { LeaveAsIs, @@ -128,6 +139,7 @@ pub fn stop_service( pub fn restart_service( name: &str, + options: ServiceRestartOptions, env: &BTreeMap, cwd: &Path, ) -> Result { @@ -143,6 +155,48 @@ pub fn restart_service( return update_service(name, ServiceUpdate::Restart, env, cwd); } + if options.force { + return force_restart_running_service(name, before, env, cwd); + } + + if before.restart_handoff.as_deref() != Some("protocol-v1") { + return Err(format!( + "env \"{name}\" has not negotiated external restart handoff protocol v1; upgrade its OpenClaw runtime or use \"ocm service restart {name} --force\" to bypass active-work draining" + )); + } + + spawn_gateway_aware_restart(name, env, cwd)?; + + if env.get("OCM_ACTIVE_ENV").map(String::as_str) == Some(name) { + let mut warnings = vec![ + "gateway-aware restart was scheduled without waiting because the request originated inside the target gateway; it will restart after active work drains" + .to_string(), + ]; + let summary = super::inspect::service_status_fast(name, env, cwd)?; + if !summary.running { + warnings + .push("gateway is already transitioning to its replacement process".to_string()); + } + return Ok(service_action_summary("restart", summary, warnings)); + } + + let status = wait_for_restart_action_summary(name, before.child_pid, env, cwd)?; + let mut warnings = status.warnings; + if !status.observed_restart { + warnings.push( + "gateway-aware restart was accepted and remains pending while active work drains; OCM did not force-stop the gateway" + .to_string(), + ); + } + Ok(service_action_summary("restart", status.summary, warnings)) +} + +fn force_restart_running_service( + name: &str, + before: ServiceSummary, + env: &BTreeMap, + cwd: &Path, +) -> Result { let supervisor = SupervisorService::new(env, cwd); let mut request_id = supervisor.request_child_restart(name)?; let restart_result = wait_for_restart_action_summary(name, before.child_pid, env, cwd); @@ -179,6 +233,9 @@ pub fn restart_service( "restart completed, but failed to clear restart request: {clear_error}" )); } + status + .warnings + .push("forced restart bypassed OpenClaw active-work draining".to_string()); Ok(service_action_summary( "restart", status.summary, @@ -189,6 +246,133 @@ pub fn restart_service( } } +fn spawn_gateway_aware_restart( + name: &str, + env: &BTreeMap, + cwd: &Path, +) -> Result<(), String> { + let args = vec![ + "gateway".to_string(), + "restart".to_string(), + "--wait".to_string(), + "0".to_string(), + "--json".to_string(), + ]; + let resolved = EnvironmentService::new(env, cwd).resolve(name, None, None, &args)?; + let command = resolved_restart_command(resolved, env)?; + let mut process = Command::new(&command.program); + process + .args(&command.args) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .env_clear() + .envs(&command.env) + .current_dir(&command.cwd); + #[cfg(unix)] + process.process_group(0); + let mut child = process + .spawn() + .map_err(|error| format!("failed to run \"{}\": {error}", command.program))?; + + let deadline = Instant::now() + Duration::from_millis(500); + while Instant::now() < deadline { + match child.try_wait() { + Ok(Some(status)) if status.success() => return Ok(()), + Ok(Some(status)) => { + return Err(format!( + "OpenClaw rejected the gateway-aware restart for env \"{name}\" (exit code {}); no forced restart was attempted. Inspect the gateway logs or use \"ocm service restart {name} --force\" to bypass active-work draining", + status.code().unwrap_or(1) + )); + } + Ok(None) => sleep(Duration::from_millis(25)), + Err(error) => { + return Err(format!( + "failed to inspect the gateway-aware restart helper for env \"{name}\": {error}" + )); + } + } + } + Ok(()) +} + +struct ResolvedRestartCommand { + program: String, + args: Vec, + env: BTreeMap, + cwd: PathBuf, +} + +fn resolved_restart_command( + resolved: ResolvedExecution, + process_env: &BTreeMap, +) -> Result { + match resolved { + ResolvedExecution::Launcher { + env, + command, + run_dir, + .. + } => { + let openclaw_env = build_openclaw_env(&env, process_env); + if cfg!(windows) { + Ok(ResolvedRestartCommand { + program: "cmd".to_string(), + args: vec!["/C".to_string(), command], + env: openclaw_env, + cwd: run_dir, + }) + } else { + Ok(ResolvedRestartCommand { + program: "sh".to_string(), + args: vec!["-lc".to_string(), command], + env: openclaw_env, + cwd: run_dir, + }) + } + } + ResolvedExecution::Runtime { + env, + program, + program_args, + path_prepend, + run_dir, + .. + } => { + let mut openclaw_env = build_openclaw_env(&env, process_env); + apply_path_prepend_to_environment(&mut openclaw_env, path_prepend.as_deref())?; + Ok(ResolvedRestartCommand { + program, + args: program_args, + env: openclaw_env, + cwd: run_dir, + }) + } + ResolvedExecution::Dev { + env, + program, + program_args, + run_dir, + .. + } + | ResolvedExecution::SourceWatch { + env, + program, + program_args, + run_dir, + .. + } => { + let openclaw_env = build_openclaw_dev_source_env(&env, process_env, &run_dir); + Ok(ResolvedRestartCommand { + program, + args: program_args, + env: openclaw_env, + cwd: run_dir, + }) + } + } +} + pub fn uninstall_service( name: &str, env: &BTreeMap, diff --git a/src/service/mod.rs b/src/service/mod.rs index fa0a8aab..4f1846e9 100644 --- a/src/service/mod.rs +++ b/src/service/mod.rs @@ -6,7 +6,7 @@ use std::collections::BTreeMap; use std::path::Path; pub use inspect::{ServiceSummary, ServiceSummaryList}; -pub use manage::{ServiceActionSummary, ServiceInstallSummary}; +pub use manage::{ServiceActionSummary, ServiceInstallSummary, ServiceRestartOptions}; pub(crate) use platform::{ ServiceManagerKind, service_backend_support_error, service_manager_kind, }; @@ -53,12 +53,24 @@ impl<'a> ServiceService<'a> { } pub fn restart(&self, name: &str) -> Result { + self.restart_with_options(name, ServiceRestartOptions::default()) + } + + pub fn restart_with_options( + &self, + name: &str, + options: ServiceRestartOptions, + ) -> Result { let _lock = crate::env::EnvironmentService::new(self.env, self.cwd).lock_operation(name)?; - self.restart_locked(name) + self.restart_locked_with_options(name, options) } - pub(crate) fn restart_locked(&self, name: &str) -> Result { - manage::restart_service(name, self.env, self.cwd) + pub(crate) fn restart_locked_with_options( + &self, + name: &str, + options: ServiceRestartOptions, + ) -> Result { + manage::restart_service(name, options, self.env, self.cwd) } pub fn uninstall(&self, name: &str) -> Result { diff --git a/tests/daemon_runtime_tests.rs b/tests/daemon_runtime_tests.rs index 5356672a..ed95f02a 100644 --- a/tests/daemon_runtime_tests.rs +++ b/tests/daemon_runtime_tests.rs @@ -842,7 +842,7 @@ fn service_restart_restarts_only_the_target_child() { let restart = run_ocm( &cwd, &restart_env, - &["service", "restart", "rescue", "--json"], + &["service", "restart", "rescue", "--force", "--json"], ); assert!(restart.status.success(), "{}", stderr(&restart)); let restarted = wait_for_runtime_child_pid_change( @@ -909,7 +909,11 @@ fn service_restart_requeues_a_stopped_desired_child() { .expect("daemon runtime state did not report the quick clean exit as stopped"); assert!(!started.exists()); - let restart = run_ocm(&cwd, &env, &["service", "restart", "demo", "--json"]); + let restart = run_ocm( + &cwd, + &env, + &["service", "restart", "demo", "--force", "--json"], + ); assert!(restart.status.success(), "{}", stderr(&restart)); let runtime = wait_for_runtime_children(&runtime_path, 1, Some("demo"), Duration::from_secs(10)) diff --git a/tests/service_command_tests.rs b/tests/service_command_tests.rs index 7e673be9..d75c3d87 100644 --- a/tests/service_command_tests.rs +++ b/tests/service_command_tests.rs @@ -59,6 +59,87 @@ fn setup_launcher_env(cwd: &Path, env: &BTreeMap) { assert!(created.status.success(), "{}", stderr(&created)); } +fn setup_gateway_aware_restart_fixture( + root: &TestDir, + restart_handoff: &str, +) -> ( + std::path::PathBuf, + BTreeMap, + std::path::PathBuf, +) { + let cwd = root.child("workspace"); + fs::create_dir_all(&cwd).unwrap(); + let mut env = launchd_env(root); + let invocation_log = root.child("gateway-restart-invocations.log"); + let launcher = root.child("bin/openclaw"); + write_executable_script( + &launcher, + &format!( + "#!/bin/sh\nprintf '%s\\n' \"$*\" >> '{}'\nprintf '{{\"ok\":true}}\\n'\n", + path_string(&invocation_log) + ), + ); + + let added = run_ocm( + &cwd, + &env, + &[ + "launcher", + "add", + "stable", + "--command", + &path_string(&launcher), + ], + ); + assert!(added.status.success(), "{}", stderr(&added)); + let created = run_ocm( + &cwd, + &env, + &["env", "create", "demo", "--launcher", "stable"], + ); + assert!(created.status.success(), "{}", stderr(&created)); + let started = run_ocm(&cwd, &env, &["service", "start", "demo"]); + assert!(started.status.success(), "{}", stderr(&started)); + + let runtime_path = supervisor_runtime_path(&env, &cwd).unwrap(); + fs::create_dir_all(runtime_path.parent().unwrap()).unwrap(); + let runtime = SupervisorRuntimeState { + kind: "ocm-supervisor-runtime".to_string(), + ocm_home: env.get("OCM_HOME").unwrap().clone(), + updated_at: now_utc(), + services: vec![SupervisorRuntimeService { + env_name: "demo".to_string(), + binding_kind: "launcher".to_string(), + binding_name: "stable".to_string(), + gateway_state: "running".to_string(), + restart_handoff: Some(restart_handoff.to_string()), + restart_count: 0, + child_port: 18789, + pid: Some(4242), + stdout_path: path_string(&root.child("demo.stdout.log")), + stderr_path: path_string(&root.child("demo.stderr.log")), + last_exit_code: None, + last_error: None, + last_event_at: None, + next_retry_at: None, + }], + children: vec![SupervisorRuntimeChild { + env_name: "demo".to_string(), + binding_kind: "launcher".to_string(), + binding_name: "stable".to_string(), + pid: 4242, + restart_count: 0, + child_port: 18789, + stdout_path: path_string(&root.child("demo.stdout.log")), + stderr_path: path_string(&root.child("demo.stderr.log")), + }], + }; + fs::write(runtime_path, serde_json::to_vec(&runtime).unwrap()).unwrap(); + env.insert("OCM_ACTIVE_ENV".to_string(), "demo".to_string()); + + (cwd, env, invocation_log) +} + fn json_output(output: &std::process::Output) -> Value { serde_json::from_slice(&output.stdout).unwrap() } @@ -475,6 +556,41 @@ fn service_restart_restores_running_policy() { assert_eq!(body["desiredRunning"], true); } +#[test] +fn service_restart_requests_gateway_aware_drain_without_self_deadlock() { + let root = TestDir::new("service-restart-gateway-aware"); + let (cwd, env, invocation_log) = setup_gateway_aware_restart_fixture(&root, "protocol-v1"); + + let output = run_ocm(&cwd, &env, &["service", "restart", "demo", "--json"]); + assert!(output.status.success(), "{}", stderr(&output)); + let body = json_output(&output); + assert_eq!(body["action"], "restart"); + assert_eq!(body["running"], true); + assert!(body["warnings"].as_array().unwrap().iter().any(|warning| { + warning + .as_str() + .unwrap() + .contains("scheduled without waiting") + })); + assert_eq!( + fs::read_to_string(invocation_log).unwrap(), + "gateway restart --wait 0 --json\n" + ); +} + +#[test] +fn service_restart_requires_handoff_or_explicit_force() { + let root = TestDir::new("service-restart-requires-handoff"); + let (cwd, env, invocation_log) = setup_gateway_aware_restart_fixture(&root, "legacy"); + + let output = run_ocm(&cwd, &env, &["service", "restart", "demo"]); + assert!(!output.status.success()); + let error = stderr(&output); + assert!(error.contains("has not negotiated external restart handoff protocol v1")); + assert!(error.contains("ocm service restart demo --force")); + assert!(!invocation_log.exists()); +} + #[test] fn service_uninstall_disables_the_env_service() { let root = TestDir::new("service-uninstall"); From 540c0bfbc54f96662de76f8b254eeeb7331854f0 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Mon, 27 Jul 2026 09:36:34 -0600 Subject: [PATCH 2/7] fix(service): resume work after immediate restart --- CHANGELOG.md | 2 +- README.md | 13 +++++++------ docs/USAGE.md | 18 +++++++++++------- src/cli/render/help.rs | 8 ++++---- src/service/manage.rs | 23 +++++++++++------------ tests/service_command_tests.rs | 4 ++-- 6 files changed, 36 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 264d41ad..ec214293 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to OCM are documented here. ### Changed -- Make `ocm service restart ` request an indefinite OpenClaw active-work drain before restart when restart-handoff protocol v1 is available, require explicit `--force` for interrupting legacy or unhealthy gateways, and return immediately for self-restarts so the requesting turn can finish. +- Make `ocm service restart ` restart immediately through OpenClaw's protocol-v1 recovery handoff so eligible interrupted sessions and subagents resume after startup, require explicit `--force` for a direct supervisor restart that bypasses recovery, and avoid self-restart deadlocks. ## 0.2.30 - 2026-07-23 diff --git a/README.md b/README.md index ad0fb23b..971ac0d2 100644 --- a/README.md +++ b/README.md @@ -251,16 +251,17 @@ OCM negotiates fresh-process restart support only when it executes an `openclaw.mjs` entrypoint directly or through OCM's managed Node.js toolchain, so the gateway PID is the process OCM owns. `ocm service status ` reports `protocol v1` when OpenClaw can hand restart intent back to OCM atomically. -With that protocol, `ocm service restart ` asks OpenClaw to drain active -work before handing the process restart back to OCM. If the drain is still in -progress after OCM's observation window, the command returns a pending warning -and leaves the gateway running; it does not escalate a slow drain into a kill. +With that protocol, `ocm service restart ` asks OpenClaw to restart +immediately through its recovery handoff. OpenClaw records eligible active +sessions and subagents before exiting, OCM starts the replacement gateway, and +OpenClaw resumes that recoverable work after startup. This does not wait for an +in-flight turn to finish before replacing the gateway process. Package-manager, shell, host-Node, and other wrapper-backed bindings run in legacy compatibility mode without OCM's native service identity or detached respawn. Bind a directly invoked OpenClaw runtime for gateway-aware restarts, or -use `ocm service restart --force` as an explicit recovery action that -bypasses active-work draining. +use `ocm service restart --force` as an explicit emergency action that +bypasses OpenClaw's recovery handoff and replaces the supervised child directly. ## Why not just run OpenClaw directly? diff --git a/docs/USAGE.md b/docs/USAGE.md index b5bc4b13..6131eb75 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -304,10 +304,10 @@ ocm service restart mira ``` Normal restart is gateway-aware when `ocm service status mira` reports restart -handoff `protocol v1`: OpenClaw drains active work and then hands the fresh -process restart back to OCM. A restart can therefore remain pending after the -command returns. OCM leaves the existing gateway running while it drains -instead of force-stopping it. +handoff `protocol v1`: OpenClaw records eligible active sessions and subagents, +hands the fresh-process restart back to OCM immediately, and resumes recoverable +work after the replacement gateway starts. The old gateway process does not wait +for an in-flight turn to finish. Use the forced path only when the gateway is unhealthy or its binding cannot negotiate the restart handoff: @@ -316,9 +316,13 @@ negotiate the restart handoff: ocm service restart mira --force ``` -Forced restart bypasses OpenClaw's active-work drain and can interrupt an -in-flight turn. OpenClaw restart recovery may resume supported sessions, but a -forced restart is intentionally the recovery path rather than the default. +Forced restart replaces the supervised child directly. It bypasses OpenClaw's +restart-recovery handoff and can lose in-flight work, so it is intentionally the +emergency path rather than the default. + +Recovery is limited to work OpenClaw knows how to persist and resume. It does +not make arbitrary child processes or non-idempotent external side effects +transactional. ### Remove the service diff --git a/src/cli/render/help.rs b/src/cli/render/help.rs index ef877fde..4b5ff404 100644 --- a/src/cli/render/help.rs +++ b/src/cli/render/help.rs @@ -2125,14 +2125,14 @@ pub fn service_command_help(cmd: &str, action: &str) -> Option { ), "restart" => render_leaf( "Restart an env under the background service", - "Ask OpenClaw to drain active work before restarting one env under the shared OCM background service.", + "Restart one env immediately through OpenClaw's recovery handoff so eligible interrupted work can resume after startup.", vec![format!( "{cmd} service restart [--force] [--raw] [--json]" )], &[ ( "--force", - "Bypass OpenClaw active-work draining and restart the supervised child directly", + "Bypass OpenClaw restart recovery and replace the supervised child directly", ), ( "--raw", @@ -2142,8 +2142,8 @@ pub fn service_command_help(cmd: &str, action: &str) -> Option { ], vec![format!("{cmd} service restart mira")], &[ - "The default gateway-aware restart can remain pending while active work drains.", - "Use --force only for recovery when the gateway cannot accept a graceful restart request.", + "The default restart does not wait for active work; OpenClaw records eligible interrupted work for recovery before exiting.", + "Use --force only for recovery when the gateway cannot accept the OpenClaw restart handoff.", ], ), "uninstall" => render_leaf( diff --git a/src/service/manage.rs b/src/service/manage.rs index 1bcdbef6..4fe655a4 100644 --- a/src/service/manage.rs +++ b/src/service/manage.rs @@ -161,15 +161,15 @@ pub fn restart_service( if before.restart_handoff.as_deref() != Some("protocol-v1") { return Err(format!( - "env \"{name}\" has not negotiated external restart handoff protocol v1; upgrade its OpenClaw runtime or use \"ocm service restart {name} --force\" to bypass active-work draining" + "env \"{name}\" has not negotiated external restart handoff protocol v1; upgrade its OpenClaw runtime or use \"ocm service restart {name} --force\" to restart the supervised child without OpenClaw recovery handoff" )); } - spawn_gateway_aware_restart(name, env, cwd)?; + spawn_recovery_aware_restart(name, env, cwd)?; if env.get("OCM_ACTIVE_ENV").map(String::as_str) == Some(name) { let mut warnings = vec![ - "gateway-aware restart was scheduled without waiting because the request originated inside the target gateway; it will restart after active work drains" + "recovery-aware restart was scheduled without waiting because the request originated inside the target gateway; OpenClaw will preserve and resume eligible interrupted work after restart" .to_string(), ]; let summary = super::inspect::service_status_fast(name, env, cwd)?; @@ -184,7 +184,7 @@ pub fn restart_service( let mut warnings = status.warnings; if !status.observed_restart { warnings.push( - "gateway-aware restart was accepted and remains pending while active work drains; OCM did not force-stop the gateway" + "recovery-aware restart was accepted, but OCM did not observe a replacement gateway within 30 seconds; no direct supervisor restart was attempted" .to_string(), ); } @@ -233,9 +233,9 @@ fn force_restart_running_service( "restart completed, but failed to clear restart request: {clear_error}" )); } - status - .warnings - .push("forced restart bypassed OpenClaw active-work draining".to_string()); + status.warnings.push( + "forced supervisor restart bypassed OpenClaw restart recovery handoff".to_string(), + ); Ok(service_action_summary( "restart", status.summary, @@ -246,7 +246,7 @@ fn force_restart_running_service( } } -fn spawn_gateway_aware_restart( +fn spawn_recovery_aware_restart( name: &str, env: &BTreeMap, cwd: &Path, @@ -254,8 +254,7 @@ fn spawn_gateway_aware_restart( let args = vec![ "gateway".to_string(), "restart".to_string(), - "--wait".to_string(), - "0".to_string(), + "--force".to_string(), "--json".to_string(), ]; let resolved = EnvironmentService::new(env, cwd).resolve(name, None, None, &args)?; @@ -281,14 +280,14 @@ fn spawn_gateway_aware_restart( Ok(Some(status)) if status.success() => return Ok(()), Ok(Some(status)) => { return Err(format!( - "OpenClaw rejected the gateway-aware restart for env \"{name}\" (exit code {}); no forced restart was attempted. Inspect the gateway logs or use \"ocm service restart {name} --force\" to bypass active-work draining", + "OpenClaw rejected the recovery-aware restart for env \"{name}\" (exit code {}); no direct supervisor restart was attempted. Inspect the gateway logs or use \"ocm service restart {name} --force\" to bypass OpenClaw recovery handoff", status.code().unwrap_or(1) )); } Ok(None) => sleep(Duration::from_millis(25)), Err(error) => { return Err(format!( - "failed to inspect the gateway-aware restart helper for env \"{name}\": {error}" + "failed to inspect the recovery-aware restart helper for env \"{name}\": {error}" )); } } diff --git a/tests/service_command_tests.rs b/tests/service_command_tests.rs index d75c3d87..9151ee24 100644 --- a/tests/service_command_tests.rs +++ b/tests/service_command_tests.rs @@ -557,7 +557,7 @@ fn service_restart_restores_running_policy() { } #[test] -fn service_restart_requests_gateway_aware_drain_without_self_deadlock() { +fn service_restart_requests_immediate_recovery_handoff_without_self_deadlock() { let root = TestDir::new("service-restart-gateway-aware"); let (cwd, env, invocation_log) = setup_gateway_aware_restart_fixture(&root, "protocol-v1"); @@ -574,7 +574,7 @@ fn service_restart_requests_gateway_aware_drain_without_self_deadlock() { })); assert_eq!( fs::read_to_string(invocation_log).unwrap(), - "gateway restart --wait 0 --json\n" + "gateway restart --force --json\n" ); } From 9aee647849286344e7164ce26cd2870d4b4b81b9 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:31:26 -0600 Subject: [PATCH 3/7] fix(service): preserve legacy restart compatibility --- CHANGELOG.md | 2 +- README.md | 8 +++++--- docs/USAGE.md | 14 +++++++++----- src/cli/render/help.rs | 3 ++- src/service/manage.rs | 25 +++++++++++++++++-------- tests/daemon_runtime_tests.rs | 15 +++++++++++++-- tests/service_command_tests.rs | 13 ------------- 7 files changed, 47 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec214293..b87499d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to OCM are documented here. ### Changed -- Make `ocm service restart ` restart immediately through OpenClaw's protocol-v1 recovery handoff so eligible interrupted sessions and subagents resume after startup, require explicit `--force` for a direct supervisor restart that bypasses recovery, and avoid self-restart deadlocks. +- Make `ocm service restart ` restart immediately through OpenClaw's protocol-v1 recovery handoff so eligible interrupted sessions and subagents resume after startup, preserve the legacy direct-restart behavior with a warning when recovery is unavailable, keep `--force` as an explicit bypass for an unhealthy handoff, and avoid self-restart deadlocks. ## 0.2.30 - 2026-07-23 diff --git a/README.md b/README.md index 971ac0d2..59ab9a33 100644 --- a/README.md +++ b/README.md @@ -259,9 +259,11 @@ in-flight turn to finish before replacing the gateway process. Package-manager, shell, host-Node, and other wrapper-backed bindings run in legacy compatibility mode without OCM's native service identity or detached -respawn. Bind a directly invoked OpenClaw runtime for gateway-aware restarts, or -use `ocm service restart --force` as an explicit emergency action that -bypasses OpenClaw's recovery handoff and replaces the supervised child directly. +respawn. `ocm service restart ` preserves their existing direct-supervisor +restart behavior and warns that in-flight work cannot be recovered. Bind a +directly invoked OpenClaw runtime to gain recovery-aware restarts. Use +`ocm service restart --force` only to explicitly bypass a recovery +handoff that is advertised but unhealthy. ## Why not just run OpenClaw directly? diff --git a/docs/USAGE.md b/docs/USAGE.md index 6131eb75..d7a1f91c 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -309,16 +309,20 @@ hands the fresh-process restart back to OCM immediately, and resumes recoverable work after the replacement gateway starts. The old gateway process does not wait for an in-flight turn to finish. -Use the forced path only when the gateway is unhealthy or its binding cannot -negotiate the restart handoff: +When a binding cannot negotiate the restart handoff, OCM preserves the existing +direct-supervisor restart behavior and prints a warning that in-flight work may +have been interrupted. Existing restart commands therefore remain compatible. + +Use the forced path only when a gateway advertises recovery support but is too +unhealthy to accept the restart handoff: ```bash ocm service restart mira --force ``` -Forced restart replaces the supervised child directly. It bypasses OpenClaw's -restart-recovery handoff and can lose in-flight work, so it is intentionally the -emergency path rather than the default. +Forced restart explicitly replaces the supervised child directly. It bypasses +OpenClaw's restart-recovery handoff and can lose in-flight work, so it is +intentionally an emergency override rather than a compatibility requirement. Recovery is limited to work OpenClaw knows how to persist and resume. It does not make arbitrary child processes or non-idempotent external side effects diff --git a/src/cli/render/help.rs b/src/cli/render/help.rs index 4b5ff404..cb64b356 100644 --- a/src/cli/render/help.rs +++ b/src/cli/render/help.rs @@ -2143,7 +2143,8 @@ pub fn service_command_help(cmd: &str, action: &str) -> Option { vec![format!("{cmd} service restart mira")], &[ "The default restart does not wait for active work; OpenClaw records eligible interrupted work for recovery before exiting.", - "Use --force only for recovery when the gateway cannot accept the OpenClaw restart handoff.", + "Gateways without recovery handoff support keep the legacy direct-restart behavior and emit a warning.", + "Use --force only to bypass a recovery handoff that is advertised but unhealthy.", ], ), "uninstall" => render_leaf( diff --git a/src/service/manage.rs b/src/service/manage.rs index 4fe655a4..8c2d1ad3 100644 --- a/src/service/manage.rs +++ b/src/service/manage.rs @@ -156,13 +156,23 @@ pub fn restart_service( } if options.force { - return force_restart_running_service(name, before, env, cwd); + return restart_running_service_directly( + name, + before, + env, + cwd, + "forced supervisor restart bypassed OpenClaw restart recovery handoff", + ); } if before.restart_handoff.as_deref() != Some("protocol-v1") { - return Err(format!( - "env \"{name}\" has not negotiated external restart handoff protocol v1; upgrade its OpenClaw runtime or use \"ocm service restart {name} --force\" to restart the supervised child without OpenClaw recovery handoff" - )); + return restart_running_service_directly( + name, + before, + env, + cwd, + "OpenClaw restart recovery handoff protocol v1 is unavailable; used the legacy direct supervisor restart path, so in-flight work may have been interrupted", + ); } spawn_recovery_aware_restart(name, env, cwd)?; @@ -191,11 +201,12 @@ pub fn restart_service( Ok(service_action_summary("restart", status.summary, warnings)) } -fn force_restart_running_service( +fn restart_running_service_directly( name: &str, before: ServiceSummary, env: &BTreeMap, cwd: &Path, + restart_warning: &str, ) -> Result { let supervisor = SupervisorService::new(env, cwd); let mut request_id = supervisor.request_child_restart(name)?; @@ -233,9 +244,7 @@ fn force_restart_running_service( "restart completed, but failed to clear restart request: {clear_error}" )); } - status.warnings.push( - "forced supervisor restart bypassed OpenClaw restart recovery handoff".to_string(), - ); + status.warnings.push(restart_warning.to_string()); Ok(service_action_summary( "restart", status.summary, diff --git a/tests/daemon_runtime_tests.rs b/tests/daemon_runtime_tests.rs index ed95f02a..198e7089 100644 --- a/tests/daemon_runtime_tests.rs +++ b/tests/daemon_runtime_tests.rs @@ -751,7 +751,7 @@ fn daemon_run_reloads_children_after_binding_changes() { } #[test] -fn service_restart_restarts_only_the_target_child() { +fn service_restart_preserves_legacy_fallback_and_restarts_only_the_target_child() { let _guard = daemon_runtime_test_lock(); let root = TestDir::new("daemon-targeted-service-restart"); let cwd = root.child("workspace"); @@ -842,9 +842,20 @@ fn service_restart_restarts_only_the_target_child() { let restart = run_ocm( &cwd, &restart_env, - &["service", "restart", "rescue", "--force", "--json"], + &["service", "restart", "rescue", "--json"], ); assert!(restart.status.success(), "{}", stderr(&restart)); + let restart_body: serde_json::Value = serde_json::from_slice(&restart.stdout).unwrap(); + assert!( + restart_body["warnings"] + .as_array() + .unwrap() + .iter() + .any(|warning| warning + .as_str() + .unwrap() + .contains("used the legacy direct supervisor restart path")) + ); let restarted = wait_for_runtime_child_pid_change( &runtime_path, "rescue", diff --git a/tests/service_command_tests.rs b/tests/service_command_tests.rs index 9151ee24..adb27e22 100644 --- a/tests/service_command_tests.rs +++ b/tests/service_command_tests.rs @@ -578,19 +578,6 @@ fn service_restart_requests_immediate_recovery_handoff_without_self_deadlock() { ); } -#[test] -fn service_restart_requires_handoff_or_explicit_force() { - let root = TestDir::new("service-restart-requires-handoff"); - let (cwd, env, invocation_log) = setup_gateway_aware_restart_fixture(&root, "legacy"); - - let output = run_ocm(&cwd, &env, &["service", "restart", "demo"]); - assert!(!output.status.success()); - let error = stderr(&output); - assert!(error.contains("has not negotiated external restart handoff protocol v1")); - assert!(error.contains("ocm service restart demo --force")); - assert!(!invocation_log.exists()); -} - #[test] fn service_uninstall_disables_the_env_service() { let root = TestDir::new("service-uninstall"); From 51215d3f62281f5975fd0e78d0dc54f691730b4c Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:55:01 -0600 Subject: [PATCH 4/7] fix(supervisor): prevent ambient fleet reloads --- CHANGELOG.md | 1 + src/supervisor/mod.rs | 165 ++++++++++++++++++++++++++++++++-- tests/daemon_runtime_tests.rs | 98 ++++++++++++++++++++ 3 files changed, 259 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b87499d5..01cbf4c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to OCM are documented here. ### Changed - Make `ocm service restart ` restart immediately through OpenClaw's protocol-v1 recovery handoff so eligible interrupted sessions and subagents resume after startup, preserve the legacy direct-restart behavior with a warning when recovery is unavailable, keep `--force` as an explicit bypass for an unhealthy handoff, and avoid self-restart deadlocks. +- Keep persisted supervisor child specifications stable when only the invoking shell's ambient environment changes, preventing unrelated gateways from reloading during scoped OCM operations, and report the exact field names behind genuine reloads. ## 0.2.30 - 2026-07-23 diff --git a/src/supervisor/mod.rs b/src/supervisor/mod.rs index 1259db77..68b5992e 100644 --- a/src/supervisor/mod.rs +++ b/src/supervisor/mod.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::fs::{self, OpenOptions}; #[cfg(unix)] use std::os::unix::fs::MetadataExt; @@ -267,7 +267,7 @@ impl<'a> SupervisorService<'a> { let state_path = supervisor_state_path(self.env, self.cwd)?; let _lock = lock_supervisor_state(&state_path)?; let mut state = self.build_state()?; - preserve_persisted_restart_requests(&state_path, &mut state); + preserve_persisted_supervisor_state(&state_path, &mut state); if let Some(parent) = state_path.parent() { ensure_dir(parent)?; } @@ -980,14 +980,104 @@ fn child_map(children: &[SupervisorChildSpec]) -> BTreeMap(state_path) else { return; }; + preserve_ambient_only_child_specs(state, &persisted_state.children); preserve_restart_requests(state, persisted_state.restart_requests); } +fn preserve_ambient_only_child_specs( + state: &mut SupervisorState, + persisted_children: &[SupervisorChildSpec], +) { + // The persisted state can be consumed by an older daemon that compares the + // full spec. Keep its exact bytes when a newer CLI only inherited a + // different shell PATH/tool environment, or that daemon will reload every + // otherwise unchanged gateway. + let persisted = persisted_children + .iter() + .map(|child| (child.env_name.as_str(), child)) + .collect::>(); + for child in &mut state.children { + let Some(persisted_child) = persisted.get(child.env_name.as_str()) else { + continue; + }; + if child_specs_differ_only_in_ambient_env(persisted_child, child) { + *child = (*persisted_child).clone(); + } + } +} + +fn child_specs_differ_only_in_ambient_env( + left: &SupervisorChildSpec, + right: &SupervisorChildSpec, +) -> bool { + if left == right { + return false; + } + + let mut left = left.clone(); + let mut right = right.clone(); + left.process_env + .retain(|key, _| !ambient_child_env_key(key)); + right + .process_env + .retain(|key, _| !ambient_child_env_key(key)); + left == right +} + +fn ambient_child_env_key(key: &str) -> bool { + matches!(key, "HOME" | "PATH" | "TMPDIR" | "OCM_SELF" | "SHELL") + || SUPERVISED_CHILD_RUNTIME_ENV_KEYS.contains(&key) + || SERVICE_PROXY_ENV_KEYS.contains(&key) + || SERVICE_EXTRA_ENV_KEYS.contains(&key) + || key.starts_with("NPM_CONFIG_") + || key.starts_with("npm_config_") + || key.starts_with("COREPACK_") +} + +fn supervisor_child_spec_changed_fields( + previous: &SupervisorChildSpec, + next: &SupervisorChildSpec, +) -> Vec { + let mut fields = Vec::new(); + macro_rules! changed { + ($field:ident, $name:literal) => { + if previous.$field != next.$field { + fields.push($name.to_string()); + } + }; + } + changed!(binding_kind, "bindingKind"); + changed!(binding_name, "bindingName"); + changed!(restart_handoff_pid_bound, "restartHandoffPidBound"); + changed!(command, "command"); + changed!(binary_path, "binaryPath"); + changed!(runtime_source_kind, "runtimeSourceKind"); + changed!(runtime_release_version, "runtimeReleaseVersion"); + changed!(runtime_release_channel, "runtimeReleaseChannel"); + changed!(args, "args"); + changed!(run_dir, "runDir"); + changed!(child_port, "childPort"); + changed!(stdout_path, "stdoutPath"); + changed!(stderr_path, "stderrPath"); + + let env_keys = previous + .process_env + .keys() + .chain(next.process_env.keys()) + .collect::>(); + for key in env_keys { + if previous.process_env.get(key) != next.process_env.get(key) { + fields.push(format!("processEnv.{key}")); + } + } + fields +} + fn preserve_persisted_child_specs_except( state: &mut SupervisorState, persisted_children: Vec, @@ -1118,10 +1208,12 @@ fn reconcile_running_children( let mut existing = running .remove(&env_name) .expect("running child should exist when needs_restart is true"); + let changed_fields = supervisor_child_spec_changed_fields(&existing.spec, next_spec); eprintln!( - "ocm service: reloading {} ({})", + "ocm service: reloading {} ({}) because {} changed", existing.spec.env_name, - child_binding_label(next_spec) + child_binding_label(next_spec), + changed_fields.join(", ") ); stop_supervisor_child(&mut existing); pending.insert( @@ -2922,4 +3014,67 @@ mod tests { assert!(!process_env.contains_key("GH_TOKEN")); assert!(!process_env.contains_key("PWD")); } + + #[test] + fn ambient_caller_env_drift_keeps_the_persisted_child_spec() { + let mut persisted = child_spec("rescue", 18790); + persisted.process_env.extend([ + ("PATH".to_string(), "/usr/bin:/bin".to_string()), + ("PNPM_HOME".to_string(), "/old/pnpm".to_string()), + ( + "OPENCLAW_STATE_DIR".to_string(), + "/tmp/rescue/.openclaw".to_string(), + ), + ("OCM_ACTIVE_ENV".to_string(), "rescue".to_string()), + ]); + let mut rebuilt = persisted.clone(); + rebuilt + .process_env + .insert("PATH".to_string(), "/opt/homebrew/bin:/usr/bin".to_string()); + rebuilt + .process_env + .insert("PNPM_HOME".to_string(), "/new/pnpm".to_string()); + + let mut state = SupervisorState { + kind: SUPERVISOR_STATE_KIND.to_string(), + ocm_home: "/tmp/ocm".to_string(), + generated_at: OffsetDateTime::UNIX_EPOCH, + children: vec![rebuilt], + skipped_envs: Vec::new(), + restart_requests: Vec::new(), + }; + preserve_ambient_only_child_specs(&mut state, &[persisted.clone()]); + + assert_eq!(state.children, vec![persisted]); + } + + #[test] + fn openclaw_env_changes_are_not_treated_as_ambient_drift() { + let mut previous = child_spec("rescue", 18790); + previous.process_env.insert( + "OPENCLAW_STATE_DIR".to_string(), + "/tmp/old-state".to_string(), + ); + let mut next = previous.clone(); + next.process_env.insert( + "OPENCLAW_STATE_DIR".to_string(), + "/tmp/new-state".to_string(), + ); + + assert!(!child_specs_differ_only_in_ambient_env(&previous, &next)); + } + + #[test] + fn child_spec_diagnostics_name_fields_without_logging_values() { + let previous = child_spec("rescue", 18790); + let mut next = previous.clone(); + next.binding_name = "next-runtime".to_string(); + next.process_env + .insert("PATH".to_string(), "/private/path".to_string()); + + assert_eq!( + supervisor_child_spec_changed_fields(&previous, &next), + vec!["bindingName".to_string(), "processEnv.PATH".to_string()] + ); + } } diff --git a/tests/daemon_runtime_tests.rs b/tests/daemon_runtime_tests.rs index 198e7089..c4318812 100644 --- a/tests/daemon_runtime_tests.rs +++ b/tests/daemon_runtime_tests.rs @@ -548,6 +548,44 @@ fn env_changes_refresh_persisted_service_state_without_extra_commands() { ); } +#[test] +fn supervisor_sync_does_not_reload_specs_for_ambient_caller_env_drift() { + let _guard = daemon_runtime_test_lock(); + let root = TestDir::new("service-state-ambient-env-drift"); + let (cwd, mut initial_env) = setup_service_fixture(&root); + initial_env.insert("PATH".to_string(), "/usr/bin:/bin".to_string()); + initial_env.insert("PNPM_HOME".to_string(), "/old/pnpm".to_string()); + let state_path = root.child("ocm-home/supervisor/state.json"); + + SupervisorService::new(&initial_env, &cwd).sync().unwrap(); + let initial_state = read_persisted_service_state(&state_path); + let initial_demo = initial_state["children"] + .as_array() + .unwrap() + .iter() + .find(|child| child["envName"] == "demo") + .unwrap() + .clone(); + + let mut later_env = initial_env; + later_env.insert( + "PATH".to_string(), + "/opt/homebrew/bin:/usr/bin:/bin".to_string(), + ); + later_env.insert("PNPM_HOME".to_string(), "/new/pnpm".to_string()); + later_env.insert("CODEX_SESSION_ID".to_string(), "session-2".to_string()); + SupervisorService::new(&later_env, &cwd).sync().unwrap(); + + let later_state = read_persisted_service_state(&state_path); + let later_demo = later_state["children"] + .as_array() + .unwrap() + .iter() + .find(|child| child["envName"] == "demo") + .unwrap(); + assert_eq!(later_demo, &initial_demo); +} + #[test] fn child_restart_request_rebuilds_missing_or_stale_state() { let _guard = daemon_runtime_test_lock(); @@ -750,6 +788,66 @@ fn daemon_run_reloads_children_after_binding_changes() { stop_process(&mut daemon); } +#[test] +fn daemon_run_keeps_child_running_after_ambient_caller_env_drift() { + let _guard = daemon_runtime_test_lock(); + let root = TestDir::new("daemon-ambient-env-drift"); + let cwd = root.child("workspace"); + fs::create_dir_all(&cwd).unwrap(); + let mut initial_env = ocm_env(&root); + initial_env.insert("PATH".to_string(), "/usr/bin:/bin".to_string()); + initial_env.insert("PNPM_HOME".to_string(), "/old/pnpm".to_string()); + let runtime_path = root.child("ocm-home/supervisor/runtime.json"); + let started = root.child("started.txt"); + let script = root.child("bin/openclaw"); + write_legacy_openclaw_script( + &script, + &format!( + "#!/bin/sh\nprintf '%s\\n' \"$$\" >> '{}'\ntrap 'exit 0' TERM INT\nwhile :; do sleep 1; done\n", + path_string(&started), + ), + ); + + let launcher = run_ocm( + &cwd, + &initial_env, + &["launcher", "add", "dev", "--command", &path_string(&script)], + ); + assert!(launcher.status.success(), "{}", stderr(&launcher)); + let created = run_ocm( + &cwd, + &initial_env, + &["env", "create", "demo", "--launcher", "dev"], + ); + assert!(created.status.success(), "{}", stderr(&created)); + set_service_enabled(&cwd, &initial_env, "demo", true); + SupervisorService::new(&initial_env, &cwd).sync().unwrap(); + + let mut daemon = spawn_daemon_process(&cwd, &initial_env); + assert!(wait_for_file(&started, Duration::from_secs(5))); + let initial_runtime = + wait_for_runtime_children(&runtime_path, 1, Some("demo"), Duration::from_secs(5)) + .expect("daemon runtime state did not report the running child"); + let initial_pid = runtime_child_pid(&initial_runtime, "demo").unwrap(); + + let mut later_env = initial_env.clone(); + later_env.insert( + "PATH".to_string(), + "/opt/homebrew/bin:/usr/bin:/bin".to_string(), + ); + later_env.insert("PNPM_HOME".to_string(), "/new/pnpm".to_string()); + SupervisorService::new(&later_env, &cwd).sync().unwrap(); + sleep(Duration::from_millis(500)); + + let later_runtime = + wait_for_runtime_children(&runtime_path, 1, Some("demo"), Duration::from_secs(2)) + .expect("daemon runtime state lost the running child"); + assert_eq!(runtime_child_pid(&later_runtime, "demo"), Some(initial_pid)); + assert_eq!(fs::read_to_string(&started).unwrap().lines().count(), 1); + + stop_process(&mut daemon); +} + #[test] fn service_restart_preserves_legacy_fallback_and_restarts_only_the_target_child() { let _guard = daemon_runtime_test_lock(); From 2bb3b998e7b10d6236697692fa72502eb4b1eb48 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Mon, 27 Jul 2026 15:21:06 -0600 Subject: [PATCH 5/7] fix(supervisor): normalize equivalent runtime launchers --- src/supervisor/mod.rs | 67 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/supervisor/mod.rs b/src/supervisor/mod.rs index 68b5992e..d896ef8a 100644 --- a/src/supervisor/mod.rs +++ b/src/supervisor/mod.rs @@ -1021,6 +1021,8 @@ fn child_specs_differ_only_in_ambient_env( let mut left = left.clone(); let mut right = right.clone(); + normalize_equivalent_runtime_launch(&mut left); + normalize_equivalent_runtime_launch(&mut right); left.process_env .retain(|key, _| !ambient_child_env_key(key)); right @@ -1029,6 +1031,42 @@ fn child_specs_differ_only_in_ambient_env( left == right } +fn normalize_equivalent_runtime_launch(spec: &mut SupervisorChildSpec) { + if spec.binding_kind != "runtime" || spec.runtime_source_kind.as_deref() != Some("installed") { + return; + } + + let Some(binary_path) = spec.binary_path.as_deref() else { + return; + }; + if Path::new(binary_path) + .file_name() + .and_then(|name| name.to_str()) + == Some("openclaw.mjs") + { + return; + } + if !matches!( + Path::new(binary_path) + .file_name() + .and_then(|name| name.to_str()), + Some("node" | "node.exe") + ) { + return; + } + + let Some(entrypoint) = spec.args.first().filter(|entrypoint| { + Path::new(entrypoint) + .file_name() + .and_then(|name| name.to_str()) + == Some("openclaw.mjs") + }) else { + return; + }; + spec.binary_path = Some(entrypoint.clone()); + spec.args.remove(0); +} + fn ambient_child_env_key(key: &str) -> bool { matches!(key, "HOME" | "PATH" | "TMPDIR" | "OCM_SELF" | "SHELL") || SUPERVISED_CHILD_RUNTIME_ENV_KEYS.contains(&key) @@ -3064,6 +3102,35 @@ mod tests { assert!(!child_specs_differ_only_in_ambient_env(&previous, &next)); } + #[test] + fn direct_and_managed_node_runtime_launches_are_equivalent_ambient_drift() { + let entrypoint = "/tmp/runtime/node_modules/openclaw/openclaw.mjs"; + let mut direct = child_spec("rescue", 18790); + direct.binding_kind = "runtime".to_string(); + direct.runtime_source_kind = Some("installed".to_string()); + direct.binary_path = Some(entrypoint.to_string()); + direct.args = vec![ + "gateway".to_string(), + "run".to_string(), + "--port".to_string(), + "18790".to_string(), + ]; + direct.process_env.insert( + "PATH".to_string(), + "/opt/homebrew/bin:/usr/bin:/bin".to_string(), + ); + + let mut managed = direct.clone(); + managed.binary_path = Some("/tmp/toolchain/bin/node".to_string()); + managed.args.insert(0, entrypoint.to_string()); + managed.process_env.insert( + "PATH".to_string(), + "/tmp/toolchain/bin:/usr/bin:/bin".to_string(), + ); + + assert!(child_specs_differ_only_in_ambient_env(&direct, &managed)); + } + #[test] fn child_spec_diagnostics_name_fields_without_logging_values() { let previous = child_spec("rescue", 18790); From 95291c7463c5cd228a6ee7c4ef5e93e86fce97f1 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:49:23 -0600 Subject: [PATCH 6/7] fix: clean supervised process groups after exit --- src/supervisor/mod.rs | 88 +++++++++++++++++++++++++++++++++-- tests/daemon_runtime_tests.rs | 56 ++++++++++++++++++++++ 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/src/supervisor/mod.rs b/src/supervisor/mod.rs index d896ef8a..ef650380 100644 --- a/src/supervisor/mod.rs +++ b/src/supervisor/mod.rs @@ -1437,9 +1437,10 @@ fn process_exited_children( let mut runtime_dirty = false; for exited_child in exited { - let Some(previous_child) = running.remove(&exited_child.env_name) else { + let Some(mut previous_child) = running.remove(&exited_child.env_name) else { continue; }; + stop_supervisor_child(&mut previous_child); runtime_dirty = true; results.push(child_run_result( &previous_child.spec, @@ -1766,22 +1767,43 @@ fn stop_supervisor_child(running_child: &mut RunningSupervisorChild) { let process_group = format!("-{}", running_child.child.id()); let _ = Command::new("kill") .args(["-TERM", "--", &process_group]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) .status(); for _ in 0..20 { - match running_child.child.try_wait() { - Ok(Some(_)) => return, - Ok(None) => sleep(Duration::from_millis(50)), - Err(_) => break, + let _ = running_child.child.try_wait(); + if !supervisor_process_group_exists(&process_group) { + return; } + sleep(Duration::from_millis(50)); } let _ = Command::new("kill") .args(["-KILL", "--", &process_group]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) .status(); + for _ in 0..20 { + let _ = running_child.child.try_wait(); + if !supervisor_process_group_exists(&process_group) { + return; + } + sleep(Duration::from_millis(50)); + } } let _ = running_child.child.kill(); let _ = running_child.child.wait(); } +#[cfg(unix)] +fn supervisor_process_group_exists(process_group: &str) -> bool { + Command::new("kill") + .args(["-0", "--", process_group]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .is_ok_and(|status| status.success()) +} + fn supervisor_state_equivalent(left: &SupervisorState, right: &SupervisorState) -> bool { left.kind == right.kind && left.ocm_home == right.ocm_home @@ -2764,6 +2786,62 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn exited_supervisor_child_cleans_remaining_process_group() { + let test_dir = std::env::temp_dir().join(format!( + "ocm-supervisor-process-group-{}-{}", + std::process::id(), + now_millis() + )); + fs::create_dir_all(&test_dir).unwrap(); + let descendant_pid_path = test_dir.join("descendant.pid"); + let mut spec = child_spec("process-group-cleanup", 19_999); + spec.command = Some( + "trap '' HUP; sleep 60 & descendant=$!; printf '%s' \"$descendant\" > \"$OCM_TEST_DESCENDANT_PID_FILE\"; exit 1" + .to_string(), + ); + spec.binary_path = None; + spec.run_dir = test_dir.to_string_lossy().into_owned(); + spec.stdout_path = test_dir.join("stdout.log").to_string_lossy().into_owned(); + spec.stderr_path = test_dir.join("stderr.log").to_string_lossy().into_owned(); + spec.process_env.insert( + "OCM_TEST_DESCENDANT_PID_FILE".to_string(), + descendant_pid_path.to_string_lossy().into_owned(), + ); + + let mut running_child = spawn_running_child(spec, 0, 0).unwrap(); + let process_group = format!("-{}", running_child.child.id()); + let status = running_child.child.wait().unwrap(); + assert_eq!(status.code(), Some(1)); + + for _ in 0..100 { + if descendant_pid_path.exists() { + break; + } + sleep(Duration::from_millis(10)); + } + let descendant_pid = fs::read_to_string(&descendant_pid_path).unwrap(); + assert!(supervisor_process_group_exists(&process_group)); + + stop_supervisor_child(&mut running_child); + + for _ in 0..100 { + let descendant_alive = Command::new("kill") + .args(["-0", "--", descendant_pid.trim()]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .is_ok_and(|status| status.success()); + if !descendant_alive { + break; + } + sleep(Duration::from_millis(10)); + } + assert!(!supervisor_process_group_exists(&process_group)); + let _ = fs::remove_dir_all(test_dir); + } + #[test] fn restart_requests_are_part_of_supervisor_state_equivalence() { let active = supervisor_state(Vec::new()); diff --git a/tests/daemon_runtime_tests.rs b/tests/daemon_runtime_tests.rs index c4318812..190380a7 100644 --- a/tests/daemon_runtime_tests.rs +++ b/tests/daemon_runtime_tests.rs @@ -1238,6 +1238,62 @@ fn daemon_stops_the_full_dev_process_tree_after_service_stop() { stop_process(&mut daemon); } +#[cfg(unix)] +#[test] +fn daemon_cleans_descendants_after_a_child_exits_before_restart() { + let _guard = daemon_runtime_test_lock(); + let root = TestDir::new("daemon-exit-process-group-cleanup"); + let cwd = root.child("workspace"); + fs::create_dir_all(&cwd).unwrap(); + let env = ocm_env(&root); + let service = SupervisorService::new(&env, &cwd); + + let starts = root.child("starts.txt"); + let descendant_pid_file = root.child("descendant.pid"); + let script = root.child("bin/openclaw.mjs"); + write_legacy_openclaw_script( + &script, + &format!( + "#!/bin/sh\ncount=0\nif [ -f '{starts}' ]; then count=$(cat '{starts}'); fi\ncount=$((count + 1))\nprintf '%s\n' \"$count\" > '{starts}'\nif [ \"$count\" -eq 1 ]; then\n trap '' HUP\n sleep 60 &\n printf '%s\n' \"$!\" > '{descendant_pid_file}'\n sleep 1\n exit 1\nfi\nsleep 10\n", + starts = path_string(&starts), + descendant_pid_file = path_string(&descendant_pid_file), + ), + ); + + let launcher = run_ocm( + &cwd, + &env, + &["launcher", "add", "dev", "--command", &path_string(&script)], + ); + assert!(launcher.status.success(), "{}", stderr(&launcher)); + + let created = run_ocm(&cwd, &env, &["env", "create", "demo", "--launcher", "dev"]); + assert!(created.status.success(), "{}", stderr(&created)); + set_service_enabled(&cwd, &env, "demo", true); + service.sync().unwrap(); + + let mut daemon = spawn_daemon_process(&cwd, &env); + assert!(wait_for_file(&descendant_pid_file, Duration::from_secs(5))); + let descendant_pid = fs::read_to_string(&descendant_pid_file) + .unwrap() + .trim() + .parse::() + .unwrap(); + assert!(process_exists(descendant_pid)); + + assert!(wait_for_file_value(&starts, "2", Duration::from_secs(5))); + let deadline = Instant::now() + Duration::from_secs(5); + while Instant::now() < deadline && process_exists(descendant_pid) { + sleep(Duration::from_millis(50)); + } + assert!( + !process_exists(descendant_pid), + "background descendant still alive after supervised child exit" + ); + + stop_process(&mut daemon); +} + #[test] fn daemon_restarts_quick_clean_exit_with_openclaw_handoff() { let _guard = daemon_runtime_test_lock(); From 62a665288d70112171a7e55ba0b97a82a0e2cac5 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:27:29 -0600 Subject: [PATCH 7/7] fix(supervisor): always reap stopped children --- src/supervisor/mod.rs | 59 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/src/supervisor/mod.rs b/src/supervisor/mod.rs index ef650380..2fe01278 100644 --- a/src/supervisor/mod.rs +++ b/src/supervisor/mod.rs @@ -1773,23 +1773,28 @@ fn stop_supervisor_child(running_child: &mut RunningSupervisorChild) { for _ in 0..20 { let _ = running_child.child.try_wait(); if !supervisor_process_group_exists(&process_group) { - return; + break; } sleep(Duration::from_millis(50)); } - let _ = Command::new("kill") - .args(["-KILL", "--", &process_group]) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status(); - for _ in 0..20 { - let _ = running_child.child.try_wait(); - if !supervisor_process_group_exists(&process_group) { - return; + if supervisor_process_group_exists(&process_group) { + let _ = Command::new("kill") + .args(["-KILL", "--", &process_group]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + for _ in 0..20 { + let _ = running_child.child.try_wait(); + if !supervisor_process_group_exists(&process_group) { + break; + } + sleep(Duration::from_millis(50)); } - sleep(Duration::from_millis(50)); } } + // Always wait on the process-group leader. It can exit after try_wait() + // reports None but before the group-existence probe; returning in that + // window leaves a zombie that can block OpenClaw's single-instance lock. let _ = running_child.child.kill(); let _ = running_child.child.wait(); } @@ -2842,6 +2847,38 @@ mod tests { let _ = fs::remove_dir_all(test_dir); } + #[cfg(unix)] + #[test] + fn stopped_supervisor_child_reaps_process_group_leader() { + let test_dir = std::env::temp_dir().join(format!( + "ocm-supervisor-child-reap-{}-{}", + std::process::id(), + now_millis() + )); + fs::create_dir_all(&test_dir).unwrap(); + let mut spec = child_spec("process-group-leader-reap", 19_998); + spec.command = Some("trap 'exit 0' TERM; while :; do sleep 1; done".to_string()); + spec.binary_path = None; + spec.run_dir = test_dir.to_string_lossy().into_owned(); + spec.stdout_path = test_dir.join("stdout.log").to_string_lossy().into_owned(); + spec.stderr_path = test_dir.join("stderr.log").to_string_lossy().into_owned(); + + let mut running_child = spawn_running_child(spec, 0, 0).unwrap(); + let child_pid = running_child.child.id().to_string(); + sleep(Duration::from_millis(50)); + + stop_supervisor_child(&mut running_child); + + let child_still_exists = Command::new("kill") + .args(["-0", "--", &child_pid]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .is_ok_and(|status| status.success()); + assert!(!child_still_exists, "supervised child was not reaped"); + let _ = fs::remove_dir_all(test_dir); + } + #[test] fn restart_requests_are_part_of_supervisor_state_equivalence() { let active = supervisor_state(Vec::new());