Fix Windows nanoserver Docker test failures

Accumulated fixes from CI testing against Windows nanoserver containers:

- exists: use tar-based check as primary on Windows (exec `if exist`
  cannot see tar-created paths due to ContainerUser permissions)
- remove: split into separate rmdir/del exec calls (compound `&`
  operator has unreliable errorlevel propagation on nanoserver)
- search: add tar-based fallback with condition_matches() for Windows
  where findstr/dir cannot access tar-created paths
- search: fix has_any() to include has_grep in the check
- search: fix findstr probe to check stdout instead of exit code
  (findstr /? exits 1 on nanoserver but still prints help)
- process: retrieve real exit codes via inspect_exec API instead of
  hardcoding success=true/code=None
- process: move PTY resize_exec after start_exec (Docker requires
  exec to be running before resize is valid)
- process: fix proc_kill to not remove process map entry (let reader
  task cleanup handle removal to avoid race conditions)

Author: chipsenkbeil

distant-docker/src/api.rs

@@ -9,8 +9,8 @@ use async_once_cell::OnceCell;
 use bollard::Docker;
 use distant_core::protocol::{
     ChangeKind, DirEntry, Environment, FileType, Metadata, PROTOCOL_VERSION, Permissions,
-    ProcessId, PtySize, SearchId, SearchQuery, SearchQueryTarget, SetPermissionsOptions,
-    SystemInfo, UnixMetadata, Version,
+    ProcessId, PtySize, SearchId, SearchQuery, SearchQueryMatch, SearchQueryTarget,
+    SetPermissionsOptions, SystemInfo, UnixMetadata, Version,
 };
 use distant_core::{Api, Ctx};
 use tokio::sync::RwLock;
@@ -128,6 +128,122 @@ impl DockerApi {
             )))
         }
     }
+
+    /// Perform a search using the Docker tar API as a fallback.
+    ///
+    /// On Windows nanoserver, exec-based search tools (`findstr`, `dir /s /b`) cannot
+    /// access paths created via the Docker tar API due to `ContainerUser` permission
+    /// restrictions. This method reads directory listings and file contents through
+    /// the Docker archive API and performs matching in Rust.
+    async fn tar_based_search(&self, query: &SearchQuery) -> Vec<SearchQueryMatch> {
+        use distant_core::protocol::{
+            SearchQueryContentsMatch, SearchQueryMatchData, SearchQueryPathMatch,
+            SearchQuerySubmatch,
+        };
+
+        let mut matches = Vec::new();
+
+        let path = query
+            .paths
+            .first()
+            .map(|p| p.to_string_lossy().to_string())
+            .unwrap_or_else(|| ".".to_string());
+
+        // List all entries in the search directory via tar
+        let entries = match utils::tar_list_dir(&self.client, &self.container, &path).await {
+            Ok(e) => e,
+            Err(_) => return matches,
+        };
+
+        match query.target {
+            SearchQueryTarget::Path => {
+                for (_entry_type, entry_path, _size, _mtime) in &entries {
+                    let filename = std::path::Path::new(entry_path)
+                        .file_name()
+                        .map(|n| n.to_string_lossy().to_string())
+                        .unwrap_or_default();
+
+                    if search::condition_matches(&query.condition, &filename) {
+                        // Reconstruct the full container path.
+                        // tar_list_dir returns paths like "dirname/file.txt" — strip
+                        // the first component and any leading separator so Path::join
+                        // treats the remainder as relative (not absolute).
+                        let relative = entry_path
+                            .strip_prefix(
+                                std::path::Path::new(entry_path)
+                                    .components()
+                                    .next()
+                                    .map(|c| c.as_os_str().to_string_lossy().to_string())
+                                    .unwrap_or_default()
+                                    .as_str(),
+                            )
+                            .unwrap_or(entry_path)
+                            .trim_start_matches(['/', '\\']);
+                        let full_path = std::path::Path::new(&path).join(relative);
+                        matches.push(SearchQueryMatch::Path(SearchQueryPathMatch {
+                            path: full_path,
+                            submatches: vec![SearchQuerySubmatch {
+                                r#match: SearchQueryMatchData::Text(filename),
+                                start: 0,
+                                end: 0,
+                            }],
+                        }));
+                    }
+                }
+            }
+            SearchQueryTarget::Contents => {
+                for (entry_type, entry_path, _size, _mtime) in &entries {
+                    // Skip directories — only search file contents
+                    if *entry_type == tar::EntryType::Directory {
+                        continue;
+                    }
+
+                    // Reconstruct the full container path for this file
+                    let relative = entry_path
+                        .strip_prefix(
+                            std::path::Path::new(entry_path)
+                                .components()
+                                .next()
+                                .map(|c| c.as_os_str().to_string_lossy().to_string())
+                                .unwrap_or_default()
+                                .as_str(),
+                        )
+                        .unwrap_or(entry_path)
+                        .trim_start_matches(['/', '\\']);
+                    let full_path = std::path::Path::new(&path).join(relative);
+                    let full_path_str = full_path.to_string_lossy().to_string();
+
+                    // Read file contents via tar API
+                    let data =
+                        match utils::tar_read_file(&self.client, &self.container, &full_path_str)
+                            .await
+                        {
+                            Ok(d) => d,
+                            Err(_) => continue,
+                        };
+
+                    let text = String::from_utf8_lossy(&data);
+                    for (line_num, line) in text.lines().enumerate() {
+                        if search::condition_matches(&query.condition, line) {
+                            matches.push(SearchQueryMatch::Contents(SearchQueryContentsMatch {
+                                path: full_path.clone(),
+                                lines: SearchQueryMatchData::Text(line.to_string()),
+                                line_number: (line_num + 1) as u64,
+                                absolute_offset: 0,
+                                submatches: vec![SearchQuerySubmatch {
+                                    r#match: SearchQueryMatchData::Text(line.to_string()),
+                                    start: 0,
+                                    end: 0,
+                                }],
+                            }));
+                        }
+                    }
+                }
+            }
+        }
+
+        matches
+    }
 }
 
 impl Api for DockerApi {
@@ -525,24 +641,30 @@ impl Api for DockerApi {
         async move {
             let path_str = path.to_string_lossy().to_string();
 
-            let cmd = match self.family {
-                DockerFamily::Unix => {
-                    if force {
-                        format!("rm -rf '{}'", path_str)
-                    } else {
-                        format!("rm -r '{}'", path_str)
-                    }
-                }
-                DockerFamily::Windows => {
-                    // Try rmdir first (for dirs), fall back to del (for files)
-                    format!(
-                        "rmdir /s /q \"{}\" 2>nul & if errorlevel 1 del /f \"{}\"",
-                        path_str, path_str
-                    )
-                }
-            };
+            if self.family == DockerFamily::Unix {
+                let cmd = if force {
+                    format!("rm -rf '{}'", path_str)
+                } else {
+                    format!("rm -r '{}'", path_str)
+                };
+                return self.run_shell_cmd_stdout(&cmd).await.map(|_| ());
+            }
 
-            self.run_shell_cmd_stdout(&cmd).await.map(|_| ())
+            // Windows: nanoserver's cmd.exe has limited compound operators —
+            // `||` is not supported and `& if errorlevel 1` has unreliable
+            // errorlevel propagation. Try `rmdir /s /q` first (handles
+            // directories), then `del /f` for files, as separate exec calls.
+            let rmdir_cmd = format!("rmdir /s /q \"{}\"", path_str);
+            if self
+                .run_shell_cmd(&rmdir_cmd)
+                .await
+                .is_ok_and(|o| o.success())
+            {
+                return Ok(());
+            }
+
+            let del_cmd = format!("del /f /q \"{}\"", path_str);
+            self.run_shell_cmd_stdout(&del_cmd).await.map(|_| ())
         }
     }
 
@@ -627,18 +749,21 @@ impl Api for DockerApi {
         async move {
             let path_str = path.to_string_lossy().to_string();
 
-            // Try exec-based check first
-            let cmd = match self.family {
-                DockerFamily::Unix => format!("test -e '{}'", path_str),
-                DockerFamily::Windows => {
-                    format!("if exist \"{}\" (exit 0) else (exit 1)", path_str)
-                }
-            };
+            if self.family == DockerFamily::Windows {
+                // On Windows nanoserver, the exec-based `if exist` check is unreliable:
+                // it cannot see directories created via the Docker tar API (used as
+                // fallback when `mkdir` fails due to ContainerUser permissions).
+                // Use the tar-based check as the primary method — Docker's archive
+                // API correctly reports existence regardless of how the path was created,
+                // and returns 404 for truly deleted paths.
+                return Ok(utils::tar_path_exists(&self.client, &self.container, &path_str).await);
+            }
 
+            // Unix: use exec-based check with tar fallback for infrastructure errors
+            let cmd = format!("test -e '{}'", path_str);
             match self.run_shell_cmd(&cmd).await {
                 Ok(output) => Ok(output.success()),
                 Err(_) => {
-                    // Fallback to tar-based existence check
                     Ok(utils::tar_path_exists(&self.client, &self.container, &path_str).await)
                 }
             }
@@ -737,26 +862,28 @@ impl Api for DockerApi {
         query: SearchQuery,
     ) -> impl std::future::Future<Output = io::Result<SearchId>> + Send {
         async move {
-            if !self.search_tools.has_any() {
-                return Err(io::Error::new(
-                    io::ErrorKind::Unsupported,
-                    "No search tools available in this container. \
-                     Install ripgrep, grep, or find for search support.",
-                ));
-            }
-
-            let cmd = search::build_search_command(&query, &self.search_tools, self.family)?;
             let search_id: SearchId = rand::random();
 
-            // Run the search command
-            let output = self.run_shell_cmd(&cmd).await?;
-            let stdout = output.stdout_str();
+            // Try exec-based search first if tools are available
+            let mut matches = Vec::new();
+            if self.search_tools.has_any()
+                && let Ok(cmd) =
+                    search::build_search_command(&query, &self.search_tools, self.family)
+                && let Ok(output) = self.run_shell_cmd(&cmd).await
+            {
+                let stdout = output.stdout_str();
+                matches = match query.target {
+                    SearchQueryTarget::Contents => search::parse_contents_matches(&stdout),
+                    SearchQueryTarget::Path => search::parse_path_matches(&stdout),
+                };
+            }
 
-            // Parse results based on search target
-            let matches = match query.target {
-                SearchQueryTarget::Contents => search::parse_contents_matches(&stdout),
-                SearchQueryTarget::Path => search::parse_path_matches(&stdout),
-            };
+            // On Windows nanoserver, exec-based search tools (findstr, dir) cannot
+            // access paths created via the Docker tar API. Fall back to a tar-based
+            // search that reads files through the Docker archive API.
+            if matches.is_empty() && self.family == DockerFamily::Windows {
+                matches = self.tar_based_search(&query).await;
+            }
 
             // Send results via reply
             use distant_core::protocol::Response;
@@ -870,12 +997,20 @@ impl Api for DockerApi {
             let mut processes = self.processes.write().await;
             match processes.get_mut(&id) {
                 Some(process) => {
-                    // Send kill signal via channel
+                    // Take the kill channel to send the signal. We do NOT remove the
+                    // map entry here — the reader task's cleanup closure is the sole
+                    // owner of removal. Removing here creates a race: if a new process
+                    // reuses the same ProcessId before the reader finishes, the stale
+                    // cleanup would delete the new entry.
                     if let Some(kill_tx) = process.kill_tx.take() {
                         let _ = kill_tx.send(()).await;
+                        Ok(())
+                    } else {
+                        Err(io::Error::new(
+                            io::ErrorKind::InvalidInput,
+                            format!("Process {} has already been killed", id),
+                        ))
                     }
-                    processes.remove(&id);
-                    Ok(())
                 }
                 None => Err(io::Error::new(
                     io::ErrorKind::NotFound,

distant-docker/src/process.rs

@@ -132,6 +132,8 @@ where
             let stderr_reply = reply.clone_reply();
             let exit_reply = reply;
             let msg_id = id;
+            let exit_client = client.clone();
+            let exit_exec_id = exec_id.clone();
 
             // Reader task: forwards stdout/stderr to the client
             tokio::spawn(async move {
@@ -157,11 +159,25 @@ where
                     }
                 }
 
-                // Process has exited
+                // Retrieve the real exit code from the Docker exec API
+                let (success, code) = match exit_client.inspect_exec(&exit_exec_id).await {
+                    Ok(inspect) => {
+                        let exit_code = inspect.exit_code.unwrap_or(-1);
+                        (exit_code == 0, Some(exit_code as i32))
+                    }
+                    Err(e) => {
+                        warn!(
+                            "Failed to inspect exec {} for process {}: {}",
+                            exit_exec_id, msg_id, e
+                        );
+                        (false, None)
+                    }
+                };
+
                 let _ = exit_reply.send(Response::ProcDone {
                     id: msg_id,
-                    success: true,
-                    code: None,
+                    success,
+                    code,
                 });
 
                 cleanup(msg_id).await;
@@ -268,18 +284,6 @@ where
 
     let exec_id = created.id.clone();
 
-    // Resize the PTY to the requested size before starting
-    client
-        .resize_exec(
-            &exec_id,
-            ResizeExecOptions {
-                height: size.rows,
-                width: size.cols,
-            },
-        )
-        .await
-        .map_err(|e| io::Error::other(format!("Failed to resize PTY: {}", e)))?;
-
     let start_result = client
         .start_exec(
             &created.id,
@@ -291,6 +295,20 @@ where
         .await
         .map_err(|e| io::Error::other(format!("Failed to start PTY process: {}", e)))?;
 
+    // Resize the PTY after starting — the Docker API requires the exec to be
+    // running before resize_exec is valid. This is best-effort; if it fails,
+    // the writer task's resize handler will correct the size on the next
+    // client-initiated resize.
+    let _ = client
+        .resize_exec(
+            &exec_id,
+            ResizeExecOptions {
+                height: size.rows,
+                width: size.cols,
+            },
+        )
+        .await;
+
     let id: ProcessId = rand::random();
 
     let (stdin_tx, mut stdin_rx) = mpsc::channel::<Vec<u8>>(32);
@@ -305,6 +323,8 @@ where
             let stdout_reply = reply.clone_reply();
             let exit_reply = reply;
             let msg_id = id;
+            let exit_client = client.clone();
+            let exit_exec_id = exec_id.clone();
 
             // Reader task: PTY combines stdout+stderr into one stream
             tokio::spawn(async move {
@@ -325,10 +345,25 @@ where
                     }
                 }
 
+                // Retrieve the real exit code from the Docker exec API
+                let (success, code) = match exit_client.inspect_exec(&exit_exec_id).await {
+                    Ok(inspect) => {
+                        let exit_code = inspect.exit_code.unwrap_or(-1);
+                        (exit_code == 0, Some(exit_code as i32))
+                    }
+                    Err(e) => {
+                        warn!(
+                            "Failed to inspect exec {} for PTY process {}: {}",
+                            exit_exec_id, msg_id, e
+                        );
+                        (false, None)
+                    }
+                };
+
                 let _ = exit_reply.send(Response::ProcDone {
                     id: msg_id,
-                    success: true,
-                    code: None,
+                    success,
+                    code,
                 });
 
                 cleanup(msg_id).await;