diff --git a/codex-rs/core/src/context/contextual_user_message.rs b/codex-rs/core/src/context/contextual_user_message.rs index 940a938fcee1..79768dd3a38c 100644 --- a/codex-rs/core/src/context/contextual_user_message.rs +++ b/codex-rs/core/src/context/contextual_user_message.rs @@ -6,7 +6,7 @@ use super::AdditionalContextUserFragment; use super::EnvironmentContext; use super::FragmentRegistration; use super::FragmentRegistrationProxy; -use super::GoalContext; +use super::InternalModelContextFragment; use super::LegacyApplyPatchExecCommandWarning; use super::LegacyModelMismatchWarning; use super::LegacyUnifiedExecProcessLimitWarning; @@ -30,8 +30,9 @@ static TURN_ABORTED_REGISTRATION: FragmentRegistrationProxy = FragmentRegistrationProxy::new(); static SUBAGENT_NOTIFICATION_REGISTRATION: FragmentRegistrationProxy = FragmentRegistrationProxy::new(); -static GOAL_CONTEXT_REGISTRATION: FragmentRegistrationProxy = - FragmentRegistrationProxy::new(); +static INTERNAL_MODEL_CONTEXT_REGISTRATION: FragmentRegistrationProxy< + InternalModelContextFragment, +> = FragmentRegistrationProxy::new(); static LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION: FragmentRegistrationProxy< LegacyUnifiedExecProcessLimitWarning, > = FragmentRegistrationProxy::new(); @@ -50,7 +51,7 @@ static CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[ &USER_SHELL_COMMAND_REGISTRATION, &TURN_ABORTED_REGISTRATION, &SUBAGENT_NOTIFICATION_REGISTRATION, - &GOAL_CONTEXT_REGISTRATION, + &INTERNAL_MODEL_CONTEXT_REGISTRATION, &LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION, &LEGACY_APPLY_PATCH_EXEC_COMMAND_WARNING_REGISTRATION, &LEGACY_MODEL_MISMATCH_WARNING_REGISTRATION, diff --git a/codex-rs/core/src/context/contextual_user_message_tests.rs b/codex-rs/core/src/context/contextual_user_message_tests.rs index 195b18992909..dc7cb0f5347f 100644 --- a/codex-rs/core/src/context/contextual_user_message_tests.rs +++ b/codex-rs/core/src/context/contextual_user_message_tests.rs @@ -1,6 +1,7 @@ use super::*; use crate::context::ContextualUserFragment; -use crate::context::GoalContext; +use crate::context::InternalContextSource; +use crate::context::InternalModelContextFragment; use crate::context::SubagentNotification; use codex_protocol::items::HookPromptFragment; use codex_protocol::items::build_hook_prompt_message; @@ -30,23 +31,55 @@ fn detects_subagent_notification_fragment_case_insensitively() { } #[test] -fn detects_goal_context_fragment() { - let text = GoalContext::new("Continue working toward the active thread goal.").render(); +fn detects_internal_model_context_fragment() { + let text = InternalModelContextFragment::new( + InternalContextSource::from_static("goal"), + "Continue working toward the active thread goal.", + ) + .render(); + assert_eq!( + text, + "\nContinue working toward the active thread goal.\n" + ); assert!(is_contextual_user_fragment(&ContentItem::InputText { text })); } +#[test] +fn detects_legacy_goal_context_fragment() { + assert!(is_contextual_user_fragment(&ContentItem::InputText { + text: "\nContinue working toward the active thread goal.\n" + .to_string(), + })); +} + +#[test] +fn does_not_hide_arbitrary_context_tags() { + assert!(!is_contextual_user_fragment(&ContentItem::InputText { + text: "\nbody\n".to_string(), + })); +} + +#[test] +fn rejects_invalid_internal_model_context_source() { + assert!(!is_contextual_user_fragment(&ContentItem::InputText { + text: "\nbody\n" + .to_string(), + })); +} + #[test] fn contextual_user_fragment_is_dyn_compatible() { - let fragment: Box = Box::new(GoalContext::new( + let fragment: Box = Box::new(InternalModelContextFragment::new( + InternalContextSource::from_static("goal"), "Continue working toward the active thread goal.", )); assert_eq!( fragment.render(), - "\nContinue working toward the active thread goal.\n" + "\nContinue working toward the active thread goal.\n" ); } diff --git a/codex-rs/core/src/context/goal_context.rs b/codex-rs/core/src/context/goal_context.rs deleted file mode 100644 index ad4c47ef062a..000000000000 --- a/codex-rs/core/src/context/goal_context.rs +++ /dev/null @@ -1,36 +0,0 @@ -//! Hidden user-context fragment for runtime-owned goal steering prompts. - -use super::ContextualUserFragment; - -/// Hidden runtime-owned goal steering context injected into model input. -#[derive(Debug, Clone, PartialEq)] -pub struct GoalContext { - prompt: String, -} - -impl GoalContext { - /// Creates goal context around an already-rendered steering prompt. - pub fn new(prompt: impl Into) -> Self { - Self { - prompt: prompt.into(), - } - } -} - -impl ContextualUserFragment for GoalContext { - fn role() -> &'static str { - "user" - } - - fn markers(&self) -> (&'static str, &'static str) { - Self::type_markers() - } - - fn type_markers() -> (&'static str, &'static str) { - ("", "") - } - - fn body(&self) -> String { - format!("\n{}\n", self.prompt) - } -} diff --git a/codex-rs/core/src/context/internal_model_context.rs b/codex-rs/core/src/context/internal_model_context.rs new file mode 100644 index 000000000000..a25e77a82a27 --- /dev/null +++ b/codex-rs/core/src/context/internal_model_context.rs @@ -0,0 +1,129 @@ +//! Hidden user-context fragment for extension-owned model steering. + +use super::ContextualUserFragment; +use std::error::Error; +use std::fmt; + +const CONTEXT_START_MARKER: &str = ""; + +/// Source label for hidden internal model context. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct InternalContextSource(String); + +impl InternalContextSource { + /// Creates a source label for an internal model-context fragment. + /// + /// Sources are intentionally constrained so the value can be embedded in the + /// wrapper without escaping and still remain easy to audit in stored history. + pub fn new(source: impl Into) -> Result { + let source = source.into(); + if is_valid_source(&source) { + Ok(Self(source)) + } else { + Err(InvalidInternalContextSource { source }) + } + } + + /// Creates a source label from a trusted static string. + pub fn from_static(source: &'static str) -> Self { + Self::new(source) + .unwrap_or_else(|_| panic!("invalid static internal context source: {source}")) + } + + pub fn as_str(&self) -> &str { + &self.0 + } +} + +/// Error returned when an internal model-context source is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct InvalidInternalContextSource { + source: String, +} + +impl fmt::Display for InvalidInternalContextSource { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let source = &self.source; + write!( + f, + "invalid internal model context source {source:?}; expected [a-z][a-z0-9_]*" + ) + } +} + +impl Error for InvalidInternalContextSource {} + +/// Hidden runtime-owned context injected into model input. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct InternalModelContextFragment { + source: InternalContextSource, + body: String, +} + +impl InternalModelContextFragment { + /// Creates hidden model context with an extension-owned source label. + pub fn new(source: InternalContextSource, body: impl Into) -> Self { + Self { + source, + body: body.into(), + } + } +} + +impl ContextualUserFragment for InternalModelContextFragment { + fn role() -> &'static str { + "user" + } + + fn markers(&self) -> (&'static str, &'static str) { + Self::type_markers() + } + + fn type_markers() -> (&'static str, &'static str) { + (CONTEXT_START_MARKER, CONTEXT_END_MARKER) + } + + fn matches_text(text: &str) -> bool { + let trimmed = text.trim(); + if matches_legacy_goal_context(trimmed) { + return true; + } + + let Some(rest) = trimmed.strip_prefix(CONTEXT_START_MARKER) else { + return false; + }; + let Some(rest) = rest.strip_prefix(SOURCE_ATTR_START) else { + return false; + }; + let Some((source, body_and_close)) = rest.split_once(SOURCE_ATTR_END) else { + return false; + }; + + is_valid_source(source) && body_and_close.ends_with(CONTEXT_END_MARKER) + } + + fn body(&self) -> String { + let source = self.source.as_str(); + let body = &self.body; + format!(" source=\"{source}\">\n{body}\n") + } +} + +fn matches_legacy_goal_context(text: &str) -> bool { + text.starts_with(LEGACY_GOAL_CONTEXT_START_MARKER) + && text.ends_with(LEGACY_GOAL_CONTEXT_END_MARKER) +} + +fn is_valid_source(source: &str) -> bool { + let mut chars = source.chars(); + let Some(first) = chars.next() else { + return false; + }; + first.is_ascii_lowercase() + && chars.all(|ch| ch.is_ascii_lowercase() || ch.is_ascii_digit() || ch == '_') +} diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index 6d88df87e273..3e01d5212e63 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -9,10 +9,10 @@ mod contextual_user_message; mod environment_context; mod fragment; mod fragments; -mod goal_context; mod guardian_followup_review_reminder; mod hook_additional_context; mod image_generation_instructions; +mod internal_model_context; mod legacy_apply_patch_exec_command_warning; mod legacy_model_mismatch_warning; mod legacy_unified_exec_process_limit_warning; @@ -43,10 +43,12 @@ pub(crate) use fragment::FragmentRegistration; pub(crate) use fragment::FragmentRegistrationProxy; pub(crate) use fragments::AdditionalContextDeveloperFragment; pub(crate) use fragments::AdditionalContextUserFragment; -pub use goal_context::GoalContext; pub(crate) use guardian_followup_review_reminder::GuardianFollowupReviewReminder; pub(crate) use hook_additional_context::HookAdditionalContext; pub(crate) use image_generation_instructions::ImageGenerationInstructions; +pub use internal_model_context::InternalContextSource; +pub use internal_model_context::InternalModelContextFragment; +pub use internal_model_context::InvalidInternalContextSource; pub(crate) use legacy_apply_patch_exec_command_warning::LegacyApplyPatchExecCommandWarning; pub(crate) use legacy_model_mismatch_warning::LegacyModelMismatchWarning; pub(crate) use legacy_unified_exec_process_limit_warning::LegacyUnifiedExecProcessLimitWarning; diff --git a/codex-rs/core/src/event_mapping_tests.rs b/codex-rs/core/src/event_mapping_tests.rs index 57d6362f55dc..df636d4a6b34 100644 --- a/codex-rs/core/src/event_mapping_tests.rs +++ b/codex-rs/core/src/event_mapping_tests.rs @@ -1,6 +1,7 @@ use super::parse_turn_item; use crate::context::ContextualUserFragment; -use crate::context::GoalContext; +use crate::context::InternalContextSource; +use crate::context::InternalModelContextFragment; use codex_protocol::items::AgentMessageContent; use codex_protocol::items::HookPromptFragment; use codex_protocol::items::TurnItem; @@ -317,12 +318,16 @@ fn parses_hook_prompt_and_hides_other_contextual_fragments() { } #[test] -fn goal_context_does_not_parse_as_visible_turn_item() { +fn internal_model_context_does_not_parse_as_visible_turn_item() { let item = ResponseItem::Message { id: Some("msg-1".to_string()), role: "user".to_string(), content: vec![ContentItem::InputText { - text: GoalContext::new("Continue working toward the active thread goal.").render(), + text: InternalModelContextFragment::new( + InternalContextSource::from_static("goal"), + "Continue working toward the active thread goal.", + ) + .render(), }], phase: None, }; diff --git a/codex-rs/core/src/goals.rs b/codex-rs/core/src/goals.rs index ece020385abb..00b68543423c 100644 --- a/codex-rs/core/src/goals.rs +++ b/codex-rs/core/src/goals.rs @@ -6,7 +6,8 @@ use crate::StateDbHandle; use crate::context::ContextualUserFragment; -use crate::context::GoalContext; +use crate::context::InternalContextSource; +use crate::context::InternalModelContextFragment; use crate::session::TurnInput; use crate::session::session::Session; use crate::session::turn_context::TurnContext; @@ -1597,7 +1598,10 @@ fn budget_limit_steering_item(goal: &ThreadGoal) -> ResponseItem { } fn goal_context_input_item(prompt: String) -> ResponseItem { - ResponseItem::from(GoalContext::new(prompt).into_response_input_item()) + ContextualUserFragment::into(InternalModelContextFragment::new( + InternalContextSource::from_static("goal"), + prompt, + )) } pub(crate) fn protocol_goal_from_state(goal: codex_state::ThreadGoal) -> ThreadGoal { @@ -1799,7 +1803,7 @@ mod tests { id: None, role: "user".to_string(), content: vec![ContentItem::InputText { - text: "\nContinue working.\n".to_string(), + text: "\nContinue working.\n".to_string(), }], phase: None, } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 7cca56dfb24a..91997069fb58 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -8556,24 +8556,13 @@ async fn active_goal_continuation_runs_again_after_no_tool_turn() -> anyhow::Res }) .await??; - let continuation_request = responses + let goal_context_text = responses .requests() .into_iter() - .find(|request| request.body_contains_text("")) - .expect("expected a goal continuation request"); - let body = continuation_request.body_json(); - let goal_context_message = body["input"] - .as_array() - .expect("input should be an array") - .iter() - .find(|item| item.to_string().contains("")) + .flat_map(|request| request.message_input_texts("user")) + .find(|text| text.contains("")) .expect("goal context message should be present"); - assert_eq!(goal_context_message["role"].as_str(), Some("user")); - assert!( - goal_context_message - .to_string() - .contains("Continue working toward the active thread goal.") - ); + assert!(goal_context_text.contains("Continue working toward the active thread goal.")); Ok(()) } @@ -8843,8 +8832,8 @@ async fn budget_limited_accounting_steers_active_turn_without_aborting() -> anyh let [ContentItem::InputText { text }] = content.as_slice() else { panic!("expected one text span in budget-limit steering message, got {content:#?}"); }; - assert!(text.starts_with("")); - assert!(text.trim_end().ends_with("")); + assert!(text.starts_with("")); + assert!(text.trim_end().ends_with("")); assert!(text.contains("budget_limited")); assert!(text.to_lowercase().contains("wrap up this turn soon")); assert!(sess.active_turn.lock().await.is_some()); @@ -9066,8 +9055,8 @@ async fn external_objective_change_steers_active_turn() -> anyhow::Result<()> { && content.iter().any(|content| matches!( content, ContentItem::InputText { text } - if text.starts_with("") - && text.trim_end().ends_with("") + if text.starts_with("") + && text.trim_end().ends_with("") && text.contains("The active thread goal objective was edited") && text.contains("Write a concise benchmark summary") )) diff --git a/codex-rs/ext/goal/src/steering.rs b/codex-rs/ext/goal/src/steering.rs index 83f41ea538dd..580a4aa971ec 100644 --- a/codex-rs/ext/goal/src/steering.rs +++ b/codex-rs/ext/goal/src/steering.rs @@ -1,14 +1,20 @@ use codex_core::context::ContextualUserFragment; -use codex_core::context::GoalContext; +use codex_core::context::InternalContextSource; +use codex_core::context::InternalModelContextFragment; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::ThreadGoal; pub(crate) fn budget_limit_steering_item(goal: &ThreadGoal) -> ResponseInputItem { - GoalContext::new(budget_limit_prompt(goal)).into_response_input_item() + goal_context_input_item(budget_limit_prompt(goal)) } pub(crate) fn objective_updated_steering_item(goal: &ThreadGoal) -> ResponseInputItem { - GoalContext::new(objective_updated_prompt(goal)).into_response_input_item() + goal_context_input_item(objective_updated_prompt(goal)) +} + +fn goal_context_input_item(prompt: String) -> ResponseInputItem { + InternalModelContextFragment::new(InternalContextSource::from_static("goal"), prompt) + .into_response_input_item() } fn budget_limit_prompt(goal: &ThreadGoal) -> String {