mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Simplify goal extension context handling
This commit is contained in:
@@ -117,29 +117,6 @@ mod tests {
|
||||
use crate::outgoing_message::OutgoingEnvelope;
|
||||
use crate::outgoing_message::OutgoingMessage;
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_event_sink_forwards_thread_goal_updates() {
|
||||
let (outgoing_tx, mut outgoing_rx) = mpsc::channel(4);
|
||||
let outgoing = Arc::new(OutgoingMessageSender::new(
|
||||
outgoing_tx,
|
||||
AnalyticsEventsClient::disabled(),
|
||||
));
|
||||
let sink = app_server_extension_event_sink(outgoing);
|
||||
let thread_id = ThreadId::default();
|
||||
|
||||
sink.emit(thread_goal_update_event(
|
||||
thread_id,
|
||||
"wire extension events",
|
||||
"turn-1",
|
||||
));
|
||||
let notification = recv_goal_update(&mut outgoing_rx).await;
|
||||
|
||||
assert_eq!(
|
||||
app_server_goal_update(thread_id, "wire extension events", "turn-1"),
|
||||
notification
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_event_sink_waits_for_outgoing_capacity() {
|
||||
let (outgoing_tx, mut outgoing_rx) = mpsc::channel(1);
|
||||
@@ -170,7 +147,10 @@ mod tests {
|
||||
let _prefill = recv_goal_update(&mut outgoing_rx).await;
|
||||
let notification = recv_goal_update(&mut outgoing_rx).await;
|
||||
|
||||
assert_eq!(Some("turn-1".to_string()), notification.turn_id);
|
||||
assert_eq!(
|
||||
app_server_goal_update(thread_id, "wait for capacity", "turn-1"),
|
||||
notification
|
||||
);
|
||||
}
|
||||
|
||||
fn thread_goal_update_event(thread_id: ThreadId, objective: &str, turn_id: &str) -> Event {
|
||||
|
||||
@@ -361,13 +361,7 @@ async fn turn_start_updates_runtime_workspace_roots_for_loaded_thread() -> Resul
|
||||
async fn thread_goal_get_rejects_unmaterialized_thread() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
config.replace("personality = true\n", "personality = true\ngoals = true\n"),
|
||||
)?;
|
||||
create_config_toml_with_goals(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -803,13 +797,7 @@ async fn thread_resume_can_skip_turns_for_metadata_only_resume() -> Result<()> {
|
||||
async fn thread_resume_keeps_paused_goal_paused() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
config.replace("personality = true\n", "personality = true\ngoals = true\n"),
|
||||
)?;
|
||||
create_config_toml_with_goals(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -920,13 +908,7 @@ async fn thread_goal_status_resume_does_not_inject_objective_update_prompt() ->
|
||||
)
|
||||
.await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
config.replace("personality = true\n", "personality = true\ngoals = true\n"),
|
||||
)?;
|
||||
create_config_toml_with_goals(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -1065,13 +1047,7 @@ async fn thread_goal_status_resume_does_not_inject_objective_update_prompt() ->
|
||||
async fn thread_goal_set_preserves_budget_limited_same_objective() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
config.replace("personality = true\n", "personality = true\ngoals = true\n"),
|
||||
)?;
|
||||
create_config_toml_with_goals(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -1163,13 +1139,7 @@ async fn thread_goal_set_preserves_budget_limited_same_objective() -> Result<()>
|
||||
async fn thread_goal_set_persists_resumable_stopped_statuses() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
config.replace("personality = true\n", "personality = true\ngoals = true\n"),
|
||||
)?;
|
||||
create_config_toml_with_goals(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -1249,13 +1219,7 @@ async fn thread_goal_set_persists_resumable_stopped_statuses() -> Result<()> {
|
||||
async fn thread_goal_set_edits_objective_without_resetting_usage() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
config.replace("personality = true\n", "personality = true\ngoals = true\n"),
|
||||
)?;
|
||||
create_config_toml_with_goals(codex_home.path(), &server.uri())?;
|
||||
let thread_id = create_fake_rollout(
|
||||
codex_home.path(),
|
||||
"2025-01-05T12-00-00",
|
||||
@@ -1358,13 +1322,7 @@ async fn thread_goal_set_edits_objective_without_resetting_usage() -> Result<()>
|
||||
async fn thread_goal_clear_deletes_goal_and_notifies() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
config.replace("personality = true\n", "personality = true\ngoals = true\n"),
|
||||
)?;
|
||||
create_config_toml_with_goals(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -3615,6 +3573,16 @@ stream_max_retries = 0
|
||||
)
|
||||
}
|
||||
|
||||
fn create_config_toml_with_goals(codex_home: &Path, server_uri: &str) -> std::io::Result<()> {
|
||||
create_config_toml(codex_home, server_uri)?;
|
||||
let config_path = codex_home.join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
config_path,
|
||||
config.replace("personality = true\n", "personality = true\ngoals = true\n"),
|
||||
)
|
||||
}
|
||||
|
||||
fn response_items_contain_text(items: &[Value], needle: &str) -> bool {
|
||||
items.iter().any(|item| {
|
||||
item.get("content")
|
||||
|
||||
@@ -8,7 +8,6 @@ use super::ExtensionContext;
|
||||
use super::FragmentRegistration;
|
||||
use super::FragmentRegistrationProxy;
|
||||
use super::LegacyApplyPatchExecCommandWarning;
|
||||
use super::LegacyGoalContext;
|
||||
use super::LegacyModelMismatchWarning;
|
||||
use super::LegacyUnifiedExecProcessLimitWarning;
|
||||
use super::SkillInstructions;
|
||||
@@ -39,8 +38,6 @@ static LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION: FragmentRegistrat
|
||||
static LEGACY_APPLY_PATCH_EXEC_COMMAND_WARNING_REGISTRATION: FragmentRegistrationProxy<
|
||||
LegacyApplyPatchExecCommandWarning,
|
||||
> = FragmentRegistrationProxy::new();
|
||||
static LEGACY_GOAL_CONTEXT_REGISTRATION: FragmentRegistrationProxy<LegacyGoalContext> =
|
||||
FragmentRegistrationProxy::new();
|
||||
static LEGACY_MODEL_MISMATCH_WARNING_REGISTRATION: FragmentRegistrationProxy<
|
||||
LegacyModelMismatchWarning,
|
||||
> = FragmentRegistrationProxy::new();
|
||||
@@ -56,7 +53,6 @@ static CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[
|
||||
&SUBAGENT_NOTIFICATION_REGISTRATION,
|
||||
&LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION,
|
||||
&LEGACY_APPLY_PATCH_EXEC_COMMAND_WARNING_REGISTRATION,
|
||||
&LEGACY_GOAL_CONTEXT_REGISTRATION,
|
||||
&LEGACY_MODEL_MISMATCH_WARNING_REGISTRATION,
|
||||
];
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::context::ExtensionContextTag;
|
||||
use crate::context::SubagentNotification;
|
||||
use codex_protocol::items::HookPromptFragment;
|
||||
use codex_protocol::items::build_hook_prompt_message;
|
||||
@@ -21,9 +22,11 @@ fn detects_extension_context_fragment() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_legacy_goal_context_fragment() {
|
||||
fn detects_goal_extension_context_fragment() {
|
||||
let text = ExtensionContext::with_tag(ExtensionContextTag::GOAL, "hidden goal prompt").render();
|
||||
|
||||
assert!(is_contextual_user_fragment(&ContentItem::InputText {
|
||||
text: "<goal_context>\nlegacy hidden goal prompt\n</goal_context>".to_string(),
|
||||
text
|
||||
}));
|
||||
}
|
||||
|
||||
|
||||
@@ -1,14 +1,51 @@
|
||||
//! Hidden user-context wrappers owned by extensions.
|
||||
//!
|
||||
//! Extensions can use one of these host-recognized tags when they need hidden
|
||||
//! model-visible context to survive history filtering without teaching core
|
||||
//! about the extension's domain concepts.
|
||||
|
||||
use super::ContextualUserFragment;
|
||||
use super::fragment::matches_marked_text;
|
||||
|
||||
/// Host-recognized marker pair for extension-owned hidden context.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct ExtensionContextTag {
|
||||
start_marker: &'static str,
|
||||
end_marker: &'static str,
|
||||
}
|
||||
|
||||
impl ExtensionContextTag {
|
||||
pub const DEFAULT: Self = Self::new("<extension_context>", "</extension_context>");
|
||||
pub const GOAL: Self = Self::new("<goal_context>", "</goal_context>");
|
||||
|
||||
const fn new(start_marker: &'static str, end_marker: &'static str) -> Self {
|
||||
Self {
|
||||
start_marker,
|
||||
end_marker,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const EXTENSION_CONTEXT_TAGS: &[ExtensionContextTag] =
|
||||
&[ExtensionContextTag::DEFAULT, ExtensionContextTag::GOAL];
|
||||
|
||||
/// Hidden user-context fragment for extension-owned steering prompts.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ExtensionContext {
|
||||
tag: ExtensionContextTag,
|
||||
body: String,
|
||||
}
|
||||
|
||||
impl ExtensionContext {
|
||||
pub fn new(body: impl Into<String>) -> Self {
|
||||
Self { body: body.into() }
|
||||
Self::with_tag(ExtensionContextTag::DEFAULT, body)
|
||||
}
|
||||
|
||||
pub fn with_tag(tag: ExtensionContextTag, body: impl Into<String>) -> Self {
|
||||
Self {
|
||||
tag,
|
||||
body: body.into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,11 +55,20 @@ impl ContextualUserFragment for ExtensionContext {
|
||||
}
|
||||
|
||||
fn markers(&self) -> (&'static str, &'static str) {
|
||||
Self::type_markers()
|
||||
(self.tag.start_marker, self.tag.end_marker)
|
||||
}
|
||||
|
||||
fn type_markers() -> (&'static str, &'static str) {
|
||||
("<extension_context>", "</extension_context>")
|
||||
(
|
||||
ExtensionContextTag::DEFAULT.start_marker,
|
||||
ExtensionContextTag::DEFAULT.end_marker,
|
||||
)
|
||||
}
|
||||
|
||||
fn matches_text(text: &str) -> bool {
|
||||
EXTENSION_CONTEXT_TAGS
|
||||
.iter()
|
||||
.any(|tag| matches_marked_text(tag.start_marker, tag.end_marker, text))
|
||||
}
|
||||
|
||||
fn body(&self) -> String {
|
||||
|
||||
@@ -97,7 +97,7 @@ pub trait ContextualUserFragment {
|
||||
}
|
||||
}
|
||||
|
||||
fn matches_marked_text(start_marker: &str, end_marker: &str, text: &str) -> bool {
|
||||
pub(crate) fn matches_marked_text(start_marker: &str, end_marker: &str, text: &str) -> bool {
|
||||
if start_marker.is_empty() || end_marker.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -1,26 +0,0 @@
|
||||
use super::ContextualUserFragment;
|
||||
|
||||
/// Legacy hidden goal-context fragment.
|
||||
///
|
||||
/// Goal prompts are now owned by the goal extension and wrapped as extension
|
||||
/// context. This keeps older hidden goal prompts from being surfaced as visible
|
||||
/// user text when stored conversation history is replayed.
|
||||
pub(crate) struct LegacyGoalContext;
|
||||
|
||||
impl ContextualUserFragment for LegacyGoalContext {
|
||||
fn role() -> &'static str {
|
||||
"user"
|
||||
}
|
||||
|
||||
fn markers(&self) -> (&'static str, &'static str) {
|
||||
Self::type_markers()
|
||||
}
|
||||
|
||||
fn type_markers() -> (&'static str, &'static str) {
|
||||
("<goal_context>", "</goal_context>")
|
||||
}
|
||||
|
||||
fn body(&self) -> String {
|
||||
String::new()
|
||||
}
|
||||
}
|
||||
@@ -14,7 +14,6 @@ mod guardian_followup_review_reminder;
|
||||
mod hook_additional_context;
|
||||
mod image_generation_instructions;
|
||||
mod legacy_apply_patch_exec_command_warning;
|
||||
mod legacy_goal_context;
|
||||
mod legacy_model_mismatch_warning;
|
||||
mod legacy_unified_exec_process_limit_warning;
|
||||
mod model_switch_instructions;
|
||||
@@ -40,6 +39,7 @@ pub(crate) use contextual_user_message::is_contextual_user_fragment;
|
||||
pub(crate) use contextual_user_message::parse_visible_hook_prompt_message;
|
||||
pub(crate) use environment_context::EnvironmentContext;
|
||||
pub use extension_context::ExtensionContext;
|
||||
pub use extension_context::ExtensionContextTag;
|
||||
pub use fragment::ContextualUserFragment;
|
||||
pub(crate) use fragment::FragmentRegistration;
|
||||
pub(crate) use fragment::FragmentRegistrationProxy;
|
||||
@@ -49,7 +49,6 @@ pub(crate) use guardian_followup_review_reminder::GuardianFollowupReviewReminder
|
||||
pub(crate) use hook_additional_context::HookAdditionalContext;
|
||||
pub(crate) use image_generation_instructions::ImageGenerationInstructions;
|
||||
pub(crate) use legacy_apply_patch_exec_command_warning::LegacyApplyPatchExecCommandWarning;
|
||||
pub(crate) use legacy_goal_context::LegacyGoalContext;
|
||||
pub(crate) use legacy_model_mismatch_warning::LegacyModelMismatchWarning;
|
||||
pub(crate) use legacy_unified_exec_process_limit_warning::LegacyUnifiedExecProcessLimitWarning;
|
||||
pub(crate) use model_switch_instructions::ModelSwitchInstructions;
|
||||
|
||||
@@ -3,6 +3,8 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_extension_api::ThreadIdleRequest;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::protocol::ThreadSettingsSnapshot;
|
||||
|
||||
use crate::context::ContextualUserFragment;
|
||||
@@ -211,13 +213,20 @@ fn idle_extension_input(prompt: String) -> TurnInput {
|
||||
MAX_IDLE_EXTENSION_PROMPT_TOKENS,
|
||||
)
|
||||
.0;
|
||||
if ExtensionContext::matches_text(&prompt) {
|
||||
return TurnInput::ResponseInputItem(ResponseInputItem::Message {
|
||||
role: ExtensionContext::role().to_string(),
|
||||
content: vec![ContentItem::InputText { text: prompt }],
|
||||
phase: None,
|
||||
});
|
||||
}
|
||||
|
||||
TurnInput::ResponseInputItem(ExtensionContext::new(prompt).into_response_input_item())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use crate::context::ExtensionContextTag;
|
||||
|
||||
use super::*;
|
||||
|
||||
@@ -229,6 +238,7 @@ mod tests {
|
||||
);
|
||||
let original_len = prompt.len();
|
||||
|
||||
let prompt = ExtensionContext::with_tag(ExtensionContextTag::GOAL, prompt).render();
|
||||
let TurnInput::ResponseInputItem(ResponseInputItem::Message { content, .. }) =
|
||||
idle_extension_input(prompt)
|
||||
else {
|
||||
@@ -238,7 +248,7 @@ mod tests {
|
||||
panic!("expected one text content item");
|
||||
};
|
||||
|
||||
assert!(text.starts_with("<extension_context>"));
|
||||
assert!(text.starts_with("<goal_context>"));
|
||||
assert!(text.contains("start"));
|
||||
assert!(text.contains("end"));
|
||||
assert!(text.len() < original_len);
|
||||
|
||||
@@ -14,7 +14,7 @@ use crate::accounting::BudgetLimitedGoalDisposition;
|
||||
use crate::accounting::GoalAccountingState;
|
||||
use crate::events::GoalEventEmitter;
|
||||
use crate::metrics::GoalMetrics;
|
||||
use crate::steering::continuation_steering_prompt;
|
||||
use crate::steering::continuation_steering_request;
|
||||
use crate::steering::objective_updated_steering_item;
|
||||
use crate::tool::protocol_goal_from_state;
|
||||
|
||||
@@ -284,9 +284,7 @@ impl GoalRuntimeHandle {
|
||||
.mark_idle_goal_active(goal.goal_id.clone());
|
||||
let goal_id = goal.goal_id.clone();
|
||||
let goal = protocol_goal_from_state(goal);
|
||||
let request =
|
||||
codex_extension_api::ThreadIdleRequest::new(continuation_steering_prompt(&goal))
|
||||
.with_validation_key(goal_id);
|
||||
let request = continuation_steering_request(&goal).with_validation_key(goal_id);
|
||||
Ok(Some(request))
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use codex_core::context::ContextualUserFragment;
|
||||
use codex_core::context::ExtensionContext;
|
||||
use codex_core::context::ExtensionContextTag;
|
||||
use codex_extension_api::ThreadIdleRequest;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::protocol::ThreadGoal;
|
||||
use codex_utils_template::Template;
|
||||
@@ -48,10 +50,9 @@ pub(crate) fn budget_limit_steering_item(goal: &ThreadGoal) -> ResponseInputItem
|
||||
goal_context_input_item(budget_limit_prompt(goal))
|
||||
}
|
||||
|
||||
pub(crate) fn continuation_steering_prompt(goal: &ThreadGoal) -> String {
|
||||
format!(
|
||||
"<goal_context>\n{}\n</goal_context>",
|
||||
continuation_prompt(goal)
|
||||
pub(crate) fn continuation_steering_request(goal: &ThreadGoal) -> ThreadIdleRequest {
|
||||
ThreadIdleRequest::new(
|
||||
ExtensionContext::with_tag(ExtensionContextTag::GOAL, continuation_prompt(goal)).render(),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -60,8 +61,7 @@ pub(crate) fn objective_updated_steering_item(goal: &ThreadGoal) -> ResponseInpu
|
||||
}
|
||||
|
||||
fn goal_context_input_item(prompt: String) -> ResponseInputItem {
|
||||
ExtensionContext::new(format!("<goal_context>\n{prompt}\n</goal_context>"))
|
||||
.into_response_input_item()
|
||||
ExtensionContext::with_tag(ExtensionContextTag::GOAL, prompt).into_response_input_item()
|
||||
}
|
||||
|
||||
fn continuation_prompt(goal: &ThreadGoal) -> String {
|
||||
|
||||
Reference in New Issue
Block a user