refactor(platform): encapsulate plateform-specific operations

src/main.rs

@@ -466,9 +466,8 @@ fn cmd_onboard() -> Result<(), Box<dyn std::error::Error>> {
     if user_exists {
         tui::print_step_done(1, TOTAL_STEPS, "System user already exists");
     } else {
-        let status = platform::create_system_user("clawshell")?;
-        if !status.success() {
-            tui::print_error("Failed to create 'clawshell' user.");
+        if let Err(error) = platform::create_system_user("clawshell") {
+            tui::print_error(&format!("Failed to create 'clawshell' user: {error}"));
             std::process::exit(1);
         }
         tui::print_step_done(1, TOTAL_STEPS, "System user created");
@@ -493,32 +492,35 @@ fn cmd_onboard() -> Result<(), Box<dyn std::error::Error>> {
     // Step 3: Set permissions and ownership
     tui::print_step(3, TOTAL_STEPS, "Setting permissions and ownership...");
 
-    let chown_spec = platform::clawshell_chown_spec();
-
-    if let Err(e) = std::process::Command::new("chmod")
-        .args(["0700", &config_dir.to_string_lossy()])
-        .status()
-    {
-        warn!(path = %config_dir.display(), error = %e, "Failed to chmod config directory");
+    if let Err(error) = platform::set_mode(&config_dir, 0o700) {
+        warn!(
+            error = %error,
+            path = %config_dir.display(),
+            "Failed to set config directory permissions"
+        );
     }
-    if let Err(e) = std::process::Command::new("chown")
-        .args(["-R", chown_spec, &config_dir.to_string_lossy()])
-        .status()
-    {
-        warn!(path = %config_dir.display(), error = %e, "Failed to chown config directory");
+    if let Err(error) = platform::set_owner(&config_dir, true) {
+        warn!(
+            error = %error,
+            path = %config_dir.display(),
+            "Failed to set config directory owner"
+        );
     }
-    if let Err(e) = std::process::Command::new("chown")
-        .args(["-R", chown_spec, &log_dir_path.to_string_lossy()])
-        .status()
-    {
-        warn!(path = %log_dir_path.display(), error = %e, "Failed to chown log directory");
+    if let Err(error) = platform::set_owner(&log_dir_path, true) {
+        warn!(
+            error = %error,
+            path = %log_dir_path.display(),
+            "Failed to set log directory owner"
+        );
     }
-    if let Some(pid_parent) = pid_path.parent()
-        && let Err(e) = std::process::Command::new("chown")
-            .args([chown_spec, &pid_parent.to_string_lossy()])
-            .status()
-    {
-        warn!(path = %pid_parent.display(), error = %e, "Failed to chown PID directory");
+    if let Some(pid_parent) = pid_path.parent() {
+        if let Err(error) = platform::set_owner(pid_parent, false) {
+            warn!(
+                error = %error,
+                path = %pid_parent.display(),
+                "Failed to set PID directory owner"
+            );
+        }
     }
     tui::print_step_done(3, TOTAL_STEPS, "Permissions set");
 
@@ -548,18 +550,34 @@ fn cmd_onboard() -> Result<(), Box<dyn std::error::Error>> {
     std::fs::write(&config_file, serde_json::to_string_pretty(&config_json)?)?;
 
     // Set permissions on config files
-    let _ = std::process::Command::new("chmod")
-        .args(["0600", &config_file.to_string_lossy()])
-        .status();
-    let _ = std::process::Command::new("chmod")
-        .args(["0600", &toml_config_path.to_string_lossy()])
-        .status();
-    let _ = std::process::Command::new("chown")
-        .args([chown_spec, &config_file.to_string_lossy()])
-        .status();
-    let _ = std::process::Command::new("chown")
-        .args([chown_spec, &toml_config_path.to_string_lossy()])
-        .status();
+    if let Err(error) = platform::set_mode(&config_file, 0o600) {
+        warn!(
+            error = %error,
+            path = %config_file.display(),
+            "Failed to set config.json permissions"
+        );
+    }
+    if let Err(error) = platform::set_mode(&toml_config_path, 0o600) {
+        warn!(
+            error = %error,
+            path = %toml_config_path.display(),
+            "Failed to set clawshell.toml permissions"
+        );
+    }
+    if let Err(error) = platform::set_owner(&config_file, false) {
+        warn!(
+            error = %error,
+            path = %config_file.display(),
+            "Failed to set config.json owner"
+        );
+    }
+    if let Err(error) = platform::set_owner(&toml_config_path, false) {
+        warn!(
+            error = %error,
+            path = %toml_config_path.display(),
+            "Failed to set clawshell.toml owner"
+        );
+    }
     tui::print_step_done(5, TOTAL_STEPS, "Configuration written");
 
     // Step 6: OpenClaw config path was already asked in step 4
@@ -938,11 +956,10 @@ fn cmd_uninstall(skip_confirm: bool) -> Result<(), Box<dyn std::error::Error>> {
         .unwrap_or(false);
 
     if user_exists {
-        let status = platform::delete_system_user("clawshell")?;
-        if status.success() {
-            tui::print_success("System user removed.");
+        if let Err(error) = platform::delete_system_user("clawshell") {
+            tui::print_warning(&format!("Failed to remove system user: {error}"));
         } else {
-            tui::print_warning(&format!("Failed to remove user (exit code: {status})."));
+            tui::print_success("System user removed.");
         }
     }
 

src/onboard.rs

@@ -4,6 +4,7 @@ use crate::tui;
 use serde_json::Value;
 use std::io::{self, BufRead, Write};
 use std::path::{Path, PathBuf};
+use tracing::warn;
 use vfs::VfsPath;
 
 /// API keys detected from an existing OpenClaw installation.
@@ -660,11 +661,14 @@ pub fn backup_openclaw_config(openclaw_path: &Path) -> Result<PathBuf, Box<dyn s
     use std::os::unix::fs::PermissionsExt;
     std::fs::set_permissions(&backup_path, std::fs::Permissions::from_mode(0o000))?;
 
-    // Chown the backup to the clawshell user
-    let chown_spec = platform::clawshell_chown_spec();
-    let _ = std::process::Command::new("chown")
-        .args([chown_spec, &backup_path.to_string_lossy()])
-        .status();
+    // Chown the backup to the clawshell user.
+    if let Err(error) = platform::set_owner(&backup_path, false) {
+        warn!(
+            error = %error,
+            path = %backup_path.display(),
+            "Failed to set backup owner"
+        );
+    }
 
     Ok(backup_path)
 }
@@ -896,6 +900,19 @@ pub fn install_autostart_service(
     let service_path = autostart_service_path();
     let root = crate::process::physical_root();
     let vfs_path = root.join(service_path.trim_start_matches('/'))?;
+
+    // Reinstall path: try to unload/disable first so replacing the unit is safe.
+    // Whether this should be best-effort is a caller policy, not a platform policy.
+    if vfs_path.exists()?
+        && let Err(error) = platform::remove_autostart_service(service_path)
+    {
+        warn!(
+            error = %error,
+            service_path,
+            "Failed to stop existing auto-start service before reinstall"
+        );
+    }
+
     install_autostart_service_vfs(&vfs_path, &content)?;
     platform::install_autostart_post_write(service_path)?;
 

src/platform/linux.rs

@@ -1,6 +1,6 @@
-use super::{Error, command_output, command_status, ensure_success};
+use super::{Error, command_status, format_octal_mode};
 use std::path::Path;
-use std::process::{Command, ExitStatus};
+use std::process::Command;
 
 pub fn clawshell_chown_spec() -> &'static str {
     "clawshell:clawshell"
@@ -22,7 +22,7 @@ pub fn autostart_service_content(exe_path: &Path, config_path: &Path) -> String
     crate::onboard::generate_systemd_unit(exe_path, config_path)
 }
 
-pub fn create_system_user(name: &str) -> Result<ExitStatus, Error> {
+pub fn create_system_user(name: &str) -> Result<(), Error> {
     let mut command = Command::new("useradd");
     command.args([
         "--system",
@@ -34,7 +34,7 @@ pub fn create_system_user(name: &str) -> Result<ExitStatus, Error> {
     command_status(&mut command, "useradd")
 }
 
-pub fn delete_system_user(name: &str) -> Result<ExitStatus, Error> {
+pub fn delete_system_user(name: &str) -> Result<(), Error> {
     let mut command = Command::new("userdel");
     command.arg(name);
     command_status(&mut command, "userdel")
@@ -43,36 +43,56 @@ pub fn delete_system_user(name: &str) -> Result<ExitStatus, Error> {
 pub fn install_autostart_post_write(_service_path: &str) -> Result<(), Error> {
     let mut daemon_reload = Command::new("systemctl");
     daemon_reload.args(["daemon-reload"]);
-    let output = command_output(&mut daemon_reload, "systemctl daemon-reload")?;
-    ensure_success("systemctl daemon-reload", output)?;
+    command_status(&mut daemon_reload, "systemctl daemon-reload")?;
 
     let mut enable = Command::new("systemctl");
     enable.args(["enable", "clawshell.service"]);
-    let output = command_output(&mut enable, "systemctl enable clawshell.service")?;
-    ensure_success("systemctl enable clawshell.service", output)?;
+    command_status(&mut enable, "systemctl enable clawshell.service")?;
 
     Ok(())
 }
 
 pub fn start_autostart_service(_service_path: &str) -> Result<(), Error> {
     let mut start = Command::new("systemctl");
     start.args(["start", "clawshell.service"]);
-    let output = command_output(&mut start, "systemctl start clawshell.service")?;
-    ensure_success("systemctl start clawshell.service", output)?;
+    command_status(&mut start, "systemctl start clawshell.service")?;
     Ok(())
 }
 
 pub fn remove_autostart_service(_service_path: &str) -> Result<(), Error> {
-    let _ = Command::new("systemctl")
-        .args(["disable", "clawshell.service"])
-        .status();
-    let _ = Command::new("systemctl")
-        .args(["stop", "clawshell.service"])
-        .status();
+    let mut disable = Command::new("systemctl");
+    disable.args(["disable", "clawshell.service"]);
+    command_status(&mut disable, "systemctl disable clawshell.service")?;
+
+    let mut stop = Command::new("systemctl");
+    stop.args(["stop", "clawshell.service"]);
+    command_status(&mut stop, "systemctl stop clawshell.service")?;
+
     Ok(())
 }
 
 pub fn remove_autostart_post_delete() -> Result<(), Error> {
-    let _ = Command::new("systemctl").args(["daemon-reload"]).status();
+    let mut daemon_reload = Command::new("systemctl");
+    daemon_reload.args(["daemon-reload"]);
+    command_status(&mut daemon_reload, "systemctl daemon-reload")?;
     Ok(())
 }
+
+pub fn set_owner(path: &Path, recursive: bool) -> Result<(), Error> {
+    let mut command = Command::new("chown");
+    if recursive {
+        command.arg("-R");
+    }
+    let path_arg = path.to_string_lossy().into_owned();
+    command.args([clawshell_chown_spec(), path_arg.as_str()]);
+    let op = if recursive { "chown -R" } else { "chown" };
+    command_status(&mut command, op)
+}
+
+pub fn set_mode(path: &Path, mode_bits: u32) -> Result<(), Error> {
+    let mode_str = format_octal_mode(mode_bits);
+    let path_arg = path.to_string_lossy().into_owned();
+    let mut command = Command::new("chmod");
+    command.args([mode_str.as_str(), path_arg.as_str()]);
+    command_status(&mut command, "chmod")
+}