diff --git a/README.md b/README.md index eec27cd..28b616c 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,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 $TMPDIR/chrome-devtools-daemon-.sock; + ├─ Try daemon (Unix socket $TMPDIR/chrome-devtools-daemon--.sock; │ loopback TCP on Windows) │ └─ If running → send command → get result │ @@ -72,6 +72,14 @@ Chrome must have remote debugging enabled: ## Auto-connect +Enable remote debugging once per browser at `chrome://inspect/#remote-debugging` +(Edge: `edge://inspect/#remote-debugging`). It applies to the running process +immediately and persists across restarts — it is stored in the profile's +`Local State` as `devtools.remote_debugging.user-enabled`, so no launch flag is +needed. The toggle is per-browser: enabling it in Chrome does not enable it in +Edge. The `--remote-debugging-port` flag is the alternative, and is meant for a +throwaway instance rather than your everyday browser. + By default, the CLI reads `DevToolsActivePort` from Chrome's user data directory: | OS | Default path | @@ -80,14 +88,29 @@ By default, the CLI reads `DevToolsActivePort` from Chrome's user data directory | Linux | `~/.config/google-chrome/` | | Windows | `%LOCALAPPDATA%\Google\Chrome\User Data\` | -Override with `--user-data-dir`, `--channel` (beta/canary/dev), or `--ws-endpoint`. All three also read from environment variables: +Override with `--user-data-dir`, `--browser` (chrome/edge), `--channel` (beta/canary/dev), or `--ws-endpoint`. All four also read from environment variables: | Environment Variable | Corresponding Flag | |----------------------|--------------------| | `CHROME_WS_ENDPOINT` | `--ws-endpoint` | | `CHROME_USER_DATA_DIR` | `--user-data-dir` | +| `CHROME_BROWSER` | `--browser` | | `CHROME_CHANNEL` | `--channel` | +### Microsoft Edge + +Edge is Chromium and speaks the same DevTools Protocol, so every command works against it. Pass `--browser edge` to auto-connect to Edge's profile instead of Chrome's: + +| OS | Default path | +|----|-------------| +| macOS | `~/Library/Application Support/Microsoft Edge/` | +| Linux | `~/.config/microsoft-edge/` | +| Windows | `%LOCALAPPDATA%\Microsoft\Edge\User Data\` | + +`--channel` composes with it (`--browser edge --channel beta`). Edge Canary is not distributed for Linux, and that combination is rejected rather than pointed at a directory that cannot exist. An explicit `--ws-endpoint` or `--user-data-dir` needs no `--browser` — it only selects the default profile location. + +Enterprise-managed Edge can have remote debugging disabled by policy; `DevToolsActivePort` then never appears and auto-connect fails with the message above. That is the same failure mode as Chrome under the equivalent policy, just more common on managed fleets. + ## Page targeting Every page-level command outputs a friendly target name like `[target:red-snake]`. This is a deterministic word-pair derived from Chrome's internal target ID — same page always gets the same name. @@ -204,7 +227,7 @@ A drain without a `--duration` returns instantly. Adding `--duration N` switches |---------|-------------| | `kill-daemon` | Stop the background daemon cleanly | -`kill-daemon` signals the running daemon with `SIGTERM`, removes the socket and PID file, and exits. It's a no-op if no daemon is running. Prefer this over `pkill -f __daemon__` — the process name is shared by legitimate Chrome children processes. +On Unix, `kill-daemon` signals the targeted daemon with `SIGTERM`, removes its socket, info and PID files, and exits. It's a no-op if no daemon is running. Prefer this over `pkill -f __daemon__` — the process name is shared by legitimate Chrome children processes. On Windows it is not supported: it prints that and exits without signalling anything or removing any files (see **Kill (Windows)** below). ## Global options @@ -217,23 +240,29 @@ A drain without a `--duration` returns instantly. Adding `--duration N` switches | `--block-url ` | Add a URL pattern to the active tab's block list (repeatable; persists until un-blocked or cleared) | | `--unblock-url ` | Remove a URL pattern from the active tab's block list (repeatable) | | `--ws-endpoint ` | Explicit WebSocket URL | -| `--user-data-dir ` | Custom Chrome profile directory | -| `--channel ` | Chrome channel (stable/beta/canary/dev) | +| `--user-data-dir ` | Custom browser profile directory | +| `--browser ` | Browser to auto-connect to (chrome/edge) | +| `--channel ` | Browser release channel (stable/beta/canary/dev) | + +Commands: `list-daemons` shows every daemon this user has on-disk state for — running ones and, on Unix, stale entries left by a daemon that died without cleaning up; `kill-daemon [--all]` stops one or all. Global `--block-url` and `--unblock-url` update the **active tab's** block list and apply via `Network.setBlockedURLs`; the daemon re-applies each tab's list when that tab is in use, so blocking is isolated per tab. **Note:** Chrome only blocks *subresources* (images, scripts, fetch/XHR, stylesheets, CDN, trackers, fonts). The top-level navigation document itself is never blocked — e.g. `--block-url "*example.com*"` then `navigate https://example.com` still loads the page, but any `*.png`, `*.woff2`, etc. subresources on it are blocked. ## Daemon details -- **Endpoint (Unix)**: socket at `$TMPDIR/chrome-devtools-daemon-.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-.pid` (Windows: `%TEMP%\chrome-devtools-daemon.pid`) -- **Lock file**: `$TMPDIR/chrome-devtools-daemon-.lock` (Windows: `%TEMP%\chrome-devtools-daemon.lock`) — serializes daemon startup/cleanup; intentionally never removed automatically. Locks bind to the inode, not the name: deleting the file while any daemon process is still starting, running, or shutting down lets a new process lock a fresh replacement inode and bypass the serialization entirely. Only delete it once no daemon process exists at all — and there's rarely a reason to, since a leftover lock file is harmless. +- **Instance identity**: one daemon per browser endpoint. The socket/PID/info filenames carry a 16-hex-digit key derived from the resolved `ws://` URL, so Chrome, Edge, every channel and every headless instance get a daemon of their own, and a command aimed at one browser can never be served by a daemon attached to another. The URL's browser GUID changes on every browser launch, so a restarted browser gets a fresh daemon instead of inheriting a dead connection; the orphan exits on its idle timeout. +- **Endpoint (Unix)**: socket at `$TMPDIR/chrome-devtools-daemon--.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 there is no uid suffix — but the instance key is still present, since one daemon per endpoint applies on every platform) +- **PID file**: `$TMPDIR/chrome-devtools-daemon--.pid` (Windows: `%TEMP%\chrome-devtools-daemon-.pid`) +- **Info file**: `$TMPDIR/chrome-devtools-daemon--.info` — JSON naming the browser, endpoint, PID and start time, so `list-daemons` can label rows. Best-effort: a daemon with no info file still lists, with `?` columns. +- **Lock file**: `$TMPDIR/chrome-devtools-daemon-.lock` (Windows: `%TEMP%\chrome-devtools-daemon.lock`) — **not** keyed per instance: one lock covers all of them, because it only serializes the brief write-pid-then-bind window, while a per-instance lock would accumulate a never-removed file per browser session. Serializes daemon startup/cleanup; intentionally never removed automatically. Locks bind to the inode, not the name: deleting the file while any daemon process is still starting, running, or shutting down lets a new process lock a fresh replacement inode and bypass the serialization entirely. Only delete it once no daemon process exists at all — and there's rarely a reason to, since a leftover lock file is harmless. - **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). SIGQUIT, SIGHUP and SIGKILL skip cleanup by design — the leftover files are harmless and are reclaimed by the next daemon start. - **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; leave the lock file — see above). It sends SIGTERM and returns once the signal is delivered, not once the process is gone: the daemon exits *between* requests, so one that is mid-command finishes it and answers that client first. Expect up to one command's latency, and note that a daemon wedged inside a CDP call outlives the command that stopped it. -- **Kill (Windows)**: not supported — `kill-daemon` says so and exits, and a backgrounded daemon has no console for Ctrl-C. Use `taskkill /PID ` with the PID from `%TEMP%\chrome-devtools-daemon.pid`, or wait out the idle timeout. +- **Spawned by**: First CLI invocation for a given endpoint (transparent to user) +- **List**: `chrome-devtools list-daemons` — PID, browser, endpoint, uptime and state for every daemon this user owns. `--json` (or `--toon`) for machine-readable output. Reads only on-disk state, so it works when every browser has exited. On Unix, rows whose PID no longer exists are marked `stale`; on Windows liveness is not probed, so every row's state is `?` and `stale` never appears. +- **Kill**: `chrome-devtools kill-daemon` stops only the daemon for the endpoint its flags resolve to, so `--browser edge kill-daemon` cannot stop your Chrome daemon. Add `--all` to stop every daemon for this user — which is also the only way to clear one whose browser has already exited, since a scoped kill has no endpoint left to resolve (it fails and says so). `--all` also sweeps the pre-key `chrome-devtools-daemon-.pid` name left by older versions. (Or delete the socket + PID file by hand; leave the lock file — see above.) It sends SIGTERM and returns once the signal is delivered, not once the process is gone: the daemon exits *between* requests, so one that is mid-command finishes it and answers that client first. Expect up to one command's latency, and note that a daemon wedged inside a CDP call outlives the command that stopped it. +- **Kill (Windows)**: not supported — `kill-daemon` says so and exits, and a backgrounded daemon has no console for Ctrl-C. Run `chrome-devtools list-daemons` to get the PID of the daemon you want (there may be several, one per endpoint) and pass it to `taskkill /PID `, or wait out the idle timeout. Reading `%TEMP%\chrome-devtools-daemon-.pid` directly works too, but only if you already know which key you want. The daemon keeps a persistent CDP session on the current page to: - Continuously collect `Network.*` and `Runtime.consoleAPICalled`/`exceptionThrown` events for `console` and `network` drains. diff --git a/skill/chrome-devtools/SKILL.md b/skill/chrome-devtools/SKILL.md index a6d52bb..85bf01c 100644 --- a/skill/chrome-devtools/SKILL.md +++ b/skill/chrome-devtools/SKILL.md @@ -10,12 +10,45 @@ A CLI that talks directly to your running Chrome via the DevTools Protocol. ## Prerequisites -Chrome must have remote debugging enabled: -1. Open Chrome -2. Go to `chrome://inspect/#remote-debugging` +The browser must have remote debugging enabled: +1. Open Chrome (or Edge) +2. Go to `chrome://inspect/#remote-debugging` (Edge: `edge://inspect/#remote-debugging`) 3. Enable the remote debugging server -The CLI auto-connects — no URL needed. A daemon is spawned on first invocation and reused across commands (5-minute idle timeout). +This is a **persistent, in-browser toggle** — not a launch flag. Three things +follow from that, and they are what make it the right route for attaching to an +everyday browser: + +- **It takes effect immediately.** The server starts on the already-running + process; no restart, no relaunch, nothing to quit. +- **It survives restarts.** The choice is stored in the profile's `Local State` + as `devtools.remote_debugging.user-enabled`, so every later launch serves a + port with no flags. To check whether a profile has it on, read that key. +- **It is per-browser and per-profile.** Enabling it in Chrome does nothing for + Edge, and vice versa — each has its own toggle and its own `Local State`. + +The port is chosen by the browser and recorded in `/DevToolsActivePort`. +Read that file rather than assuming a number: it differs per browser and can +change between launches. Auto-connect does this for you, which is why no URL is +needed. A daemon is spawned on first invocation and reused across commands +(5-minute idle timeout). + +**Microsoft Edge** works the same — it is Chromium and speaks the same protocol. +Add `--browser edge` so auto-connect reads Edge's profile instead of Chrome's +(`--channel` still selects stable/beta/dev/canary, except that Edge ships no +Canary for Linux — `--browser edge --channel canary` is rejected there rather +than pointed at a directory that cannot exist). `--ws-endpoint` and +`--user-data-dir` need no `--browser`; they already say where to connect. +Everything below applies unchanged — only the profile location differs. + +The `--remote-debugging-port` launch flag is the *other* route, and it is for a +throwaway instance rather than your everyday browser — see the headless recipe +below. Two traps if you reach for it: it is ignored when that profile is already +running (the launch hands off to the existing process and no port file appears, +so quit first or use a different `--user-data-dir`), and a fresh Edge profile is +not the clean room it looks like — Edge's first-run import pulls open tabs and +extensions from your default browser, so a `mktemp -d` profile can come up +holding your real session. ## ⚠️ Critical: How Page Targeting Works @@ -377,9 +410,14 @@ 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 +# 1. Clear any daemon left over from a previous run of THIS profile. Not needed +# for isolation — daemons are per-endpoint, so the user's browser is +# unaffected either way — but a stale one here would hold a dead connection. +# Best-effort: a scoped kill has to resolve this profile's endpoint, and on a +# fresh $PROFILE there is none yet, so it exits non-zero and removes nothing. +# `|| true` keeps that expected failure from aborting the script under `set -e`. +# Do NOT reach for --all here: it would also stop the user's own daemons. +chrome-devtools --user-data-dir "$PROFILE" kill-daemon --force 2>/dev/null || true # 2. Spawn headless Chrome with an isolated profile; port 0 = pick a free port "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" \ @@ -389,11 +427,15 @@ chrome-devtools kill-daemon --force about:blank & CHROME_PID=$! -# 3. Cleanup — REQUIRED even if a later step fails: the daemon is bound to the -# headless instance and would otherwise hijack later commands aimed at the -# user's real Chrome. A trap runs it on every exit path, not just success. +# 3. Cleanup — a trap runs it on every exit path, not just success. The daemon +# can no longer hijack commands aimed at another browser, but it does hold a +# CDP connection and an idle timer, so stop it rather than leaking it. cleanup() { - chrome-devtools kill-daemon --force + # Scoped to this profile: a bare kill-daemon would resolve the user's default + # Chrome profile and stop their daemon instead of this one. Best-effort for + # the same reason as step 1 — if Chrome died before writing its port file + # there is no endpoint to resolve, and a trap must not fail on that. + chrome-devtools --user-data-dir "$PROFILE" kill-daemon --force 2>/dev/null || true kill "$CHROME_PID" 2>/dev/null # Chrome shuts down asynchronously, so deleting the profile right after # SIGTERM races its teardown and can leave it running against a directory @@ -431,13 +473,47 @@ chrome-devtools --user-data-dir "$PROFILE" screenshot --output /tmp/shot.png ``` 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 -(step 1 and the EXIT trap above). +`.app` binary path. For a headless Edge, swap the binary for +`/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge` (Linux: +`microsoft-edge`) — the flags are identical, and `--user-data-dir` already +points the CLI at the right profile, so `--browser edge` is optional here. + +**⚠️ A temp profile is not automatically a clean room in Edge.** Edge's +first-run import pulls open tabs *and* extensions from the default browser, and +`--no-first-run` does not reliably suppress it, so a `mktemp -d` profile can come +up holding a copy of the real browsing session — and the debug port then fronts a +signed-in profile. Verify what you actually got with `list-pages` before +assuming isolation, and treat the port as sensitive until you have. + +**One daemon per browser endpoint.** A daemon is identified by the endpoint it +is attached to, so the user's Chrome, the user's Edge, and each headless +instance each get their own — you can drive them concurrently, and a command +can never be answered by a daemon attached to a different browser. Two +consequences for the recipe above: + +- Step 1 is not needed for isolation. It only clears a daemon left over from a + *previous* run of this same profile. +- The EXIT trap must scope its kill to this profile: + `chrome-devtools --user-data-dir "$PROFILE" kill-daemon --force`. A bare + `kill-daemon` resolves the default Chrome profile and would stop the user's + daemon instead of the headless one. + +`list-daemons` shows every daemon with on-disk state — running ones and, on +Unix, stale entries whose process is gone — with PID, browser, endpoint and +uptime for each, and on +Unix `kill-daemon --all` clears every daemon regardless of endpoint — including +ones whose browser has already exited, which a scoped kill cannot reach because +it has no endpoint left to resolve. + +On Windows neither form of `kill-daemon` stops anything: it prints that it is +unsupported and exits. Take the PID from `list-daemons` (its state column reads +`?` there, since liveness is not probed) and pass it to `taskkill /PID `, +or wait out the 5-minute idle timeout. + +Older versions ran a single daemon per user, bound to whichever browser the +first command resolved, and silently ignored later `--browser`/`--user-data-dir` +flags. If you are reading advice that says to `kill-daemon` when switching +browsers, it predates this. ## Complete Command Reference @@ -513,13 +589,16 @@ chrome-devtools kill-daemon # refuses when run non-interactively (age chrome-devtools kill-daemon --force # kills unconditionally ``` -## Failure Handling: "Failed to connect to Chrome" / a command hangs +## Failure Handling: "Failed to connect to \" / a command hangs Chrome's remote-debugging connection requires a one-time **human approval dialog** in Chrome. If a command hangs or fails with a connection/timeout error, the most likely cause is that this dialog is open and waiting for the human — not a bug you can fix by retrying. +The error names whichever browser you targeted — `Failed to connect to Chrome`, +or `Failed to connect to Microsoft Edge` under `--browser edge`. + **If a command hangs for a long time or errors with "Failed to connect to Chrome" or "Timed out ... connecting to Chrome":** 1. Retry **at most once** (the human may have already approved it just now). diff --git a/src/browser.rs b/src/browser.rs index 3482dfb..ce587c0 100644 --- a/src/browser.rs +++ b/src/browser.rs @@ -1,41 +1,55 @@ use anyhow::{anyhow, bail, Result}; use std::path::{Path, PathBuf}; -/// Resolve the WebSocket URL for connecting to Chrome. +/// Resolve the WebSocket URL for connecting to the browser. /// /// Priority: -/// 1. Explicit --ws-endpoint -/// 2. Auto-connect via DevToolsActivePort (default) +/// 1. Explicit `--ws-endpoint` +/// 2. Auto-connect via `DevToolsActivePort` (default) +/// +/// An explicit endpoint wins because it names the browser directly, so it must +/// not be second-guessed by local profile discovery: it is how you reach a +/// browser this machine cannot find on disk — another host, a container, a +/// port-forwarded device, or an instance whose profile lives somewhere the +/// channel tables don't describe. It also short-circuits `--browser` and +/// `--channel` entirely, since those exist only to locate a profile directory. +/// `DevToolsActivePort` is the automatic fallback for the ordinary case where +/// the browser is local and its profile is where the vendor puts it. pub fn resolve_ws_url( ws_endpoint: Option<&str>, user_data_dir: Option<&str>, + browser: &str, channel: &str, ) -> Result { if let Some(ws) = ws_endpoint { return Ok(ws.to_string()); } - // Auto-connect: read DevToolsActivePort from Chrome's user data directory + let browser = Browser::parse(browser)?; + + // Auto-connect: read DevToolsActivePort from the browser's user data directory let data_dir = match user_data_dir { Some(dir) => PathBuf::from(dir), - None => default_chrome_user_data_dir(channel)?, + None => browser.default_user_data_dir(channel)?, }; - read_devtools_active_port(&data_dir) + read_devtools_active_port(&data_dir, browser) } /// Read DevToolsActivePort file and construct the WebSocket URL. -fn read_devtools_active_port(user_data_dir: &Path) -> Result { +fn read_devtools_active_port(user_data_dir: &Path, browser: Browser) -> Result { let port_path = user_data_dir.join("DevToolsActivePort"); + let label = browser.label(); let content = std::fs::read_to_string(&port_path).map_err(|_| { anyhow!( "Could not read DevToolsActivePort at {}\n\n\ - Make sure Chrome is running with remote debugging enabled:\n\ - 1. Open Chrome\n\ - 2. Go to chrome://inspect/#remote-debugging\n\ + Make sure {label} is running with remote debugging enabled:\n\ + 1. Open {label}\n\ + 2. Go to {}://inspect/#remote-debugging\n\ 3. Enable the remote debugging server", - port_path.display() + port_path.display(), + browser.scheme() ) })?; @@ -57,54 +71,326 @@ fn read_devtools_active_port(user_data_dir: &Path) -> Result { .map_err(|_| anyhow!("Invalid port '{}' in DevToolsActivePort", lines[0]))?; if port == 0 { - bail!("Port 0 in DevToolsActivePort — Chrome may not be running"); + bail!("Port 0 in DevToolsActivePort — {label} may not be running"); } let path = lines[1]; Ok(format!("ws://127.0.0.1:{port}{path}")) } -/// Get the default Chrome user data directory for the given channel. -fn default_chrome_user_data_dir(channel: &str) -> Result { +/// Canonical spelling of a browser name, for recording and display — so a +/// daemon spawned by `--browser EDGE` or `--browser msedge` is still labelled +/// `edge` in `list-daemons`. +/// +/// Falls back to the input for names we don't know: endpoint resolution has +/// already rejected those, so this is only reached for display. +pub fn canonical_name(name: &str) -> String { + Browser::parse(name).map_or_else(|_| name.to_string(), |b| b.flag_name().to_string()) +} + +/// Human-readable browser name for messages the user reads — "Chrome", +/// "Microsoft Edge". +/// +/// Falls back to the input for names we don't know, so a diagnostic never +/// silently claims the wrong browser. +pub fn display_name(name: &str) -> String { + Browser::parse(name).map_or_else(|_| name.to_string(), |b| b.label().to_string()) +} + +/// A Chromium-based browser the CLI knows how to auto-connect to. +/// +/// Both speak the same DevTools Protocol; they differ only in where the +/// profile (and therefore `DevToolsActivePort`) lives. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Browser { + Chrome, + Edge, +} + +impl Browser { + /// Case- and whitespace-insensitive, so `--browser Edge` and + /// `CHROME_BROWSER=EDGE` work. The error quotes the name as typed. + fn parse(name: &str) -> Result { + match name.trim().to_ascii_lowercase().as_str() { + "chrome" => Ok(Self::Chrome), + "edge" | "msedge" => Ok(Self::Edge), + _ => bail!("Unknown browser: {name} (expected 'chrome' or 'edge')"), + } + } + + /// Human-readable name, for error messages. + fn label(self) -> &'static str { + match self { + Self::Chrome => "Chrome", + Self::Edge => "Microsoft Edge", + } + } + + /// Canonical `--browser` spelling, for anything that records or displays + /// the choice. Kept separate from [`Browser::scheme`] so the two can + /// diverge if a browser ever needs different values. + fn flag_name(self) -> &'static str { + match self { + Self::Chrome => "chrome", + Self::Edge => "edge", + } + } + + /// URL scheme for the `://inspect` hint. + fn scheme(self) -> &'static str { + match self { + Self::Chrome => "chrome", + Self::Edge => "edge", + } + } + + /// Default user data directory for the given release channel. + fn default_user_data_dir(self, channel: &str) -> Result { + // Matched case-insensitively for the same reason as `parse`; the error + // arms still report the channel as the user typed it. + let normalized = channel.trim().to_ascii_lowercase(); + let channel_key = normalized.as_str(); + + #[cfg(target_os = "macos")] + { + let home = + dirs::home_dir().ok_or_else(|| anyhow!("Cannot determine home directory"))?; + let base = home.join("Library/Application Support"); + let dir = match (self, channel_key) { + (Self::Chrome, "stable" | "chrome") => base.join("Google/Chrome"), + (Self::Chrome, "beta") => base.join("Google/Chrome Beta"), + (Self::Chrome, "canary") => base.join("Google/Chrome Canary"), + (Self::Chrome, "dev") => base.join("Google/Chrome Dev"), + (Self::Edge, "stable" | "edge") => base.join("Microsoft Edge"), + (Self::Edge, "beta") => base.join("Microsoft Edge Beta"), + (Self::Edge, "canary") => base.join("Microsoft Edge Canary"), + (Self::Edge, "dev") => base.join("Microsoft Edge Dev"), + _ => bail!("Unknown {} channel: {channel}", self.label()), + }; + Ok(dir) + } + + #[cfg(target_os = "linux")] + { + let home = + dirs::home_dir().ok_or_else(|| anyhow!("Cannot determine home directory"))?; + let dir = match (self, channel_key) { + (Self::Chrome, "stable" | "chrome") => home.join(".config/google-chrome"), + (Self::Chrome, "beta") => home.join(".config/google-chrome-beta"), + // Chrome ships no Canary for Linux; unstable is the dev channel. + (Self::Chrome, "canary" | "dev") => home.join(".config/google-chrome-unstable"), + (Self::Edge, "stable" | "edge") => home.join(".config/microsoft-edge"), + (Self::Edge, "beta") => home.join(".config/microsoft-edge-beta"), + (Self::Edge, "dev") => home.join(".config/microsoft-edge-dev"), + (Self::Edge, "canary") => { + bail!("Microsoft Edge Canary is not distributed for Linux") + } + _ => bail!("Unknown {} channel: {channel}", self.label()), + }; + Ok(dir) + } + + #[cfg(target_os = "windows")] + { + let local_app_data = + std::env::var("LOCALAPPDATA").map_err(|_| anyhow!("LOCALAPPDATA not set"))?; + let base = PathBuf::from(local_app_data); + let dir = match (self, channel_key) { + (Self::Chrome, "stable" | "chrome") => base.join("Google/Chrome/User Data"), + (Self::Chrome, "beta") => base.join("Google/Chrome Beta/User Data"), + (Self::Chrome, "canary") => base.join("Google/Chrome SxS/User Data"), + (Self::Chrome, "dev") => base.join("Google/Chrome Dev/User Data"), + (Self::Edge, "stable" | "edge") => base.join("Microsoft/Edge/User Data"), + (Self::Edge, "beta") => base.join("Microsoft/Edge Beta/User Data"), + (Self::Edge, "canary") => base.join("Microsoft/Edge SxS/User Data"), + (Self::Edge, "dev") => base.join("Microsoft/Edge Dev/User Data"), + _ => bail!("Unknown {} channel: {channel}", self.label()), + }; + Ok(dir) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_known_browsers() { + assert_eq!(Browser::parse("chrome").unwrap(), Browser::Chrome); + assert_eq!(Browser::parse("edge").unwrap(), Browser::Edge); + assert_eq!(Browser::parse("msedge").unwrap(), Browser::Edge); + } + + #[test] + fn parses_browsers_case_insensitively() { + for name in ["Chrome", "CHROME", " chrome "] { + assert_eq!(Browser::parse(name).unwrap(), Browser::Chrome, "{name}"); + } + for name in ["Edge", "EDGE", "MSEdge", " edge "] { + assert_eq!(Browser::parse(name).unwrap(), Browser::Edge, "{name}"); + } + } + + #[test] + fn channels_are_matched_case_insensitively() { + for browser in [Browser::Chrome, Browser::Edge] { + assert_eq!( + browser.default_user_data_dir("stable").unwrap(), + browser.default_user_data_dir("STABLE").unwrap() + ); + assert_eq!( + browser.default_user_data_dir("beta").unwrap(), + browser.default_user_data_dir(" Beta ").unwrap() + ); + } + } + + /// The error must quote what the user typed, not the normalized form. + #[test] + fn unknown_browser_error_quotes_the_original_spelling() { + let err = Browser::parse("FireFox").unwrap_err().to_string(); + assert!(err.contains("FireFox"), "{err}"); + } + + #[test] + fn canonical_name_normalizes_spelling_and_aliases() { + for name in ["edge", "Edge", "EDGE", "msedge", "MSEdge"] { + assert_eq!(canonical_name(name), "edge", "{name}"); + } + for name in ["chrome", "Chrome", "CHROME"] { + assert_eq!(canonical_name(name), "chrome", "{name}"); + } + // Unknown names pass through rather than being silently relabelled. + assert_eq!(canonical_name("firefox"), "firefox"); + } + + #[test] + fn rejects_unknown_browser() { + let err = Browser::parse("firefox").unwrap_err().to_string(); + assert!(err.contains("Unknown browser: firefox"), "{err}"); + } + + #[test] + fn rejects_cross_browser_channel_alias() { + // "chrome" is a stable alias for Chrome only, "edge" for Edge only. + assert!(Browser::Edge.default_user_data_dir("chrome").is_err()); + assert!(Browser::Chrome.default_user_data_dir("edge").is_err()); + } + + #[test] + fn rejects_unknown_channel() { + let err = Browser::Edge + .default_user_data_dir("nightly") + .unwrap_err() + .to_string(); + assert!(err.contains("Microsoft Edge channel: nightly"), "{err}"); + } + + #[test] + fn stable_and_self_named_channels_agree() { + for browser in [Browser::Chrome, Browser::Edge] { + let stable = browser.default_user_data_dir("stable").unwrap(); + let alias = browser.default_user_data_dir(browser.scheme()).unwrap(); + assert_eq!(stable, alias); + } + } + + #[test] + fn browsers_resolve_to_distinct_dirs() { + let chrome = Browser::Chrome.default_user_data_dir("stable").unwrap(); + let edge = Browser::Edge.default_user_data_dir("stable").unwrap(); + assert_ne!(chrome, edge); + } + #[cfg(target_os = "macos")] - { - let home = dirs::home_dir().ok_or_else(|| anyhow!("Cannot determine home directory"))?; + #[test] + fn macos_edge_paths() { + let home = dirs::home_dir().unwrap(); + let base = home.join("Library/Application Support"); + for (channel, expected) in [ + ("stable", "Microsoft Edge"), + ("beta", "Microsoft Edge Beta"), + ("dev", "Microsoft Edge Dev"), + ("canary", "Microsoft Edge Canary"), + ] { + assert_eq!( + Browser::Edge.default_user_data_dir(channel).unwrap(), + base.join(expected), + "channel {channel}" + ); + } + } + + #[cfg(target_os = "macos")] + #[test] + fn macos_chrome_paths_unchanged() { + let home = dirs::home_dir().unwrap(); let base = home.join("Library/Application Support/Google"); - let dir = match channel { - "stable" | "chrome" => base.join("Chrome"), - "beta" => base.join("Chrome Beta"), - "canary" => base.join("Chrome Canary"), - "dev" => base.join("Chrome Dev"), - _ => bail!("Unknown Chrome channel: {channel}"), - }; - Ok(dir) + for (channel, expected) in [ + ("stable", "Chrome"), + ("beta", "Chrome Beta"), + ("dev", "Chrome Dev"), + ("canary", "Chrome Canary"), + ] { + assert_eq!( + Browser::Chrome.default_user_data_dir(channel).unwrap(), + base.join(expected), + "channel {channel}" + ); + } } #[cfg(target_os = "linux")] - { - let home = dirs::home_dir().ok_or_else(|| anyhow!("Cannot determine home directory"))?; - let dir = match channel { - "stable" | "chrome" => home.join(".config/google-chrome"), - "beta" => home.join(".config/google-chrome-beta"), - "canary" => home.join(".config/google-chrome-unstable"), - "dev" => home.join(".config/google-chrome-unstable"), - _ => bail!("Unknown Chrome channel: {channel}"), - }; - Ok(dir) + #[test] + fn linux_edge_paths() { + let home = dirs::home_dir().unwrap(); + for (channel, expected) in [ + ("stable", ".config/microsoft-edge"), + ("beta", ".config/microsoft-edge-beta"), + ("dev", ".config/microsoft-edge-dev"), + ] { + assert_eq!( + Browser::Edge.default_user_data_dir(channel).unwrap(), + home.join(expected), + "channel {channel}" + ); + } + assert!(Browser::Edge.default_user_data_dir("canary").is_err()); } #[cfg(target_os = "windows")] - { - let local_app_data = - std::env::var("LOCALAPPDATA").map_err(|_| anyhow!("LOCALAPPDATA not set"))?; - let base = PathBuf::from(local_app_data).join("Google"); - let dir = match channel { - "stable" | "chrome" => base.join("Chrome/User Data"), - "beta" => base.join("Chrome Beta/User Data"), - "canary" => base.join("Chrome SxS/User Data"), - "dev" => base.join("Chrome Dev/User Data"), - _ => bail!("Unknown Chrome channel: {channel}"), - }; - Ok(dir) + #[test] + fn windows_edge_paths() { + let base = PathBuf::from(std::env::var("LOCALAPPDATA").unwrap()); + for (channel, expected) in [ + ("stable", "Microsoft/Edge/User Data"), + ("beta", "Microsoft/Edge Beta/User Data"), + ("dev", "Microsoft/Edge Dev/User Data"), + ("canary", "Microsoft/Edge SxS/User Data"), + ] { + assert_eq!( + Browser::Edge.default_user_data_dir(channel).unwrap(), + base.join(expected), + "channel {channel}" + ); + } + } + + #[test] + fn ws_endpoint_short_circuits_browser_validation() { + // An explicit endpoint needs no profile, so the browser is irrelevant. + let ws = resolve_ws_url(Some("ws://127.0.0.1:9222/x"), None, "firefox", "stable").unwrap(); + assert_eq!(ws, "ws://127.0.0.1:9222/x"); + } + + #[test] + fn active_port_error_names_the_browser() { + let dir = std::env::temp_dir().join("chrome-devtools-cli-nonexistent-profile"); + let err = resolve_ws_url(None, Some(dir.to_str().unwrap()), "edge", "stable") + .unwrap_err() + .to_string(); + assert!(err.contains("Microsoft Edge is running"), "{err}"); + assert!(err.contains("edge://inspect"), "{err}"); } } diff --git a/src/cdp.rs b/src/cdp.rs index 4a104d1..d8063de 100644 --- a/src/cdp.rs +++ b/src/cdp.rs @@ -192,27 +192,31 @@ fn connect_timeout() -> std::time::Duration { } impl CdpClient { - /// Connect to Chrome via WebSocket and return a CDP client. + /// Connect to the browser via WebSocket and return a CDP client. /// - /// Bounded by a timeout: without it, a pending Chrome remote-debugging - /// consent dialog leaves the WebSocket handshake hanging indefinitely, - /// which (via the daemon) makes every command appear to hang forever - /// with no diagnostic. - pub async fn connect(ws_url: &str) -> Result { + /// Bounded by a timeout: without it, a pending remote-debugging consent + /// dialog leaves the WebSocket handshake hanging indefinitely, which (via + /// the daemon) makes every command appear to hang forever with no + /// diagnostic. + /// + /// `browser` is the human-readable name for the diagnostics only — these + /// messages are the ones a user actually reads, and telling an Edge user to + /// go and check Chrome sends them to the wrong window. + pub async fn connect(ws_url: &str, browser: &str) -> Result { let timeout_dur = connect_timeout(); let (ws, _) = tokio::time::timeout(timeout_dur, connect_async(ws_url)) .await .map_err(|_| { anyhow!( - "Timed out after {}s connecting to Chrome at {ws_url}. Chrome may be \ + "Timed out after {}s connecting to {browser} at {ws_url}. {browser} may be \ waiting for a human to approve the remote-debugging connection dialog. \ If you are an automated agent: do not retry in a loop and do not run \ kill-daemon (it will not fix this and will require a fresh approval) — \ - stop and ask the human to check Chrome, then retry once.", + stop and ask the human to check {browser}, then retry once.", timeout_dur.as_secs() ) })? - .map_err(|e| anyhow!("Failed to connect to Chrome at {ws_url}: {e}"))?; + .map_err(|e| anyhow!("Failed to connect to {browser} at {ws_url}: {e}"))?; let (write, read) = ws.split(); Ok(Self { write, diff --git a/src/client.rs b/src/client.rs index aa812b3..b4757ed 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,4 +1,4 @@ -use anyhow::{bail, Result}; +use anyhow::{bail, Context, Result}; #[cfg(windows)] use std::os::windows::process::CommandExt; use std::time::{Duration, SystemTime}; @@ -10,14 +10,26 @@ use tokio::net::UnixStream; use crate::protocol::*; #[cfg(unix)] -async fn connect_daemon() -> Result { - Ok(UnixStream::connect(socket_path()).await?) +async fn connect_daemon(key: &str) -> Result { + let path = socket_path(key); + UnixStream::connect(&path) + .await + .with_context(|| format!("Failed to connect to daemon {key} at {}", path.display())) } #[cfg(windows)] -async fn connect_daemon() -> Result { - let addr = std::fs::read_to_string(addr_path())?; - Ok(TcpStream::connect(addr.trim()).await?) +async fn connect_daemon(key: &str) -> Result { + let path = addr_path(key); + let addr = std::fs::read_to_string(&path).with_context(|| { + format!( + "Failed to read daemon {key} address file {}", + path.display() + ) + })?; + let addr = addr.trim(); + TcpStream::connect(addr) + .await + .with_context(|| format!("Failed to connect to daemon {key} at {addr}")) } /// Read the daemon wait timeout from `DAEMON_WAIT_TIMEOUT_SECS`, defaulting to @@ -32,9 +44,10 @@ pub(crate) fn daemon_wait_timeout() -> Duration { .unwrap_or(Duration::from_secs(5)) } -/// Try to send a request to the daemon. Returns error if daemon is not running. -pub async fn send_to_daemon(request: &DaemonRequest) -> Result { - let mut stream = connect_daemon().await?; +/// Try to send a request to the daemon for `key`. Returns an error if no +/// daemon is attached to that endpoint. +pub async fn send_to_daemon(key: &str, request: &DaemonRequest) -> Result { + let mut stream = connect_daemon(key).await?; let req_bytes = serde_json::to_vec(request)?; write_msg(&mut stream, &req_bytes).await?; @@ -45,10 +58,14 @@ pub async fn send_to_daemon(request: &DaemonRequest) -> Result { } /// Spawn the daemon process in the background. -pub fn spawn_daemon(ws_url: &str) -> Result<()> { +/// +/// `browser` is descriptive only — it is recorded in the daemon's info file so +/// `list-daemons` can name the browser. The daemon's identity comes from +/// `ws_url` alone. +pub fn spawn_daemon(ws_url: &str, browser: &str) -> Result<()> { let exe = std::env::current_exe()?; let mut cmd = std::process::Command::new(&exe); - cmd.args(["__daemon__", ws_url]) + cmd.args(["__daemon__", ws_url, browser]) .stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()); @@ -60,8 +77,9 @@ pub fn spawn_daemon(ws_url: &str) -> Result<()> { Ok(()) } -/// Wait for the daemon socket to become available, with exponential backoff. -pub async fn wait_for_daemon() -> Result<()> { +/// Wait for the daemon socket for `key` to become available, with exponential +/// backoff. +pub async fn wait_for_daemon(key: &str) -> Result<()> { let deadline = tokio::time::Instant::now() + daemon_wait_timeout(); let mut delay = Duration::from_millis(50); loop { @@ -71,7 +89,7 @@ pub async fn wait_for_daemon() -> Result<()> { daemon_wait_timeout().as_secs() ); } - if connect_daemon().await.is_ok() { + if connect_daemon(key).await.is_ok() { return Ok(()); } // Simple jitter based on current time subseconds diff --git a/src/commands/executor.rs b/src/commands/executor.rs index 75f4bcf..e936f78 100644 --- a/src/commands/executor.rs +++ b/src/commands/executor.rs @@ -88,7 +88,8 @@ pub fn known_args(cmd: &str) -> &'static [&'static str] { "output", "track_navigation", ], - "kill-daemon" => &["force"], + "kill-daemon" => &["force", "all"], + "list-daemons" => &[], _ => &[], } } @@ -130,7 +131,10 @@ fn validate_args(cmd: &str, args: &serde_json::Value) -> Result<()> { /// rather than hitting the `_ => unreachable!()` arm in the browser-level /// dispatch and panicking. fn is_browser_level(cmd: &str) -> bool { - matches!(cmd, "list-pages" | "new-page" | "sw-logs" | "kill-daemon") + matches!( + cmd, + "list-pages" | "new-page" | "sw-logs" | "kill-daemon" | "list-daemons" + ) } /// Execute a single command from a [`DaemonRequest`]. @@ -207,6 +211,9 @@ pub async fn execute_command(client: &mut CdpClient, req: &DaemonRequest) -> Res commands::sw_logs::collect_sw_logs(client, duration, extension_id, req.format()) .await } + "list-daemons" => Ok(CommandResult::output( + "list-daemons is handled directly by the CLI, not the daemon.", + )), "kill-daemon" => Ok(CommandResult::output( "kill-daemon is handled directly by the CLI, not the daemon.", )), diff --git a/src/daemon.rs b/src/daemon.rs index 2d0b1f8..4900d25 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -36,7 +36,7 @@ enum ConnectionOutcome { /// open() itself (it has no effect on regular files); the `is_file` check /// then rejects it. macOS $TMPDIR and Windows %TEMP% are per-user, so there /// the checks are inert. -fn open_lock_file() -> Result { +pub(crate) fn open_lock_file() -> Result { open_lock_file_at(&lock_path()) } @@ -95,7 +95,7 @@ fn open_lock_file_at(path: &std::path::Path) -> Result { /// pass the uid check, but planting one requires owning the target under /// Linux's default `protected_hardlinks`. #[cfg(unix)] -fn write_pid_file_checked(path: &std::path::Path) -> Result<()> { +fn write_private_file_checked(path: &std::path::Path, contents: &[u8], what: &str) -> Result<()> { use std::io::Write; use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; let mut f = std::fs::OpenOptions::new() @@ -106,10 +106,10 @@ fn write_pid_file_checked(path: &std::path::Path) -> Result<()> { .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK) .mode(0o600) .open(path) - .with_context(|| format!("Failed to open daemon PID file {}", path.display()))?; + .with_context(|| format!("Failed to open daemon {what} file {}", path.display()))?; let md = f.metadata().with_context(|| { format!( - "Failed to read metadata of daemon PID file {}", + "Failed to read metadata of daemon {what} file {}", path.display() ) })?; @@ -117,22 +117,53 @@ fn write_pid_file_checked(path: &std::path::Path) -> Result<()> { // thread-safe and cannot fail. if !md.is_file() || md.uid() != unsafe { libc::geteuid() } { anyhow::bail!( - "Daemon PID path {} is not a regular file owned by the current user; refusing to write it", + "Daemon {what} path {} is not a regular file owned by the current user; refusing to write it", path.display() ); } f.set_len(0) - .with_context(|| format!("Failed to truncate daemon PID file {}", path.display()))?; - f.write_all(std::process::id().to_string().as_bytes()) - .with_context(|| format!("Failed to write daemon PID file {}", path.display())) + .with_context(|| format!("Failed to truncate daemon {what} file {}", path.display()))?; + f.write_all(contents) + .with_context(|| format!("Failed to write daemon {what} file {}", path.display())) } /// Windows has no O_NOFOLLOW, and `%TEMP%` is already per-user, so the /// shared-/tmp squatting the Unix version defends against doesn't apply. #[cfg(not(unix))] +fn write_private_file_checked(path: &std::path::Path, contents: &[u8], what: &str) -> Result<()> { + std::fs::write(path, contents) + .with_context(|| format!("Failed to write daemon {what} file {}", path.display())) +} + +/// Write this process's PID to `path` (see [`write_private_file_checked`]). fn write_pid_file_checked(path: &std::path::Path) -> Result<()> { - std::fs::write(path, std::process::id().to_string()) - .with_context(|| format!("Failed to write daemon PID file {}", path.display())) + write_private_file_checked(path, std::process::id().to_string().as_bytes(), "PID") +} + +/// Publish the daemon's metadata sidecar, so `list-daemons` can name the +/// browser without inspecting process arguments. +/// +/// Best-effort by design: readers tolerate a missing info file, so a failure +/// here is reported to the daemon's (usually null) stderr rather than failing +/// startup — losing a display column must not cost a working daemon. +fn write_info_file(key: &str, browser: &str, ws_url: &str) { + let info = crate::protocol::DaemonInfo { + browser: browser.to_string(), + ws_url: ws_url.to_string(), + pid: std::process::id(), + started_unix: std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_secs()), + }; + let path = crate::protocol::info_path(key); + match serde_json::to_vec(&info) { + Ok(bytes) => { + if let Err(e) = write_private_file_checked(&path, &bytes, "info") { + eprintln!("daemon: could not write info file: {e:#}"); + } + } + Err(e) => eprintln!("daemon: could not serialize info file: {e}"), + } } /// Ceiling on the lock wait. Legitimate holders (a predecessor's cleanup, @@ -173,7 +204,7 @@ fn lock_wait_timeout() -> Duration { /// pre-acquired the predictable lock path park daemon startup forever. /// Async (poll + `tokio::time::sleep`) so a contended lock never blocks the /// runtime's worker thread. -async fn lock_daemon_files() -> Result { +pub(crate) async fn lock_daemon_files() -> Result { let f = open_lock_file()?; let timeout = lock_wait_timeout(); let deadline = tokio::time::Instant::now() + timeout; @@ -213,17 +244,32 @@ async fn lock_daemon_files() -> Result { /// place and self-heal on the next daemon start), so the messages exist for /// interactive debugging, not operational monitoring. fn cleanup() { + // Published by run_daemon before the guard is armed. Unset means nothing + // has been written yet, so there is nothing to remove. + let Some(key) = INSTANCE_KEY.get() else { + return; + }; #[cfg(unix)] - let endpoint = socket_path(); + let endpoint = socket_path(key); #[cfg(windows)] - let endpoint = addr_path(); - cleanup_at(&lock_path(), &pid_path(), &endpoint); + let endpoint = addr_path(key); + cleanup_at(&lock_path(), &pid_path(key), &endpoint, &info_path(key)); } +/// This daemon's instance key, so [`cleanup`] can derive its paths when it +/// runs from a `Drop` guard, a signal handler or the panic hook — none of +/// which can be passed an argument. +static INSTANCE_KEY: std::sync::OnceLock = std::sync::OnceLock::new(); + /// Path-parameterized body of [`cleanup`] (see its doc for the locking and /// ownership rules), so tests can drive it against a scratch directory /// instead of the real `temp_dir()` files. -fn cleanup_at(lock: &std::path::Path, pid: &std::path::Path, endpoint: &std::path::Path) { +fn cleanup_at( + lock: &std::path::Path, + pid: &std::path::Path, + endpoint: &std::path::Path, + info: &std::path::Path, +) { // `lock_file` holds the OS lock until it drops at the end of this scope, // covering the ownership check and removals below. let lock_file = match open_lock_file_at(lock) { @@ -266,6 +312,7 @@ fn cleanup_at(lock: &std::path::Path, pid: &std::path::Path, endpoint: &std::pat return; } let _ = std::fs::remove_file(endpoint); + let _ = std::fs::remove_file(info); let _ = std::fs::remove_file(pid); } @@ -288,7 +335,7 @@ impl Drop for CleanupGuard { /// - `$shutdown` is polled by `&mut` reference, so the caller must pin it /// first (`tokio::pin!`); passing an unpinned future fails to compile. macro_rules! run_accept_loop_body { - ($accept:expr, $client:expr, $ws_url:expr, $shutdown:expr) => { + ($accept:expr, $client:expr, $ws_url:expr, $browser:expr, $shutdown:expr) => { loop { tokio::select! { // Shutdown first, and `biased` so a ready signal always wins @@ -304,7 +351,7 @@ macro_rules! run_accept_loop_body { break; } accept = tokio::time::timeout(idle_timeout(), $accept) => match accept { - Ok(Ok((stream, _))) => match handle_connection(stream, $client, $ws_url).await { + Ok(Ok((stream, _))) => match handle_connection(stream, $client, $ws_url, $browser).await { ConnectionOutcome::Continue => {} ConnectionOutcome::Fatal => break, }, @@ -381,7 +428,12 @@ fn shutdown_signal() -> impl std::future::Future { } } -pub async fn run_daemon(ws_url: &str) -> Result<()> { +pub async fn run_daemon(ws_url: &str, browser: &str) -> Result<()> { + // Published before the guard is armed: this only sets an in-memory cell — + // nothing on disk yet — and cleanup() needs it to find the files at all. + let key = crate::protocol::instance_key(ws_url); + let key = INSTANCE_KEY.get_or_init(|| key).clone(); + // 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) @@ -401,12 +453,19 @@ pub async fn run_daemon(ws_url: &str) -> Result<()> { // predecessor can delete files this daemon just claimed. let startup_lock = lock_daemon_files().await?; - write_pid_file_checked(&pid_path())?; + // Order matters beyond this function: the PID file is written *before* the + // endpoint is bound, and both happen under `startup_lock`. `kill-daemon` + // relies on that pairing — holding the same lock, it can treat a live + // listener as proof that the PID it just read is this daemon's, because no + // daemon can have bound the socket without its PID already being on disk. + // Reordering these, or moving either outside the lock, breaks that. + write_pid_file_checked(&pid_path(&key))?; + write_info_file(&key, browser, ws_url); #[cfg(unix)] let listener = { // Clean up stale socket - let sock = socket_path(); + let sock = socket_path(&key); let _ = std::fs::remove_file(&sock); // Bind socket FIRST so the CLI knows the daemon is alive and can connect. @@ -418,13 +477,19 @@ pub async fn run_daemon(ws_url: &str) -> Result<()> { #[cfg(windows)] let listener = { // Clean up stale address file - let _ = std::fs::remove_file(addr_path()); + let _ = std::fs::remove_file(addr_path(&key)); // Bind listener FIRST so the CLI knows the daemon is alive and can connect. // If we wait for CdpClient::connect first, a Chrome/network permission prompt // can block the daemon and cause the CLI's 5-second wait_for_daemon timeout to expire. let listener = TcpListener::bind("127.0.0.1:0").await?; - std::fs::write(addr_path(), listener.local_addr()?.to_string())?; + let addr_file = addr_path(&key); + std::fs::write(&addr_file, listener.local_addr()?.to_string()).with_context(|| { + format!( + "Failed to write daemon address file {}", + addr_file.display() + ) + })?; listener }; @@ -436,7 +501,7 @@ pub async fn run_daemon(ws_url: &str) -> Result<()> { let mut client: Option = None; // Signal readiness by socket/address existence (it's already bound) - run_accept_loop_body!(listener.accept(), &mut client, ws_url, shutdown); + run_accept_loop_body!(listener.accept(), &mut client, ws_url, browser, shutdown); // File cleanup is handled by `_guard` (also covers signal/panic exits). @@ -451,6 +516,7 @@ async fn handle_connection( mut stream: S, client: &mut Option, ws_url: &str, + browser: &str, ) -> ConnectionOutcome where S: AsyncReadExt + AsyncWriteExt + Unpin, @@ -482,13 +548,17 @@ where // Connect lazily if client.is_none() { - match CdpClient::connect(ws_url).await { + let label = crate::browser::display_name(browser); + match CdpClient::connect(ws_url, &label).await { Ok(c) => *client = Some(c), Err(e) => { let resp = DaemonResponse { success: false, output: String::new(), - error: format!("Failed to connect to Chrome: {e:#}"), + // No prefix: CdpClient::connect's error already names the + // browser and the endpoint, so wrapping it here produced + // "Failed to connect to X: Failed to connect to X at ...". + error: format!("{e:#}"), navigated_to: None, error_code: Some(ErrorCode::ChromeConnection as u32), }; @@ -506,7 +576,10 @@ where None => DaemonResponse { success: false, output: String::new(), - error: String::from("Failed to connect to Chrome: client initialization failed"), + error: format!( + "Failed to connect to {}: client initialization failed", + crate::browser::display_name(browser) + ), navigated_to: None, error_code: Some(ErrorCode::ChromeConnection as u32), }, @@ -671,14 +744,17 @@ mod tests { let lock = dir.path().join("daemon.lock"); let pid = dir.path().join("daemon.pid"); let endpoint = dir.path().join("daemon.sock"); + let info = dir.path().join("daemon.info"); std::fs::write(&pid, std::process::id().wrapping_add(1).to_string()).unwrap(); std::fs::write(&endpoint, "").unwrap(); - cleanup_at(&lock, &pid, &endpoint); + std::fs::write(&info, "{}").unwrap(); + cleanup_at(&lock, &pid, &endpoint, &info); assert!(pid.exists(), "foreign PID file must survive cleanup"); assert!( endpoint.exists(), "foreign endpoint file must survive cleanup" ); + assert!(info.exists(), "foreign info file must survive cleanup"); } #[test] @@ -687,11 +763,14 @@ mod tests { let lock = dir.path().join("daemon.lock"); let pid = dir.path().join("daemon.pid"); let endpoint = dir.path().join("daemon.sock"); + let info = dir.path().join("daemon.info"); std::fs::write(&pid, std::process::id().to_string()).unwrap(); std::fs::write(&endpoint, "").unwrap(); - cleanup_at(&lock, &pid, &endpoint); + std::fs::write(&info, "{}").unwrap(); + cleanup_at(&lock, &pid, &endpoint, &info); assert!(!pid.exists()); assert!(!endpoint.exists()); + assert!(!info.exists(), "own info file must be removed"); assert!( lock.exists(), "the lock file is intentionally never removed" @@ -706,14 +785,17 @@ mod tests { let lock = dir.path().join("daemon.lock"); let pid = dir.path().join("daemon.pid"); let endpoint = dir.path().join("daemon.sock"); + let info = dir.path().join("daemon.info"); std::fs::write(&pid, std::process::id().to_string()).unwrap(); std::fs::write(&endpoint, "").unwrap(); + std::fs::write(&info, "{}").unwrap(); let holder = open_lock_file_at(&lock).unwrap(); holder.try_lock().unwrap(); // A replacement holds the lock (mid-startup): even our own files must // be left alone, since the replacement may be about to rebind them. - cleanup_at(&lock, &pid, &endpoint); + cleanup_at(&lock, &pid, &endpoint, &info); assert!(pid.exists()); assert!(endpoint.exists()); + assert!(info.exists()); } } diff --git a/src/lib.rs b/src/lib.rs index 10d63cd..3dc3078 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,11 +28,15 @@ pub struct Cli { #[arg(long, global = true, env = "CHROME_WS_ENDPOINT")] pub ws_endpoint: Option, - /// Chrome user data directory (for auto-connect) + /// Browser user data directory (for auto-connect) #[arg(long, global = true, env = "CHROME_USER_DATA_DIR")] pub user_data_dir: Option, - /// Chrome channel: stable, beta, canary, dev + /// Browser to auto-connect to: chrome, edge + #[arg(long, global = true, default_value = "chrome", env = "CHROME_BROWSER")] + pub browser: String, + + /// Browser release channel: stable, beta, canary, dev #[arg(long, global = true, default_value = "stable", env = "CHROME_CHANNEL")] pub channel: String, @@ -408,17 +412,29 @@ pub enum Commands { track_navigation: bool, }, + /// List the running daemons and the browser each is attached to + #[command(name = "list-daemons")] + ListDaemons, + /// Stop the background daemon process #[command(name = "kill-daemon")] KillDaemon { /// Skip the confirmation/refusal guard and kill unconditionally. /// - /// Killing the daemon drops any already-approved Chrome remote-debugging - /// connection; reconnecting requires the human to re-approve Chrome's - /// consent dialog. This does NOT fix "Failed to connect to Chrome" - /// errors — do not use it as a retry step. + /// Killing the daemon drops any already-approved remote-debugging + /// connection; reconnecting requires the human to re-approve the + /// browser's consent dialog. This does NOT fix "Failed to connect to + /// " errors — do not use it as a retry step. #[arg(long)] force: bool, + + /// Stop every daemon for this user, whatever browser each is attached + /// to, instead of only the one for the resolved target. + /// + /// Needs no reachable browser, so this is the way to clear daemons + /// orphaned by a browser that has already exited. + #[arg(long)] + all: bool, }, } @@ -466,8 +482,346 @@ impl Cli { Commands::RunScript { .. } => "run-script", Commands::Adapter { .. } => "adapter", Commands::KillDaemon { .. } => "kill-daemon", + Commands::ListDaemons => "list-daemons", + } + } +} + +/// Whether a PID names a live process. +/// +/// `kill(pid, 0)` delivers no signal but performs the same existence and +/// permission checks: `EPERM` means the process exists and is someone else's, +/// which for a daemon list is still "running". +#[cfg(unix)] +fn daemon_pid_alive(pid: i32) -> bool { + // SAFETY: kill() has no memory-safety preconditions, and signal 0 is the + // no-op existence probe. The pid is validated positive by the caller, so + // it cannot address a process group. + let ret = unsafe { libc::kill(pid as libc::pid_t, 0) }; + ret == 0 || std::io::Error::last_os_error().raw_os_error() == Some(libc::EPERM) +} + +/// Whether a daemon is actually listening on `sock_path`. +/// +/// Used to confirm a PID file still describes a live daemon before signalling +/// it. Connecting is the check: only the daemon binds this path, so a successful +/// connect proves one is alive, while `ECONNREFUSED` proves the opposite — a +/// socket file left behind by a daemon that died without cleaning up. +/// +/// The probe sends nothing and drops the connection immediately; the daemon +/// treats that as a read error and continues serving. A daemon wedged inside a +/// CDP call still passes, because the kernel completes the connect from the +/// listen backlog without the daemon having to accept it. +/// +/// Any other error (a missing socket file, a permission problem) counts as "not +/// verified" and therefore as not alive: leaving an orphan that exits on its own +/// idle timeout is a better failure than signalling an unrelated process. +#[cfg(unix)] +fn daemon_listening_at(sock_path: &std::path::Path) -> bool { + std::os::unix::net::UnixStream::connect(sock_path).is_ok() +} + +/// Human-readable uptime, coarsened to the largest two units that matter. +fn format_uptime(secs: u64) -> String { + match secs { + s if s < 60 => format!("{s}s"), + s if s < 3600 => format!("{}m{}s", s / 60, s % 60), + s => format!("{}h{}m", s / 3600, (s % 3600) / 60), + } +} + +/// Endpoint as `host:port`, dropping the browser-GUID path that makes the full +/// URL too wide to tabulate. +fn short_endpoint(ws_url: &str) -> String { + // Auto-connect only ever builds ws://, but --ws-endpoint is taken verbatim + // and may be wss://. Strip the longer scheme first: "ws://" is not a prefix + // of "wss://", but checking in the other order would still be a trap for + // anyone adding schemes later. + ws_url + .strip_prefix("wss://") + .or_else(|| ws_url.strip_prefix("ws://")) + .unwrap_or(ws_url) + .split('/') + .next() + .unwrap_or(ws_url) + .to_string() +} + +/// Print one row per daemon: which browser it is attached to, where, and for +/// how long. +/// +/// Reads only on-disk daemon state, so it works when every browser is gone. +/// A daemon whose info sidecar is missing still lists — with `?` columns — +/// because knowing a daemon holds a connection matters more than labeling it. +fn print_daemon_list(format: format::OutputFormat) -> Result<()> { + #[derive(serde::Serialize)] + struct Row { + pid: Option, + browser: String, + endpoint: String, + uptime_secs: Option, + running: Option, + key: String, + } + + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_secs()); + + let mut rows = Vec::new(); + for key in protocol::enumerate_instance_keys() { + let info: Option = std::fs::read(protocol::info_path(&key)) + .ok() + .and_then(|b| serde_json::from_slice(&b).ok()); + + // Same validation kill-daemon applies before signalling: a PID file + // holding 0, or a value outside pid_t, is not a PID we will act on. + // It matters for a read-only listing too — `kill(0, 0)` probes the + // caller's own process group and succeeds, so a stray 0 would other- + // wise be reported as a running daemon. + #[cfg(unix)] + let (pid, running) = { + let contents = read_pid_file_checked(&protocol::pid_path(&key)).ok(); + let pid = contents.as_deref().and_then(parse_pid_file_contents); + ( + pid.and_then(|p| u32::try_from(p).ok()), + pid.map(daemon_pid_alive), + ) + }; + // No cheap liveness probe without libc::kill, so liveness is unknown + // and the state column stays "?" rather than claiming stale/running. + #[cfg(not(unix))] + let (pid, running) = ( + std::fs::read_to_string(protocol::pid_path(&key)) + .ok() + .and_then(|s| s.trim().parse::().ok()), + None::, + ); + + rows.push(Row { + pid, + browser: info + .as_ref() + .map_or_else(|| "?".to_string(), |i| i.browser.clone()), + endpoint: info + .as_ref() + .map_or_else(|| "?".to_string(), |i| short_endpoint(&i.ws_url)), + uptime_secs: info + .as_ref() + .filter(|i| i.started_unix > 0 && now >= i.started_unix) + .map(|i| now - i.started_unix), + running, + key, + }); + } + + // Both structured formats go through the shared encoder, so --toon gets + // TOON instead of silently falling through to the text table. The empty + // list is emitted structurally too: `[]` is the answer, not the + // "No daemons running." sentence, which belongs to text output only. + if !format.is_text() { + let value = serde_json::to_value(&rows)?; + println!("{}", format::format_structured(&value, format)?); + return Ok(()); + } + + if rows.is_empty() { + println!("No daemons running."); + return Ok(()); + } + + println!("PID BROWSER ENDPOINT UPTIME STATE"); + println!("{}", "-".repeat(58)); + for r in &rows { + let pid = r.pid.map_or_else(|| "?".to_string(), |p| p.to_string()); + let uptime = r.uptime_secs.map_or_else(|| "?".to_string(), format_uptime); + let state = match r.running { + Some(true) => "running", + Some(false) => "stale", + None => "?", + }; + println!( + "{:<7} {:<8} {:<21} {:<9} {}", + pid, r.browser, r.endpoint, uptime, state + ); + } + if rows.iter().any(|r| r.running == Some(false)) { + println!("\nstale = PID file with no live process; `kill-daemon --all` clears them."); + } + Ok(()) +} + +/// Stop the daemon instance identified by `key`, removing its socket, info and +/// PID files. +/// +/// Split out of `kill-daemon` so the same ownership-checked, signal-once logic +/// serves both the target-scoped kill and the `--all` sweep. Prints what it did +/// (or that there was nothing to do) and returns an error only when the daemon +/// may still be running — the caller decides whether one failure aborts a +/// whole sweep. +async fn stop_daemon_instance(key: &str) -> Result<()> { + stop_daemon_at( + &protocol::pid_path(key), + &protocol::info_path(key), + #[cfg(unix)] + &protocol::socket_path(key), + ) + .await +} + +/// Stop the pre-instance-key daemon (`chrome-devtools-daemon-.pid`) left +/// by a version that ran one daemon per user. +/// +/// Swept by `--all` only: it is attached to some browser we can no longer +/// identify, so no scoped target can claim it, and after an upgrade nothing +/// else will ever reach it. Silent when the files don't exist, which is the +/// common case. +async fn stop_legacy_unkeyed_daemon() -> Result { + let pid_path = protocol::legacy_unkeyed_pid_path(); + if !pid_path.exists() { + return Ok(false); + } + // No info sidecar existed in that layout; point at a path that is + // guaranteed absent so the removal is a no-op. + let info_path = pid_path.with_extension("info"); + stop_daemon_at( + &pid_path, + &info_path, + #[cfg(unix)] + &protocol::legacy_unkeyed_socket_path(), + ) + .await + .map(|()| true) +} + +/// Signal one daemon and remove its files, given their paths. +/// +/// The ownership-checked read and single SIGTERM live here so the keyed and +/// legacy layouts cannot drift apart in how carefully they treat a predictable +/// path in shared `/tmp`. +async fn stop_daemon_at( + pid_path: &std::path::Path, + info_path: &std::path::Path, + #[cfg(unix)] sock_path: &std::path::Path, +) -> Result<()> { + // Held across the whole read -> probe -> signal sequence, and it must be + // the same lock the daemon takes to write its PID file and bind its socket. + // + // Without it the liveness probe below proves only that *some* daemon holds + // the key, not that the PID already in hand is that daemon's: a daemon can + // start between the read and the probe, so the read could return a stale + // PID the OS has since recycled while the probe sees the newcomer's + // listener — and the signal would land on an unrelated process. Holding the + // lock makes the two observations one atomic step. + // + // This cannot be blocked by a daemon wedged inside a CDP call: the daemon + // drops the startup lock before entering its accept loop, so only a + // concurrent startup or cleanup contends, and both are brief. + #[cfg(unix)] + let _startup_lock = { + use anyhow::Context as _; + daemon::lock_daemon_files().await.context( + "Could not take the daemon startup lock, so a PID could not be safely matched to \ + its daemon; nothing was signalled. Retry — a concurrent daemon start or shutdown \ + holds this lock only briefly.", + )? + }; + + // Ownership-checked read: the path is predictable in shared /tmp, so + // never act on a PID file that was planted there by another user. + #[cfg(unix)] + let read_result = read_pid_file_checked(pid_path); + #[cfg(not(unix))] + let read_result = { + use anyhow::Context as _; + std::fs::read_to_string(pid_path) + .with_context(|| format!("Failed to read PID file {}", pid_path.display())) + }; + match read_result { + Ok(pid_str) => { + #[cfg(unix)] + { + // parse_pid_file_contents enforces the safety rules for + // what may be passed to kill() (no pid 0, no pid_t + // overflow — see its doc comment). + let pid = parse_pid_file_contents(&pid_str).ok_or_else(|| { + anyhow::anyhow!( + "PID {:?} in {} is not a positive integer that fits libc::pid_t; refusing to signal", + pid_str.trim(), + pid_path.display() + ) + })?; + // Do not signal a PID we cannot tie back to a live daemon. + // A daemon killed with SIGKILL leaves its PID file behind + // (cleanup is skipped by design), the OS is free to reuse that + // PID for an unrelated process of this same user, and SIGTERM + // would then land on that process instead. + // + // Only the daemon binds this socket, and it binds it *after* + // writing its PID file, both under the startup lock this + // function holds. So no daemon can have appeared since the read + // above, and a live listener is proof that the PID just read is + // that listener's. + if !daemon_listening_at(sock_path) { + let _ = std::fs::remove_file(sock_path); + let _ = std::fs::remove_file(info_path); + let _ = std::fs::remove_file(pid_path); + println!( + "Daemon (PID {pid}) is not listening on {}; cleaned up its files \ + without signalling, since that PID may now belong to another process.", + sock_path.display() + ); + return Ok(()); + } + // Signal the process directly via libc to avoid shelling out + // to /usr/bin/kill. A return of 0 means the signal was + // delivered; -1 with errno ESRCH means the process is gone + // (and the PID file was stale). + // SAFETY: kill() has no memory-safety preconditions; the + // pid was validated positive and in pid_t range above, so + // it cannot alias a process group. + let ret = unsafe { libc::kill(pid as libc::pid_t, libc::SIGTERM) }; + if ret == 0 { + // Signal delivered — daemon is shutting down; clean up. + let _ = std::fs::remove_file(sock_path); + let _ = std::fs::remove_file(info_path); + let _ = std::fs::remove_file(pid_path); + println!("Daemon (PID {pid}) stopped."); + } else { + let err = std::io::Error::last_os_error(); + if err.raw_os_error() == Some(libc::ESRCH) { + // Process is gone — the PID file was stale; clean up. + let _ = std::fs::remove_file(sock_path); + let _ = std::fs::remove_file(info_path); + let _ = std::fs::remove_file(pid_path); + println!("Daemon (PID {pid}) was not running. Cleaned up stale files."); + } else { + // e.g. EPERM: the daemon may still be running. Leave the + // socket/PID files so it stays reachable. + return Err(anyhow::anyhow!( + "Failed to signal daemon (PID {pid}): {err}. Left socket/PID files in place." + )); + } + } + } + #[cfg(not(unix))] + { + let _ = pid_str; + println!("kill-daemon is only supported on Unix systems."); + } + } + Err(e) if pid_file_missing(&e) => { + println!("No daemon running (PID file not found)."); + } + Err(e) => { + // Surface via the standard error path (uniform formatting, + // telemetry flush, typed exit code) — matching the EPERM + // signal-failure case above rather than exiting directly. The + // error already names the failing operation and the path. + return Err(e); } } + Ok(()) } /// What to do when `kill-daemon` is invoked, given `--force` and whether @@ -939,6 +1293,9 @@ fn build_request(cli: &Cli) -> Result { }), ), Commands::KillDaemon { .. } => unreachable!("KillDaemon is handled before build_request"), + Commands::ListDaemons => { + unreachable!("ListDaemons is handled before build_request") + } Commands::InspectHeapSnapshotNode { .. } => { unreachable!("InspectHeapSnapshotNode is handled before build_request") } @@ -1031,12 +1388,21 @@ pub async fn run() -> Result<()> { } }; + // Pure local state: reads daemon files and never connects to a browser, so + // it must run before endpoint resolution — which would fail in exactly the + // situation where you most want the list (a browser that has exited, + // leaving a daemon behind). + if matches!(cli.command, Commands::ListDaemons) { + print_daemon_list(cli.output_format())?; + return Ok(()); + } + // Handle kill-daemon without connecting to Chrome. Match by reference // like the other `cli.command` intercepts below: binding only the Copy // `force` field happens to avoid a partial move today, but a by-ref match // keeps that from silently breaking if the variant ever gains a non-Copy // field. - if let Commands::KillDaemon { force } = &cli.command { + if let Commands::KillDaemon { force, all } = &cli.command { use std::io::IsTerminal; // A prompt is only useful if the user can both type (stdin) and see it // (stderr). If stderr is not a TTY (e.g. `2>file`) while stdin still is, @@ -1070,78 +1436,59 @@ pub async fn run() -> Result<()> { KillDaemonDecision::Proceed => {} } - let pid_path = protocol::pid_path(); - #[cfg(unix)] - let sock_path = protocol::socket_path(); - // Ownership-checked read: the path is predictable in shared /tmp, so - // never act on a PID file that was planted there by another user. - #[cfg(unix)] - let read_result = read_pid_file_checked(&pid_path); - #[cfg(not(unix))] - let read_result = { - use anyhow::Context as _; - std::fs::read_to_string(&pid_path) - .with_context(|| format!("Failed to read PID file {}", pid_path.display())) - }; - match read_result { - Ok(pid_str) => { - #[cfg(unix)] - { - // parse_pid_file_contents enforces the safety rules for - // what may be passed to kill() (no pid 0, no pid_t - // overflow — see its doc comment). - let pid = parse_pid_file_contents(&pid_str).ok_or_else(|| { - anyhow::anyhow!( - "PID {:?} in {} is not a positive integer that fits libc::pid_t; refusing to signal", - pid_str.trim(), - pid_path.display() - ) - })?; - // Signal the process directly via libc to avoid shelling out - // to /usr/bin/kill. A return of 0 means the signal was - // delivered; -1 with errno ESRCH means the process is gone - // (and the PID file was stale). - // SAFETY: kill() has no memory-safety preconditions; the - // pid was validated positive and in pid_t range above, so - // it cannot alias a process group. - let ret = unsafe { libc::kill(pid as libc::pid_t, libc::SIGTERM) }; - if ret == 0 { - // Signal delivered — daemon is shutting down; clean up. - let _ = std::fs::remove_file(&sock_path); - let _ = std::fs::remove_file(&pid_path); - println!("Daemon (PID {pid}) stopped."); - } else { - let err = std::io::Error::last_os_error(); - if err.raw_os_error() == Some(libc::ESRCH) { - // Process is gone — the PID file was stale; clean up. - let _ = std::fs::remove_file(&sock_path); - let _ = std::fs::remove_file(&pid_path); - println!("Daemon (PID {pid}) was not running. Cleaned up stale files."); - } else { - // e.g. EPERM: the daemon may still be running. Leave the - // socket/PID files so it stays reachable. - return Err(anyhow::anyhow!( - "Failed to signal daemon (PID {pid}): {err}. Left socket/PID files in place." - )); - } - } - } - #[cfg(not(unix))] - { - let _ = pid_str; - println!("kill-daemon is only supported on Unix systems."); + if *all { + // Clean slate: every daemon this user owns, whatever browser it is + // attached to. Keep going after a failure so one unkillable daemon + // cannot strand the rest. + let keys = protocol::enumerate_instance_keys(); + if keys.is_empty() { + println!("No daemons running."); + } + let mut failures = 0usize; + let mut attempted = keys.len(); + for key in &keys { + if let Err(e) = stop_daemon_instance(key).await { + failures += 1; + eprintln!("{e:#}"); } } - Err(e) if pid_file_missing(&e) => { - println!("No daemon running (PID file not found)."); + // The legacy daemon is another stop attempt, so it counts toward + // the total — otherwise a legacy-only failure reports "1 of 0". + // It counts only when one was actually there, so the total does + // not inflate on the common no-legacy-files path. + match stop_legacy_unkeyed_daemon().await { + Ok(found) => attempted += usize::from(found), + Err(e) => { + attempted += 1; + failures += 1; + eprintln!("{e:#}"); + } } - Err(e) => { - // Surface via the standard error path (uniform formatting, - // telemetry flush, typed exit code) — matching the EPERM - // signal-failure case above rather than exiting directly. The - // error already names the failing operation and the path. - return Err(e); + if failures > 0 { + return Err(anyhow::anyhow!( + "{failures} of {attempted} daemon(s) could not be stopped" + )); } + } else { + // Target-scoped: resolve the same endpoint a real command would, + // so `--browser edge kill-daemon` can only ever stop the Edge + // daemon. Resolution needs the browser to be reachable, which a + // dead browser with an orphaned daemon is not — hence the pointer + // to --all, which needs no endpoint. + let ws_url = browser::resolve_ws_url( + cli.ws_endpoint.as_deref(), + cli.user_data_dir.as_deref(), + &cli.browser, + &cli.channel, + ) + .map_err(|e| { + anyhow::anyhow!( + "{e:#}\n\nkill-daemon targets the daemon for a specific browser, so it \ + needs to resolve that browser's endpoint. If the browser is already gone \ + and you just want to stop leftover daemons, use --all." + ) + })?; + stop_daemon_instance(&protocol::instance_key(&ws_url)).await?; } // Best-effort sweep of the pre-uid-suffix file names: a daemon @@ -1278,25 +1625,33 @@ pub async fn run() -> Result<()> { let ws_url = browser::resolve_ws_url( cli.ws_endpoint.as_deref(), cli.user_data_dir.as_deref(), + &cli.browser, &cli.channel, )?; let request = build_request(&cli)?; + // One daemon per endpoint: a daemon serves only the browser session it was + // spawned for, so this command can never be answered by a daemon attached + // to a different browser. + let key = protocol::instance_key(&ws_url); + // Try daemon first - if let Ok(resp) = client::send_to_daemon(&request).await { + if let Ok(resp) = client::send_to_daemon(&key, &request).await { print_response(&resp); return Ok(()); } // Daemon not running — spawn it - client::spawn_daemon(&ws_url)?; - if let Err(e) = client::wait_for_daemon().await { + // Canonical spelling, so `--browser EDGE` and `--browser msedge` both label + // the daemon `edge` in list-daemons. + client::spawn_daemon(&ws_url, &browser::canonical_name(&cli.browser))?; + if let Err(e) = client::wait_for_daemon(&key).await { return run_direct_fallback(&cli, &ws_url, &e).await; } // Retry via daemon - match client::send_to_daemon(&request).await { + match client::send_to_daemon(&key, &request).await { Ok(resp) => { print_response(&resp); Ok(()) @@ -1332,7 +1687,7 @@ async fn run_direct_fallback(cli: &Cli, ws_url: &str, error: &anyhow::Error) -> /// Direct execution without daemon (fallback). async fn run_direct(cli: &Cli, ws_url: &str) -> Result { - let mut client = cdp::CdpClient::connect(ws_url).await?; + let mut client = cdp::CdpClient::connect(ws_url, &browser::display_name(&cli.browser)).await?; let is_browser = cli.is_browser_level(); @@ -1809,6 +2164,111 @@ mod tests { assert!(!obj.get("bool_false").unwrap().as_bool().unwrap()); } + #[test] + fn test_format_uptime_units() { + assert_eq!(format_uptime(0), "0s"); + assert_eq!(format_uptime(8), "8s"); + assert_eq!(format_uptime(72), "1m12s"); + assert_eq!(format_uptime(3600), "1h0m"); + assert_eq!(format_uptime(7500), "2h5m"); + } + + #[test] + fn test_short_endpoint_drops_guid_path() { + assert_eq!( + short_endpoint("ws://127.0.0.1:56912/devtools/browser/1fff9d85"), + "127.0.0.1:56912" + ); + // Degrades to the input rather than empty when the shape is unexpected. + assert_eq!(short_endpoint("127.0.0.1:9222"), "127.0.0.1:9222"); + assert_eq!(short_endpoint(""), ""); + } + + /// --ws-endpoint is taken verbatim, so a wss:// URL reaches this function + /// and must not render as the bare scheme "wss:". + #[test] + fn test_short_endpoint_handles_secure_scheme() { + assert_eq!( + short_endpoint("wss://example.test:9222/devtools/browser/abc"), + "example.test:9222" + ); + assert_eq!( + short_endpoint("wss://example.test:9222"), + "example.test:9222" + ); + } + + /// `kill-daemon` must not signal while it cannot hold the startup lock: + /// without it, the PID it read and the listener it probed can belong to + /// different daemons. `flock` is per open-file-description, so a second + /// handle in this process contends exactly as a starting daemon would. + /// + /// Takes the real lock file, briefly, because the path is not injectable; + /// no other test contends for it, and nothing here is ever signalled — the + /// lock is unavailable, so the function returns before reaching `kill`. + #[cfg(unix)] + #[tokio::test] + async fn test_stop_daemon_at_will_not_signal_without_the_startup_lock() { + let dir = tempfile::tempdir().unwrap(); + let pid_path = dir.path().join("d.pid"); + let info_path = dir.path().join("d.info"); + let sock_path = dir.path().join("d.sock"); + std::fs::write(&pid_path, std::process::id().to_string()).unwrap(); + std::fs::write(&info_path, "{}").unwrap(); + + let holder = daemon::open_lock_file().unwrap(); + holder.try_lock().unwrap(); + + let err = stop_daemon_at(&pid_path, &info_path, &sock_path) + .await + .expect_err("must refuse while the startup lock is held elsewhere"); + let msg = err.to_string(); + assert!(msg.contains("startup lock"), "{msg}"); + assert!(msg.contains("nothing was signalled"), "{msg}"); + + // Failing closed also means leaving state alone for the retry. + assert!(pid_path.exists(), "PID file must survive a refused stop"); + assert!(info_path.exists(), "info file must survive a refused stop"); + } + + /// The guard that keeps SIGTERM off a PID the daemon no longer owns. + #[cfg(unix)] + #[test] + fn test_daemon_listening_at_requires_a_live_listener() { + let dir = tempfile::tempdir().unwrap(); + + // Nothing bound: a bare path, and a plain file where a socket should be. + assert!(!daemon_listening_at(&dir.path().join("absent.sock"))); + let regular = dir.path().join("regular.sock"); + std::fs::write(®ular, "").unwrap(); + assert!(!daemon_listening_at(®ular)); + + // A real listener at the path is what proves a daemon is alive. + let live = dir.path().join("live.sock"); + let listener = std::os::unix::net::UnixListener::bind(&live).unwrap(); + assert!(daemon_listening_at(&live)); + + // Once it goes away the socket file remains but connects are refused — + // exactly the SIGKILLed-daemon case that must not be signalled. + drop(listener); + assert!(live.exists(), "socket file outlives the listener"); + assert!(!daemon_listening_at(&live)); + } + + /// A live process must read as running; PID 0 is never probed because + /// `kill(0, …)` addresses the caller's own process group. + #[cfg(unix)] + #[test] + fn test_daemon_pid_alive_detects_self_and_absence() { + let me = i32::try_from(std::process::id()).unwrap(); + assert!(daemon_pid_alive(me), "this process is alive"); + // PID 1 exists on every Unix; a very high PID almost certainly does not. + assert!( + !daemon_pid_alive(0x7FFF_FFFE), + "implausible PID is not alive" + ); + } + #[test] fn test_kill_daemon_decision_force_always_proceeds() { assert_eq!( diff --git a/src/main.rs b/src/main.rs index 386ec4b..697674a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,7 +13,10 @@ async fn main() { let args: Vec = std::env::args().collect(); if args.get(1).map(|s| s.as_str()) == Some("__daemon__") { let ws_url = args.get(2).expect("daemon requires ws_url argument"); - let result = daemon::run_daemon(ws_url).await; + // Descriptive only, for `list-daemons`. Older spawners passed no + // browser argument, so default rather than panic. + let browser = args.get(3).map(String::as_str).unwrap_or("chrome"); + let result = daemon::run_daemon(ws_url, browser).await; telemetry::shutdown_logger(); if let Err(e) = result { eprintln!("daemon error: {e:#}"); diff --git a/src/protocol.rs b/src/protocol.rs index 9bf4a6e..ab8eab7 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -70,29 +70,116 @@ fn user_suffix() -> std::borrow::Cow<'static, str> { std::borrow::Cow::Borrowed("") } -/// Path to the Unix domain socket for daemon communication. +/// Identity of a single daemon instance: a short hash of the resolved +/// WebSocket endpoint. +/// +/// A daemon owns exactly one live CDP connection, so its identity *is* that +/// connection. Keying the socket/PID/info filenames by endpoint gives Chrome, +/// Edge, every release channel and every headless instance a daemon of its +/// own, which is what stops a command aimed at one browser from being served +/// by a daemon attached to another — the failure mode when a single per-user +/// daemon bound to whichever browser happened to be resolved first, and every +/// later `--browser`/`--user-data-dir` flag was silently ignored. +/// +/// The endpoint's browser GUID changes on every browser launch, so a restarted +/// browser deliberately gets a fresh daemon instead of inheriting a dead +/// connection. The orphan holds no lock and exits on its own idle timeout. +pub fn instance_key(ws_url: &str) -> String { + // FNV-1a, 64-bit. Not cryptographic, and does not need to be: the input is + // a loopback URL this process just derived, not attacker-chosen, and a + // collision would only mean two browser sessions sharing one daemon — + // exactly the behavior that predates keying. + let mut hash: u64 = 0xcbf2_9ce4_8422_2325; + for byte in ws_url.as_bytes() { + hash ^= u64::from(*byte); + hash = hash.wrapping_mul(0x0000_0100_0000_01b3); + } + format!("{hash:016x}") +} + +/// Shared leading part of every per-user daemon filename. +fn daemon_file_prefix() -> String { + format!("chrome-devtools-daemon{}", user_suffix()) +} + +/// Unix domain socket a daemon listens on. The key keeps concurrent daemons +/// from binding the same path, which is what lets one browser's daemon exist +/// alongside another's. #[cfg(unix)] -pub fn socket_path() -> PathBuf { - std::env::temp_dir().join(format!("chrome-devtools-daemon{}.sock", user_suffix())) +pub fn socket_path(key: &str) -> PathBuf { + std::env::temp_dir().join(format!("{}-{key}.sock", daemon_file_prefix())) } -/// Path to the named-pipe address file for daemon communication (Windows). +/// File recording the loopback TCP address a daemon listens on (Windows has no +/// Unix sockets, so the address is published rather than the endpoint itself). +/// Keyed for the same reason as [`socket_path`]. #[cfg(windows)] -pub fn addr_path() -> PathBuf { - std::env::temp_dir().join(format!("chrome-devtools-daemon{}.addr", user_suffix())) +pub fn addr_path(key: &str) -> PathBuf { + std::env::temp_dir().join(format!("{}-{key}.addr", daemon_file_prefix())) +} + +/// PID file for one daemon. Keyed so a sweep can tell the instances apart and +/// signal only the one it means to — an unkeyed name would make every daemon +/// look like the same process to `kill-daemon`. +pub fn pid_path(key: &str) -> PathBuf { + std::env::temp_dir().join(format!("{}-{key}.pid", daemon_file_prefix())) } -/// Path to the daemon PID file. -pub fn pid_path() -> PathBuf { - std::env::temp_dir().join(format!("chrome-devtools-daemon{}.pid", user_suffix())) +/// Metadata sidecar for one daemon (see [`DaemonInfo`]), keyed alongside its +/// PID and socket so the three always describe the same instance. +pub fn info_path(key: &str) -> PathBuf { + std::env::temp_dir().join(format!("{}-{key}.info", daemon_file_prefix())) } /// 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. +/// Deliberately *not* keyed per instance. It only serializes the brief +/// write-pid-then-bind critical section, so one lock for all instances costs +/// nothing measurable — while a per-instance lock would accumulate a file per +/// browser session forever, since 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())) + std::env::temp_dir().join(format!("{}.lock", daemon_file_prefix())) +} + +/// Every daemon instance key with a PID file belonging to this user. +/// +/// Used by `list-daemons` and `kill-daemon --all`. Unreadable temp dirs and +/// non-UTF-8 names are skipped rather than reported: a name we cannot parse is +/// not a daemon of ours. +pub fn enumerate_instance_keys() -> Vec { + let prefix = format!("{}-", daemon_file_prefix()); + let Ok(entries) = std::fs::read_dir(std::env::temp_dir()) else { + return Vec::new(); + }; + let mut keys: Vec = entries + .flatten() + .filter_map(|e| { + let name = e.file_name(); + let name = name.to_str()?; + let key = name.strip_prefix(&prefix)?.strip_suffix(".pid")?; + (!key.is_empty()).then(|| key.to_string()) + }) + .collect(); + keys.sort(); + keys +} + +/// Metadata a daemon publishes about itself, so `list-daemons` can name the +/// browser it is attached to without inspecting process arguments. +/// +/// Best-effort and purely descriptive: the daemon works if this file is +/// missing, and readers must tolerate its absence. +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct DaemonInfo { + /// Browser the daemon was spawned for, as passed to `--browser`. + pub browser: String, + /// The resolved endpoint this daemon is attached to. + pub ws_url: String, + pub pid: u32, + /// Unix seconds at daemon start, for the uptime column. + pub started_unix: u64, } /// Pre-uid-suffix PID file name, so `kill-daemon` can still stop a daemon @@ -102,6 +189,19 @@ pub fn legacy_pid_path() -> PathBuf { std::env::temp_dir().join("chrome-devtools-daemon.pid") } +/// Pre-instance-key PID file name: `chrome-devtools-daemon-.pid`, written +/// by versions that ran one daemon per user. Swept by `kill-daemon --all` so an +/// upgrade doesn't leave an unreachable daemon holding a CDP connection. +pub fn legacy_unkeyed_pid_path() -> PathBuf { + std::env::temp_dir().join(format!("{}.pid", daemon_file_prefix())) +} + +/// Pre-instance-key socket name (see [`legacy_unkeyed_pid_path`]). +#[cfg(unix)] +pub fn legacy_unkeyed_socket_path() -> PathBuf { + std::env::temp_dir().join(format!("{}.sock", daemon_file_prefix())) +} + /// Pre-uid-suffix socket name (see [`legacy_pid_path`]). #[cfg(unix)] pub fn legacy_socket_path() -> PathBuf { @@ -130,3 +230,71 @@ pub async fn read_msg(r: &mut R) -> anyhow::Result