mirror of
https://github.com/openai/codex.git
synced 2026-09-04 15:08:45 +00:00
Reuse directory entry metadata for skill scans
This commit is contained in:
@@ -558,46 +558,21 @@ async fn discover_skills_under_root(
|
||||
}
|
||||
|
||||
let path = dir.join(&file_name);
|
||||
let path_uri = PathUri::from_abs_path(&path);
|
||||
let metadata = match fs.get_metadata(&path_uri, /*sandbox*/ None).await {
|
||||
Ok(metadata) => metadata,
|
||||
Err(e) => {
|
||||
error!("failed to stat skills path {}: {e:#}", path.display());
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
if metadata.is_symlink {
|
||||
if !follow_symlinks {
|
||||
continue;
|
||||
}
|
||||
match fs.read_directory(&path_uri, /*sandbox*/ None).await {
|
||||
Ok(_) => {
|
||||
let resolved_dir = canonicalize_for_skill_identity(fs, &path).await;
|
||||
enqueue_dir(
|
||||
&mut queue,
|
||||
&mut visited_dirs,
|
||||
&mut truncated_by_dir_limit,
|
||||
resolved_dir,
|
||||
depth + 1,
|
||||
);
|
||||
}
|
||||
Err(err)
|
||||
if matches!(
|
||||
err.kind(),
|
||||
io::ErrorKind::NotADirectory | io::ErrorKind::NotFound
|
||||
) => {}
|
||||
Err(err) => {
|
||||
error!(
|
||||
"failed to read skills symlink dir {}: {err:#}",
|
||||
path.display()
|
||||
);
|
||||
}
|
||||
if entry.is_symlink {
|
||||
if follow_symlinks && entry.is_directory {
|
||||
let resolved_dir = canonicalize_for_skill_identity(fs, &path).await;
|
||||
enqueue_dir(
|
||||
&mut queue,
|
||||
&mut visited_dirs,
|
||||
&mut truncated_by_dir_limit,
|
||||
resolved_dir,
|
||||
depth + 1,
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if metadata.is_directory {
|
||||
if entry.is_directory {
|
||||
let resolved_dir = canonicalize_for_skill_identity(fs, &path).await;
|
||||
enqueue_dir(
|
||||
&mut queue,
|
||||
@@ -609,7 +584,7 @@ async fn discover_skills_under_root(
|
||||
continue;
|
||||
}
|
||||
|
||||
if metadata.is_file && file_name == SKILLS_FILENAME {
|
||||
if entry.is_file && file_name == SKILLS_FILENAME {
|
||||
match parse_skill_file(fs, &path, scope, plugin_id, plugin_root.as_ref()).await {
|
||||
Ok(skill) => {
|
||||
outcome.skills.push(skill);
|
||||
|
||||
@@ -257,6 +257,7 @@ pub(crate) async fn run_direct_request(
|
||||
file_name: entry.file_name,
|
||||
is_directory: entry.is_directory,
|
||||
is_file: entry.is_file,
|
||||
is_symlink: entry.is_symlink,
|
||||
})
|
||||
.collect();
|
||||
Ok(FsHelperPayload::ReadDirectory(FsReadDirectoryResponse {
|
||||
|
||||
@@ -597,13 +597,23 @@ impl DirectFileSystem {
|
||||
let mut entries = Vec::new();
|
||||
let mut read_dir = tokio::fs::read_dir(path.as_path()).await?;
|
||||
while let Some(entry) = read_dir.next_entry().await? {
|
||||
let Ok(metadata) = tokio::fs::metadata(entry.path()).await else {
|
||||
let Ok(file_type) = entry.file_type().await else {
|
||||
continue;
|
||||
};
|
||||
let is_symlink = file_type.is_symlink();
|
||||
let (is_directory, is_file) = if is_symlink {
|
||||
let Ok(metadata) = tokio::fs::metadata(entry.path()).await else {
|
||||
continue;
|
||||
};
|
||||
(metadata.is_dir(), metadata.is_file())
|
||||
} else {
|
||||
(file_type.is_dir(), file_type.is_file())
|
||||
};
|
||||
entries.push(ReadDirectoryEntry {
|
||||
file_name: entry.file_name().to_string_lossy().into_owned(),
|
||||
is_directory: metadata.is_dir(),
|
||||
is_file: metadata.is_file(),
|
||||
is_directory,
|
||||
is_file,
|
||||
is_symlink,
|
||||
});
|
||||
}
|
||||
Ok(entries)
|
||||
|
||||
@@ -320,6 +320,8 @@ pub struct FsReadDirectoryEntry {
|
||||
pub file_name: String,
|
||||
pub is_directory: bool,
|
||||
pub is_file: bool,
|
||||
#[serde(default)]
|
||||
pub is_symlink: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
|
||||
@@ -179,6 +179,7 @@ impl RemoteFileSystem {
|
||||
file_name: entry.file_name,
|
||||
is_directory: entry.is_directory,
|
||||
is_file: entry.is_file,
|
||||
is_symlink: entry.is_symlink,
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
|
||||
@@ -196,6 +196,7 @@ impl SandboxedFileSystem {
|
||||
file_name: entry.file_name,
|
||||
is_directory: entry.is_directory,
|
||||
is_file: entry.is_file,
|
||||
is_symlink: entry.is_symlink,
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
|
||||
@@ -193,6 +193,7 @@ impl FileSystemHandler {
|
||||
file_name: entry.file_name,
|
||||
is_directory: entry.is_directory,
|
||||
is_file: entry.is_file,
|
||||
is_symlink: entry.is_symlink,
|
||||
})
|
||||
.collect();
|
||||
Ok(FsReadDirectoryResponse { entries })
|
||||
|
||||
@@ -343,11 +343,13 @@ async fn file_system_read_directory_lists_entries(
|
||||
file_name: "nested".to_string(),
|
||||
is_directory: true,
|
||||
is_file: false,
|
||||
is_symlink: false,
|
||||
},
|
||||
ReadDirectoryEntry {
|
||||
file_name: "root.txt".to_string(),
|
||||
is_directory: false,
|
||||
is_file: true,
|
||||
is_symlink: false,
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
@@ -23,6 +23,7 @@ use codex_exec_server::CreateDirectoryOptions;
|
||||
#[cfg(target_os = "linux")]
|
||||
use codex_exec_server::Environment;
|
||||
use codex_exec_server::FileMetadata;
|
||||
use codex_exec_server::ReadDirectoryEntry;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -79,6 +80,51 @@ fn assert_normalized_path_rejected(error: &std::io::Error) {
|
||||
}
|
||||
}
|
||||
|
||||
#[test_case(FileSystemImplementation::Local ; "local")]
|
||||
#[test_case(FileSystemImplementation::Remote ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn file_system_read_directory_reports_symlink_target_kind(
|
||||
implementation: FileSystemImplementation,
|
||||
) -> Result<()> {
|
||||
let context = create_file_system_context(implementation).await?;
|
||||
let file_system = context.file_system;
|
||||
|
||||
let tmp = TempDir::new()?;
|
||||
let source_dir = tmp.path().join("source");
|
||||
let target_dir = tmp.path().join("target-dir");
|
||||
let target_file = tmp.path().join("target.txt");
|
||||
std::fs::create_dir_all(&source_dir)?;
|
||||
std::fs::create_dir_all(&target_dir)?;
|
||||
std::fs::write(&target_file, "hello")?;
|
||||
symlink(&target_dir, source_dir.join("directory-link"))?;
|
||||
symlink(&target_file, source_dir.join("file-link"))?;
|
||||
|
||||
let mut entries = file_system
|
||||
.read_directory(&PathUri::from_path(&source_dir)?, /*sandbox*/ None)
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
entries.sort_by(|left, right| left.file_name.cmp(&right.file_name));
|
||||
assert_eq!(
|
||||
entries,
|
||||
vec![
|
||||
ReadDirectoryEntry {
|
||||
file_name: "directory-link".to_string(),
|
||||
is_directory: true,
|
||||
is_file: false,
|
||||
is_symlink: true,
|
||||
},
|
||||
ReadDirectoryEntry {
|
||||
file_name: "file-link".to_string(),
|
||||
is_directory: false,
|
||||
is_file: true,
|
||||
is_symlink: true,
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn alias_root_candidate() -> Result<Option<PathBuf>> {
|
||||
for root in [Path::new("/tmp").to_path_buf(), std::env::temp_dir()] {
|
||||
if root.is_dir() && root.canonicalize().is_ok_and(|canonical| canonical != root) {
|
||||
|
||||
@@ -60,12 +60,14 @@ impl SyntheticFileSystem {
|
||||
file_name: "skill".to_string(),
|
||||
is_directory: true,
|
||||
is_file: false,
|
||||
is_symlink: false,
|
||||
}])
|
||||
} else if path == self.canonical_root.join("skill") {
|
||||
Ok(vec![ReadDirectoryEntry {
|
||||
file_name: "SKILL.md".to_string(),
|
||||
is_directory: false,
|
||||
is_file: true,
|
||||
is_symlink: false,
|
||||
}])
|
||||
} else {
|
||||
Err(io::Error::new(io::ErrorKind::NotFound, "not found"))
|
||||
@@ -146,7 +148,17 @@ impl ExecutorFileSystem for SyntheticFileSystem {
|
||||
path: &'a PathUri,
|
||||
_sandbox: Option<&'a FileSystemSandboxContext>,
|
||||
) -> ExecutorFileSystemFuture<'a, FileMetadata> {
|
||||
Box::pin(async move { self.metadata(&path.to_abs_path()?) })
|
||||
Box::pin(async move {
|
||||
let path = path.to_abs_path()?;
|
||||
if path == self.canonical_root {
|
||||
self.metadata(&path)
|
||||
} else {
|
||||
Err(io::Error::new(
|
||||
io::ErrorKind::Unsupported,
|
||||
"synthetic filesystem exposes child entry types through read_directory",
|
||||
))
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn read_directory<'a>(
|
||||
|
||||
@@ -54,6 +54,7 @@ pub struct ReadDirectoryEntry {
|
||||
pub file_name: String,
|
||||
pub is_directory: bool,
|
||||
pub is_file: bool,
|
||||
pub is_symlink: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
|
||||
|
||||
Reference in New Issue
Block a user