diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index acddceab7..54ff61440 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2695,6 +2695,7 @@ dependencies = [ "tokio", "toml 0.9.11+spec-1.1.0", "tracing", + "tracing-test", "zip 2.4.2", ] diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index 3c18bee60..a769a36ae 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -41,3 +41,4 @@ zip = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } tempfile = { workspace = true } +tracing-test = { workspace = true, features = ["no-env-filter"] } diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 88af6ada8..40318e86e 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -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; } }; diff --git a/codex-rs/core-skills/src/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs index f6c71b2be..4995ec88c 100644 --- a/codex-rs/core-skills/src/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -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"; @@ -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() {