From 1233c1b3197471823b2f48fa7c391e35c29abac6 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 12 Aug 2026 01:35:02 +0300 Subject: [PATCH 1/2] fix(skills): keep unreadable directories non-fatal Downgrade recoverable skill discovery read failures so background supervision does not treat them as terminal run errors. Add a regression that keeps loading valid configured skills beside an unreadable unrelated directory. Agent: Marcellinus --- codex-rs/core-skills/Cargo.toml | 1 + codex-rs/core-skills/src/loader.rs | 4 +- codex-rs/core-skills/src/loader_tests.rs | 64 ++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index 3c18bee603..a769a36ae5 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 88af6ada8d..40318e86ed 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 f6c71b2be5..4995ec88c1 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() { From 65964a54feebf022dc38d536d62b24e3439d818c Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 12 Aug 2026 01:39:14 +0300 Subject: [PATCH 2/2] fix(skills): update core-skills lock entry Record the existing tracing-test workspace package in codex-core-skills so remote cargo fetch --locked can run the regression. Agent: Marcellinus --- codex-rs/Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index acddceab74..54ff61440b 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", ]