Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions crates/cli/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4396,7 +4396,10 @@ impl App {
"agentd" => {
// Subcommand dispatch:
//
// /agentd restart → daemon.restart (exec self)
// /agentd restart [binary path]
// → daemon.restart; exec the daemon's own binary,
// or the given one (e.g. a freshly-built worktree
// binary). The path is validated daemon-side.
//
// Other subcommands are reserved for future use
// (e.g. `/agentd info` to print build version). The
Expand All @@ -4405,10 +4408,13 @@ impl App {
// observes that as a "daemon disconnected" status
// and the user must re-run `agent` to reconnect
// (auto-reconnect is follow-up work, see issue #90).
let sub = arg.trim();
let mut parts = arg.trim().splitn(2, char::is_whitespace);
let sub = parts.next().unwrap_or("");
let rest = parts.next().unwrap_or("").trim();
match sub {
"restart" => {
match self.client.daemon_restart().await {
let exe = (!rest.is_empty()).then(|| rest.to_string());
match self.client.daemon_restart(exe).await {
Ok(r) => self.set_status(format!(
"agentd: restart requested (exe={}, pid={}) — reconnect when ready",
r.exe, r.pid
Expand Down
14 changes: 11 additions & 3 deletions crates/client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,17 @@ impl Client {
/// the recv side with "broken pipe". Callers should treat any
/// reply (Ok or `BrokenPipe`-style error) as "restart in flight"
/// and re-attempt connect with backoff.
pub async fn daemon_restart(&self) -> Result<agentd_protocol::DaemonRestartResult> {
self.request(ipc_method::DAEMON_RESTART, &serde_json::Value::Null)
.await
/// `exe: Some(path)` execs that binary instead of the daemon's own
/// (e.g. a freshly-built one); `None` re-execs in place.
pub async fn daemon_restart(
&self,
exe: Option<String>,
) -> Result<agentd_protocol::DaemonRestartResult> {
self.request(
ipc_method::DAEMON_RESTART,
&agentd_protocol::DaemonRestartParams { exe },
)
.await
}
/// Dev-only: point the daemon's web server at a directory of assets
/// (or `None` to revert to embedded). No-op on release daemons.
Expand Down
17 changes: 11 additions & 6 deletions crates/daemon/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1219,12 +1219,17 @@ async fn dispatch(
}
m if m == ipc_method::DAEMON_RESTART => {
// Hand off to main's `tokio::select!` arm which calls
// `exec()` on the resolved current_exe. The reply
// races the kernel: we send it back here, then the
// IPC socket closes when exec() replaces the process
// image. Clients detect that as a disconnect and
// reconnect.
match manager.request_daemon_restart() {
// `exec()` on the resolved exe (the daemon's own binary, or
// the caller-supplied `exe` path). The reply races the
// kernel: we send it back here, then the IPC socket closes
// when exec() replaces the process image. Clients detect
// that as a disconnect and reconnect.
// Params are optional; older clients send `null` (no exe
// override), which fails to deserialize — treat that as None.
let exe = parse_params::<agentd_protocol::DaemonRestartParams>(req.params.clone())
.ok()
.and_then(|p| p.exe);
match manager.request_daemon_restart(exe.map(std::path::PathBuf::from)) {
Ok(cmd) => {
// Cloudflared survives the daemon's exec()
// because it's spawned in a separate process
Expand Down
58 changes: 56 additions & 2 deletions crates/daemon/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,29 @@ pub fn capture_startup_exe() {
}
}

/// Validate a caller-supplied restart binary: resolve it to an absolute
/// path (relative paths resolve against the *daemon's* cwd), confirm it
/// exists, is a regular file, and is executable. Returns the canonical
/// path to exec, or an error that's surfaced to the caller so a typo
/// never leaves the daemon trying to exec() a missing binary.
fn validate_restart_exe(path: &std::path::Path) -> Result<PathBuf> {
let abs = std::fs::canonicalize(path)
.with_context(|| format!("restart binary not found: {}", path.display()))?;
let meta = std::fs::metadata(&abs)
.with_context(|| format!("cannot stat restart binary: {}", abs.display()))?;
if !meta.is_file() {
anyhow::bail!("restart binary is not a file: {}", abs.display());
}
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
if meta.permissions().mode() & 0o111 == 0 {
anyhow::bail!("restart binary is not executable: {}", abs.display());
}
}
Ok(abs)
}

/// Best exe path for re-`exec()` on restart: the startup-captured
/// path if available, else `current_exe()` with any trailing
/// `" (deleted)"` marker stripped (defensive — the startup capture
Expand Down Expand Up @@ -588,8 +611,13 @@ impl SessionManager {
/// the exe path can't be resolved or the receiver was dropped
/// (which shouldn't happen — main holds it for the daemon's
/// lifetime).
pub fn request_daemon_restart(&self) -> Result<RestartCommand> {
let exe = restart_exe_path().context("resolve restart exe")?;
pub fn request_daemon_restart(&self, exe_override: Option<PathBuf>) -> Result<RestartCommand> {
let exe = match exe_override {
// Validate a caller-supplied binary BEFORE tearing the
// daemon down — exec()ing a bad path would never come back.
Some(p) => validate_restart_exe(&p)?,
None => restart_exe_path().context("resolve restart exe")?,
};
let args: Vec<String> = std::env::args().skip(1).collect();
let cmd = RestartCommand { exe, args };
self.restart_tx
Expand Down Expand Up @@ -2871,6 +2899,32 @@ mod tests {
use super::*;
use agentd_protocol::{Capabilities, PtySize};

#[test]
fn validate_restart_exe_accepts_executable_rejects_bad_paths() {
use std::os::unix::fs::PermissionsExt;
let dir = tempfile::tempdir().unwrap();

// An executable file → returns the canonical (absolute) path.
let good = dir.path().join("agentd");
std::fs::write(&good, b"#!/bin/sh\n").unwrap();
std::fs::set_permissions(&good, std::fs::Permissions::from_mode(0o755)).unwrap();
let resolved = validate_restart_exe(&good).expect("executable accepted");
assert!(resolved.is_absolute());
assert_eq!(resolved, std::fs::canonicalize(&good).unwrap());

// Missing path → error.
assert!(validate_restart_exe(&dir.path().join("nope")).is_err());

// A directory → error (not a regular file).
assert!(validate_restart_exe(dir.path()).is_err());

// A non-executable file → error.
let plain = dir.path().join("plain");
std::fs::write(&plain, b"data").unwrap();
std::fs::set_permissions(&plain, std::fs::Permissions::from_mode(0o644)).unwrap();
assert!(validate_restart_exe(&plain).is_err());
}

#[test]
fn startup_resume_retries_errored_sessions() {
assert!(should_resume_on_startup(SessionState::Pending));
Expand Down
6 changes: 3 additions & 3 deletions crates/e2e/tests/restart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ async fn restart_reloads_updated_binary() {
// Trigger the restart. The reply races the exec() — either we
// get it or the socket closes mid-flight; both mean "restart
// in progress".
let _ = d.client.daemon_restart().await;
let _ = d.client.daemon_restart(None).await;

// New daemon must come back up on the same socket.
let client = d
Expand Down Expand Up @@ -109,7 +109,7 @@ async fn tui_auto_reconnects_after_restart() {
.expect("modeline never rendered");

// Restart the daemon out from under the TUI.
let _ = d.client.daemon_restart().await;
let _ = d.client.daemon_restart(None).await;

// The TUI sets "reconnected to daemon" only from its
// successful-reconnect path (crates/cli/src/app.rs). Seeing
Expand Down Expand Up @@ -179,7 +179,7 @@ async fn web_client_reconnects_to_same_url_after_restart() {
// Restart the daemon. Token + port + password are persisted in
// runtime/remote.json and adopted by the new daemon, so the
// URL stays valid.
let _ = d.client.daemon_restart().await;
let _ = d.client.daemon_restart(None).await;

// The WS drops on the daemon exec(). Best-effort confirm we
// observed it leave "open" (it can recover fast, so don't fail
Expand Down
11 changes: 11 additions & 0 deletions crates/protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,17 @@ pub struct RemoteStopResult {
pub was_running: bool,
}

/// Params for `daemon.restart`. `exe: None` re-execs the daemon's own
/// binary (the upgrade-in-place case); `Some(path)` execs a different
/// binary instead — e.g. a freshly-built one from a worktree. The path
/// is validated (must exist + be executable) before the restart fires,
/// so a typo returns an error rather than bricking the daemon.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct DaemonRestartParams {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub exe: Option<String>,
}

/// Result of `daemon.restart`. Echoed back to the caller right
/// before the daemon exec()s itself — clients see this reply, then
/// observe the IPC socket close as the new process replaces the
Expand Down
Loading