fix(sandbox): try Docker socket before CLI binary check (#2467)

* fix(sandbox): try Docker socket before CLI binary check

The sandbox detection checked `which docker` first and returned
NotInstalled if the CLI binary was absent — even when the Docker
daemon was reachable via a bind-mounted socket. This broke
container-in-container deployments (e.g., Nomad shards with
/var/run/docker.sock mounted) where bollard can talk to the daemon
but no CLI is installed in the slim image.

Reorder check_docker() to try connect_docker() (bollard socket ping)
first. If the daemon responds, return Available immediately. The CLI
check is now only used as a fallback for error-message quality when
the socket connection fails.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(sandbox): skip slow daemon ping when Docker is clearly absent

check_docker() called connect_docker() before checking whether Docker
was even present, causing a 120s bollard timeout on hosts with an
unreachable DOCKER_HOST and no Docker installation. Add a fast-path
that checks for the docker binary, DOCKER_HOST env var, and socket
files on disk before attempting the daemon ping. This preserves DinD
support (bind-mounted socket, no CLI binary) while avoiding the
latency regression for non-Docker hosts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(sandbox): add regression tests for check_docker fast-path

Extract should_skip_daemon_ping() predicate from check_docker() and
add unit tests covering all combinations: skip when no binary, no
DOCKER_HOST, and no socket (the bug scenario); no skip when any of
the three signals is present (DinD socket, DOCKER_HOST, CLI binary).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: ymcrcat

src/sandbox/container.rs

@@ -683,7 +683,7 @@ pub async fn connect_docker() -> Result<Docker> {
 }
 
 #[cfg(unix)]
-fn unix_socket_candidates() -> Vec<PathBuf> {
+pub(crate) fn unix_socket_candidates() -> Vec<PathBuf> {
     unix_socket_candidates_from_env(
         std::env::var_os("HOME").map(PathBuf::from),
         std::env::var_os("XDG_RUNTIME_DIR").map(PathBuf::from),

src/sandbox/detect.rs

@@ -1,8 +1,13 @@
 //! Proactive Docker detection with platform-specific guidance.
 //!
-//! Checks whether Docker is both installed (binary on PATH) and running
-//! (daemon responding to ping), and provides platform-appropriate
-//! installation or startup instructions when it is not.
+//! First performs cheap filesystem checks (socket existence, `DOCKER_HOST`,
+//! PATH lookup) to fast-exit on hosts where Docker is clearly absent —
+//! avoiding bollard's 120 s daemon-ping timeout.  Then attempts a direct
+//! socket connection via bollard (covers container-in-container deployments
+//! where the socket is bind-mounted but the CLI is absent), and falls back
+//! to a `which docker` PATH check for error-message quality.  Provides
+//! platform-appropriate installation or startup instructions when Docker
+//! is not available.
 //!
 //! # Detection Limitations
 //!
@@ -102,28 +107,48 @@ pub struct DockerDetection {
 
 /// Check whether Docker is installed and running.
 ///
-/// 1. Checks if `docker` binary exists on PATH
-/// 2. If found, tries to connect and ping the Docker daemon via `connect_docker()`
-/// 3. Returns `Available`, `NotInstalled`, or `NotRunning`
+/// 1. Fast path: if no Docker socket exists on the filesystem, `DOCKER_HOST`
+///    is unset, *and* the `docker` CLI binary is absent, returns `NotInstalled`
+///    immediately — without a daemon ping.  This avoids a 120 s bollard timeout
+///    on hosts where Docker is clearly not present (see #P2).
+/// 2. Tries to connect and ping the Docker daemon directly via
+///    `connect_docker()` (bollard). This covers container-in-container (DinD)
+///    deployments where the socket is bind-mounted but the CLI binary is not
+///    installed inside the container.
+/// 3. If the daemon is unreachable and the binary is also missing, returns
+///    `NotInstalled`; otherwise `NotRunning`.
 pub async fn check_docker() -> DockerDetection {
     let platform = Platform::current();
+    let binary_found = docker_binary_exists();
+    let docker_host_set = std::env::var_os("DOCKER_HOST").is_some();
+    let socket_found = any_docker_socket_exists();
 
-    // Step 1: Check if docker binary is on PATH
-    if !docker_binary_exists() {
+    // Fast path: no CLI binary, no DOCKER_HOST, and no socket file on disk.
+    // Skip the daemon ping that would block on an unreachable host.
+    if should_skip_daemon_ping(binary_found, docker_host_set, socket_found) {
         return DockerDetection {
             status: DockerStatus::NotInstalled,
             platform,
         };
     }
 
-    // Step 2: Try to connect to the daemon
+    // Authoritative check: try to connect and ping the daemon via bollard.
+    // Covers DinD (socket bind-mounted, no CLI binary) and standard installs.
     if crate::sandbox::connect_docker().await.is_ok() {
         return DockerDetection {
             status: DockerStatus::Available,
             platform,
         };
     }
 
+    // Daemon unreachable. Distinguish "not installed" from "not running".
+    if !binary_found {
+        return DockerDetection {
+            status: DockerStatus::NotInstalled,
+            platform,
+        };
+    }
+
     // Windows fallback: if the named pipe probe fails but docker CLI can still
     // reach the daemon/server, treat Docker as available.
     #[cfg(windows)]
@@ -162,6 +187,38 @@ fn docker_binary_exists() -> bool {
     }
 }
 
+/// Check if any well-known Docker socket file exists on disk.
+///
+/// This is a cheap filesystem probe (no daemon ping) used to decide whether
+/// it is worth attempting the slower `connect_docker()` call.  Covers the
+/// default `/var/run/docker.sock` plus the user-space candidates that
+/// `connect_docker()` already tries.
+fn any_docker_socket_exists() -> bool {
+    #[cfg(unix)]
+    {
+        use std::path::PathBuf;
+
+        // Default socket that bollard's connect_with_local_defaults() checks.
+        if PathBuf::from("/var/run/docker.sock").exists() {
+            return true;
+        }
+
+        // Same user-space candidates that connect_docker() iterates.
+        crate::sandbox::container::unix_socket_candidates()
+            .iter()
+            .any(|sock| sock.exists())
+    }
+
+    #[cfg(windows)]
+    {
+        // On Windows, bollard probes the named pipe `//./pipe/docker_engine`.
+        // `Path::exists()` doesn't work for named pipes, so we conservatively
+        // return true — the binary-exists check is the primary fast-path on
+        // Windows.
+        true
+    }
+}
+
 #[cfg(windows)]
 fn docker_cli_daemon_reachable() -> bool {
     let stdout = std::process::Stdio::null();
@@ -188,6 +245,12 @@ fn docker_cli_daemon_reachable() -> bool {
         .is_ok_and(|s| s.success())
 }
 
+/// Returns `true` when we can confidently say Docker is not installed without
+/// performing a daemon ping.  All three signals must be absent.
+fn should_skip_daemon_ping(binary_found: bool, docker_host_set: bool, socket_found: bool) -> bool {
+    !binary_found && !docker_host_set && !socket_found
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -232,4 +295,45 @@ mod tests {
             DockerStatus::Disabled => panic!("check_docker should never return Disabled"),
         }
     }
+
+    // --- Regression tests for the fast-path that skips the daemon ping ---
+
+    #[test]
+    fn skip_ping_when_no_binary_no_host_no_socket() {
+        // The bug: connect_docker() was called unconditionally, blocking for
+        // 120 s on hosts with an unreachable DOCKER_HOST and no Docker.
+        assert!(should_skip_daemon_ping(false, false, false));
+    }
+
+    #[test]
+    fn no_skip_when_docker_host_set() {
+        // DOCKER_HOST points somewhere — must attempt the ping even without
+        // a binary (could be a remote Docker host).
+        assert!(!should_skip_daemon_ping(false, true, false));
+    }
+
+    #[test]
+    fn no_skip_when_socket_exists() {
+        // Socket on disk (DinD bind-mount) — must ping even without the CLI.
+        assert!(!should_skip_daemon_ping(false, false, true));
+    }
+
+    #[test]
+    fn no_skip_when_binary_found() {
+        // CLI binary present — Docker may be installed but daemon stopped.
+        assert!(!should_skip_daemon_ping(true, false, false));
+    }
+
+    #[test]
+    fn no_skip_when_all_signals_present() {
+        assert!(!should_skip_daemon_ping(true, true, true));
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn any_docker_socket_nonexistent_path_returns_false() {
+        // Sanity: a path that doesn't exist should not count as a socket.
+        use std::path::PathBuf;
+        assert!(!PathBuf::from("/tmp/definitely-not-a-docker-socket.sock").exists());
+    }
 }