diff --git a/AGENTS.md b/AGENTS.md index 608b48209c..85d61141ae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,7 +41,7 @@ In the codex-rs folder where the rust code lives: - Model-visible prompt context should go through the shared fragment abstractions described in `docs/model-visible-context.md`. - Every new model-visible fragment should implement `ModelVisibleContextFragment` and set `type Role`. - Turn-state model-visible context assembly should produce exactly two envelopes (one developer message + one contextual-user message) via the shared envelope builders. -- Contextual-user fragments should use the shared detection path in `model_visible_context`: implement `TaggedContextualUserFragment` when marker-based detection is sufficient, or implement `ContextualUserFragmentDetector` for custom matching (for example AGENTS.md dynamic tags). +- Contextual-user fragments should use the shared detection path in `model_visible_context`: implement `ContextualUserFragment`, prefer defining `markers()` when marker-based detection/wrapping is sufficient, and override `matches_contextual_user_text()` only for genuinely custom matching (for example AGENTS.md). - Use the developer envelope for developer guidance. Custom override text (for example config/app-server `developer_instructions`) should use `CustomDeveloperInstructions`; system-generated developer context should use typed fragments plus the neutral `developer_*_text` helpers rather than reusing the custom override type. - Use the contextual-user envelope for user-role contextual state or runtime markers such as AGENTS instructions, plugin instructions, environment context, skills, and shell-command markers. Contextual-user fragments must provide stable markers so history parsing treats them as contextual state rather than user intent. - Use `` specifically for environment facts derived from `TurnContext` that may need turn-to-turn diffs (`cwd`, `shell`, optional `current_date`, optional `timezone`, optional network allow/deny domain summaries). Do not put policy text, plugin/skill listings, or other guidance into ``; those should use dedicated fragments. diff --git a/codex-rs/core/src/arc_monitor_tests.rs b/codex-rs/core/src/arc_monitor_tests.rs index 0b5cdf3029..38217cefe9 100644 --- a/codex-rs/core/src/arc_monitor_tests.rs +++ b/codex-rs/core/src/arc_monitor_tests.rs @@ -71,11 +71,11 @@ async fn build_arc_monitor_request_includes_relevant_history_and_null_policies() .await; session .record_into_history( - &[ - crate::contextual_user_message::ENVIRONMENT_CONTEXT_FRAGMENT.into_message( - "\n/tmp\n".to_string(), - ), - ], + &[crate::model_visible_context::model_visible_message::< + crate::model_visible_context::ContextualUserContextRole, + >( + "\n/tmp\n".to_string(), + )], &turn_context, ) .await; diff --git a/codex-rs/core/src/environment_context.rs b/codex-rs/core/src/environment_context.rs index f32c6e276b..d959959f0f 100644 --- a/codex-rs/core/src/environment_context.rs +++ b/codex-rs/core/src/environment_context.rs @@ -1,8 +1,8 @@ use crate::codex::TurnContext; use crate::model_visible_context::ContextualUserContextRole; +use crate::model_visible_context::ContextualUserFragment; use crate::model_visible_context::ContextualUserFragmentMarkers; use crate::model_visible_context::ModelVisibleContextFragment; -use crate::model_visible_context::TaggedContextualUserFragment; use crate::model_visible_context::TurnContextDiffFragment; use crate::model_visible_context::TurnContextDiffParams; use crate::shell::Shell; @@ -134,8 +134,10 @@ impl ModelVisibleContextFragment for EnvironmentContext { } } -impl TaggedContextualUserFragment for EnvironmentContext { - const MARKERS: ContextualUserFragmentMarkers = Self::MARKERS; +impl ContextualUserFragment for EnvironmentContext { + fn markers() -> Option { + Some(Self::MARKERS) + } } impl TurnContextDiffFragment for EnvironmentContext { diff --git a/codex-rs/core/src/instructions/contextual_user_fragments.rs b/codex-rs/core/src/instructions/contextual_user_fragments.rs index f38138b772..ae9922e9ae 100644 --- a/codex-rs/core/src/instructions/contextual_user_fragments.rs +++ b/codex-rs/core/src/instructions/contextual_user_fragments.rs @@ -9,14 +9,13 @@ use serde::Serialize; use crate::codex::TurnContext; use crate::model_visible_context::ContextualUserContextRole; -use crate::model_visible_context::ContextualUserFragmentDetector; +use crate::model_visible_context::ContextualUserFragment; use crate::model_visible_context::ContextualUserFragmentMarkers; use crate::model_visible_context::ModelVisibleContextFragment; use crate::model_visible_context::PLUGINS_CLOSE_TAG; use crate::model_visible_context::PLUGINS_OPEN_TAG; use crate::model_visible_context::SKILL_CLOSE_TAG; use crate::model_visible_context::SKILL_OPEN_TAG; -use crate::model_visible_context::TaggedContextualUserFragment; use crate::model_visible_context::TurnContextDiffFragment; use crate::model_visible_context::TurnContextDiffParams; use codex_protocol::protocol::TurnContextItem; @@ -76,7 +75,7 @@ impl TurnContextDiffFragment for AgentsMdInstructions { } } -impl ContextualUserFragmentDetector for AgentsMdInstructions { +impl ContextualUserFragment for AgentsMdInstructions { fn matches_contextual_user_text(text: &str) -> bool { let trimmed = text.trim_start(); // TODO(ccunningham): Switch detection to the XML-ish wrapper once we @@ -102,16 +101,20 @@ impl ModelVisibleContextFragment for SkillInstructions { type Role = ContextualUserContextRole; fn render_text(&self) -> String { - Self::wrap_contextual_user_body(format!( + ::wrap_contextual_user_body(format!( "{}\n{}\n{}", self.name, self.path, self.contents )) } } -impl TaggedContextualUserFragment for SkillInstructions { - const MARKERS: ContextualUserFragmentMarkers = - ContextualUserFragmentMarkers::new(SKILL_OPEN_TAG, SKILL_CLOSE_TAG); +impl ContextualUserFragment for SkillInstructions { + fn markers() -> Option { + Some(ContextualUserFragmentMarkers::new( + SKILL_OPEN_TAG, + SKILL_CLOSE_TAG, + )) + } } // --------------------------------------------------------------------------- @@ -128,13 +131,17 @@ impl ModelVisibleContextFragment for PluginInstructions { type Role = ContextualUserContextRole; fn render_text(&self) -> String { - Self::wrap_contextual_user_body(self.text.clone()) + ::wrap_contextual_user_body(self.text.clone()) } } -impl TaggedContextualUserFragment for PluginInstructions { - const MARKERS: ContextualUserFragmentMarkers = - ContextualUserFragmentMarkers::new(PLUGINS_OPEN_TAG, PLUGINS_CLOSE_TAG); +impl ContextualUserFragment for PluginInstructions { + fn markers() -> Option { + Some(ContextualUserFragmentMarkers::new( + PLUGINS_OPEN_TAG, + PLUGINS_CLOSE_TAG, + )) + } } // --------------------------------------------------------------------------- @@ -181,7 +188,7 @@ mod tests { } )); assert!( - ::matches_contextual_user_text( + ::matches_contextual_user_text( "# AGENTS.md instructions for test_directory\n\n\ntest_text\n" ) ); @@ -215,12 +222,12 @@ mod tests { #[test] fn test_is_skill_instructions() { assert!( - ::matches_contextual_user_text( + ::matches_contextual_user_text( "\ndemo-skill\nskills/demo/SKILL.md\nbody\n" ) ); assert!( - !::matches_contextual_user_text( + !::matches_contextual_user_text( "regular text" ) ); diff --git a/codex-rs/core/src/model_visible_context.rs b/codex-rs/core/src/model_visible_context.rs index 58dffd50ae..76db9cfd6e 100644 --- a/codex-rs/core/src/model_visible_context.rs +++ b/codex-rs/core/src/model_visible_context.rs @@ -53,21 +53,20 @@ pub(crate) struct ContextualUserFragmentMarkers { end_marker: &'static str, } -pub(crate) trait ContextualUserFragmentDetector { - fn matches_contextual_user_text(text: &str) -> bool; -} +pub(crate) trait ContextualUserFragment { + fn markers() -> Option { + None + } -pub(crate) trait TaggedContextualUserFragment { - const MARKERS: ContextualUserFragmentMarkers; + fn matches_contextual_user_text(text: &str) -> bool { + Self::markers().is_some_and(|markers| markers.matches_text(text)) + } fn wrap_contextual_user_body(body: String) -> String { - Self::MARKERS.wrap_body(body) - } -} - -impl ContextualUserFragmentDetector for T { - fn matches_contextual_user_text(text: &str) -> bool { - T::MARKERS.matches_text(text) + let markers = Self::markers().expect( + "contextual-user fragments using wrap_contextual_user_body must define markers", + ); + markers.wrap_body(body) } } @@ -241,7 +240,7 @@ pub(crate) trait TurnContextDiffFragment: ModelVisibleContextFragment + Sized { ) -> Option; } -fn detect_contextual_user_fragment(text: &str) -> bool { +fn detect_contextual_user_fragment(text: &str) -> bool { F::matches_contextual_user_text(text) } @@ -251,7 +250,7 @@ fn build_contextual_user_turn_state_fragment( params: &TurnContextDiffParams<'_>, ) -> Option where - F: TurnContextDiffFragment, + F: TurnContextDiffFragment + ContextualUserFragment, { let fragment = F::build(turn_context, reference_context_item, params)?; Some(ContextualUserTextFragment::new(fragment.render_text())) @@ -263,8 +262,9 @@ where /// 1. Defining a typed fragment struct. /// 2. Implementing `ModelVisibleContextFragment` with /// `Role = ContextualUserContextRole`. -/// 3. Implementing contextual-user detection via -/// `TaggedContextualUserFragment` or `ContextualUserFragmentDetector`. +/// 3. Implementing `ContextualUserFragment` for detection. Prefer defining +/// `markers()` so the default matcher/wrapper behavior applies; override +/// `matches_contextual_user_text()` only for genuinely custom formats. /// 4. If the fragment is derived from turn state, implementing /// `TurnContextDiffFragment::build` and registering it with /// `Some(build_contextual_user_turn_state_fragment::)`. @@ -403,7 +403,7 @@ mod tests { #[test] fn marker_matching_ignores_plain_text() { assert!( - !::matches_contextual_user_text("plain text") + !::matches_contextual_user_text("plain text") ); } } diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index c23ea16d68..4c844ddf1d 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -24,11 +24,11 @@ use crate::codex::Session; use crate::codex::TurnContext; use crate::event_mapping::parse_turn_item; use crate::model_visible_context::ContextualUserContextRole; +use crate::model_visible_context::ContextualUserFragment; use crate::model_visible_context::ContextualUserFragmentMarkers; use crate::model_visible_context::ModelVisibleContextFragment; use crate::model_visible_context::TURN_ABORTED_CLOSE_TAG; use crate::model_visible_context::TURN_ABORTED_OPEN_TAG; -use crate::model_visible_context::TaggedContextualUserFragment; use crate::models_manager::manager::ModelsManager; use crate::protocol::EventMsg; use crate::protocol::TokenUsage; @@ -70,13 +70,17 @@ impl ModelVisibleContextFragment for TurnAbortedMarker { type Role = ContextualUserContextRole; fn render_text(&self) -> String { - Self::wrap_contextual_user_body(self.guidance.to_string()) + ::wrap_contextual_user_body(self.guidance.to_string()) } } -impl TaggedContextualUserFragment for TurnAbortedMarker { - const MARKERS: ContextualUserFragmentMarkers = - ContextualUserFragmentMarkers::new(TURN_ABORTED_OPEN_TAG, TURN_ABORTED_CLOSE_TAG); +impl ContextualUserFragment for TurnAbortedMarker { + fn markers() -> Option { + Some(ContextualUserFragmentMarkers::new( + TURN_ABORTED_OPEN_TAG, + TURN_ABORTED_CLOSE_TAG, + )) + } } fn emit_turn_network_proxy_metric( diff --git a/codex-rs/core/src/user_shell_command.rs b/codex-rs/core/src/user_shell_command.rs index 83b8ebaa64..360e0709da 100644 --- a/codex-rs/core/src/user_shell_command.rs +++ b/codex-rs/core/src/user_shell_command.rs @@ -5,9 +5,9 @@ use codex_protocol::models::ResponseItem; use crate::codex::TurnContext; use crate::exec::ExecToolCallOutput; use crate::model_visible_context::ContextualUserContextRole; +use crate::model_visible_context::ContextualUserFragment; use crate::model_visible_context::ContextualUserFragmentMarkers; use crate::model_visible_context::ModelVisibleContextFragment; -use crate::model_visible_context::TaggedContextualUserFragment; use crate::model_visible_context::USER_SHELL_COMMAND_CLOSE_TAG; use crate::model_visible_context::USER_SHELL_COMMAND_OPEN_TAG; use crate::tools::format_exec_output_str; @@ -19,11 +19,13 @@ fn format_duration_line(duration: Duration) -> String { pub(crate) struct UserShellCommandFragment; -impl TaggedContextualUserFragment for UserShellCommandFragment { - const MARKERS: ContextualUserFragmentMarkers = ContextualUserFragmentMarkers::new( - USER_SHELL_COMMAND_OPEN_TAG, - USER_SHELL_COMMAND_CLOSE_TAG, - ); +impl ContextualUserFragment for UserShellCommandFragment { + fn markers() -> Option { + Some(ContextualUserFragmentMarkers::new( + USER_SHELL_COMMAND_OPEN_TAG, + USER_SHELL_COMMAND_CLOSE_TAG, + )) + } } struct UserShellCommandRecord<'a> { @@ -49,7 +51,9 @@ impl ModelVisibleContextFragment for UserShellCommandRecord<'_> { self.turn_context.truncation_policy, )); sections.push("".to_string()); - UserShellCommandFragment::wrap_contextual_user_body(sections.join("\n")) + ::wrap_contextual_user_body( + sections.join("\n"), + ) } } diff --git a/codex-rs/core/src/user_shell_command_tests.rs b/codex-rs/core/src/user_shell_command_tests.rs index ca98922577..4b30865f04 100644 --- a/codex-rs/core/src/user_shell_command_tests.rs +++ b/codex-rs/core/src/user_shell_command_tests.rs @@ -7,10 +7,10 @@ use pretty_assertions::assert_eq; #[test] fn detects_user_shell_command_text_variants() { assert!( - ::matches_contextual_user_text("\necho hi\n") + ::matches_contextual_user_text("\necho hi\n") ); assert!( - !::matches_contextual_user_text("echo hi") + !::matches_contextual_user_text("echo hi") ); } diff --git a/docs/model-visible-context.md b/docs/model-visible-context.md index 6ea3ec3453..6c91f32d0c 100644 --- a/docs/model-visible-context.md +++ b/docs/model-visible-context.md @@ -26,11 +26,12 @@ When adding new model-visible context: 2. Implement `ModelVisibleContextFragment` for it. 3. Set the fragment `type Role` to the correct developer or contextual-user role. 4. If it is a contextual-user fragment, implement contextual-user detection: - - prefer `TaggedContextualUserFragment` for marker-based detection/wrapping - - use `ContextualUserFragmentDetector` when matching is dynamic (for example AGENTS.md tags that embed directory names) + - implement `ContextualUserFragment` + - prefer defining `markers()` so the default marker-based detection/wrapping applies + - override `matches_contextual_user_text()` only when matching is genuinely custom (for example AGENTS.md) 5. If the fragment is derived from durable/current turn state and should be diffed/reinjected after history mutations, also implement `TurnContextDiffFragment`. 6. Register the fragment type: - - developer turn-state fragments: add to `REGISTERED_DEVELOPER_TURN_STATE_FRAGMENT_BUILDERS` in `context_manager/updates.rs`. + - developer turn-state fragments: add to `REGISTERED_TURN_STATE_FRAGMENT_BUILDERS` in `context_manager/updates.rs`. - contextual-user fragments: add once to `REGISTERED_CONTEXTUAL_USER_FRAGMENTS` in `model_visible_context.rs` (this powers both history detection and optional turn-state diff assembly). 7. Push the resulting fragments through the shared envelope builders.