mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Merge branch 'nicholasclark/tool-call-task-headers-_meta' into nicholasclark/app-server-inprocess-parent-request-headers
This commit is contained in:
@@ -1670,6 +1670,14 @@
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"terminal_title": {
|
||||
"default": null,
|
||||
"description": "Ordered list of terminal title item identifiers.\n\nWhen set, the TUI renders the selected items into the terminal window/tab title. When unset, the TUI defaults to: `spinner` and `project`.",
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"theme": {
|
||||
"default": null,
|
||||
"description": "Syntax highlighting theme name (kebab-case).\n\nWhen set, overrides automatic light/dark theme detection. Use `/theme` in the TUI or see `$CODEX_HOME/themes` for custom themes.",
|
||||
@@ -1895,6 +1903,10 @@
|
||||
"experimental_compact_prompt_file": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
},
|
||||
"experimental_exec_server_url": {
|
||||
"description": "Experimental / do not use. Overrides the URL used when connecting to a remote exec server.",
|
||||
"type": "string"
|
||||
},
|
||||
"experimental_realtime_start_instructions": {
|
||||
"description": "Experimental / do not use. Replaces the built-in realtime start instructions inserted into developer messages when realtime becomes active.",
|
||||
"type": "string"
|
||||
|
||||
@@ -88,6 +88,7 @@ use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::items::PlanItem;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::items::UserMessageItem;
|
||||
use codex_protocol::items::build_hook_prompt_message;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
@@ -1849,7 +1850,9 @@ impl Session {
|
||||
code_mode_service: crate::tools::code_mode::CodeModeService::new(
|
||||
config.js_repl_node_path.clone(),
|
||||
),
|
||||
environment: Arc::new(Environment),
|
||||
environment: Arc::new(
|
||||
Environment::create(config.experimental_exec_server_url.clone()).await?,
|
||||
),
|
||||
};
|
||||
let js_repl = Arc::new(JsReplHandle::with_node_path(
|
||||
config.js_repl_node_path.clone(),
|
||||
@@ -5764,13 +5767,12 @@ pub(crate) async fn run_turn(
|
||||
.await;
|
||||
}
|
||||
if stop_outcome.should_block {
|
||||
if let Some(continuation_prompt) = stop_outcome.continuation_prompt.clone()
|
||||
if let Some(hook_prompt_message) =
|
||||
build_hook_prompt_message(&stop_outcome.continuation_fragments)
|
||||
{
|
||||
let developer_message: ResponseItem =
|
||||
DeveloperInstructions::new(continuation_prompt).into();
|
||||
sess.record_conversation_items(
|
||||
&turn_context,
|
||||
std::slice::from_ref(&developer_message),
|
||||
std::slice::from_ref(&hook_prompt_message),
|
||||
)
|
||||
.await;
|
||||
stop_hook_active = true;
|
||||
|
||||
@@ -2495,7 +2495,11 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
true,
|
||||
));
|
||||
let network_approval = Arc::new(NetworkApprovalService::default());
|
||||
let environment = Arc::new(codex_exec_server::Environment);
|
||||
let environment = Arc::new(
|
||||
codex_exec_server::Environment::create(None)
|
||||
.await
|
||||
.expect("create environment"),
|
||||
);
|
||||
|
||||
let file_watcher = Arc::new(FileWatcher::noop());
|
||||
let services = SessionServices {
|
||||
@@ -3291,7 +3295,11 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
|
||||
true,
|
||||
));
|
||||
let network_approval = Arc::new(NetworkApprovalService::default());
|
||||
let environment = Arc::new(codex_exec_server::Environment);
|
||||
let environment = Arc::new(
|
||||
codex_exec_server::Environment::create(None)
|
||||
.await
|
||||
.expect("create environment"),
|
||||
);
|
||||
|
||||
let file_watcher = Arc::new(FileWatcher::noop());
|
||||
let services = SessionServices {
|
||||
|
||||
@@ -196,7 +196,7 @@ pub(crate) async fn process_compacted_history(
|
||||
/// - `developer` messages because remote output can include stale/duplicated
|
||||
/// instruction content.
|
||||
/// - non-user-content `user` messages (session prefix/instruction wrappers),
|
||||
/// keeping only real user messages as parsed by `parse_turn_item`.
|
||||
/// while preserving real user messages and persisted hook prompts.
|
||||
///
|
||||
/// This intentionally keeps:
|
||||
/// - `assistant` messages (future remote compaction models may emit them)
|
||||
@@ -208,7 +208,7 @@ fn should_keep_compacted_history_item(item: &ResponseItem) -> bool {
|
||||
ResponseItem::Message { role, .. } if role == "user" => {
|
||||
matches!(
|
||||
crate::event_mapping::parse_turn_item(item),
|
||||
Some(TurnItem::UserMessage(_))
|
||||
Some(TurnItem::UserMessage(_) | TurnItem::HookPrompt(_))
|
||||
)
|
||||
}
|
||||
ResponseItem::Message { role, .. } if role == "assistant" => true,
|
||||
|
||||
@@ -237,6 +237,7 @@ fn config_toml_deserializes_model_availability_nux() {
|
||||
show_tooltips: true,
|
||||
alternate_screen: AltScreenMode::default(),
|
||||
status_line: None,
|
||||
terminal_title: None,
|
||||
theme: None,
|
||||
model_availability_nux: ModelAvailabilityNuxConfig {
|
||||
shown_count: HashMap::from([
|
||||
@@ -921,6 +922,7 @@ fn tui_config_missing_notifications_field_defaults_to_enabled() {
|
||||
show_tooltips: true,
|
||||
alternate_screen: AltScreenMode::Auto,
|
||||
status_line: None,
|
||||
terminal_title: None,
|
||||
theme: None,
|
||||
model_availability_nux: ModelAvailabilityNuxConfig::default(),
|
||||
}
|
||||
@@ -4310,6 +4312,7 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
model_verbosity: None,
|
||||
personality: Some(Personality::Pragmatic),
|
||||
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
|
||||
experimental_exec_server_url: None,
|
||||
realtime_audio: RealtimeAudioConfig::default(),
|
||||
experimental_realtime_start_instructions: None,
|
||||
experimental_realtime_ws_base_url: None,
|
||||
@@ -4348,6 +4351,7 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
tool_suggest: ToolSuggestConfig::default(),
|
||||
tui_alternate_screen: AltScreenMode::Auto,
|
||||
tui_status_line: None,
|
||||
tui_terminal_title: None,
|
||||
tui_theme: None,
|
||||
otel: OtelConfig::default(),
|
||||
},
|
||||
@@ -4451,6 +4455,7 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
model_verbosity: None,
|
||||
personality: Some(Personality::Pragmatic),
|
||||
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
|
||||
experimental_exec_server_url: None,
|
||||
realtime_audio: RealtimeAudioConfig::default(),
|
||||
experimental_realtime_start_instructions: None,
|
||||
experimental_realtime_ws_base_url: None,
|
||||
@@ -4489,6 +4494,7 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
tool_suggest: ToolSuggestConfig::default(),
|
||||
tui_alternate_screen: AltScreenMode::Auto,
|
||||
tui_status_line: None,
|
||||
tui_terminal_title: None,
|
||||
tui_theme: None,
|
||||
otel: OtelConfig::default(),
|
||||
};
|
||||
@@ -4590,6 +4596,7 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
model_verbosity: None,
|
||||
personality: Some(Personality::Pragmatic),
|
||||
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
|
||||
experimental_exec_server_url: None,
|
||||
realtime_audio: RealtimeAudioConfig::default(),
|
||||
experimental_realtime_start_instructions: None,
|
||||
experimental_realtime_ws_base_url: None,
|
||||
@@ -4628,6 +4635,7 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
tool_suggest: ToolSuggestConfig::default(),
|
||||
tui_alternate_screen: AltScreenMode::Auto,
|
||||
tui_status_line: None,
|
||||
tui_terminal_title: None,
|
||||
tui_theme: None,
|
||||
otel: OtelConfig::default(),
|
||||
};
|
||||
@@ -4715,6 +4723,7 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
model_verbosity: Some(Verbosity::High),
|
||||
personality: Some(Personality::Pragmatic),
|
||||
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
|
||||
experimental_exec_server_url: None,
|
||||
realtime_audio: RealtimeAudioConfig::default(),
|
||||
experimental_realtime_start_instructions: None,
|
||||
experimental_realtime_ws_base_url: None,
|
||||
@@ -4753,6 +4762,7 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
tool_suggest: ToolSuggestConfig::default(),
|
||||
tui_alternate_screen: AltScreenMode::Auto,
|
||||
tui_status_line: None,
|
||||
tui_terminal_title: None,
|
||||
tui_theme: None,
|
||||
otel: OtelConfig::default(),
|
||||
};
|
||||
@@ -5974,6 +5984,34 @@ experimental_realtime_start_instructions = "start instructions from config"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn experimental_exec_server_url_loads_from_config_toml() -> std::io::Result<()> {
|
||||
let cfg: ConfigToml = toml::from_str(
|
||||
r#"
|
||||
experimental_exec_server_url = "http://127.0.0.1:8080"
|
||||
"#,
|
||||
)
|
||||
.expect("TOML deserialization should succeed");
|
||||
|
||||
assert_eq!(
|
||||
cfg.experimental_exec_server_url.as_deref(),
|
||||
Some("http://127.0.0.1:8080")
|
||||
);
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
cfg,
|
||||
ConfigOverrides::default(),
|
||||
codex_home.path().to_path_buf(),
|
||||
)?;
|
||||
|
||||
assert_eq!(
|
||||
config.experimental_exec_server_url.as_deref(),
|
||||
Some("http://127.0.0.1:8080")
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn experimental_realtime_ws_base_url_loads_from_config_toml() -> std::io::Result<()> {
|
||||
let cfg: ConfigToml = toml::from_str(
|
||||
|
||||
@@ -60,7 +60,7 @@ pub enum ConfigEdit {
|
||||
ClearPath { segments: Vec<String> },
|
||||
}
|
||||
|
||||
/// Produces a config edit that sets `[tui] theme = "<name>"`.
|
||||
/// Produces a config edit that sets `[tui].theme = "<name>"`.
|
||||
pub fn syntax_theme_edit(name: &str) -> ConfigEdit {
|
||||
ConfigEdit::SetPath {
|
||||
segments: vec!["tui".to_string(), "theme".to_string()],
|
||||
@@ -68,11 +68,12 @@ pub fn syntax_theme_edit(name: &str) -> ConfigEdit {
|
||||
}
|
||||
}
|
||||
|
||||
/// Produces a config edit that sets `[tui].status_line` to an explicit ordered list.
|
||||
///
|
||||
/// The array is written even when it is empty so "hide the status line" stays
|
||||
/// distinct from "unset, so use defaults".
|
||||
pub fn status_line_items_edit(items: &[String]) -> ConfigEdit {
|
||||
let mut array = toml_edit::Array::new();
|
||||
for item in items {
|
||||
array.push(item.clone());
|
||||
}
|
||||
let array = items.iter().cloned().collect::<toml_edit::Array>();
|
||||
|
||||
ConfigEdit::SetPath {
|
||||
segments: vec!["tui".to_string(), "status_line".to_string()],
|
||||
@@ -80,6 +81,19 @@ pub fn status_line_items_edit(items: &[String]) -> ConfigEdit {
|
||||
}
|
||||
}
|
||||
|
||||
/// Produces a config edit that sets `[tui].terminal_title` to an explicit ordered list.
|
||||
///
|
||||
/// The array is written even when it is empty so "disabled title updates" stays
|
||||
/// distinct from "unset, so use defaults".
|
||||
pub fn terminal_title_items_edit(items: &[String]) -> ConfigEdit {
|
||||
let array = items.iter().cloned().collect::<toml_edit::Array>();
|
||||
|
||||
ConfigEdit::SetPath {
|
||||
segments: vec!["tui".to_string(), "terminal_title".to_string()],
|
||||
value: TomlItem::Value(array.into()),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn model_availability_nux_count_edits(shown_count: &HashMap<String, u32>) -> Vec<ConfigEdit> {
|
||||
let mut shown_count_entries: Vec<_> = shown_count.iter().collect();
|
||||
shown_count_entries.sort_unstable_by(|(left, _), (right, _)| left.cmp(right));
|
||||
|
||||
@@ -359,6 +359,11 @@ pub struct Config {
|
||||
/// `current-dir`.
|
||||
pub tui_status_line: Option<Vec<String>>,
|
||||
|
||||
/// Ordered list of terminal title item identifiers for the TUI.
|
||||
///
|
||||
/// When unset, the TUI defaults to: `project` and `spinner`.
|
||||
pub tui_terminal_title: Option<Vec<String>>,
|
||||
|
||||
/// Syntax highlighting theme override (kebab-case name).
|
||||
pub tui_theme: Option<String>,
|
||||
|
||||
@@ -493,6 +498,10 @@ pub struct Config {
|
||||
/// Base URL for requests to ChatGPT (as opposed to the OpenAI API).
|
||||
pub chatgpt_base_url: String,
|
||||
|
||||
/// Experimental / do not use. Overrides the URL used when connecting to
|
||||
/// a remote exec server.
|
||||
pub experimental_exec_server_url: Option<String>,
|
||||
|
||||
/// Machine-local realtime audio device preferences used by realtime voice.
|
||||
pub realtime_audio: RealtimeAudioConfig,
|
||||
|
||||
@@ -1393,6 +1402,10 @@ pub struct ConfigToml {
|
||||
/// Base URL override for the built-in `openai` model provider.
|
||||
pub openai_base_url: Option<String>,
|
||||
|
||||
/// Experimental / do not use. Overrides the URL used when connecting to
|
||||
/// a remote exec server.
|
||||
pub experimental_exec_server_url: Option<String>,
|
||||
|
||||
/// Machine-local realtime audio device preferences used by realtime voice.
|
||||
#[serde(default)]
|
||||
pub audio: Option<RealtimeAudioToml>,
|
||||
@@ -2745,6 +2758,7 @@ impl Config {
|
||||
.chatgpt_base_url
|
||||
.or(cfg.chatgpt_base_url)
|
||||
.unwrap_or("https://chatgpt.com/backend-api/".to_string()),
|
||||
experimental_exec_server_url: cfg.experimental_exec_server_url,
|
||||
realtime_audio: cfg
|
||||
.audio
|
||||
.map_or_else(RealtimeAudioConfig::default, |audio| RealtimeAudioConfig {
|
||||
@@ -2814,6 +2828,7 @@ impl Config {
|
||||
.map(|t| t.alternate_screen)
|
||||
.unwrap_or_default(),
|
||||
tui_status_line: cfg.tui.as_ref().and_then(|t| t.status_line.clone()),
|
||||
tui_terminal_title: cfg.tui.as_ref().and_then(|t| t.terminal_title.clone()),
|
||||
tui_theme: cfg.tui.as_ref().and_then(|t| t.theme.clone()),
|
||||
otel: {
|
||||
let t: OtelConfigToml = cfg.otel.unwrap_or_default();
|
||||
|
||||
@@ -752,6 +752,13 @@ pub struct Tui {
|
||||
#[serde(default)]
|
||||
pub status_line: Option<Vec<String>>,
|
||||
|
||||
/// Ordered list of terminal title item identifiers.
|
||||
///
|
||||
/// When set, the TUI renders the selected items into the terminal window/tab title.
|
||||
/// When unset, the TUI defaults to: `spinner` and `project`.
|
||||
#[serde(default)]
|
||||
pub terminal_title: Option<Vec<String>>,
|
||||
|
||||
/// Syntax highlighting theme name (kebab-case).
|
||||
///
|
||||
/// When set, overrides automatic light/dark theme detection.
|
||||
|
||||
@@ -177,8 +177,7 @@ impl ContextManager {
|
||||
/// Returns true when a tool image was replaced, false otherwise.
|
||||
pub(crate) fn replace_last_turn_images(&mut self, placeholder: &str) -> bool {
|
||||
let Some(index) = self.items.iter().rposition(|item| {
|
||||
matches!(item, ResponseItem::FunctionCallOutput { .. })
|
||||
|| matches!(item, ResponseItem::Message { role, .. } if role == "user")
|
||||
matches!(item, ResponseItem::FunctionCallOutput { .. }) || is_user_turn_boundary(item)
|
||||
}) else {
|
||||
return false;
|
||||
};
|
||||
@@ -200,7 +199,7 @@ impl ContextManager {
|
||||
}
|
||||
replaced
|
||||
}
|
||||
ResponseItem::Message { role, .. } if role == "user" => false,
|
||||
ResponseItem::Message { .. } => false,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
@@ -250,11 +249,7 @@ impl ContextManager {
|
||||
|
||||
fn get_non_last_reasoning_items_tokens(&self) -> i64 {
|
||||
// Get reasoning items excluding all the ones after the last user message.
|
||||
let Some(last_user_index) = self
|
||||
.items
|
||||
.iter()
|
||||
.rposition(|item| matches!(item, ResponseItem::Message { role, .. } if role == "user"))
|
||||
else {
|
||||
let Some(last_user_index) = self.items.iter().rposition(is_user_turn_boundary) else {
|
||||
return 0;
|
||||
};
|
||||
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
use codex_protocol::items::HookPromptItem;
|
||||
use codex_protocol::items::parse_hook_prompt_fragment;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG;
|
||||
@@ -94,10 +96,7 @@ const CONTEXTUAL_USER_FRAGMENTS: &[ContextualUserFragmentDefinition] = &[
|
||||
SUBAGENT_NOTIFICATION_FRAGMENT,
|
||||
];
|
||||
|
||||
pub(crate) fn is_contextual_user_fragment(content_item: &ContentItem) -> bool {
|
||||
let ContentItem::InputText { text } = content_item else {
|
||||
return false;
|
||||
};
|
||||
fn is_standard_contextual_user_text(text: &str) -> bool {
|
||||
CONTEXTUAL_USER_FRAGMENTS
|
||||
.iter()
|
||||
.any(|definition| definition.matches_text(text))
|
||||
@@ -118,6 +117,40 @@ pub(crate) fn is_memory_excluded_contextual_user_fragment(content_item: &Content
|
||||
AGENTS_MD_FRAGMENT.matches_text(text) || SKILL_FRAGMENT.matches_text(text)
|
||||
}
|
||||
|
||||
pub(crate) fn is_contextual_user_fragment(content_item: &ContentItem) -> bool {
|
||||
let ContentItem::InputText { text } = content_item else {
|
||||
return false;
|
||||
};
|
||||
parse_hook_prompt_fragment(text).is_some() || is_standard_contextual_user_text(text)
|
||||
}
|
||||
|
||||
pub(crate) fn parse_visible_hook_prompt_message(
|
||||
id: Option<&String>,
|
||||
content: &[ContentItem],
|
||||
) -> Option<HookPromptItem> {
|
||||
let mut fragments = Vec::new();
|
||||
|
||||
for content_item in content {
|
||||
let ContentItem::InputText { text } = content_item else {
|
||||
return None;
|
||||
};
|
||||
if let Some(fragment) = parse_hook_prompt_fragment(text) {
|
||||
fragments.push(fragment);
|
||||
continue;
|
||||
}
|
||||
if is_standard_contextual_user_text(text) {
|
||||
continue;
|
||||
}
|
||||
return None;
|
||||
}
|
||||
|
||||
if fragments.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(HookPromptItem::from_fragments(id, fragments))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "contextual_user_message_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use super::*;
|
||||
use codex_protocol::items::HookPromptFragment;
|
||||
use codex_protocol::items::build_hook_prompt_message;
|
||||
|
||||
#[test]
|
||||
fn detects_environment_context_fragment() {
|
||||
@@ -61,3 +63,36 @@ fn classifies_memory_excluded_fragments() {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_hook_prompt_fragment_and_roundtrips_escaping() {
|
||||
let message = build_hook_prompt_message(&[HookPromptFragment::from_single_hook(
|
||||
r#"Retry with "waves" & <tides>"#,
|
||||
"hook-run-1",
|
||||
)])
|
||||
.expect("hook prompt message");
|
||||
|
||||
let ResponseItem::Message { content, .. } = message else {
|
||||
panic!("expected hook prompt response item");
|
||||
};
|
||||
|
||||
let [content_item] = content.as_slice() else {
|
||||
panic!("expected a single content item");
|
||||
};
|
||||
|
||||
assert!(is_contextual_user_fragment(content_item));
|
||||
|
||||
let ContentItem::InputText { text } = content_item else {
|
||||
panic!("expected input text content item");
|
||||
};
|
||||
let parsed =
|
||||
parse_visible_hook_prompt_message(None, content.as_slice()).expect("visible hook prompt");
|
||||
assert_eq!(
|
||||
parsed.fragments,
|
||||
vec![HookPromptFragment {
|
||||
text: r#"Retry with "waves" & <tides>"#.to_string(),
|
||||
hook_run_id: "hook-run-1".to_string(),
|
||||
}],
|
||||
);
|
||||
assert!(!text.contains(""waves" & <tides>"));
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ use tracing::warn;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::contextual_user_message::is_contextual_user_fragment;
|
||||
use crate::contextual_user_message::parse_visible_hook_prompt_message;
|
||||
use crate::web_search::web_search_action_detail;
|
||||
|
||||
pub(crate) fn is_contextual_user_message_content(message: &[ContentItem]) -> bool {
|
||||
@@ -100,7 +101,9 @@ pub fn parse_turn_item(item: &ResponseItem) -> Option<TurnItem> {
|
||||
phase,
|
||||
..
|
||||
} => match role.as_str() {
|
||||
"user" => parse_user_message(content).map(TurnItem::UserMessage),
|
||||
"user" => parse_visible_hook_prompt_message(id.as_ref(), content)
|
||||
.map(TurnItem::HookPrompt)
|
||||
.or_else(|| parse_user_message(content).map(TurnItem::UserMessage)),
|
||||
"assistant" => Some(TurnItem::AgentMessage(parse_agent_message(
|
||||
id.as_ref(),
|
||||
content,
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
use super::parse_turn_item;
|
||||
use codex_protocol::items::AgentMessageContent;
|
||||
use codex_protocol::items::HookPromptFragment;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::items::WebSearchItem;
|
||||
use codex_protocol::items::build_hook_prompt_message;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ReasoningItemContent;
|
||||
use codex_protocol::models::ReasoningItemReasoningSummary;
|
||||
@@ -208,6 +210,67 @@ fn skips_user_instructions_and_env() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_hook_prompt_message_as_distinct_turn_item() {
|
||||
let item = build_hook_prompt_message(&[HookPromptFragment::from_single_hook(
|
||||
"Retry with exactly the phrase meow meow meow.",
|
||||
"hook-run-1",
|
||||
)])
|
||||
.expect("hook prompt message");
|
||||
|
||||
let turn_item = parse_turn_item(&item).expect("expected hook prompt turn item");
|
||||
|
||||
match turn_item {
|
||||
TurnItem::HookPrompt(hook_prompt) => {
|
||||
assert_eq!(hook_prompt.fragments.len(), 1);
|
||||
assert_eq!(
|
||||
hook_prompt.fragments[0],
|
||||
HookPromptFragment {
|
||||
text: "Retry with exactly the phrase meow meow meow.".to_string(),
|
||||
hook_run_id: "hook-run-1".to_string(),
|
||||
}
|
||||
);
|
||||
}
|
||||
other => panic!("expected TurnItem::HookPrompt, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_hook_prompt_and_hides_other_contextual_fragments() {
|
||||
let item = ResponseItem::Message {
|
||||
id: Some("msg-1".to_string()),
|
||||
role: "user".to_string(),
|
||||
content: vec![
|
||||
ContentItem::InputText {
|
||||
text: "<environment_context>ctx</environment_context>".to_string(),
|
||||
},
|
||||
ContentItem::InputText {
|
||||
text:
|
||||
"<hook_prompt hook_run_id=\"hook-run-1\">Retry with care & joy.</hook_prompt>"
|
||||
.to_string(),
|
||||
},
|
||||
],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
|
||||
let turn_item = parse_turn_item(&item).expect("expected hook prompt turn item");
|
||||
|
||||
match turn_item {
|
||||
TurnItem::HookPrompt(hook_prompt) => {
|
||||
assert_eq!(hook_prompt.id, "msg-1");
|
||||
assert_eq!(
|
||||
hook_prompt.fragments,
|
||||
vec![HookPromptFragment {
|
||||
text: "Retry with care & joy.".to_string(),
|
||||
hook_run_id: "hook-run-1".to_string(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
other => panic!("expected TurnItem::HookPrompt, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_agent_message() {
|
||||
let item = ResponseItem::Message {
|
||||
|
||||
@@ -112,6 +112,7 @@ impl ToolOrchestrator {
|
||||
let otel_tn = &tool_ctx.tool_name;
|
||||
let otel_ci = &tool_ctx.call_id;
|
||||
let otel_user = ToolDecisionSource::User;
|
||||
let otel_automated_reviewer = ToolDecisionSource::AutomatedReviewer;
|
||||
let otel_cfg = ToolDecisionSource::Config;
|
||||
|
||||
// 1) Approval
|
||||
@@ -136,8 +137,13 @@ impl ToolOrchestrator {
|
||||
network_approval_context: None,
|
||||
};
|
||||
let decision = tool.start_approval_async(req, approval_ctx).await;
|
||||
let otel_source = if routes_approval_to_guardian(turn_ctx) {
|
||||
otel_automated_reviewer.clone()
|
||||
} else {
|
||||
otel_user.clone()
|
||||
};
|
||||
|
||||
otel.tool_decision(otel_tn, otel_ci, &decision, otel_user.clone());
|
||||
otel.tool_decision(otel_tn, otel_ci, &decision, otel_source);
|
||||
|
||||
match decision {
|
||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||
@@ -286,7 +292,12 @@ impl ToolOrchestrator {
|
||||
};
|
||||
|
||||
let decision = tool.start_approval_async(req, approval_ctx).await;
|
||||
otel.tool_decision(otel_tn, otel_ci, &decision, otel_user);
|
||||
let otel_source = if routes_approval_to_guardian(turn_ctx) {
|
||||
otel_automated_reviewer
|
||||
} else {
|
||||
otel_user
|
||||
};
|
||||
otel.tool_decision(otel_tn, otel_ci, &decision, otel_source);
|
||||
|
||||
match decision {
|
||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||
|
||||
@@ -4,6 +4,7 @@ use std::path::Path;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_core::features::Feature;
|
||||
use codex_protocol::items::parse_hook_prompt_fragment;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -82,6 +83,48 @@ else:
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_parallel_stop_hooks(home: &Path, prompts: &[&str]) -> Result<()> {
|
||||
let hook_entries = prompts
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, prompt)| {
|
||||
let script_path = home.join(format!("stop_hook_{index}.py"));
|
||||
let script = format!(
|
||||
r#"import json
|
||||
import sys
|
||||
|
||||
payload = json.load(sys.stdin)
|
||||
if payload["stop_hook_active"]:
|
||||
print(json.dumps({{"systemMessage": "done"}}))
|
||||
else:
|
||||
print(json.dumps({{"decision": "block", "reason": {prompt:?}}}))
|
||||
"#
|
||||
);
|
||||
fs::write(&script_path, script).with_context(|| {
|
||||
format!(
|
||||
"write stop hook script fixture at {}",
|
||||
script_path.display()
|
||||
)
|
||||
})?;
|
||||
Ok(serde_json::json!({
|
||||
"type": "command",
|
||||
"command": format!("python3 {}", script_path.display()),
|
||||
}))
|
||||
})
|
||||
.collect::<Result<Vec<_>>>()?;
|
||||
|
||||
let hooks = serde_json::json!({
|
||||
"hooks": {
|
||||
"Stop": [{
|
||||
"hooks": hook_entries,
|
||||
}]
|
||||
}
|
||||
});
|
||||
|
||||
fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_user_prompt_submit_hook(
|
||||
home: &Path,
|
||||
blocked_prompt: &str,
|
||||
@@ -168,7 +211,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn rollout_developer_texts(text: &str) -> Result<Vec<String>> {
|
||||
fn rollout_hook_prompt_texts(text: &str) -> Result<Vec<String>> {
|
||||
let mut texts = Vec::new();
|
||||
for line in text.lines() {
|
||||
let trimmed = line.trim();
|
||||
@@ -177,11 +220,13 @@ fn rollout_developer_texts(text: &str) -> Result<Vec<String>> {
|
||||
}
|
||||
let rollout: RolloutLine = serde_json::from_str(trimmed).context("parse rollout line")?;
|
||||
if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rollout.item
|
||||
&& role == "developer"
|
||||
&& role == "user"
|
||||
{
|
||||
for item in content {
|
||||
if let ContentItem::InputText { text } = item {
|
||||
texts.push(text);
|
||||
if let ContentItem::InputText { text } = item
|
||||
&& let Some(fragment) = parse_hook_prompt_fragment(&text)
|
||||
{
|
||||
texts.push(fragment.text);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -189,6 +234,16 @@ fn rollout_developer_texts(text: &str) -> Result<Vec<String>> {
|
||||
Ok(texts)
|
||||
}
|
||||
|
||||
fn request_hook_prompt_texts(
|
||||
request: &core_test_support::responses::ResponsesRequest,
|
||||
) -> Vec<String> {
|
||||
request
|
||||
.message_input_texts("user")
|
||||
.into_iter()
|
||||
.filter_map(|text| parse_hook_prompt_fragment(&text).map(|fragment| fragment.text))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn read_stop_hook_inputs(home: &Path) -> Result<Vec<serde_json::Value>> {
|
||||
fs::read_to_string(home.join("stop_hook_log.jsonl"))
|
||||
.context("read stop hook log")?
|
||||
@@ -298,23 +353,18 @@ async fn stop_hook_can_block_multiple_times_in_same_turn() -> Result<()> {
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 3);
|
||||
assert!(
|
||||
requests[1]
|
||||
.message_input_texts("developer")
|
||||
.contains(&FIRST_CONTINUATION_PROMPT.to_string()),
|
||||
"second request should include the first continuation prompt",
|
||||
assert_eq!(
|
||||
request_hook_prompt_texts(&requests[1]),
|
||||
vec![FIRST_CONTINUATION_PROMPT.to_string()],
|
||||
"second request should include the first continuation prompt as user hook context",
|
||||
);
|
||||
assert!(
|
||||
requests[2]
|
||||
.message_input_texts("developer")
|
||||
.contains(&FIRST_CONTINUATION_PROMPT.to_string()),
|
||||
"third request should retain the first continuation prompt from history",
|
||||
);
|
||||
assert!(
|
||||
requests[2]
|
||||
.message_input_texts("developer")
|
||||
.contains(&SECOND_CONTINUATION_PROMPT.to_string()),
|
||||
"third request should include the second continuation prompt",
|
||||
assert_eq!(
|
||||
request_hook_prompt_texts(&requests[2]),
|
||||
vec![
|
||||
FIRST_CONTINUATION_PROMPT.to_string(),
|
||||
SECOND_CONTINUATION_PROMPT.to_string(),
|
||||
],
|
||||
"third request should retain hook prompts in user history",
|
||||
);
|
||||
|
||||
let hook_inputs = read_stop_hook_inputs(test.codex_home_path())?;
|
||||
@@ -356,13 +406,13 @@ async fn stop_hook_can_block_multiple_times_in_same_turn() -> Result<()> {
|
||||
|
||||
let rollout_path = test.codex.rollout_path().expect("rollout path");
|
||||
let rollout_text = fs::read_to_string(&rollout_path)?;
|
||||
let developer_texts = rollout_developer_texts(&rollout_text)?;
|
||||
let hook_prompt_texts = rollout_hook_prompt_texts(&rollout_text)?;
|
||||
assert!(
|
||||
developer_texts.contains(&FIRST_CONTINUATION_PROMPT.to_string()),
|
||||
hook_prompt_texts.contains(&FIRST_CONTINUATION_PROMPT.to_string()),
|
||||
"rollout should persist the first continuation prompt",
|
||||
);
|
||||
assert!(
|
||||
developer_texts.contains(&SECOND_CONTINUATION_PROMPT.to_string()),
|
||||
hook_prompt_texts.contains(&SECOND_CONTINUATION_PROMPT.to_string()),
|
||||
"rollout should persist the second continuation prompt",
|
||||
);
|
||||
|
||||
@@ -481,11 +531,76 @@ async fn resumed_thread_keeps_stop_continuation_prompt_in_history() -> Result<()
|
||||
resumed.submit_turn("and now continue").await?;
|
||||
|
||||
let resumed_request = resumed_response.single_request();
|
||||
assert!(
|
||||
resumed_request
|
||||
.message_input_texts("developer")
|
||||
.contains(&FIRST_CONTINUATION_PROMPT.to_string()),
|
||||
"resumed request should keep the persisted continuation prompt in history",
|
||||
assert_eq!(
|
||||
request_hook_prompt_texts(&resumed_request),
|
||||
vec![FIRST_CONTINUATION_PROMPT.to_string()],
|
||||
"resumed request should keep the persisted continuation prompt in user history",
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn multiple_blocking_stop_hooks_persist_multiple_hook_prompt_fragments() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_assistant_message("msg-1", "draft one"),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-2", "final draft"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_pre_build_hook(|home| {
|
||||
if let Err(error) = write_parallel_stop_hooks(
|
||||
home,
|
||||
&[FIRST_CONTINUATION_PROMPT, SECOND_CONTINUATION_PROMPT],
|
||||
) {
|
||||
panic!("failed to write parallel stop hook fixtures: {error}");
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodexHooks)
|
||||
.expect("test config should allow feature update");
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.submit_turn("hello again").await?;
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
assert_eq!(
|
||||
request_hook_prompt_texts(&requests[1]),
|
||||
vec![
|
||||
FIRST_CONTINUATION_PROMPT.to_string(),
|
||||
SECOND_CONTINUATION_PROMPT.to_string(),
|
||||
],
|
||||
"second request should receive one user hook prompt message with both fragments",
|
||||
);
|
||||
|
||||
let rollout_path = test.codex.rollout_path().expect("rollout path");
|
||||
let rollout_text = fs::read_to_string(&rollout_path)?;
|
||||
assert_eq!(
|
||||
rollout_hook_prompt_texts(&rollout_text)?,
|
||||
vec![
|
||||
FIRST_CONTINUATION_PROMPT.to_string(),
|
||||
SECOND_CONTINUATION_PROMPT.to_string(),
|
||||
],
|
||||
"rollout should preserve both hook prompt fragments in order",
|
||||
);
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -683,6 +683,8 @@ async fn remote_models_do_not_append_removed_builtin_presets() -> Result<()> {
|
||||
1,
|
||||
"expected a single /models request"
|
||||
);
|
||||
// Keep the mock server alive until after async assertions complete.
|
||||
drop(server);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user