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

AGENTS.md

@@ -62,9 +62,13 @@ 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 keeps a persistent CDP WebSocket connection. On Unix it
+listens on `$TMPDIR/chrome-devtools-daemon-<uid>.sock` (uid-suffixed to isolate
+users sharing /tmp); on Windows, on a loopback TCP port published via an
+unsuffixed `%TEMP%` addr file. First CLI invocation spawns it; subsequent
+commands reuse it. 5-minute idle timeout; endpoint/PID files are cleaned up on
+panics too, and on Unix on SIGTERM/SIGINT (Windows Ctrl-C cleanup is
+best-effort — a background daemon has no console).
 
 `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,8 @@ 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;
+        │              loopback TCP on Windows)
         │   └─ If running → send command → get result
         │
         ├─ If no daemon → spawn one (background process)
@@ -215,10 +216,12 @@ 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`
-- **Idle timeout**: 5 minutes (auto-exits, cleans up socket)
-- **Protocol**: Length-prefixed JSON over Unix socket
+- **Endpoint (Unix)**: socket at `$TMPDIR/chrome-devtools-daemon-<uid>.sock` (uid-suffixed so users on a shared machine don't collide)
+- **Endpoint (Windows)**: loopback TCP listener; its address is written to `%TEMP%\chrome-devtools-daemon.addr` (`%TEMP%` is already per-user, so no suffix)
+- **PID file**: `$TMPDIR/chrome-devtools-daemon-<uid>.pid` (Windows: `%TEMP%\chrome-devtools-daemon.pid`)
+- **Idle timeout**: 5 minutes (auto-exits, cleans up its files)
+- **Cleanup**: endpoint + PID files are also removed on panics, and on Unix on SIGTERM/SIGINT; Windows Ctrl-C cleanup is best-effort only (a background daemon has no console to receive it)
+- **Protocol**: Length-prefixed JSON over the Unix socket / loopback TCP
 - **Spawned by**: First CLI invocation (transparent to user)
 - **Kill**: `chrome-devtools kill-daemon` (or delete the socket + PID file)
 

skill/chrome-devtools/SKILL.md

@@ -367,6 +367,53 @@ profiles/machines) produces a meaningless result where nearly everything is
 reported as both added and removed — the CLI prints a warning on stderr when
 it detects this.
 
+### Pattern 16: Headless Chrome (No Login, No Human Approval)
+
+When the flow under test doesn't need the user's cookies/credentials, spawn a
+throwaway headless Chrome instead of attaching to the user's browser. Because
+the instance is launched with remote debugging already enabled, **no consent
+prompt ever appears** — the whole flow runs unattended.
+
+```bash
+PROFILE=$(mktemp -d)
+
+# 1. If a daemon is already attached to the user's real Chrome, stop it first
+#    (the daemon is per-user and sticks to whichever Chrome it first connected to)
+chrome-devtools kill-daemon --force
+
+# 2. Spawn headless Chrome with an isolated profile; port 0 = pick a free port
+"/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" \
+  --headless=new --remote-debugging-port=0 \
+  --user-data-dir="$PROFILE" \
+  --no-first-run --no-default-browser-check \
+  about:blank &
+CHROME_PID=$!
+
+# 3. Wait for Chrome to publish its debug port
+while [ ! -f "$PROFILE/DevToolsActivePort" ]; do sleep 0.5; done
+
+# 4. Every command needs --user-data-dir pointing at the headless profile;
+#    the CLI auto-connects by reading its DevToolsActivePort
+chrome-devtools --user-data-dir "$PROFILE" navigate https://example.com
+chrome-devtools --user-data-dir "$PROFILE" evaluate 'document.title'
+chrome-devtools --user-data-dir "$PROFILE" screenshot --output /tmp/shot.png
+
+# 5. Cleanup — REQUIRED: the daemon is now bound to the headless instance and
+#    would otherwise hijack later commands aimed at the user's real Chrome
+chrome-devtools kill-daemon --force
+kill $CHROME_PID
+rm -rf "$PROFILE"
+```
+
+Linux path: `google-chrome` or `chromium` on `$PATH` replaces the macOS
+`.app` binary path.
+
+**⚠️ One daemon per user, bound to one Chrome.** The daemon connects to
+whichever Chrome the first command resolved, and later commands reuse it even
+if their flags point elsewhere. Always `kill-daemon --force` when switching
+between the user's Chrome and a headless instance — in both directions
+(steps 1 and 5 above).
+
 ## Complete Command Reference
 
 ### Navigation

src/daemon.rs

@@ -1,4 +1,4 @@
-use anyhow::Result;
+use anyhow::{Context, Result};
 use std::time::Duration;
 use tokio::io::{AsyncReadExt, AsyncWriteExt};
 #[cfg(windows)]
@@ -29,30 +29,171 @@ enum ConnectionOutcome {
     Fatal,
 }
 
+fn open_lock_file() -> std::io::Result<std::fs::File> {
+    std::fs::OpenOptions::new()
+        .create(true)
+        .write(true)
+        .truncate(false)
+        .open(lock_path())
+}
+
+/// 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.
+///
+/// Errors instead of degrading: the lock is what makes ownership handoff
+/// correct, and a daemon that can't create a file in temp_dir couldn't write
+/// its PID file either — failing here just surfaces the cause sooner.
+fn lock_daemon_files() -> Result<std::fs::File> {
+    let f = open_lock_file()
+        .with_context(|| format!("Failed to open daemon lock file {}", lock_path().display()))?;
+    f.lock()
+        .with_context(|| format!("Failed to lock daemon lock file {}", lock_path().display()))?;
+    Ok(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 lock_file = match open_lock_file() {
+        Ok(f) => f,
+        Err(e) => {
+            // Without the lock, removal could race a replacement's startup —
+            // leaving the files is the safe side (they self-heal on the next
+            // daemon start), but say why so the cause isn't swallowed.
+            eprintln!(
+                "daemon: leaving socket/PID files in place: cannot open lock file {}: {e}",
+                lock_path().display()
+            );
+            return;
+        }
+    };
+    match lock_file.try_lock() {
+        Ok(()) => {}
+        Err(std::fs::TryLockError::WouldBlock) => {
+            // Contended: a replacement is mid-startup. Its rebind supersedes
+            // our files, and any leftovers self-heal on the next start.
+            return;
+        }
+        Err(std::fs::TryLockError::Error(e)) => {
+            eprintln!(
+                "daemon: leaving socket/PID files in place: cannot lock {}: {e}",
+                lock_path().display()
+            );
+            return;
+        }
+    }
+    let _lock = lock_file;
+    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 +221,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);
+    let shutdown = shutdown_signal();
+    tokio::pin!(shutdown);
 
-    #[cfg(unix)]
-    let _ = std::fs::remove_file(socket_path());
-
-    #[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.