Merge remote-tracking branch 'refs/remotes/origin/main' into abhinav/codex-env-file-conflict-fix

# Conflicts:
#	codex-rs/core/src/tools/runtimes/mod.rs
#	codex-rs/core/src/tools/runtimes/shell.rs
#	codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
#	codex-rs/core/src/tools/runtimes/unified_exec.rs
This commit is contained in:
Abhinav Vedmala
2026-05-28 15:16:25 -07:00
516 changed files with 19606 additions and 5332 deletions
+43 -26
View File
@@ -317,6 +317,7 @@ impl AgentControl {
.services
.analytics_events_client,
client_metadata,
new_thread.thread.codex.session.session_id(),
new_thread.thread_id,
/*parent_thread_id*/ None,
thread_config,
@@ -703,22 +704,6 @@ impl AgentControl {
result
}
/// Append a prebuilt message to an existing agent thread outside the normal user-input path.
#[cfg(test)]
pub(crate) async fn append_message(
&self,
agent_id: ThreadId,
message: ResponseItem,
) -> CodexResult<String> {
let state = self.upgrade()?;
self.handle_thread_request_result(
agent_id,
&state,
state.append_message(agent_id, message).await,
)
.await
}
pub(crate) async fn send_inter_agent_communication(
&self,
agent_id: ThreadId,
@@ -787,15 +772,45 @@ impl AgentControl {
/// agent and any live descendants reached from the in-memory tree.
pub(crate) async fn close_agent(&self, agent_id: ThreadId) -> CodexResult<String> {
let state = self.upgrade()?;
if let Ok(thread) = state.get_thread(agent_id).await
&& let Some(state_db_ctx) = thread.state_db()
&& let Err(err) = state_db_ctx
.set_thread_spawn_edge_status(agent_id, DirectionalThreadSpawnEdgeStatus::Closed)
.await
{
warn!("failed to persist thread-spawn edge status for {agent_id}: {err}");
let known_agent = self.state.agent_metadata_for_thread(agent_id).is_some();
match state.get_thread(agent_id).await {
Ok(thread) => {
if let Some(state_db_ctx) = thread.state_db()
&& let Err(err) = state_db_ctx
.set_thread_spawn_edge_status(
agent_id,
DirectionalThreadSpawnEdgeStatus::Closed,
)
.await
{
warn!("failed to persist thread-spawn edge status for {agent_id}: {err}");
}
}
Err(CodexErr::ThreadNotFound(_)) if known_agent => {
if let Some(state_db_ctx) = state.state_db()
&& let Err(err) = state_db_ctx
.set_thread_spawn_edge_status(
agent_id,
DirectionalThreadSpawnEdgeStatus::Closed,
)
.await
{
return Err(CodexErr::Fatal(format!(
"failed to persist stale thread-spawn edge status for {agent_id}: {err}"
)));
}
}
Err(CodexErr::ThreadNotFound(_)) => {}
Err(err) => {
warn!("failed to inspect agent before close {agent_id}: {err}");
}
}
match Box::pin(self.shutdown_agent_tree(agent_id)).await {
Err(CodexErr::ThreadNotFound(_)) | Err(CodexErr::InternalAgentDied) if known_agent => {
Ok(String::new())
}
result => result,
}
Box::pin(self.shutdown_agent_tree(agent_id)).await
}
/// Shut down `agent_id` and any live descendants reachable from the in-memory spawn tree.
@@ -1281,8 +1296,10 @@ pub(crate) fn render_input_preview(initial_operation: &Op) -> String {
UserInput::LocalImage { path, .. } => {
format!("[local_image:{}]", path.display())
}
UserInput::Skill { name, path } => format!("[skill:${name}]({})", path.display()),
UserInput::Mention { name, path } => format!("[mention:${name}]({path})"),
UserInput::Skill { name, path, .. } => {
format!("[skill:${name}]({})", path.display())
}
UserInput::Mention { name, path, .. } => format!("[mention:${name}]({path})"),
_ => "[input]".to_string(),
})
.collect::<Vec<_>>()
+102 -54
View File
@@ -518,60 +518,6 @@ async fn send_inter_agent_communication_without_turn_queues_message_without_trig
));
}
#[tokio::test]
async fn append_message_records_assistant_message() {
let harness = AgentControlHarness::new().await;
let (thread_id, thread) = harness.start_thread().await;
let message =
"author: /root\nrecipient: /root/worker\nother_recipients: []\nContent: hello from tests";
let submission_id = harness
.control
.append_message(
thread_id,
ResponseItem::Message {
id: None,
role: "assistant".to_string(),
content: vec![ContentItem::InputText {
text: message.to_string(),
}],
phase: None,
},
)
.await
.expect("append_message should succeed");
assert!(!submission_id.is_empty());
timeout(Duration::from_secs(5), async {
loop {
let history_items = thread
.codex
.session
.clone_history()
.await
.raw_items()
.to_vec();
let recorded = history_items.iter().any(|item| {
matches!(
item,
ResponseItem::Message { role, content, .. }
if role == "assistant"
&& content.iter().any(|content_item| matches!(
content_item,
ContentItem::InputText { text } if text == message
))
)
});
if recorded {
break;
}
sleep(Duration::from_millis(10)).await;
}
})
.await
.expect("assistant message should be recorded");
}
#[tokio::test]
async fn spawn_agent_creates_thread_and_sends_prompt() {
let harness = AgentControlHarness::new().await;
@@ -1113,6 +1059,108 @@ async fn spawn_agent_fork_last_n_turns_keeps_only_recent_turns() {
.expect("parent shutdown should submit");
}
#[tokio::test]
async fn spawn_agent_fork_last_n_turns_drops_parent_startup_prefix_when_under_limit() {
let harness = AgentControlHarness::new().await;
let (parent_thread_id, parent_thread) = harness.start_thread().await;
let startup_turn_context = parent_thread.codex.session.new_default_turn().await;
parent_thread
.codex
.session
.record_conversation_items(
startup_turn_context.as_ref(),
&[ResponseItem::Message {
id: None,
role: "developer".to_string(),
content: vec![ContentItem::InputText {
text: "parent startup developer context".to_string(),
}],
phase: None,
}],
)
.await;
parent_thread
.inject_user_message_without_turn("current parent task".to_string())
.await;
let spawn_turn_context = parent_thread.codex.session.new_default_turn().await;
let parent_spawn_call_id = "spawn-call-last-n-under-limit".to_string();
parent_thread
.codex
.session
.record_conversation_items(
spawn_turn_context.as_ref(),
&[spawn_agent_call(&parent_spawn_call_id)],
)
.await;
parent_thread
.codex
.session
.ensure_rollout_materialized()
.await;
parent_thread
.codex
.session
.flush_rollout()
.await
.expect("parent rollout should flush");
let child_thread_id = harness
.control
.spawn_agent_with_metadata(
harness.config.clone(),
text_input("child task"),
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id,
depth: 1,
agent_path: None,
agent_nickname: None,
agent_role: None,
})),
SpawnAgentOptions {
fork_parent_spawn_call_id: Some(parent_spawn_call_id),
fork_mode: Some(SpawnAgentForkMode::LastNTurns(2)),
..Default::default()
},
)
.await
.expect("bounded forked spawn should drop startup prefix")
.thread_id;
let child_thread = harness
.manager
.get_thread(child_thread_id)
.await
.expect("child thread should be registered");
let history = child_thread.codex.session.clone_history().await;
assert!(
history_contains_text(history.raw_items(), "current parent task"),
"bounded fork should retain the requested recent parent turn"
);
assert!(
!history_contains_text(history.raw_items(), "parent startup developer context"),
"bounded fork should drop parent startup context even when fewer turns exist than requested"
);
assert!(
child_thread
.codex
.session
.reference_context_item()
.await
.is_none(),
"bounded forked child should still rebuild context after truncating the cached prefix"
);
let _ = harness
.control
.shutdown_live_agent(child_thread_id)
.await
.expect("child shutdown should submit");
let _ = parent_thread
.submit(Op::Shutdown {})
.await
.expect("parent shutdown should submit");
}
#[tokio::test]
async fn spawn_agent_fork_last_n_turns_strips_parent_usage_hints() {
let harness = AgentControlHarness::new().await;
+20 -3
View File
@@ -218,6 +218,7 @@ impl RequestRouteTelemetry {
#[derive(Debug, Clone)]
pub struct ModelClient {
state: Arc<ModelClientState>,
prompt_cache_key_override: Option<String>,
}
/// A turn-scoped streaming session created from a [`ModelClient`].
@@ -352,9 +353,24 @@ impl ModelClient {
disable_websockets: AtomicBool::new(false),
cached_websocket_session: StdMutex::new(WebsocketSession::default()),
}),
prompt_cache_key_override: None,
}
}
pub(crate) fn with_prompt_cache_key_override(
mut self,
prompt_cache_key_override: Option<String>,
) -> Self {
self.prompt_cache_key_override = prompt_cache_key_override;
self
}
fn prompt_cache_key(&self) -> String {
self.prompt_cache_key_override
.clone()
.unwrap_or_else(|| self.state.thread_id.to_string())
}
/// Creates a fresh turn-scoped streaming session.
///
/// This constructor does not perform network I/O itself; the session opens a websocket lazily
@@ -383,7 +399,7 @@ impl ModelClient {
self.store_cached_websocket_session(WebsocketSession::default());
}
fn current_window_id(&self) -> String {
pub(crate) fn current_window_id(&self) -> String {
let thread_id = self.state.thread_id;
let window_generation = self.state.window_generation.load(Ordering::Relaxed);
format!("{thread_id}:{window_generation}")
@@ -441,6 +457,7 @@ impl ModelClient {
settings: CompactConversationRequestSettings,
session_telemetry: &SessionTelemetry,
compaction_trace: &CompactionTraceContext,
turn_metadata_header: Option<&str>,
) -> Result<Vec<ResponseItem>> {
if prompt.input.is_empty() {
return Ok(Vec::new());
@@ -496,7 +513,7 @@ impl ModelClient {
extra_headers.extend(build_responses_headers(
self.state.beta_features_header.as_deref(),
/*turn_state*/ None,
/*turn_metadata_header*/ None,
parse_turn_metadata_header(turn_metadata_header).as_ref(),
));
extra_headers.extend(self.build_responses_identity_headers());
extra_headers.extend(build_session_headers(
@@ -748,7 +765,7 @@ impl ModelClient {
&prompt.output_schema,
prompt.output_schema_strict,
);
let prompt_cache_key = Some(self.state.thread_id.to_string());
let prompt_cache_key = Some(self.prompt_cache_key());
let service_tier = model_info.service_tier_for_request(service_tier);
let request = ResponsesApiRequest {
model: model_info.slug.clone(),
+2
View File
@@ -109,6 +109,7 @@ pub(crate) async fn run_codex_thread_interactive(
emit_subagent_session_started(
&parent_session.services.analytics_events_client,
client_metadata,
codex.session.session_id(),
codex.session.conversation_id,
Some(parent_session.conversation_id),
thread_config,
@@ -217,6 +218,7 @@ pub(crate) async fn run_codex_thread_one_shot(
.send(Submission {
id: "shutdown".to_string(),
op: Op::Shutdown {},
client_user_message_id: None,
trace: None,
})
.await;
@@ -129,6 +129,7 @@ async fn forward_ops_preserves_submission_trace_context() {
let submission = Submission {
id: "sub-1".to_string(),
op: Op::Interrupt,
client_user_message_id: None,
trace: Some(codex_protocol::protocol::W3cTraceContext {
traceparent: Some(
"00-1234567890abcdef1234567890abcdef-1234567890abcdef-01".to_string(),
+25 -73
View File
@@ -230,6 +230,17 @@ impl CodexThread {
self.codex.submit_with_trace(op, trace).await
}
pub async fn submit_user_input_with_client_user_message_id(
&self,
op: Op,
trace: Option<W3cTraceContext>,
client_user_message_id: Option<String>,
) -> CodexResult<String> {
self.codex
.submit_user_input_with_client_user_message_id(op, trace, client_user_message_id)
.await
}
/// Persist whether this thread is eligible for future memory generation.
pub async fn set_thread_memory_mode(&self, mode: ThreadMemoryMode) -> anyhow::Result<()> {
self.codex.set_thread_memory_mode(mode).await
@@ -240,6 +251,7 @@ impl CodexThread {
input: Vec<UserInput>,
additional_context: BTreeMap<String, AdditionalContextEntry>,
expected_turn_id: Option<&str>,
client_user_message_id: Option<String>,
responsesapi_client_metadata: Option<HashMap<String, String>>,
) -> Result<String, SteerInputError> {
self.codex
@@ -247,6 +259,7 @@ impl CodexThread {
input,
additional_context,
expected_turn_id,
client_user_message_id,
responsesapi_client_metadata,
)
.await
@@ -260,7 +273,12 @@ impl CodexThread {
&self,
items: Vec<ResponseInputItem>,
) -> Result<(), Vec<ResponseInputItem>> {
self.codex.session.inject_response_items(items).await
let response_items = items.iter().cloned().map(ResponseItem::from).collect();
self.codex
.session
.inject_if_running(response_items)
.await
.map_err(|_| items)
}
pub async fn set_app_server_client_info(
@@ -366,58 +384,16 @@ impl CodexThread {
/// Records a user-role session-prefix message without creating a new user turn boundary.
pub(crate) async fn inject_user_message_without_turn(&self, message: String) {
let message = ResponseItem::Message {
let item = ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText { text: message }],
phase: None,
};
let pending_item = match pending_message_input_item(&message) {
Ok(pending_item) => pending_item,
Err(err) => {
debug_assert!(false, "session-prefix message append should succeed: {err}");
return;
}
};
if self
.codex
self.codex
.session
.inject_response_items(vec![pending_item])
.await
.is_err()
{
let turn_context = self.codex.session.new_default_turn().await;
self.codex
.session
.record_conversation_items(turn_context.as_ref(), &[message])
.await;
}
}
/// Append a prebuilt message to the thread history without treating it as a user turn.
///
/// If the thread already has an active turn, the message is queued as pending input for that
/// turn. Otherwise it is queued at session scope and a regular turn is started so the agent
/// can consume that pending input through the normal turn pipeline.
#[cfg(test)]
pub(crate) async fn append_message(&self, message: ResponseItem) -> CodexResult<String> {
let submission_id = uuid::Uuid::new_v4().to_string();
let pending_item = pending_message_input_item(&message)?;
if let Err(items) = self
.codex
.session
.inject_response_items(vec![pending_item])
.await
{
self.codex
.session
.input_queue
.queue_response_items_for_next_turn(items)
.await;
self.codex.session.maybe_start_turn_for_pending_work().await;
}
Ok(submission_id)
.inject_no_new_turn(vec![item], /*current_turn_context*/ None)
.await;
}
/// Append raw Responses API items to the thread's model-visible history.
@@ -437,7 +413,7 @@ impl CodexThread {
}
self.codex
.session
.record_conversation_items(turn_context.as_ref(), &items)
.inject_no_new_turn(items, Some(turn_context.as_ref()))
.await;
self.codex.session.flush_rollout().await?;
Ok(())
@@ -540,13 +516,7 @@ impl CodexThread {
let result = self
.codex
.session
.read_resource(
server,
ReadResourceRequestParams {
meta: None,
uri: uri.to_string(),
},
)
.read_resource(server, ReadResourceRequestParams::new(uri))
.await?;
Ok(serde_json::to_value(result)?)
@@ -604,21 +574,3 @@ impl CodexThread {
Ok(*guard)
}
}
fn pending_message_input_item(message: &ResponseItem) -> CodexResult<ResponseInputItem> {
match message {
ResponseItem::Message {
role,
content,
phase,
..
} => Ok(ResponseInputItem::Message {
role: role.clone(),
content: content.clone(),
phase: phase.clone(),
}),
_ => Err(CodexErr::InvalidRequest(
"append_message only supports ResponseItem::Message".to_string(),
)),
}
}
+10 -2
View File
@@ -13,6 +13,7 @@ use crate::session::PreviousTurnSettings;
use crate::session::session::Session;
use crate::session::turn::get_last_assistant_message_from_turn;
use crate::session::turn_context::TurnContext;
use crate::turn_metadata::CompactionTurnMetadata;
use crate::util::backoff;
use codex_analytics::CodexCompactionEvent;
use codex_analytics::CompactionImplementation;
@@ -128,6 +129,8 @@ async fn run_compact_task_inner(
reason: CompactionReason,
phase: CompactionPhase,
) -> CodexResult<()> {
let compaction_metadata =
CompactionTurnMetadata::new(trigger, reason, CompactionImplementation::Responses, phase);
let attempt = CompactionAnalyticsAttempt::begin(
sess.as_ref(),
turn_context.as_ref(),
@@ -153,6 +156,7 @@ async fn run_compact_task_inner(
Arc::clone(&turn_context),
input,
initial_context_injection,
compaction_metadata,
)
.await;
let status = compaction_status_from_result(&result);
@@ -173,6 +177,7 @@ async fn run_compact_task_inner_impl(
turn_context: Arc<TurnContext>,
input: Vec<UserInput>,
initial_context_injection: InitialContextInjection,
compaction_metadata: CompactionTurnMetadata,
) -> CodexResult<String> {
let compaction_item = TurnItem::ContextCompaction(ContextCompactionItem::new());
sess.emit_turn_item_started(&turn_context, &compaction_item)
@@ -204,7 +209,10 @@ async fn run_compact_task_inner_impl(
personality: turn_context.personality,
..Default::default()
};
let turn_metadata_header = turn_context.turn_metadata_state.current_header_value();
let window_id = sess.services.model_client.current_window_id();
let turn_metadata_header = turn_context
.turn_metadata_state
.current_header_value_for_compaction(&window_id, compaction_metadata);
let attempt_result = drain_to_completed(
&sess,
turn_context.as_ref(),
@@ -560,7 +568,7 @@ async fn drain_to_completed(
};
match event {
Ok(ResponseEvent::OutputItemDone(item)) => {
sess.record_into_history(std::slice::from_ref(&item), turn_context)
sess.record_conversation_items(turn_context, std::slice::from_ref(&item))
.await;
}
Ok(ResponseEvent::ServerReasoningIncluded(included)) => {
+20 -2
View File
@@ -17,6 +17,7 @@ use crate::hook_runtime::run_pre_compact_hooks;
use crate::session::session::Session;
use crate::session::turn::built_tools;
use crate::session::turn_context::TurnContext;
use crate::turn_metadata::CompactionTurnMetadata;
use codex_analytics::CompactionImplementation;
use codex_analytics::CompactionPhase;
use codex_analytics::CompactionReason;
@@ -89,6 +90,12 @@ async fn run_remote_compact_task_inner(
reason: CompactionReason,
phase: CompactionPhase,
) -> CodexResult<()> {
let compaction_metadata = CompactionTurnMetadata::new(
trigger,
reason,
CompactionImplementation::ResponsesCompact,
phase,
);
let attempt = CompactionAnalyticsAttempt::begin(
sess.as_ref(),
turn_context.as_ref(),
@@ -113,8 +120,13 @@ async fn run_remote_compact_task_inner(
return Err(CodexErr::TurnAborted);
}
}
let result =
run_remote_compact_task_inner_impl(sess, turn_context, initial_context_injection).await;
let result = run_remote_compact_task_inner_impl(
sess,
turn_context,
initial_context_injection,
compaction_metadata,
)
.await;
let status = compaction_status_from_result(&result);
let error = result.as_ref().err().map(ToString::to_string);
if result.is_ok() {
@@ -139,6 +151,7 @@ async fn run_remote_compact_task_inner_impl(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
initial_context_injection: InitialContextInjection,
compaction_metadata: CompactionTurnMetadata,
) -> CodexResult<()> {
let context_compaction_item = ContextCompactionItem::new();
// Use the UI compaction item ID as the trace compaction ID so protocol lifecycle events,
@@ -186,6 +199,10 @@ async fn run_remote_compact_task_inner_impl(
output_schema: None,
output_schema_strict: true,
};
let window_id = sess.services.model_client.current_window_id();
let turn_metadata_header = turn_context
.turn_metadata_state
.current_header_value_for_compaction(&window_id, compaction_metadata);
let mut new_history = sess
.services
.model_client
@@ -203,6 +220,7 @@ async fn run_remote_compact_task_inner_impl(
},
&turn_context.session_telemetry,
&compaction_trace,
turn_metadata_header.as_deref(),
)
.or_else(|err| async {
let total_usage_breakdown = sess.get_total_token_usage_breakdown().await;
+13 -1
View File
@@ -21,6 +21,7 @@ use crate::responses_retry::handle_retryable_response_stream_error;
use crate::session::session::Session;
use crate::session::turn::built_tools;
use crate::session::turn_context::TurnContext;
use crate::turn_metadata::CompactionTurnMetadata;
use codex_analytics::CompactionImplementation;
use codex_analytics::CompactionPhase;
use codex_analytics::CompactionReason;
@@ -105,6 +106,12 @@ async fn run_remote_compact_task_inner(
reason: CompactionReason,
phase: CompactionPhase,
) -> CodexResult<()> {
let compaction_metadata = CompactionTurnMetadata::new(
trigger,
reason,
CompactionImplementation::ResponsesCompactionV2,
phase,
);
let attempt = CompactionAnalyticsAttempt::begin(
sess.as_ref(),
turn_context.as_ref(),
@@ -134,6 +141,7 @@ async fn run_remote_compact_task_inner(
turn_context,
client_session,
initial_context_injection,
compaction_metadata,
)
.await;
let status = compaction_status_from_result(&result);
@@ -161,6 +169,7 @@ async fn run_remote_compact_task_inner_impl(
turn_context: &Arc<TurnContext>,
client_session: Option<&mut ModelClientSession>,
initial_context_injection: InitialContextInjection,
compaction_metadata: CompactionTurnMetadata,
) -> CodexResult<()> {
let context_compaction_item = ContextCompactionItem::new();
let compaction_trace = sess.services.rollout_thread_trace.compaction_trace_context(
@@ -208,7 +217,10 @@ async fn run_remote_compact_task_inner_impl(
output_schema_strict: true,
};
let turn_metadata_header = turn_context.turn_metadata_state.current_header_value();
let window_id = sess.services.model_client.current_window_id();
let turn_metadata_header = turn_context
.turn_metadata_state
.current_header_value_for_compaction(&window_id, compaction_metadata);
let trace_attempt = compaction_trace.start_attempt(&serde_json::json!({
"model": turn_context.model_info.slug.as_str(),
"instructions": prompt.base_instructions.text.as_str(),
+70 -21
View File
@@ -1554,7 +1554,6 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::
.await?;
let cwd_root = cwd.path().abs();
let memories_root = codex_home.path().join("memories").abs();
assert_eq!(
config.permissions.file_system_sandbox_policy(),
FileSystemSandboxPolicy::restricted(vec![
@@ -1576,18 +1575,12 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: memories_root.clone(),
},
access: FileSystemAccessMode::Write,
},
]),
);
assert_eq!(
&config.legacy_sandbox_policy(),
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![memories_root],
writable_roots: vec![],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
@@ -1816,7 +1809,7 @@ async fn managed_unrestricted_permission_profile_still_enables_network_requireme
}
#[tokio::test]
async fn permission_profile_override_applies_runtime_roots_to_legacy_projection()
async fn permission_profile_override_keeps_memories_root_out_of_legacy_projection()
-> std::io::Result<()> {
let codex_home = TempDir::new()?;
let cwd = TempDir::new()?;
@@ -1851,7 +1844,7 @@ async fn permission_profile_override_applies_runtime_roots_to_legacy_projection(
let memories_root = codex_home.path().join("memories").abs();
assert!(
config
!config
.permissions
.file_system_sandbox_policy()
.can_write_path_with_cwd(memories_root.as_path(), cwd.path())
@@ -1859,7 +1852,7 @@ async fn permission_profile_override_applies_runtime_roots_to_legacy_projection(
assert_eq!(
&config.legacy_sandbox_policy(),
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![memories_root],
writable_roots: vec![],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
@@ -2757,9 +2750,6 @@ async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root()
description: Some("Workspace access.".to_string()),
}]
);
let memories_root = AbsolutePathBuf::from_absolute_path(std::fs::canonicalize(
codex_home.path().join("memories"),
)?)?;
assert!(
config
.permissions
@@ -2769,7 +2759,7 @@ async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root()
assert_eq!(
&config.legacy_sandbox_policy(),
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![external_write_path, memories_root],
writable_roots: vec![external_write_path],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
@@ -4514,13 +4504,15 @@ async fn sqlite_home_defaults_to_codex_home_for_workspace_write() -> std::io::Re
}
#[tokio::test]
async fn workspace_write_always_includes_memories_root_once() -> std::io::Result<()> {
async fn workspace_write_includes_configured_writable_root_once_without_memories_root()
-> std::io::Result<()> {
let codex_home = TempDir::new()?;
let memories_root = codex_home.path().join("memories");
let writable_root = codex_home.path().join("writable").abs();
let config = Config::load_from_base_config_with_overrides(
ConfigToml {
sandbox_workspace_write: Some(SandboxWorkspaceWrite {
writable_roots: vec![memories_root.abs()],
writable_roots: vec![writable_root.clone(), writable_root.clone()],
..Default::default()
}),
..Default::default()
@@ -4540,21 +4532,22 @@ async fn workspace_write_always_includes_memories_root_once() -> std::io::Result
}
} else {
assert!(
memories_root.is_dir(),
"expected memories root directory to exist at {}",
!memories_root.exists(),
"expected config load not to create memories root at {}",
memories_root.display()
);
let expected_memories_root = memories_root.abs();
match &config.legacy_sandbox_policy() {
SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {
assert!(!writable_roots.contains(&expected_memories_root));
assert_eq!(
writable_roots
.iter()
.filter(|root| **root == expected_memories_root)
.filter(|root| **root == writable_root)
.count(),
1,
"expected single writable root entry for {}",
expected_memories_root.display()
writable_root.display()
);
}
other => panic!("expected workspace-write policy, got {other:?}"),
@@ -4564,6 +4557,62 @@ async fn workspace_write_always_includes_memories_root_once() -> std::io::Result
Ok(())
}
#[tokio::test]
async fn memory_tool_makes_memories_root_readable_without_creating_or_widening_writes()
-> std::io::Result<()> {
let codex_home = TempDir::new()?;
let cwd = TempDir::new()?;
let memories_root = codex_home.path().join("memories");
let memories_root_abs = memories_root.abs();
let config = Config::load_from_base_config_with_overrides(
ConfigToml {
features: Some(FeaturesToml::from(BTreeMap::from([(
"memories".to_string(),
true,
)]))),
sandbox_workspace_write: Some(SandboxWorkspaceWrite {
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
..Default::default()
}),
..Default::default()
},
ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
sandbox_mode: Some(SandboxMode::WorkspaceWrite),
..Default::default()
},
codex_home.abs(),
)
.await?;
assert!(
!memories_root.exists(),
"expected config load not to create memories root at {}",
memories_root.display()
);
let file_system_policy = config.permissions.file_system_sandbox_policy();
assert!(file_system_policy.can_read_path_with_cwd(memories_root_abs.as_path(), cwd.path()));
assert!(!file_system_policy.can_write_path_with_cwd(memories_root_abs.as_path(), cwd.path()));
if cfg!(target_os = "windows") {
match &config.legacy_sandbox_policy() {
SandboxPolicy::ReadOnly { .. } => {}
other => panic!("expected read-only policy on Windows, got {other:?}"),
}
} else {
match &config.legacy_sandbox_policy() {
SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {
assert!(!writable_roots.contains(&memories_root_abs));
}
other => panic!("expected workspace-write policy, got {other:?}"),
}
}
Ok(())
}
#[tokio::test]
async fn config_defaults_to_file_cli_auth_store_mode() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
+3 -3
View File
@@ -179,7 +179,7 @@ pub fn keymap_binding_clear_edit(context: &str, action: &str) -> ConfigEdit {
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));
shown_count_entries.sort_unstable_by_key(|(left, _)| *left);
let mut edits = vec![ConfigEdit::ClearPath {
segments: vec!["tui".to_string(), "model_availability_nux".to_string()],
@@ -354,7 +354,7 @@ mod document_helpers {
if !config.tools.is_empty() {
let mut tools = new_implicit_table();
let mut tool_entries: Vec<_> = config.tools.iter().collect();
tool_entries.sort_by(|(left, _), (right, _)| left.cmp(right));
tool_entries.sort_by_key(|(name, _)| *name);
for (name, tool_config) in tool_entries {
tools.insert(name, serialize_mcp_server_tool(tool_config));
}
@@ -501,7 +501,7 @@ mod document_helpers {
I: IntoIterator<Item = (&'a String, &'a String)>,
{
let mut entries: Vec<_> = pairs.into_iter().collect();
entries.sort_by(|(a, _), (b, _)| a.cmp(b));
entries.sort_by_key(|(key, _)| *key);
let mut table = TomlTable::new();
table.set_implicit(false);
for (key, val) in entries {
+10 -77
View File
@@ -2631,9 +2631,8 @@ impl Config {
Some(WindowsSandboxModeToml::Unelevated) => WindowsSandboxLevel::RestrictedToken,
None => WindowsSandboxLevel::from_features(&features),
};
let memories_config: MemoriesConfig = cfg.memories.clone().unwrap_or_default().into();
let memories_root = memory_root(&codex_home);
std::fs::create_dir_all(&memories_root)?;
let internal_writable_roots = vec![memories_root];
let profiles_are_active = effective_permission_selection.profiles_are_active(
default_permissions_override.as_deref(),
@@ -2701,8 +2700,8 @@ impl Config {
file_system_sandbox_policy,
mut active_permission_profile,
mut profile_workspace_roots,
) = if let Some(mut permission_profile) = permission_profile {
let (mut file_system_sandbox_policy, network_sandbox_policy) =
) = if let Some(permission_profile) = permission_profile {
let (file_system_sandbox_policy, _network_sandbox_policy) =
permission_profile.to_runtime_permissions();
let configured_network_proxy_config =
if profile_allows_configured_network_proxy(&permission_profile)
@@ -2726,30 +2725,6 @@ impl Config {
} else {
NetworkProxyConfig::default()
};
let materialized_file_system_sandbox_policy = file_system_sandbox_policy
.clone()
.materialize_project_roots_with_workspace_roots(&workspace_roots);
let materialized_permission_profile =
PermissionProfile::from_runtime_permissions_with_enforcement(
permission_profile.enforcement(),
&materialized_file_system_sandbox_policy,
network_sandbox_policy,
);
let sandbox_policy = compatibility_sandbox_policy_for_permission_profile(
&materialized_permission_profile,
&materialized_file_system_sandbox_policy,
network_sandbox_policy,
resolved_cwd.as_path(),
);
if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) {
file_system_sandbox_policy = file_system_sandbox_policy
.with_additional_legacy_workspace_writable_roots(&internal_writable_roots);
permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement(
permission_profile.enforcement(),
&file_system_sandbox_policy,
network_sandbox_policy,
);
}
(
configured_network_proxy_config,
permission_profile,
@@ -2794,7 +2769,7 @@ impl Config {
dedupe_absolute_paths(&mut configured_workspace_roots);
file_system_sandbox_policy = file_system_sandbox_policy
.with_materialized_project_roots_for_workspace_roots(&configured_workspace_roots);
let mut permission_profile = if let Some(permission_profile) =
let permission_profile = if let Some(permission_profile) =
builtin_permission_profile(default_permissions, builtin_workspace_write_settings)
{
permission_profile
@@ -2804,30 +2779,6 @@ impl Config {
network_sandbox_policy,
)
};
let materialized_file_system_sandbox_policy = file_system_sandbox_policy
.clone()
.materialize_project_roots_with_workspace_roots(&workspace_roots);
let materialized_permission_profile =
PermissionProfile::from_runtime_permissions_with_enforcement(
permission_profile.enforcement(),
&materialized_file_system_sandbox_policy,
network_sandbox_policy,
);
let sandbox_policy = compatibility_sandbox_policy_for_permission_profile(
&materialized_permission_profile,
&materialized_file_system_sandbox_policy,
network_sandbox_policy,
resolved_cwd.as_path(),
);
if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) {
file_system_sandbox_policy = file_system_sandbox_policy
.with_additional_legacy_workspace_writable_roots(&internal_writable_roots);
permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement(
permission_profile.enforcement(),
&file_system_sandbox_policy,
network_sandbox_policy,
);
}
let active_permission_profile = if using_implicit_builtin_profile
&& default_permissions == BUILT_IN_WORKSPACE_PROFILE
&& cfg.sandbox_workspace_write.is_some()
@@ -2885,29 +2836,8 @@ impl Config {
);
permission_profile = PermissionProfile::read_only();
}
let (mut file_system_sandbox_policy, network_sandbox_policy) =
let (file_system_sandbox_policy, _network_sandbox_policy) =
permission_profile.to_runtime_permissions();
let materialized_file_system_sandbox_policy = permission_profile
.clone()
.materialize_project_roots_with_workspace_roots(&workspace_roots)
.file_system_sandbox_policy();
if matches!(permission_profile.enforcement(), SandboxEnforcement::Managed)
&& materialized_file_system_sandbox_policy.can_write_path_with_cwd(
resolved_cwd.as_path(),
resolved_cwd.as_path(),
)
&& !materialized_file_system_sandbox_policy.has_full_disk_write_access()
{
// Keep Codex runtime write access while storing the runtime
// workspace roots separately on the thread.
file_system_sandbox_policy = file_system_sandbox_policy
.with_additional_legacy_workspace_writable_roots(&internal_writable_roots);
permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement(
permission_profile.enforcement(),
&file_system_sandbox_policy,
network_sandbox_policy,
);
}
(
configured_network_proxy_config,
permission_profile,
@@ -3324,11 +3254,14 @@ impl Config {
network_requirements,
&network_permission_profile,
)?;
let helper_readable_roots = get_readable_roots_required_for_codex_runtime(
let mut helper_readable_roots = get_readable_roots_required_for_codex_runtime(
&codex_home,
zsh_path.as_ref(),
main_execve_wrapper_exe.as_ref(),
);
if features.enabled(Feature::MemoryTool) && memories_config.use_memories {
helper_readable_roots.push(memories_root);
}
let effective_permission_profile = constrained_permission_profile.value.get().clone();
let (mut effective_file_system_sandbox_policy, effective_network_sandbox_policy) =
effective_permission_profile.to_runtime_permissions();
@@ -3438,7 +3371,7 @@ impl Config {
agent_max_threads,
agent_max_depth,
agent_roles,
memories: cfg.memories.unwrap_or_default().into(),
memories: memories_config,
agent_job_max_runtime_seconds,
agent_interrupt_message_enabled,
codex_home,
+11 -27
View File
@@ -31,13 +31,13 @@ use std::sync::Arc;
use tempfile::tempdir;
fn annotations(destructive_hint: Option<bool>, open_world_hint: Option<bool>) -> ToolAnnotations {
ToolAnnotations {
ToolAnnotations::from_raw(
/*title*/ None,
/*read_only_hint*/ None,
destructive_hint,
idempotent_hint: None,
/*idempotent_hint*/ None,
open_world_hint,
read_only_hint: None,
title: None,
}
)
}
fn app(id: &str) -> AppInfo {
@@ -63,17 +63,7 @@ fn plugin_names(names: &[&str]) -> Vec<String> {
}
fn test_tool_definition(tool_name: &str) -> Tool {
Tool {
name: tool_name.to_string().into(),
title: None,
description: None,
input_schema: Arc::new(JsonObject::default()),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
}
Tool::new_with_raw(tool_name.to_string(), None, Arc::new(JsonObject::default()))
}
fn codex_app_tool(
@@ -243,17 +233,11 @@ fn accessible_connectors_from_mcp_tools_preserves_description() {
callable_name: "calendar_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
namespace_description: Some("Plan events".to_string()),
tool: Tool {
name: "calendar_create_event".to_string().into(),
title: None,
description: Some("Create a calendar event".into()),
input_schema: Arc::new(JsonObject::default()),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
tool: Tool::new(
"calendar_create_event",
"Create a calendar event",
Arc::new(JsonObject::default()),
),
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
@@ -1,9 +1,17 @@
use crate::session::turn_context::TurnContext;
use crate::session::turn_context::TurnEnvironment;
use crate::shell::Shell;
use codex_protocol::models::ManagedFileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::protocol::TurnContextItem;
use codex_protocol::protocol::TurnContextNetworkItem;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::HashSet;
use std::path::PathBuf;
use super::ContextualUserFragment;
@@ -13,6 +21,7 @@ pub(crate) struct EnvironmentContext {
pub(crate) current_date: Option<String>,
pub(crate) timezone: Option<String>,
pub(crate) network: Option<NetworkContext>,
pub(crate) filesystem: Option<FileSystemContext>,
pub(crate) subagents: Option<String>,
}
@@ -83,6 +92,208 @@ impl EnvironmentContextEnvironments {
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct FileSystemContext {
workspace_roots: Vec<String>,
permission_profile: FileSystemPermissionProfileContext,
}
#[derive(Debug, Clone, PartialEq, Eq)]
enum FileSystemPermissionProfileContext {
Managed(ManagedFileSystemContext),
Disabled,
External,
}
#[derive(Debug, Clone, PartialEq, Eq)]
enum ManagedFileSystemContext {
Restricted {
entries: Vec<FileSystemSandboxEntry>,
glob_scan_max_depth: Option<usize>,
},
Unrestricted,
}
impl FileSystemContext {
fn from_permission_profile(
permission_profile: &PermissionProfile,
workspace_roots: &[AbsolutePathBuf],
) -> Self {
let permission_profile = permission_profile
.clone()
.materialize_project_roots_with_workspace_roots(workspace_roots);
let workspace_roots = workspace_roots
.iter()
.map(|root| root.to_string_lossy().into_owned())
.collect();
let permission_profile = match permission_profile {
PermissionProfile::Managed { file_system, .. } => {
FileSystemPermissionProfileContext::Managed(ManagedFileSystemContext::from(
file_system,
))
}
PermissionProfile::Disabled => FileSystemPermissionProfileContext::Disabled,
PermissionProfile::External { .. } => FileSystemPermissionProfileContext::External,
};
Self {
workspace_roots,
permission_profile,
}
}
fn render(&self) -> String {
let mut rendered = "<filesystem>".to_string();
if !self.workspace_roots.is_empty() {
rendered.push_str("<workspace_roots>");
for root in &self.workspace_roots {
push_text_element(&mut rendered, "root", root);
}
rendered.push_str("</workspace_roots>");
}
self.permission_profile.render(&mut rendered);
rendered.push_str("</filesystem>");
rendered
}
}
impl From<ManagedFileSystemPermissions> for ManagedFileSystemContext {
fn from(file_system: ManagedFileSystemPermissions) -> Self {
match file_system {
ManagedFileSystemPermissions::Restricted {
mut entries,
glob_scan_max_depth,
} => {
dedupe_file_system_entries(&mut entries);
Self::Restricted {
entries,
glob_scan_max_depth: glob_scan_max_depth.map(usize::from),
}
}
ManagedFileSystemPermissions::Unrestricted => Self::Unrestricted,
}
}
}
impl FileSystemPermissionProfileContext {
fn render(&self, rendered: &mut String) {
match self {
Self::Managed(file_system) => {
rendered.push_str("<permission_profile type=\"managed\">");
file_system.render(rendered);
rendered.push_str("</permission_profile>");
}
Self::Disabled => {
rendered.push_str(
"<permission_profile type=\"disabled\"><file_system type=\"unrestricted\" /></permission_profile>",
);
}
Self::External => {
rendered.push_str(
"<permission_profile type=\"external\"><file_system type=\"external\" /></permission_profile>",
);
}
}
}
}
impl ManagedFileSystemContext {
fn render(&self, rendered: &mut String) {
match self {
Self::Restricted {
entries,
glob_scan_max_depth,
} => {
if entries.is_empty() && glob_scan_max_depth.is_none() {
rendered.push_str("<file_system type=\"restricted\" />");
return;
}
rendered.push_str("<file_system type=\"restricted\"");
if let Some(glob_scan_max_depth) = glob_scan_max_depth {
rendered.push_str(&format!(" glob_scan_max_depth=\"{glob_scan_max_depth}\""));
}
rendered.push('>');
for entry in entries {
render_file_system_entry(rendered, entry);
}
rendered.push_str("</file_system>");
}
Self::Unrestricted => {
rendered.push_str("<file_system type=\"unrestricted\" />");
}
}
}
}
fn render_file_system_entry(rendered: &mut String, entry: &FileSystemSandboxEntry) {
rendered.push_str("<entry access=\"");
let access = entry.access.to_string();
rendered.push_str(&access);
if entry.access == FileSystemAccessMode::Deny {
rendered.push_str("\" escalatable=\"false");
}
rendered.push_str("\">");
match &entry.path {
FileSystemPath::Path { path } => {
push_text_element(rendered, "path", path.to_string_lossy().as_ref());
}
FileSystemPath::GlobPattern { pattern } => {
push_text_element(rendered, "glob", pattern);
}
FileSystemPath::Special { value } => {
let value = render_special_path(value);
push_text_element(rendered, "special", &value);
}
}
rendered.push_str("</entry>");
}
fn render_special_path(value: &FileSystemSpecialPath) -> String {
match value {
FileSystemSpecialPath::Root => ":root".to_string(),
FileSystemSpecialPath::Minimal => ":minimal".to_string(),
FileSystemSpecialPath::ProjectRoots { subpath } => {
render_special_path_with_subpath(":workspace_roots", subpath)
}
FileSystemSpecialPath::Tmpdir => ":tmpdir".to_string(),
FileSystemSpecialPath::SlashTmp => ":slash_tmp".to_string(),
FileSystemSpecialPath::Unknown { path, subpath } => {
render_special_path_with_subpath(path, subpath)
}
}
}
fn render_special_path_with_subpath(base: &str, subpath: &Option<PathBuf>) -> String {
match subpath {
Some(subpath) => format!("{base}/{}", subpath.display()),
None => base.to_string(),
}
}
fn dedupe_file_system_entries(entries: &mut Vec<FileSystemSandboxEntry>) {
let mut seen = HashSet::new();
entries.retain(|entry| seen.insert(entry.clone()));
}
fn push_text_element(rendered: &mut String, name: &str, value: &str) {
rendered.push_str(&format!("<{name}>"));
push_xml_escaped_text(rendered, value);
rendered.push_str(&format!("</{name}>"));
}
fn push_xml_escaped_text(rendered: &mut String, value: &str) {
for ch in value.chars() {
match ch {
'&' => rendered.push_str("&amp;"),
'<' => rendered.push_str("&lt;"),
'>' => rendered.push_str("&gt;"),
'"' => rendered.push_str("&quot;"),
'\'' => rendered.push_str("&apos;"),
_ => rendered.push(ch),
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub(crate) struct NetworkContext {
allowed_domains: Vec<String>,
@@ -129,6 +340,7 @@ impl EnvironmentContext {
current_date,
timezone,
network,
filesystem: None,
subagents,
}
}
@@ -138,6 +350,7 @@ impl EnvironmentContext {
current_date: Option<String>,
timezone: Option<String>,
network: Option<NetworkContext>,
filesystem: Option<FileSystemContext>,
subagents: Option<String>,
) -> Self {
Self {
@@ -145,6 +358,7 @@ impl EnvironmentContext {
current_date,
timezone,
network,
filesystem,
subagents,
}
}
@@ -157,6 +371,7 @@ impl EnvironmentContext {
&& self.current_date == other.current_date
&& self.timezone == other.timezone
&& self.network == other.network
&& self.filesystem == other.filesystem
&& self.subagents == other.subagents
}
@@ -165,6 +380,7 @@ impl EnvironmentContext {
after: &EnvironmentContext,
) -> Self {
let before_network = Self::network_from_turn_context_item(before);
let before_filesystem = Self::filesystem_from_turn_context_item(before);
let environments = match &after.environments {
EnvironmentContextEnvironments::Single(environment) => {
if before.cwd.as_path() != environment.cwd.as_path() {
@@ -186,17 +402,23 @@ impl EnvironmentContext {
} else {
before_network
};
let filesystem = if before_filesystem != after.filesystem {
after.filesystem.clone()
} else {
before_filesystem
};
EnvironmentContext::new_with_environments(
environments,
after.current_date.clone(),
after.timezone.clone(),
network,
filesystem,
/*subagents*/ None,
)
}
pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self {
Self::new(
let mut context = Self::new(
EnvironmentContextEnvironment::from_turn_environments(
&turn_context.environments.turn_environments,
shell,
@@ -205,7 +427,12 @@ impl EnvironmentContext {
turn_context.timezone.clone(),
Self::network_from_turn_context(turn_context),
/*subagents*/ None,
)
);
context.filesystem = Some(FileSystemContext::from_permission_profile(
&turn_context.permission_profile,
&turn_context.config.effective_workspace_roots(),
));
context
}
pub(crate) fn from_turn_context_item(
@@ -216,11 +443,14 @@ impl EnvironmentContext {
Ok(cwd) => cwd,
Err(_) => AbsolutePathBuf::resolve_path_against_base(&turn_context_item.cwd, "/"),
};
Self::new(
vec![EnvironmentContextEnvironment::legacy(cwd, shell)],
Self::new_with_environments(
EnvironmentContextEnvironments::from_vec(vec![EnvironmentContextEnvironment::legacy(
cwd, shell,
)]),
turn_context_item.current_date.clone(),
turn_context_item.timezone.clone(),
Self::network_from_turn_context_item(turn_context_item),
Self::filesystem_from_turn_context_item(turn_context_item),
/*subagents*/ None,
)
}
@@ -266,6 +496,30 @@ impl EnvironmentContext {
denied_domains.clone(),
))
}
fn filesystem_from_turn_context_item(
turn_context_item: &TurnContextItem,
) -> Option<FileSystemContext> {
Some(FileSystemContext::from_permission_profile(
&turn_context_item.permission_profile(),
&workspace_roots_from_turn_context_item(turn_context_item),
))
}
}
fn workspace_roots_from_turn_context_item(
turn_context_item: &TurnContextItem,
) -> Vec<AbsolutePathBuf> {
if let Some(workspace_roots) = turn_context_item.workspace_roots.as_ref() {
return workspace_roots.clone();
}
// Older rollout items did not persist workspace roots. Fall back to the
// legacy cwd binding only when reconstructing that historical context.
match AbsolutePathBuf::try_from(turn_context_item.cwd.clone()) {
Ok(cwd) => vec![cwd],
Err(_) => Vec::new(),
}
}
impl ContextualUserFragment for EnvironmentContext {
@@ -324,6 +578,9 @@ impl ContextualUserFragment for EnvironmentContext {
// lines.push(" <network enabled=\"false\" />".to_string());
}
}
if let Some(filesystem) = &self.filesystem {
lines.push(format!(" {}", filesystem.render()));
}
if let Some(subagents) = &self.subagents {
lines.push(" <subagents>".to_string());
lines.extend(subagents.lines().map(|line| format!(" {line}")));
@@ -1,9 +1,21 @@
use crate::shell::ShellType;
use super::*;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::permissions::project_roots_glob_pattern;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::TurnContextItem;
use codex_utils_absolute_path::test_support::PathBufExt;
use core_test_support::test_path_buf;
use pretty_assertions::assert_eq;
use std::path::Path;
use std::path::PathBuf;
fn fake_shell_name() -> String {
@@ -79,6 +91,125 @@ fn serialize_environment_context_with_network() {
assert_eq!(context.render(), expected);
}
fn workspace_write_permission_profile_with_private_denials() -> PermissionProfile {
PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(PathBuf::from("private"))),
},
access: FileSystemAccessMode::Deny,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: project_roots_glob_pattern(Path::new("private/**")),
},
access: FileSystemAccessMode::Deny,
},
]),
NetworkSandboxPolicy::Restricted,
)
}
#[test]
fn serialize_environment_context_with_full_filesystem_profile() {
let repo = test_abs_path("/repo");
let other_repo = test_abs_path("/other-repo");
let repo_private = repo.join("private");
let other_repo_private = other_repo.join("private");
let repo_private_glob =
AbsolutePathBuf::resolve_path_against_base(Path::new("private/**"), repo.as_path());
let other_repo_private_glob =
AbsolutePathBuf::resolve_path_against_base(Path::new("private/**"), other_repo.as_path());
let mut context = EnvironmentContext::new(
vec![EnvironmentContextEnvironment {
id: "local".to_string(),
cwd: test_path_buf("/repo").abs(),
shell: fake_shell_name(),
}],
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
/*subagents*/ None,
);
context.filesystem = Some(FileSystemContext::from_permission_profile(
&workspace_write_permission_profile_with_private_denials(),
&[repo.clone(), other_repo.clone()],
));
let expected = format!(
r#"<environment_context>
<cwd>{}</cwd>
<shell>bash</shell>
<filesystem><workspace_roots><root>{repo}</root><root>{other_repo}</root></workspace_roots><permission_profile type="managed"><file_system type="restricted"><entry access="write"><path>{repo}</path></entry><entry access="write"><path>{other_repo}</path></entry><entry access="deny" escalatable="false"><path>{repo_private}</path></entry><entry access="deny" escalatable="false"><path>{other_repo_private}</path></entry><entry access="deny" escalatable="false"><glob>{repo_private_glob}</glob></entry><entry access="deny" escalatable="false"><glob>{other_repo_private_glob}</glob></entry></file_system></permission_profile></filesystem>
</environment_context>"#,
test_path_buf("/repo").display(),
repo = repo.to_string_lossy(),
other_repo = other_repo.to_string_lossy(),
repo_private = repo_private.to_string_lossy(),
other_repo_private = other_repo_private.to_string_lossy(),
repo_private_glob = repo_private_glob.to_string_lossy(),
other_repo_private_glob = other_repo_private_glob.to_string_lossy(),
);
assert_eq!(context.render(), expected);
}
#[test]
fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() {
let repo = test_abs_path("/repo");
let other_repo = test_abs_path("/other-repo");
let repo_private = repo.join("private");
let item = TurnContextItem {
turn_id: None,
cwd: test_path_buf("/not-the-workspace"),
workspace_roots: Some(vec![repo.clone(), other_repo.clone()]),
current_date: None,
timezone: None,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
permission_profile: Some(workspace_write_permission_profile_with_private_denials()),
network: None,
file_system_sandbox_policy: None,
model: "gpt-5".to_string(),
personality: None,
collaboration_mode: None,
realtime_active: None,
effort: None,
summary: codex_protocol::config_types::ReasoningSummary::Auto,
};
let context = EnvironmentContext::from_turn_context_item(&item, fake_shell_name()).render();
assert!(
context.contains(&format!(
"<root>{}</root><root>{}</root>",
repo.to_string_lossy(),
other_repo.to_string_lossy()
)),
"{context}"
);
assert!(
context.contains(&format!("<path>{}</path>", repo_private.to_string_lossy())),
"{context}"
);
assert!(
!context.contains(
test_abs_path("/not-the-workspace")
.join("private")
.to_string_lossy()
.as_ref()
),
"{context}"
);
}
#[test]
fn serialize_read_only_environment_context() {
let context = EnvironmentContext::new(
@@ -4,6 +4,7 @@ use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::format_allow_prefixes;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::GranularApprovalConfig;
@@ -68,9 +69,11 @@ impl PermissionsInstructions {
exec_permission_approvals_enabled: bool,
request_permissions_tool_enabled: bool,
) -> Self {
let (sandbox_mode, writable_roots) = sandbox_prompt_from_profile(permission_profile, cwd);
let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy();
let (sandbox_mode, writable_roots) =
sandbox_prompt_from_policy(&file_system_sandbox_policy, cwd);
Self::from_permissions_with_network(
Self::from_permissions_with_network_and_denied_reads(
sandbox_mode,
network_access_from_policy(permission_profile.network_sandbox_policy()),
PermissionsPromptConfig {
@@ -81,14 +84,32 @@ impl PermissionsInstructions {
request_permissions_tool_enabled,
},
writable_roots,
denied_reads_text(&file_system_sandbox_policy, cwd),
)
}
#[cfg(test)]
fn from_permissions_with_network(
sandbox_mode: SandboxMode,
network_access: NetworkAccess,
config: PermissionsPromptConfig<'_>,
writable_roots: Option<Vec<WritableRoot>>,
) -> Self {
Self::from_permissions_with_network_and_denied_reads(
sandbox_mode,
network_access,
config,
writable_roots,
/*denied_reads*/ None,
)
}
fn from_permissions_with_network_and_denied_reads(
sandbox_mode: SandboxMode,
network_access: NetworkAccess,
config: PermissionsPromptConfig<'_>,
writable_roots: Option<Vec<WritableRoot>>,
denied_reads: Option<String>,
) -> Self {
let mut text = String::new();
append_section(&mut text, &sandbox_text(sandbox_mode, network_access));
@@ -105,6 +126,9 @@ impl PermissionsInstructions {
if let Some(writable_roots) = writable_roots_text(writable_roots) {
append_section(&mut text, &writable_roots);
}
if let Some(denied_reads) = denied_reads {
append_section(&mut text, &denied_reads);
}
if !text.ends_with('\n') {
text.push('\n');
}
@@ -112,27 +136,19 @@ impl PermissionsInstructions {
}
}
fn sandbox_prompt_from_profile(
permission_profile: &PermissionProfile,
fn sandbox_prompt_from_policy(
file_system_policy: &FileSystemSandboxPolicy,
cwd: &Path,
) -> (SandboxMode, Option<Vec<WritableRoot>>) {
match permission_profile {
PermissionProfile::Disabled | PermissionProfile::External { .. } => {
(SandboxMode::DangerFullAccess, None)
}
PermissionProfile::Managed { .. } => {
let file_system_policy = permission_profile.file_system_sandbox_policy();
if file_system_policy.has_full_disk_write_access() {
return (SandboxMode::DangerFullAccess, None);
}
if file_system_policy.has_full_disk_write_access() {
return (SandboxMode::DangerFullAccess, None);
}
let writable_roots = file_system_policy.get_writable_roots_with_cwd(cwd);
if writable_roots.is_empty() {
(SandboxMode::ReadOnly, None)
} else {
(SandboxMode::WorkspaceWrite, Some(writable_roots))
}
}
let writable_roots = file_system_policy.get_writable_roots_with_cwd(cwd);
if writable_roots.is_empty() {
(SandboxMode::ReadOnly, None)
} else {
(SandboxMode::WorkspaceWrite, Some(writable_roots))
}
}
@@ -254,6 +270,28 @@ fn writable_roots_text(writable_roots: Option<Vec<WritableRoot>>) -> Option<Stri
})
}
fn denied_reads_text(file_system_policy: &FileSystemSandboxPolicy, cwd: &Path) -> Option<String> {
let mut entries = file_system_policy
.get_unreadable_roots_with_cwd(cwd)
.into_iter()
.map(|root| format!("- path `{}`", root.to_string_lossy()))
.collect::<Vec<_>>();
entries.extend(
file_system_policy
.get_unreadable_globs_with_cwd(cwd)
.into_iter()
.map(|glob| format!("- glob `{glob}`")),
);
if entries.is_empty() {
return None;
}
Some(format!(
"## Denied filesystem reads\nThe active permission profile denies reading these paths/globs. Do not request escalation or additional permissions to read them; these denials are policy restrictions.\n{}",
entries.join("\n")
))
}
fn approved_command_prefixes_text(exec_policy: &Policy) -> Option<String> {
format_allow_prefixes(exec_policy.get_allowed_prefixes())
.filter(|prefixes| !prefixes.is_empty())
@@ -6,6 +6,7 @@ use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
@@ -83,6 +84,52 @@ fn builds_permissions_from_profile() {
assert!(text.contains(writable_root.to_string_lossy().as_ref()));
}
#[test]
fn builds_permissions_from_profile_with_denied_reads() {
let cwd = test_path_buf("/tmp");
let denied_root =
AbsolutePathBuf::from_absolute_path(cwd.join("blocked")).expect("absolute path");
let denied_glob = cwd.join("blocked").join("**");
let permission_profile = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied_root.clone(),
},
access: FileSystemAccessMode::Deny,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: denied_glob.to_string_lossy().into_owned(),
},
access: FileSystemAccessMode::Deny,
},
]),
NetworkSandboxPolicy::Restricted,
);
let instructions = PermissionsInstructions::from_permission_profile(
&permission_profile,
AskForApproval::OnRequest,
ApprovalsReviewer::AutoReview,
&Policy::empty(),
&cwd,
/*exec_permission_approvals_enabled*/ false,
/*request_permissions_tool_enabled*/ false,
);
let text = instructions.body();
assert!(text.contains("## Denied filesystem reads"));
assert!(text.contains("Do not request escalation or additional permissions"));
assert!(text.contains(denied_root.to_string_lossy().as_ref()));
assert!(text.contains(&format!("glob `{}`", denied_glob.to_string_lossy())));
}
#[test]
fn includes_request_rule_instructions_for_on_request() {
let mut exec_policy = Policy::empty();
@@ -121,6 +121,7 @@ fn reference_context_item() -> TurnContextItem {
TurnContextItem {
turn_id: Some("reference-turn".to_string()),
cwd: PathBuf::from("/tmp/reference-cwd"),
workspace_roots: None,
current_date: Some("2026-03-23".to_string()),
timezone: Some("America/Los_Angeles".to_string()),
approval_policy: AskForApproval::OnRequest,
+67 -25
View File
@@ -56,6 +56,7 @@ const SIGKILL_CODE: i32 = 9;
const TIMEOUT_CODE: i32 = 64;
const EXIT_CODE_SIGNAL_BASE: i32 = 128; // conventional shell: 128 + signal
const EXEC_TIMEOUT_EXIT_CODE: i32 = 124; // conventional timeout exit code
const CANCELLATION_TERMINATION_GRACE_PERIOD: Duration = Duration::from_millis(50);
// I/O buffer sizing
const READ_CHUNK_SIZE: usize = 8192; // bytes per read
@@ -224,6 +225,17 @@ impl ExecExpiration {
}
}
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
pub(crate) fn cancellation_token(&self) -> Option<CancellationToken> {
match self {
ExecExpiration::Timeout(_) | ExecExpiration::DefaultTimeout => None,
ExecExpiration::Cancellation(cancellation)
| ExecExpiration::TimeoutOrCancellation { cancellation, .. } => {
Some(cancellation.clone())
}
}
}
pub(crate) fn with_cancellation(self, cancellation: CancellationToken) -> Self {
match self {
ExecExpiration::Timeout(timeout) => ExecExpiration::TimeoutOrCancellation {
@@ -423,7 +435,6 @@ pub(crate) async fn execute_exec_request(
stdout_stream: Option<StdoutStream>,
after_spawn: Option<Box<dyn FnOnce() + Send>>,
) -> Result<ExecToolCallOutput> {
let sandbox_policy = exec_request.compatibility_sandbox_policy();
let ExecRequest {
command,
cwd,
@@ -464,7 +475,6 @@ pub(crate) async fn execute_exec_request(
stdout_stream,
after_spawn,
sandbox,
&sandbox_policy,
&permission_profile,
&windows_sandbox_policy_cwd,
windows_sandbox_filesystem_overrides.as_ref(),
@@ -481,7 +491,6 @@ async fn get_raw_output_result(
stdout_stream: Option<StdoutStream>,
after_spawn: Option<Box<dyn FnOnce() + Send>>,
#[cfg_attr(not(windows), allow(unused_variables))] sandbox: SandboxType,
#[cfg_attr(not(windows), allow(unused_variables))] sandbox_policy: &SandboxPolicy,
#[cfg_attr(not(windows), allow(unused_variables))] permission_profile: &PermissionProfile,
#[cfg_attr(not(windows), allow(unused_variables))] windows_sandbox_policy_cwd: &AbsolutePathBuf,
#[cfg_attr(not(windows), allow(unused_variables))] windows_sandbox_filesystem_overrides: Option<
@@ -492,7 +501,6 @@ async fn get_raw_output_result(
if sandbox == SandboxType::WindowsRestrictedToken {
return exec_windows_sandbox(
params,
sandbox_policy,
permission_profile,
windows_sandbox_policy_cwd,
windows_sandbox_filesystem_overrides,
@@ -572,7 +580,6 @@ fn record_windows_sandbox_spawn_failure(
#[cfg(target_os = "windows")]
async fn exec_windows_sandbox(
params: ExecParams,
sandbox_policy: &SandboxPolicy,
permission_profile: &PermissionProfile,
windows_sandbox_policy_cwd: &AbsolutePathBuf,
windows_sandbox_filesystem_overrides: Option<&WindowsSandboxFilesystemOverrides>,
@@ -596,19 +603,18 @@ async fn exec_windows_sandbox(
network.apply_to_env(&mut env);
}
// TODO(iceweasel-oai): run_windows_sandbox_capture should support all
// variants of ExecExpiration, not just timeout.
let timeout_ms = if capture_policy.uses_expiration() {
expiration.timeout_ms()
// Windows sandbox capture still receives timeout and cancellation separately.
let (cancellation, timeout_ms) = if capture_policy.uses_expiration() {
let cancellation = expiration.cancellation_token().map(|token| {
codex_windows_sandbox::WindowsSandboxCancellationToken::new(move || {
token.is_cancelled()
})
});
(cancellation, expiration.timeout_ms())
} else {
None
(None, None)
};
let policy_str = serde_json::to_string(sandbox_policy).map_err(|err| {
CodexErr::Io(io::Error::other(format!(
"failed to serialize Windows sandbox policy: {err}"
)))
})?;
let sandbox_cwd = windows_sandbox_policy_cwd.clone();
let permission_profile = permission_profile.clone();
let codex_home = find_codex_home().map_err(|err| {
@@ -643,6 +649,7 @@ async fn exec_windows_sandbox(
cwd: &cwd,
env_map: env,
timeout_ms,
cancellation,
use_private_desktop: windows_sandbox_private_desktop,
proxy_enforced,
read_roots_override: elevated_read_roots_override.as_deref(),
@@ -655,13 +662,14 @@ async fn exec_windows_sandbox(
)
} else {
run_windows_sandbox_capture_with_filesystem_overrides(
policy_str.as_str(),
&permission_profile,
&sandbox_cwd,
codex_home.as_ref(),
command,
&cwd,
env,
timeout_ms,
cancellation,
&additional_deny_read_paths,
&additional_deny_write_paths,
windows_sandbox_private_desktop,
@@ -1368,15 +1376,49 @@ async fn consume_output(
(exit_status, false)
}
outcome = &mut expiration_wait => {
kill_child_process_group(&mut child)?;
child.start_kill()?;
let timed_out = matches!(outcome, Some(ExecExpirationOutcome::TimedOut));
let exit_status = if timed_out {
synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + TIMEOUT_CODE)
} else {
synthetic_exit_status_for_code(/*code*/ 1)
};
(exit_status, timed_out)
match outcome {
Some(ExecExpirationOutcome::TimedOut) => {
kill_child_process_group(&mut child)?;
child.start_kill()?;
(
synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + TIMEOUT_CODE),
true,
)
}
Some(ExecExpirationOutcome::Cancelled) => {
// Let TERM-aware processes run cleanup briefly, then kill any
// remaining members of the original process group.
let process_group_id = child.id();
let should_escalate = if let Some(process_group_id) = process_group_id {
codex_utils_pty::process_group::terminate_process_group(process_group_id)?
} else {
false
};
match tokio::time::timeout(
CANCELLATION_TERMINATION_GRACE_PERIOD,
child.wait(),
)
.await
{
Ok(status) => {
status?;
if should_escalate
&& let Some(process_group_id) = process_group_id
{
codex_utils_pty::process_group::kill_process_group(
process_group_id,
)?;
}
}
Err(_) => {
kill_child_process_group(&mut child)?;
child.start_kill()?;
}
}
(synthetic_exit_status_for_code(/*code*/ 1), false)
}
None => unreachable!("expiration wait only resolves while expiration is active"),
}
}
_ = tokio::signal::ctrl_c() => {
kill_child_process_group(&mut child)?;
+109
View File
@@ -1128,6 +1128,115 @@ async fn process_exec_tool_call_respects_cancellation_token() -> Result<()> {
Ok(())
}
#[cfg(unix)]
#[tokio::test]
async fn process_exec_tool_call_cancellation_allows_sigterm_cleanup() -> Result<()> {
let temp_dir = tempfile::TempDir::new()?;
let ready_marker = temp_dir.path().join("ready");
let cleanup_marker = temp_dir.path().join("cleanup");
let descendant_pid_marker = temp_dir.path().join("descendant-pid");
// The parent handles TERM and records cleanup, while a TERM-ignoring child
// proves cancellation still escalates any survivors in the process group.
let command = vec![
"/bin/sh".to_string(),
"-c".to_string(),
r#"(trap '' TERM; sleep 60) &
printf '%s' "$!" > "$DESCENDANT_PID_MARKER"
trap 'printf cleaned > "$CLEANUP_MARKER"; exit 0' TERM
printf ready > "$READY_MARKER"
while :; do sleep 1; done"#
.to_string(),
];
let cwd = codex_utils_absolute_path::AbsolutePathBuf::current_dir()?;
let mut env: HashMap<String, String> = std::env::vars().collect();
env.insert(
"READY_MARKER".to_string(),
ready_marker.to_string_lossy().into_owned(),
);
env.insert(
"CLEANUP_MARKER".to_string(),
cleanup_marker.to_string_lossy().into_owned(),
);
env.insert(
"DESCENDANT_PID_MARKER".to_string(),
descendant_pid_marker.to_string_lossy().into_owned(),
);
let cancel_token = CancellationToken::new();
let cancel_tx = cancel_token.clone();
tokio::spawn(async move {
for _ in 0..50 {
if ready_marker.exists() {
cancel_tx.cancel();
return;
}
tokio::time::sleep(Duration::from_millis(20)).await;
}
cancel_tx.cancel();
});
let params = ExecParams {
command,
cwd: cwd.clone(),
expiration: ExecExpiration::DefaultTimeout.with_cancellation(cancel_token),
capture_policy: ExecCapturePolicy::ShellTool,
env,
network: None,
sandbox_permissions: SandboxPermissions::UseDefault,
windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled,
windows_sandbox_private_desktop: false,
justification: None,
arg0: None,
};
let result = timeout(
Duration::from_secs(5),
process_exec_tool_call(
params,
&PermissionProfile::Disabled,
&cwd,
&None,
/*use_legacy_landlock*/ false,
/*stdout_stream*/ None,
),
)
.await
.expect("cancellation should stop the process promptly");
let output = result.expect("cancellation should return a non-timeout exec result");
assert!(!output.timed_out);
assert_eq!(
std::fs::read_to_string(cleanup_marker)?,
"cleaned",
"SIGTERM cleanup trap should run before cancellation falls back to a hard kill"
);
let descendant_pid = std::fs::read_to_string(descendant_pid_marker)?
.parse::<i32>()
.map_err(|error| {
io::Error::new(
io::ErrorKind::InvalidData,
format!("failed to parse descendant pid: {error}"),
)
})?;
let mut killed = false;
for _ in 0..20 {
if unsafe { libc::kill(descendant_pid, 0) } == -1
&& let Some(libc::ESRCH) = std::io::Error::last_os_error().raw_os_error()
{
killed = true;
break;
}
tokio::time::sleep(Duration::from_millis(100)).await;
}
if !killed {
unsafe {
libc::kill(descendant_pid, libc::SIGKILL);
}
}
assert!(
killed,
"TERM-ignoring descendant process with pid {descendant_pid} is still alive"
);
Ok(())
}
#[cfg(unix)]
fn long_running_command() -> Vec<String> {
vec![
+11 -44
View File
@@ -26,7 +26,7 @@ use codex_otel::GOAL_TOKEN_COUNT_METRIC;
use codex_otel::GOAL_USAGE_LIMITED_METRIC;
use codex_protocol::ThreadId;
use codex_protocol::config_types::ModeKind;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ThreadGoal;
use codex_protocol::protocol::ThreadGoalStatus;
@@ -172,13 +172,12 @@ pub(crate) struct GoalRuntimeState {
pub(crate) budget_limit_reported_goal_id: Mutex<Option<String>>,
accounting_lock: Semaphore,
accounting: Mutex<GoalAccountingSnapshot>,
continuation_turn_id: Mutex<Option<String>>,
pub(crate) continuation_lock: Semaphore,
}
struct GoalContinuationCandidate {
goal_id: String,
items: Vec<ResponseInputItem>,
items: Vec<ResponseItem>,
}
impl GoalRuntimeState {
@@ -188,7 +187,6 @@ impl GoalRuntimeState {
budget_limit_reported_goal_id: Mutex::new(None),
accounting_lock: Semaphore::new(/*permits*/ 1),
accounting: Mutex::new(GoalAccountingSnapshot::new()),
continuation_turn_id: Mutex::new(None),
continuation_lock: Semaphore::new(/*permits*/ 1),
}
}
@@ -688,7 +686,7 @@ impl Session {
.await;
if let Some(goal) = goal_for_steering {
let item = goal_context_input_item(objective_updated_prompt(&goal));
if self.inject_response_items(vec![item]).await.is_err() {
if self.inject_if_running(vec![item]).await.is_err() {
tracing::debug!(
"skipping objective-updated goal steering because no turn is active"
);
@@ -900,20 +898,6 @@ impl Session {
}
}
async fn mark_thread_goal_continuation_turn_started(&self, turn_id: String) {
*self.goal_runtime.continuation_turn_id.lock().await = Some(turn_id);
}
async fn take_thread_goal_continuation_turn(&self, turn_id: &str) -> bool {
let mut continuation_turn_id = self.goal_runtime.continuation_turn_id.lock().await;
if continuation_turn_id.as_deref() == Some(turn_id) {
*continuation_turn_id = None;
true
} else {
false
}
}
async fn clear_reserved_goal_continuation_turn(&self, turn_state: &Arc<Mutex<TurnState>>) {
let mut active_turn_guard = self.active_turn.lock().await;
if let Some(active_turn) = active_turn_guard.as_ref()
@@ -941,8 +925,6 @@ impl Session {
tracing::warn!("failed to account thread goal progress at turn end: {err}");
}
self.take_thread_goal_continuation_turn(&turn_context.sub_id)
.await;
if turn_completed {
let mut accounting = self.goal_runtime.accounting.lock().await;
if accounting
@@ -957,8 +939,6 @@ impl Session {
async fn handle_thread_goal_task_abort(&self, turn_context: Option<&TurnContext>) {
if let Some(turn_context) = turn_context {
self.take_thread_goal_continuation_turn(&turn_context.sub_id)
.await;
if let Err(err) = self
.account_thread_goal_progress(
turn_context,
@@ -1094,7 +1074,7 @@ impl Session {
.await;
if should_steer_budget_limit {
let item = budget_limit_steering_item(&goal);
if self.inject_response_items(vec![item]).await.is_err() {
if self.inject_if_running(vec![item]).await.is_err() {
tracing::debug!("skipping budget-limit goal steering because no turn is active");
}
*self.goal_runtime.budget_limit_reported_goal_id.lock().await = Some(goal_id);
@@ -1352,7 +1332,7 @@ impl Session {
candidate
.items
.into_iter()
.map(TurnInput::ResponseInputItem)
.map(TurnInput::ResponseItem)
.collect(),
)
.await;
@@ -1373,8 +1353,6 @@ impl Session {
.await;
return;
}
self.mark_thread_goal_continuation_turn_started(turn_context.sub_id.clone())
.await;
self.start_task(turn_context, Vec::new(), RegularTask::new())
.await;
}
@@ -1393,14 +1371,6 @@ impl Session {
tracing::debug!("skipping active goal continuation because a turn is already active");
return None;
}
if self
.input_queue
.has_queued_response_items_for_next_turn()
.await
{
tracing::debug!("skipping active goal continuation because queued input exists");
return None;
}
if self.input_queue.has_trigger_turn_mailbox_items().await {
tracing::debug!(
"skipping active goal continuation because trigger-turn mailbox input is pending"
@@ -1438,10 +1408,6 @@ impl Session {
return None;
}
if self.active_turn.lock().await.is_some()
|| self
.input_queue
.has_queued_response_items_for_next_turn()
.await
|| self.input_queue.has_trigger_turn_mailbox_items().await
{
tracing::debug!("skipping active goal continuation because pending work appeared");
@@ -1626,12 +1592,12 @@ fn escape_xml_text(input: &str) -> String {
.replace('>', "&gt;")
}
fn budget_limit_steering_item(goal: &ThreadGoal) -> ResponseInputItem {
fn budget_limit_steering_item(goal: &ThreadGoal) -> ResponseItem {
goal_context_input_item(budget_limit_prompt(goal))
}
fn goal_context_input_item(prompt: String) -> ResponseInputItem {
GoalContext::new(prompt).into_response_input_item()
fn goal_context_input_item(prompt: String) -> ResponseItem {
ResponseItem::from(GoalContext::new(prompt).into_response_input_item())
}
pub(crate) fn protocol_goal_from_state(goal: codex_state::ThreadGoal) -> ThreadGoal {
@@ -1700,7 +1666,7 @@ mod tests {
use codex_protocol::ThreadId;
use codex_protocol::config_types::ModeKind;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::ThreadGoal;
use codex_protocol::protocol::ThreadGoalStatus;
use codex_protocol::protocol::TokenUsage;
@@ -1829,7 +1795,8 @@ mod tests {
assert_eq!(
item,
ResponseInputItem::Message {
ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText {
text: "<goal_context>\nContinue working.\n</goal_context>".to_string(),
+418
View File
@@ -0,0 +1,418 @@
use std::time::Duration;
use codex_analytics::GuardianApprovalRequestSource;
use codex_analytics::GuardianReviewAnalyticsResult;
use codex_analytics::GuardianReviewDecision;
use codex_analytics::GuardianReviewFailureReason;
use codex_analytics::GuardianReviewSessionKind;
use codex_analytics::GuardianReviewTerminalStatus;
use codex_analytics::GuardianReviewedAction;
use codex_otel::GUARDIAN_REVIEW_COUNT_METRIC;
use codex_otel::GUARDIAN_REVIEW_DURATION_METRIC;
use codex_otel::GUARDIAN_REVIEW_TOKEN_USAGE_METRIC;
use codex_otel::GUARDIAN_REVIEW_TTFT_DURATION_METRIC;
use codex_otel::SessionTelemetry;
use codex_otel::sanitize_metric_tag_value;
use codex_protocol::protocol::GuardianAssessmentOutcome;
use codex_protocol::protocol::GuardianRiskLevel;
use codex_protocol::protocol::GuardianUserAuthorization;
use codex_protocol::protocol::TokenUsage;
pub(crate) fn emit_guardian_review_metrics(
session_telemetry: &SessionTelemetry,
result: &GuardianReviewAnalyticsResult,
approval_request_source: GuardianApprovalRequestSource,
reviewed_action: &GuardianReviewedAction,
completion_latency_ms: u64,
) {
let tags = guardian_review_metric_tags(result, approval_request_source, reviewed_action);
let tag_refs: Vec<(&str, &str)> = tags
.iter()
.map(|(key, value)| (*key, value.as_str()))
.collect();
session_telemetry.counter(GUARDIAN_REVIEW_COUNT_METRIC, /*inc*/ 1, &tag_refs);
session_telemetry.record_duration(
GUARDIAN_REVIEW_DURATION_METRIC,
Duration::from_millis(completion_latency_ms),
&tag_refs,
);
if let Some(time_to_first_token_ms) = result.time_to_first_token_ms {
session_telemetry.record_duration(
GUARDIAN_REVIEW_TTFT_DURATION_METRIC,
Duration::from_millis(time_to_first_token_ms),
&tag_refs,
);
}
if let Some(token_usage) = result.token_usage.as_ref() {
emit_guardian_token_usage_histograms(session_telemetry, token_usage, tags);
}
}
fn emit_guardian_token_usage_histograms(
session_telemetry: &SessionTelemetry,
token_usage: &TokenUsage,
base_tags: Vec<(&'static str, String)>,
) {
for (token_type, value) in [
("total", token_usage.total_tokens.max(0)),
("input", token_usage.input_tokens.max(0)),
("cached_input", token_usage.cached_input()),
("non_cached_input", token_usage.non_cached_input()),
("output", token_usage.output_tokens.max(0)),
(
"reasoning_output",
token_usage.reasoning_output_tokens.max(0),
),
] {
let mut tags = base_tags.clone();
tags.push(("token_type", token_type.to_string()));
let tag_refs: Vec<(&str, &str)> = tags
.iter()
.map(|(key, value)| (*key, value.as_str()))
.collect();
session_telemetry.histogram(GUARDIAN_REVIEW_TOKEN_USAGE_METRIC, value, &tag_refs);
}
}
fn guardian_review_metric_tags(
result: &GuardianReviewAnalyticsResult,
approval_request_source: GuardianApprovalRequestSource,
reviewed_action: &GuardianReviewedAction,
) -> Vec<(&'static str, String)> {
vec![
("decision", decision_tag(result.decision).to_string()),
(
"terminal_status",
terminal_status_tag(result.terminal_status).to_string(),
),
(
"failure_reason",
failure_reason_tag(result.failure_reason).to_string(),
),
(
"approval_request_source",
approval_request_source_tag(approval_request_source).to_string(),
),
("action", reviewed_action_tag(reviewed_action).to_string()),
(
"session_kind",
session_kind_tag(result.guardian_session_kind).to_string(),
),
(
"had_prior_review_context",
optional_bool_tag(result.had_prior_review_context).to_string(),
),
(
"reviewed_action_truncated",
bool_tag(result.reviewed_action_truncated).to_string(),
),
("risk_level", risk_level_tag(result.risk_level).to_string()),
(
"user_authorization",
user_authorization_tag(result.user_authorization).to_string(),
),
("outcome", outcome_tag(result.outcome).to_string()),
(
"guardian_model",
result
.guardian_model
.as_deref()
.map(sanitize_metric_tag_value)
.unwrap_or_else(|| "none".to_string()),
),
(
"guardian_reasoning_effort",
result
.guardian_reasoning_effort
.as_deref()
.map(sanitize_metric_tag_value)
.unwrap_or_else(|| "none".to_string()),
),
]
}
fn decision_tag(decision: GuardianReviewDecision) -> &'static str {
match decision {
GuardianReviewDecision::Approved => "approved",
GuardianReviewDecision::Denied => "denied",
GuardianReviewDecision::Aborted => "aborted",
}
}
fn terminal_status_tag(status: GuardianReviewTerminalStatus) -> &'static str {
match status {
GuardianReviewTerminalStatus::Approved => "approved",
GuardianReviewTerminalStatus::Denied => "denied",
GuardianReviewTerminalStatus::Aborted => "aborted",
GuardianReviewTerminalStatus::TimedOut => "timed_out",
GuardianReviewTerminalStatus::FailedClosed => "failed_closed",
}
}
fn failure_reason_tag(reason: Option<GuardianReviewFailureReason>) -> &'static str {
match reason {
Some(GuardianReviewFailureReason::Timeout) => "timeout",
Some(GuardianReviewFailureReason::Cancelled) => "cancelled",
Some(GuardianReviewFailureReason::PromptBuildError) => "prompt_build_error",
Some(GuardianReviewFailureReason::SessionError) => "session_error",
Some(GuardianReviewFailureReason::ParseError) => "parse_error",
None => "none",
}
}
fn approval_request_source_tag(source: GuardianApprovalRequestSource) -> &'static str {
match source {
GuardianApprovalRequestSource::MainTurn => "main_turn",
GuardianApprovalRequestSource::DelegatedSubagent => "delegated_subagent",
}
}
fn reviewed_action_tag(action: &GuardianReviewedAction) -> &'static str {
match action {
GuardianReviewedAction::Shell { .. } => "shell",
GuardianReviewedAction::UnifiedExec { .. } => "unified_exec",
GuardianReviewedAction::Execve { .. } => "execve",
GuardianReviewedAction::ApplyPatch {} => "apply_patch",
GuardianReviewedAction::NetworkAccess { .. } => "network_access",
GuardianReviewedAction::McpToolCall { .. } => "mcp_tool_call",
GuardianReviewedAction::RequestPermissions {} => "request_permissions",
}
}
fn session_kind_tag(kind: Option<GuardianReviewSessionKind>) -> &'static str {
match kind {
Some(GuardianReviewSessionKind::TrunkNew) => "trunk_new",
Some(GuardianReviewSessionKind::TrunkReused) => "trunk_reused",
Some(GuardianReviewSessionKind::EphemeralForked) => "ephemeral_forked",
None => "none",
}
}
fn optional_bool_tag(value: Option<bool>) -> &'static str {
match value {
Some(true) => "true",
Some(false) => "false",
None => "unknown",
}
}
fn bool_tag(value: bool) -> &'static str {
if value { "true" } else { "false" }
}
fn risk_level_tag(risk_level: Option<GuardianRiskLevel>) -> &'static str {
match risk_level {
Some(GuardianRiskLevel::Low) => "low",
Some(GuardianRiskLevel::Medium) => "medium",
Some(GuardianRiskLevel::High) => "high",
Some(GuardianRiskLevel::Critical) => "critical",
None => "none",
}
}
fn user_authorization_tag(user_authorization: Option<GuardianUserAuthorization>) -> &'static str {
match user_authorization {
Some(GuardianUserAuthorization::Unknown) => "unknown",
Some(GuardianUserAuthorization::Low) => "low",
Some(GuardianUserAuthorization::Medium) => "medium",
Some(GuardianUserAuthorization::High) => "high",
None => "none",
}
}
fn outcome_tag(outcome: Option<GuardianAssessmentOutcome>) -> &'static str {
match outcome {
Some(GuardianAssessmentOutcome::Allow) => "allow",
Some(GuardianAssessmentOutcome::Deny) => "deny",
None => "none",
}
}
#[cfg(test)]
mod tests {
use super::*;
use codex_otel::MetricsClient;
use codex_otel::MetricsConfig;
use codex_protocol::ThreadId;
use codex_protocol::protocol::SessionSource;
use opentelemetry::KeyValue;
use opentelemetry_sdk::metrics::InMemoryMetricExporter;
use opentelemetry_sdk::metrics::data::AggregatedMetrics;
use opentelemetry_sdk::metrics::data::Metric;
use opentelemetry_sdk::metrics::data::MetricData;
use opentelemetry_sdk::metrics::data::ResourceMetrics;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
fn test_session_telemetry() -> SessionTelemetry {
let exporter = InMemoryMetricExporter::default();
let metrics = MetricsClient::new(
MetricsConfig::in_memory("test", "codex-core", env!("CARGO_PKG_VERSION"), exporter)
.with_runtime_reader(),
)
.expect("in-memory metrics client");
SessionTelemetry::new(
ThreadId::new(),
"gpt-5.4",
"gpt-5.4",
/*account_id*/ None,
/*account_email*/ None,
/*auth_mode*/ None,
"test_originator".to_string(),
/*log_user_prompts*/ false,
"tty".to_string(),
SessionSource::Cli,
)
.with_metrics_without_metadata_tags(metrics)
}
fn find_metric<'a>(resource_metrics: &'a ResourceMetrics, name: &str) -> &'a Metric {
for scope_metrics in resource_metrics.scope_metrics() {
for metric in scope_metrics.metrics() {
if metric.name() == name {
return metric;
}
}
}
panic!("metric {name} missing");
}
fn attributes_to_map<'a>(
attributes: impl Iterator<Item = &'a KeyValue>,
) -> BTreeMap<String, String> {
attributes
.map(|kv| (kv.key.as_str().to_string(), kv.value.as_str().to_string()))
.collect()
}
fn counter_point(
resource_metrics: &ResourceMetrics,
name: &str,
) -> (BTreeMap<String, String>, u64) {
let metric = find_metric(resource_metrics, name);
match metric.data() {
AggregatedMetrics::U64(data) => match data {
MetricData::Sum(sum) => {
let points: Vec<_> = sum.data_points().collect();
assert_eq!(points.len(), 1);
let point = points[0];
(attributes_to_map(point.attributes()), point.value())
}
_ => panic!("unexpected counter aggregation"),
},
_ => panic!("unexpected counter data type"),
}
}
fn histogram_sums(resource_metrics: &ResourceMetrics, name: &str) -> BTreeMap<String, u64> {
let metric = find_metric(resource_metrics, name);
match metric.data() {
AggregatedMetrics::F64(data) => match data {
MetricData::Histogram(histogram) => histogram
.data_points()
.map(|point| {
let attrs = attributes_to_map(point.attributes());
(
attrs
.get("token_type")
.cloned()
.unwrap_or_else(|| "sample".to_string()),
point.sum() as u64,
)
})
.collect(),
_ => panic!("unexpected histogram aggregation"),
},
_ => panic!("unexpected histogram data type"),
}
}
#[test]
fn guardian_review_metrics_record_counts_durations_and_token_usage() {
let session_telemetry = test_session_telemetry();
let result = GuardianReviewAnalyticsResult {
decision: GuardianReviewDecision::Approved,
terminal_status: GuardianReviewTerminalStatus::Approved,
risk_level: Some(GuardianRiskLevel::Low),
user_authorization: Some(GuardianUserAuthorization::High),
outcome: Some(GuardianAssessmentOutcome::Allow),
guardian_session_kind: Some(GuardianReviewSessionKind::TrunkReused),
guardian_model: Some("gpt-5.4 guardian".to_string()),
guardian_reasoning_effort: Some("low".to_string()),
had_prior_review_context: Some(true),
reviewed_action_truncated: true,
token_usage: Some(TokenUsage {
input_tokens: 10,
cached_input_tokens: 4,
output_tokens: 3,
reasoning_output_tokens: 2,
total_tokens: 15,
}),
time_to_first_token_ms: Some(123),
..GuardianReviewAnalyticsResult::without_session()
};
emit_guardian_review_metrics(
&session_telemetry,
&result,
GuardianApprovalRequestSource::DelegatedSubagent,
&GuardianReviewedAction::NetworkAccess {
protocol: codex_protocol::approvals::NetworkApprovalProtocol::Https,
port: 443,
},
/*completion_latency_ms*/ 456,
);
let snapshot = session_telemetry
.snapshot_metrics()
.expect("runtime metrics snapshot");
let (attrs, value) = counter_point(&snapshot, GUARDIAN_REVIEW_COUNT_METRIC);
assert_eq!(value, 1);
assert_eq!(
attrs,
BTreeMap::from([
("action".to_string(), "network_access".to_string()),
(
"approval_request_source".to_string(),
"delegated_subagent".to_string()
),
("decision".to_string(), "approved".to_string()),
("failure_reason".to_string(), "none".to_string()),
("guardian_model".to_string(), "gpt-5.4_guardian".to_string()),
("guardian_reasoning_effort".to_string(), "low".to_string()),
("had_prior_review_context".to_string(), "true".to_string()),
("outcome".to_string(), "allow".to_string()),
("reviewed_action_truncated".to_string(), "true".to_string()),
("risk_level".to_string(), "low".to_string()),
("session_kind".to_string(), "trunk_reused".to_string()),
("terminal_status".to_string(), "approved".to_string()),
("user_authorization".to_string(), "high".to_string()),
])
);
assert_eq!(
histogram_sums(&snapshot, GUARDIAN_REVIEW_TOKEN_USAGE_METRIC),
BTreeMap::from([
("cached_input".to_string(), 4),
("input".to_string(), 10),
("non_cached_input".to_string(), 6),
("output".to_string(), 3),
("reasoning_output".to_string(), 2),
("total".to_string(), 15),
])
);
assert_eq!(
histogram_sums(&snapshot, GUARDIAN_REVIEW_DURATION_METRIC),
BTreeMap::from([("sample".to_string(), 456)])
);
assert_eq!(
histogram_sums(&snapshot, GUARDIAN_REVIEW_TTFT_DURATION_METRIC),
BTreeMap::from([("sample".to_string(), 123)])
);
}
}
+4
View File
@@ -12,6 +12,7 @@
//! 4. Apply the guardian's explicit allow/deny outcome.
mod approval_request;
mod metrics;
mod prompt;
mod review;
mod review_session;
@@ -40,6 +41,7 @@ pub(crate) use review::review_approval_request_with_cancel;
pub(crate) use review::routes_approval_to_guardian;
pub(crate) use review::spawn_approval_request_review;
pub(crate) use review_session::GuardianReviewSessionManager;
pub(crate) use review_session::prompt_cache_key_override_for_review_session;
pub(crate) const GUARDIAN_REVIEW_TIMEOUT: Duration = Duration::from_secs(90);
pub(crate) const GUARDIAN_REVIEWER_NAME: &str = "guardian";
@@ -147,6 +149,8 @@ use prompt::GuardianTranscriptEntryKind;
#[cfg(test)]
use prompt::build_guardian_prompt_items;
#[cfg(test)]
use prompt::build_guardian_prompt_items_with_parent_turn;
#[cfg(test)]
use prompt::collect_guardian_transcript_entries;
#[cfg(test)]
use prompt::guardian_output_schema;
+49
View File
@@ -10,6 +10,7 @@ use serde_json::Value;
use crate::compact::content_items_to_text;
use crate::event_mapping::is_contextual_user_message_content;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use codex_utils_output_truncation::approx_bytes_for_tokens;
use codex_utils_output_truncation::approx_token_count;
use codex_utils_output_truncation::approx_tokens_from_byte_count;
@@ -86,11 +87,29 @@ pub(crate) enum GuardianPromptMode {
/// Split the variable request into separate user content items so the
/// Responses request snapshot shows clear boundaries while preserving exact
/// prompt text through trailing newlines.
#[cfg(test)]
pub(crate) async fn build_guardian_prompt_items(
session: &Session,
retry_reason: Option<String>,
request: GuardianApprovalRequest,
mode: GuardianPromptMode,
) -> serde_json::Result<GuardianPromptItems> {
build_guardian_prompt_items_with_parent_turn(
session,
/*parent_turn*/ None,
retry_reason,
request,
mode,
)
.await
}
pub(crate) async fn build_guardian_prompt_items_with_parent_turn(
session: &Session,
parent_turn: Option<&TurnContext>,
retry_reason: Option<String>,
request: GuardianApprovalRequest,
mode: GuardianPromptMode,
) -> serde_json::Result<GuardianPromptItems> {
let history = session.clone_history().await;
let transcript_entries = collect_guardian_transcript_entries(history.raw_items());
@@ -172,6 +191,11 @@ pub(crate) async fn build_guardian_prompt_items(
if let Some(note) = omission_note {
push_text(format!("\n{note}\n"));
}
if let Some(denied_reads_context) = parent_turn.and_then(parent_turn_denied_reads_context) {
push_text("\n>>> PARENT TURN PERMISSION CONTEXT START\n".to_string());
push_text(denied_reads_context);
push_text(">>> PARENT TURN PERMISSION CONTEXT END\n".to_string());
}
match &request {
GuardianApprovalRequest::NetworkAccess { trigger, .. } => {
push_text(">>> APPROVAL REQUEST START\n".to_string());
@@ -216,6 +240,31 @@ pub(crate) async fn build_guardian_prompt_items(
})
}
fn parent_turn_denied_reads_context(turn: &TurnContext) -> Option<String> {
#[allow(deprecated)]
let cwd = &turn.cwd;
let file_system_policy = turn.permission_profile.file_system_sandbox_policy();
let mut entries = file_system_policy
.get_unreadable_roots_with_cwd(cwd)
.into_iter()
.map(|root| format!("- path `{}`", root.to_string_lossy()))
.collect::<Vec<_>>();
entries.extend(
file_system_policy
.get_unreadable_globs_with_cwd(cwd)
.into_iter()
.map(|glob| format!("- glob `{glob}`")),
);
if entries.is_empty() {
return None;
}
Some(format!(
"The parent turn's active permission profile denies reading these paths/globs. These are policy restrictions; do not approve escalation whose purpose is to read them.\n{}\n",
entries.join("\n")
))
}
enum GuardianPromptShape {
Full,
Delta { already_seen_entry_count: usize },
+23 -1
View File
@@ -4,6 +4,7 @@ use codex_analytics::GuardianReviewDecision;
use codex_analytics::GuardianReviewFailureReason;
use codex_analytics::GuardianReviewTerminalStatus;
use codex_analytics::GuardianReviewTrackContext;
use codex_analytics::GuardianReviewedAction;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
@@ -36,6 +37,7 @@ use super::approval_request::guardian_assessment_action;
use super::approval_request::guardian_request_target_item_id;
use super::approval_request::guardian_request_turn_id;
use super::approval_request::guardian_reviewed_action;
use super::metrics::emit_guardian_review_metrics;
use super::prompt::guardian_output_schema;
use super::prompt::parse_guardian_assessment;
use super::review_session::GuardianReviewSessionOutcome;
@@ -162,9 +164,18 @@ pub(crate) fn is_guardian_reviewer_source(
fn track_guardian_review(
session: &Session,
tracking: &GuardianReviewTrackContext,
approval_request_source: GuardianApprovalRequestSource,
reviewed_action: &GuardianReviewedAction,
result: GuardianReviewAnalyticsResult,
completed_at_ms: u64,
) {
emit_guardian_review_metrics(
&session.services.session_telemetry,
&result,
approval_request_source,
reviewed_action,
completed_at_ms.saturating_sub(tracking.started_at_ms),
);
session
.services
.analytics_events_client
@@ -244,13 +255,14 @@ async fn run_guardian_review(
let target_item_id = guardian_request_target_item_id(&request).map(str::to_string);
let assessment_turn_id = guardian_request_turn_id(&request, &turn.sub_id).to_string();
let action_summary = guardian_assessment_action(&request);
let reviewed_action = guardian_reviewed_action(&request);
let review_tracking = GuardianReviewTrackContext::new(
session.conversation_id.to_string(),
assessment_turn_id.clone(),
review_id.clone(),
target_item_id.clone(),
approval_request_source,
guardian_reviewed_action(&request),
reviewed_action.clone(),
GUARDIAN_REVIEW_TIMEOUT.as_millis() as u64,
);
let started_at_ms = review_tracking.started_at_ms.try_into().unwrap_or_default();
@@ -281,6 +293,8 @@ async fn run_guardian_review(
track_guardian_review(
session.as_ref(),
&review_tracking,
approval_request_source,
&reviewed_action,
GuardianReviewAnalyticsResult {
decision: GuardianReviewDecision::Aborted,
terminal_status: GuardianReviewTerminalStatus::Aborted,
@@ -330,6 +344,8 @@ async fn run_guardian_review(
track_guardian_review(
session.as_ref(),
&review_tracking,
approval_request_source,
&reviewed_action,
GuardianReviewAnalyticsResult {
decision: if approved {
GuardianReviewDecision::Approved
@@ -361,6 +377,8 @@ async fn run_guardian_review(
track_guardian_review(
session.as_ref(),
&review_tracking,
approval_request_source,
&reviewed_action,
GuardianReviewAnalyticsResult {
decision: GuardianReviewDecision::Denied,
terminal_status: GuardianReviewTerminalStatus::TimedOut,
@@ -402,6 +420,8 @@ async fn run_guardian_review(
track_guardian_review(
session.as_ref(),
&review_tracking,
approval_request_source,
&reviewed_action,
GuardianReviewAnalyticsResult {
decision: GuardianReviewDecision::Aborted,
terminal_status: GuardianReviewTerminalStatus::Aborted,
@@ -446,6 +466,8 @@ async fn run_guardian_review(
track_guardian_review(
session.as_ref(),
&review_tracking,
approval_request_source,
&reviewed_action,
GuardianReviewAnalyticsResult {
decision: GuardianReviewDecision::Denied,
terminal_status: GuardianReviewTerminalStatus::FailedClosed,
+61 -4
View File
@@ -7,6 +7,7 @@ use std::time::Duration;
use anyhow::anyhow;
use codex_analytics::GuardianReviewAnalyticsResult;
use codex_analytics::GuardianReviewSessionKind;
use codex_protocol::ThreadId;
use codex_protocol::config_types::AutoCompactTokenLimitScope;
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
@@ -20,6 +21,7 @@ use codex_protocol::protocol::InitialHistory;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::TokenUsage;
use serde_json::Value;
@@ -49,7 +51,7 @@ use super::GUARDIAN_REVIEWER_NAME;
use super::GuardianApprovalRequest;
use super::prompt::GuardianPromptMode;
use super::prompt::GuardianTranscriptCursor;
use super::prompt::build_guardian_prompt_items;
use super::prompt::build_guardian_prompt_items_with_parent_turn;
use super::prompt::guardian_policy_prompt;
use super::prompt::guardian_policy_prompt_with_config;
@@ -183,6 +185,20 @@ impl GuardianReviewSessionReuseKey {
}
}
pub(crate) fn prompt_cache_key_override_for_review_session(
session_source: &SessionSource,
parent_thread_id: Option<ThreadId>,
) -> Option<String> {
let SessionSource::SubAgent(SubAgentSource::Other(name)) = session_source else {
return None;
};
if name != GUARDIAN_REVIEWER_NAME {
return None;
}
let parent_thread_id = parent_thread_id?;
Some(format!("guardian:{parent_thread_id}"))
}
impl GuardianReviewSession {
async fn shutdown(&self) {
self.cancel_token.cancel();
@@ -670,8 +686,9 @@ async fn run_review_on_session(
)
.await;
build_guardian_prompt_items(
build_guardian_prompt_items_with_parent_turn(
params.parent_session.as_ref(),
Some(params.parent_turn.as_ref()),
params.retry_reason.clone(),
params.request.clone(),
prompt_mode,
@@ -774,12 +791,11 @@ async fn run_review_on_session(
}
async fn append_guardian_followup_reminder(review_session: &GuardianReviewSession) {
let turn_context = review_session.codex.session.new_default_turn().await;
let reminder: ResponseItem = ContextualUserFragment::into(GuardianFollowupReviewReminder);
review_session
.codex
.session
.record_conversation_items(turn_context.as_ref(), std::slice::from_ref(&reminder))
.inject_no_new_turn(vec![reminder], /*current_turn_context*/ None)
.await;
}
@@ -905,6 +921,7 @@ pub(crate) fn build_guardian_review_session_config(
.map(guardian_policy_prompt_with_config)
.unwrap_or_else(guardian_policy_prompt),
);
guardian_config.notify = None;
guardian_config.developer_instructions = None;
guardian_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
guardian_config
@@ -1159,6 +1176,46 @@ mod tests {
);
}
#[tokio::test]
async fn guardian_prompt_cache_key_is_scoped_to_parent_thread() {
let session_source =
SessionSource::SubAgent(SubAgentSource::Other(GUARDIAN_REVIEWER_NAME.to_string()));
let parent_thread_id = ThreadId::new();
let key =
prompt_cache_key_override_for_review_session(&session_source, Some(parent_thread_id))
.expect("guardian prompt cache key");
assert_eq!(key, format!("guardian:{parent_thread_id}"));
assert!(
key.len() <= 64,
"guardian prompt cache key should fit the Responses API limit"
);
assert_eq!(
key,
prompt_cache_key_override_for_review_session(&session_source, Some(parent_thread_id))
.expect("same guardian prompt cache key")
);
assert_ne!(
key,
prompt_cache_key_override_for_review_session(&session_source, Some(ThreadId::new()))
.expect("different parent guardian prompt cache key")
);
assert_eq!(
None,
prompt_cache_key_override_for_review_session(
&SessionSource::Cli,
Some(parent_thread_id)
)
);
assert_eq!(
None,
prompt_cache_key_override_for_review_session(
&session_source,
/*parent_thread_id*/ None
)
);
}
#[tokio::test]
async fn guardian_review_session_compact_scope_change_invalidates_cached_session() {
let parent_config = crate::config::test_config().await;
+102 -14
View File
@@ -33,6 +33,11 @@ use codex_protocol::models::ContentItem;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
@@ -191,7 +196,8 @@ async fn guardian_test_session_and_turn_with_base_url(
async fn seed_guardian_parent_history(session: &Arc<Session>, turn: &Arc<TurnContext>) {
session
.record_into_history(
.record_conversation_items(
turn.as_ref(),
&[
ResponseItem::Message {
id: None,
@@ -225,7 +231,6 @@ async fn seed_guardian_parent_history(session: &Arc<Session>, turn: &Arc<TurnCon
phase: None,
},
],
turn.as_ref(),
)
.await;
}
@@ -357,12 +362,71 @@ async fn build_guardian_prompt_full_mode_preserves_initial_review_format() -> an
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Result<()> {
let (mut session, mut turn) = crate::session::tests::make_session_and_context().await;
session.conversation_id = fixed_guardian_parent_session_id();
let denied_root = test_path_buf("/repo/private").abs();
let denied_glob = test_path_buf("/repo/private/**").display().to_string();
turn.permission_profile = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied_root.clone(),
},
access: FileSystemAccessMode::Deny,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: denied_glob.clone(),
},
access: FileSystemAccessMode::Deny,
},
]),
NetworkSandboxPolicy::Restricted,
);
let session = Arc::new(session);
let turn = Arc::new(turn);
seed_guardian_parent_history(&session, &turn).await;
let prompt = build_guardian_prompt_items_with_parent_turn(
session.as_ref(),
Some(turn.as_ref()),
Some("Sandbox denied reading /repo/private/secret.txt.".to_string()),
GuardianApprovalRequest::Shell {
id: "shell-1".to_string(),
command: vec!["cat".to_string(), "/repo/private/secret.txt".to_string()],
cwd: test_path_buf("/repo").abs(),
sandbox_permissions: crate::sandboxing::SandboxPermissions::RequireEscalated,
additional_permissions: None,
justification: Some("Need to inspect the secret file.".to_string()),
},
GuardianPromptMode::Full,
)
.await?;
let text = guardian_prompt_text(&prompt.items);
assert!(text.contains("PARENT TURN PERMISSION CONTEXT START"));
assert!(text.contains("do not approve escalation whose purpose is to read them"));
assert!(text.contains(denied_root.to_string_lossy().as_ref()));
assert!(text.contains(&format!("glob `{denied_glob}`")));
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn build_guardian_prompt_delta_mode_preserves_original_numbering() -> anyhow::Result<()> {
let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await;
seed_guardian_parent_history(&session, &turn).await;
session
.record_into_history(
.record_conversation_items(
turn.as_ref(),
&[
ResponseItem::Message {
id: None,
@@ -381,7 +445,6 @@ async fn build_guardian_prompt_delta_mode_preserves_original_numbering() -> anyh
phase: None,
},
],
turn.as_ref(),
)
.await;
@@ -516,7 +579,8 @@ async fn build_guardian_prompt_stale_delta_version_falls_back_to_full_prompt() -
)
.await;
session
.record_into_history(
.record_conversation_items(
turn.as_ref(),
&[
ResponseItem::Message {
id: None,
@@ -535,7 +599,6 @@ async fn build_guardian_prompt_stale_delta_version_falls_back_to_full_prompt() -
phase: None,
},
],
turn.as_ref(),
)
.await;
@@ -1469,7 +1532,8 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
)
.await;
session
.record_into_history(
.record_conversation_items(
turn.as_ref(),
&[
ResponseItem::Message {
id: None,
@@ -1488,7 +1552,6 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
phase: None,
},
],
turn.as_ref(),
)
.await;
let second_request = GuardianApprovalRequest::Shell {
@@ -1513,7 +1576,8 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
)
.await;
session
.record_into_history(
.record_conversation_items(
turn.as_ref(),
&[
ResponseItem::Message {
id: None,
@@ -1532,7 +1596,6 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
phase: None,
},
],
turn.as_ref(),
)
.await;
let third_request = GuardianApprovalRequest::Shell {
@@ -2005,7 +2068,8 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a
ReviewDecision::Approved
);
session
.record_into_history(
.record_conversation_items(
turn.as_ref(),
&[
ResponseItem::Message {
id: None,
@@ -2024,7 +2088,6 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a
phase: None,
},
],
turn.as_ref(),
)
.await;
@@ -2072,7 +2135,8 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a
"second guardian request was not observed"
);
session
.record_into_history(
.record_conversation_items(
turn.as_ref(),
&[
ResponseItem::Message {
id: None,
@@ -2091,7 +2155,6 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a
phase: None,
},
],
turn.as_ref(),
)
.await;
@@ -2106,7 +2169,13 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a
assert_eq!(third_decision, ReviewDecision::Approved);
let requests = server.requests().await;
assert_eq!(requests.len(), 3);
let second_request_body = serde_json::from_slice::<serde_json::Value>(&requests[1])?;
let third_request_body = serde_json::from_slice::<serde_json::Value>(&requests[2])?;
assert_eq!(
second_request_body["prompt_cache_key"],
third_request_body["prompt_cache_key"],
"forked guardian review should reuse the trunk guardian prompt cache key"
);
let third_request_body_text = third_request_body.to_string();
assert!(
third_request_body_text.contains("first guardian rationale"),
@@ -2215,6 +2284,25 @@ async fn guardian_review_session_config_clears_parent_developer_instructions() {
);
}
#[tokio::test]
async fn guardian_review_session_config_clears_legacy_notify() {
let mut parent_config = test_config().await;
parent_config.notify = Some(vec![
"/path/to/notify".to_string(),
"turn-ended".to_string(),
]);
let guardian_config = build_guardian_review_session_config_for_test(
&parent_config,
/*live_network_config*/ None,
"active-model",
/*reasoning_effort*/ None,
)
.expect("guardian config");
assert_eq!(guardian_config.notify, None);
}
#[tokio::test]
async fn guardian_review_session_config_uses_live_network_proxy_state() {
let mut parent_config = test_config().await;
+11 -8
View File
@@ -500,7 +500,7 @@ pub(crate) async fn inspect_pending_input(
pending_input_item: &TurnInput,
) -> HookRuntimeOutcome {
match pending_input_item {
TurnInput::UserInput(content) => {
TurnInput::UserInput { content, .. } => {
let request = UserPromptSubmitRequest {
session_id: sess.session_id().into(),
turn_id: turn_context.sub_id.clone(),
@@ -522,7 +522,7 @@ pub(crate) async fn inspect_pending_input(
)
.await
}
TurnInput::ResponseInputItem(_) => HookRuntimeOutcome {
TurnInput::ResponseItem(_) => HookRuntimeOutcome {
should_stop: false,
additional_contexts: Vec::new(),
},
@@ -536,13 +536,16 @@ pub(crate) async fn record_pending_input(
additional_contexts: Vec<String>,
) {
match pending_input {
TurnInput::UserInput(content) => {
sess.record_user_prompt_and_emit_turn_item(turn_context.as_ref(), content.as_slice())
.await;
TurnInput::UserInput { content, client_id } => {
sess.record_user_prompt_and_emit_turn_item(
turn_context.as_ref(),
content.as_slice(),
client_id,
)
.await;
}
TurnInput::ResponseInputItem(input) => {
let response_item = ResponseItem::from(input);
sess.record_conversation_items(turn_context, std::slice::from_ref(&response_item))
TurnInput::ResponseItem(item) => {
sess.record_conversation_items(turn_context, std::slice::from_ref(&item))
.await;
}
}
@@ -170,7 +170,7 @@ fn render_tool_params(
.iter()
.filter(|(name, _)| !handled_names.contains(name.as_str()))
.collect::<Vec<_>>();
remaining_params.sort_by(|(left_name, _), (right_name, _)| left_name.cmp(right_name));
remaining_params.sort_by_key(|(name, _)| *name);
for (name, value) in remaining_params {
if handled_names.contains(name.as_str()) {
+7 -7
View File
@@ -56,13 +56,13 @@ fn annotations(
destructive: Option<bool>,
open_world: Option<bool>,
) -> ToolAnnotations {
ToolAnnotations {
destructive_hint: destructive,
idempotent_hint: None,
open_world_hint: open_world,
read_only_hint: read_only,
title: None,
}
ToolAnnotations::from_raw(
/*title*/ None,
read_only,
destructive,
/*idempotent_hint*/ None,
open_world,
)
}
fn approval_metadata(
+7 -1
View File
@@ -3,6 +3,7 @@ use std::collections::HashSet;
use codex_features::Feature;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::ToolInfo as McpToolInfo;
use codex_mcp::tool_is_model_visible;
use crate::config::Config;
use crate::connectors;
@@ -51,7 +52,9 @@ pub(crate) fn build_mcp_tool_exposure(
fn filter_non_codex_apps_mcp_tools_only(mcp_tools: &[McpToolInfo]) -> Vec<McpToolInfo> {
mcp_tools
.iter()
.filter(|tool| tool.server_name != CODEX_APPS_MCP_SERVER_NAME)
.filter(|tool| {
tool.server_name != CODEX_APPS_MCP_SERVER_NAME && tool_is_model_visible(tool)
})
.cloned()
.collect()
}
@@ -72,6 +75,9 @@ fn filter_codex_apps_mcp_tools(
if tool.server_name != CODEX_APPS_MCP_SERVER_NAME {
return false;
}
if !tool_is_model_visible(tool) {
return false;
}
let Some(connector_id) = tool.connector_id.as_deref() else {
return false;
};
+94 -11
View File
@@ -7,6 +7,7 @@ use codex_mcp::ToolInfo;
use codex_tools::ToolName;
use pretty_assertions::assert_eq;
use rmcp::model::JsonObject;
use rmcp::model::Meta;
use rmcp::model::Tool;
use super::*;
@@ -46,17 +47,11 @@ fn make_mcp_tool(
callable_name: callable_name.to_string(),
callable_namespace: callable_namespace.to_string(),
namespace_description: None,
tool: Tool {
name: tool_name.to_string().into(),
title: None,
description: Some(format!("Test tool: {tool_name}").into()),
input_schema: Arc::new(JsonObject::default()),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
tool: Tool::new(
tool_name.to_string(),
format!("Test tool: {tool_name}"),
Arc::new(JsonObject::default()),
),
connector_id: connector_id.map(str::to_string),
connector_name: connector_name.map(str::to_string),
plugin_display_names: Vec::new(),
@@ -86,6 +81,16 @@ fn tool_names(tools: &[ToolInfo]) -> HashSet<ToolName> {
.collect()
}
fn with_visibility(mut tool: ToolInfo, visibility: &[&str]) -> ToolInfo {
tool.tool.meta = Some(Meta(
serde_json::json!({ "ui": { "visibility": visibility } })
.as_object()
.expect("metadata object")
.clone(),
));
tool
}
#[tokio::test]
async fn directly_exposes_small_effective_tool_sets() {
let config = test_config().await;
@@ -99,6 +104,84 @@ async fn directly_exposes_small_effective_tool_sets() {
assert!(exposure.deferred_tools.is_none());
}
#[tokio::test]
async fn excludes_tools_hidden_from_model_exposure() {
let config = test_config().await;
let visible_tool = make_mcp_tool(
"rmcp",
"visible_tool",
"mcp__rmcp",
"visible_tool",
/*connector_id*/ None,
/*connector_name*/ None,
);
let hidden_tool = with_visibility(
make_mcp_tool(
"rmcp",
"hidden_tool",
"mcp__rmcp",
"hidden_tool",
/*connector_id*/ None,
/*connector_name*/ None,
),
&["app"],
);
let empty_visibility_tool = with_visibility(
make_mcp_tool(
"rmcp",
"empty_visibility_tool",
"mcp__rmcp",
"empty_visibility_tool",
/*connector_id*/ None,
/*connector_name*/ None,
),
&[],
);
let visible_app_tool = with_visibility(
make_mcp_tool(
CODEX_APPS_MCP_SERVER_NAME,
"calendar_read",
"mcp__codex_apps__calendar",
"read",
Some("calendar"),
Some("Calendar"),
),
&["app", "model"],
);
let hidden_app_tool = with_visibility(
make_mcp_tool(
CODEX_APPS_MCP_SERVER_NAME,
"calendar_open",
"mcp__codex_apps__calendar",
"open",
Some("calendar"),
Some("Calendar"),
),
&["app"],
);
let mcp_tools = vec![
visible_tool.clone(),
hidden_tool,
empty_visibility_tool,
visible_app_tool.clone(),
hidden_app_tool,
];
let connectors = vec![make_connector("calendar", "Calendar")];
let exposure = build_mcp_tool_exposure(
&mcp_tools,
Some(connectors.as_slice()),
&config,
/*search_tool_enabled*/ false,
);
assert_eq!(
tool_names(&exposure.direct_tools),
tool_names(&[visible_tool, visible_app_tool])
);
assert!(exposure.deferred_tools.is_none());
}
#[tokio::test]
async fn searches_large_effective_tool_sets() {
let config = test_config().await;
@@ -68,6 +68,7 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R
let user_event = RolloutLine {
timestamp: TEST_TIMESTAMP.to_string(),
item: RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "hello".to_string(),
images: None,
local_images: Vec::new(),
+22 -11
View File
@@ -27,6 +27,7 @@ use crate::tasks::UserShellCommandTask;
use crate::tasks::execute_user_shell_command;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::ErrorEvent;
use codex_protocol::protocol::Event;
@@ -84,12 +85,18 @@ pub async fn realtime_conversation_list_voices(sess: &Session, sub_id: String) {
.await;
}
pub async fn user_input_or_turn(sess: &Arc<Session>, sub_id: String, op: Op) {
pub async fn user_input_or_turn(
sess: &Arc<Session>,
sub_id: String,
op: Op,
client_user_message_id: Option<String>,
) {
user_input_or_turn_inner(
sess,
sub_id,
op,
/*mirror_user_text_to_realtime*/ Some(()),
client_user_message_id,
)
.await;
}
@@ -189,6 +196,7 @@ pub(super) async fn user_input_or_turn_inner(
sub_id: String,
op: Op,
mirror_user_text_to_realtime: Option<()>,
client_user_message_id: Option<String>,
) {
let Op::UserInput {
items,
@@ -228,6 +236,7 @@ pub(super) async fn user_input_or_turn_inner(
items.clone(),
additional_context.clone(),
/*expected_turn_id*/ None,
client_user_message_id.clone(),
responsesapi_client_metadata.clone(),
)
.await
@@ -255,10 +264,14 @@ pub(super) async fn user_input_or_turn_inner(
};
let mut task_input = additional_context_input
.into_iter()
.map(TurnInput::ResponseInputItem)
.map(ResponseItem::from)
.map(TurnInput::ResponseItem)
.collect::<Vec<_>>();
if !items.is_empty() {
task_input.push(TurnInput::UserInput(items));
task_input.push(TurnInput::UserInput {
content: items,
client_id: client_user_message_id,
});
}
sess.spawn_task(
Arc::clone(&current_context),
@@ -771,7 +784,8 @@ pub(super) async fn submission_loop(
false
}
Op::UserInput { .. } => {
user_input_or_turn(&sess, sub.id.clone(), sub.op).await;
user_input_or_turn(&sess, sub.id.clone(), sub.op, sub.client_user_message_id)
.await;
false
}
Op::ThreadSettings { thread_settings } => {
@@ -899,17 +913,14 @@ Do not assume this also authorizes similar operations with different payloads.
Approved action:
{approved_action_json}"#,
);
let items = vec![ResponseInputItem::Message {
let items = vec![ResponseItem::from(ResponseInputItem::Message {
role: "developer".to_string(),
content: vec![ContentItem::InputText { text }],
phase: None,
}];
})];
if let Err(items) = sess.inject_response_items(items).await {
sess.input_queue
.queue_response_items_for_next_turn(items)
.await;
}
sess.inject_no_new_turn(items, /*current_turn_context*/ None)
.await;
}
pub(super) fn submission_dispatch_span(sub: &Submission) -> tracing::Span {
+50
View File
@@ -0,0 +1,50 @@
use super::input_queue::TurnInput;
use super::session::Session;
use super::turn_context::TurnContext;
use codex_protocol::models::ResponseItem;
impl Session {
/// Returns the input if there is no active turn to inject into.
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn inject_if_running(
&self,
input: Vec<ResponseItem>,
) -> Result<(), Vec<ResponseItem>> {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
Some(active_turn) => {
self.input_queue
.extend_pending_input_for_turn_state(
active_turn.turn_state.as_ref(),
input.into_iter().map(TurnInput::ResponseItem).collect(),
)
.await;
Ok(())
}
None => Err(input),
}
}
/// Injects items into active work, or records them without starting a turn.
pub(crate) async fn inject_no_new_turn(
&self,
items: Vec<ResponseItem>,
current_turn_context: Option<&TurnContext>,
) {
let Err(items) = self.inject_if_running(items).await else {
return;
};
let default_turn_context;
let turn_context = match current_turn_context {
Some(turn_context) => turn_context,
None => {
default_turn_context = self.new_default_turn().await;
default_turn_context.as_ref()
}
};
self.record_conversation_items(turn_context, &items).await;
}
}
+11 -53
View File
@@ -1,7 +1,7 @@
use crate::state::ActiveTurn;
use crate::state::MailboxDeliveryPhase;
use crate::state::TurnState;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::InterAgentCommunication;
use codex_protocol::user_input::UserInput;
use std::collections::VecDeque;
@@ -11,8 +11,11 @@ use tokio::sync::watch;
#[derive(Clone, Debug, PartialEq)]
pub(crate) enum TurnInput {
UserInput(Vec<UserInput>),
ResponseInputItem(ResponseInputItem),
UserInput {
content: Vec<UserInput>,
client_id: Option<String>,
},
ResponseItem(ResponseItem),
}
/// Turn-local pending input storage owned by the input queue flow.
@@ -25,8 +28,6 @@ pub(crate) struct TurnInputQueue {
pub(crate) struct InputQueue {
mailbox_tx: watch::Sender<()>,
mailbox_pending_mails: Mutex<VecDeque<InterAgentCommunication>>,
idle_pending_input: Mutex<Vec<ResponseInputItem>>,
}
impl InputQueue {
@@ -35,7 +36,6 @@ impl InputQueue {
Self {
mailbox_tx,
mailbox_pending_mails: Mutex::new(VecDeque::new()),
idle_pending_input: Mutex::new(Vec::new()),
}
}
@@ -70,31 +70,15 @@ impl InputQueue {
.any(|mail| mail.trigger_turn)
}
pub(crate) async fn drain_mailbox_input_items(&self) -> Vec<ResponseInputItem> {
pub(crate) async fn drain_mailbox_input_items(&self) -> Vec<ResponseItem> {
self.mailbox_pending_mails
.lock()
.await
.drain(..)
.map(|mail| mail.to_response_input_item())
.map(|mail| ResponseItem::from(mail.to_response_input_item()))
.collect()
}
pub(crate) async fn queue_response_items_for_next_turn(&self, items: Vec<ResponseInputItem>) {
if items.is_empty() {
return;
}
self.idle_pending_input.lock().await.extend(items);
}
pub(crate) async fn take_queued_response_items_for_next_turn(&self) -> Vec<ResponseInputItem> {
std::mem::take(&mut *self.idle_pending_input.lock().await)
}
pub(crate) async fn has_queued_response_items_for_next_turn(&self) -> bool {
!self.idle_pending_input.lock().await.is_empty()
}
pub(crate) async fn turn_state_for_sub_id(
&self,
active_turn: &Mutex<Option<ActiveTurn>>,
@@ -181,32 +165,6 @@ impl InputQueue {
turn_state.lock().await.pending_input.items.split_off(0)
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub(crate) async fn inject_response_items(
&self,
active_turn: &Mutex<Option<ActiveTurn>>,
input: Vec<ResponseInputItem>,
) -> Result<(), Vec<ResponseInputItem>> {
let mut active = active_turn.lock().await;
match active.as_mut() {
Some(active_turn) => {
self.extend_pending_input_for_turn_state(
active_turn.turn_state.as_ref(),
input
.into_iter()
.map(TurnInput::ResponseInputItem)
.collect(),
)
.await;
Ok(())
}
None => Err(input),
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
@@ -235,7 +193,7 @@ impl InputQueue {
.drain_mailbox_input_items()
.await
.into_iter()
.map(TurnInput::ResponseInputItem);
.map(TurnInput::ResponseItem);
if pending_input.is_empty() {
mailbox_items.collect()
} else {
@@ -345,8 +303,8 @@ mod tests {
assert_eq!(
input_queue.drain_mailbox_input_items().await,
vec![
mail_one.to_response_input_item(),
mail_two.to_response_input_item()
ResponseItem::from(mail_one.to_response_input_item()),
ResponseItem::from(mail_two.to_response_input_item())
]
);
assert!(!input_queue.has_pending_mailbox_items().await);
+54 -99
View File
@@ -72,6 +72,7 @@ use codex_network_proxy::normalize_host;
use codex_otel::current_span_trace_id;
use codex_otel::current_span_w3c_trace_context;
use codex_otel::set_parent_from_w3c_trace_context;
use codex_protocol::SessionId;
use codex_protocol::ThreadId;
use codex_protocol::approvals::ElicitationRequestEvent;
use codex_protocol::approvals::ExecPolicyAmendment;
@@ -195,6 +196,7 @@ use codex_protocol::exec_output::StreamOutput;
mod config_lock;
mod handlers;
mod inject;
mod input_queue;
mod mcp;
mod multi_agents;
@@ -323,7 +325,6 @@ use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
@@ -548,37 +549,9 @@ impl Codex {
.or_else(|| conversation_history.get_base_instructions().map(|s| s.text))
.unwrap_or_else(|| model_info.get_model_instructions(config.personality));
// Respect thread-start tools. When missing (resumed/forked threads), read from the db
// first, then fall back to rollout-file tools.
let persisted_tools = if dynamic_tools.is_empty() {
let thread_id = match &conversation_history {
InitialHistory::Resumed(resumed) => Some(resumed.conversation_id),
InitialHistory::Forked(_) => conversation_history.forked_from_id(),
InitialHistory::New | InitialHistory::Cleared => None,
};
match thread_id {
Some(thread_id) => {
let state_db_ctx = if config.ephemeral {
None
} else if let Some(local_store) =
thread_store.as_any().downcast_ref::<LocalThreadStore>()
{
local_store.state_db().await
} else {
None
};
state_db::get_dynamic_tools(state_db_ctx.as_deref(), thread_id, "codex_spawn")
.await
}
None => None,
}
} else {
None
};
// Dynamic tools are defined at thread start and persisted in rollout session metadata.
let dynamic_tools = if dynamic_tools.is_empty() {
persisted_tools
.or_else(|| conversation_history.get_dynamic_tools())
.unwrap_or_default()
conversation_history.get_dynamic_tools().unwrap_or_default()
} else {
dynamic_tools
};
@@ -694,6 +667,25 @@ impl Codex {
let sub = Submission {
id: id.clone(),
op,
client_user_message_id: None,
trace,
};
self.submit_with_id(sub).await?;
Ok(id)
}
pub async fn submit_user_input_with_client_user_message_id(
&self,
op: Op,
trace: Option<W3cTraceContext>,
client_user_message_id: Option<String>,
) -> CodexResult<String> {
debug_assert!(matches!(op, Op::UserInput { .. }));
let id = Uuid::now_v7().to_string();
let sub = Submission {
id: id.clone(),
op,
client_user_message_id,
trace,
};
self.submit_with_id(sub).await?;
@@ -749,6 +741,7 @@ impl Codex {
input: Vec<UserInput>,
additional_context: BTreeMap<String, AdditionalContextEntry>,
expected_turn_id: Option<&str>,
client_user_message_id: Option<String>,
responsesapi_client_metadata: Option<HashMap<String, String>>,
) -> Result<String, SteerInputError> {
self.session
@@ -756,6 +749,7 @@ impl Codex {
input,
additional_context,
expected_turn_id,
client_user_message_id,
responsesapi_client_metadata,
)
.await
@@ -1101,6 +1095,7 @@ impl Session {
thread_settings: Default::default(),
},
/*mirror_user_text_to_realtime*/ None,
/*client_user_message_id*/ None,
)
.await;
}
@@ -1870,27 +1865,11 @@ impl Session {
warn!("execpolicy amendment for {sub_id} had no command prefix");
return;
};
let fragment = ApprovedCommandPrefixSaved::new(prefixes);
let text = fragment.render();
let message: ResponseItem = ContextualUserFragment::into(fragment);
if let Some(turn_context) = self.turn_context_for_sub_id(sub_id).await {
self.record_conversation_items(&turn_context, std::slice::from_ref(&message))
.await;
return;
}
if self
.inject_response_items(vec![ResponseInputItem::Message {
role: "developer".to_string(),
content: vec![ContentItem::InputText { text }],
phase: None,
}])
.await
.is_err()
{
warn!("no active turn found to record execpolicy amendment message for {sub_id}");
}
let message: ResponseItem =
ContextualUserFragment::into(ApprovedCommandPrefixSaved::new(prefixes));
let turn_context = self.turn_context_for_sub_id(sub_id).await;
self.inject_no_new_turn(vec![message], turn_context.as_deref())
.await;
}
pub(crate) async fn persist_network_policy_amendment(
@@ -1967,27 +1946,10 @@ impl Session {
sub_id: &str,
amendment: &NetworkPolicyAmendment,
) {
let fragment = NetworkRuleSaved::new(amendment);
let text = fragment.render();
let message: ResponseItem = ContextualUserFragment::into(fragment);
if let Some(turn_context) = self.turn_context_for_sub_id(sub_id).await {
self.record_conversation_items(&turn_context, std::slice::from_ref(&message))
.await;
return;
}
if self
.inject_response_items(vec![ResponseInputItem::Message {
role: "developer".to_string(),
content: vec![ContentItem::InputText { text }],
phase: None,
}])
.await
.is_err()
{
warn!("no active turn found to record network policy amendment message for {sub_id}");
}
let message: ResponseItem = ContextualUserFragment::into(NetworkRuleSaved::new(amendment));
let turn_context = self.turn_context_for_sub_id(sub_id).await;
self.inject_no_new_turn(vec![message], turn_context.as_deref())
.await;
}
/// Emit an exec approval request event and await the user's decision.
@@ -2538,28 +2500,21 @@ impl Session {
}
}
/// Records input items: always append to conversation history and
/// persist these response items to rollout.
/// Records conversation items: append to history, persist to rollout, and
/// notify clients observing raw response items.
pub(crate) async fn record_conversation_items(
&self,
turn_context: &TurnContext,
items: &[ResponseItem],
) {
self.record_into_history(items, turn_context).await;
{
let mut state = self.state.lock().await;
state.record_items(items.iter(), turn_context.truncation_policy);
}
self.persist_rollout_response_items(items).await;
self.send_raw_response_items(turn_context, items).await;
}
/// Append ResponseItems to the in-memory conversation history only.
pub(crate) async fn record_into_history(
&self,
items: &[ResponseItem],
turn_context: &TurnContext,
) {
let mut state = self.state.lock().await;
state.record_items(items.iter(), turn_context.truncation_policy);
}
async fn maybe_warn_on_server_model_mismatch(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
@@ -3124,6 +3079,7 @@ impl Session {
&self,
turn_context: &TurnContext,
input: &[UserInput],
client_id: Option<String>,
) {
// Persist the user message to history, but emit the turn item from `UserInput` so
// UI-only `text_elements` are preserved. `ResponseItem::Message` does not carry
@@ -3131,7 +3087,9 @@ impl Session {
let response_item = ResponseItem::from(ResponseInputItem::from(input.to_vec()));
self.record_conversation_items(turn_context, std::slice::from_ref(&response_item))
.await;
let turn_item = TurnItem::UserMessage(UserMessageItem::new(input));
let mut user_message_item = UserMessageItem::new(input);
user_message_item.client_id = client_id;
let turn_item = TurnItem::UserMessage(user_message_item);
self.emit_turn_item_started(turn_context, &turn_item).await;
self.emit_turn_item_completed(turn_context, turn_item).await;
self.ensure_rollout_materialized().await;
@@ -3167,6 +3125,7 @@ impl Session {
input: Vec<UserInput>,
additional_context: BTreeMap<String, AdditionalContextEntry>,
expected_turn_id: Option<&str>,
client_user_message_id: Option<String>,
responsesapi_client_metadata: Option<HashMap<String, String>>,
) -> Result<String, SteerInputError> {
let mut active = self.active_turn.lock().await;
@@ -3220,9 +3179,13 @@ impl Session {
let mut pending_input = additional_context_input
.into_iter()
.map(TurnInput::ResponseInputItem)
.map(ResponseItem::from)
.map(TurnInput::ResponseItem)
.collect::<Vec<_>>();
pending_input.push(TurnInput::UserInput(input));
pending_input.push(TurnInput::UserInput {
content: input,
client_id: client_user_message_id,
});
self.input_queue
.extend_pending_input_and_accept_mailbox_delivery_for_turn_state(
active_turn.turn_state.as_ref(),
@@ -3232,16 +3195,6 @@ impl Session {
Ok(active_turn_id.clone())
}
/// Returns the input if there was no task running to inject into.
pub async fn inject_response_items(
&self,
input: Vec<ResponseInputItem>,
) -> Result<(), Vec<ResponseInputItem>> {
self.input_queue
.inject_response_items(&self.active_turn, input)
.await
}
pub(crate) async fn record_memory_citation_for_turn(&self, sub_id: &str) {
let turn_state = self
.input_queue
@@ -3316,6 +3269,7 @@ impl Session {
pub(crate) fn emit_subagent_session_started(
analytics_events_client: &AnalyticsEventsClient,
client_metadata: AppServerClientMetadata,
session_id: SessionId,
thread_id: ThreadId,
parent_thread_id: Option<ThreadId>,
thread_config: ThreadConfigSnapshot,
@@ -3334,6 +3288,7 @@ pub(crate) fn emit_subagent_session_started(
.unwrap_or_default()
.as_secs();
analytics_events_client.track_subagent_thread_started(SubAgentThreadStartedInput {
session_id: session_id.to_string(),
thread_id: thread_id.to_string(),
parent_thread_id: parent_thread_id.map(|thread_id| thread_id.to_string()),
product_client_id: client_name.clone(),
+8 -5
View File
@@ -137,11 +137,14 @@ pub(super) async fn spawn_review_thread(
};
// Seed the child task with the review prompt as the initial user message.
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: review_prompt,
// Review prompt is synthesized; no UI element ranges to preserve.
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: review_prompt,
// Review prompt is synthesized; no UI element ranges to preserve.
text_elements: Vec::new(),
}],
client_id: None,
}];
let tc = Arc::new(review_turn_context);
tc.turn_metadata_state.spawn_git_enrichment_task();
// TODO(ccunningham): Review turns currently rely on `spawn_task` for TurnComplete but do not
@@ -61,6 +61,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ
turn_id: Some(turn_context.sub_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -98,6 +99,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
turn_id: Some(turn_context.sub_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -130,6 +132,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -198,6 +201,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 1 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -228,6 +232,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 2 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -300,6 +305,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_inc
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 1 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -330,6 +336,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_inc
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 2 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -394,6 +401,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 1 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -424,6 +432,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 2 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -517,6 +526,7 @@ async fn reconstruct_history_rollback_counts_inter_agent_assistant_turns() {
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 1 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -608,6 +618,7 @@ async fn reconstruct_history_rollback_clears_history_and_metadata_when_exceeding
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "only user".to_string(),
images: None,
local_images: Vec::new(),
@@ -662,6 +673,7 @@ async fn record_initial_history_resumed_rollback_skips_only_user_turns() {
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -737,6 +749,7 @@ async fn record_initial_history_resumed_rollback_drops_incomplete_user_turn_comp
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -765,6 +778,7 @@ async fn record_initial_history_resumed_rollback_drops_incomplete_user_turn_comp
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "rolled back".to_string(),
images: None,
local_images: Vec::new(),
@@ -898,6 +912,7 @@ async fn reconstruct_history_legacy_compaction_without_replacement_history_clear
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "after legacy compact".to_string(),
images: None,
local_images: Vec::new(),
@@ -933,6 +948,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
turn_id: Some(turn_context.sub_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -963,6 +979,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -1009,6 +1026,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
turn_id: Some(turn_context.sub_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -1036,6 +1054,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
turn_id: Some(turn_context.sub_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -1068,6 +1087,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -1096,6 +1116,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "aborted".to_string(),
images: None,
local_images: Vec::new(),
@@ -1151,6 +1172,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
turn_id: Some(current_turn_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -1178,6 +1200,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -1206,6 +1229,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "current".to_string(),
images: None,
local_images: Vec::new(),
@@ -1265,6 +1289,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
turn_id: Some(turn_context.sub_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -1297,6 +1322,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -1325,6 +1351,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "incomplete".to_string(),
images: None,
local_images: Vec::new(),
@@ -1377,6 +1404,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_preserves_turn_
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "incomplete".to_string(),
images: None,
local_images: Vec::new(),
@@ -1419,6 +1447,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
turn_id: Some(turn_context.sub_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -1452,6 +1481,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -1480,6 +1510,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "compacted".to_string(),
images: None,
local_images: Vec::new(),
+8
View File
@@ -972,6 +972,8 @@ impl Session {
for contributor in extensions.thread_lifecycle_contributors() {
contributor.on_thread_start(codex_extension_api::ThreadStartInput {
config: config.as_ref(),
session_source: &session_configuration.session_source,
persistent_thread_state_available: state_db_ctx.is_some(),
session_store: &session_extension_data,
thread_store: &thread_extension_data,
}).await;
@@ -1041,6 +1043,12 @@ impl Session {
config.features.enabled(Feature::RuntimeMetrics),
Self::build_model_client_beta_features_header(config.as_ref()),
attestation_provider,
)
.with_prompt_cache_key_override(
crate::guardian::prompt_cache_key_override_for_review_session(
&session_configuration.session_source,
session_configuration.forked_from_thread_id,
),
),
code_mode_service: crate::tools::code_mode::CodeModeService::new(),
environment_manager,
+407 -133
View File
@@ -103,9 +103,9 @@ use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::Settings;
use codex_protocol::models::BaseInstructions;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::CompactedItem;
use codex_protocol::protocol::ConversationAudioParams;
use codex_protocol::protocol::CreditsSnapshot;
@@ -398,6 +398,12 @@ async fn interrupting_regular_turn_waiting_on_startup_prewarm_emits_turn_aborted
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
let marker_evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("expected turn aborted marker event")
.expect("channel open");
assert!(matches!(marker_evt.msg, EventMsg::RawResponseItem(_)));
let second = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("expected turn aborted event")
@@ -1772,7 +1778,7 @@ async fn recompute_token_usage_uses_session_base_instructions() {
let item = user_message("hello");
session
.record_into_history(std::slice::from_ref(&item), &turn_context)
.record_conversation_items(&turn_context, std::slice::from_ref(&item))
.await;
let history = session.clone_history().await;
@@ -2037,6 +2043,85 @@ async fn turn_start_lifecycle_exposes_turn_metadata_and_token_baseline() {
assert_eq!(vec![expected], actual);
}
#[tokio::test]
async fn turn_error_lifecycle_exposes_error_and_stores() {
struct SessionTurnErrorMarker;
struct ThreadTurnErrorMarker;
#[derive(Debug, PartialEq, Eq)]
struct RecordedTurnError {
session_level_id: String,
thread_level_id: String,
turn_level_id: String,
turn_id: String,
error: CodexErrorInfo,
saw_session_store: bool,
saw_thread_store: bool,
}
struct TurnErrorRecorder {
records: Arc<std::sync::Mutex<Vec<RecordedTurnError>>>,
}
#[async_trait::async_trait]
impl codex_extension_api::TurnLifecycleContributor for TurnErrorRecorder {
async fn on_turn_error(&self, input: codex_extension_api::TurnErrorInput<'_>) {
self.records
.lock()
.expect("turn error records lock")
.push(RecordedTurnError {
session_level_id: input.session_store.level_id().to_string(),
thread_level_id: input.thread_store.level_id().to_string(),
turn_level_id: input.turn_store.level_id().to_string(),
turn_id: input.turn_id.to_string(),
error: input.error,
saw_session_store: input
.session_store
.get::<SessionTurnErrorMarker>()
.is_some(),
saw_thread_store: input.thread_store.get::<ThreadTurnErrorMarker>().is_some(),
});
}
}
let (mut session, turn_context) = make_session_and_context().await;
let records = Arc::new(std::sync::Mutex::new(Vec::new()));
let mut builder = codex_extension_api::ExtensionRegistryBuilder::<crate::config::Config>::new();
builder.turn_lifecycle_contributor(Arc::new(TurnErrorRecorder {
records: Arc::clone(&records),
}));
session.services.extensions = Arc::new(builder.build());
session
.services
.session_extension_data
.insert(SessionTurnErrorMarker);
session
.services
.thread_extension_data
.insert(ThreadTurnErrorMarker);
let expected = RecordedTurnError {
session_level_id: session.session_id().to_string(),
thread_level_id: session.conversation_id.to_string(),
turn_level_id: turn_context.sub_id.clone(),
turn_id: turn_context.sub_id.clone(),
error: CodexErrorInfo::UsageLimitExceeded,
saw_session_store: true,
saw_thread_store: true,
};
session
.emit_turn_error_lifecycle(&turn_context, CodexErrorInfo::UsageLimitExceeded)
.await;
let actual = records
.lock()
.expect("turn error records lock")
.drain(..)
.collect::<Vec<_>>();
assert_eq!(vec![expected], actual);
}
#[tokio::test]
async fn config_change_contributor_observes_effective_config_changes() {
struct SessionConfigMarker;
@@ -2375,6 +2460,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() {
turn_id: Some(turn_context.sub_id.clone()),
#[allow(deprecated)]
cwd: turn_context.cwd.to_path_buf(),
workspace_roots: None,
current_date: turn_context.current_date.clone(),
timezone: turn_context.timezone.clone(),
approval_policy: turn_context.approval_policy.value(),
@@ -2405,6 +2491,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() {
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "forked seed".to_string(),
images: None,
local_images: Vec::new(),
@@ -2547,7 +2634,7 @@ async fn thread_rollback_fails_without_persisted_thread_history() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let initial_context = sess.build_initial_context(tc.as_ref()).await;
sess.record_into_history(&initial_context, tc.as_ref())
sess.record_conversation_items(tc.as_ref(), &initial_context)
.await;
handlers::thread_rollback(&sess, "sub-1".to_string(), /*num_turns*/ 1).await;
@@ -2601,6 +2688,7 @@ async fn thread_rollback_recomputes_previous_turn_settings_and_reference_context
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 1 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -2629,6 +2717,7 @@ async fn thread_rollback_recomputes_previous_turn_settings_and_reference_context
)),
RolloutItem::EventMsg(EventMsg::UserMessage(
codex_protocol::protocol::UserMessageEvent {
client_id: None,
message: "turn 2 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -2713,6 +2802,7 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio
},
)),
RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "turn 1 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -2759,6 +2849,7 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio
},
)),
RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "turn 2 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -2815,6 +2906,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() {
},
)),
RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "turn 1 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -2841,6 +2933,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() {
},
)),
RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "turn 2 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -2867,6 +2960,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() {
},
)),
RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "turn 3 user".to_string(),
images: None,
local_images: Vec::new(),
@@ -2920,7 +3014,7 @@ async fn thread_rollback_fails_when_turn_in_progress() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let initial_context = sess.build_initial_context(tc.as_ref()).await;
sess.record_into_history(&initial_context, tc.as_ref())
sess.record_conversation_items(tc.as_ref(), &initial_context)
.await;
*sess.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
@@ -2941,7 +3035,7 @@ async fn thread_rollback_fails_when_num_turns_is_zero() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let initial_context = sess.build_initial_context(tc.as_ref()).await;
sess.record_into_history(&initial_context, tc.as_ref())
sess.record_conversation_items(tc.as_ref(), &initial_context)
.await;
handlers::thread_rollback(&sess, "sub-1".to_string(), /*num_turns*/ 0).await;
@@ -5326,6 +5420,7 @@ async fn submit_with_id_captures_current_span_trace_context() {
.submit_with_id(Submission {
id: "sub-1".into(),
op: Op::Interrupt,
client_user_message_id: None,
trace: None,
})
.await
@@ -5397,6 +5492,7 @@ fn submission_dispatch_span_prefers_submission_trace_context() {
submission_dispatch_span(&Submission {
id: "sub-1".into(),
op: Op::Interrupt,
client_user_message_id: None,
trace: Some(submission_trace),
})
});
@@ -5423,6 +5519,7 @@ fn submission_dispatch_span_uses_debug_for_realtime_audio() {
item_id: None,
},
}),
client_user_message_id: None,
trace: None,
});
@@ -5490,6 +5587,7 @@ async fn user_turn_updates_approvals_reviewer() {
..Default::default()
},
},
/*client_user_message_id*/ None,
)
.await;
@@ -5783,6 +5881,7 @@ async fn spawn_task_turn_span_inherits_dispatch_trace_context() {
let dispatch_span = submission_dispatch_span(&Submission {
id: "sub-1".into(),
op: Op::Interrupt,
client_user_message_id: None,
trace: Some(submission_trace.clone()),
});
let dispatch_span_id = dispatch_span.context().span().span_context().span_id();
@@ -5793,10 +5892,13 @@ async fn spawn_task_turn_span_inherits_dispatch_trace_context() {
async {
sess.spawn_task(
Arc::clone(&tc),
vec![TurnInput::UserInput(vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}])],
vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}],
TraceCaptureTask {
captured_trace: Arc::clone(&captured_trace),
},
@@ -6606,10 +6708,13 @@ async fn spawn_task_does_not_update_previous_turn_settings_for_non_run_turn_task
let (sess, tc, _rx) = make_session_and_context_with_rx().await;
sess.set_previous_turn_settings(/*previous_turn_settings*/ None)
.await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(
Arc::clone(&tc),
@@ -7533,7 +7638,7 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co
phase: None,
};
session
.record_into_history(std::slice::from_ref(&compacted_summary), &turn_context)
.record_conversation_items(&turn_context, std::slice::from_ref(&compacted_summary))
.await;
session
.record_context_updates_and_set_reference_context_item(&turn_context)
@@ -7692,6 +7797,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_full_rei
session
.persist_rollout_items(&[RolloutItem::EventMsg(EventMsg::UserMessage(
UserMessageEvent {
client_id: None,
message: "seed rollout".to_string(),
images: None,
local_images: Vec::new(),
@@ -7811,6 +7917,29 @@ async fn realtime_conversation_list_voices_emits_builtin_list() {
);
}
#[derive(Clone, Copy)]
struct CompletingTask;
impl SessionTask for CompletingTask {
fn kind(&self) -> TaskKind {
TaskKind::Regular
}
fn span_name(&self) -> &'static str {
"session_task.completing"
}
async fn run(
self: Arc<Self>,
_session: Arc<SessionTaskContext>,
_ctx: Arc<TurnContext>,
_input: Vec<TurnInput>,
_cancellation_token: CancellationToken,
) -> Option<String> {
None
}
}
#[derive(Clone, Copy)]
struct NeverEndingTask {
kind: TaskKind,
@@ -7875,10 +8004,13 @@ impl SessionTask for GuardianDeniedApprovalTask {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn guardian_auto_review_interrupts_after_three_consecutive_denials() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "trigger guardian denials".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "trigger guardian denials".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(Arc::clone(&tc), input, GuardianDeniedApprovalTask)
.await;
@@ -7906,10 +8038,13 @@ async fn guardian_auto_review_interrupts_after_three_consecutive_denials() {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn guardian_helper_review_interrupts_after_three_consecutive_denials() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "keep turn active for helper reviews".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "keep turn active for helper reviews".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(
Arc::clone(&tc),
input,
@@ -7964,12 +8099,15 @@ async fn guardian_helper_review_interrupts_after_three_consecutive_denials() {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_log::test]
async fn abort_regular_task_emits_turn_aborted_only() {
async fn abort_regular_task_emits_marker_before_turn_aborted() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(
Arc::clone(&tc),
input,
@@ -7982,8 +8120,13 @@ async fn abort_regular_task_emits_turn_aborted_only() {
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
// Interrupts persist a model-visible `<turn_aborted>` marker into history, but there is no
// separate client-visible event for that marker (only `EventMsg::TurnAborted`).
// Interrupts surface the model-visible `<turn_aborted>` marker before the abort event.
let marker_evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for marker event")
.expect("event");
assert!(matches!(marker_evt.msg, EventMsg::RawResponseItem(_)));
let evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for event")
@@ -7997,12 +8140,15 @@ async fn abort_regular_task_emits_turn_aborted_only() {
}
#[tokio::test]
async fn abort_gracefully_emits_turn_aborted_only() {
async fn abort_gracefully_emits_marker_before_turn_aborted() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(
Arc::clone(&tc),
input,
@@ -8015,8 +8161,13 @@ async fn abort_gracefully_emits_turn_aborted_only() {
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
// Even if tasks handle cancellation gracefully, interrupts still result in `TurnAborted`
// being the only client-visible signal.
// Gracefully cancelled tasks surface the model-visible marker before the abort event too.
let marker_evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for marker event")
.expect("event");
assert!(matches!(marker_evt.msg, EventMsg::RawResponseItem(_)));
let evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for event")
@@ -8032,10 +8183,13 @@ async fn abort_gracefully_emits_turn_aborted_only() {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(
Arc::clone(&tc),
input,
@@ -8060,6 +8214,7 @@ async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input()
pending_user_input.clone(),
/*additional_context*/ Default::default(),
Some(&tc.sub_id),
/*client_user_message_id*/ None,
/*responsesapi_client_metadata*/ None,
)
.await
@@ -8119,6 +8274,7 @@ async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input()
assert!(matches!(
fourth.msg,
EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message,
images,
text_elements,
@@ -8145,6 +8301,86 @@ async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input()
));
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn task_finish_emits_thread_idle_lifecycle_after_active_turn_clears() {
struct ThreadIdleRecorder {
calls: Arc<std::sync::atomic::AtomicUsize>,
idle_tx: async_channel::Sender<()>,
expected_thread_id: ThreadId,
}
#[async_trait::async_trait]
impl codex_extension_api::ThreadLifecycleContributor<crate::config::Config> for ThreadIdleRecorder {
async fn on_thread_idle(&self, input: codex_extension_api::ThreadIdleInput<'_>) {
assert_eq!(
self.expected_thread_id.to_string(),
input.thread_store.level_id()
);
self.calls.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
self.idle_tx.send(()).await.expect("idle receiver open");
}
}
let (mut session, turn_context) = make_session_and_context().await;
let calls = Arc::new(std::sync::atomic::AtomicUsize::new(0));
let (idle_tx, idle_rx) = async_channel::bounded(1);
let mut builder = codex_extension_api::ExtensionRegistryBuilder::<crate::config::Config>::new();
builder.thread_lifecycle_contributor(Arc::new(ThreadIdleRecorder {
calls: Arc::clone(&calls),
idle_tx,
expected_thread_id: session.conversation_id,
}));
session.services.extensions = Arc::new(builder.build());
let session = Arc::new(session);
session
.spawn_task(Arc::new(turn_context), Vec::new(), CompletingTask)
.await;
timeout(StdDuration::from_secs(2), idle_rx.recv())
.await
.expect("thread idle lifecycle")
.expect("idle receiver open");
assert_eq!(1, calls.load(std::sync::atomic::Ordering::SeqCst));
assert!(session.active_turn.lock().await.is_none());
}
#[tokio::test]
async fn thread_idle_lifecycle_waits_for_trigger_turn_mailbox_work() {
struct ThreadIdleRecorder {
calls: Arc<std::sync::atomic::AtomicUsize>,
}
#[async_trait::async_trait]
impl codex_extension_api::ThreadLifecycleContributor<crate::config::Config> for ThreadIdleRecorder {
async fn on_thread_idle(&self, _input: codex_extension_api::ThreadIdleInput<'_>) {
self.calls.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
}
}
let (mut session, _turn_context) = make_session_and_context().await;
let calls = Arc::new(std::sync::atomic::AtomicUsize::new(0));
let mut builder = codex_extension_api::ExtensionRegistryBuilder::<crate::config::Config>::new();
builder.thread_lifecycle_contributor(Arc::new(ThreadIdleRecorder {
calls: Arc::clone(&calls),
}));
session.services.extensions = Arc::new(builder.build());
session
.input_queue
.enqueue_mailbox_communication(InterAgentCommunication::new(
AgentPath::root(),
AgentPath::root(),
Vec::new(),
"pending trigger".to_string(),
/*trigger_turn*/ true,
))
.await;
session.emit_thread_idle_lifecycle_if_idle().await;
assert_eq!(0, calls.load(std::sync::atomic::Ordering::SeqCst));
}
#[tokio::test]
async fn steer_input_requires_active_turn() {
let (sess, _tc, _rx) = make_session_and_context_with_rx().await;
@@ -8158,6 +8394,7 @@ async fn steer_input_requires_active_turn() {
input,
/*additional_context*/ Default::default(),
/*expected_turn_id*/ None,
/*client_user_message_id*/ None,
/*responsesapi_client_metadata*/ None,
)
.await
@@ -8169,10 +8406,13 @@ async fn steer_input_requires_active_turn() {
#[tokio::test]
async fn steer_input_enforces_expected_turn_id() {
let (sess, tc, _rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(
Arc::clone(&tc),
input,
@@ -8192,6 +8432,7 @@ async fn steer_input_enforces_expected_turn_id() {
steer_input,
/*additional_context*/ Default::default(),
Some("different-turn-id"),
/*client_user_message_id*/ None,
/*responsesapi_client_metadata*/ None,
)
.await
@@ -8215,10 +8456,13 @@ async fn steer_input_rejects_non_regular_turns() {
(TaskKind::Compact, NonSteerableTurnKind::Compact),
] {
let (sess, _tc, _rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
let turn_context = sess.new_default_turn_with_sub_id("turn".to_string()).await;
sess.spawn_task(
turn_context,
@@ -8239,6 +8483,7 @@ async fn steer_input_rejects_non_regular_turns() {
steer_input,
/*additional_context*/ Default::default(),
/*expected_turn_id*/ None,
/*client_user_message_id*/ None,
/*responsesapi_client_metadata*/ None,
)
.await
@@ -8253,10 +8498,13 @@ async fn steer_input_rejects_non_regular_turns() {
#[tokio::test]
async fn steer_input_returns_active_turn_id() {
let (sess, tc, _rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(
Arc::clone(&tc),
input,
@@ -8276,6 +8524,7 @@ async fn steer_input_returns_active_turn_id() {
steer_input,
/*additional_context*/ Default::default(),
Some(&tc.sub_id),
/*client_user_message_id*/ None,
/*responsesapi_client_metadata*/ None,
)
.await
@@ -8285,66 +8534,11 @@ async fn steer_input_returns_active_turn_id() {
assert!(sess.input_queue.has_pending_input(&sess.active_turn).await);
}
#[tokio::test]
async fn queued_response_items_for_next_turn_move_into_next_active_turn() {
let (sess, tc, _rx) = make_session_and_context_with_rx().await;
let queued_item = ResponseInputItem::Message {
role: "assistant".to_string(),
content: vec![ContentItem::InputText {
text: "queued before wake".to_string(),
}],
phase: None,
};
sess.input_queue
.queue_response_items_for_next_turn(vec![queued_item.clone()])
.await;
sess.spawn_task(
Arc::clone(&tc),
Vec::new(),
NeverEndingTask {
kind: TaskKind::Regular,
listen_to_cancellation_token: false,
},
)
.await;
assert_eq!(
sess.input_queue.get_pending_input(&sess.active_turn).await,
vec![TurnInput::ResponseInputItem(queued_item)]
);
}
#[tokio::test]
async fn idle_interrupt_does_not_wake_queued_next_turn_items() {
let (sess, _tc, _rx) = make_session_and_context_with_rx().await;
let queued_item = ResponseInputItem::Message {
role: "assistant".to_string(),
content: vec![ContentItem::InputText {
text: "queued before interrupt".to_string(),
}],
phase: None,
};
sess.input_queue
.queue_response_items_for_next_turn(vec![queued_item])
.await;
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
assert!(sess.active_turn.lock().await.is_none());
assert!(
sess.input_queue
.has_queued_response_items_for_next_turn()
.await
);
}
#[tokio::test]
async fn abort_empty_active_turn_preserves_pending_input() {
let (sess, _tc, _rx) = make_session_and_context_with_rx().await;
let pending_item = ResponseInputItem::Message {
let pending_item = ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText {
text: "late pending input".to_string(),
@@ -8359,7 +8553,7 @@ async fn abort_empty_active_turn_preserves_pending_input() {
sess.input_queue
.extend_pending_input_for_turn_state(
turn_state.as_ref(),
vec![TurnInput::ResponseInputItem(pending_item.clone())],
vec![TurnInput::ResponseItem(pending_item.clone())],
)
.await;
@@ -8370,7 +8564,7 @@ async fn abort_empty_active_turn_preserves_pending_input() {
sess.input_queue
.take_pending_input_for_turn_state(turn_state.as_ref())
.await,
vec![TurnInput::ResponseInputItem(pending_item)]
vec![TurnInput::ResponseItem(pending_item)]
);
}
@@ -8796,7 +8990,7 @@ async fn budget_limited_accounting_steers_active_turn_without_aborting() -> anyh
.await?;
let pending_input = sess.input_queue.get_pending_input(&sess.active_turn).await;
let [TurnInput::ResponseInputItem(ResponseInputItem::Message { role, content, .. })] =
let [TurnInput::ResponseItem(ResponseItem::Message { role, content, .. })] =
pending_input.as_slice()
else {
panic!("expected one budget-limit steering message, got {pending_input:#?}");
@@ -9023,7 +9217,7 @@ async fn external_objective_change_steers_active_turn() -> anyhow::Result<()> {
pending_input.iter().any(|item| {
matches!(
item,
TurnInput::ResponseInputItem(ResponseInputItem::Message { role, content, .. })
TurnInput::ResponseItem(ResponseItem::Message { role, content, .. })
if role == "user"
&& content.iter().any(|content| matches!(
content,
@@ -9247,9 +9441,9 @@ async fn queue_only_mailbox_mail_waits_for_next_turn_after_answer_boundary() {
assert_eq!(
sess.input_queue.get_pending_input(&sess.active_turn).await,
vec![TurnInput::ResponseInputItem(
vec![TurnInput::ResponseItem(ResponseItem::from(
communication.to_response_input_item()
)],
))],
);
}
@@ -9322,6 +9516,7 @@ async fn steered_input_reopens_mailbox_delivery_for_current_turn() {
}],
/*additional_context*/ Default::default(),
Some(&tc.sub_id),
/*client_user_message_id*/ None,
/*responsesapi_client_metadata*/ None,
)
.await
@@ -9330,11 +9525,14 @@ async fn steered_input_reopens_mailbox_delivery_for_current_turn() {
assert_eq!(
sess.input_queue.get_pending_input(&sess.active_turn).await,
vec![
TurnInput::UserInput(vec![UserInput::Text {
text: "follow up".to_string(),
text_elements: Vec::new(),
}]),
TurnInput::ResponseInputItem(communication.to_response_input_item()),
TurnInput::UserInput {
content: vec![UserInput::Text {
text: "follow up".to_string(),
text_elements: Vec::new(),
}],
client_id: None
},
TurnInput::ResponseItem(ResponseItem::from(communication.to_response_input_item())),
],
);
}
@@ -9372,6 +9570,7 @@ async fn stale_defer_mailbox_delivery_does_not_override_steered_input() {
}],
/*additional_context*/ Default::default(),
Some(&tc.sub_id),
/*client_user_message_id*/ None,
/*responsesapi_client_metadata*/ None,
)
.await
@@ -9384,11 +9583,14 @@ async fn stale_defer_mailbox_delivery_does_not_override_steered_input() {
assert_eq!(
sess.input_queue.get_pending_input(&sess.active_turn).await,
vec![
TurnInput::UserInput(vec![UserInput::Text {
text: "follow up".to_string(),
text_elements: Vec::new(),
}]),
TurnInput::ResponseInputItem(communication.to_response_input_item()),
TurnInput::UserInput {
content: vec![UserInput::Text {
text: "follow up".to_string(),
text_elements: Vec::new(),
}],
client_id: None
},
TurnInput::ResponseItem(ResponseItem::from(communication.to_response_input_item())),
],
);
}
@@ -9443,19 +9645,22 @@ async fn tool_calls_reopen_mailbox_delivery_for_current_turn() {
assert!(output.tool_future.is_some());
assert_eq!(
sess.input_queue.get_pending_input(&sess.active_turn).await,
vec![TurnInput::ResponseInputItem(
vec![TurnInput::ResponseItem(ResponseItem::from(
communication.to_response_input_item()
)],
))],
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn abort_review_task_emits_exited_then_aborted_and_records_history() {
let (sess, tc, rx) = make_session_and_context_with_rx().await;
let input = vec![TurnInput::UserInput(vec![UserInput::Text {
text: "start review".to_string(),
text_elements: Vec::new(),
}])];
let input = vec![TurnInput::UserInput {
content: vec![UserInput::Text {
text: "start review".to_string(),
text_elements: Vec::new(),
}],
client_id: None,
}];
sess.spawn_task(Arc::clone(&tc), input, ReviewTask::new())
.await;
@@ -9503,8 +9708,7 @@ async fn abort_review_task_emits_exited_then_aborted_and_records_history() {
);
let history = sess.clone_history().await;
// The `<turn_aborted>` marker is silent in the event stream, so verify it is still
// recorded in history for the model.
// Verify the `<turn_aborted>` marker is still recorded in history for the model.
assert!(
history.raw_items().iter().any(|item| {
let ResponseItem::Message { role, content, .. } = item else {
@@ -10133,6 +10337,76 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
ExecApprovalRequirement::Skip { .. }
));
}
#[cfg(unix)]
#[tokio::test]
async fn shell_tool_cancellation_waits_for_runtime_cleanup() -> anyhow::Result<()> {
let session = make_session_with_config(|config| {
let cwd = config.cwd.clone();
config
.permissions
.set_legacy_sandbox_policy(SandboxPolicy::DangerFullAccess, cwd.as_path())
.expect("test setup should allow sandbox policy");
})
.await?;
let turn_context = session.new_default_turn().await;
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let temp_dir = tempfile::TempDir::new()?;
let ready_marker = temp_dir.path().join("ready");
let cleanup_marker = temp_dir.path().join("cleanup");
// Interrupt after the shell starts, then verify dispatch waits for its TERM cleanup trap.
let command = format!(
r#"trap 'printf cleaned > "{}"; exit 0' TERM
printf ready > "{}"
while :; do sleep 1; done"#,
cleanup_marker.display(),
ready_marker.display(),
);
let item = ResponseItem::FunctionCall {
id: None,
name: "shell_command".to_string(),
namespace: None,
arguments: serde_json::json!({
"command": command,
"timeout_ms": 60_000,
})
.to_string(),
call_id: "shell-cleanup-call".to_string(),
};
let call = ToolRouter::build_tool_call(item)?
.expect("shell command response item should build a tool call");
let cancellation_token = CancellationToken::new();
let cancellation_tx = cancellation_token.clone();
let handle = tokio::spawn(
test_tool_runtime(Arc::clone(&session), Arc::clone(&turn_context))
.handle_tool_call(call, cancellation_token),
);
let mut ready = false;
for _ in 0..50 {
if ready_marker.exists() {
ready = true;
break;
}
tokio::time::sleep(Duration::from_millis(20)).await;
}
if !ready {
cancellation_tx.cancel();
let _ = timeout(Duration::from_secs(5), handle).await;
anyhow::bail!("shell command should reach the ready marker");
}
cancellation_tx.cancel();
timeout(Duration::from_secs(5), handle)
.await
.expect("cancelled shell tool should finish promptly")
.expect("shell tool task should join")
.expect("cancelled shell tool should return a response item");
assert_eq!(std::fs::read_to_string(cleanup_marker)?, "cleaned");
Ok(())
}
#[tokio::test]
async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request() {
use crate::sandboxing::SandboxPermissions;
+39 -11
View File
@@ -145,7 +145,10 @@ pub(crate) async fn run_turn(
// diffs/full reinjection + user input) and trigger compaction preemptively
// when they would push the thread over the compaction threshold.
if let Err(err) = run_pre_sampling_compact(&sess, &turn_context, &mut client_session).await {
if err.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
let error = err.to_codex_protocol_error();
sess.emit_turn_error_lifecycle(turn_context.as_ref(), error.clone())
.await;
if error == CodexErrorInfo::UsageLimitExceeded
&& let Err(err) = sess
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
turn_context: turn_context.as_ref(),
@@ -234,7 +237,10 @@ pub(crate) async fn run_turn(
.for_prompt(&turn_context.model_info.input_modalities)
};
let turn_metadata_header = turn_context.turn_metadata_state.current_header_value();
let window_id = sess.services.model_client.current_window_id();
let turn_metadata_header = turn_context
.turn_metadata_state
.current_header_value_for_model_request(&window_id);
match run_sampling_request(
Arc::clone(&sess),
Arc::clone(&turn_context),
@@ -292,7 +298,10 @@ pub(crate) async fn run_turn(
)
.await
{
if err.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
let error = err.to_codex_protocol_error();
sess.emit_turn_error_lifecycle(turn_context.as_ref(), error.clone())
.await;
if error == CodexErrorInfo::UsageLimitExceeded
&& let Err(err) = sess
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
turn_context: turn_context.as_ref(),
@@ -371,17 +380,23 @@ pub(crate) async fn run_turn(
}
}
let error = CodexErrorInfo::BadRequest;
sess.emit_turn_error_lifecycle(turn_context.as_ref(), error.clone())
.await;
let event = EventMsg::Error(ErrorEvent {
message: "Invalid image in your last message. Please remove it and try again."
.to_string(),
codex_error_info: Some(CodexErrorInfo::BadRequest),
codex_error_info: Some(error),
});
sess.send_event(&turn_context, event).await;
break;
}
Err(e) => {
info!("Turn error: {e:#}");
if e.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
let error = e.to_codex_protocol_error();
sess.emit_turn_error_lifecycle(turn_context.as_ref(), error.clone())
.await;
if error == CodexErrorInfo::UsageLimitExceeded
&& let Err(err) = sess
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
turn_context: turn_context.as_ref(),
@@ -414,7 +429,7 @@ async fn run_hooks_and_record_inputs(
blocked_input = true;
record_additional_contexts(sess, turn_context, hook_outcome.additional_contexts).await;
} else {
if matches!(input_item, TurnInput::UserInput(items) if !items.is_empty()) {
if matches!(input_item, TurnInput::UserInput { content, .. } if !content.is_empty()) {
accepted_user_input = true;
}
record_pending_input(
@@ -442,8 +457,8 @@ async fn build_skills_and_plugins(
let user_input = input
.iter()
.filter_map(|item| match item {
TurnInput::UserInput(content) => Some(content.as_slice()),
TurnInput::ResponseInputItem(_) => None,
TurnInput::UserInput { content, .. } => Some(content.as_slice()),
TurnInput::ResponseItem(_) => None,
})
.flatten()
.cloned()
@@ -595,8 +610,8 @@ async fn track_turn_resolved_config_analytics(
num_input_images: input
.iter()
.filter_map(|item| match item {
TurnInput::UserInput(content) => Some(content.as_slice()),
TurnInput::ResponseInputItem(_) => None,
TurnInput::UserInput { content, .. } => Some(content.as_slice()),
TurnInput::ResponseItem(_) => None,
})
.flatten()
.filter(|item| {
@@ -1006,12 +1021,25 @@ async fn run_sampling_request(
clippy::await_holding_invalid_type,
reason = "tool router construction reads through the session-owned manager guard"
)]
#[instrument(level = "trace",
skip_all,
fields(
turn_id = %turn_context.sub_id,
model = %turn_context.model_info.slug,
apps_enabled = turn_context.apps_enabled()
)
)]
pub(crate) async fn built_tools(
sess: &Session,
turn_context: &TurnContext,
cancellation_token: &CancellationToken,
) -> CodexResult<Arc<ToolRouter>> {
let mcp_connection_manager = sess.services.mcp_connection_manager.read().await;
let mcp_connection_manager = sess
.services
.mcp_connection_manager
.read()
.instrument(trace_span!("read_mcp_connection_manager"))
.await;
let has_mcp_servers = mcp_connection_manager.has_servers();
let all_mcp_tools = mcp_connection_manager
.list_all_tools()
@@ -322,10 +322,12 @@ impl TurnContext {
}
pub(crate) fn to_turn_context_item(&self) -> TurnContextItem {
let workspace_roots = self.config.effective_workspace_roots();
TurnContextItem {
turn_id: Some(self.sub_id.clone()),
#[allow(deprecated)]
cwd: self.cwd.to_path_buf(),
workspace_roots: (!workspace_roots.is_empty()).then_some(workspace_roots),
current_date: self.current_date.clone(),
timezone: self.timezone.clone(),
approval_policy: self.approval_policy.value(),
+2 -1
View File
@@ -256,9 +256,10 @@ async fn schedule_startup_prewarm_inner(
build_prompt_started_at.elapsed(),
/*status*/ None,
);
let window_id = session.services.model_client.current_window_id();
let startup_turn_metadata_header = startup_turn_context
.turn_metadata_state
.current_header_value();
.current_header_value_for_prewarm(&window_id);
let mut client_session = session.services.model_client.new_session();
let websocket_warmup_started_at = Instant::now();
client_session
+36
View File
@@ -1,4 +1,5 @@
use codex_extension_api::ExtensionData;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::TokenUsage;
use codex_protocol::protocol::TurnAbortReason;
@@ -37,6 +38,23 @@ impl Session {
}
}
pub(crate) async fn emit_thread_idle_lifecycle_if_idle(&self) {
if self.active_turn.lock().await.is_some()
|| self.input_queue.has_trigger_turn_mailbox_items().await
{
return;
}
for contributor in self.services.extensions.thread_lifecycle_contributors() {
contributor
.on_thread_idle(codex_extension_api::ThreadIdleInput {
session_store: &self.services.session_extension_data,
thread_store: &self.services.thread_extension_data,
})
.await;
}
}
pub(super) async fn emit_turn_abort_lifecycle(
&self,
reason: TurnAbortReason,
@@ -53,4 +71,22 @@ impl Session {
.await;
}
}
pub(crate) async fn emit_turn_error_lifecycle(
&self,
turn_context: &TurnContext,
error: CodexErrorInfo,
) {
for contributor in self.services.extensions.turn_lifecycle_contributors() {
contributor
.on_turn_error(codex_extension_api::TurnErrorInput {
turn_id: turn_context.sub_id.as_str(),
error: error.clone(),
session_store: &self.services.session_extension_data,
thread_store: &self.services.thread_extension_data,
turn_store: turn_context.extension_data.as_ref(),
})
.await;
}
}
}
+11 -25
View File
@@ -44,7 +44,6 @@ use codex_otel::TURN_TOKEN_USAGE_METRIC;
use codex_otel::TURN_TOOL_CALL_METRIC;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::TokenUsage;
use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::TurnAbortedEvent;
@@ -347,11 +346,7 @@ impl Session {
{
warn!("failed to apply goal runtime turn-start event: {err}");
}
let queued_response_items = self
.input_queue
.take_queued_response_items_for_next_turn()
.await;
let mailbox_items = self.input_queue.get_pending_input(&self.active_turn).await;
let pending_items = self.input_queue.get_pending_input(&self.active_turn).await;
let turn_state = {
let mut active = self.active_turn.lock().await;
let turn = active.get_or_insert_with(ActiveTurn::default);
@@ -359,11 +354,6 @@ impl Session {
Arc::clone(&turn.turn_state)
};
turn_state.lock().await.token_usage_at_turn_start = token_usage_at_turn_start.clone();
let mut pending_items = queued_response_items
.into_iter()
.map(TurnInput::ResponseInputItem)
.collect::<Vec<_>>();
pending_items.extend(mailbox_items);
self.input_queue
.extend_pending_input_for_turn_state(turn_state.as_ref(), pending_items)
.await;
@@ -452,8 +442,7 @@ impl Session {
/// Starts a regular turn when the session is idle and pending work is waiting.
///
/// Pending work currently includes queued next-turn items and mailbox mail marked with
/// `trigger_turn`.
/// Pending work currently includes mailbox mail marked with `trigger_turn`.
///
/// This helper generates a fresh sub-id for the synthetic turn before delegating to the
/// explicit-sub-id variant.
@@ -465,18 +454,13 @@ impl Session {
/// Starts a regular turn with the provided sub-id when pending work should wake an idle
/// session.
///
/// The turn is created only when there are queued next-turn items or mailbox mail marked with
/// `trigger_turn`, and only if the session is currently idle.
/// The turn is created only when there is mailbox mail marked with `trigger_turn`, and only
/// if the session is currently idle.
pub(crate) async fn maybe_start_turn_for_pending_work_with_sub_id(
self: &Arc<Self>,
sub_id: String,
) {
if !self
.input_queue
.has_queued_response_items_for_next_turn()
.await
&& !self.input_queue.has_trigger_turn_mailbox_items().await
{
if !self.input_queue.has_trigger_turn_mailbox_items().await {
return;
}
@@ -809,6 +793,7 @@ impl Session {
{
warn!("failed to apply goal runtime maybe-continue event: {err}");
}
self.emit_thread_idle_lifecycle_if_idle().await;
}
async fn take_active_turn(&self) -> Option<ActiveTurn> {
@@ -859,10 +844,11 @@ impl Session {
InterruptedTurnHistoryMarker::from_config(task.turn_context.config.as_ref()),
)
{
self.record_into_history(std::slice::from_ref(&marker), task.turn_context.as_ref())
.await;
self.persist_rollout_items(&[RolloutItem::ResponseItem(marker)])
.await;
self.record_conversation_items(
task.turn_context.as_ref(),
std::slice::from_ref(&marker),
)
.await;
// Ensure the marker is durably visible before emitting TurnAborted: some clients
// synchronously re-read the rollout on receipt of the abort event.
if let Err(err) = self.flush_rollout().await {
+2 -2
View File
@@ -72,8 +72,8 @@ impl SessionTask for ReviewTask {
let mut user_input = Vec::new();
for item in input {
match item {
TurnInput::UserInput(mut content) => user_input.append(&mut content),
TurnInput::ResponseInputItem(_) => {}
TurnInput::UserInput { mut content, .. } => user_input.append(&mut content),
TurnInput::ResponseItem(_) => {}
}
}
+3 -28
View File
@@ -38,8 +38,6 @@ use super::SessionTask;
use super::SessionTaskContext;
use crate::session::session::Session;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
const USER_SHELL_TIMEOUT_MS: u64 = 60 * 60 * 1000; // 1 hour
@@ -363,30 +361,7 @@ async fn persist_user_shell_output(
return;
}
let response_input_item = match output_item {
ResponseItem::Message {
role,
content,
phase,
..
} => ResponseInputItem::Message {
role,
content,
phase,
},
_ => unreachable!("user shell command output record should always be a message"),
};
if let Err(items) = session
.inject_response_items(vec![response_input_item])
.await
{
let response_items = items
.into_iter()
.map(ResponseItem::from)
.collect::<Vec<_>>();
session
.record_conversation_items(turn_context, &response_items)
.await;
}
session
.inject_no_new_turn(vec![output_item], Some(turn_context))
.await;
}
+1 -1
View File
@@ -30,7 +30,7 @@ use crate::unified_exec;
static TEST_MODEL_PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
let mut response = bundled_models_response()
.unwrap_or_else(|err| panic!("bundled models.json should parse: {err}"));
response.models.sort_by(|a, b| a.priority.cmp(&b.priority));
response.models.sort_by_key(|model| model.priority);
let mut presets: Vec<ModelPreset> = response.models.into_iter().map(Into::into).collect();
ModelPreset::mark_default_by_picker_visibility(&mut presets);
presets
-13
View File
@@ -33,8 +33,6 @@ use codex_protocol::ThreadId;
use codex_protocol::config_types::CollaborationModeMask;
use codex_protocol::error::CodexErr;
use codex_protocol::error::Result as CodexResult;
#[cfg(test)]
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
@@ -1016,17 +1014,6 @@ impl ThreadManagerState {
thread.submit(op).await
}
#[cfg(test)]
/// Append a prebuilt message to a thread by ID outside the normal user-input path.
pub(crate) async fn append_message(
&self,
thread_id: ThreadId,
message: ResponseItem,
) -> CodexResult<String> {
let thread = self.get_thread(thread_id).await?;
thread.append_message(message).await
}
/// Remove a thread from the manager by ID, returning it when present.
pub(crate) async fn remove_thread(&self, thread_id: &ThreadId) -> Option<Arc<CodexThread>> {
self.threads.write().await.remove(thread_id)
@@ -1124,6 +1124,7 @@ fn multi_agent_v2_interrupted_marker_uses_developer_input_message() {
fn completed_legacy_event_history_is_not_mid_turn() {
let completed_history = InitialHistory::Forked(vec![
RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "hello".to_string(),
images: None,
text_elements: Vec::new(),
@@ -1152,6 +1153,7 @@ fn mixed_response_and_legacy_user_event_history_is_mid_turn() {
let mixed_history = InitialHistory::Forked(vec![
RolloutItem::ResponseItem(user_msg("hello")),
RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "hello".to_string(),
images: None,
text_elements: Vec::new(),
+10 -6
View File
@@ -130,7 +130,8 @@ pub(crate) fn truncate_rollout_before_nth_user_message_from_start(
/// Return a suffix of `items` that keeps the last `n_from_end` fork turns.
///
/// If fewer than or equal to `n_from_end` fork turns exist, this returns the full rollout.
/// If fewer than or equal to `n_from_end` fork turns exist, this keeps from the first fork-turn
/// boundary and still drops pre-turn startup context.
pub(crate) fn truncate_rollout_to_last_n_fork_turns(
items: &[RolloutItem],
n_from_end: usize,
@@ -140,11 +141,14 @@ pub(crate) fn truncate_rollout_to_last_n_fork_turns(
}
let fork_turn_positions = fork_turn_positions_in_rollout(items);
if fork_turn_positions.len() <= n_from_end {
return items.to_vec();
}
let keep_idx = fork_turn_positions[fork_turn_positions.len() - n_from_end];
let Some(keep_idx) = fork_turn_positions
.len()
.checked_sub(n_from_end)
.map(|position| fork_turn_positions[position])
.or_else(|| fork_turn_positions.first().copied())
else {
return Vec::new();
};
items[keep_idx..].to_vec()
}
@@ -29,6 +29,17 @@ fn assistant_msg(text: &str) -> ResponseItem {
}
}
fn developer_msg(text: &str) -> ResponseItem {
ResponseItem::Message {
id: None,
role: "developer".to_string(),
content: vec![ContentItem::InputText {
text: text.to_string(),
}],
phase: None,
}
}
fn inter_agent_msg(text: &str, trigger_turn: bool) -> ResponseItem {
let communication = InterAgentCommunication::new(
AgentPath::root(),
@@ -197,6 +208,23 @@ fn truncates_rollout_to_last_n_fork_turns_counts_trigger_turn_messages() {
);
}
#[test]
fn truncates_rollout_to_last_n_fork_turns_drops_startup_prefix_even_when_under_limit() {
let rollout = vec![
RolloutItem::ResponseItem(developer_msg("startup developer context")),
RolloutItem::ResponseItem(user_msg("current task")),
RolloutItem::ResponseItem(assistant_msg("answer")),
];
let truncated = truncate_rollout_to_last_n_fork_turns(&rollout, /*n_from_end*/ 2);
let expected = rollout[1..].to_vec();
assert_eq!(
serde_json::to_value(&truncated).unwrap(),
serde_json::to_value(&expected).unwrap()
);
}
#[test]
fn truncates_rollout_to_last_n_fork_turns_applies_thread_rollback_markers() {
let rollout = vec![
+2 -2
View File
@@ -13,7 +13,7 @@ use codex_code_mode::CodeModeTurnHost;
use codex_code_mode::RuntimeResponse;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use serde_json::Value as JsonValue;
use tokio_util::sync::CancellationToken;
@@ -134,7 +134,7 @@ impl CodeModeTurnHost for CoreTurnHost {
}
self.exec
.session
.inject_response_items(vec![ResponseInputItem::CustomToolCallOutput {
.inject_if_running(vec![ResponseItem::CustomToolCallOutput {
call_id,
name: Some(PUBLIC_TOOL_NAME.to_string()),
output: FunctionCallOutputPayload::from_text(text),
@@ -17,6 +17,8 @@ impl IntoProtocol<ImageDetail> for CodeModeImageDetail {
fn into_protocol(self) -> ImageDetail {
let value = self;
match value {
CodeModeImageDetail::Auto => ImageDetail::Auto,
CodeModeImageDetail::Low => ImageDetail::Low,
CodeModeImageDetail::High => ImageDetail::High,
CodeModeImageDetail::Original => ImageDetail::Original,
}
@@ -1,11 +1,18 @@
use std::sync::Arc;
use std::sync::Weak;
use codex_protocol::items::TurnItem;
use codex_tools::ConversationHistory;
use codex_tools::ExtensionTurnItem;
use codex_tools::ToolCall as ExtensionToolCall;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use codex_tools::TurnItemEmissionFuture;
use codex_tools::TurnItemEmitter;
use crate::function_tool::FunctionCallError;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
@@ -52,6 +59,39 @@ impl CoreToolRuntime for ExtensionToolAdapter {
}
}
struct CoreTurnItemEmitter {
session: Weak<Session>,
turn: Weak<TurnContext>,
}
fn extension_turn_item(item: ExtensionTurnItem) -> TurnItem {
match item {
ExtensionTurnItem::WebSearch(item) => TurnItem::WebSearch(item),
}
}
impl TurnItemEmitter for CoreTurnItemEmitter {
fn emit_started<'a>(&'a self, item: ExtensionTurnItem) -> TurnItemEmissionFuture<'a> {
Box::pin(async move {
let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else {
return;
};
let item = extension_turn_item(item);
session.emit_turn_item_started(turn.as_ref(), &item).await;
})
}
fn emit_completed<'a>(&'a self, item: ExtensionTurnItem) -> TurnItemEmissionFuture<'a> {
Box::pin(async move {
let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else {
return;
};
let item = extension_turn_item(item);
session.emit_turn_item_completed(turn.as_ref(), item).await;
})
}
}
async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
let conversation_history =
ConversationHistory::new(invocation.session.clone_history().await.into_raw_items());
@@ -61,6 +101,10 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
tool_name: invocation.tool_name.clone(),
truncation_policy: invocation.turn.truncation_policy,
conversation_history,
turn_item_emitter: Arc::new(CoreTurnItemEmitter {
session: Arc::downgrade(&invocation.session),
turn: Arc::downgrade(&invocation.turn),
}),
payload: invocation.payload.clone(),
}
}
@@ -69,8 +113,13 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
mod tests {
use std::sync::Arc;
use codex_protocol::items::TurnItem;
use codex_protocol::items::WebSearchItem;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::models::WebSearchAction;
use codex_protocol::protocol::EventMsg;
use codex_tools::ExtensionTurnItem;
use pretty_assertions::assert_eq;
use serde_json::json;
use tokio::sync::Mutex;
@@ -147,6 +196,16 @@ mod tests {
&self,
call: codex_tools::ToolCall,
) -> Result<Box<dyn codex_tools::ToolOutput>, codex_tools::FunctionCallError> {
let item = ExtensionTurnItem::WebSearch(WebSearchItem {
id: call.call_id.clone(),
query: "rust trait object".to_string(),
action: WebSearchAction::Search {
query: Some("rust trait object".to_string()),
queries: None,
},
});
call.turn_item_emitter.emit_started(item.clone()).await;
call.turn_item_emitter.emit_completed(item).await;
*self.captured_call.lock().await = Some(call);
Ok(Box::new(codex_tools::JsonToolOutput::new(
json!({ "ok": true }),
@@ -191,12 +250,14 @@ mod tests {
}
#[tokio::test]
async fn passes_turn_fields_to_extension_call() {
async fn passes_turn_fields_and_scoped_turn_item_emitter_to_extension_call() {
let captured_call = Arc::new(Mutex::new(None));
let handler = ExtensionToolAdapter::new(Arc::new(CapturingExtensionExecutor {
captured_call: Arc::clone(&captured_call),
}));
let (session, turn) = crate::session::tests::make_session_and_context().await;
let (session, turn, rx) = crate::session::tests::make_session_and_context_with_rx().await;
let weak_session = Arc::downgrade(&session);
let weak_turn = Arc::downgrade(&turn);
let turn_id = turn.sub_id.clone();
let truncation_policy = turn.truncation_policy;
let history_item = ResponseItem::Message {
@@ -208,11 +269,16 @@ mod tests {
phase: None,
};
session
.record_into_history(std::slice::from_ref(&history_item), &turn)
.record_conversation_items(&turn, std::slice::from_ref(&history_item))
.await;
let raw_history_event = rx.recv().await.expect("history raw response item event");
let EventMsg::RawResponseItem(raw_history_item) = raw_history_event.msg else {
panic!("expected raw response item event");
};
assert_eq!(raw_history_item.item, history_item);
let invocation = ToolInvocation {
session: session.into(),
turn: turn.into(),
session,
turn,
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())),
call_id: "call-extension".to_string(),
@@ -228,6 +294,8 @@ mod tests {
.expect("extension call should succeed");
let captured_call = captured_call.lock().await.clone().expect("captured call");
assert!(weak_session.upgrade().is_none());
assert!(weak_turn.upgrade().is_none());
assert_eq!(captured_call.turn_id, turn_id);
assert_eq!(captured_call.call_id, "call-extension");
assert_eq!(
@@ -245,5 +313,43 @@ mod tests {
}
payload => panic!("expected function payload, got {payload:?}"),
}
let started = rx.recv().await.expect("item started event");
let EventMsg::ItemStarted(started) = started.msg else {
panic!("expected item started event");
};
let TurnItem::WebSearch(started_item) = started.item else {
panic!("expected web search item");
};
let begin = rx.recv().await.expect("legacy web search begin event");
let EventMsg::WebSearchBegin(begin) = begin.msg else {
panic!("expected legacy web search begin event");
};
let completed = rx.recv().await.expect("item completed event");
let EventMsg::ItemCompleted(completed) = completed.msg else {
panic!("expected item completed event");
};
let TurnItem::WebSearch(completed_item) = completed.item else {
panic!("expected web search item");
};
let end = rx.recv().await.expect("legacy web search end event");
let EventMsg::WebSearchEnd(end) = end.msg else {
panic!("expected legacy web search end event");
};
let expected = WebSearchItem {
id: "call-extension".to_string(),
query: "rust trait object".to_string(),
action: WebSearchAction::Search {
query: Some("rust trait object".to_string()),
queries: None,
},
};
assert_eq!(started_item, expected);
assert_eq!(completed_item, expected);
assert_eq!(begin.call_id, expected.id);
assert_eq!(end.call_id, expected.id);
assert_eq!(end.query, expected.query);
assert_eq!(end.action, expected.action);
}
}
+5 -11
View File
@@ -522,19 +522,13 @@ mod tests {
callable_name: tool_name.to_string(),
callable_namespace: callable_namespace.to_string(),
namespace_description: None,
tool: rmcp::model::Tool {
name: tool_name.to_string().into(),
title: None,
description: None,
input_schema: Arc::new(rmcp::model::object(serde_json::json!({
tool: rmcp::model::Tool::new_with_raw(
tool_name.to_string(),
None,
Arc::new(rmcp::model::object(serde_json::json!({
"type": "object",
}))),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
),
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
@@ -82,10 +82,9 @@ impl ToolExecutor<ToolInvocation> for ListMcpResourceTemplatesHandler {
let payload_result: Result<ListResourceTemplatesPayload, FunctionCallError> = async {
if let Some(server_name) = server.clone() {
let params = cursor.clone().map(|value| PaginatedRequestParams {
meta: None,
cursor: Some(value),
});
let params = cursor
.clone()
.map(|value| PaginatedRequestParams::default().with_cursor(Some(value)));
let result = session
.list_resource_templates(&server_name, params)
.await
@@ -82,10 +82,9 @@ impl ToolExecutor<ToolInvocation> for ListMcpResourcesHandler {
let payload_result: Result<ListResourcesPayload, FunctionCallError> = async {
if let Some(server_name) = server.clone() {
let params = cursor.clone().map(|value| PaginatedRequestParams {
meta: None,
cursor: Some(value),
});
let params = cursor
.clone()
.map(|value| PaginatedRequestParams::default().with_cursor(Some(value)));
let result = session
.list_resources(&server_name, params)
.await
@@ -78,13 +78,7 @@ impl ToolExecutor<ToolInvocation> for ReadMcpResourceHandler {
let payload_result: Result<ReadResourcePayload, FunctionCallError> = async {
let result = session
.read_resource(
&server,
ReadResourceRequestParams {
meta: None,
uri: uri.clone(),
},
)
.read_resource(&server, ReadResourceRequestParams::new(uri.clone()))
.await
.map_err(|err| {
FunctionCallError::RespondToModel(format!("resources/read failed: {err:#}"))
@@ -50,11 +50,10 @@ fn tool_info() -> ToolInfo {
callable_name: "_create_event".to_string(),
callable_namespace: "mcp__calendar__".to_string(),
namespace_description: Some("Plan events.".to_string()),
tool: rmcp::model::Tool {
name: "createEvent".to_string().into(),
title: Some("Create event".to_string()),
description: Some("Create a calendar event.".to_string().into()),
input_schema: Arc::new(rmcp::model::object(json!({
tool: rmcp::model::Tool::new(
"createEvent",
"Create a calendar event.",
Arc::new(rmcp::model::object(json!({
"type": "object",
"properties": {
"start_time": { "type": "string" },
@@ -62,12 +61,8 @@ fn tool_info() -> ToolInfo {
},
"additionalProperties": false
}))),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
)
.with_title("Create event"),
connector_id: None,
connector_name: Some("Calendar".to_string()),
plugin_display_names: vec![" Calendar plugin ".to_string(), " ".to_string()],
@@ -1,6 +1,7 @@
use super::*;
use crate::tools::handlers::multi_agents_spec::create_close_agent_tool_v1;
use crate::turn_timing::now_unix_timestamp_ms;
use codex_protocol::error::CodexErr;
use codex_tools::ToolSpec;
pub(crate) struct Handler;
@@ -36,11 +37,9 @@ async fn handle_close_agent(
let arguments = function_arguments(payload)?;
let args: CloseAgentArgs = parse_arguments(&arguments)?;
let agent_id = parse_agent_id_target(&args.target)?;
let receiver_agent = session
.services
.agent_control
.get_agent_metadata(agent_id)
.unwrap_or_default();
let receiver_agent = session.services.agent_control.get_agent_metadata(agent_id);
let known_agent = receiver_agent.is_some();
let receiver_agent = receiver_agent.unwrap_or_default();
session
.send_event(
&turn,
@@ -60,6 +59,9 @@ async fn handle_close_agent(
.await
{
Ok(mut status_rx) => status_rx.borrow_and_update().clone(),
Err(CodexErr::ThreadNotFound(_)) if known_agent => {
session.services.agent_control.get_status(agent_id).await
}
Err(err) => {
let status = session.services.agent_control.get_status(agent_id).await;
session
@@ -105,7 +105,7 @@ pub(crate) fn build_wait_agent_statuses(
status: status.clone(),
})
.collect::<Vec<_>>();
extras.sort_by(|left, right| left.thread_id.to_string().cmp(&right.thread_id.to_string()));
extras.sort_by_key(|entry| entry.thread_id.to_string());
entries.extend(extras);
entries
}
@@ -52,6 +52,7 @@ use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::TurnAbortedEvent;
use codex_protocol::protocol::TurnCompleteEvent;
use codex_protocol::user_input::UserInput;
use codex_state::DirectionalThreadSpawnEdgeStatus;
use core_test_support::TempDirExt;
use pretty_assertions::assert_eq;
use serde::Deserialize;
@@ -3751,6 +3752,126 @@ async fn multi_agent_v2_close_agent_accepts_task_name_target() {
);
}
#[tokio::test]
async fn multi_agent_v2_close_agent_reaps_stale_task_name_target() {
let (mut session, mut turn) = make_session_and_context().await;
let mut config = (*turn.config).clone();
config.agent_max_threads = Some(1);
config
.features
.enable(Feature::MultiAgentV2)
.expect("test config should allow feature update");
config
.features
.enable(Feature::Sqlite)
.expect("test config should allow sqlite");
let state_db = init_state_db(&config)
.await
.expect("sqlite state db should initialize");
let manager = ThreadManager::with_models_provider_home_and_state_for_tests(
CodexAuth::from_api_key("dummy"),
config.model_provider.clone(),
config.codex_home.to_path_buf(),
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
Some(state_db.clone()),
);
let root = manager
.start_thread(config.clone())
.await
.expect("root thread should start");
session.services.agent_control = manager.agent_control();
session.conversation_id = root.thread_id;
turn.config = Arc::new(config.clone());
let session = Arc::new(session);
let turn = Arc::new(turn);
SpawnAgentHandlerV2::default()
.handle(invocation(
session.clone(),
turn.clone(),
"spawn_agent",
function_payload(json!({
"message": "inspect this repo",
"task_name": "worker"
})),
))
.await
.expect("spawn_agent should succeed");
let agent_id = session
.services
.agent_control
.resolve_agent_reference(session.conversation_id, &turn.session_source, "worker")
.await
.expect("worker path should resolve");
let stale_thread = manager
.remove_thread(&agent_id)
.await
.expect("worker thread should be loaded before removal");
stale_thread
.submit(Op::Shutdown {})
.await
.expect("removed worker thread should still accept shutdown");
stale_thread.wait_until_terminated().await;
let output = CloseAgentHandlerV2
.handle(invocation(
session.clone(),
turn.clone(),
"close_agent",
function_payload(json!({"target": "worker"})),
))
.await
.expect("close_agent should reap stale v2 task names");
let (content, success) = expect_text_output(output);
let result: close_agent::CloseAgentResult =
serde_json::from_str(&content).expect("close_agent result should be json");
assert_eq!(result.previous_status, AgentStatus::NotFound);
assert_eq!(success, Some(true));
let open_children = state_db
.list_thread_spawn_children_with_status(
root.thread_id,
DirectionalThreadSpawnEdgeStatus::Open,
)
.await
.expect("open children should load");
assert_eq!(open_children, Vec::<ThreadId>::new());
let closed_children = state_db
.list_thread_spawn_children_with_status(
root.thread_id,
DirectionalThreadSpawnEdgeStatus::Closed,
)
.await
.expect("closed children should load");
assert_eq!(closed_children, vec![agent_id]);
SpawnAgentHandlerV2::default()
.handle(invocation(
session.clone(),
turn.clone(),
"spawn_agent",
function_payload(json!({
"message": "inspect this repo again",
"task_name": "replacement"
})),
))
.await
.expect("spawn_agent should succeed after stale close releases the slot");
let replacement_id = session
.services
.agent_control
.resolve_agent_reference(session.conversation_id, &turn.session_source, "replacement")
.await
.expect("replacement path should resolve");
let _ = session
.services
.agent_control
.shutdown_live_agent(replacement_id)
.await
.expect("replacement should shut down");
}
#[tokio::test]
async fn multi_agent_v2_close_agent_rejects_root_target_and_id() {
let (mut session, mut turn) = make_session_and_context().await;
@@ -1,6 +1,7 @@
use super::*;
use crate::tools::handlers::multi_agents_spec::create_close_agent_tool_v2;
use crate::turn_timing::now_unix_timestamp_ms;
use codex_protocol::error::CodexErr;
use codex_tools::ToolSpec;
pub(crate) struct Handler;
@@ -36,11 +37,9 @@ async fn handle_close_agent(
let arguments = function_arguments(payload)?;
let args: CloseAgentArgs = parse_arguments(&arguments)?;
let agent_id = resolve_agent_target(&session, &turn, &args.target).await?;
let receiver_agent = session
.services
.agent_control
.get_agent_metadata(agent_id)
.unwrap_or_default();
let receiver_agent = session.services.agent_control.get_agent_metadata(agent_id);
let known_agent = receiver_agent.is_some();
let receiver_agent = receiver_agent.unwrap_or_default();
if receiver_agent
.agent_path
.as_ref()
@@ -69,6 +68,9 @@ async fn handle_close_agent(
.await
{
Ok(mut status_rx) => status_rx.borrow_and_update().clone(),
Err(CodexErr::ThreadNotFound(_)) if known_agent => {
session.services.agent_control.get_status(agent_id).await
}
Err(err) => {
let status = session.services.agent_control.get_status(agent_id).await;
session
@@ -2,6 +2,7 @@ use codex_features::Feature;
use codex_protocol::models::ShellCommandToolCallParams;
use serde_json::Value as JsonValue;
use std::sync::Arc;
use tokio_util::sync::CancellationToken;
use crate::exec::ExecParams;
use crate::exec_policy::ExecApprovalRequest;
@@ -44,6 +45,7 @@ fn shell_command_payload_command(payload: &ToolPayload) -> Option<String> {
struct RunExecLikeArgs {
tool_name: ToolName,
exec_params: ExecParams,
cancellation_token: CancellationToken,
hook_command: String,
shell_type: Option<ShellType>,
additional_permissions: Option<AdditionalPermissionProfile>,
@@ -59,6 +61,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
let RunExecLikeArgs {
tool_name,
exec_params,
cancellation_token,
hook_command,
shell_type,
additional_permissions,
@@ -183,6 +186,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
hook_command,
cwd: exec_params.cwd.clone(),
timeout_ms: exec_params.expiration.timeout_ms(),
cancellation_token,
env: exec_params.env.clone(),
explicit_env_overrides,
network: exec_params.network.clone(),
@@ -151,6 +151,7 @@ impl ToolExecutor<ToolInvocation> for ShellCommandHandler {
let ToolInvocation {
session,
turn,
cancellation_token,
tracker,
call_id,
payload,
@@ -188,6 +189,7 @@ impl ToolExecutor<ToolInvocation> for ShellCommandHandler {
run_exec_like(RunExecLikeArgs {
tool_name,
exec_params,
cancellation_token,
hook_command: params.command,
shell_type,
additional_permissions: params.additional_permissions.clone(),
@@ -208,6 +210,10 @@ impl CoreToolRuntime for ShellCommandHandler {
matches!(payload, ToolPayload::Function { .. })
}
fn waits_for_runtime_cancellation(&self) -> bool {
true
}
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
shell_command_payload_command(&invocation.payload).map(|command| PreToolUsePayload {
tool_name: HookToolName::bash(),
@@ -258,21 +258,15 @@ mod tests {
callable_name: tool_name.to_string(),
callable_namespace: format!("mcp__{server_name}"),
namespace_description: None,
tool: Tool {
name: tool_name.to_string().into(),
title: None,
description: Some(format!("{description_prefix} desktop tool").into()),
input_schema: Arc::new(rmcp::model::object(serde_json::json!({
tool: Tool::new(
tool_name.to_string(),
format!("{description_prefix} desktop tool"),
Arc::new(rmcp::model::object(serde_json::json!({
"type": "object",
"properties": {},
"additionalProperties": false,
}))),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
),
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
@@ -40,7 +40,7 @@ pub fn create_view_image_tool(options: ViewImageToolOptions) -> ToolSpec {
ToolSpec::Function(ResponsesApiTool {
name: VIEW_IMAGE_TOOL_NAME.to_string(),
description: "View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within <image ...> tags)."
description: "View a local image file from the filesystem when visual inspection is needed. Use this for images already available on disk."
.to_string(),
strict: false,
defer_loading: None,
+178 -15
View File
@@ -92,6 +92,7 @@ impl ToolCallRuntime {
let tracker = Arc::clone(&self.tracker);
let lock = Arc::clone(&self.parallel_execution);
let invocation_cancellation_token = cancellation_token.clone();
let wait_for_runtime_cancellation = self.router.tool_waits_for_runtime_cancellation(&call);
let started = Instant::now();
let abort_session = Arc::clone(&session);
let abort_source = source.clone();
@@ -140,23 +141,35 @@ impl ToolCallRuntime {
} else {
let secs = started.elapsed().as_secs_f32().max(0.1);
abort_dispatch_span.record("aborted", true);
handle.abort();
match handle.await {
Ok(result) => result,
Err(err) if err.is_cancelled() => {
let response = Self::aborted_response(&call, secs);
notify_tool_aborted(
abort_session.as_ref(),
abort_turn.as_ref(),
call.call_id.as_str(),
&call.tool_name,
abort_source,
)
.await;
Ok(response)
if wait_for_runtime_cancellation {
if terminal_outcome_reached.swap(true, Ordering::AcqRel) {
return handle.await.map_err(Self::tool_task_join_error)?;
}
// The abort owns the terminal outcome; await only so
// the runtime can finish process teardown.
match handle.await {
Ok(_) => {}
Err(err) if err.is_cancelled() => {}
Err(err) => return Err(Self::tool_task_join_error(err)),
}
} else {
handle.abort();
match handle.await {
Ok(result) => return result,
Err(err) if err.is_cancelled() => {}
Err(err) => return Err(Self::tool_task_join_error(err)),
}
Err(err) => Err(Self::tool_task_join_error(err)),
}
let response = Self::aborted_response(&call, secs);
notify_tool_aborted(
abort_session.as_ref(),
abort_turn.as_ref(),
call.call_id.as_str(),
&call.tool_name,
abort_source,
)
.await;
Ok(response)
}
},
}
@@ -274,6 +287,85 @@ mod tests {
impl CoreToolRuntime for ImmediateHandler {}
struct CancellationCleanupHandler {
tool_name: codex_tools::ToolName,
started: std::sync::Mutex<Option<oneshot::Sender<()>>>,
cleanup_started: std::sync::Mutex<Option<oneshot::Sender<()>>>,
allow_cleanup: Arc<Notify>,
}
#[async_trait::async_trait]
impl ToolExecutor<ToolInvocation> for CancellationCleanupHandler {
fn tool_name(&self) -> codex_tools::ToolName {
self.tool_name.clone()
}
fn spec(&self) -> codex_tools::ToolSpec {
codex_tools::ToolSpec::Function(codex_tools::ResponsesApiTool {
name: self.tool_name.name.clone(),
description: "Cancellation cleanup test tool.".to_string(),
strict: false,
defer_loading: None,
parameters: codex_tools::JsonSchema::default(),
output_schema: None,
})
}
async fn handle(
&self,
invocation: ToolInvocation,
) -> Result<Box<dyn crate::tools::context::ToolOutput>, FunctionCallError> {
let started = self
.started
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.take();
if let Some(started) = started {
let _ = started.send(());
}
invocation.cancellation_token.cancelled().await;
let cleanup_started = self
.cleanup_started
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.take();
if let Some(cleanup_started) = cleanup_started {
let _ = cleanup_started.send(());
}
self.allow_cleanup.notified().await;
Ok(Box::new(FunctionToolOutput::from_text(
"cleanup complete".to_string(),
Some(false),
)))
}
}
impl CoreToolRuntime for CancellationCleanupHandler {
fn waits_for_runtime_cancellation(&self) -> bool {
true
}
}
struct FinishRecorder {
records: Arc<std::sync::Mutex<Vec<ToolCallOutcome>>>,
}
impl codex_extension_api::ToolLifecycleContributor for FinishRecorder {
fn on_tool_finish<'a>(
&'a self,
input: codex_extension_api::ToolFinishInput<'a>,
) -> codex_extension_api::ToolLifecycleFuture<'a> {
let records = Arc::clone(&self.records);
let outcome = input.outcome;
Box::pin(async move {
records
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.push(outcome);
})
}
}
struct BlockingFinishContributor {
records: Arc<std::sync::Mutex<Vec<ToolCallOutcome>>>,
finish_started: std::sync::Mutex<Option<oneshot::Sender<()>>>,
@@ -375,4 +467,75 @@ mod tests {
Ok(())
}
#[tokio::test]
async fn cancellation_waiting_for_runtime_cleanup_emits_only_aborted_lifecycle()
-> anyhow::Result<()> {
let (mut session, turn_context) = crate::session::tests::make_session_and_context().await;
let records = Arc::new(std::sync::Mutex::new(Vec::new()));
let mut builder =
codex_extension_api::ExtensionRegistryBuilder::<crate::config::Config>::new();
builder.tool_lifecycle_contributor(Arc::new(FinishRecorder {
records: Arc::clone(&records),
}));
session.services.extensions = Arc::new(builder.build());
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let tool_name = codex_tools::ToolName::plain("cleanup_tool");
let (started_tx, started_rx) = oneshot::channel();
let (cleanup_started_tx, cleanup_started_rx) = oneshot::channel();
let allow_cleanup = Arc::new(Notify::new());
let handler = Arc::new(CancellationCleanupHandler {
tool_name: tool_name.clone(),
started: std::sync::Mutex::new(Some(started_tx)),
cleanup_started: std::sync::Mutex::new(Some(cleanup_started_tx)),
allow_cleanup: Arc::clone(&allow_cleanup),
}) as Arc<dyn CoreToolRuntime>;
let router = Arc::new(ToolRouter::from_parts(
ToolRegistry::from_tools([handler]),
Vec::new(),
));
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
let runtime = ToolCallRuntime::new(router, session, turn_context, tracker);
let cancellation_token = CancellationToken::new();
let call = ToolCall {
tool_name,
call_id: "call-1".to_string(),
payload: ToolPayload::Function {
arguments: "{}".to_string(),
},
};
let response_task =
tokio::spawn(runtime.handle_tool_call(call, cancellation_token.clone()));
started_rx.await.expect("handler should start");
cancellation_token.cancel();
cleanup_started_rx
.await
.expect("handler should start cleanup");
tokio::time::sleep(Duration::from_millis(10)).await;
allow_cleanup.notify_one();
let response = tokio::time::timeout(Duration::from_secs(1), response_task)
.await
.expect("timed out waiting for tool response")
.expect("tool response task should join")?;
let ResponseInputItem::FunctionCallOutput { output, .. } = response else {
anyhow::bail!("cancelled tool should return function output");
};
let FunctionCallOutputBody::Text(text) = output.body else {
anyhow::bail!("cancelled tool output should be text");
};
assert!(text.contains("aborted by user"));
let actual = records
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.drain(..)
.collect::<Vec<_>>();
assert_eq!(vec![ToolCallOutcome::Aborted], actual);
Ok(())
}
}
+50 -19
View File
@@ -57,6 +57,12 @@ pub(crate) trait CoreToolRuntime: ToolExecutor<ToolInvocation> {
)
}
/// Whether cancellation should let the handler finish teardown before the
/// host returns an aborted tool response.
fn waits_for_runtime_cancellation(&self) -> bool {
false
}
fn telemetry_tags<'a>(
&'a self,
_invocation: &'a ToolInvocation,
@@ -284,6 +290,10 @@ impl CoreToolRuntime for ExposureOverride {
self.handler.matches_kind(payload)
}
fn waits_for_runtime_cancellation(&self) -> bool {
self.handler.waits_for_runtime_cancellation()
}
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
self.handler.pre_tool_use_payload(invocation)
}
@@ -381,6 +391,11 @@ impl ToolRegistry {
Some(tool.supports_parallel_tool_calls())
}
pub(crate) fn waits_for_runtime_cancellation(&self, name: &ToolName) -> Option<bool> {
let tool = self.tool(name)?;
Some(tool.waits_for_runtime_cancellation())
}
#[allow(dead_code)]
pub(crate) async fn dispatch_any(
&self,
@@ -497,10 +512,12 @@ impl ToolRegistry {
PreToolUseHookResult::Blocked(message) => {
let err = FunctionCallError::RespondToModel(message);
dispatch_trace.record_failed(&err);
if let Some(terminal_outcome_reached) = &terminal_outcome_reached {
terminal_outcome_reached.store(true, Ordering::Release);
}
notify_tool_finish(&invocation, ToolCallOutcome::Blocked).await;
notify_tool_finish_if_unclaimed(
&invocation,
terminal_outcome_reached.as_deref(),
ToolCallOutcome::Blocked,
)
.await;
return Err(err);
}
PreToolUseHookResult::Continue {
@@ -511,11 +528,9 @@ impl ToolRegistry {
}
Err(err) => {
dispatch_trace.record_failed(&err);
if let Some(terminal_outcome_reached) = &terminal_outcome_reached {
terminal_outcome_reached.store(true, Ordering::Release);
}
notify_tool_finish(
notify_tool_finish_if_unclaimed(
&invocation,
terminal_outcome_reached.as_deref(),
ToolCallOutcome::Failed {
handler_executed: false,
},
@@ -638,18 +653,21 @@ impl ToolRegistry {
handler_executed: true,
},
};
if let Some(terminal_outcome_reached) = &terminal_outcome_reached {
terminal_outcome_reached.store(true, Ordering::Release);
}
notify_tool_finish(&invocation, lifecycle_outcome).await;
let finished = notify_tool_finish_if_unclaimed(
&invocation,
terminal_outcome_reached.as_deref(),
lifecycle_outcome,
)
.await;
if let Err(err) = invocation
.session
.goal_runtime_apply(GoalRuntimeEvent::ToolCompleted {
turn_context: invocation.turn.as_ref(),
tool_name: tool_name.name.as_str(),
})
.await
if finished
&& let Err(err) = invocation
.session
.goal_runtime_apply(GoalRuntimeEvent::ToolCompleted {
turn_context: invocation.turn.as_ref(),
tool_name: tool_name.name.as_str(),
})
.await
{
warn!("failed to account thread goal progress after tool call: {err}");
}
@@ -676,6 +694,19 @@ impl ToolRegistry {
}
}
async fn notify_tool_finish_if_unclaimed(
invocation: &ToolInvocation,
terminal_outcome_reached: Option<&AtomicBool>,
outcome: ToolCallOutcome,
) -> bool {
if terminal_outcome_reached.is_some_and(|reached| reached.swap(true, Ordering::AcqRel)) {
return false;
}
notify_tool_finish(invocation, outcome).await;
true
}
async fn handle_any_tool(
tool: &dyn CoreToolRuntime,
invocation: ToolInvocation,
+6
View File
@@ -86,6 +86,12 @@ impl ToolRouter {
.unwrap_or(false)
}
pub fn tool_waits_for_runtime_cancellation(&self, call: &ToolCall) -> bool {
self.registry
.waits_for_runtime_cancellation(&call.tool_name)
.unwrap_or(false)
}
#[instrument(level = "trace", skip_all, err)]
pub fn build_tool_call(item: ResponseItem) -> Result<Option<ToolCall>, FunctionCallError> {
match item {
+6 -12
View File
@@ -306,19 +306,13 @@ fn mcp_tool_info(
callable_name: tool_name.to_string(),
callable_namespace: callable_namespace.to_string(),
namespace_description: None,
tool: rmcp::model::Tool {
name: tool_name.to_string().into(),
title: None,
description: Some("Test MCP tool".to_string().into()),
input_schema: Arc::new(rmcp::model::object(json!({
tool: rmcp::model::Tool::new(
tool_name.to_string(),
"Test MCP tool",
Arc::new(rmcp::model::object(json!({
"type": "object",
}))),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
),
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
@@ -338,7 +332,7 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow
phase: None,
};
session
.record_into_history(std::slice::from_ref(&history_item), &turn)
.record_conversation_items(&turn, std::slice::from_ref(&history_item))
.await;
let router = ToolRouter::from_turn_context(
+38
View File
@@ -98,6 +98,44 @@ pub(crate) fn with_shell_env_file_read_permission(
additional_permissions
}
#[cfg(unix)]
fn prepend_path_entry(env: &mut HashMap<String, String>, path_entry: &str) -> String {
let updated_path = match env.get("PATH") {
Some(path) if !path.is_empty() => std::iter::once(path_entry)
.chain(path.split(':').filter(|entry| *entry != path_entry))
.collect::<Vec<_>>()
.join(":"),
_ => path_entry.to_string(),
};
env.insert("PATH".to_string(), updated_path.clone());
updated_path
}
#[cfg(unix)]
pub(crate) fn prepend_zsh_fork_bin_to_path(
env: &mut HashMap<String, String>,
shell_zsh_path: &Path,
) -> Option<String> {
let zsh_bin_dir = shell_zsh_path
.parent()
.map(|path| path.to_string_lossy().to_string())?;
Some(prepend_path_entry(env, &zsh_bin_dir))
}
#[cfg(unix)]
pub(crate) fn apply_zsh_fork_path_prepend(
env: &mut HashMap<String, String>,
explicit_env_overrides: &mut HashMap<String, String>,
shell_zsh_path: &Path,
) {
let Some(updated_path) = prepend_zsh_fork_bin_to_path(env, shell_zsh_path) else {
return;
};
// Snapshot wrapping restores explicit overrides after sourcing the shell
// snapshot, so capture this PATH override there as well.
explicit_env_overrides.insert("PATH".to_string(), updated_path);
}
pub(crate) fn disable_powershell_profile_for_elevated_windows_sandbox(
command: &[String],
shell_type: Option<&ShellType>,
@@ -148,6 +148,48 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow::
Ok(())
}
#[cfg(unix)]
#[test]
fn apply_zsh_fork_path_prepend_uses_shell_parent() {
let mut env = HashMap::from([("PATH".to_string(), "/usr/bin:/bin".to_string())]);
let mut explicit_env_overrides = HashMap::new();
apply_zsh_fork_path_prepend(
&mut env,
&mut explicit_env_overrides,
PathBuf::from("/package/codex-resources/zsh/bin/zsh").as_path(),
);
let expected = "/package/codex-resources/zsh/bin:/usr/bin:/bin";
assert_eq!(env.get("PATH").map(String::as_str), Some(expected));
assert_eq!(
explicit_env_overrides.get("PATH").map(String::as_str),
Some(expected)
);
}
#[cfg(unix)]
#[test]
fn apply_zsh_fork_path_prepend_moves_existing_shell_parent_to_front() {
let mut env = HashMap::from([(
"PATH".to_string(),
"/usr/bin:/package/codex-resources/zsh/bin:/bin:/package/codex-resources/zsh/bin"
.to_string(),
)]);
let mut explicit_env_overrides = HashMap::new();
apply_zsh_fork_path_prepend(
&mut env,
&mut explicit_env_overrides,
PathBuf::from("/package/codex-resources/zsh/bin/zsh").as_path(),
);
assert_eq!(
env.get("PATH").map(String::as_str),
Some("/package/codex-resources/zsh/bin:/usr/bin:/bin")
);
}
#[test]
fn explicit_escalation_keeps_user_proxy_env_without_codex_marker() {
let env = HashMap::from([
@@ -987,6 +1029,58 @@ fn maybe_wrap_shell_lc_with_snapshot_applies_explicit_path_override() {
assert_eq!(String::from_utf8_lossy(&output.stdout), "/worktree/bin");
}
#[cfg(unix)]
#[test]
fn maybe_wrap_shell_lc_with_snapshot_preserves_zsh_fork_path_prepend() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
std::fs::write(
&snapshot_path,
"# Snapshot file\nexport PATH='/snapshot/bin'\n",
)
.expect("write snapshot");
let session_shell = shell_with_snapshot(
ShellType::Bash,
"/bin/bash",
snapshot_path.abs(),
dir.path().abs(),
);
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"printf '%s' \"$PATH\"".to_string(),
];
let zsh_path = dir
.path()
.join("codex-resources")
.join("zsh")
.join("bin")
.join("zsh");
let zsh_bin_dir = zsh_path.parent().expect("zsh path should have parent");
let mut env = HashMap::from([("PATH".to_string(), "/worktree/bin".to_string())]);
let mut explicit_env_overrides = HashMap::new();
apply_zsh_fork_path_prepend(&mut env, &mut explicit_env_overrides, zsh_path.as_path());
let rewritten = maybe_wrap_shell_command_with_runtime_env(
&command,
&session_shell,
&dir.path().abs(),
/*env_file*/ None,
&explicit_env_overrides,
&env,
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env("PATH", env.get("PATH").expect("PATH should be set"))
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(
String::from_utf8_lossy(&output.stdout),
format!("{}:/worktree/bin", zsh_bin_dir.display())
);
}
#[test]
fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() {
let dir = tempdir().expect("create temp dir");
+18 -1
View File
@@ -20,6 +20,8 @@ use crate::shell::ShellType;
use crate::tools::flat_tool_name;
use crate::tools::network_approval::NetworkApprovalMode;
use crate::tools::network_approval::NetworkApprovalSpec;
#[cfg(unix)]
use crate::tools::runtimes::apply_zsh_fork_path_prepend;
use crate::tools::runtimes::build_sandbox_command;
use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox;
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
@@ -45,6 +47,7 @@ use codex_shell_command::powershell::prefix_powershell_script_with_utf8;
use codex_utils_absolute_path::AbsolutePathBuf;
use futures::future::BoxFuture;
use std::collections::HashMap;
use tokio_util::sync::CancellationToken;
#[derive(Clone, Debug)]
pub struct ShellRequest {
@@ -53,6 +56,7 @@ pub struct ShellRequest {
pub hook_command: String,
pub cwd: AbsolutePathBuf,
pub timeout_ms: Option<u64>,
pub cancellation_token: CancellationToken,
pub env: HashMap<String, String>,
pub explicit_env_overrides: HashMap<String, String>,
pub network: Option<NetworkProxy>,
@@ -233,13 +237,25 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
let managed_network =
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions);
let env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
let explicit_env_overrides = req.explicit_env_overrides.clone();
#[cfg(unix)]
let (env, explicit_env_overrides) = {
let mut env = env;
let mut explicit_env_overrides = explicit_env_overrides;
if self.backend == ShellRuntimeBackend::ShellCommandZshFork
&& let Some(shell_zsh_path) = ctx.session.services.shell_zsh_path.as_deref()
{
apply_zsh_fork_path_prepend(&mut env, &mut explicit_env_overrides, shell_zsh_path);
}
(env, explicit_env_overrides)
};
let shell_env_file_path = ctx.session.shell_env_file_path();
let command = maybe_wrap_shell_command_with_runtime_env(
&req.command,
session_shell.as_ref(),
&req.cwd,
shell_env_file_path,
&req.explicit_env_overrides,
&explicit_env_overrides,
&env,
);
let command = disable_powershell_profile_for_elevated_windows_sandbox(
@@ -271,6 +287,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
);
let command = build_sandbox_command(&command, &req.cwd, &env, additional_permissions)?;
let mut expiration: crate::exec::ExecExpiration = req.timeout_ms.into();
expiration = expiration.with_cancellation(req.cancellation_token.clone());
if let Some(cancellation) = attempt.network_denial_cancellation_token.clone() {
expiration = expiration.with_cancellation(cancellation);
}
@@ -16,6 +16,7 @@ use crate::sandboxing::SandboxPermissions;
use crate::shell::ShellType;
use crate::tools::runtimes::build_sandbox_command;
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
use crate::tools::runtimes::prepend_zsh_fork_bin_to_path;
use crate::tools::runtimes::with_shell_env_file_read_permission;
use crate::tools::sandboxing::PermissionRequestPayload;
use crate::tools::sandboxing::SandboxAttempt;
@@ -117,7 +118,8 @@ pub(super) async fn try_run_zsh_fork(
return Ok(None);
}
let env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
prepend_zsh_fork_bin_to_path(&mut env, shell_zsh_path);
let additional_permissions = with_shell_env_file_read_permission(
req.additional_permissions.clone(),
ctx.session.shell_env_file_path(),
@@ -17,6 +17,8 @@ use crate::shell::ShellType;
use crate::tools::flat_tool_name;
use crate::tools::network_approval::NetworkApprovalMode;
use crate::tools::network_approval::NetworkApprovalSpec;
#[cfg(unix)]
use crate::tools::runtimes::apply_zsh_fork_path_prepend;
use crate::tools::runtimes::build_sandbox_command;
use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox;
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
@@ -262,6 +264,19 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
if let Some(network) = managed_network {
network.apply_to_env(&mut env);
}
let explicit_env_overrides = req.explicit_env_overrides.clone();
#[cfg(unix)]
let explicit_env_overrides = {
let mut explicit_env_overrides = explicit_env_overrides;
if let UnifiedExecShellMode::ZshFork(zsh_fork_config) = &self.shell_mode {
apply_zsh_fork_path_prepend(
&mut env,
&mut explicit_env_overrides,
zsh_fork_config.shell_zsh_path.as_path(),
);
}
explicit_env_overrides
};
let environment_is_remote = req.environment.is_remote();
let shell_env_file_path = (!environment_is_remote)
.then(|| ctx.session.shell_env_file_path())
@@ -274,7 +289,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
session_shell.as_ref(),
&req.cwd,
shell_env_file_path,
&req.explicit_env_overrides,
&explicit_env_overrides,
&env,
)
};
+34 -5
View File
@@ -85,6 +85,8 @@ use std::sync::Arc;
use tracing::warn;
const MULTI_AGENT_V2_NAMESPACE_DESCRIPTION: &str = "Tools for spawning and managing sub-agents.";
const IMAGE_GEN_NAMESPACE: &str = "image_gen";
const IMAGEGEN_TOOL_NAME: &str = "imagegen";
type PlannedRuntime = Arc<dyn CoreToolRuntime>;
@@ -257,7 +259,9 @@ fn hosted_model_tool_specs(context: &CoreToolPlanContext<'_>) -> Vec<ToolSpec> {
}) {
specs.push(web_search_tool);
}
if image_generation_tool_enabled(turn_context) {
if image_generation_tool_enabled(turn_context)
&& !standalone_image_generation_available(turn_context, context.extension_tool_executors)
{
specs.push(create_image_generation_tool("png"));
}
specs
@@ -316,21 +320,41 @@ fn agent_jobs_worker_tools_enabled(turn_context: &TurnContext) -> bool {
}
fn image_generation_tool_enabled(turn_context: &TurnContext) -> bool {
image_generation_runtime_enabled(turn_context)
&& turn_context
.features
.get()
.enabled(Feature::ImageGeneration)
}
fn image_generation_runtime_enabled(turn_context: &TurnContext) -> bool {
turn_context
.auth_manager
.as_deref()
.is_some_and(AuthManager::current_auth_uses_codex_backend)
&& turn_context.provider.capabilities().image_generation
&& turn_context
.features
.get()
.enabled(Feature::ImageGeneration)
&& turn_context
.model_info
.input_modalities
.contains(&InputModality::Image)
}
fn standalone_image_generation_model_visible(turn_context: &TurnContext) -> bool {
image_generation_runtime_enabled(turn_context)
&& turn_context.features.get().enabled(Feature::ImageGenExt)
&& namespace_tools_enabled(turn_context)
}
fn standalone_image_generation_available(
turn_context: &TurnContext,
extension_tools: &[Arc<dyn ToolExecutor<ExtensionToolCall>>],
) -> bool {
standalone_image_generation_model_visible(turn_context)
&& extension_tools.iter().any(|executor| {
executor.tool_name() == ToolName::namespaced(IMAGE_GEN_NAMESPACE, IMAGEGEN_TOOL_NAME)
})
}
fn wait_agent_timeout_options(turn_context: &TurnContext) -> WaitAgentTimeoutOptions {
if multi_agent_v2_enabled(turn_context) {
return WaitAgentTimeoutOptions {
@@ -839,6 +863,11 @@ fn append_extension_tool_executors(
for executor in executors.iter().cloned() {
let tool_name = executor.tool_name();
if tool_name == ToolName::namespaced(IMAGE_GEN_NAMESPACE, IMAGEGEN_TOOL_NAME)
&& !standalone_image_generation_model_visible(turn_context)
{
continue;
}
if !reserved_tool_names.insert(tool_name.clone()) {
warn!("Skipping extension tool `{tool_name}`: tool already registered");
continue;
+24 -20
View File
@@ -302,21 +302,15 @@ fn mcp_tool(server: &str, namespace: &str, name: &str) -> ToolInfo {
callable_name: name.to_string(),
callable_namespace: namespace.to_string(),
namespace_description: Some(format!("Tools from {server}.")),
tool: rmcp::model::Tool {
name: name.to_string().into(),
title: None,
description: Some(format!("{name} test tool").into()),
input_schema: Arc::new(rmcp::model::object(json!({
tool: rmcp::model::Tool::new(
name.to_string(),
format!("{name} test tool"),
Arc::new(rmcp::model::object(json!({
"type": "object",
"properties": {},
"additionalProperties": false,
}))),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
),
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
@@ -526,7 +520,7 @@ async fn mcp_and_tool_search_follow_direct_and_deferred_tool_exposure() {
"read_mcp_resource",
]);
let missing_namespace_capability = probe_with(
let bedrock_namespace_capability = probe_with(
|turn| {
turn.model_info.supports_search_tool = true;
use_bedrock_provider(turn);
@@ -537,7 +531,7 @@ async fn mcp_and_tool_search_follow_direct_and_deferred_tool_exposure() {
},
)
.await;
missing_namespace_capability.assert_visible_lacks(&["tool_search"]);
bedrock_namespace_capability.assert_visible_contains(&["tool_search"]);
let enabled = probe_with(
|turn| {
@@ -890,7 +884,7 @@ async fn multi_agent_v2_can_use_configured_tool_namespace() {
}
#[tokio::test]
async fn multi_agent_v2_namespace_is_ignored_without_provider_namespace_support() {
async fn multi_agent_v2_namespace_is_supported_by_bedrock_provider() {
let plan = probe(|turn| {
set_feature(turn, Feature::MultiAgentV2, /*enabled*/ true);
update_config(turn, |config| {
@@ -900,15 +894,15 @@ async fn multi_agent_v2_namespace_is_ignored_without_provider_namespace_support(
})
.await;
plan.assert_visible_contains(&["spawn_agent", "send_message", "list_agents"]);
plan.assert_visible_lacks(&["agents"]);
assert!(
plan.registered_names
.contains(&ToolName::plain("spawn_agent").to_string())
);
plan.assert_visible_contains(&["agents"]);
plan.assert_visible_lacks(&["spawn_agent", "send_message", "list_agents"]);
assert!(
!plan
.registered_names
.contains(&ToolName::plain("spawn_agent").to_string())
);
assert!(
plan.registered_names
.contains(&ToolName::namespaced("agents", "spawn_agent").to_string())
);
}
@@ -966,6 +960,16 @@ async fn hosted_tools_follow_provider_auth_model_and_config_gates() {
.await;
image_generation.assert_visible_contains(&["image_generation"]);
let extension_flag_without_imagegen_tool = probe(|turn| {
use_chatgpt_auth(turn);
set_feature(turn, Feature::ImageGeneration, /*enabled*/ true);
set_feature(turn, Feature::ImageGenExt, /*enabled*/ true);
turn.model_info.input_modalities = vec![InputModality::Image];
})
.await;
extension_flag_without_imagegen_tool.assert_visible_contains(&["image_generation"]);
extension_flag_without_imagegen_tool.assert_visible_lacks(&["image_gen"]);
let live_web_search = probe(|turn| {
set_web_search_mode(turn, WebSearchMode::Live);
turn.model_info.web_search_tool_type = WebSearchToolType::TextAndImage;
+173 -43
View File
@@ -6,6 +6,11 @@ use std::sync::RwLock;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::Ordering;
use codex_analytics::CompactionImplementation;
use codex_analytics::CompactionPhase;
use codex_analytics::CompactionReason;
use codex_analytics::CompactionStrategy;
use codex_analytics::CompactionTrigger;
use codex_utils_string::to_ascii_json_string;
use serde::Serialize;
use serde_json::Value;
@@ -27,12 +32,54 @@ const MODEL_KEY: &str = "model";
const REASONING_EFFORT_KEY: &str = "reasoning_effort";
const TURN_STARTED_AT_UNIX_MS_KEY: &str = "turn_started_at_unix_ms";
const USER_INPUT_REQUESTED_DURING_TURN_KEY: &str = "user_input_requested_during_turn";
const REQUEST_KIND_KEY: &str = "request_kind";
const COMPACTION_KEY: &str = "compaction";
const WINDOW_ID_KEY: &str = "window_id";
pub(crate) struct McpTurnMetadataContext<'a> {
pub(crate) model: &'a str,
pub(crate) reasoning_effort: Option<ReasoningEffortConfig>,
}
/// Metadata present only on outbound model requests that perform compaction.
///
/// These fields describe the operation at dispatch time. Post-response outcomes such as status,
/// error, duration, and token deltas remain in compaction analytics events.
#[derive(Clone, Copy, Debug, Serialize)]
pub(crate) struct CompactionTurnMetadata {
trigger: CompactionTrigger,
reason: CompactionReason,
implementation: CompactionImplementation,
phase: CompactionPhase,
strategy: CompactionStrategy,
}
impl CompactionTurnMetadata {
pub(crate) fn new(
trigger: CompactionTrigger,
reason: CompactionReason,
implementation: CompactionImplementation,
phase: CompactionPhase,
) -> Self {
Self {
trigger,
reason,
implementation,
phase,
strategy: CompactionStrategy::Memento,
}
}
}
#[derive(Clone, Copy, Debug, Serialize)]
#[serde(rename_all = "snake_case")]
enum TurnMetadataRequestKind {
Turn,
Prewarm,
Compaction,
Memory,
}
#[derive(Clone, Debug, Default)]
struct WorkspaceGitMetadata {
associated_remote_urls: Option<BTreeMap<String, String>>,
@@ -68,8 +115,15 @@ impl From<WorkspaceGitMetadata> for TurnMetadataWorkspace {
}
}
#[derive(Clone, Debug, Serialize, Default)]
/// Base payload for the outbound model request `x-codex-turn-metadata` header.
///
/// Turn-owned state populates identity fields, including optional fork lineage. A concrete
/// request kind is added at outbound model dispatch so turns, startup prewarm, and compaction
/// remain distinguishable. Detached memory requests are constructed as `memory` directly.
#[derive(Clone, Debug, Serialize)]
pub(crate) struct TurnMetadataBag {
#[serde(default, skip_serializing_if = "Option::is_none")]
request_kind: Option<TurnMetadataRequestKind>,
#[serde(default, skip_serializing_if = "Option::is_none")]
session_id: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
@@ -87,6 +141,41 @@ pub(crate) struct TurnMetadataBag {
}
impl TurnMetadataBag {
fn new(
request_kind: Option<TurnMetadataRequestKind>,
session_id: Option<String>,
thread_id: Option<String>,
forked_from_thread_id: Option<ThreadId>,
thread_source: Option<ThreadSource>,
turn_id: Option<String>,
sandbox: Option<String>,
) -> Self {
Self {
request_kind,
session_id,
thread_id,
forked_from_thread_id,
thread_source,
turn_id,
workspaces: BTreeMap::new(),
sandbox,
}
}
fn with_workspace_git_metadata(
mut self,
repo_root: Option<String>,
workspace_git_metadata: Option<WorkspaceGitMetadata>,
) -> Self {
if let (Some(repo_root), Some(workspace_git_metadata)) = (repo_root, workspace_git_metadata)
&& !workspace_git_metadata.is_empty()
{
self.workspaces
.insert(repo_root, workspace_git_metadata.into());
}
self
}
fn to_header_value(&self) -> Option<String> {
to_ascii_json_string(self).ok()
}
@@ -117,6 +206,9 @@ fn merge_turn_metadata(
| "turn_id"
| TURN_STARTED_AT_UNIX_MS_KEY
| "forked_from_thread_id"
| REQUEST_KIND_KEY
| COMPACTION_KEY
| WINDOW_ID_KEY
) {
continue;
}
@@ -140,32 +232,24 @@ pub async fn build_turn_metadata_header(
get_has_changes(cwd),
);
let latest_git_commit_hash = head_commit_hash.map(|sha| sha.0);
if latest_git_commit_hash.is_none()
&& associated_remote_urls.is_none()
&& has_changes.is_none()
&& sandbox.is_none()
{
return None;
}
let workspace_git_metadata = WorkspaceGitMetadata {
associated_remote_urls,
latest_git_commit_hash,
has_changes,
};
let mut metadata = TurnMetadataBag {
sandbox: sandbox.map(ToString::to_string),
..Default::default()
};
if let Some(repo_root) = repo_root
&& !workspace_git_metadata.is_empty()
{
metadata
.workspaces
.insert(repo_root, workspace_git_metadata.into());
}
metadata.to_header_value()
TurnMetadataBag::new(
Some(TurnMetadataRequestKind::Memory),
/*session_id*/ None,
/*thread_id*/ None,
/*forked_from_thread_id*/ None,
/*thread_source*/ None,
/*turn_id*/ None,
sandbox.map(ToString::to_string),
)
.with_workspace_git_metadata(
repo_root,
Some(WorkspaceGitMetadata {
associated_remote_urls,
latest_git_commit_hash,
has_changes,
}),
)
.to_header_value()
}
#[derive(Clone, Debug)]
@@ -173,7 +257,7 @@ pub(crate) struct TurnMetadataState {
cwd: AbsolutePathBuf,
repo_root: Option<String>,
base_metadata: TurnMetadataBag,
base_header: String,
base_header: Option<String>,
enriched_header: Arc<RwLock<Option<String>>>,
turn_started_at_unix_ms: Arc<RwLock<Option<i64>>>,
responsesapi_client_metadata: Arc<RwLock<Option<HashMap<String, String>>>>,
@@ -203,18 +287,16 @@ impl TurnMetadataState {
)
.to_string(),
);
let base_metadata = TurnMetadataBag {
session_id: Some(session_id),
thread_id: Some(thread_id),
let base_metadata = TurnMetadataBag::new(
/*request_kind*/ None,
Some(session_id),
Some(thread_id),
forked_from_thread_id,
thread_source,
turn_id: Some(turn_id),
Some(turn_id),
sandbox,
..Default::default()
};
let base_header = base_metadata
.to_header_value()
.unwrap_or_else(|| "{}".to_string());
);
let base_header = base_metadata.to_header_value();
Self {
cwd,
@@ -239,7 +321,7 @@ impl TurnMetadataState {
{
header
} else {
self.base_header.clone()
self.base_header.clone()?
};
let turn_started_at_unix_ms = *self
.turn_started_at_unix_ms
@@ -264,6 +346,7 @@ impl TurnMetadataState {
) -> Option<serde_json::Value> {
let header = self.current_header_value()?;
let mut metadata = serde_json::from_str::<serde_json::Map<String, Value>>(&header).ok()?;
metadata.remove(REQUEST_KIND_KEY);
metadata.insert(
MODEL_KEY.to_string(),
Value::String(context.model.to_string()),
@@ -293,6 +376,52 @@ impl TurnMetadataState {
Some(Value::Object(metadata))
}
fn current_header_value_for_model_request_kind(
&self,
window_id: &str,
request_kind: TurnMetadataRequestKind,
) -> Option<String> {
let header = self.current_header_value()?;
let mut metadata = serde_json::from_str::<serde_json::Map<String, Value>>(&header).ok()?;
metadata.insert(
REQUEST_KIND_KEY.to_string(),
serde_json::to_value(request_kind).ok()?,
);
metadata.insert(
WINDOW_ID_KEY.to_string(),
Value::String(window_id.to_string()),
);
to_ascii_json_string(&metadata).ok()
}
pub(crate) fn current_header_value_for_model_request(&self, window_id: &str) -> Option<String> {
self.current_header_value_for_model_request_kind(window_id, TurnMetadataRequestKind::Turn)
}
pub(crate) fn current_header_value_for_prewarm(&self, window_id: &str) -> Option<String> {
self.current_header_value_for_model_request_kind(
window_id,
TurnMetadataRequestKind::Prewarm,
)
}
pub(crate) fn current_header_value_for_compaction(
&self,
window_id: &str,
compaction: CompactionTurnMetadata,
) -> Option<String> {
let header = self.current_header_value_for_model_request_kind(
window_id,
TurnMetadataRequestKind::Compaction,
)?;
let mut metadata = serde_json::from_str::<serde_json::Map<String, Value>>(&header).ok()?;
metadata.insert(
COMPACTION_KEY.to_string(),
serde_json::to_value(compaction).ok()?,
);
to_ascii_json_string(&metadata).ok()
}
pub(crate) fn mark_user_input_requested_during_turn(&self) {
self.user_input_requested_during_turn
.store(true, Ordering::Relaxed);
@@ -335,13 +464,14 @@ impl TurnMetadataState {
let Some(repo_root) = state.repo_root.clone() else {
return;
};
if workspace_git_metadata.is_empty() {
let enriched_metadata = state
.base_metadata
.clone()
.with_workspace_git_metadata(Some(repo_root), Some(workspace_git_metadata));
if enriched_metadata.workspaces.is_empty() {
return;
}
let mut enriched_metadata = state.base_metadata.clone();
enriched_metadata
.workspaces
.insert(repo_root, workspace_git_metadata.into());
if let Some(header_value) = enriched_metadata.to_header_value() {
*state
+96 -2
View File
@@ -20,7 +20,7 @@ fn test_mcp_turn_metadata_context() -> McpTurnMetadataContext<'static> {
}
#[tokio::test]
async fn build_turn_metadata_header_includes_has_changes_for_clean_repo() {
async fn build_turn_metadata_header_marks_detached_memory_without_turn_identity() {
let temp_dir = TempDir::new().expect("temp dir");
let repo_path = temp_dir.path().join("repo-東京").abs();
std::fs::create_dir_all(&repo_path).expect("create repo");
@@ -64,6 +64,13 @@ async fn build_turn_metadata_header_includes_has_changes_for_clean_repo() {
assert!(header.is_ascii());
assert!(!header.contains("東京"));
let parsed: Value = serde_json::from_str(&header).expect("valid json");
assert_eq!(parsed["request_kind"].as_str(), Some("memory"));
assert!(parsed.get("session_id").is_none());
assert!(parsed.get("thread_id").is_none());
assert!(parsed.get("forked_from_thread_id").is_none());
assert!(parsed.get("turn_id").is_none());
assert!(parsed.get(WINDOW_ID_KEY).is_none());
let expected_repo_path = repo_path.to_string_lossy().into_owned();
let actual_repo_path = parsed
.get("workspaces")
@@ -77,13 +84,25 @@ async fn build_turn_metadata_header_includes_has_changes_for_clean_repo() {
.and_then(|workspaces| workspaces.values().next())
.cloned()
.expect("workspace");
assert_eq!(
workspace.get("has_changes").and_then(Value::as_bool),
Some(false)
);
}
#[tokio::test]
async fn build_turn_metadata_header_marks_memory_without_workspace_metadata() {
let temp_dir = TempDir::new().expect("temp dir");
let cwd = temp_dir.path().abs();
let header = build_turn_metadata_header(&cwd, /*sandbox*/ None)
.await
.expect("detached memory should emit its request kind");
let parsed: Value = serde_json::from_str(&header).expect("valid json");
assert_eq!(parsed, serde_json::json!({"request_kind": "memory"}));
}
#[test]
fn turn_metadata_state_uses_platform_sandbox_tag() {
let temp_dir = TempDir::new().expect("temp dir");
@@ -109,6 +128,7 @@ fn turn_metadata_state_uses_platform_sandbox_tag() {
let thread_id = json.get("thread_id").and_then(Value::as_str);
let thread_source = json.get("thread_source").and_then(Value::as_str);
assert!(json.get("request_kind").is_none());
let expected_sandbox = permission_profile_sandbox_tag(
&permission_profile,
WindowsSandboxLevel::Disabled,
@@ -228,6 +248,7 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta(
let meta = state
.current_meta_value_for_mcp_request(test_mcp_turn_metadata_context())
.expect("turn metadata should be present");
assert!(meta.get("request_kind").is_none());
assert_eq!(meta["model"].as_str(), Some("gpt-5.4"));
assert_eq!(meta["reasoning_effort"].as_str(), Some("high"));
@@ -367,7 +388,10 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
"forked_from_thread_id".to_string(),
"client-supplied".to_string(),
),
("turn_id".to_string(), "client-supplied".to_string()),
(WINDOW_ID_KEY.to_string(), "client-supplied".to_string()),
("thread_source".to_string(), "client-supplied".to_string()),
("request_kind".to_string(), "client-supplied".to_string()),
(
"turn_started_at_unix_ms".to_string(),
"client-supplied".to_string(),
@@ -392,14 +416,84 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
);
assert_eq!(json["thread_source"].as_str(), Some("user"));
assert_eq!(json["turn_id"].as_str(), Some("turn-a"));
assert!(json.get("request_kind").is_none());
assert!(json.get(WINDOW_ID_KEY).is_none());
assert_eq!(
json["turn_started_at_unix_ms"].as_i64(),
Some(1_700_000_000_123)
);
let model_request_header = state
.current_header_value_for_model_request("thread-a:1")
.expect("model request header");
let model_request_json: Value =
serde_json::from_str(&model_request_header).expect("model request json");
assert_eq!(model_request_json["request_kind"].as_str(), Some("turn"));
assert_eq!(
model_request_json[WINDOW_ID_KEY].as_str(),
Some("thread-a:1")
);
let meta = state
.current_meta_value_for_mcp_request(test_mcp_turn_metadata_context())
.expect("turn metadata should be present");
assert_eq!(meta["model"].as_str(), Some("gpt-5.4"));
assert_eq!(meta["reasoning_effort"].as_str(), Some("high"));
assert!(meta.get(WINDOW_ID_KEY).is_none());
}
#[test]
fn turn_metadata_state_overlays_compaction_only_on_compaction_requests() {
let temp_dir = TempDir::new().expect("temp dir");
let cwd = temp_dir.path().abs();
let permission_profile = PermissionProfile::read_only();
let state = TurnMetadataState::new(
"session-a".to_string(),
"thread-a".to_string(),
/*forked_from_thread_id*/ None,
Some(ThreadSource::User),
"turn-a".to_string(),
cwd,
&permission_profile,
WindowsSandboxLevel::Disabled,
/*enforce_managed_network*/ false,
);
state.set_responsesapi_client_metadata(HashMap::from([(
"compaction".to_string(),
"client-supplied".to_string(),
)]));
let compact_header = state
.current_header_value_for_compaction(
"thread-a:2",
CompactionTurnMetadata::new(
CompactionTrigger::Auto,
CompactionReason::ContextLimit,
CompactionImplementation::ResponsesCompactionV2,
CompactionPhase::MidTurn,
),
)
.expect("compact header");
let compact_json: Value = serde_json::from_str(&compact_header).expect("json");
assert_eq!(compact_json["request_kind"].as_str(), Some("compaction"));
assert_eq!(compact_json["turn_id"].as_str(), Some("turn-a"));
assert_eq!(compact_json[WINDOW_ID_KEY].as_str(), Some("thread-a:2"));
assert_eq!(
compact_json["compaction"],
serde_json::json!({
"trigger": "auto",
"reason": "context_limit",
"implementation": "responses_compaction_v2",
"phase": "mid_turn",
"strategy": "memento",
})
);
let regular_header = state
.current_header_value_for_model_request("thread-a:3")
.expect("regular header");
let regular_json: Value = serde_json::from_str(&regular_header).expect("json");
assert_eq!(regular_json["request_kind"].as_str(), Some("turn"));
assert_eq!(regular_json[WINDOW_ID_KEY].as_str(), Some("thread-a:3"));
assert!(regular_json.get("compaction").is_none());
}
@@ -866,12 +866,6 @@ impl UnifiedExecProcessManager {
#[cfg(target_os = "windows")]
if request.sandbox == codex_sandboxing::SandboxType::WindowsRestrictedToken {
let sandbox_policy = request.compatibility_sandbox_policy();
let policy_json = serde_json::to_string(&sandbox_policy).map_err(|err| {
UnifiedExecError::create_process(format!(
"failed to serialize Windows sandbox policy: {err}"
))
})?;
let codex_home = crate::config::find_codex_home().map_err(|err| {
UnifiedExecError::create_process(format!(
"windows sandbox: failed to resolve codex_home: {err}"
@@ -923,7 +917,7 @@ impl UnifiedExecProcessManager {
codex_protocol::config_types::WindowsSandboxLevel::RestrictedToken
| codex_protocol::config_types::WindowsSandboxLevel::Disabled => {
codex_windows_sandbox::spawn_windows_sandbox_session_legacy(
policy_json.as_str(),
&request.permission_profile,
request.windows_sandbox_policy_cwd.as_path(),
codex_home.as_ref(),
request.command.clone(),
+29 -28
View File
@@ -8,7 +8,7 @@ use codex_features::FeaturesToml;
use codex_login::default_client::originator;
use codex_otel::sanitize_metric_tag_value;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::models::PermissionProfile;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::path::Path;
@@ -145,16 +145,17 @@ pub fn elevated_setup_failure_metric_name(_err: &anyhow::Error) -> &'static str
#[cfg(target_os = "windows")]
pub fn run_elevated_setup(
policy: &SandboxPolicy,
policy_cwd: &Path,
permission_profile: &PermissionProfile,
permission_profile_cwd: &Path,
command_cwd: &Path,
env_map: &HashMap<String, String>,
codex_home: &Path,
) -> anyhow::Result<()> {
let permissions =
codex_windows_sandbox::ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(
policy, policy_cwd,
);
codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_cwd(
permission_profile,
permission_profile_cwd,
)?;
codex_windows_sandbox::run_elevated_setup(
codex_windows_sandbox::SandboxSetupRequest {
permissions: &permissions,
@@ -169,8 +170,8 @@ pub fn run_elevated_setup(
#[cfg(not(target_os = "windows"))]
pub fn run_elevated_setup(
_policy: &SandboxPolicy,
_policy_cwd: &Path,
_permission_profile: &PermissionProfile,
_permission_profile_cwd: &Path,
_command_cwd: &Path,
_env_map: &HashMap<String, String>,
_codex_home: &Path,
@@ -180,15 +181,15 @@ pub fn run_elevated_setup(
#[cfg(target_os = "windows")]
pub fn run_legacy_setup_preflight(
policy: &SandboxPolicy,
policy_cwd: &Path,
permission_profile: &PermissionProfile,
permission_profile_cwd: &Path,
command_cwd: &Path,
env_map: &HashMap<String, String>,
codex_home: &Path,
) -> anyhow::Result<()> {
codex_windows_sandbox::run_windows_sandbox_legacy_preflight(
policy,
policy_cwd,
permission_profile,
permission_profile_cwd,
codex_home,
command_cwd,
env_map,
@@ -197,16 +198,16 @@ pub fn run_legacy_setup_preflight(
#[cfg(target_os = "windows")]
pub fn run_setup_refresh_with_extra_read_roots(
policy: &SandboxPolicy,
policy_cwd: &Path,
permission_profile: &PermissionProfile,
permission_profile_cwd: &Path,
command_cwd: &Path,
env_map: &HashMap<String, String>,
codex_home: &Path,
extra_read_roots: Vec<PathBuf>,
) -> anyhow::Result<()> {
codex_windows_sandbox::run_setup_refresh_with_extra_read_roots(
policy,
policy_cwd,
permission_profile,
permission_profile_cwd,
command_cwd,
env_map,
codex_home,
@@ -217,8 +218,8 @@ pub fn run_setup_refresh_with_extra_read_roots(
#[cfg(not(target_os = "windows"))]
pub fn run_legacy_setup_preflight(
_policy: &SandboxPolicy,
_policy_cwd: &Path,
_permission_profile: &PermissionProfile,
_permission_profile_cwd: &Path,
_command_cwd: &Path,
_env_map: &HashMap<String, String>,
_codex_home: &Path,
@@ -228,8 +229,8 @@ pub fn run_legacy_setup_preflight(
#[cfg(not(target_os = "windows"))]
pub fn run_setup_refresh_with_extra_read_roots(
_policy: &SandboxPolicy,
_policy_cwd: &Path,
_permission_profile: &PermissionProfile,
_permission_profile_cwd: &Path,
_command_cwd: &Path,
_env_map: &HashMap<String, String>,
_codex_home: &Path,
@@ -247,8 +248,8 @@ pub enum WindowsSandboxSetupMode {
#[derive(Debug, Clone)]
pub struct WindowsSandboxSetupRequest {
pub mode: WindowsSandboxSetupMode,
pub policy: SandboxPolicy,
pub policy_cwd: PathBuf,
pub permission_profile: PermissionProfile,
pub permission_profile_cwd: PathBuf,
pub command_cwd: PathBuf,
pub env_map: HashMap<String, String>,
pub codex_home: PathBuf,
@@ -285,8 +286,8 @@ async fn run_windows_sandbox_setup_and_persist(
request: WindowsSandboxSetupRequest,
) -> anyhow::Result<()> {
let mode = request.mode;
let policy = request.policy;
let policy_cwd = request.policy_cwd;
let permission_profile = request.permission_profile;
let permission_profile_cwd = request.permission_profile_cwd;
let command_cwd = request.command_cwd;
let env_map = request.env_map;
let codex_home = request.codex_home;
@@ -297,8 +298,8 @@ async fn run_windows_sandbox_setup_and_persist(
WindowsSandboxSetupMode::Elevated => {
if !sandbox_setup_is_complete(setup_codex_home.as_path()) {
run_elevated_setup(
&policy,
policy_cwd.as_path(),
&permission_profile,
permission_profile_cwd.as_path(),
command_cwd.as_path(),
&env_map,
setup_codex_home.as_path(),
@@ -307,8 +308,8 @@ async fn run_windows_sandbox_setup_and_persist(
}
WindowsSandboxSetupMode::Unelevated => {
run_legacy_setup_preflight(
&policy,
policy_cwd.as_path(),
&permission_profile,
permission_profile_cwd.as_path(),
command_cwd.as_path(),
&env_map,
setup_codex_home.as_path(),
@@ -1,13 +1,13 @@
use crate::windows_sandbox::run_setup_refresh_with_extra_read_roots;
use anyhow::Result;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::models::PermissionProfile;
use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
pub fn grant_read_root_non_elevated(
policy: &SandboxPolicy,
policy_cwd: &Path,
permission_profile: &PermissionProfile,
permission_profile_cwd: &Path,
command_cwd: &Path,
env_map: &HashMap<String, String>,
codex_home: &Path,
@@ -25,8 +25,8 @@ pub fn grant_read_root_non_elevated(
let canonical_root = dunce::canonicalize(read_root)?;
run_setup_refresh_with_extra_read_roots(
policy,
policy_cwd,
permission_profile,
permission_profile_cwd,
command_cwd,
env_map,
codex_home,
@@ -1,18 +1,18 @@
use super::grant_read_root_non_elevated;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::models::PermissionProfile;
use std::collections::HashMap;
use std::path::Path;
use tempfile::TempDir;
fn policy() -> SandboxPolicy {
SandboxPolicy::new_workspace_write_policy()
fn permission_profile() -> PermissionProfile {
PermissionProfile::workspace_write()
}
#[test]
fn rejects_relative_path() {
let tmp = TempDir::new().expect("tempdir");
let err = grant_read_root_non_elevated(
&policy(),
&permission_profile(),
tmp.path(),
tmp.path(),
&HashMap::new(),
@@ -28,7 +28,7 @@ fn rejects_missing_path() {
let tmp = TempDir::new().expect("tempdir");
let missing = tmp.path().join("does-not-exist");
let err = grant_read_root_non_elevated(
&policy(),
&permission_profile(),
tmp.path(),
tmp.path(),
&HashMap::new(),
@@ -45,7 +45,7 @@ fn rejects_file_path() {
let file_path = tmp.path().join("file.txt");
std::fs::write(&file_path, "hello").expect("write file");
let err = grant_read_root_non_elevated(
&policy(),
&permission_profile(),
tmp.path(),
tmp.path(),
&HashMap::new(),