Follow directory symlinks in filesystem walks (#29844)

Stack 3 of 3. Stacked on #29842.

## What changes

Adds an opt-in `followDirectorySymlinks` setting to `fs/walk`.

When enabled, the walk follows directory symlinks but continues to
ignore symlinked files. Canonical directory identities prevent symlink
cycles, while normal paths keep their existing spelling.

Environment skill discovery enables the setting so symlinked skill
directories continue to work with the new single-RPC scan.
This commit is contained in:
jif
2026-06-24 20:52:36 +01:00
committed by GitHub
Unverified
parent 74dcce594d
commit 96d8e34712
4 changed files with 81 additions and 9 deletions
@@ -116,6 +116,7 @@ pub async fn load_environment_skills_from_root(
max_depth: MAX_SCAN_DEPTH,
max_directories: MAX_SKILLS_DIRS_PER_ROOT,
max_entries: MAX_SKILLS_ENTRIES_PER_ROOT,
follow_directory_symlinks: true,
},
/*sandbox*/ None,
)
@@ -401,6 +401,7 @@ async fn file_system_walk_returns_a_bounded_tree(
max_depth: 4,
max_directories: 10,
max_entries: 10,
follow_directory_symlinks: false,
},
/*sandbox*/ None,
)
@@ -445,6 +446,7 @@ async fn file_system_walk_returns_a_bounded_tree(
max_depth: 0,
max_directories: 10,
max_entries: 10,
follow_directory_symlinks: false,
},
/*sandbox*/ None,
)
@@ -466,6 +468,7 @@ async fn file_system_walk_returns_a_bounded_tree(
max_depth: 4,
max_directories: 1,
max_entries: 10,
follow_directory_symlinks: false,
},
/*sandbox*/ None,
)
@@ -487,6 +490,7 @@ async fn file_system_walk_returns_a_bounded_tree(
max_depth: 4,
max_directories: 10,
max_entries: 1,
follow_directory_symlinks: false,
},
/*sandbox*/ None,
)
@@ -530,6 +534,7 @@ async fn file_system_walk_honors_read_sandbox(
max_depth: 1,
max_directories: 2,
max_entries: 2,
follow_directory_symlinks: false,
},
Some(&sandbox),
)
+38 -5
View File
@@ -273,17 +273,23 @@ async fn file_system_get_metadata_reports_symlink_targets(
#[test_case(FileSystemImplementation::Local ; "local")]
#[test_case(FileSystemImplementation::Remote ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn file_system_walk_ignores_symlinks(implementation: FileSystemImplementation) -> Result<()> {
async fn file_system_walk_handles_directory_symlinks(
implementation: FileSystemImplementation,
) -> Result<()> {
let context = create_file_system_context(implementation).await?;
let file_system = context.file_system;
let tmp = TempDir::new()?;
let root = tmp.path().join("root");
let target = root.join("target");
let target = tmp.path().join("target");
let target_file = target.join("note.txt");
let target_link = root.join("target-link");
let root_link = target.join("root-link");
std::fs::create_dir_all(&root)?;
std::fs::create_dir_all(&target)?;
std::fs::write(&target_file, "target")?;
symlink(&target, root.join("target-link"))?;
symlink(&target, &target_link)?;
symlink(&root, &root_link)?;
let outcome = file_system
.walk(
@@ -292,6 +298,29 @@ async fn file_system_walk_ignores_symlinks(implementation: FileSystemImplementat
max_depth: 2,
max_directories: 4,
max_entries: 8,
follow_directory_symlinks: false,
},
/*sandbox*/ None,
)
.await
.with_context(|| format!("mode={implementation}"))?;
assert_eq!(
outcome,
WalkOutcome {
entries: Vec::new(),
errors: Vec::new(),
truncated: false,
}
);
let outcome = file_system
.walk(
&PathUri::from_host_native_path(&root)?,
WalkOptions {
max_depth: 2,
max_directories: 4,
max_entries: 8,
follow_directory_symlinks: true,
},
/*sandbox*/ None,
)
@@ -302,13 +331,17 @@ async fn file_system_walk_ignores_symlinks(implementation: FileSystemImplementat
WalkOutcome {
entries: vec![
WalkEntry {
path: PathUri::from_host_native_path(&target)?,
path: PathUri::from_host_native_path(&target_link)?,
kind: WalkEntryKind::Directory,
},
WalkEntry {
path: PathUri::from_host_native_path(target_file)?,
path: PathUri::from_host_native_path(target_link.join("note.txt"))?,
kind: WalkEntryKind::File,
},
WalkEntry {
path: PathUri::from_host_native_path(target_link.join("root-link"))?,
kind: WalkEntryKind::Directory,
},
],
errors: Vec::new(),
truncated: false,
+37 -4
View File
@@ -12,6 +12,7 @@ use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use futures::Stream;
use std::collections::HashSet;
use std::collections::VecDeque;
use std::future::Future;
use std::io;
@@ -72,6 +73,8 @@ pub struct WalkOptions {
pub max_directories: usize,
/// Maximum number of directory entries that may be examined.
pub max_entries: usize,
/// Whether directory symlinks should be followed.
pub follow_directory_symlinks: bool,
}
/// Type of a filesystem entry returned by a walk.
@@ -82,7 +85,7 @@ pub enum WalkEntryKind {
File,
}
/// One non-symlink entry returned by a walk.
/// One entry returned by a walk.
#[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
#[serde(rename_all = "camelCase")]
pub struct WalkEntry {
@@ -300,7 +303,7 @@ pub trait ExecutorFileSystem: Send + Sync {
sandbox: Option<&'a FileSystemSandboxContext>,
) -> ExecutorFileSystemFuture<'a, Vec<ReadDirectoryEntry>>;
/// Recursively lists descendants, skipping symlinks.
/// Recursively lists descendants, optionally following directory symlinks.
fn walk<'a>(
&'a self,
path: &'a PathUri,
@@ -351,12 +354,20 @@ async fn walk<F: ExecutorFileSystem + ?Sized>(
}
let root_metadata = file_system.get_metadata(root, sandbox).await?;
if root_metadata.is_symlink || !root_metadata.is_directory {
if !root_metadata.is_directory
|| (root_metadata.is_symlink && !options.follow_directory_symlinks)
{
return Ok(WalkOutcome::default());
}
let root_identity = if options.follow_directory_symlinks {
file_system.canonicalize(root, sandbox).await?
} else {
root.clone()
};
let mut outcome = WalkOutcome::default();
let mut queue = VecDeque::from([(root.clone(), 0usize)]);
let mut visited_directories = HashSet::from([root_identity]);
let mut directory_count = 1usize;
let mut entry_count = 0usize;
let mut response_bytes = 0usize;
@@ -409,7 +420,8 @@ async fn walk<F: ExecutorFileSystem + ?Sized>(
continue;
}
};
if metadata.is_symlink {
if metadata.is_symlink && (!options.follow_directory_symlinks || !metadata.is_directory)
{
continue;
}
@@ -433,6 +445,27 @@ async fn walk<F: ExecutorFileSystem + ?Sized>(
});
if kind == WalkEntryKind::Directory && depth < options.max_depth {
let directory_identity = if options.follow_directory_symlinks {
match file_system.canonicalize(&path, sandbox).await {
Ok(path) => path,
Err(error) => {
if !push_walk_error(
&mut outcome,
&mut response_bytes,
path,
error.to_string(),
) {
return Ok(outcome);
}
continue;
}
}
} else {
path.clone()
};
if !visited_directories.insert(directory_identity) {
continue;
}
if directory_count == options.max_directories {
outcome.truncated = true;
} else {