From 26df9c9efe2a8a84e1ca72d33c0630b3a3142ec1 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Tue, 23 Jun 2026 00:46:38 +0000 Subject: [PATCH] Support Codex home instructions directory --- codex-rs/codex-home/src/instructions/mod.rs | 207 +++++++++++++++++- codex-rs/codex-home/src/instructions/tests.rs | 106 ++++++++- 2 files changed, 301 insertions(+), 12 deletions(-) diff --git a/codex-rs/codex-home/src/instructions/mod.rs b/codex-rs/codex-home/src/instructions/mod.rs index 4f4ea76525..7721e31a3d 100644 --- a/codex-rs/codex-home/src/instructions/mod.rs +++ b/codex-rs/codex-home/src/instructions/mod.rs @@ -1,4 +1,5 @@ use std::io; +use std::path::Path; use codex_extension_api::LoadUserInstructionsFuture; use codex_extension_api::LoadedUserInstructions; @@ -8,6 +9,14 @@ use codex_utils_absolute_path::AbsolutePathBuf; const DEFAULT_AGENTS_MD_FILENAME: &str = "AGENTS.md"; const LOCAL_AGENTS_MD_FILENAME: &str = "AGENTS.override.md"; +const INSTRUCTIONS_DIR_NAME: &str = "instructions"; +const INSTRUCTIONS_FILE_EXTENSION: &str = "md"; + +#[derive(Clone, Debug)] +struct HomeInstructionFile { + text: String, + source: AbsolutePathBuf, +} /// Loads user instructions from a Codex home directory. #[derive(Clone, Debug)] @@ -23,6 +32,34 @@ impl CodexHomeUserInstructionsProvider { async fn load_from_codex_home(&self) -> LoadedUserInstructions { let mut warnings = Vec::new(); + + if let Some(instructions) = self.load_agents_md(&mut warnings).await { + return LoadedUserInstructions { + instructions: Some(UserInstructions { + text: instructions.text, + source: instructions.source, + }), + warnings, + }; + } + + let Some(instructions) = self.load_instructions_dir(&mut warnings).await else { + return LoadedUserInstructions { + instructions: None, + warnings, + }; + }; + + LoadedUserInstructions { + instructions: Some(UserInstructions { + text: instructions.text, + source: instructions.source, + }), + warnings, + } + } + + async fn load_agents_md(&self, warnings: &mut Vec) -> Option { for candidate in [LOCAL_AGENTS_MD_FILENAME, DEFAULT_AGENTS_MD_FILENAME] { let path = self.codex_home.join(candidate); match tokio::fs::metadata(path.as_path()).await { @@ -51,20 +88,168 @@ impl CodexHomeUserInstructionsProvider { let contents = String::from_utf8_lossy(&data); let trimmed = contents.trim(); if !trimmed.is_empty() { - return LoadedUserInstructions { - instructions: Some(UserInstructions { - text: trimmed.to_string(), - source: path, - }), - warnings, - }; + return Some(HomeInstructionFile { + text: trimmed.to_string(), + source: path, + }); } } - LoadedUserInstructions { - instructions: None, - warnings, - } + + None } + + async fn load_instructions_dir( + &self, + warnings: &mut Vec, + ) -> Option { + let instructions_dir = self.codex_home.join(INSTRUCTIONS_DIR_NAME); + match tokio::fs::symlink_metadata(instructions_dir.as_path()).await { + Ok(metadata) if !metadata.is_dir() => return None, + Ok(_) => {} + Err(err) if err.kind() == io::ErrorKind::NotFound => return None, + Err(err) => { + warnings.push(format!( + "Failed to read global instructions directory from `{}`: {err}", + instructions_dir.display() + )); + return None; + } + } + + let mut pending_dirs = vec![instructions_dir.as_path().to_path_buf()]; + let mut candidates = Vec::new(); + while let Some(dir) = pending_dirs.pop() { + let mut read_dir = match tokio::fs::read_dir(&dir).await { + Ok(read_dir) => read_dir, + Err(err) if err.kind() == io::ErrorKind::NotFound => continue, + Err(err) => { + warnings.push(format!( + "Failed to read global instructions directory from `{}`: {err}", + dir.display() + )); + continue; + } + }; + + loop { + match read_dir.next_entry().await { + Ok(Some(entry)) => { + let path = entry.path(); + let metadata = match tokio::fs::symlink_metadata(&path).await { + Ok(metadata) => metadata, + Err(err) if err.kind() == io::ErrorKind::NotFound => continue, + Err(err) => { + warnings.push(format!( + "Failed to read global instruction path from `{}`: {err}", + path.display() + )); + continue; + } + }; + let file_type = metadata.file_type(); + if file_type.is_symlink() { + continue; + } + if file_type.is_dir() { + pending_dirs.push(path); + } else if file_type.is_file() + && is_markdown_file(&path) + && let Ok(relative_path) = path + .strip_prefix(instructions_dir.as_path()) + .map(Path::to_path_buf) + { + candidates.push((relative_path, path)); + } + } + Ok(None) => break, + Err(err) => { + warnings.push(format!( + "Failed to read global instructions directory entry from `{}`: {err}", + dir.display() + )); + break; + } + } + } + } + candidates.sort_by(|(left, _), (right, _)| left.cmp(right)); + + let mut files = Vec::new(); + for (_relative_path, path) in candidates { + let Ok(source) = AbsolutePathBuf::try_from(path.clone()) else { + warnings.push(format!( + "Failed to read global instruction file from `{}`: path is not absolute", + path.display() + )); + continue; + }; + + match tokio::fs::metadata(&path).await { + Ok(metadata) if !metadata.is_file() => continue, + Ok(_) => {} + Err(err) if err.kind() == io::ErrorKind::NotFound => continue, + Err(err) => { + warnings.push(format!( + "Failed to read global instruction file from `{}`: {err}", + path.display() + )); + continue; + } + } + + let data = match tokio::fs::read(&path).await { + Ok(data) => data, + Err(err) if err.kind() == io::ErrorKind::NotFound => continue, + Err(err) => { + warnings.push(format!( + "Failed to read global instruction file from `{}`: {err}", + path.display() + )); + continue; + } + }; + let contents = String::from_utf8_lossy(&data); + let trimmed = contents.trim(); + if !trimmed.is_empty() { + files.push(HomeInstructionFile { + text: trimmed.to_string(), + source, + }); + } + } + + self.combine_instruction_files(files) + } + + fn combine_instruction_files( + &self, + files: Vec, + ) -> Option { + let mut files = files.into_iter(); + let first = files.next()?; + let mut combined = first.text; + let mut source = first.source; + let mut has_multiple_sources = false; + + for file in files { + combined.push_str("\n\n"); + combined.push_str(&file.text); + has_multiple_sources = true; + } + + if has_multiple_sources { + source = self.codex_home.join(INSTRUCTIONS_DIR_NAME); + } + + Some(HomeInstructionFile { + text: combined, + source, + }) + } +} + +fn is_markdown_file(path: &Path) -> bool { + path.extension().and_then(|extension| extension.to_str()) == Some(INSTRUCTIONS_FILE_EXTENSION) } impl UserInstructionsProvider for CodexHomeUserInstructionsProvider { diff --git a/codex-rs/codex-home/src/instructions/tests.rs b/codex-rs/codex-home/src/instructions/tests.rs index ee2ba9035c..5c9329450f 100644 --- a/codex-rs/codex-home/src/instructions/tests.rs +++ b/codex-rs/codex-home/src/instructions/tests.rs @@ -10,6 +10,7 @@ use tempfile::TempDir; use super::CodexHomeUserInstructionsProvider; use super::DEFAULT_AGENTS_MD_FILENAME; +use super::INSTRUCTIONS_DIR_NAME; use super::LOCAL_AGENTS_MD_FILENAME; fn provider(home: &TempDir) -> CodexHomeUserInstructionsProvider { @@ -23,11 +24,19 @@ fn expected( filename: &str, text: &str, warnings: Vec, +) -> LoadedUserInstructions { + expected_with_source(home.path().join(filename), text, warnings) +} + +fn expected_with_source( + source: impl AsRef, + text: &str, + warnings: Vec, ) -> LoadedUserInstructions { LoadedUserInstructions { instructions: Some(UserInstructions { text: text.to_string(), - source: AbsolutePathBuf::try_from(home.path().join(filename)) + source: AbsolutePathBuf::try_from(source.as_ref().to_path_buf()) .expect("absolute source path"), }), warnings, @@ -107,6 +116,101 @@ async fn directory_override_falls_back_to_default() { ); } +#[tokio::test] +async fn instructions_directory_markdown_files_are_loaded_recursively_in_sorted_order() { + let home = TempDir::new().expect("temp dir"); + let instructions_dir = home.path().join(INSTRUCTIONS_DIR_NAME); + fs::create_dir_all(instructions_dir.join("eng")).expect("create eng instructions dir"); + fs::create_dir_all(instructions_dir.join("sales")).expect("create sales instructions dir"); + fs::write( + instructions_dir.join("sales").join("20-team-specifics.md"), + "third", + ) + .expect("write third"); + fs::write(instructions_dir.join("00-guardrails.md"), "first").expect("write first"); + fs::write( + instructions_dir.join("eng").join("10-internal-tooling.md"), + "second", + ) + .expect("write second"); + fs::write(instructions_dir.join("c.txt"), "ignored").expect("write ignored"); + fs::write(instructions_dir.join("eng").join("05-empty.md"), " \n\t").expect("write empty"); + + assert_eq!( + provider(&home).load_user_instructions().await, + expected_with_source(&instructions_dir, "first\n\nsecond\n\nthird", Vec::new()) + ); +} + +#[tokio::test] +async fn agents_md_prevents_instructions_directory_fallback() { + let home = TempDir::new().expect("temp dir"); + let instructions_dir = home.path().join(INSTRUCTIONS_DIR_NAME); + fs::create_dir(&instructions_dir).expect("create instructions dir"); + fs::write(home.path().join(DEFAULT_AGENTS_MD_FILENAME), "default").expect("write default"); + fs::write(instructions_dir.join("extra.md"), "extra").expect("write extra"); + + assert_eq!( + provider(&home).load_user_instructions().await, + expected(&home, DEFAULT_AGENTS_MD_FILENAME, "default", Vec::new()) + ); +} + +#[tokio::test] +async fn override_prevents_instructions_directory_fallback() { + let home = TempDir::new().expect("temp dir"); + let instructions_dir = home.path().join(INSTRUCTIONS_DIR_NAME); + fs::create_dir(&instructions_dir).expect("create instructions dir"); + fs::write(home.path().join(DEFAULT_AGENTS_MD_FILENAME), "default").expect("write default"); + fs::write(home.path().join(LOCAL_AGENTS_MD_FILENAME), "override").expect("write override"); + fs::write(instructions_dir.join("extra.md"), "extra").expect("write extra"); + + assert_eq!( + provider(&home).load_user_instructions().await, + expected(&home, LOCAL_AGENTS_MD_FILENAME, "override", Vec::new()) + ); +} + +#[tokio::test] +async fn empty_agents_files_fall_back_to_instructions_directory() { + let home = TempDir::new().expect("temp dir"); + let instructions_dir = home.path().join(INSTRUCTIONS_DIR_NAME); + fs::create_dir(&instructions_dir).expect("create instructions dir"); + fs::write(home.path().join(DEFAULT_AGENTS_MD_FILENAME), " \n\t").expect("write default"); + fs::write(home.path().join(LOCAL_AGENTS_MD_FILENAME), " \n\t").expect("write override"); + fs::write(instructions_dir.join("fallback.md"), "fallback").expect("write fallback"); + + assert_eq!( + provider(&home).load_user_instructions().await, + expected(&home, "instructions/fallback.md", "fallback", Vec::new()) + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn instructions_directory_ignores_symlinked_files_and_directories() { + let home = TempDir::new().expect("temp dir"); + let instructions_dir = home.path().join(INSTRUCTIONS_DIR_NAME); + let linked_dir = home.path().join("linked"); + fs::create_dir(&instructions_dir).expect("create instructions dir"); + fs::create_dir(&linked_dir).expect("create linked dir"); + fs::write(instructions_dir.join("real.md"), "real").expect("write real"); + fs::write(linked_dir.join("from-linked-dir.md"), "linked dir").expect("write linked dir"); + fs::write(linked_dir.join("linked-file.md"), "linked file").expect("write linked file"); + std::os::unix::fs::symlink(&linked_dir, instructions_dir.join("linked-dir")) + .expect("create directory symlink"); + std::os::unix::fs::symlink( + linked_dir.join("linked-file.md"), + instructions_dir.join("linked-file.md"), + ) + .expect("create file symlink"); + + assert_eq!( + provider(&home).load_user_instructions().await, + expected(&home, "instructions/real.md", "real", Vec::new()) + ); +} + #[tokio::test] async fn recoverable_override_read_error_warns_and_falls_back_to_default() { let home = TempDir::new().expect("temp dir");