mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-09 22:33:37 +00:00
Fix Sandbox::glob to use consistent glob semantics across all provide… (#546)
## 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 <pattern>`, but
`find -name` only matches the basename and rejects patterns containing
`/`. So `find <base> -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 <root> -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
<details>
<summary>Ran 8 stages in 64m 31s for $14.97</summary>
| 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** |
</details>
<details>
<summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14
edges)</summary>
```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
}
```
</details>
⚒️ Generated with [Fabro](https://fabro.sh)
---------
Co-authored-by: Fabro <noreply@fabro.sh>
Co-authored-by: Scott Werner <stwerner@vt.edu>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c945fb404b
commit
bb369181b6
7 changed files with 617 additions and 57 deletions
|
|
@ -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<Vec<String>> {
|
||||
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::<Vec<_>>();
|
||||
matches.sort();
|
||||
Ok(matches)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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::<Vec<_>>();
|
||||
matches.sort();
|
||||
Ok(matches)
|
||||
}
|
||||
|
||||
fn working_directory(&self) -> &str {
|
||||
|
|
|
|||
243
lib/crates/fabro-sandbox/src/glob_match.rs
Normal file
243
lib/crates/fabro-sandbox/src/glob_match.rs
Normal file
|
|
@ -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<Self> {
|
||||
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<Vec<String>> {
|
||||
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::<Vec<_>>();
|
||||
|
||||
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::<Vec<_>>();
|
||||
expected.sort();
|
||||
|
||||
let mut actual = match_glob(&path_string(base), pattern, &candidates).unwrap();
|
||||
actual.sort();
|
||||
|
||||
assert_eq!(actual, expected, "pattern {full_pattern}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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<Vec<String>> {
|
||||
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::<Vec<_>>();
|
||||
|
||||
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<String> = 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<Vec<(String, SystemTime)>> {
|
||||
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();
|
||||
|
|
|
|||
|
|
@ -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 <base> -name <pattern>`, 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<DaytonaSandbox>) -> Result<()> {
|
||||
let chunks = Arc::new(Mutex::new(Vec::new()));
|
||||
let cancel_token = CancellationToken::new();
|
||||
|
|
|
|||
|
|
@ -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 <base> -name <pattern>`, 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:?}"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue