mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Route AGENTS.md loading through environment filesystems (#26205)
## Why Workspace-specific `AGENTS.md` loading needs to use the selected environment filesystem so remote workspaces and child agents read instructions from their actual environment instead of the host filesystem. The app-server should report the same instruction sources the initialized thread actually loaded, rather than independently rescanning configuration and filesystem state. ## What changed - Introduce `LoadedAgentsMd` to retain ordered user, project, and internal instructions with their provenance. - Load and canonicalize workspace `AGENTS.md` paths through the primary `EnvironmentManager` environment, then render the loaded instructions when constructing turn context. - Expose cached loaded instruction sources from initialized threads and use them for app-server start, resume, and fork responses. - Preserve global `CODEX_HOME` loading and separator behavior while excluding empty project files that did not supply model-visible instructions. - Add integration coverage for CLI injection, selected-environment provenance and rendering, empty environment selection, and cached sources on loaded-thread resume. ## Validation - `just test -p codex-core agents_md` - `just test -p codex-core selected_environment_sources_match_model_visible_instructions` - `just test -p codex-exec agents_md` - `just test -p codex-app-server instruction_sources` - `just test -p codex-app-server --status-level fail`
This commit is contained in:
+136
-68
@@ -23,11 +23,9 @@ use codex_config::merge_toml_values;
|
||||
use codex_config::project_root_markers_from_config;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_features::Feature;
|
||||
use codex_prompts::HIERARCHICAL_AGENTS_MESSAGE;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use dunce::canonicalize as normalize_path;
|
||||
use std::io;
|
||||
use toml::Value as TomlValue;
|
||||
use tracing::error;
|
||||
@@ -37,8 +35,8 @@ pub const DEFAULT_AGENTS_MD_FILENAME: &str = "AGENTS.md";
|
||||
/// Preferred local override for AGENTS.md instructions.
|
||||
pub const LOCAL_AGENTS_MD_FILENAME: &str = "AGENTS.override.md";
|
||||
|
||||
/// When both `Config::instructions` and AGENTS.md docs are present, they will
|
||||
/// be concatenated with the following separator.
|
||||
/// When both user and project AGENTS.md docs are present, they will be
|
||||
/// concatenated with the following separator.
|
||||
const AGENTS_MD_SEPARATOR: &str = "\n\n--- project-doc ---\n\n";
|
||||
|
||||
/// Resolves AGENTS.md files into model-visible user instructions and source
|
||||
@@ -47,11 +45,6 @@ pub struct AgentsMdManager<'a> {
|
||||
config: &'a Config,
|
||||
}
|
||||
|
||||
pub(crate) struct LoadedAgentsMd {
|
||||
pub(crate) contents: String,
|
||||
pub(crate) path: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
impl<'a> AgentsMdManager<'a> {
|
||||
pub fn new(config: &'a Config) -> Self {
|
||||
Self { config }
|
||||
@@ -81,10 +74,7 @@ impl<'a> AgentsMdManager<'a> {
|
||||
let contents = String::from_utf8_lossy(&data);
|
||||
let trimmed = contents.trim();
|
||||
if !trimmed.is_empty() {
|
||||
return Some(LoadedAgentsMd {
|
||||
contents: trimmed.to_string(),
|
||||
path,
|
||||
});
|
||||
return Some(LoadedAgentsMd::new_user(trimmed.to_string(), path));
|
||||
}
|
||||
}
|
||||
None
|
||||
@@ -94,10 +84,10 @@ impl<'a> AgentsMdManager<'a> {
|
||||
/// single model-visible instruction string.
|
||||
pub(crate) async fn user_instructions(
|
||||
&self,
|
||||
environment: Option<&Environment>,
|
||||
environment: &Environment,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
) -> Option<String> {
|
||||
let fs = environment?.get_filesystem();
|
||||
) -> Option<LoadedAgentsMd> {
|
||||
let fs = environment.get_filesystem();
|
||||
self.user_instructions_with_fs(fs.as_ref(), startup_warnings)
|
||||
.await
|
||||
}
|
||||
@@ -106,22 +96,13 @@ impl<'a> AgentsMdManager<'a> {
|
||||
&self,
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
) -> Option<String> {
|
||||
) -> Option<LoadedAgentsMd> {
|
||||
let agents_md_docs = self.read_agents_md(fs, startup_warnings).await;
|
||||
|
||||
let mut output = String::new();
|
||||
|
||||
if let Some(instructions) = self.config.user_instructions.clone() {
|
||||
output.push_str(&instructions);
|
||||
}
|
||||
let mut loaded = self.config.user_instructions.clone().unwrap_or_default();
|
||||
|
||||
match agents_md_docs {
|
||||
Ok(Some(docs)) => {
|
||||
if !output.is_empty() {
|
||||
output.push_str(AGENTS_MD_SEPARATOR);
|
||||
}
|
||||
output.push_str(&docs);
|
||||
}
|
||||
Ok(Some(docs)) => loaded.entries.extend(docs.entries),
|
||||
Ok(None) => {}
|
||||
Err(e) => {
|
||||
error!("error trying to find AGENTS.md docs: {e:#}");
|
||||
@@ -129,50 +110,26 @@ impl<'a> AgentsMdManager<'a> {
|
||||
};
|
||||
|
||||
if self.config.features.enabled(Feature::ChildAgentsMd) {
|
||||
if !output.is_empty() {
|
||||
output.push_str("\n\n");
|
||||
}
|
||||
output.push_str(HIERARCHICAL_AGENTS_MESSAGE);
|
||||
loaded.entries.push(InstructionEntry {
|
||||
contents: HIERARCHICAL_AGENTS_MESSAGE.to_string(),
|
||||
provenance: InstructionProvenance::Internal,
|
||||
});
|
||||
}
|
||||
|
||||
if !output.is_empty() {
|
||||
Some(output)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns all instruction source files included in the current config.
|
||||
pub async fn instruction_sources(&self, fs: &dyn ExecutorFileSystem) -> Vec<AbsolutePathBuf> {
|
||||
let mut global_instruction_warnings = Vec::new();
|
||||
let mut paths = Self::load_global_instructions(
|
||||
LOCAL_FS.as_ref(),
|
||||
Some(&self.config.codex_home),
|
||||
&mut global_instruction_warnings,
|
||||
)
|
||||
.await
|
||||
.map(|loaded| vec![loaded.path])
|
||||
.unwrap_or_default();
|
||||
match self.agents_md_paths(fs).await {
|
||||
Ok(agents_md_paths) => paths.extend(agents_md_paths),
|
||||
Err(err) => {
|
||||
tracing::warn!(error = %err, "failed to discover AGENTS.md docs for instruction sources");
|
||||
}
|
||||
}
|
||||
paths
|
||||
(!loaded.is_empty()).then_some(loaded)
|
||||
}
|
||||
|
||||
/// Attempt to locate and load AGENTS.md documentation.
|
||||
///
|
||||
/// On success returns `Ok(Some(contents))` where `contents` is the
|
||||
/// concatenation of all discovered docs. If no documentation file is found
|
||||
/// the function returns `Ok(None)`. Unexpected I/O failures bubble up as
|
||||
/// `Err` so callers can decide how to handle them.
|
||||
/// On success returns `Ok(Some(loaded))` where `loaded` contains every
|
||||
/// discovered doc. If no documentation file is found the function returns
|
||||
/// `Ok(None)`. Unexpected I/O failures bubble up as `Err` so callers can
|
||||
/// decide how to handle them.
|
||||
async fn read_agents_md(
|
||||
&self,
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
) -> io::Result<Option<String>> {
|
||||
) -> io::Result<Option<LoadedAgentsMd>> {
|
||||
let max_total = self.config.project_doc_max_bytes;
|
||||
|
||||
if max_total == 0 {
|
||||
@@ -185,7 +142,7 @@ impl<'a> AgentsMdManager<'a> {
|
||||
}
|
||||
|
||||
let mut remaining: u64 = max_total as u64;
|
||||
let mut parts: Vec<String> = Vec::new();
|
||||
let mut loaded = LoadedAgentsMd::default();
|
||||
|
||||
for p in paths {
|
||||
if remaining == 0 {
|
||||
@@ -221,15 +178,18 @@ impl<'a> AgentsMdManager<'a> {
|
||||
|
||||
let text = String::from_utf8_lossy(&data).to_string();
|
||||
if !text.trim().is_empty() {
|
||||
parts.push(text);
|
||||
loaded.entries.push(InstructionEntry {
|
||||
contents: text,
|
||||
provenance: InstructionProvenance::Project(p),
|
||||
});
|
||||
remaining = remaining.saturating_sub(data.len() as u64);
|
||||
}
|
||||
}
|
||||
|
||||
if parts.is_empty() {
|
||||
if loaded.is_empty() {
|
||||
Ok(None)
|
||||
} else {
|
||||
Ok(Some(parts.join("\n\n")))
|
||||
Ok(Some(loaded))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -247,8 +207,8 @@ impl<'a> AgentsMdManager<'a> {
|
||||
}
|
||||
|
||||
let mut dir = self.config.cwd.clone();
|
||||
if let Ok(canon) = normalize_path(&dir) {
|
||||
dir = AbsolutePathBuf::try_from(canon)?;
|
||||
if let Ok(canonical_dir) = fs.canonicalize(&dir, /*sandbox*/ None).await {
|
||||
dir = canonical_dir;
|
||||
}
|
||||
|
||||
let mut merged = TomlValue::Table(toml::map::Map::new());
|
||||
@@ -348,6 +308,114 @@ impl<'a> AgentsMdManager<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Model-visible instructions loaded from AGENTS.md files and internal
|
||||
/// guidance.
|
||||
#[derive(Clone, Debug, Default, PartialEq, Eq)]
|
||||
pub struct LoadedAgentsMd {
|
||||
/// Ordered instructions and their provenance.
|
||||
entries: Vec<InstructionEntry>,
|
||||
}
|
||||
|
||||
impl LoadedAgentsMd {
|
||||
/// Creates loaded instructions containing one user-level AGENTS.md entry.
|
||||
pub fn new_user(contents: String, path: AbsolutePathBuf) -> Self {
|
||||
if contents.trim().is_empty() {
|
||||
return Self::default();
|
||||
}
|
||||
Self {
|
||||
entries: vec![InstructionEntry {
|
||||
contents,
|
||||
provenance: InstructionProvenance::User(path),
|
||||
}],
|
||||
}
|
||||
}
|
||||
|
||||
/// Creates source-less user instructions for tests.
|
||||
///
|
||||
/// This cannot be gated with `#[cfg(test)]` because integration tests
|
||||
/// compile `codex-core` as a normal dependency without that configuration.
|
||||
pub fn from_text_for_testing(contents: impl Into<String>) -> Self {
|
||||
let contents = contents.into();
|
||||
if contents.trim().is_empty() {
|
||||
return Self::default();
|
||||
}
|
||||
Self {
|
||||
entries: vec![InstructionEntry {
|
||||
contents,
|
||||
provenance: InstructionProvenance::Internal,
|
||||
}],
|
||||
}
|
||||
}
|
||||
|
||||
fn is_empty(&self) -> bool {
|
||||
self.entries
|
||||
.iter()
|
||||
.all(|entry| entry.contents.trim().is_empty())
|
||||
}
|
||||
|
||||
/// Returns the concatenated model-visible instruction text.
|
||||
pub fn text(&self) -> String {
|
||||
let mut output = String::new();
|
||||
let mut previous_provenance: Option<&InstructionProvenance> = None;
|
||||
for entry in &self.entries {
|
||||
if let Some(previous_provenance) = previous_provenance {
|
||||
// The project-doc marker tells the model where workspace-scoped
|
||||
// instructions begin, so it is only needed on the transition
|
||||
// from user or internal instructions to project instructions.
|
||||
let separator = match (previous_provenance, &entry.provenance) {
|
||||
(
|
||||
InstructionProvenance::User(_) | InstructionProvenance::Internal,
|
||||
InstructionProvenance::Project(_),
|
||||
) => AGENTS_MD_SEPARATOR,
|
||||
_ => "\n\n",
|
||||
};
|
||||
output.push_str(separator);
|
||||
}
|
||||
output.push_str(&entry.contents);
|
||||
previous_provenance = Some(&entry.provenance);
|
||||
}
|
||||
output
|
||||
}
|
||||
|
||||
/// Returns the AGENTS.md files that supplied instruction entries.
|
||||
pub fn sources(&self) -> impl Iterator<Item = &AbsolutePathBuf> {
|
||||
self.entries
|
||||
.iter()
|
||||
.filter_map(|entry| entry.provenance.path())
|
||||
}
|
||||
}
|
||||
|
||||
/// One model-visible instruction and its provenance.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
struct InstructionEntry {
|
||||
/// Model-visible instruction text.
|
||||
contents: String,
|
||||
|
||||
/// Origin of the instruction.
|
||||
provenance: InstructionProvenance,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
enum InstructionProvenance {
|
||||
/// User-level instructions, normally loaded from CODEX_HOME.
|
||||
User(AbsolutePathBuf),
|
||||
|
||||
/// Workspace instructions discovered from project AGENTS.md files.
|
||||
Project(AbsolutePathBuf),
|
||||
|
||||
/// Instructions without a file source, including internally defined guidance.
|
||||
Internal,
|
||||
}
|
||||
|
||||
impl InstructionProvenance {
|
||||
fn path(&self) -> Option<&AbsolutePathBuf> {
|
||||
match self {
|
||||
Self::User(path) | Self::Project(path) => Some(path),
|
||||
Self::Internal => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn warn_invalid_utf8(
|
||||
path: &AbsolutePathBuf,
|
||||
data: &[u8],
|
||||
|
||||
Reference in New Issue
Block a user