Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions codex-rs/core-skills/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,4 @@ zip = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = { workspace = true }
tracing-test = { workspace = true, features = ["no-env-filter"] }
4 changes: 3 additions & 1 deletion codex-rs/core-skills/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,9 @@ async fn discover_skills_under_root(
let entries = match fs.read_directory(&dir, /*sandbox*/ None).await {
Ok(entries) => entries,
Err(e) => {
error!("failed to read skills dir {}: {e:#}", dir.display());
// Skill roots are optional discovery inputs. Keep a denied descendant visible,
// but do not emit an error that supervisors can mistake for a terminal run failure.
tracing::warn!("failed to read skills dir {}: {e:#}", dir.display());
continue;
}
};
Expand Down
64 changes: 64 additions & 0 deletions codex-rs/core-skills/src/loader_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use std::path::PathBuf;
use std::sync::Arc;
use tempfile::TempDir;
use toml::Value as TomlValue;
#[cfg(unix)]
use tracing_test::traced_test;

const REPO_ROOT_CONFIG_DIR_NAME: &str = ".codex";

Expand Down Expand Up @@ -977,6 +979,68 @@ async fn loads_skills_via_symlinked_subdir_for_user_scope() {
);
}

#[tokio::test]
#[cfg(unix)]
#[traced_test]
async fn unreadable_unrelated_directory_does_not_emit_an_error() {
use std::os::unix::fs::PermissionsExt;

let skills_root = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill_at(skills_root.path(), "valid", "valid-skill", "still loads");
let unrelated_dir = skills_root.path().join("Photos Library.photoslibrary");
fs::create_dir_all(&unrelated_dir).unwrap();
fs::set_permissions(&unrelated_dir, fs::Permissions::from_mode(0o000)).unwrap();

let outcome = load_skills_from_roots([SkillRoot {
path: skills_root.path().abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
}])
.await;

fs::set_permissions(&unrelated_dir, fs::Permissions::from_mode(0o700)).unwrap();

assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![SkillMetadata {
name: "valid-skill".to_string(),
description: "still loads".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
}]
);
logs_assert(|lines: &[&str]| {
let line = lines
.iter()
.find(|line| {
line.contains("failed to read skills dir")
&& line.contains("Photos Library.photoslibrary")
})
.ok_or_else(|| "expected a diagnostic for the unreadable directory".to_string())?;
if line.contains("ERROR") {
return Err(format!(
"recoverable discovery failure was logged as ERROR: {line}"
));
}
if !line.contains("WARN") {
return Err(format!("expected a WARN diagnostic: {line}"));
}
Ok(())
});
}

#[tokio::test]
#[cfg(unix)]
async fn ignores_symlinked_skill_file_for_user_scope() {
Expand Down
Loading