mirror of
https://github.com/openai/codex.git
synced 2026-10-31 19:28:49 +00:00
Hoist AGENTS.md loading out of core sessions
This commit is contained in:
@@ -474,6 +474,7 @@ mod search;
|
||||
mod thread_processor;
|
||||
mod token_usage_replay;
|
||||
mod turn_processor;
|
||||
mod user_instructions;
|
||||
mod windows_sandbox_processor;
|
||||
|
||||
pub(crate) use account_processor::AccountRequestProcessor;
|
||||
@@ -508,6 +509,7 @@ use crate::thread_state::ThreadState;
|
||||
use crate::thread_state::ThreadStateManager;
|
||||
use token_usage_replay::latest_token_usage_turn_id_from_rollout_items;
|
||||
use token_usage_replay::send_thread_token_usage_update_to_connection;
|
||||
use user_instructions::load_thread_user_instructions;
|
||||
|
||||
mod config_errors;
|
||||
mod request_errors;
|
||||
|
||||
@@ -42,6 +42,7 @@ use std::path::PathBuf;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use super::ConfigRequestProcessor;
|
||||
use super::load_thread_user_instructions;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct ExternalAgentConfigRequestProcessor {
|
||||
@@ -283,7 +284,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
title,
|
||||
rollout_items,
|
||||
} = session;
|
||||
let config = self
|
||||
let mut config = self
|
||||
.config_manager
|
||||
.load_with_overrides(
|
||||
/*request_overrides*/ None,
|
||||
@@ -301,6 +302,9 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
let environments = self
|
||||
.thread_manager
|
||||
.default_environment_selections(&config.cwd);
|
||||
load_thread_user_instructions(self.thread_manager.as_ref(), &mut config, &environments)
|
||||
.await
|
||||
.map_err(|err| internal_error(format!("failed to load user instructions: {err}")))?;
|
||||
let imported_thread = self
|
||||
.thread_manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
|
||||
@@ -1042,6 +1042,13 @@ impl ThreadRequestProcessor {
|
||||
.thread_manager
|
||||
.default_environment_selections(&config.cwd)
|
||||
});
|
||||
load_thread_user_instructions(
|
||||
listener_task_context.thread_manager.as_ref(),
|
||||
&mut config,
|
||||
&environments,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| invalid_request(err.to_string()))?;
|
||||
let dynamic_tools = dynamic_tools.unwrap_or_default();
|
||||
let core_dynamic_tools = if dynamic_tools.is_empty() {
|
||||
Vec::new()
|
||||
@@ -2505,7 +2512,7 @@ impl ThreadRequestProcessor {
|
||||
.await;
|
||||
|
||||
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
|
||||
let config = match self
|
||||
let mut config = match self
|
||||
.config_manager
|
||||
.load_for_cwd(request_overrides, typesafe_overrides, history_cwd)
|
||||
.await
|
||||
@@ -2517,6 +2524,12 @@ impl ThreadRequestProcessor {
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
let environments = self
|
||||
.thread_manager
|
||||
.default_environment_selections(&config.cwd);
|
||||
load_thread_user_instructions(self.thread_manager.as_ref(), &mut config, &environments)
|
||||
.await
|
||||
.map_err(|err| invalid_request(err.to_string()))?;
|
||||
|
||||
let response_history = thread_history.clone();
|
||||
|
||||
@@ -3217,11 +3230,17 @@ impl ThreadRequestProcessor {
|
||||
);
|
||||
typesafe_overrides.ephemeral = ephemeral.then_some(true);
|
||||
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
|
||||
let config = self
|
||||
let mut config = self
|
||||
.config_manager
|
||||
.load_for_cwd(request_overrides, typesafe_overrides, history_cwd)
|
||||
.await
|
||||
.map_err(|err| config_load_error(&err))?;
|
||||
let environments = self
|
||||
.thread_manager
|
||||
.default_environment_selections(&config.cwd);
|
||||
load_thread_user_instructions(self.thread_manager.as_ref(), &mut config, &environments)
|
||||
.await
|
||||
.map_err(|err| invalid_request(err.to_string()))?;
|
||||
|
||||
let fallback_model_provider = config.model_provider_id.clone();
|
||||
|
||||
|
||||
@@ -1080,6 +1080,12 @@ impl TurnRequestProcessor {
|
||||
if let Some(review_model) = &config.review_model {
|
||||
config.model = Some(review_model.clone());
|
||||
}
|
||||
let environments = self
|
||||
.thread_manager
|
||||
.default_environment_selections(&config.cwd);
|
||||
load_thread_user_instructions(self.thread_manager.as_ref(), &mut config, &environments)
|
||||
.await
|
||||
.map_err(|err| invalid_request(err.to_string()))?;
|
||||
|
||||
let NewThread {
|
||||
thread_id,
|
||||
@@ -1089,7 +1095,7 @@ impl TurnRequestProcessor {
|
||||
.thread_manager
|
||||
.fork_thread_from_history(
|
||||
ForkSnapshot::Interrupted,
|
||||
config.clone(),
|
||||
config,
|
||||
InitialHistory::Resumed(ResumedHistory {
|
||||
conversation_id: parent_thread_id,
|
||||
history: parent_history.items,
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
use codex_core::AgentsMdManager;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::config::Config;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::Result as CodexResult;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
|
||||
/// Loads AGENTS.md instructions from the primary turn environment into
|
||||
/// `config` before the app server passes it to the thread manager.
|
||||
pub(super) async fn load_thread_user_instructions(
|
||||
thread_manager: &ThreadManager,
|
||||
config: &mut Config,
|
||||
environments: &[TurnEnvironmentSelection],
|
||||
) -> CodexResult<()> {
|
||||
let Some(primary_selection) = environments.first() else {
|
||||
config.user_instructions = None;
|
||||
return Ok(());
|
||||
};
|
||||
let environment = thread_manager
|
||||
.environment_manager()
|
||||
.get_environment(&primary_selection.environment_id)
|
||||
.ok_or_else(|| {
|
||||
CodexErr::InvalidRequest(format!(
|
||||
"unknown turn environment id `{}`",
|
||||
primary_selection.environment_id
|
||||
))
|
||||
})?;
|
||||
let mut warnings = Vec::new();
|
||||
let user_instructions = AgentsMdManager::new(config)
|
||||
.load_user_instructions(environment.as_ref(), &mut warnings)
|
||||
.await;
|
||||
config.startup_warnings.extend(warnings);
|
||||
config.user_instructions = user_instructions;
|
||||
Ok(())
|
||||
}
|
||||
@@ -1846,12 +1846,31 @@ async fn run_debug_prompt_input_command(
|
||||
additional_writable_roots: shared.add_dir,
|
||||
..Default::default()
|
||||
};
|
||||
let config = ConfigBuilder::default()
|
||||
let mut config = ConfigBuilder::default()
|
||||
.cli_overrides(cli_kv_overrides)
|
||||
.harness_overrides(overrides)
|
||||
.loader_overrides(loader_overrides)
|
||||
.build()
|
||||
.await?;
|
||||
let runtime_paths = codex_exec_server::ExecServerRuntimePaths::from_optional_paths(
|
||||
config.codex_self_exe.clone(),
|
||||
config.codex_linux_sandbox_exe.clone(),
|
||||
)?;
|
||||
let environment_manager = codex_exec_server::EnvironmentManager::from_codex_home(
|
||||
config.codex_home.clone(),
|
||||
Some(runtime_paths),
|
||||
)
|
||||
.await?;
|
||||
let mut warnings = Vec::new();
|
||||
config.user_instructions = match environment_manager.default_environment() {
|
||||
Some(environment) => {
|
||||
codex_core::AgentsMdManager::new(&config)
|
||||
.load_user_instructions(environment.as_ref(), &mut warnings)
|
||||
.await
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
config.startup_warnings.extend(warnings);
|
||||
|
||||
let mut input = shared
|
||||
.images
|
||||
|
||||
@@ -24,6 +24,7 @@ pub use codex_config::types::TuiKeymap;
|
||||
pub use codex_config::types::TuiNotificationSettings;
|
||||
pub use codex_config::types::TuiPetAnchor;
|
||||
pub use codex_config::types::UriBasedFileOpener;
|
||||
pub use codex_core::AgentsMdManager;
|
||||
pub use codex_core::CodexThread;
|
||||
pub use codex_core::ForkSnapshot;
|
||||
pub use codex_core::LoadedAgentsMd;
|
||||
|
||||
@@ -80,9 +80,12 @@ impl<'a> AgentsMdManager<'a> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Combines configured user instructions and AGENTS.md content into a
|
||||
/// single model-visible instruction string.
|
||||
pub(crate) async fn user_instructions(
|
||||
/// Loads project instructions from `environment` and combines them with
|
||||
/// the configured user instructions.
|
||||
///
|
||||
/// Callers should invoke this before constructing a thread and provide the
|
||||
/// returned value through [`Config::user_instructions`].
|
||||
pub async fn load_user_instructions(
|
||||
&self,
|
||||
environment: &Environment,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
|
||||
@@ -645,7 +645,7 @@ pub struct Config {
|
||||
/// Defaults to `false`.
|
||||
pub show_raw_agent_reasoning: bool,
|
||||
|
||||
/// User-provided instructions from AGENTS.md.
|
||||
/// Instructions preloaded from AGENTS.md.
|
||||
pub user_instructions: Option<LoadedAgentsMd>,
|
||||
|
||||
/// Base instructions override.
|
||||
|
||||
@@ -41,6 +41,7 @@ impl ResolvedTurnEnvironments {
|
||||
self.turn_environments.first()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn primary_environment(&self) -> Option<Arc<codex_exec_server::Environment>> {
|
||||
self.primary()
|
||||
.map(|environment| Arc::clone(&environment.environment))
|
||||
|
||||
@@ -285,7 +285,6 @@ use crate::SkillLoadOutcome;
|
||||
#[cfg(test)]
|
||||
use crate::SkillMetadata;
|
||||
use crate::SkillsManager;
|
||||
use crate::agents_md::AgentsMdManager;
|
||||
use crate::context::UserInstructions;
|
||||
use crate::exec_policy::ExecPolicyUpdateError;
|
||||
use crate::guardian::GuardianReviewSessionManager;
|
||||
@@ -474,7 +473,7 @@ impl Codex {
|
||||
|
||||
async fn spawn_internal(args: CodexSpawnArgs) -> CodexResult<CodexSpawnOk> {
|
||||
let CodexSpawnArgs {
|
||||
mut config,
|
||||
config,
|
||||
installation_id,
|
||||
auth_manager,
|
||||
models_manager,
|
||||
@@ -505,16 +504,7 @@ impl Codex {
|
||||
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
|
||||
let (tx_event, rx_event) = async_channel::unbounded();
|
||||
|
||||
let primary_environment = environment_selections.primary_environment();
|
||||
let mut user_instruction_warnings = Vec::new();
|
||||
let user_instructions = if let Some(primary_environment) = primary_environment {
|
||||
AgentsMdManager::new(&config)
|
||||
.user_instructions(primary_environment.as_ref(), &mut user_instruction_warnings)
|
||||
.await
|
||||
} else {
|
||||
None
|
||||
};
|
||||
config.startup_warnings.extend(user_instruction_warnings);
|
||||
let user_instructions = config.user_instructions.clone();
|
||||
|
||||
let exec_policy = if crate::guardian::is_guardian_reviewer_source(&session_source) {
|
||||
// Guardian review should rely on the built-in shell safety checks,
|
||||
|
||||
@@ -552,6 +552,7 @@ impl ThreadManager {
|
||||
Ok(subtree_thread_ids)
|
||||
}
|
||||
|
||||
/// Starts a thread using the provided config.
|
||||
pub async fn start_thread(&self, config: Config) -> CodexResult<NewThread> {
|
||||
// Box delegated thread-spawn futures so these convenience wrappers do
|
||||
// not inline the full spawn path into every caller's async state.
|
||||
|
||||
@@ -293,6 +293,35 @@ async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() {
|
||||
assert!(manager.list_thread_ids().await.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn start_thread_does_not_load_user_instructions_from_filesystem() {
|
||||
let temp_dir = tempdir().expect("tempdir");
|
||||
let mut config = test_config().await;
|
||||
config.codex_home = temp_dir.path().join("codex-home").abs();
|
||||
config.cwd = temp_dir.path().join("workspace").abs();
|
||||
config.user_instructions = None;
|
||||
std::fs::create_dir_all(&config.codex_home).expect("create codex home");
|
||||
std::fs::create_dir_all(&config.cwd).expect("create workspace");
|
||||
// This file should be ignored because its contents were not preloaded into
|
||||
// the config passed to the thread manager.
|
||||
std::fs::write(config.cwd.join("AGENTS.md"), "workspace instructions")
|
||||
.expect("write AGENTS.md");
|
||||
|
||||
let manager = ThreadManager::with_models_provider_and_home_for_tests(
|
||||
CodexAuth::from_api_key("dummy"),
|
||||
config.model_provider.clone(),
|
||||
config.codex_home.to_path_buf(),
|
||||
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
|
||||
);
|
||||
|
||||
let thread = manager
|
||||
.start_thread(config)
|
||||
.await
|
||||
.expect("thread should start");
|
||||
|
||||
assert_eq!(thread.thread.instruction_sources().await, Vec::new());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn start_thread_rejects_explicit_local_environment_when_default_provider_is_disabled() {
|
||||
let temp_dir = tempdir().expect("tempdir");
|
||||
|
||||
@@ -13,6 +13,7 @@ use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use codex_config::CloudConfigBundleLoader;
|
||||
use codex_core::AgentsMdManager;
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::config::Config;
|
||||
@@ -457,13 +458,18 @@ impl TestCodexBuilder {
|
||||
|
||||
async fn build_from_config(
|
||||
&mut self,
|
||||
config: Config,
|
||||
mut config: Config,
|
||||
cwd: Arc<TempDir>,
|
||||
home: Arc<TempDir>,
|
||||
resume_from: Option<PathBuf>,
|
||||
test_env: TestEnv,
|
||||
environment_manager: Arc<codex_exec_server::EnvironmentManager>,
|
||||
) -> anyhow::Result<TestCodex> {
|
||||
let mut warnings = Vec::new();
|
||||
config.user_instructions = AgentsMdManager::new(&config)
|
||||
.load_user_instructions(test_env.environment(), &mut warnings)
|
||||
.await;
|
||||
config.startup_warnings.extend(warnings);
|
||||
let auth = self.auth.clone();
|
||||
let state_db = codex_core::init_state_db(&config).await;
|
||||
let thread_store = thread_store_from_config(&config, state_db.clone());
|
||||
|
||||
@@ -9,6 +9,7 @@ use crate::exec_approval::handle_exec_approval_request;
|
||||
use crate::outgoing_message::OutgoingMessageSender;
|
||||
use crate::outgoing_message::OutgoingNotificationMeta;
|
||||
use crate::patch_approval::handle_patch_approval_request;
|
||||
use codex_core::AgentsMdManager;
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::NewThread;
|
||||
use codex_core::ThreadManager;
|
||||
@@ -57,16 +58,26 @@ pub(crate) fn create_call_tool_result_with_thread_id(
|
||||
pub async fn run_codex_tool_session(
|
||||
id: RequestId,
|
||||
initial_prompt: String,
|
||||
config: CodexConfig,
|
||||
mut config: CodexConfig,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
thread_manager: Arc<ThreadManager>,
|
||||
running_requests_id_to_codex_uuid: Arc<Mutex<HashMap<RequestId, ThreadId>>>,
|
||||
) {
|
||||
let mut warnings = Vec::new();
|
||||
config.user_instructions = match thread_manager.environment_manager().default_environment() {
|
||||
Some(environment) => {
|
||||
AgentsMdManager::new(&config)
|
||||
.load_user_instructions(environment.as_ref(), &mut warnings)
|
||||
.await
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
config.startup_warnings.extend(warnings);
|
||||
let NewThread {
|
||||
thread_id,
|
||||
thread,
|
||||
session_configured,
|
||||
} = match thread_manager.start_thread(config.clone()).await {
|
||||
} = match thread_manager.start_thread(config).await {
|
||||
Ok(res) => res,
|
||||
Err(e) => {
|
||||
let result = CallToolResult::error(vec![Content::text(format!(
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use codex_core::AgentsMdManager;
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::ModelClient;
|
||||
use codex_core::NewThread;
|
||||
@@ -230,12 +231,26 @@ impl MemoryStartupContext {
|
||||
|
||||
pub(crate) async fn spawn_consolidation_agent(
|
||||
&self,
|
||||
config: Config,
|
||||
mut config: Config,
|
||||
prompt: Vec<UserInput>,
|
||||
) -> anyhow::Result<SpawnedConsolidationAgent> {
|
||||
let environments = self
|
||||
.thread_manager
|
||||
.default_environment_selections(&config.cwd);
|
||||
let mut warnings = Vec::new();
|
||||
config.user_instructions = match environments.first().and_then(|selection| {
|
||||
self.thread_manager
|
||||
.environment_manager()
|
||||
.get_environment(&selection.environment_id)
|
||||
}) {
|
||||
Some(environment) => {
|
||||
AgentsMdManager::new(&config)
|
||||
.load_user_instructions(environment.as_ref(), &mut warnings)
|
||||
.await
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
config.startup_warnings.extend(warnings);
|
||||
let NewThread {
|
||||
thread_id, thread, ..
|
||||
} = self
|
||||
|
||||
@@ -9,6 +9,7 @@ use anyhow::Context;
|
||||
use anyhow::bail;
|
||||
use clap::Parser;
|
||||
use codex_core_api::AbsolutePathBuf;
|
||||
use codex_core_api::AgentsMdManager;
|
||||
use codex_core_api::AltScreenMode;
|
||||
use codex_core_api::ApprovalsReviewer;
|
||||
use codex_core_api::Arg0DispatchPaths;
|
||||
@@ -105,7 +106,7 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
args.prompt.join(" ")
|
||||
};
|
||||
|
||||
let config = new_config(args.model, arg0_paths)?;
|
||||
let mut config = new_config(args.model, arg0_paths)?;
|
||||
let state_db = init_state_db(&config).await;
|
||||
|
||||
let auth_manager =
|
||||
@@ -120,6 +121,16 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
.await?,
|
||||
);
|
||||
let installation_id = resolve_installation_id(&config.codex_home).await?;
|
||||
let mut warnings = Vec::new();
|
||||
config.user_instructions = match environment_manager.default_environment() {
|
||||
Some(environment) => {
|
||||
AgentsMdManager::new(&config)
|
||||
.load_user_instructions(environment.as_ref(), &mut warnings)
|
||||
.await
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
config.startup_warnings.extend(warnings);
|
||||
let thread_manager = ThreadManager::new(
|
||||
&config,
|
||||
auth_manager,
|
||||
|
||||
Reference in New Issue
Block a user