feat: implement cleanup on daemon exit and enhance file path handling

AGENTS.md

@@ -62,9 +62,10 @@ directly. `skill/chrome-devtools/CUSTOM_SCRIPTING.md` documents `run-script` and
 
 ### Daemon Architecture
 
-A background daemon (`/tmp/chrome-devtools-daemon.sock`) keeps a persistent CDP
-WebSocket connection. First CLI invocation spawns it; subsequent commands reuse
-it. 5-minute idle timeout.
+A background daemon (`$TMPDIR/chrome-devtools-daemon-<uid>.sock` — uid-suffixed
+to isolate users sharing /tmp) keeps a persistent CDP WebSocket connection.
+First CLI invocation spawns it; subsequent commands reuse it. 5-minute idle
+timeout; socket/PID files are cleaned up on signals and panics too.
 
 `CdpClient::connect` (`cdp.rs`) bounds the WebSocket handshake with a timeout
 (`CHROME_CONNECT_TIMEOUT_SECS`, default 10s). Without it, a pending Chrome

README.md

@@ -41,7 +41,7 @@ This is a lightweight Rust binary that talks directly to Chrome's DevTools Proto
 ```
 chrome-devtools navigate https://example.com
         │
-        ├─ Try daemon (Unix socket /tmp/chrome-devtools-daemon.sock)
+        ├─ Try daemon (Unix socket $TMPDIR/chrome-devtools-daemon-<uid>.sock)
         │   └─ If running → send command → get result
         │
         ├─ If no daemon → spawn one (background process)
@@ -215,9 +215,10 @@ Global `--block-url` and `--unblock-url` update the **active tab's** block list
 
 ## Daemon details
 
-- **Socket**: `/tmp/chrome-devtools-daemon.sock`
-- **PID file**: `/tmp/chrome-devtools-daemon.pid`
+- **Socket**: `$TMPDIR/chrome-devtools-daemon-<uid>.sock` (uid-suffixed so users on a shared machine don't collide)
+- **PID file**: `$TMPDIR/chrome-devtools-daemon-<uid>.pid`
 - **Idle timeout**: 5 minutes (auto-exits, cleans up socket)
+- **Cleanup**: socket + PID files are also removed on SIGTERM/SIGINT and panics, not just normal exit
 - **Protocol**: Length-prefixed JSON over Unix socket
 - **Spawned by**: First CLI invocation (transparent to user)
 - **Kill**: `chrome-devtools kill-daemon` (or delete the socket + PID file)

src/daemon.rs

@@ -29,30 +29,154 @@ enum ConnectionOutcome {
     Fatal,
 }
 
+/// Acquire the cross-process lock serializing the daemon-file critical
+/// sections: startup's pid-write/rebind and cleanup's check-then-remove.
+/// Blocks until the lock is free; the OS releases it when the handle drops.
+/// `None` means the lock file couldn't be created/locked — callers proceed
+/// unlocked (degrading to pre-lock behavior) rather than refusing to run.
+fn lock_daemon_files() -> Option<std::fs::File> {
+    let f = std::fs::OpenOptions::new()
+        .create(true)
+        .write(true)
+        .truncate(false)
+        .open(lock_path())
+        .ok()?;
+    f.lock().ok()?;
+    Some(f)
+}
+
+/// Non-blocking variant for `cleanup()`: if the lock is held, another daemon
+/// is inside its own critical section — exactly when deleting the shared
+/// files is guaranteed wrong — so contention means "don't touch anything".
+fn try_lock_daemon_files() -> Option<std::fs::File> {
+    let f = std::fs::OpenOptions::new()
+        .create(true)
+        .write(true)
+        .truncate(false)
+        .open(lock_path())
+        .ok()?;
+    f.try_lock().ok()?;
+    Some(f)
+}
+
+/// Best-effort removal of the daemon's socket/address and PID files.
+///
+/// Only cleans up when the PID file still names this process: the paths are
+/// shared, so a stale-but-alive old daemon exiting must not delete the files
+/// of a newer daemon that has since rebound them. The ownership check and the
+/// removals happen under the daemon-file lock so a replacement can't write
+/// its pid and rebind in between (it would keep running but be unreachable,
+/// and every later CLI call would spawn yet another daemon).
+fn cleanup() {
+    let Some(_lock) = try_lock_daemon_files() else {
+        // Contended: a replacement is mid-startup. Its rebind supersedes our
+        // files, and any leftovers self-heal on the next daemon start.
+        return;
+    };
+    let owns_files = std::fs::read_to_string(pid_path())
+        .ok()
+        .and_then(|s| s.trim().parse::<u32>().ok())
+        == Some(std::process::id());
+    if !owns_files {
+        return;
+    }
+    #[cfg(unix)]
+    let _ = std::fs::remove_file(socket_path());
+    #[cfg(windows)]
+    let _ = std::fs::remove_file(addr_path());
+    let _ = std::fs::remove_file(pid_path());
+}
+
+/// Removes the daemon's on-disk files when `run_daemon`'s frame is left for
+/// any reason: normal return, early `?` error, or an unwinding panic.
+/// (SIGKILL and other non-catchable terminations are inherently not covered.)
+struct CleanupGuard;
+
+impl Drop for CleanupGuard {
+    fn drop(&mut self) {
+        cleanup();
+    }
+}
+
 macro_rules! run_accept_loop_body {
-    ($accept:expr, $client:expr, $ws_url:expr) => {
+    ($accept:expr, $client:expr, $ws_url:expr, $shutdown:expr) => {
         loop {
-            let accept = tokio::time::timeout(idle_timeout(), $accept).await;
-
-            match accept {
-                Ok(Ok((stream, _))) => match handle_connection(stream, $client, $ws_url).await {
-                    ConnectionOutcome::Continue => {}
-                    ConnectionOutcome::Fatal => break,
-                },
-                Ok(Err(e)) => {
-                    eprintln!("daemon: accept error: {e}");
-                }
-                Err(_) => {
-                    // Idle timeout — exit
+            tokio::select! {
+                _ = &mut $shutdown => {
+                    // SIGTERM/SIGINT (or Ctrl-C on Windows) — exit cleanly.
+                    // Only observed between requests: an in-flight command
+                    // finishes and its response is written before shutdown.
                     break;
                 }
+                accept = tokio::time::timeout(idle_timeout(), $accept) => match accept {
+                    Ok(Ok((stream, _))) => match handle_connection(stream, $client, $ws_url).await {
+                        ConnectionOutcome::Continue => {}
+                        ConnectionOutcome::Fatal => break,
+                    },
+                    Ok(Err(e)) => {
+                        eprintln!("daemon: accept error: {e}");
+                    }
+                    Err(_) => {
+                        // Idle timeout — exit
+                        break;
+                    }
+                }
             }
         }
     };
 }
 
+/// SIGTERM/SIGINT must be turned into a normal accept-loop exit: their
+/// default disposition kills the process without unwinding, so `CleanupGuard`
+/// would never run and the socket/PID files would go stale. SIGHUP is
+/// deliberately left unhandled — convention reserves it for config reload,
+/// not shutdown.
+#[cfg(unix)]
+async fn shutdown_signal() {
+    use tokio::signal::unix::{signal, SignalKind};
+    // If a signal stream can't be registered, fall back to never resolving —
+    // the daemon then behaves as before this handler existed (default
+    // disposition still terminates the process; only file cleanup is lost).
+    let mut sigterm = match signal(SignalKind::terminate()) {
+        Ok(s) => s,
+        Err(_) => return std::future::pending().await,
+    };
+    let mut sigint = match signal(SignalKind::interrupt()) {
+        Ok(s) => s,
+        Err(_) => return std::future::pending().await,
+    };
+    tokio::select! {
+        _ = sigterm.recv() => {}
+        _ = sigint.recv() => {}
+    }
+}
+
+/// Best-effort on Windows: the daemon is spawned with CREATE_NO_WINDOW (no
+/// console), so SetConsoleCtrlHandler-based Ctrl-C delivery typically never
+/// fires for a backgrounded daemon. It does work when `__daemon__` is run
+/// manually in a foreground console for debugging.
+#[cfg(windows)]
+async fn shutdown_signal() {
+    if tokio::signal::ctrl_c().await.is_err() {
+        std::future::pending::<()>().await;
+    }
+}
+
 pub async fn run_daemon(ws_url: &str) -> Result<()> {
     // Write PID
+    // Armed before anything is written: cleanup() verifies pid-file ownership
+    // first, so firing "too early" is a no-op, and this declaration order
+    // means the startup lock below is released (locals drop in reverse order)
+    // before the guard's cleanup() tries to take it — no self-deadlock on an
+    // early `?` return or panic. Covers every way this frame is left,
+    // including unwinding panics, which previously leaked the files.
+    let _guard = CleanupGuard;
+
+    // Startup critical section: the pid write and endpoint (re)bind must not
+    // interleave with a predecessor's cleanup() check-then-remove, or the
+    // predecessor can delete files this daemon just claimed.
+    let startup_lock = lock_daemon_files();
+
     std::fs::write(pid_path(), std::process::id().to_string())?;
 
     #[cfg(unix)]
@@ -80,21 +204,20 @@ pub async fn run_daemon(ws_url: &str) -> Result<()> {
         listener
     };
 
+    drop(startup_lock);
+
     // We don't connect immediately. We wait for the first connection from the CLI.
     // This ensures the CLI wait_for_daemon() succeeds, and the CLI blocks on read_msg()
     // while the daemon handles the potentially slow macOS/Chrome network permission prompt.
     let mut client: Option<CdpClient> = None;
 
-    // Signal readiness by socket/address existence (it's already bound)
-    run_accept_loop_body!(listener.accept(), &mut client, ws_url);
-
-    #[cfg(unix)]
-    let _ = std::fs::remove_file(socket_path());
+    let shutdown = shutdown_signal();
+    tokio::pin!(shutdown);
 
-    #[cfg(windows)]
-    let _ = std::fs::remove_file(addr_path());
+    // Signal readiness by socket/address existence (it's already bound)
+    run_accept_loop_body!(listener.accept(), &mut client, ws_url, shutdown);
 
-    let _ = std::fs::remove_file(pid_path());
+    // File cleanup is handled by `_guard` (also covers signal/panic exits).
 
     // Shut down telemetry before exiting so the background thread
     // flushes pending entries and exits cleanly.

src/lib.rs

@@ -989,6 +989,43 @@ pub async fn run() -> Result<()> {
                 ));
             }
         }
+
+        // Best-effort sweep of the pre-uid-suffix file names: a daemon
+        // started by an older binary is invisible to the paths above, so
+        // without this it could only be stopped by waiting out its idle
+        // timeout. Silent when no legacy files exist (the common case).
+        #[cfg(unix)]
+        {
+            let legacy_pid_path = protocol::legacy_pid_path();
+            if let Ok(pid_str) = std::fs::read_to_string(&legacy_pid_path) {
+                // Same corrupted-PID-file guards as above: never signal pid 0
+                // (whole process group) or a value that wraps negative.
+                let legacy_pid = pid_str
+                    .trim()
+                    .parse::<u32>()
+                    .ok()
+                    .filter(|&p| p != 0)
+                    .and_then(|p| i32::try_from(p).ok());
+                if let Some(pid) = legacy_pid {
+                    let ret = unsafe { libc::kill(pid as libc::pid_t, libc::SIGTERM) };
+                    let gone = ret == 0
+                        || std::io::Error::last_os_error().raw_os_error() == Some(libc::ESRCH);
+                    if gone {
+                        // Old binaries have no SIGTERM handler and exit
+                        // without cleanup, so remove their files here.
+                        let _ = std::fs::remove_file(protocol::legacy_socket_path());
+                        let _ = std::fs::remove_file(&legacy_pid_path);
+                        if ret == 0 {
+                            println!("Also stopped legacy daemon (PID {pid}).");
+                        }
+                    }
+                } else {
+                    // Unusable PID content — the files are junk; remove them.
+                    let _ = std::fs::remove_file(protocol::legacy_socket_path());
+                    let _ = std::fs::remove_file(&legacy_pid_path);
+                }
+            }
+        }
         return Ok(());
     }
 

src/protocol.rs

@@ -48,23 +48,57 @@ pub struct DaemonResponse {
     pub error_code: Option<u32>,
 }
 
+/// Per-user filename suffix. `temp_dir()` is per-user on macOS ($TMPDIR) and
+/// Windows (%TEMP%), but shared /tmp on Linux — a fixed name there lets users
+/// collide on (or squat) each other's daemon files.
+#[cfg(unix)]
+fn user_suffix() -> String {
+    format!("-{}", unsafe { libc::getuid() })
+}
+
+#[cfg(windows)]
+fn user_suffix() -> String {
+    String::new()
+}
+
 /// Path to the Unix domain socket for daemon communication.
 #[cfg(unix)]
 pub fn socket_path() -> PathBuf {
-    std::env::temp_dir().join("chrome-devtools-daemon.sock")
+    std::env::temp_dir().join(format!("chrome-devtools-daemon{}.sock", user_suffix()))
 }
 
 /// Path to the named-pipe address file for daemon communication (Windows).
 #[cfg(windows)]
 pub fn addr_path() -> PathBuf {
-    std::env::temp_dir().join("chrome-devtools-daemon.addr")
+    std::env::temp_dir().join(format!("chrome-devtools-daemon{}.addr", user_suffix()))
 }
 
 /// Path to the daemon PID file.
 pub fn pid_path() -> PathBuf {
+    std::env::temp_dir().join(format!("chrome-devtools-daemon{}.pid", user_suffix()))
+}
+
+/// Path to the lock file serializing daemon startup and cleanup.
+///
+/// The lock file is never removed once created: deleting it while another
+/// process may be about to lock it would reintroduce the race it prevents.
+pub fn lock_path() -> PathBuf {
+    std::env::temp_dir().join(format!("chrome-devtools-daemon{}.lock", user_suffix()))
+}
+
+/// Pre-uid-suffix PID file name, so `kill-daemon` can still stop a daemon
+/// left running by an older binary after an upgrade.
+#[cfg(unix)]
+pub fn legacy_pid_path() -> PathBuf {
     std::env::temp_dir().join("chrome-devtools-daemon.pid")
 }
 
+/// Pre-uid-suffix socket name (see [`legacy_pid_path`]).
+#[cfg(unix)]
+pub fn legacy_socket_path() -> PathBuf {
+    std::env::temp_dir().join("chrome-devtools-daemon.sock")
+}
+
 /// Write a length-prefixed message to a stream.
 pub async fn write_msg<W: AsyncWriteExt + Unpin>(w: &mut W, data: &[u8]) -> anyhow::Result<()> {
     let len = (data.len() as u32).to_be_bytes();