mirror of
https://github.com/openai/codex.git
synced 2026-09-06 15:29:32 +00:00
Support Codex home instructions directory
This commit is contained in:
@@ -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<String>) -> Option<HomeInstructionFile> {
|
||||
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<String>,
|
||||
) -> Option<HomeInstructionFile> {
|
||||
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<HomeInstructionFile>,
|
||||
) -> Option<HomeInstructionFile> {
|
||||
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 {
|
||||
|
||||
@@ -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<String>,
|
||||
) -> LoadedUserInstructions {
|
||||
expected_with_source(home.path().join(filename), text, warnings)
|
||||
}
|
||||
|
||||
fn expected_with_source(
|
||||
source: impl AsRef<Path>,
|
||||
text: &str,
|
||||
warnings: Vec<String>,
|
||||
) -> 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");
|
||||
|
||||
Reference in New Issue
Block a user