From 59ca34206b4606a8800a4e565d6006f02fa37206 Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Thu, 4 Jun 2026 15:08:52 -0700 Subject: [PATCH] [codex] Preserve logical paths during AGENTS.md discovery (#26465) ## Intent Follow up on #26205 by avoiding unnecessary filesystem canonicalization during `AGENTS.md` discovery. The configured working directory is already absolute, and canonicalization incorrectly switches symlinked workspaces from their logical parent hierarchy to the target's hierarchy. ## User-facing behavior For a symlinked working directory such as: ```text test-root/ |-- logical-repo/ | |-- AGENTS.md ("logical parent doc") | `-- workspace ------------> physical-repo/workspace/ `-- physical-repo/ |-- AGENTS.md ("physical parent doc") `-- workspace/ `-- AGENTS.md ("workspace doc") ``` Before this change, Codex canonicalized `logical-repo/workspace` to `physical-repo/workspace` before discovery. It therefore loaded `physical-repo/AGENTS.md` and `physical-repo/workspace/AGENTS.md`, ignoring the instructions from the repository through which the user entered the workspace. After this change, ancestor discovery walks the configured logical path, so Codex loads `logical-repo/AGENTS.md`. Opening `logical-repo/workspace/AGENTS.md` still follows the symlink through the host filesystem, so the workspace document is also loaded. `physical-repo/AGENTS.md` is not loaded. ## Implementation Use the logical absolute working directory when discovering project instructions and reporting instruction sources. Filesystem reads still follow the working-directory symlink, so an `AGENTS.md` in the target workspace continues to load while ancestor discovery uses the symlink's parents. ## Validation Added integration coverage proving that discovery loads the logical parent's instructions and the target workspace's instructions, but not the target parent's instructions. --- .../tests/suite/v2/thread_resume.rs | 2 +- .../app-server/tests/suite/v2/thread_start.rs | 2 +- codex-rs/core/src/agents_md.rs | 5 +- codex-rs/core/src/agents_md_tests.rs | 59 ++++++------ codex-rs/core/tests/common/lib.rs | 15 ++++ codex-rs/core/tests/suite/agents_md.rs | 90 +++++++++++++++++-- 6 files changed, 130 insertions(+), 43 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 4e052041a..4ae68b0e7 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -284,7 +284,7 @@ async fn thread_resume_running_thread_uses_cached_instruction_sources() -> Resul instruction_sources, .. } = to_response::(start_resp)?; - let project_agents = AbsolutePathBuf::try_from(std::fs::canonicalize(project_agents)?)?; + let project_agents = AbsolutePathBuf::try_from(project_agents)?; assert_eq!(instruction_sources, vec![project_agents.clone()]); let turn_id = mcp diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 208af1429..a2da95545 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -348,7 +348,7 @@ async fn thread_start_response_includes_loaded_instruction_sources() -> Result<( .collect::>(); let expected_instruction_sources = vec![ std::fs::canonicalize(global_agents_path)?, - std::fs::canonicalize(project_agents_path)?, + project_agents_path, ] .into_iter() .map(normalize_path_for_comparison) diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index bc3740a46..5739e42f3 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -206,10 +206,7 @@ impl<'a> AgentsMdManager<'a> { return Ok(Vec::new()); } - let mut dir = self.config.cwd.clone(); - if let Ok(canonical_dir) = fs.canonicalize(&dir, /*sandbox*/ None).await { - dir = canonical_dir; - } + let dir = self.config.cwd.clone(); let mut merged = TomlValue::Table(toml::map::Map::new()); for layer in self.config.config_layer_stack.get_layers( diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index 95659511b..d8298dfd7 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -5,6 +5,7 @@ use codex_features::Feature; use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::PathBufExt; use core_test_support::TempDirExt; +use core_test_support::create_directory_symlink; use pretty_assertions::assert_eq; use std::fs; use std::path::Path; @@ -223,8 +224,7 @@ async fn project_doc_invalid_utf8_warns_and_uses_lossy_text() { .text(); assert_eq!(res, "project\u{FFFD} doc"); - let canonical_path = dunce::canonicalize(&path).expect("canonical doc path"); - assert_invalid_utf8_warning(&warnings, "Project", &canonical_path); + assert_invalid_utf8_warning(&warnings, "Project", config.cwd.join("AGENTS.md").as_path()); } /// Oversize file is truncated to `project_doc_max_bytes`. @@ -330,10 +330,7 @@ async fn sourceless_user_instructions_preserve_separator_without_reporting_a_sou .user_instructions_with_fs(LOCAL_FS.as_ref(), &mut warnings) .await .expect("instructions expected"); - let project_agents = AbsolutePathBuf::try_from( - dunce::canonicalize(cfg.cwd.join("AGENTS.md")).expect("canonical project doc path"), - ) - .expect("absolute project doc path"); + let project_agents = cfg.cwd.join("AGENTS.md"); assert_eq!( loaded.text(), @@ -385,14 +382,8 @@ async fn concatenates_root_and_cwd_docs() { .user_instructions_with_fs(LOCAL_FS.as_ref(), &mut warnings) .await .expect("doc expected"); - let root_agents = AbsolutePathBuf::try_from( - dunce::canonicalize(repo.path().join("AGENTS.md")).expect("canonical root doc path"), - ) - .expect("absolute root doc path"); - let crate_agents = AbsolutePathBuf::try_from( - dunce::canonicalize(cfg.cwd.join("AGENTS.md")).expect("canonical crate doc path"), - ) - .expect("absolute crate doc path"); + let root_agents = repo.path().join("AGENTS.md").abs(); + let crate_agents = cfg.cwd.join("AGENTS.md"); let expected = LoadedAgentsMd { entries: vec![ InstructionEntry { @@ -434,14 +425,8 @@ async fn project_root_markers_are_honored_for_agents_discovery() { cfg.cwd = nested.abs(); let discovery = agents_md_paths(&cfg).await.expect("discover paths"); - let expected_parent = AbsolutePathBuf::try_from( - dunce::canonicalize(root.path().join("AGENTS.md")).expect("canonical parent doc path"), - ) - .expect("absolute parent doc path"); - let expected_child = AbsolutePathBuf::try_from( - dunce::canonicalize(cfg.cwd.join("AGENTS.md")).expect("canonical child doc path"), - ) - .expect("absolute child doc path"); + let expected_parent = root.path().join("AGENTS.md").abs(); + let expected_child = cfg.cwd.join("AGENTS.md"); assert_eq!(discovery.len(), 2); assert_eq!(discovery[0], expected_parent); assert_eq!(discovery[1], expected_child); @@ -450,6 +435,26 @@ async fn project_root_markers_are_honored_for_agents_discovery() { assert_eq!(res, "parent doc\n\nchild doc"); } +#[tokio::test] +async fn agents_md_paths_preserve_symlinked_cwd() { + let tmp = tempfile::tempdir().expect("tempdir"); + let target = tmp.path().join("target"); + fs::create_dir(&target).unwrap(); + fs::write(target.join("AGENTS.md"), "project doc").unwrap(); + + let linked_cwd = tmp.path().join("linked"); + create_directory_symlink(&target, &linked_cwd); + + let mut cfg = make_config(&tmp, /*limit*/ 4096, /*instructions*/ None).await; + cfg.cwd = linked_cwd.abs(); + + let discovery = agents_md_paths(&cfg).await.expect("discover paths"); + assert_eq!(discovery, vec![cfg.cwd.join("AGENTS.md")]); + + let res = get_user_instructions(&cfg).await.expect("doc expected"); + assert_eq!(res, "project doc"); +} + #[tokio::test] async fn child_agents_message_after_global_instructions_uses_plain_separator() { let tmp = tempfile::tempdir().expect("tempdir"); @@ -497,10 +502,7 @@ async fn instruction_sources_include_global_before_agents_md_docs() { .user_instructions_with_fs(LOCAL_FS.as_ref(), &mut warnings) .await .expect("instructions expected"); - let project_agents = AbsolutePathBuf::try_from( - dunce::canonicalize(cfg.cwd.join("AGENTS.md")).expect("canonical project doc path"), - ) - .expect("absolute project doc path"); + let project_agents = cfg.cwd.join("AGENTS.md"); let expected = LoadedAgentsMd { entries: vec![ @@ -541,10 +543,7 @@ async fn child_agents_message_after_project_docs_is_not_an_instruction_source() .user_instructions_with_fs(LOCAL_FS.as_ref(), &mut warnings) .await .expect("instructions expected"); - let project_agents = AbsolutePathBuf::try_from( - dunce::canonicalize(cfg.cwd.join("AGENTS.md")).expect("canonical project doc path"), - ) - .expect("absolute project doc path"); + let project_agents = cfg.cwd.join("AGENTS.md"); let expected = LoadedAgentsMd { entries: vec![ diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index be07b803f..94d72d619 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -19,6 +19,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; pub use codex_utils_absolute_path::test_support::PathBufExt; pub use codex_utils_absolute_path::test_support::PathExt; use regex_lite::Regex; +use std::path::Path; use std::path::PathBuf; pub mod apps_test_server; @@ -110,6 +111,20 @@ pub fn test_absolute_path(unix_path: &str) -> AbsolutePathBuf { test_absolute_path_with_windows(unix_path, /*windows_path*/ None) } +#[cfg(unix)] +#[allow(clippy::expect_used)] +pub fn create_directory_symlink(source: &Path, link: &Path) { + std::os::unix::fs::symlink(source, link).expect("create directory symlink"); +} + +#[cfg(windows)] +#[allow(clippy::expect_used)] +pub fn create_directory_symlink(source: &Path, link: &Path) { + // Running this test locally may require Windows Developer Mode or an elevated process. + std::os::windows::fs::symlink_dir(source, link) + .expect("create directory symlink; enable Developer Mode or run the test elevated"); +} + pub trait TempDirExt { fn abs(&self) -> AbsolutePathBuf; } diff --git a/codex-rs/core/tests/suite/agents_md.rs b/codex-rs/core/tests/suite/agents_md.rs index 318c288dc..3fa2136ec 100644 --- a/codex-rs/core/tests/suite/agents_md.rs +++ b/codex-rs/core/tests/suite/agents_md.rs @@ -1,6 +1,7 @@ use anyhow::Result; use codex_exec_server::CreateDirectoryOptions; use codex_utils_absolute_path::AbsolutePathBuf; +use core_test_support::create_directory_symlink; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; @@ -8,6 +9,7 @@ use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::test_codex::TestCodexBuilder; use core_test_support::test_codex::test_codex; +use pretty_assertions::assert_eq; use std::sync::Arc; use tempfile::TempDir; @@ -143,6 +145,86 @@ async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn symlinked_cwd_uses_logical_parent_for_agents_discovery() -> Result<()> { + let server = start_mock_server().await; + let resp_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + + let mut builder = test_codex() + .with_config(|config| { + config.cwd = config.cwd.join("logical-repo/workspace"); + }) + .with_workspace_setup(|cwd, _fs| async move { + // Construct two sibling repositories with the configured cwd as a + // directory symlink from the logical repository into the physical + // repository: + // + // test-root/ + // |-- logical-repo/ + // | |-- .git + // | |-- AGENTS.md ("logical parent doc") + // | `-- workspace ------------> physical-repo/workspace/ + // `-- physical-repo/ + // |-- .git + // |-- AGENTS.md ("physical parent doc") + // `-- workspace/ + // `-- AGENTS.md ("workspace doc") + // + // Discovery should walk the lexical path through logical-repo, + // while opening logical-repo/workspace/AGENTS.md still follows the + // symlink into physical-repo/workspace. + let logical_root = cwd.parent().expect("symlink should have a parent"); + let test_root = logical_root + .parent() + .expect("logical repository should have a parent"); + let physical_root = test_root.join("physical-repo"); + let physical_workspace = physical_root.join("workspace"); + + std::fs::create_dir_all(logical_root.as_path())?; + std::fs::write(logical_root.join(".git"), "")?; + std::fs::write(logical_root.join("AGENTS.md"), "logical parent doc")?; + + std::fs::create_dir_all(physical_workspace.as_path())?; + std::fs::write(physical_root.join(".git"), "")?; + std::fs::write(physical_root.join("AGENTS.md"), "physical parent doc")?; + std::fs::write(physical_workspace.join("AGENTS.md"), "workspace doc")?; + + create_directory_symlink(physical_workspace.as_path(), cwd.as_path()); + Ok(()) + }); + let test = builder.build(&server).await?; + let logical_root = test + .config + .cwd + .parent() + .expect("symlink should have a parent"); + + assert_eq!( + test.codex.instruction_sources().await, + vec![ + logical_root.join("AGENTS.md"), + test.config.cwd.join("AGENTS.md") + ] + ); + + test.submit_turn("hello").await?; + let instructions = resp_mock + .single_request() + .message_input_texts("user") + .into_iter() + .find(|text| text.starts_with("# AGENTS.md instructions for ")) + .expect("instructions message"); + assert!(instructions.contains("logical parent doc")); + assert!(instructions.contains("workspace doc")); + assert!(!instructions.contains("physical parent doc")); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn selected_environment_sources_match_model_visible_instructions() -> Result<()> { let server = start_mock_server().await; @@ -167,13 +249,7 @@ async fn selected_environment_sources_match_model_visible_instructions() -> Resu Ok::<(), anyhow::Error>(()) }); let test = builder.build_with_remote_env(&server).await?; - let project_agents = test - .fs() - .canonicalize( - &test.executor_environment().cwd().join("AGENTS.md"), - /*sandbox*/ None, - ) - .await?; + let project_agents = test.config.cwd.join("AGENTS.md"); let global_agents = AbsolutePathBuf::try_from(global_agents).expect("absolute path"); assert_eq!(