From bb369181b60ef723723a5ef169ffb23372e10192 Mon Sep 17 00:00:00 2001 From: "fabro-sh-fabro[bot]" <296591931+fabro-sh-fabro[bot]@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:13:08 -0400 Subject: [PATCH] =?UTF-8?q?Fix=20`Sandbox::glob`=20to=20use=20consistent?= =?UTF-8?q?=20glob=20semantics=20across=20all=20provide=E2=80=A6=20(#546)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `Sandbox::glob` worked correctly on the Local provider but silently returned empty results on Docker and Daytona for any pattern containing `/` or `**` (e.g. `*/SKILL.md`). This broke skill discovery on every remote sandbox — the production path — and degraded the agent's `Glob` tool for common patterns like `**/*.rs`. ## Root cause The remote providers delegated matching to `find -name `, but `find -name` only matches the basename and rejects patterns containing `/`. So `find -name "*/SKILL.md"` exits 0 with empty output while the file is sitting right there. ## Fix Glob is two distinct operations: **traversal** (needs filesystem access) and **matching** (pure string logic). The fix separates them cleanly: - A new `glob_match` module (`src/glob_match.rs`) provides `GlobMatcher` and `traversal_root` helpers, backed by the already-present `glob` crate's `Pattern` matcher with `require_literal_separator: true` so `*` stays within a single path segment. - Remote providers (Docker, Daytona) now run `find -type f` (traversal only) and pass results through `GlobMatcher` on the host side. - Daytona additionally gains a `list_files_recursive` path that uses the Daytona filesystem API directly instead of shelling out, which is more robust when the shell is fail-closed. - Local is also rerouted through `GlobMatcher` with a `collect_local_files` walker, making all three providers share identical matching semantics by construction. mtime-based sort is preserved using metadata collected during traversal. ```mermaid flowchart TB caller["glob(pattern, path)"] traversal_root["traversal_root(base, pattern)\nextract literal prefix"] list["list files under root\n(find -type f / fs API / std::fs)"] matcher["GlobMatcher::new(base, pattern)\nglob::Pattern + MatchOptions"] filter["filter candidates"] sort["sort results"] caller --> traversal_root --> list --> filter caller --> matcher --> filter --> sort ``` ### Plan Summary - New `glob_match.rs` module: `GlobMatcher`, `traversal_root`, `join_path` utilities + unit tests proving parity with `glob::glob` on shared fixtures - Docker: replace `find -name` with `find -type f` + host-side `GlobMatcher` - Daytona: replace `find -name` with `list_files_recursive` (Daytona FS API) + `GlobMatcher` - Local: replace `glob::glob()` walk with `collect_local_files` (symlink-safe) + `GlobMatcher`; mtime sort preserved - New `LocalSandbox::glob` tests: relative path resolution, `**` depth, `*/SKILL.md` one-level semantics, symlink non-recursion ### Fabro Details
Ran 8 stages in 64m 31s for $14.97 | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 21s | – | 0 | | preflight_lint | 2m 41s | – | 0 | | implement | 39m 47s | $11.23 | 0 | | simplify_opus | 7m 36s | $2.55 | 0 | | simplify_gpt | 3m 48s | $1.19 | 0 | | verify | 7m 47s | – | 0 | | **Total** | **64m 31s** | **$14.97** | **0** |
Ran ImplementPlan.fabro (11 nodes and 14 edges) ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ```
⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro Co-authored-by: Scott Werner Co-authored-by: Claude Opus 4.8 (1M context) --- lib/crates/fabro-sandbox/src/daytona/mod.rs | 79 ++++-- lib/crates/fabro-sandbox/src/docker.rs | 20 +- lib/crates/fabro-sandbox/src/glob_match.rs | 243 ++++++++++++++++++ lib/crates/fabro-sandbox/src/lib.rs | 1 + lib/crates/fabro-sandbox/src/local.rs | 159 ++++++++++-- .../tests/daytona_streaming_live.rs | 84 ++++++ .../fabro-sandbox/tests/docker_streaming.rs | 88 +++++++ 7 files changed, 617 insertions(+), 57 deletions(-) create mode 100644 lib/crates/fabro-sandbox/src/glob_match.rs diff --git a/lib/crates/fabro-sandbox/src/daytona/mod.rs b/lib/crates/fabro-sandbox/src/daytona/mod.rs index 2ce40b8b5..3c1568fc0 100644 --- a/lib/crates/fabro-sandbox/src/daytona/mod.rs +++ b/lib/crates/fabro-sandbox/src/daytona/mod.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fmt::Write; use std::path::Path; use std::sync::Arc; @@ -29,7 +29,7 @@ use crate::redact::redact_auth_url; use crate::sandbox::{optional_timeout, resolve_path}; use crate::{ CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, Sandbox, - SandboxEvent, SandboxEventCallback, StdioProcess, managed_labels, shell_quote, + SandboxEvent, SandboxEventCallback, StdioProcess, glob_match, managed_labels, shell_quote, }; pub(crate) const WORKING_DIRECTORY: &str = "/home/daytona/workspace"; @@ -493,6 +493,49 @@ impl DaytonaSandbox { }) } + async fn list_files_recursive(&self, root: &str) -> crate::Result> { + let sandbox = self.sandbox()?; + let fs_svc = sandbox + .fs() + .await + .map_err(|e| crate::Error::context("Failed to get Daytona fs service", e))?; + let mut candidates = Vec::new(); + let mut stack = vec![root.to_string()]; + let mut visited_dirs = HashSet::new(); + + while let Some(dir) = stack.pop() { + if !visited_dirs.insert(dir.clone()) { + continue; + } + + let entries = match fs_svc.list_files(&dir).await { + Ok(entries) => entries, + Err(daytona_sdk::DaytonaError::NotFound { .. }) => continue, + Err(err) => { + return Err(crate::Error::context( + format!("Failed to list Daytona directory {dir}"), + err, + )); + } + }; + + for entry in entries { + if entry.name.is_empty() || entry.name == "." || entry.name == ".." { + continue; + } + + let child_path = glob_match::join_path(&dir, &entry.name); + if entry.is_dir { + stack.push(child_path); + } else { + candidates.push(child_path); + } + } + } + + Ok(candidates) + } + /// Read-only access to the SDK sandbox once initialized. Returns `None` /// before `initialize()` or `reconnect()` has populated the cell. pub fn sandbox_handle(&self) -> Option<&daytona_sdk::Sandbox> { @@ -1856,28 +1899,16 @@ impl Sandbox for DaytonaSandbox { |p| self.resolve_path(p), ); - let cmd = format!( - "find {} -name {} -type f | sort", - shell_quote(&base), - shell_quote(pattern), - ); - - let result = self.exec_command(&cmd, 30_000, None, None, None).await?; - - if !result.is_success() { - return Err(crate::Error::message(format!( - "glob failed (exit {}): {}", - result.display_exit_code(), - result.stderr - ))); - } - - Ok(result - .stdout - .lines() - .filter(|l| !l.is_empty()) - .map(String::from) - .collect()) + let traversal_root = glob_match::traversal_root(&base, pattern); + let matcher = glob_match::GlobMatcher::new(&base, pattern)?; + let mut matches = self + .list_files_recursive(&traversal_root) + .await? + .into_iter() + .filter(|path| matcher.matches(path)) + .collect::>(); + matches.sort(); + Ok(matches) } } diff --git a/lib/crates/fabro-sandbox/src/docker.rs b/lib/crates/fabro-sandbox/src/docker.rs index 60ed3c19b..43da3970b 100644 --- a/lib/crates/fabro-sandbox/src/docker.rs +++ b/lib/crates/fabro-sandbox/src/docker.rs @@ -32,7 +32,8 @@ use crate::sandbox::{StdioProcessControl, optional_timeout, resolve_path}; use crate::{ CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector, - StdioProcess, StdioProcessHandle, StdioProcessTermination, format_lines_numbered, shell_quote, + StdioProcess, StdioProcessHandle, StdioProcessTermination, format_lines_numbered, glob_match, + shell_quote, }; pub(crate) const WORKING_DIRECTORY: &str = "/workspace"; @@ -1847,11 +1848,9 @@ impl Sandbox for DockerSandbox { || self.working_directory().to_string(), |path| self.resolve_container_path(path), ); - let command = format!( - "find {} -name {} -type f | sort", - shell_quote(&base_dir), - shell_quote(pattern) - ); + let traversal_root = glob_match::traversal_root(&base_dir, pattern); + let quoted_root = shell_quote(&traversal_root); + let command = format!("if [ -e {quoted_root} ]; then find {quoted_root} -type f; fi"); let result = self .docker_exec_shell(&command, 30_000, None, None, None) .await?; @@ -1863,12 +1862,15 @@ impl Sandbox for DockerSandbox { ))); } - Ok(result + let matcher = glob_match::GlobMatcher::new(&base_dir, pattern)?; + let mut matches = result .stdout .lines() + .filter(|line| !line.is_empty() && matcher.matches(line)) .map(String::from) - .filter(|line| !line.is_empty()) - .collect()) + .collect::>(); + matches.sort(); + Ok(matches) } fn working_directory(&self) -> &str { diff --git a/lib/crates/fabro-sandbox/src/glob_match.rs b/lib/crates/fabro-sandbox/src/glob_match.rs new file mode 100644 index 000000000..61249049d --- /dev/null +++ b/lib/crates/fabro-sandbox/src/glob_match.rs @@ -0,0 +1,243 @@ +use std::path::Path; + +pub(crate) struct GlobMatcher { + pattern: glob::Pattern, +} + +const MATCH_OPTIONS: glob::MatchOptions = glob::MatchOptions { + case_sensitive: true, + require_literal_separator: true, + require_literal_leading_dot: false, +}; + +impl GlobMatcher { + pub(crate) fn new(base: &str, pattern: &str) -> crate::Result { + let full_pattern = full_pattern(base, pattern); + let pattern = glob::Pattern::new(&full_pattern) + .map_err(|err| crate::Error::context("Invalid glob pattern", err))?; + Ok(Self { pattern }) + } + + pub(crate) fn matches(&self, path: &str) -> bool { + self.pattern.matches_with(path, MATCH_OPTIONS) + } +} + +pub(crate) fn traversal_root(base: &str, pattern: &str) -> String { + let full_pattern = full_pattern(base, pattern); + literal_traversal_root(&full_pattern) +} + +pub(crate) fn join_path(base: &str, path: &str) -> String { + if path.is_empty() { + return base.to_string(); + } + if is_absolute(path) { + return path.to_string(); + } + if base.is_empty() { + return path.to_string(); + } + if base == "/" { + return format!("/{path}"); + } + format!("{}/{}", base.trim_end_matches('/'), path) +} + +fn full_pattern(base: &str, pattern: &str) -> String { + if is_absolute(pattern) { + pattern.to_string() + } else { + join_path(base, pattern) + } +} + +fn is_absolute(path: &str) -> bool { + path.starts_with('/') || Path::new(path).is_absolute() +} + +fn literal_traversal_root(pattern: &str) -> String { + let absolute = pattern.starts_with('/'); + let mut literal_segments = Vec::new(); + let mut saw_meta = false; + + for segment in pattern.split('/').filter(|segment| !segment.is_empty()) { + if has_glob_meta(segment) { + saw_meta = true; + break; + } + literal_segments.push(segment); + } + + if saw_meta { + return build_path(absolute, &literal_segments); + } + + parent_path(&build_path(absolute, &literal_segments)) +} + +fn has_glob_meta(segment: &str) -> bool { + segment + .chars() + .any(|character| matches!(character, '*' | '?' | '[')) +} + +fn build_path(absolute: bool, segments: &[&str]) -> String { + if segments.is_empty() { + return if absolute { + "/".to_string() + } else { + ".".to_string() + }; + } + + let joined = segments.join("/"); + if absolute { + format!("/{joined}") + } else { + joined + } +} + +fn parent_path(path: &str) -> String { + if path == "/" { + return "/".to_string(); + } + + let trimmed = path.trim_end_matches('/'); + if trimmed.is_empty() { + return ".".to_string(); + } + + match trimmed.rfind('/') { + Some(0) => "/".to_string(), + Some(index) => trimmed[..index].to_string(), + None => ".".to_string(), + } +} + +#[cfg(test)] +#[expect( + clippy::disallowed_methods, + reason = "glob matcher tests stage filesystem fixtures with sync std::fs writes" +)] +mod tests { + use std::path::Path; + + use super::GlobMatcher; + + fn path_string(path: &Path) -> String { + path.to_string_lossy().into_owned() + } + + fn match_glob( + base: &str, + pattern: &str, + candidate_paths: &[String], + ) -> crate::Result> { + let matcher = GlobMatcher::new(base, pattern)?; + Ok(candidate_paths + .iter() + .filter(|path| matcher.matches(path)) + .cloned() + .collect()) + } + + #[test] + fn skill_pattern_matches_exactly_one_directory_level() { + let candidates = vec![ + "/workspace/SKILL.md".to_string(), + "/workspace/a/SKILL.md".to_string(), + "/workspace/a/b/SKILL.md".to_string(), + "/workspace/a/README.md".to_string(), + ]; + + let results = match_glob("/workspace", "*/SKILL.md", &candidates).unwrap(); + + assert_eq!(results, vec!["/workspace/a/SKILL.md"]); + } + + #[test] + fn star_matches_only_top_level_files_under_base() { + let candidates = vec![ + "/workspace/a.rs".to_string(), + "/workspace/src/lib.rs".to_string(), + "/workspace/b.txt".to_string(), + ]; + + let results = match_glob("/workspace", "*.rs", &candidates).unwrap(); + + assert_eq!(results, vec!["/workspace/a.rs"]); + } + + #[test] + fn recursive_glob_matches_files_at_any_depth() { + let candidates = vec![ + "/workspace/a.rs".to_string(), + "/workspace/src/lib.rs".to_string(), + "/workspace/src/nested/main.rs".to_string(), + "/workspace/src/nested/readme.md".to_string(), + ]; + + let results = match_glob("/workspace", "**/*.rs", &candidates).unwrap(); + + assert_eq!(results, vec![ + "/workspace/a.rs", + "/workspace/src/lib.rs", + "/workspace/src/nested/main.rs", + ]); + } + + #[test] + fn matcher_matches_glob_crate_on_fixture_patterns() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let src = root.join("src"); + let nested = src.join("nested"); + let skills = root.join("skills"); + let skill_a = skills.join("a"); + let skill_b = skill_a.join("b"); + std::fs::create_dir_all(&nested).unwrap(); + std::fs::create_dir_all(&skill_b).unwrap(); + std::fs::write(root.join("a.rs"), "").unwrap(); + std::fs::write(root.join("b.txt"), "").unwrap(); + std::fs::write(src.join("lib.rs"), "").unwrap(); + std::fs::write(nested.join("main.rs"), "").unwrap(); + std::fs::write(skills.join("SKILL.md"), "").unwrap(); + std::fs::write(skill_a.join("SKILL.md"), "").unwrap(); + std::fs::write(skill_b.join("SKILL.md"), "").unwrap(); + + let candidates = [ + root.join("a.rs"), + root.join("b.txt"), + src.join("lib.rs"), + nested.join("main.rs"), + skills.join("SKILL.md"), + skill_a.join("SKILL.md"), + skill_b.join("SKILL.md"), + ] + .into_iter() + .map(|path| path_string(&path)) + .collect::>(); + + for (base, pattern) in [ + (root, "*.rs"), + (root, "**/*.rs"), + (root, "src/*.rs"), + (skills.as_path(), "*/SKILL.md"), + ] { + let full_pattern = format!("{}/{pattern}", base.display()); + let mut expected = glob::glob(&full_pattern) + .unwrap() + .filter_map(Result::ok) + .map(|path| path_string(&path)) + .collect::>(); + expected.sort(); + + let mut actual = match_glob(&path_string(base), pattern, &candidates).unwrap(); + actual.sort(); + + assert_eq!(actual, expected, "pattern {full_pattern}"); + } + } +} diff --git a/lib/crates/fabro-sandbox/src/lib.rs b/lib/crates/fabro-sandbox/src/lib.rs index e53b25756..b7e4751e6 100644 --- a/lib/crates/fabro-sandbox/src/lib.rs +++ b/lib/crates/fabro-sandbox/src/lib.rs @@ -2,6 +2,7 @@ pub mod config; pub mod error; #[cfg(any(feature = "docker", feature = "daytona"))] pub mod from_environment; +mod glob_match; pub mod provider; pub mod sandbox; pub mod sandbox_spec; diff --git a/lib/crates/fabro-sandbox/src/local.rs b/lib/crates/fabro-sandbox/src/local.rs index aae5ef9f4..ca21e5631 100644 --- a/lib/crates/fabro-sandbox/src/local.rs +++ b/lib/crates/fabro-sandbox/src/local.rs @@ -1,5 +1,5 @@ use std::path::{Path, PathBuf}; -use std::time::Instant; +use std::time::{Instant, SystemTime, UNIX_EPOCH}; use async_trait::async_trait; use fabro_static::EnvVars; @@ -16,7 +16,7 @@ use crate::sandbox::{StdioProcessControl, optional_timeout}; use crate::{ CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector, - StdioProcess, StdioProcessHandle, StdioProcessTermination, + StdioProcess, StdioProcessHandle, StdioProcessTermination, glob_match, }; pub struct LocalSandbox { @@ -627,30 +627,20 @@ impl Sandbox for LocalSandbox { async fn glob(&self, pattern: &str, path: Option<&str>) -> crate::Result> { let base_dir = - path.map_or_else(|| self.working_directory.clone(), std::path::PathBuf::from); + path.map_or_else(|| self.working_directory.clone(), |p| self.resolve_path(p)); + let base = base_dir.to_string_lossy().into_owned(); + let traversal_root = PathBuf::from(glob_match::traversal_root(&base, pattern)); + let matcher = glob_match::GlobMatcher::new(&base, pattern)?; + let mut matches = collect_local_files(traversal_root) + .await? + .into_iter() + .filter(|(path, _)| matcher.matches(path)) + .collect::>(); - let full_pattern = if Path::new(pattern).is_absolute() { - pattern.to_string() - } else { - format!("{}/{pattern}", base_dir.display()) - }; + // Sort by mtime (newest first) using the metadata collected during traversal. + matches.sort_by_key(|(_, modified)| std::cmp::Reverse(*modified)); - let mut results: Vec = glob::glob(&full_pattern) - .map_err(|e| crate::Error::context("Invalid glob pattern", e))? - .filter_map(Result::ok) - .map(|p| p.to_string_lossy().into_owned()) - .collect(); - - // Sort by mtime (newest first), caching metadata to avoid O(n log n) syscalls - results.sort_by_cached_key(|path| { - std::cmp::Reverse( - std::fs::metadata(path) - .and_then(|m| m.modified()) - .unwrap_or(std::time::SystemTime::UNIX_EPOCH), - ) - }); - - Ok(results) + Ok(matches.into_iter().map(|(path, _)| path).collect()) } async fn download_file_to_local( @@ -865,6 +855,53 @@ where } } +async fn collect_local_files(root: PathBuf) -> crate::Result> { + let mut files = Vec::new(); + let mut stack = vec![root]; + + while let Some(path) = stack.pop() { + let metadata = match fs::symlink_metadata(&path).await { + Ok(metadata) => metadata, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, + Err(err) => { + return Err(crate::Error::context( + format!("Failed to stat {}", path.display()), + err, + )); + } + }; + + let file_type = metadata.file_type(); + if file_type.is_file() { + files.push(( + path.to_string_lossy().into_owned(), + metadata.modified().unwrap_or(UNIX_EPOCH), + )); + } else if file_type.is_dir() { + let mut entries = match fs::read_dir(&path).await { + Ok(entries) => entries, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, + Err(err) => { + return Err(crate::Error::context( + format!("Failed to read directory {}", path.display()), + err, + )); + } + }; + while let Some(entry) = entries.next_entry().await.map_err(|err| { + crate::Error::context( + format!("Failed to read directory entry in {}", path.display()), + err, + ) + })? { + stack.push(entry.path()); + } + } + } + + Ok(files) +} + #[cfg(test)] #[expect( clippy::disallowed_methods, @@ -1352,6 +1389,80 @@ mod tests { std::fs::remove_dir_all(&dir).unwrap(); } + #[tokio::test] + async fn glob_resolves_relative_search_path_against_working_directory() { + let dir = temp_dir(); + std::fs::create_dir_all(dir.join("src")).unwrap(); + std::fs::write(dir.join("src/lib.rs"), "").unwrap(); + + let env = LocalSandbox::new(dir.clone()); + let results = env.glob("*.rs", Some("src")).await.unwrap(); + + assert_eq!(results, vec![ + dir.join("src/lib.rs").to_string_lossy().into_owned() + ]); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[tokio::test] + async fn glob_recursive_pattern_finds_files_at_any_depth() { + let dir = temp_dir(); + std::fs::create_dir_all(dir.join("src/nested")).unwrap(); + std::fs::write(dir.join("a.rs"), "").unwrap(); + std::fs::write(dir.join("src/lib.rs"), "").unwrap(); + std::fs::write(dir.join("src/nested/main.rs"), "").unwrap(); + std::fs::write(dir.join("src/nested/readme.md"), "").unwrap(); + + let env = LocalSandbox::new(dir.clone()); + let mut results = env.glob("**/*.rs", None).await.unwrap(); + results.sort(); + + assert_eq!(results, vec![ + dir.join("a.rs").to_string_lossy().into_owned(), + dir.join("src/lib.rs").to_string_lossy().into_owned(), + dir.join("src/nested/main.rs") + .to_string_lossy() + .into_owned(), + ]); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[tokio::test] + async fn glob_finds_skill_files_one_level_below_search_dir() { + let dir = temp_dir(); + let skills = dir.join(".fabro/skills"); + std::fs::create_dir_all(skills.join("patch")).unwrap(); + std::fs::create_dir_all(skills.join("nested/deeper")).unwrap(); + std::fs::write(skills.join("SKILL.md"), "").unwrap(); + std::fs::write(skills.join("patch/SKILL.md"), "").unwrap(); + std::fs::write(skills.join("nested/deeper/SKILL.md"), "").unwrap(); + + let env = LocalSandbox::new(dir.clone()); + let skills_path = skills.to_string_lossy().into_owned(); + let results = env.glob("*/SKILL.md", Some(&skills_path)).await.unwrap(); + + assert_eq!(results, vec![ + skills.join("patch/SKILL.md").to_string_lossy().into_owned() + ]); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[cfg(unix)] + #[tokio::test] + async fn glob_does_not_recurse_through_symlinked_directories() { + let dir = temp_dir(); + let target = dir.join("target"); + std::fs::create_dir_all(&target).unwrap(); + std::fs::write(target.join("lib.rs"), "").unwrap(); + std::os::unix::fs::symlink(&target, dir.join("linked")).unwrap(); + + let env = LocalSandbox::new(dir.clone()); + let results = env.glob("linked/**/*.rs", None).await.unwrap(); + + assert!(results.is_empty()); + std::fs::remove_dir_all(&dir).unwrap(); + } + #[tokio::test] async fn local_sandbox_download_file_to_local() { let dir = temp_dir(); diff --git a/lib/crates/fabro-sandbox/tests/daytona_streaming_live.rs b/lib/crates/fabro-sandbox/tests/daytona_streaming_live.rs index ad84e9335..4b5dd6590 100644 --- a/lib/crates/fabro-sandbox/tests/daytona_streaming_live.rs +++ b/lib/crates/fabro-sandbox/tests/daytona_streaming_live.rs @@ -166,6 +166,90 @@ mod daytona_streaming_live { Ok(()) } + // Regression test for glob patterns that contain a path separator. Before + // the glob fix, Daytona ran `find -name `, and `find -name` + // matches only the basename and rejects patterns containing `/`. So + // `*/SKILL.md` and `**/SKILL.md` silently returned an empty list even though + // the files existed. Both `glob` calls below fail against that old + // implementation and pass once traversal (the Daytona filesystem API) and + // matching (host-side) are split. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + #[ignore = "requires live Daytona credentials and provisions a sandbox"] + async fn daytona_glob_matches_patterns_containing_a_path_separator() -> Result<()> { + ensure!( + daytona_api_key_present(), + "DAYTONA_API_KEY must be set to run this live glob test" + ); + + let sandbox = DaytonaSandbox::new( + DaytonaConfig { + skip_clone: true, + ..Default::default() + }, + None, + None, + None, + None, + None, + ) + .await?; + + sandbox.initialize().await?; + + let glob_result = run_glob_checks(&sandbox).await; + let cleanup_result = sandbox.cleanup().await.context("clean up Daytona sandbox"); + + glob_result?; + cleanup_result?; + + Ok(()) + } + + async fn run_glob_checks(sandbox: &DaytonaSandbox) -> Result<()> { + // Build a skills tree with a SKILL.md at the search root, one level + // below it, and two levels below it. + let seed = sandbox + .exec_command( + "mkdir -p skills/patch skills/nested/deeper && \ + touch skills/SKILL.md skills/patch/SKILL.md skills/nested/deeper/SKILL.md", + 30_000, + None, + None, + None, + ) + .await?; + ensure!( + seed.is_success(), + "seeding the skills tree failed: stdout={} stderr={}", + seed.stdout, + seed.stderr + ); + + // `*/SKILL.md` matches exactly one path segment: only the file one level + // below the search directory, not the root file or the deeper one. + let one_level = sandbox.glob("*/SKILL.md", Some("skills")).await?; + ensure_eq( + &one_level.len(), + &1, + "`*/SKILL.md` should match exactly one level below the search dir", + )?; + ensure!( + one_level[0].ends_with("skills/patch/SKILL.md"), + "`*/SKILL.md` should match the one-level-deep file, got {one_level:?}" + ); + + // `**/SKILL.md` matches at any depth, including several levels down. + let recursive = sandbox.glob("**/SKILL.md", Some("skills")).await?; + ensure!( + recursive + .iter() + .any(|path| path.ends_with("skills/nested/deeper/SKILL.md")), + "`**/SKILL.md` should match files nested several levels deep, got {recursive:?}" + ); + + Ok(()) + } + async fn run_smoke(sandbox: Arc) -> Result<()> { let chunks = Arc::new(Mutex::new(Vec::new())); let cancel_token = CancellationToken::new(); diff --git a/lib/crates/fabro-sandbox/tests/docker_streaming.rs b/lib/crates/fabro-sandbox/tests/docker_streaming.rs index 72ceebbce..f77674a30 100644 --- a/lib/crates/fabro-sandbox/tests/docker_streaming.rs +++ b/lib/crates/fabro-sandbox/tests/docker_streaming.rs @@ -147,3 +147,91 @@ async fn cloned_docker_sandbox_uses_repos_checkout_and_workspace_symlink() { ); assert!(result.stdout.contains("true")); } + +// Regression test for glob patterns that contain a path separator. Before the +// glob fix, the remote providers ran `find -name `, and +// `find -name` matches only the basename and rejects patterns containing `/`. +// So `*/SKILL.md` and `**/SKILL.md` silently returned an empty list inside a +// real container even though the files existed. Both `glob` calls below fail +// against that old implementation and pass once traversal and matching are +// split (find files, then match host-side). +#[tokio::test] +#[ignore = "requires real Docker container lifecycle; run explicitly when changing Sandbox::glob"] +async fn docker_glob_matches_patterns_containing_a_path_separator() { + let image = "buildpack-deps:noble"; + let Ok(docker) = Docker::connect_with_local_defaults() else { + return; + }; + if docker.inspect_image(image).await.is_err() { + return; + } + + let sandbox = DockerSandbox::new( + DockerSandboxOptions { + image: image.to_string(), + auto_pull: false, + skip_clone: true, + ..DockerSandboxOptions::default() + }, + None, + None, + None, + None, + ) + .expect("docker sandbox should construct"); + sandbox + .initialize() + .await + .expect("docker sandbox should initialize"); + + // Build a skills tree with a SKILL.md at the search root, one level below + // it, and two levels below it. + let seed = sandbox + .exec_command( + "mkdir -p skills/patch skills/nested/deeper && \ + touch skills/SKILL.md skills/patch/SKILL.md skills/nested/deeper/SKILL.md", + 10_000, + None, + None, + None, + ) + .await + .expect("seed command should run"); + + // `*/SKILL.md` matches exactly one path segment: only the file one level + // below the search directory, not the root file or the deeper one. + let one_level = sandbox.glob("*/SKILL.md", Some("skills")).await; + // `**/SKILL.md` matches at any depth, including several levels down. + let recursive = sandbox.glob("**/SKILL.md", Some("skills")).await; + + sandbox + .cleanup() + .await + .expect("docker cleanup should succeed"); + + assert!( + seed.is_success(), + "seeding the skills tree failed: stdout={} stderr={}", + seed.stdout, + seed.stderr + ); + + let one_level = one_level.expect("glob should run"); + assert_eq!( + one_level.len(), + 1, + "`*/SKILL.md` should match exactly one level below the search dir, got: {one_level:?}" + ); + assert!( + one_level[0].ends_with("skills/patch/SKILL.md"), + "`*/SKILL.md` should match the one-level-deep file, got: {one_level:?}" + ); + + let recursive = recursive.expect("recursive glob should run"); + assert!( + recursive + .iter() + .any(|path| path.ends_with("skills/nested/deeper/SKILL.md")), + "`**/SKILL.md` should match files nested several levels deep, got: {recursive:?}" + ); +}