mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Cap project instructions across environments (#37424)
## Why Applying `project_doc_max_bytes` independently to every selected environment allows the total project instruction payload to grow with the number of environments. ## What changed Treat `project_doc_max_bytes` as one shared byte budget. Consume it in selected environment order, truncate the entry that reaches the remaining limit, and skip later environments once the budget is exhausted. Update the configuration descriptions to document the aggregate limit. ## Testing Add unit and integration coverage for partial and exhausted budgets across multiple environments. GitOrigin-RevId: 4664bdf1f23b66986349d503f44cf51560b9024f
This commit is contained in:
1 parent
95c7265e84
commit
85e0661c3b
6 files changed
+104
-18
No files matched your search
@@ -282,7 +282,7 @@ pub struct ConfigToml {
|
|||||||
#[serde(default, deserialize_with = "deserialize_model_providers")]
|
#[serde(default, deserialize_with = "deserialize_model_providers")]
|
||||||
pub model_providers: HashMap<String, ModelProviderInfo>,
|
pub model_providers: HashMap<String, ModelProviderInfo>,
|
||||||
|
|
||||||
/// Maximum number of bytes to include from an AGENTS.md project doc file.
|
/// Maximum total bytes of project instruction content across all selected environments.
|
||||||
#[serde(default = "default_project_doc_max_bytes")]
|
#[serde(default = "default_project_doc_max_bytes")]
|
||||||
pub project_doc_max_bytes: Option<usize>,
|
pub project_doc_max_bytes: Option<usize>,
|
||||||
|
|
||||||
|
|||||||
@@ -5753,7 +5753,7 @@
|
|||||||
},
|
},
|
||||||
"project_doc_max_bytes": {
|
"project_doc_max_bytes": {
|
||||||
"default": 32768,
|
"default": 32768,
|
||||||
"description": "Maximum number of bytes to include from an AGENTS.md project doc file.",
|
"description": "Maximum total bytes of project instruction content across all selected environments.",
|
||||||
"format": "uint",
|
"format": "uint",
|
||||||
"minimum": 0.0,
|
"minimum": 0.0,
|
||||||
"type": "integer"
|
"type": "integer"
|
||||||
|
|||||||
@@ -55,17 +55,28 @@ pub(crate) async fn load_project_instructions(
|
|||||||
environments: &TurnEnvironmentSnapshot,
|
environments: &TurnEnvironmentSnapshot,
|
||||||
) -> Option<LoadedAgentsMd> {
|
) -> Option<LoadedAgentsMd> {
|
||||||
let mut loaded = LoadedAgentsMd::from_user_instructions(user_instructions);
|
let mut loaded = LoadedAgentsMd::from_user_instructions(user_instructions);
|
||||||
|
let mut remaining = config.project_doc_max_bytes;
|
||||||
for turn_environment in environments.turn_environments() {
|
for turn_environment in environments.turn_environments() {
|
||||||
|
if remaining == 0 {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
let filesystem = turn_environment.environment.get_filesystem();
|
let filesystem = turn_environment.environment.get_filesystem();
|
||||||
match read_agents_md(
|
match read_agents_md(
|
||||||
config,
|
config,
|
||||||
filesystem.as_ref(),
|
filesystem.as_ref(),
|
||||||
&turn_environment.environment_id,
|
&turn_environment.environment_id,
|
||||||
turn_environment.cwd(),
|
turn_environment.cwd(),
|
||||||
|
remaining,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(Some(docs)) => loaded.entries.extend(docs.entries),
|
Ok(Some(docs)) => {
|
||||||
|
for entry in docs.entries {
|
||||||
|
remaining = remaining.saturating_sub(entry.contents.len());
|
||||||
|
loaded.entries.push(entry);
|
||||||
|
}
|
||||||
|
}
|
||||||
Ok(None) => {}
|
Ok(None) => {}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
error!(
|
error!(
|
||||||
@@ -90,9 +101,8 @@ async fn read_agents_md(
|
|||||||
fs: &dyn ExecutorFileSystem,
|
fs: &dyn ExecutorFileSystem,
|
||||||
environment_id: &str,
|
environment_id: &str,
|
||||||
cwd: &PathUri,
|
cwd: &PathUri,
|
||||||
|
max_total: usize,
|
||||||
) -> io::Result<Option<LoadedAgentsMd>> {
|
) -> io::Result<Option<LoadedAgentsMd>> {
|
||||||
let max_total = config.project_doc_max_bytes;
|
|
||||||
|
|
||||||
if max_total == 0 {
|
if max_total == 0 {
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -675,7 +675,13 @@ async fn read_agents_md_propagates_metadata_errors() {
|
|||||||
};
|
};
|
||||||
|
|
||||||
let cwd = config.cwd.clone();
|
let cwd = config.cwd.clone();
|
||||||
let err = read_agents_md(&config.config, &fs, "local", &PathUri::from_abs_path(&cwd))
|
let err = read_agents_md(
|
||||||
|
&config.config,
|
||||||
|
&fs,
|
||||||
|
"local",
|
||||||
|
&PathUri::from_abs_path(&cwd),
|
||||||
|
config.project_doc_max_bytes,
|
||||||
|
)
|
||||||
.await
|
.await
|
||||||
.expect_err("metadata error");
|
.expect_err("metadata error");
|
||||||
|
|
||||||
@@ -694,7 +700,13 @@ async fn read_agents_md_propagates_read_errors() {
|
|||||||
};
|
};
|
||||||
|
|
||||||
let cwd = config.cwd.clone();
|
let cwd = config.cwd.clone();
|
||||||
let err = read_agents_md(&config.config, &fs, "local", &PathUri::from_abs_path(&cwd))
|
let err = read_agents_md(
|
||||||
|
&config.config,
|
||||||
|
&fs,
|
||||||
|
"local",
|
||||||
|
&PathUri::from_abs_path(&cwd),
|
||||||
|
config.project_doc_max_bytes,
|
||||||
|
)
|
||||||
.await
|
.await
|
||||||
.expect_err("read error");
|
.expect_err("read error");
|
||||||
|
|
||||||
@@ -713,7 +725,13 @@ async fn read_agents_md_ignores_files_removed_after_discovery() {
|
|||||||
};
|
};
|
||||||
|
|
||||||
let cwd = config.cwd.clone();
|
let cwd = config.cwd.clone();
|
||||||
let loaded = read_agents_md(&config.config, &fs, "local", &PathUri::from_abs_path(&cwd))
|
let loaded = read_agents_md(
|
||||||
|
&config.config,
|
||||||
|
&fs,
|
||||||
|
"local",
|
||||||
|
&PathUri::from_abs_path(&cwd),
|
||||||
|
config.project_doc_max_bytes,
|
||||||
|
)
|
||||||
.await
|
.await
|
||||||
.expect("removed file is recoverable");
|
.expect("removed file is recoverable");
|
||||||
|
|
||||||
@@ -1175,12 +1193,12 @@ async fn primary_only_project_doc_preserves_legacy_layout_with_multiple_bound_en
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn project_doc_byte_limit_is_applied_independently_per_environment() {
|
async fn project_doc_byte_limit_is_shared_across_environments() {
|
||||||
let primary = tempfile::tempdir().expect("primary tempdir");
|
let primary = tempfile::tempdir().expect("primary tempdir");
|
||||||
let secondary = tempfile::tempdir().expect("secondary tempdir");
|
let secondary = tempfile::tempdir().expect("secondary tempdir");
|
||||||
fs::write(primary.path().join("AGENTS.md"), "ABCDE").unwrap();
|
fs::write(primary.path().join("AGENTS.md"), "ABCDE").unwrap();
|
||||||
fs::write(secondary.path().join("AGENTS.md"), "VWXYZ").unwrap();
|
fs::write(secondary.path().join("AGENTS.md"), "VWXYZ").unwrap();
|
||||||
let config = make_config(&primary, /*limit*/ 3, /*instructions*/ None).await;
|
let config = make_config(&primary, /*limit*/ 7, /*instructions*/ None).await;
|
||||||
let environments = resolved_local_environments([
|
let environments = resolved_local_environments([
|
||||||
("primary", config.cwd.clone()),
|
("primary", config.cwd.clone()),
|
||||||
("secondary", secondary.abs()),
|
("secondary", secondary.abs()),
|
||||||
@@ -1194,7 +1212,7 @@ async fn project_doc_byte_limit_is_applied_independently_per_environment() {
|
|||||||
assert_eq!(
|
assert_eq!(
|
||||||
loaded.text(),
|
loaded.text(),
|
||||||
format!(
|
format!(
|
||||||
"for `primary` with root {}\n\nABC\n\nfor `secondary` with root {}\n\nVWX",
|
"for `primary` with root {}\n\nABCDE\n\nfor `secondary` with root {}\n\nVW",
|
||||||
primary.path().display(),
|
primary.path().display(),
|
||||||
secondary.path().display()
|
secondary.path().display()
|
||||||
)
|
)
|
||||||
@@ -1202,9 +1220,7 @@ async fn project_doc_byte_limit_is_applied_independently_per_environment() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn multiple_environments_can_exceed_single_environment_project_doc_limit() {
|
async fn full_primary_environment_budget_excludes_later_environment_docs() {
|
||||||
// TODO(anp): Add an aggregate cap across environments instead of allowing the combined
|
|
||||||
// project instructions to grow by one full per-environment budget for every binding.
|
|
||||||
const LIMIT: usize = 8;
|
const LIMIT: usize = 8;
|
||||||
let primary = tempfile::tempdir().expect("primary tempdir");
|
let primary = tempfile::tempdir().expect("primary tempdir");
|
||||||
let secondary = tempfile::tempdir().expect("secondary tempdir");
|
let secondary = tempfile::tempdir().expect("secondary tempdir");
|
||||||
@@ -1232,10 +1248,9 @@ async fn multiple_environments_can_exceed_single_environment_project_doc_limit()
|
|||||||
.map(|entry| entry.contents.len())
|
.map(|entry| entry.contents.len())
|
||||||
.sum::<usize>();
|
.sum::<usize>();
|
||||||
|
|
||||||
assert_eq!(project_bytes, LIMIT * 2);
|
assert_eq!(project_bytes, LIMIT);
|
||||||
assert!(project_bytes > config.project_doc_max_bytes);
|
|
||||||
assert!(loaded.text().contains(&primary_doc));
|
assert!(loaded.text().contains(&primary_doc));
|
||||||
assert!(loaded.text().contains(&secondary_doc));
|
assert!(!loaded.text().contains(&secondary_doc));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
@@ -864,7 +864,7 @@ pub struct Config {
|
|||||||
/// Combined provider map (defaults plus user-defined providers).
|
/// Combined provider map (defaults plus user-defined providers).
|
||||||
pub model_providers: HashMap<String, ModelProviderInfo>,
|
pub model_providers: HashMap<String, ModelProviderInfo>,
|
||||||
|
|
||||||
/// Maximum number of bytes to include from an AGENTS.md project doc file.
|
/// Maximum total bytes of project instruction content across all selected environments.
|
||||||
pub project_doc_max_bytes: usize,
|
pub project_doc_max_bytes: usize,
|
||||||
|
|
||||||
/// Additional filenames to try when looking for project-level docs.
|
/// Additional filenames to try when looking for project-level docs.
|
||||||
|
|||||||
@@ -633,6 +633,67 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn multi_environment_project_instructions_share_one_byte_budget() -> Result<()> {
|
||||||
|
skip_if_no_network!(Ok(()));
|
||||||
|
skip_if_no_remote_env!(Ok(()));
|
||||||
|
|
||||||
|
let server = responses::start_mock_server().await;
|
||||||
|
let response_mock = responses::mount_sse_once(
|
||||||
|
&server,
|
||||||
|
responses::sse(vec![
|
||||||
|
responses::ev_response_created("multi-env-budget-response"),
|
||||||
|
responses::ev_completed("multi-env-budget-response"),
|
||||||
|
]),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let local_root = TempDir::new()?;
|
||||||
|
std::fs::write(local_root.path().join(GLOBAL_AGENTS_FILENAME), "VWXYZ")?;
|
||||||
|
let mut builder = test_codex()
|
||||||
|
.with_config(|config| config.project_doc_max_bytes = 7)
|
||||||
|
.with_workspace_setup(|cwd, fs| async move {
|
||||||
|
fs.write_file(
|
||||||
|
&PathUri::from_host_native_path(cwd.join(GLOBAL_AGENTS_FILENAME))?,
|
||||||
|
b"ABCDE".to_vec(),
|
||||||
|
/*sandbox*/ None,
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
Ok(())
|
||||||
|
});
|
||||||
|
let test = builder.build_with_remote_and_local_env(&server).await?;
|
||||||
|
let thread = test
|
||||||
|
.thread_manager
|
||||||
|
.start_thread(StartThreadOptions {
|
||||||
|
environments: Some(vec![
|
||||||
|
TurnEnvironmentSelection {
|
||||||
|
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
||||||
|
cwd: PathUri::from_abs_path(&test.config.cwd),
|
||||||
|
workspace_roots: vec![PathUri::from_abs_path(&test.config.cwd)],
|
||||||
|
},
|
||||||
|
TurnEnvironmentSelection {
|
||||||
|
environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
|
||||||
|
cwd: PathUri::from_host_native_path(local_root.path())?,
|
||||||
|
workspace_roots: vec![PathUri::from_host_native_path(local_root.path())?],
|
||||||
|
},
|
||||||
|
]),
|
||||||
|
..StartThreadOptions::new(test.config.clone())
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
submit_thread_turn(&thread.thread, "inspect the shared AGENTS.md budget").await?;
|
||||||
|
|
||||||
|
let contents = format!(
|
||||||
|
"for `{REMOTE_ENVIRONMENT_ID}` with root {}\n\nABCDE\n\nfor `{LOCAL_ENVIRONMENT_ID}` with root {}\n\nVW",
|
||||||
|
PathUri::from_abs_path(&test.config.cwd).inferred_native_path_string(),
|
||||||
|
local_root.path().display(),
|
||||||
|
);
|
||||||
|
let expected =
|
||||||
|
format!("# AGENTS.md instructions\n\n<INSTRUCTIONS>\n{contents}\n</INSTRUCTIONS>");
|
||||||
|
assert_single_instruction_fragment(&response_mock.single_request(), &expected);
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapshot() -> Result<()> {
|
async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapshot() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|||||||
Reference in new issue
Block a user