mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Load user instructions through an injected provider (#27101)
## Why We want to remove implicit use of `$CODEX_HOME` from `codex-core` and make embedders responsible for supplying user-level instructions. This also ensures user instructions load when no primary environment is selected. ## What changed Stacked on #27415, which makes `codex exec` surface thread-scoped runtime warnings. - Added `UserInstructionsProvider` to `codex-extension-api`, with absolute source attribution and recoverable loading warnings. - Added `codex-home` with the filesystem-backed provider for `AGENTS.override.md` and `AGENTS.md`, preserving precedence, fallback, trimming, lossy UTF-8 handling, and the existing uncapped global instruction size. - Removed global instruction loading from `Config` and require `ThreadManager` callers to inject a provider. - Load provider instructions once for each fresh root runtime, including runtimes without a primary environment. Running sessions retain their snapshot, while child agents inherit the parent snapshot without invoking the provider. - Keep provider instructions separate while loading project `AGENTS.md`, then assemble the model-visible instructions with the existing ordering, source attribution, warning, and turn-context behavior. - Wired the Codex home provider through the CLI, app server, MCP server, core facade, and thread-manager sample. ## Validation - `just test -p codex-home -p codex-extension-api` - `just test -p codex-core agents_md` - `just test -p codex-core guardian` - `just test -p codex-app-server thread_start_without_selected_environment_includes_only_global_instruction_source` - `just test -p codex-exec warning` - `just bazel-lock-check`
This commit is contained in:
committed by
GitHub
Unverified
parent
b2a4e3be27
commit
236b50125d
@@ -0,0 +1,83 @@
|
||||
use std::io;
|
||||
|
||||
use codex_extension_api::LoadUserInstructionsFuture;
|
||||
use codex_extension_api::LoadedUserInstructions;
|
||||
use codex_extension_api::UserInstructions;
|
||||
use codex_extension_api::UserInstructionsProvider;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
const DEFAULT_AGENTS_MD_FILENAME: &str = "AGENTS.md";
|
||||
const LOCAL_AGENTS_MD_FILENAME: &str = "AGENTS.override.md";
|
||||
|
||||
/// Loads user instructions from a Codex home directory.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct CodexHomeUserInstructionsProvider {
|
||||
codex_home: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
impl CodexHomeUserInstructionsProvider {
|
||||
/// Creates a provider rooted at the supplied absolute Codex home directory.
|
||||
pub fn new(codex_home: AbsolutePathBuf) -> Self {
|
||||
Self { codex_home }
|
||||
}
|
||||
|
||||
async fn load_from_codex_home(&self) -> LoadedUserInstructions {
|
||||
let mut warnings = Vec::new();
|
||||
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 {
|
||||
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 AGENTS.md instructions from `{}`: {err}",
|
||||
path.display()
|
||||
));
|
||||
continue;
|
||||
}
|
||||
}
|
||||
let data = match tokio::fs::read(path.as_path()).await {
|
||||
Ok(data) => data,
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => continue,
|
||||
Err(err) => {
|
||||
warnings.push(format!(
|
||||
"Failed to read global AGENTS.md instructions from `{}`: {err}",
|
||||
path.display()
|
||||
));
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if let Err(err) = std::str::from_utf8(&data) {
|
||||
warnings.push(format!(
|
||||
"Global AGENTS.md instructions from `{}` contain invalid UTF-8: {err}. Invalid byte sequences were replaced.",
|
||||
path.display()
|
||||
));
|
||||
}
|
||||
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,
|
||||
};
|
||||
}
|
||||
}
|
||||
LoadedUserInstructions {
|
||||
instructions: None,
|
||||
warnings,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl UserInstructionsProvider for CodexHomeUserInstructionsProvider {
|
||||
fn load_user_instructions(&self) -> LoadUserInstructionsFuture<'_> {
|
||||
Box::pin(self.load_from_codex_home())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
@@ -0,0 +1,152 @@
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
|
||||
use codex_extension_api::LoadedUserInstructions;
|
||||
use codex_extension_api::UserInstructions;
|
||||
use codex_extension_api::UserInstructionsProvider;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
use super::CodexHomeUserInstructionsProvider;
|
||||
use super::DEFAULT_AGENTS_MD_FILENAME;
|
||||
use super::LOCAL_AGENTS_MD_FILENAME;
|
||||
|
||||
fn provider(home: &TempDir) -> CodexHomeUserInstructionsProvider {
|
||||
CodexHomeUserInstructionsProvider::new(
|
||||
AbsolutePathBuf::try_from(home.path().to_path_buf()).expect("absolute temp dir"),
|
||||
)
|
||||
}
|
||||
|
||||
fn expected(
|
||||
home: &TempDir,
|
||||
filename: &str,
|
||||
text: &str,
|
||||
warnings: Vec<String>,
|
||||
) -> LoadedUserInstructions {
|
||||
LoadedUserInstructions {
|
||||
instructions: Some(UserInstructions {
|
||||
text: text.to_string(),
|
||||
source: AbsolutePathBuf::try_from(home.path().join(filename))
|
||||
.expect("absolute source path"),
|
||||
}),
|
||||
warnings,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn create_symlink_loop(path: &Path) {
|
||||
std::os::unix::fs::symlink(
|
||||
path.file_name().expect("override path should have a name"),
|
||||
path,
|
||||
)
|
||||
.expect("create symlink loop");
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn create_symlink_loop(path: &Path) {
|
||||
std::os::windows::fs::symlink_file(
|
||||
path.file_name().expect("override path should have a name"),
|
||||
path,
|
||||
)
|
||||
.expect("create symlink loop");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn missing_files_return_no_instructions() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
|
||||
assert_eq!(
|
||||
provider(&home).load_user_instructions().await,
|
||||
LoadedUserInstructions::default()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn override_takes_precedence_over_default() {
|
||||
let home = TempDir::new().expect("temp 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");
|
||||
|
||||
assert_eq!(
|
||||
provider(&home).load_user_instructions().await,
|
||||
expected(&home, LOCAL_AGENTS_MD_FILENAME, "override", Vec::new())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn empty_override_falls_back_to_trimmed_default() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
fs::write(home.path().join(LOCAL_AGENTS_MD_FILENAME), " \n\t").expect("write override");
|
||||
fs::write(
|
||||
home.path().join(DEFAULT_AGENTS_MD_FILENAME),
|
||||
"\n default instructions \n",
|
||||
)
|
||||
.expect("write default");
|
||||
|
||||
assert_eq!(
|
||||
provider(&home).load_user_instructions().await,
|
||||
expected(
|
||||
&home,
|
||||
DEFAULT_AGENTS_MD_FILENAME,
|
||||
"default instructions",
|
||||
Vec::new()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn directory_override_falls_back_to_default() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
fs::create_dir(home.path().join(LOCAL_AGENTS_MD_FILENAME)).expect("create override directory");
|
||||
fs::write(home.path().join(DEFAULT_AGENTS_MD_FILENAME), "default").expect("write default");
|
||||
|
||||
assert_eq!(
|
||||
provider(&home).load_user_instructions().await,
|
||||
expected(&home, DEFAULT_AGENTS_MD_FILENAME, "default", Vec::new())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn recoverable_override_read_error_warns_and_falls_back_to_default() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let override_path = home.path().join(LOCAL_AGENTS_MD_FILENAME);
|
||||
create_symlink_loop(&override_path);
|
||||
fs::write(home.path().join(DEFAULT_AGENTS_MD_FILENAME), "default").expect("write default");
|
||||
let read_error = fs::read(&override_path).expect_err("symlink loop should not be readable");
|
||||
let warning = format!(
|
||||
"Failed to read global AGENTS.md instructions from `{}`: {read_error}",
|
||||
override_path.display()
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
provider(&home).load_user_instructions().await,
|
||||
expected(&home, DEFAULT_AGENTS_MD_FILENAME, "default", vec![warning])
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn invalid_utf8_is_lossy_and_warned() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let path = home.path().join(DEFAULT_AGENTS_MD_FILENAME);
|
||||
let mut invalid_utf8 = b"global".to_vec();
|
||||
invalid_utf8.push(0xff);
|
||||
invalid_utf8.extend_from_slice(b" doc");
|
||||
fs::write(&path, &invalid_utf8).expect("write invalid utf-8");
|
||||
|
||||
let outcome = provider(&home).load_user_instructions().await;
|
||||
let utf8_error = std::str::from_utf8(&invalid_utf8).expect_err("invalid utf-8");
|
||||
let warning = format!(
|
||||
"Global AGENTS.md instructions from `{}` contain invalid UTF-8: {utf8_error}. Invalid byte sequences were replaced.",
|
||||
path.display(),
|
||||
);
|
||||
assert_eq!(
|
||||
outcome,
|
||||
expected(
|
||||
&home,
|
||||
DEFAULT_AGENTS_MD_FILENAME,
|
||||
"global\u{fffd} doc",
|
||||
vec![warning]
|
||||
)
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user