-
Notifications
You must be signed in to change notification settings - Fork 15.4k
core: make AGENTS.md react to environment changes #29977
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| use crate::agents_md::LoadedAgentsMd; | ||
| use crate::agents_md::load_project_instructions; | ||
| use crate::config::Config; | ||
| use crate::environment_selection::TurnEnvironmentSnapshot; | ||
| use codex_extension_api::UserInstructions; | ||
| use codex_protocol::protocol::TurnEnvironmentSelection; | ||
| use std::sync::Arc; | ||
| use tokio::sync::Mutex; | ||
|
|
||
| /// Owns the inputs and cached result of AGENTS.md discovery for a session. | ||
| pub(crate) struct AgentsMdManager { | ||
| user_instructions: Option<UserInstructions>, | ||
| cache: Mutex<AgentsMdCache>, | ||
| } | ||
|
|
||
| #[derive(Default)] | ||
| struct AgentsMdCache { | ||
| selections: Option<Vec<TurnEnvironmentSelection>>, | ||
| loaded: Option<Arc<LoadedAgentsMd>>, | ||
| } | ||
|
|
||
| impl AgentsMdManager { | ||
| pub(crate) fn new(user_instructions: Option<UserInstructions>) -> Self { | ||
| Self { | ||
| user_instructions: user_instructions | ||
| .filter(|instructions| !instructions.text.trim().is_empty()), | ||
| cache: Mutex::new(AgentsMdCache::default()), | ||
| } | ||
| } | ||
|
|
||
| pub(crate) async fn refresh(&self, config: &Config, environments: &TurnEnvironmentSnapshot) { | ||
| let selections = environments.to_selections(); | ||
| if self.cache.lock().await.selections.as_ref() == Some(&selections) { | ||
| return; | ||
| } | ||
|
|
||
| let loaded = | ||
| load_project_instructions(config, self.user_instructions.clone(), environments) | ||
| .await | ||
| .map(Arc::new); | ||
| let mut cache = self.cache.lock().await; | ||
| cache.selections = Some(selections); | ||
| cache.loaded = loaded; | ||
| } | ||
|
|
||
| pub(crate) async fn get_loaded(&self) -> Option<Arc<LoadedAgentsMd>> { | ||
| self.cache.lock().await.loaded.clone() | ||
| } | ||
|
|
||
| pub(crate) fn user_instructions(&self) -> Option<UserInstructions> { | ||
| self.user_instructions.clone() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| use super::WorldStateSection; | ||
| use crate::agents_md::LoadedAgentsMd; | ||
| use crate::context::ContextualUserFragment; | ||
| use crate::context::UserInstructions; | ||
| use serde::Deserialize; | ||
| use serde::Serialize; | ||
|
|
||
| const REPLACEMENT_NOTICE: &str = | ||
| "These AGENTS.md instructions replace all previously provided AGENTS.md instructions."; | ||
| const REMOVAL_NOTICE: &str = "The previously provided AGENTS.md instructions no longer apply."; | ||
|
|
||
| /// The AGENTS.md instructions currently visible to the model. | ||
| #[derive(Clone, Debug, Default)] | ||
| pub(crate) struct AgentsMdState { | ||
| instructions: Option<UserInstructions>, | ||
| } | ||
|
|
||
| /// Persisted model-visible AGENTS.md state, without filesystem provenance. | ||
| #[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq, Serialize)] | ||
| pub(crate) struct AgentsMdSnapshot { | ||
| directory: Option<String>, | ||
| text: Option<String>, | ||
| } | ||
|
|
||
| impl AgentsMdState { | ||
| pub(crate) fn new(loaded: Option<&LoadedAgentsMd>) -> Self { | ||
| Self { | ||
| instructions: loaded.map(LoadedAgentsMd::contextual_user_fragment), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl WorldStateSection for AgentsMdState { | ||
| const ID: &'static str = "agents_md"; | ||
| type Snapshot = AgentsMdSnapshot; | ||
|
|
||
| fn snapshot(&self) -> Self::Snapshot { | ||
| match &self.instructions { | ||
| Some(instructions) => AgentsMdSnapshot { | ||
| directory: instructions.directory.clone(), | ||
| text: Some(instructions.text.clone()), | ||
| }, | ||
| None => AgentsMdSnapshot::default(), | ||
| } | ||
| } | ||
|
|
||
| fn render_diff( | ||
| &self, | ||
| previous: Option<&Self::Snapshot>, | ||
| ) -> Option<Box<dyn ContextualUserFragment>> { | ||
| let current = self.snapshot(); | ||
| if previous == Some(¤t) { | ||
| return None; | ||
| } | ||
|
|
||
| let previous_instructions = previous.and_then(|state| state.text.as_ref()); | ||
| let instructions = match (&self.instructions, previous_instructions) { | ||
| (Some(instructions), Some(_)) => UserInstructions { | ||
| directory: instructions.directory.clone(), | ||
| text: format!("{REPLACEMENT_NOTICE}\n\n{}", instructions.text), | ||
|
sayan-oai marked this conversation as resolved.
|
||
| }, | ||
| (Some(instructions), None) => instructions.clone(), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When resuming a rollout written before this change, the prompt history already contains the old AGENTS.md contextual user message, but the persisted AGENTS.md reference: AGENTS.md:L102-L110 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. coming in stacked pr |
||
| (None, Some(_)) => UserInstructions { | ||
| directory: None, | ||
| text: REMOVAL_NOTICE.to_string(), | ||
| }, | ||
| (None, None) => return None, | ||
| }; | ||
| Some(Box::new(instructions)) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| #[path = "agents_md_tests.rs"] | ||
| mod tests; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| use super::*; | ||
| use crate::context::world_state::WorldState; | ||
| use codex_protocol::models::ContentItem; | ||
| use codex_protocol::models::ResponseItem; | ||
| use pretty_assertions::assert_eq; | ||
| use serde_json::json; | ||
|
|
||
| #[test] | ||
| fn renders_full_state_and_omits_unchanged_state() { | ||
| let loaded = LoadedAgentsMd::from_text_for_testing("use the project formatter"); | ||
| let mut state = WorldState::default(); | ||
| state.add_section(AgentsMdState::new(Some(&loaded))); | ||
|
|
||
| assert_eq!( | ||
| vec![user_message( | ||
| "# AGENTS.md instructions\n\n<INSTRUCTIONS>\nuse the project formatter\n</INSTRUCTIONS>", | ||
| )], | ||
| render_fragments(state.render_full()), | ||
| ); | ||
| assert_eq!( | ||
| Vec::<ResponseItem>::new(), | ||
| render_fragments(state.render_diff(&state.snapshot())) | ||
| ); | ||
| assert_eq!( | ||
| state.snapshot().into_value(), | ||
| json!({"agents_md": {"text": "use the project formatter"}}), | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn changed_and_removed_state_supersedes_previous_instructions() { | ||
| let previous_loaded = LoadedAgentsMd::from_text_for_testing("old instructions"); | ||
| let mut previous = WorldState::default(); | ||
| previous.add_section(AgentsMdState::new(Some(&previous_loaded))); | ||
|
|
||
| let current_loaded = LoadedAgentsMd::from_text_for_testing("new instructions"); | ||
| let mut current = WorldState::default(); | ||
| current.add_section(AgentsMdState::new(Some(¤t_loaded))); | ||
| assert_eq!( | ||
| vec![user_message( | ||
| "# AGENTS.md instructions\n\n<INSTRUCTIONS>\nThese AGENTS.md instructions replace all previously provided AGENTS.md instructions.\n\nnew instructions\n</INSTRUCTIONS>", | ||
| )], | ||
| render_fragments(current.render_diff(&previous.snapshot())), | ||
| ); | ||
|
|
||
| let mut removed = WorldState::default(); | ||
| removed.add_section(AgentsMdState::default()); | ||
| assert_eq!( | ||
| vec![user_message( | ||
| "# AGENTS.md instructions\n\n<INSTRUCTIONS>\nThe previously provided AGENTS.md instructions no longer apply.\n</INSTRUCTIONS>", | ||
| )], | ||
| render_fragments(removed.render_diff(¤t.snapshot())), | ||
| ); | ||
| } | ||
|
|
||
| fn render_fragments(fragments: Vec<Box<dyn ContextualUserFragment>>) -> Vec<ResponseItem> { | ||
| fragments | ||
| .into_iter() | ||
| .map(ContextualUserFragment::into_boxed_response_item) | ||
| .collect() | ||
| } | ||
|
|
||
| fn user_message(text: &str) -> ResponseItem { | ||
| ResponseItem::Message { | ||
| id: None, | ||
| role: "user".to_string(), | ||
| content: vec![ContentItem::InputText { | ||
| text: text.to_string(), | ||
| }], | ||
| phase: None, | ||
| internal_chat_message_metadata_passthrough: None, | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.