From 4605c653085ac1ea3a4c48e4d1727022bd942f68 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Mar 2026 22:56:17 +0000 Subject: [PATCH 01/55] feat: custom watcher for multi-agent v2 (#15570) Custom watcher that sends an InterAgentCommunication on end of turn --- codex-rs/core/src/agent/control.rs | 36 +++ codex-rs/core/src/agent/control_tests.rs | 243 ++++++++++++++++++ .../src/codex/rollout_reconstruction_tests.rs | 16 +- .../src/tools/handlers/multi_agents_tests.rs | 74 +++--- 4 files changed, 327 insertions(+), 42 deletions(-) diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 3bdcc2efd9..282e29c48a 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -266,6 +266,7 @@ impl AgentControl { new_thread.thread_id, notification_source, child_reference, + agent_metadata.agent_path.clone(), ); Ok(LiveAgent { @@ -437,6 +438,7 @@ impl AgentControl { resumed_thread.thread_id, Some(notification_source.clone()), child_reference, + agent_metadata.agent_path.clone(), ); self.persist_thread_spawn_edge_for_source( resumed_thread.thread.as_ref(), @@ -687,6 +689,7 @@ impl AgentControl { child_thread_id: ThreadId, session_source: Option, child_reference: String, + child_agent_path: Option, ) { let Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, .. @@ -717,6 +720,39 @@ impl AgentControl { let Ok(state) = control.upgrade() else { return; }; + let child_thread = state.get_thread(child_thread_id).await.ok(); + if let Some(child_agent_path) = child_agent_path + && child_thread + .as_ref() + .map(|thread| thread.enabled(Feature::MultiAgentV2)) + .unwrap_or(true) + { + let AgentStatus::Completed(Some(content)) = &status else { + return; + }; + let Some((parent_path, _)) = child_agent_path.as_str().rsplit_once('/') else { + return; + }; + let Ok(parent_agent_path) = AgentPath::try_from(parent_path) else { + return; + }; + let Some(parent_thread_id) = control.state.agent_id_for_path(&parent_agent_path) + else { + return; + }; + let _ = control + .send_inter_agent_communication( + parent_thread_id, + InterAgentCommunication::new( + child_agent_path, + parent_agent_path, + Vec::new(), + content.clone(), + ), + ) + .await; + return; + } let Ok(parent_thread) = state.get_thread(parent_thread_id).await else { return; }; diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 677bf5a2f3..2cf7398f7d 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -11,11 +11,13 @@ use crate::contextual_user_message::SUBAGENT_NOTIFICATION_OPEN_TAG; use assert_matches::assert_matches; use chrono::Utc; use codex_features::Feature; +use codex_protocol::AgentPath; use codex_protocol::config_types::ModeKind; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::InterAgentCommunication; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::TurnAbortReason; @@ -125,6 +127,29 @@ fn history_contains_text(history_items: &[ResponseItem], needle: &str) -> bool { }) } +fn history_contains_assistant_inter_agent_communication( + history_items: &[ResponseItem], + expected: &InterAgentCommunication, +) -> bool { + history_items.iter().any(|item| { + let ResponseItem::Message { role, content, .. } = item else { + return false; + }; + if role != "assistant" { + return false; + } + content.iter().any(|content_item| match content_item { + ContentItem::OutputText { text } => { + serde_json::from_str::(text) + .ok() + .as_ref() + == Some(expected) + } + ContentItem::InputText { .. } | ContentItem::InputImage { .. } => false, + }) + }) +} + async fn wait_for_subagent_notification(parent_thread: &Arc) -> bool { let wait = async { loop { @@ -937,6 +962,223 @@ async fn spawn_child_completion_notifies_parent_history() { assert_eq!(wait_for_subagent_notification(&parent_thread).await, true); } +#[tokio::test] +async fn multi_agent_v2_completion_sends_inter_agent_message_to_direct_parent() { + let harness = AgentControlHarness::new().await; + let (root_thread_id, _) = harness.start_thread().await; + let mut config = harness.config.clone(); + let _ = config.features.enable(Feature::MultiAgentV2); + let worker_path = AgentPath::root().join("worker_a").expect("worker path"); + let worker_thread_id = harness + .control + .spawn_agent( + config.clone(), + text_input("hello worker"), + Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: root_thread_id, + depth: 1, + agent_path: Some(worker_path.clone()), + agent_nickname: None, + agent_role: Some("explorer".to_string()), + })), + ) + .await + .expect("worker spawn should succeed"); + let tester_path = worker_path.join("tester").expect("tester path"); + let tester_thread_id = harness + .control + .spawn_agent( + config, + text_input("hello tester"), + Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: worker_thread_id, + depth: 2, + agent_path: Some(tester_path.clone()), + agent_nickname: None, + agent_role: Some("explorer".to_string()), + })), + ) + .await + .expect("tester spawn should succeed"); + + let tester_thread = harness + .manager + .get_thread(tester_thread_id) + .await + .expect("tester thread should exist"); + let tester_turn = tester_thread.codex.session.new_default_turn().await; + tester_thread + .codex + .session + .send_event( + tester_turn.as_ref(), + EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: tester_turn.sub_id.clone(), + last_agent_message: Some("done".to_string()), + }), + ) + .await; + + timeout(Duration::from_secs(2), async { + loop { + let delivered = harness + .manager + .captured_ops() + .into_iter() + .any(|(thread_id, op)| { + thread_id == worker_thread_id + && matches!( + op, + Op::InterAgentCommunication { communication } + if communication.author == tester_path + && communication.recipient == worker_path + && communication.other_recipients.is_empty() + && communication.content == "done" + ) + }); + if delivered { + break; + } + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("completion watcher should send inter-agent communication"); + + let worker_thread = harness + .manager + .get_thread(worker_thread_id) + .await + .expect("worker thread should exist"); + let expected_message = InterAgentCommunication::new( + tester_path.clone(), + worker_path.clone(), + Vec::new(), + "done".to_string(), + ); + timeout(Duration::from_secs(2), async { + loop { + let history_items = worker_thread + .codex + .session + .clone_history() + .await + .raw_items() + .to_vec(); + if history_contains_assistant_inter_agent_communication( + &history_items, + &expected_message, + ) && !has_subagent_notification(&history_items) + { + break; + } + sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("worker should record assistant inter-agent message"); +} + +#[tokio::test] +async fn multi_agent_v2_completion_ignores_dead_direct_parent() { + let harness = AgentControlHarness::new().await; + let (root_thread_id, root_thread) = harness.start_thread().await; + let mut config = harness.config.clone(); + let _ = config.features.enable(Feature::MultiAgentV2); + let worker_path = AgentPath::root().join("worker_a").expect("worker path"); + let worker_thread_id = harness + .control + .spawn_agent( + config.clone(), + text_input("hello worker"), + Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: root_thread_id, + depth: 1, + agent_path: Some(worker_path.clone()), + agent_nickname: None, + agent_role: Some("explorer".to_string()), + })), + ) + .await + .expect("worker spawn should succeed"); + let tester_path = worker_path.join("tester").expect("tester path"); + let tester_thread_id = harness + .control + .spawn_agent( + config, + text_input("hello tester"), + Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: worker_thread_id, + depth: 2, + agent_path: Some(tester_path.clone()), + agent_nickname: None, + agent_role: Some("explorer".to_string()), + })), + ) + .await + .expect("tester spawn should succeed"); + harness + .control + .shutdown_live_agent(worker_thread_id) + .await + .expect("worker shutdown should succeed"); + + let tester_thread = harness + .manager + .get_thread(tester_thread_id) + .await + .expect("tester thread should exist"); + let tester_turn = tester_thread.codex.session.new_default_turn().await; + tester_thread + .codex + .session + .send_event( + tester_turn.as_ref(), + EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: tester_turn.sub_id.clone(), + last_agent_message: Some("done".to_string()), + }), + ) + .await; + + sleep(Duration::from_millis(100)).await; + + assert!( + !harness + .manager + .captured_ops() + .into_iter() + .any(|(thread_id, op)| { + thread_id == worker_thread_id + && matches!( + op, + Op::InterAgentCommunication { communication } + if communication.author == tester_path + && communication.recipient == worker_path + && communication.content == "done" + ) + }) + ); + + let root_history_items = root_thread + .codex + .session + .clone_history() + .await + .raw_items() + .to_vec(); + assert!(!history_contains_assistant_inter_agent_communication( + &root_history_items, + &InterAgentCommunication::new( + tester_path, + AgentPath::root(), + Vec::new(), + "done".to_string(), + ) + )); + assert!(!has_subagent_notification(&root_history_items)); +} + #[tokio::test] async fn completion_watcher_notifies_parent_when_child_is_missing() { let harness = AgentControlHarness::new().await; @@ -953,6 +1195,7 @@ async fn completion_watcher_notifies_parent_when_child_is_missing() { agent_role: Some("explorer".to_string()), })), child_thread_id.to_string(), + None, ); assert_eq!(wait_for_subagent_notification(&parent_thread).await, true); diff --git a/codex-rs/core/src/codex/rollout_reconstruction_tests.rs b/codex-rs/core/src/codex/rollout_reconstruction_tests.rs index 96423754db..33fb5f9af7 100644 --- a/codex-rs/core/src/codex/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/codex/rollout_reconstruction_tests.rs @@ -36,19 +36,17 @@ fn assistant_message(text: &str) -> ResponseItem { } fn inter_agent_assistant_message(text: &str) -> ResponseItem { + let communication = InterAgentCommunication::new( + AgentPath::root(), + AgentPath::root().join("worker").unwrap(), + Vec::new(), + text.to_string(), + ); ResponseItem::Message { id: None, role: "assistant".to_string(), content: vec![ContentItem::OutputText { - text: serde_json::to_string(&InterAgentCommunication::new( - AgentPath::root(), - AgentPath::root() - .join("worker") - .expect("worker path should be valid"), - Vec::new(), - text.to_string(), - )) - .expect("inter-agent communication should serialize"), + text: serde_json::to_string(&communication).unwrap(), }], end_turn: None, phase: None, diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index f74ebf0577..b780986d98 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -80,14 +80,31 @@ fn thread_manager() -> ThreadManager { ) } +fn history_contains_inter_agent_communication( + history_items: &[ResponseItem], + expected: &InterAgentCommunication, +) -> bool { + history_items.iter().any(|item| { + let ResponseItem::Message { role, content, .. } = item else { + return false; + }; + if role != "assistant" { + return false; + } + content.iter().any(|content_item| match content_item { + ContentItem::OutputText { text } => { + serde_json::from_str::(text) + .ok() + .as_ref() + == Some(expected) + } + ContentItem::InputText { .. } | ContentItem::InputImage { .. } => false, + }) + }) +} + fn inter_agent_message_text(recipient: &str, content: &str) -> String { - serde_json::to_string(&InterAgentCommunication::new( - AgentPath::root(), - AgentPath::try_from(recipient).expect("recipient path should be valid"), - Vec::new(), - content.to_string(), - )) - .expect("inter-agent communication should serialize") + format!("author: /root\nrecipient: {recipient}\nother_recipients: []\nContent: {content}") } #[derive(Clone, Copy)] @@ -401,7 +418,12 @@ async fn multi_agent_v2_spawn_returns_path_and_send_input_accepts_relative_path( .get_thread(child_thread_id) .await .expect("child thread should exist"); - let expected_message = inter_agent_message_text("/root/test_process", "continue"); + let expected_communication = InterAgentCommunication::new( + AgentPath::root(), + AgentPath::try_from("/root/test_process").expect("agent path"), + Vec::new(), + "continue".to_string(), + ); timeout(Duration::from_secs(2), async { loop { let history_items = child_thread @@ -411,18 +433,8 @@ async fn multi_agent_v2_spawn_returns_path_and_send_input_accepts_relative_path( .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::OutputText { text } - if text == &expected_message - )) - ) - }); + let recorded = + history_contains_inter_agent_communication(&history_items, &expected_communication); let saw_user_message = history_items.iter().any(|item| { matches!( item, @@ -664,7 +676,6 @@ async fn multi_agent_v2_send_input_interrupts_busy_child_without_losing_message( )) ))); - let expected_message = inter_agent_message_text("/root/worker", "continue"); timeout(Duration::from_secs(5), async { loop { let history_items = thread @@ -674,18 +685,15 @@ async fn multi_agent_v2_send_input_interrupts_busy_child_without_losing_message( .await .raw_items() .to_vec(); - let saw_envelope = history_items.iter().any(|item| { - matches!( - item, - ResponseItem::Message { role, content, .. } - if role == "assistant" - && content.iter().any(|content_item| matches!( - content_item, - ContentItem::OutputText { text } - if text == &expected_message - )) - ) - }); + let saw_envelope = history_contains_inter_agent_communication( + &history_items, + &InterAgentCommunication::new( + AgentPath::root(), + AgentPath::try_from("/root/worker").expect("agent path"), + Vec::new(), + "continue".to_string(), + ), + ); let saw_user_message = history_items.iter().any(|item| { matches!( item, From 0b5ba25b467869c35891c9c2b38178a5d13011bd Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Mar 2026 22:57:54 +0000 Subject: [PATCH 02/55] feat: custom watcher for multi-agent v2 (#15575) From 527244910fb851cea6147334dbc08f8fbce4cb9d Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Mar 2026 23:27:55 +0000 Subject: [PATCH 03/55] feat: custom watcher for multi-agent v2 (#15576) The new wait tool just returns `Wait timed out.` or `Wait completed.`. The actual content is done through the notification watcher --- .../src/tools/handlers/multi_agents_tests.rs | 71 ++++++++++++++++--- .../tools/handlers/multi_agents_v2/wait.rs | 38 +++++----- codex-rs/core/src/tools/spec.rs | 65 +++++++++++------ codex-rs/core/src/tools/spec_tests.rs | 6 +- 4 files changed, 123 insertions(+), 57 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index b780986d98..873b55d660 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -9,12 +9,14 @@ use crate::config::types::ShellEnvironmentPolicy; use crate::function_tool::FunctionCallError; use crate::protocol::AgentStatus; use crate::protocol::AskForApproval; +use crate::protocol::EventMsg; use crate::protocol::FileSystemSandboxPolicy; use crate::protocol::NetworkSandboxPolicy; use crate::protocol::Op; use crate::protocol::SandboxPolicy; use crate::protocol::SessionSource; use crate::protocol::SubAgentSource; +use crate::protocol::TurnCompleteEvent; use crate::state::TaskKind; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; @@ -1414,7 +1416,7 @@ async fn multi_agent_v2_wait_agent_accepts_targets_argument() { assert_eq!( result, crate::tools::handlers::multi_agents_v2::wait::WaitAgentResult { - status: HashMap::from([(target, AgentStatus::NotFound)]), + message: "Wait completed.".to_string(), timed_out: false, } ); @@ -1582,12 +1584,7 @@ async fn wait_agent_returns_final_status_without_timeout() { } #[tokio::test] -async fn multi_agent_v2_wait_agent_returns_statuses_keyed_by_path() { - #[derive(Debug, Deserialize)] - struct SpawnAgentResult { - task_name: String, - } - +async fn multi_agent_v2_wait_agent_returns_summary_for_named_targets() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager @@ -1617,9 +1614,7 @@ async fn multi_agent_v2_wait_agent_returns_statuses_keyed_by_path() { )) .await .expect("spawn_agent should succeed"); - let (content, _) = expect_text_output(spawn_output); - let spawn_result: SpawnAgentResult = - serde_json::from_str(&content).expect("spawn result should parse"); + let _ = expect_text_output(spawn_output); let agent_id = session .services @@ -1667,13 +1662,67 @@ async fn multi_agent_v2_wait_agent_returns_statuses_keyed_by_path() { assert_eq!( result, crate::tools::handlers::multi_agents_v2::wait::WaitAgentResult { - status: HashMap::from([(spawn_result.task_name, AgentStatus::Shutdown)]), + message: "Wait completed.".to_string(), timed_out: false, } ); assert_eq!(success, None); } +#[tokio::test] +async fn multi_agent_v2_wait_agent_does_not_return_completed_content() { + let (mut session, mut turn) = make_session_and_context().await; + let manager = thread_manager(); + session.services.agent_control = manager.agent_control(); + let mut config = (*turn.config).clone(); + config + .features + .enable(Feature::MultiAgentV2) + .expect("test config should allow feature update"); + turn.config = Arc::new(config.clone()); + + let thread = manager.start_thread(config).await.expect("start thread"); + let agent_id = thread.thread_id; + let child_turn = thread.thread.codex.session.new_default_turn().await; + thread + .thread + .codex + .session + .send_event( + child_turn.as_ref(), + EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: child_turn.sub_id.clone(), + last_agent_message: Some("sensitive child output".to_string()), + }), + ) + .await; + + let output = WaitAgentHandlerV2 + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "wait_agent", + function_payload(json!({ + "targets": [agent_id.to_string()], + "timeout_ms": 1000 + })), + )) + .await + .expect("wait_agent should succeed"); + let (content, success) = expect_text_output(output); + let result: crate::tools::handlers::multi_agents_v2::wait::WaitAgentResult = + serde_json::from_str(&content).expect("wait_agent result should be json"); + assert_eq!( + result, + crate::tools::handlers::multi_agents_v2::wait::WaitAgentResult { + message: "Wait completed.".to_string(), + timed_out: false, + } + ); + assert!(!content.contains("sensitive child output")); + assert_eq!(success, None); +} + #[tokio::test] async fn close_agent_submits_shutdown_and_returns_previous_status() { let (mut session, turn) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs index e4bc7d3f4a..1b1dc90e42 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs @@ -35,21 +35,12 @@ impl ToolHandler for Handler { let args: WaitArgs = parse_arguments(&arguments)?; let receiver_thread_ids = resolve_agent_targets(&session, &turn, args.targets).await?; let mut receiver_agents = Vec::with_capacity(receiver_thread_ids.len()); - let mut target_by_thread_id = HashMap::with_capacity(receiver_thread_ids.len()); for receiver_thread_id in &receiver_thread_ids { let agent_metadata = session .services .agent_control .get_agent_metadata(*receiver_thread_id) .unwrap_or_default(); - target_by_thread_id.insert( - *receiver_thread_id, - agent_metadata - .agent_path - .as_ref() - .map(ToString::to_string) - .unwrap_or_else(|| receiver_thread_id.to_string()), - ); receiver_agents.push(CollabAgentRef { thread_id: *receiver_thread_id, agent_nickname: agent_metadata.agent_nickname, @@ -152,18 +143,7 @@ impl ToolHandler for Handler { let timed_out = statuses.is_empty(); let statuses_by_id = statuses.clone().into_iter().collect::>(); let agent_statuses = build_wait_agent_statuses(&statuses_by_id, &receiver_agents); - let result = WaitAgentResult { - status: statuses - .into_iter() - .filter_map(|(thread_id, status)| { - target_by_thread_id - .get(&thread_id) - .cloned() - .map(|target| (target, status)) - }) - .collect(), - timed_out, - }; + let result = WaitAgentResult::from_timed_out(timed_out); session .send_event( @@ -191,10 +171,24 @@ struct WaitArgs { #[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] pub(crate) struct WaitAgentResult { - pub(crate) status: HashMap, + pub(crate) message: String, pub(crate) timed_out: bool, } +impl WaitAgentResult { + fn from_timed_out(timed_out: bool) -> Self { + let message = if timed_out { + "Wait timed out." + } else { + "Wait completed." + }; + Self { + message: message.to_string(), + timed_out, + } + } +} + impl ToolOutput for WaitAgentResult { fn log_preview(&self) -> String { tool_output_json_text(self, "wait_agent") diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index d8419e1f58..b115e572e6 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -178,23 +178,41 @@ fn resume_agent_output_schema() -> JsonValue { }) } -fn wait_output_schema() -> JsonValue { - json!({ - "type": "object", - "properties": { - "status": { - "type": "object", - "description": "Final statuses keyed by canonical task name when available, otherwise by agent id.", - "additionalProperties": agent_status_output_schema() +fn wait_output_schema(multi_agent_v2: bool) -> JsonValue { + if multi_agent_v2 { + json!({ + "type": "object", + "properties": { + "message": { + "type": "string", + "description": "Brief wait summary without the agent's final content." + }, + "timed_out": { + "type": "boolean", + "description": "Whether the wait call returned due to timeout before any agent reached a final status." + } }, - "timed_out": { - "type": "boolean", - "description": "Whether the wait call returned due to timeout before any agent reached a final status." - } - }, - "required": ["status", "timed_out"], - "additionalProperties": false - }) + "required": ["message", "timed_out"], + "additionalProperties": false + }) + } else { + json!({ + "type": "object", + "properties": { + "status": { + "type": "object", + "description": "Final statuses keyed by canonical task name when available, otherwise by agent id.", + "additionalProperties": agent_status_output_schema() + }, + "timed_out": { + "type": "boolean", + "description": "Whether the wait call returned due to timeout before any agent reached a final status." + } + }, + "required": ["status", "timed_out"], + "additionalProperties": false + }) + } } fn close_agent_output_schema() -> JsonValue { @@ -1422,7 +1440,7 @@ fn create_resume_agent_tool() -> ToolSpec { }) } -fn create_wait_agent_tool() -> ToolSpec { +fn create_wait_agent_tool(multi_agent_v2: bool) -> ToolSpec { let mut properties = BTreeMap::new(); properties.insert( "targets".to_string(), @@ -1445,8 +1463,13 @@ fn create_wait_agent_tool() -> ToolSpec { ToolSpec::Function(ResponsesApiTool { name: "wait_agent".to_string(), - description: "Wait for agents to reach a final status. Completed statuses may include the agent's final message. Returns empty status when timed out. Once the agent reaches a final status, a notification message will be received containing the same completed status." - .to_string(), + description: if multi_agent_v2 { + "Wait for agents to reach a final status. Returns a brief wait summary instead of the agent's final content. Returns a timeout summary when no agent reaches a final status before the deadline." + .to_string() + } else { + "Wait for agents to reach a final status. Completed statuses may include the agent's final message. Returns empty status when timed out. Once the agent reaches a final status, a notification message will be received containing the same completed status." + .to_string() + }, strict: false, defer_loading: None, parameters: JsonSchema::Object { @@ -1454,7 +1477,7 @@ fn create_wait_agent_tool() -> ToolSpec { required: Some(vec!["targets".to_string()]), additional_properties: Some(false.into()), }, - output_schema: Some(wait_output_schema()), + output_schema: Some(wait_output_schema(multi_agent_v2)), }) } @@ -3006,7 +3029,7 @@ pub(crate) fn build_specs_with_discoverable_tools( } push_tool_spec( &mut builder, - create_wait_agent_tool(), + create_wait_agent_tool(config.multi_agent_v2), /*supports_parallel_tool_calls*/ false, config.code_mode_enabled, ); diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 1cb6bb1664..b33748dd35 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -469,7 +469,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { create_view_image_tool(config.can_request_original_image_detail), create_spawn_agent_tool(&config), create_send_input_tool(), - create_wait_agent_tool(), + create_wait_agent_tool(config.multi_agent_v2), create_close_agent_tool(), ] { expected.insert(tool_name(&spec).to_string(), spec); @@ -607,8 +607,8 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { .as_ref() .expect("wait_agent should define output schema"); assert_eq!( - output_schema["properties"]["status"]["description"], - json!("Final statuses keyed by canonical task name when available, otherwise by agent id.") + output_schema["properties"]["message"]["description"], + json!("Brief wait summary without the agent's final content.") ); assert_lacks_tool_name(&tools, "resume_agent"); } From 84fb180eeb9ef8bbef9986a715925c10d6e15e37 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 23 Mar 2026 18:59:04 -0700 Subject: [PATCH 04/55] fix: build PATH env var using OsString instead of String (#15360) --- codex-rs/arg0/src/lib.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index d8dbaaa30d..e210783a35 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -309,14 +309,16 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result { - format!("{path_element}{PATH_SEPARATOR}{existing_path}") - } - Err(_) => { - format!("{path_element}") + let updated_path_env_var = match std::env::var_os("PATH") { + Some(existing_path) => { + let mut path_env_var = + std::ffi::OsString::with_capacity(path.as_os_str().len() + 1 + existing_path.len()); + path_env_var.push(path); + path_env_var.push(PATH_SEPARATOR); + path_env_var.push(existing_path); + path_env_var } + None => path.as_os_str().to_owned(), }; unsafe { From f547b79bd0003fe2e12b7c9f9449b303f37fcc27 Mon Sep 17 00:00:00 2001 From: Charley Cunningham Date: Mon, 23 Mar 2026 19:05:42 -0700 Subject: [PATCH 05/55] Add fork snapshot modes (#15239) ## Summary - add `ForkSnapshotMode` to `ThreadManager::fork_thread` so callers can request either a committed snapshot or an interrupted snapshot - share the model-visible `` history marker between the live interrupt path and interrupted forks - update the small set of direct fork callsites to pass `ForkSnapshotMode::Committed` Note: this enables /btw to work similarly as Esc to interrupt (hopefully somewhat in distribution) --------- Co-authored-by: Codex --- .../src/protocol/thread_history.rs | 22 + codex-rs/app-server/README.md | 4 +- .../app-server/src/codex_message_processor.rs | 5 +- .../app-server/tests/suite/v2/thread_fork.rs | 2 +- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/rollout/truncation.rs | 8 +- codex-rs/core/src/rollout/truncation_tests.rs | 6 +- codex-rs/core/src/tasks/mod.rs | 29 +- codex-rs/core/src/thread_manager.rs | 198 ++++++- codex-rs/core/src/thread_manager_tests.rs | 541 +++++++++++++++++- .../core/tests/suite/compact_resume_fork.rs | 24 +- codex-rs/core/tests/suite/fork_thread.rs | 17 +- .../core/tests/suite/permissions_messages.rs | 9 +- codex-rs/core/tests/suite/shell_snapshot.rs | 4 +- codex-rs/tui/src/app.rs | 5 +- 15 files changed, 823 insertions(+), 52 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index 11dfe29769..48fa56d687 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -74,6 +74,8 @@ pub struct ThreadHistoryBuilder { turns: Vec, current_turn: Option, next_item_index: i64, + current_rollout_index: usize, + next_rollout_index: usize, } impl Default for ThreadHistoryBuilder { @@ -88,6 +90,8 @@ impl ThreadHistoryBuilder { turns: Vec::new(), current_turn: None, next_item_index: 1, + current_rollout_index: 0, + next_rollout_index: 0, } } @@ -111,6 +115,19 @@ impl ThreadHistoryBuilder { self.current_turn.is_some() } + pub fn active_turn_id_if_explicit(&self) -> Option { + self.current_turn + .as_ref() + .filter(|turn| turn.opened_explicitly) + .map(|turn| turn.id.clone()) + } + + pub fn active_turn_start_index(&self) -> Option { + self.current_turn + .as_ref() + .map(|turn| turn.rollout_start_index) + } + /// Shared reducer for persisted rollout replay and in-memory current-turn /// tracking used by running thread resume/rejoin. /// @@ -182,6 +199,8 @@ impl ThreadHistoryBuilder { } pub fn handle_rollout_item(&mut self, item: &RolloutItem) { + self.current_rollout_index = self.next_rollout_index; + self.next_rollout_index += 1; match item { RolloutItem::EventMsg(event) => self.handle_event(event), RolloutItem::Compacted(payload) => self.handle_compacted(payload), @@ -974,6 +993,7 @@ impl ThreadHistoryBuilder { status: TurnStatus::Completed, opened_explicitly: false, saw_compaction: false, + rollout_start_index: self.current_rollout_index, } } @@ -1137,6 +1157,8 @@ struct PendingTurn { /// True when this turn includes a persisted `RolloutItem::Compacted`, which /// should keep the turn from being dropped even without normal items. saw_compaction: bool, + /// Index of the rollout item that opened this turn during replay. + rollout_start_index: usize, } impl PendingTurn { diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index ace7f2bd43..a536e5c68b 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -125,7 +125,7 @@ Example with notification opt-out: - `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. - `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. -- `thread/fork` — fork an existing thread into a new thread id by copying the stored history; accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. +- `thread/fork` — fork an existing thread into a new thread id by copying the stored history; if the source thread is currently mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. - `thread/list` — page through stored rollouts; supports cursor-based pagination and optional `modelProviders`, `sourceKinds`, `archived`, `cwd`, and `searchTerm` filters. Each returned `thread` includes `status` (`ThreadStatus`), defaulting to `notLoaded` when the thread is not currently loaded. - `thread/loaded/list` — list the thread ids currently loaded in memory. - `thread/read` — read a stored thread by id without resuming it; optionally include turns via `includeTurns`. The returned `thread` includes `status` (`ThreadStatus`), defaulting to `notLoaded` when the thread is not currently loaded. @@ -240,7 +240,7 @@ Example: { "id": 11, "result": { "thread": { "id": "thr_123", … } } } ``` -To branch from a stored session, call `thread/fork` with the `thread.id`. This creates a new thread id and emits a `thread/started` notification for it. Pass `ephemeral: true` when the fork should stay in-memory only: +To branch from a stored session, call `thread/fork` with the `thread.id`. This creates a new thread id and emits a `thread/started` notification for it. If the source thread is actively running, the fork snapshots it as if the current turn had been interrupted first. Pass `ephemeral: true` when the fork should stay in-memory only: ```json { "method": "thread/fork", "id": 12, "params": { "threadId": "thr_123", "ephemeral": true } } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 947a49ff2f..37ff0ca4f2 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -183,6 +183,7 @@ use codex_core::AuthManager; use codex_core::CodexAuth; use codex_core::CodexThread; use codex_core::Cursor as RolloutCursor; +use codex_core::ForkSnapshot; use codex_core::NewThread; use codex_core::RolloutRecorder; use codex_core::SessionMeta; @@ -4039,7 +4040,7 @@ impl CodexMessageProcessor { } = match self .thread_manager .fork_thread( - usize::MAX, + ForkSnapshot::Interrupted, config, rollout_path.clone(), persist_extended_history, @@ -6508,7 +6509,7 @@ impl CodexMessageProcessor { } = self .thread_manager .fork_thread( - usize::MAX, + ForkSnapshot::Interrupted, config, rollout_path, /*persist_extended_history*/ false, diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 1b40fadc67..a81c87bd4e 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -123,7 +123,7 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> { "expected forked thread to include one turn" ); let turn = &thread.turns[0]; - assert_eq!(turn.status, TurnStatus::Completed); + assert_eq!(turn.status, TurnStatus::Interrupted); assert_eq!(turn.items.len(), 1, "expected user message item"); match &turn.items[0] { ThreadItem::UserMessage { content, .. } => { diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 29436a0d7f..e51bf45076 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -97,6 +97,7 @@ mod seatbelt_permissions; mod thread_manager; pub mod web_search; pub mod windows_sandbox_read_grants; +pub use thread_manager::ForkSnapshot; pub use thread_manager::NewThread; pub use thread_manager::ThreadManager; #[deprecated(note = "use ThreadManager")] diff --git a/codex-rs/core/src/rollout/truncation.rs b/codex-rs/core/src/rollout/truncation.rs index 490bf42b97..fee84974fe 100644 --- a/codex-rs/core/src/rollout/truncation.rs +++ b/codex-rs/core/src/rollout/truncation.rs @@ -46,8 +46,8 @@ pub(crate) fn user_message_positions_in_rollout(items: &[RolloutItem]) -> Vec ResponseItem { + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: format!( + "{TURN_ABORTED_OPEN_TAG}\n{TURN_ABORTED_INTERRUPTED_GUIDANCE}\n{TURN_ABORTED_CLOSE_TAG}" + ), + }], + end_turn: None, + phase: None, + } +} + fn emit_turn_network_proxy_metric( session_telemetry: &SessionTelemetry, network_proxy_active: bool, @@ -457,17 +474,7 @@ impl Session { if reason == TurnAbortReason::Interrupted { self.cleanup_after_interrupt(&task.turn_context).await; - let marker = ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: format!( - "{TURN_ABORTED_OPEN_TAG}\n{TURN_ABORTED_INTERRUPTED_GUIDANCE}\n" - ), - }], - end_turn: None, - phase: None, - }; + let marker = interrupted_turn_history_marker(); self.record_into_history(std::slice::from_ref(&marker), task.turn_context.as_ref()) .await; self.persist_rollout_items(&[RolloutItem::ResponseItem(marker)]) diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 185a32a70f..642def5559 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -24,6 +24,9 @@ use crate::rollout::RolloutRecorder; use crate::rollout::truncation; use crate::shell_snapshot::ShellSnapshot; use crate::skills::SkillsManager; +use crate::tasks::interrupted_turn_history_marker; +use codex_app_server_protocol::ThreadHistoryBuilder; +use codex_app_server_protocol::TurnStatus; use codex_protocol::ThreadId; use codex_protocol::config_types::CollaborationModeMask; #[cfg(test)] @@ -34,6 +37,8 @@ use codex_protocol::protocol::McpServerRefreshConfig; use codex_protocol::protocol::Op; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionSource; +use codex_protocol::protocol::TurnAbortReason; +use codex_protocol::protocol::TurnAbortedEvent; use codex_protocol::protocol::W3cTraceContext; use futures::StreamExt; use futures::stream::FuturesUnordered; @@ -126,6 +131,45 @@ pub struct NewThread { pub session_configured: SessionConfiguredEvent, } +// TODO(ccunningham): Add an explicit non-interrupting live-turn snapshot once +// core can represent sampling boundaries directly instead of relying on +// whichever items happened to be persisted mid-turn. +// +// Two likely future variants: +// - `TruncateToLastSamplingBoundary` for callers that want a coherent fork from +// the last stable model boundary without synthesizing an interrupt. +// - `WaitUntilNextSamplingBoundary` (or similar) for callers that prefer to +// fork after the next sampling boundary rather than interrupting immediately. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ForkSnapshot { + /// Fork a committed prefix ending strictly before the nth user message. + /// + /// When `n` is within range, this cuts before that 0-based user-message + /// boundary. When `n` is out of range and the source thread is currently + /// mid-turn, this instead cuts before the active turn's opening boundary + /// so the fork drops the unfinished turn suffix. When `n` is out of range + /// and the source thread is already at a turn boundary, this returns the + /// full committed history unchanged. + TruncateBeforeNthUserMessage(usize), + + /// Fork the current persisted history as if the source thread had been + /// interrupted now. + /// + /// If the persisted snapshot ends mid-turn, this appends the same + /// `` marker produced by a real interrupt. If the snapshot is + /// already at a turn boundary, this returns the current persisted history + /// unchanged. + Interrupted, +} + +/// Preserve legacy `fork_thread(usize, ...)` callsites by mapping them to the +/// existing truncate-before-nth-user-message snapshot mode. +impl From for ForkSnapshot { + fn from(value: usize) -> Self { + Self::TruncateBeforeNthUserMessage(value) + } +} + #[derive(Debug, Default, PartialEq, Eq)] pub struct ThreadShutdownReport { pub completed: Vec, @@ -541,20 +585,41 @@ impl ThreadManager { report } - /// Fork an existing thread by taking messages up to the given position (not including - /// the message at the given position) and starting a new thread with identical - /// configuration (unless overridden by the caller's `config`). The new thread will have - /// a fresh id. Pass `usize::MAX` to keep the full rollout history. - pub async fn fork_thread( + /// Fork an existing thread by snapshotting rollout history according to + /// `snapshot` and starting a new thread with identical configuration + /// (unless overridden by the caller's `config`). The new thread will have + /// a fresh id. + pub async fn fork_thread( &self, - nth_user_message: usize, + snapshot: S, config: Config, path: PathBuf, persist_extended_history: bool, parent_trace: Option, - ) -> CodexResult { + ) -> CodexResult + where + S: Into, + { + let snapshot = snapshot.into(); let history = RolloutRecorder::get_rollout_history(&path).await?; - let history = truncate_before_nth_user_message(history, nth_user_message); + let snapshot_state = snapshot_turn_state(&history); + let history = match snapshot { + ForkSnapshot::TruncateBeforeNthUserMessage(nth_user_message) => { + truncate_before_nth_user_message(history, nth_user_message, &snapshot_state) + } + ForkSnapshot::Interrupted => { + let history = match history { + InitialHistory::New => InitialHistory::New, + InitialHistory::Forked(history) => InitialHistory::Forked(history), + InitialHistory::Resumed(resumed) => InitialHistory::Forked(resumed.history), + }; + if snapshot_state.ends_mid_turn { + append_interrupted_boundary(history, snapshot_state.active_turn_id) + } else { + history + } + } + }; Box::pin(self.state.spawn_thread( config, history, @@ -838,11 +903,31 @@ impl ThreadManagerState { } } -/// Return a prefix of `items` obtained by cutting strictly before the nth user message -/// (0-based) and all items that follow it. -fn truncate_before_nth_user_message(history: InitialHistory, n: usize) -> InitialHistory { +/// Return a fork snapshot cut strictly before the nth user message (0-based). +/// +/// Out-of-range values keep the full committed history at a turn boundary, but +/// when the source thread is currently mid-turn they fall back to cutting +/// before the active turn's opening boundary so the fork omits the unfinished +/// suffix entirely. +fn truncate_before_nth_user_message( + history: InitialHistory, + n: usize, + snapshot_state: &SnapshotTurnState, +) -> InitialHistory { let items: Vec = history.get_rollout_items(); - let rolled = truncation::truncate_rollout_before_nth_user_message_from_start(&items, n); + let user_positions = truncation::user_message_positions_in_rollout(&items); + let rolled = if snapshot_state.ends_mid_turn && n >= user_positions.len() { + if let Some(cut_idx) = snapshot_state + .active_turn_start_index + .or_else(|| user_positions.last().copied()) + { + items[..cut_idx].to_vec() + } else { + items + } + } else { + truncation::truncate_rollout_before_nth_user_message_from_start(&items, n) + }; if rolled.is_empty() { InitialHistory::New @@ -851,6 +936,95 @@ fn truncate_before_nth_user_message(history: InitialHistory, n: usize) -> Initia } } +#[derive(Debug, Eq, PartialEq)] +struct SnapshotTurnState { + ends_mid_turn: bool, + active_turn_id: Option, + active_turn_start_index: Option, +} + +fn snapshot_turn_state(history: &InitialHistory) -> SnapshotTurnState { + let rollout_items = history.get_rollout_items(); + let mut builder = ThreadHistoryBuilder::new(); + for item in &rollout_items { + builder.handle_rollout_item(item); + } + let active_turn_id = builder.active_turn_id_if_explicit(); + if builder.has_active_turn() && active_turn_id.is_some() { + let active_turn_snapshot = builder.active_turn_snapshot(); + if active_turn_snapshot + .as_ref() + .is_some_and(|turn| turn.status != TurnStatus::InProgress) + { + return SnapshotTurnState { + ends_mid_turn: false, + active_turn_id: None, + active_turn_start_index: None, + }; + } + + return SnapshotTurnState { + ends_mid_turn: true, + active_turn_id, + active_turn_start_index: builder.active_turn_start_index(), + }; + } + + let Some(last_user_position) = truncation::user_message_positions_in_rollout(&rollout_items) + .last() + .copied() + else { + return SnapshotTurnState { + ends_mid_turn: false, + active_turn_id: None, + active_turn_start_index: None, + }; + }; + + // Synthetic fork/resume histories can contain user/assistant response items + // without explicit turn lifecycle events. If the persisted snapshot has no + // terminating boundary after its last user message, treat it as mid-turn. + SnapshotTurnState { + ends_mid_turn: !rollout_items[last_user_position + 1..].iter().any(|item| { + matches!( + item, + RolloutItem::EventMsg(EventMsg::TurnComplete(_) | EventMsg::TurnAborted(_)) + ) + }), + active_turn_id: None, + active_turn_start_index: None, + } +} + +/// Append the same persisted interrupt boundary used by the live interrupt path +/// to an existing fork snapshot after the source thread has been confirmed to +/// be mid-turn. +fn append_interrupted_boundary(history: InitialHistory, turn_id: Option) -> InitialHistory { + let aborted_event = RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { + turn_id, + reason: TurnAbortReason::Interrupted, + })); + + match history { + InitialHistory::New => InitialHistory::Forked(vec![ + RolloutItem::ResponseItem(interrupted_turn_history_marker()), + aborted_event, + ]), + InitialHistory::Forked(mut history) => { + history.push(RolloutItem::ResponseItem(interrupted_turn_history_marker())); + history.push(aborted_event); + InitialHistory::Forked(history) + } + InitialHistory::Resumed(mut resumed) => { + resumed + .history + .push(RolloutItem::ResponseItem(interrupted_turn_history_marker())); + resumed.history.push(aborted_event); + InitialHistory::Forked(resumed.history) + } + } +} + #[cfg(test)] #[path = "thread_manager_tests.rs"] mod tests; diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index e69e88fe73..40e7439ccc 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -3,11 +3,15 @@ use crate::codex::make_session_and_context; use crate::config::test_config; use crate::models_manager::collaboration_mode_presets::CollaborationModesConfig; use crate::models_manager::manager::RefreshStrategy; -use assert_matches::assert_matches; +use crate::rollout::RolloutRecorder; +use crate::tasks::interrupted_turn_history_marker; use codex_protocol::models::ContentItem; use codex_protocol::models::ReasoningItemReasoningSummary; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelsResponse; +use codex_protocol::protocol::AgentMessageEvent; +use codex_protocol::protocol::TurnStartedEvent; +use codex_protocol::protocol::UserMessageEvent; use core_test_support::responses::mount_models_once; use pretty_assertions::assert_eq; use std::time::Duration; @@ -38,7 +42,7 @@ fn assistant_msg(text: &str) -> ResponseItem { } #[test] -fn drops_from_last_user_only() { +fn truncates_before_requested_user_message() { let items = [ user_msg("u1"), assistant_msg("a1"), @@ -68,7 +72,15 @@ fn drops_from_last_user_only() { .cloned() .map(RolloutItem::ResponseItem) .collect(); - let truncated = truncate_before_nth_user_message(InitialHistory::Forked(initial), 1); + let truncated = truncate_before_nth_user_message( + InitialHistory::Forked(initial), + 1, + &SnapshotTurnState { + ends_mid_turn: false, + active_turn_id: None, + active_turn_start_index: None, + }, + ); let got_items = truncated.get_rollout_items(); let expected_items = vec![ RolloutItem::ResponseItem(items[0].clone()), @@ -85,8 +97,99 @@ fn drops_from_last_user_only() { .cloned() .map(RolloutItem::ResponseItem) .collect(); - let truncated2 = truncate_before_nth_user_message(InitialHistory::Forked(initial2), 2); - assert_matches!(truncated2, InitialHistory::New); + let truncated2 = truncate_before_nth_user_message( + InitialHistory::Forked(initial2.clone()), + 2, + &SnapshotTurnState { + ends_mid_turn: false, + active_turn_id: None, + active_turn_start_index: None, + }, + ); + assert_eq!( + serde_json::to_value(truncated2.get_rollout_items()).unwrap(), + serde_json::to_value(initial2).unwrap() + ); +} + +#[test] +fn out_of_range_truncation_drops_only_unfinished_suffix_mid_turn() { + let items = vec![ + RolloutItem::ResponseItem(user_msg("u1")), + RolloutItem::ResponseItem(assistant_msg("a1")), + RolloutItem::ResponseItem(user_msg("u2")), + RolloutItem::ResponseItem(assistant_msg("partial")), + ]; + + let truncated = truncate_before_nth_user_message( + InitialHistory::Forked(items.clone()), + usize::MAX, + &SnapshotTurnState { + ends_mid_turn: true, + active_turn_id: None, + active_turn_start_index: None, + }, + ); + + assert_eq!( + serde_json::to_value(truncated.get_rollout_items()).unwrap(), + serde_json::to_value(items[..2].to_vec()).unwrap() + ); +} + +#[test] +fn fork_thread_accepts_legacy_usize_snapshot_argument() { + fn assert_legacy_snapshot_callsite( + manager: &ThreadManager, + config: Config, + path: std::path::PathBuf, + ) { + let _future = manager.fork_thread( + usize::MAX, + config, + path, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ); + } + + let _: fn(&ThreadManager, Config, std::path::PathBuf) = assert_legacy_snapshot_callsite; +} + +#[test] +fn out_of_range_truncation_drops_pre_user_active_turn_prefix() { + let items = vec![ + RolloutItem::ResponseItem(user_msg("u1")), + RolloutItem::ResponseItem(assistant_msg("a1")), + RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-2".to_string(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + })), + RolloutItem::ResponseItem(user_msg("u2")), + RolloutItem::ResponseItem(assistant_msg("partial")), + ]; + + let snapshot_state = snapshot_turn_state(&InitialHistory::Forked(items.clone())); + assert_eq!( + snapshot_state, + SnapshotTurnState { + ends_mid_turn: true, + active_turn_id: Some("turn-2".to_string()), + active_turn_start_index: Some(2), + }, + ); + + let truncated = truncate_before_nth_user_message( + InitialHistory::Forked(items.clone()), + usize::MAX, + &snapshot_state, + ); + + assert_eq!( + serde_json::to_value(truncated.get_rollout_items()).unwrap(), + serde_json::to_value(items[..2].to_vec()).unwrap() + ); } #[tokio::test] @@ -104,7 +207,15 @@ async fn ignores_session_prefix_messages_when_truncating() { .map(RolloutItem::ResponseItem) .collect(); - let truncated = truncate_before_nth_user_message(InitialHistory::Forked(rollout_items), 1); + let truncated = truncate_before_nth_user_message( + InitialHistory::Forked(rollout_items), + 1, + &SnapshotTurnState { + ends_mid_turn: false, + active_turn_id: None, + active_turn_start_index: None, + }, + ); let got_items = truncated.get_rollout_items(); let expected: Vec = vec![ @@ -185,3 +296,421 @@ async fn new_uses_configured_openai_provider_for_model_refresh() { let _ = manager.list_models(RefreshStrategy::Online).await; assert_eq!(models_mock.requests().len(), 1); } + +#[test] +fn interrupted_fork_snapshot_appends_interrupt_boundary() { + let committed_history = + InitialHistory::Forked(vec![RolloutItem::ResponseItem(user_msg("hello"))]); + + assert_eq!( + serde_json::to_value( + append_interrupted_boundary(committed_history, /*turn_id*/ None).get_rollout_items() + ) + .expect("serialize interrupted fork history"), + serde_json::to_value(vec![ + RolloutItem::ResponseItem(user_msg("hello")), + RolloutItem::ResponseItem(interrupted_turn_history_marker()), + RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: None, + reason: TurnAbortReason::Interrupted, + })), + ]) + .expect("serialize expected interrupted fork history"), + ); + assert_eq!( + serde_json::to_value( + append_interrupted_boundary(InitialHistory::New, /*turn_id*/ None).get_rollout_items() + ) + .expect("serialize interrupted empty fork history"), + serde_json::to_value(vec![ + RolloutItem::ResponseItem(interrupted_turn_history_marker()), + RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: None, + reason: TurnAbortReason::Interrupted, + })), + ]) + .expect("serialize expected interrupted empty history"), + ); +} + +#[test] +fn interrupted_snapshot_is_not_mid_turn() { + let interrupted_history = InitialHistory::Forked(vec![ + RolloutItem::ResponseItem(user_msg("hello")), + RolloutItem::ResponseItem(assistant_msg("partial")), + RolloutItem::ResponseItem(interrupted_turn_history_marker()), + RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), + reason: TurnAbortReason::Interrupted, + })), + ]); + + assert_eq!( + snapshot_turn_state(&interrupted_history), + SnapshotTurnState { + ends_mid_turn: false, + active_turn_id: None, + active_turn_start_index: None, + }, + ); +} + +#[test] +fn completed_legacy_event_history_is_not_mid_turn() { + let completed_history = InitialHistory::Forked(vec![ + RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent { + message: "hello".to_string(), + images: None, + text_elements: Vec::new(), + local_images: Vec::new(), + })), + RolloutItem::EventMsg(EventMsg::AgentMessage(AgentMessageEvent { + message: "done".to_string(), + phase: None, + memory_citation: None, + })), + ]); + + assert_eq!( + snapshot_turn_state(&completed_history), + SnapshotTurnState { + ends_mid_turn: false, + active_turn_id: None, + active_turn_start_index: None, + }, + ); +} + +#[test] +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 { + message: "hello".to_string(), + images: None, + text_elements: Vec::new(), + local_images: Vec::new(), + })), + ]); + + assert_eq!( + snapshot_turn_state(&mixed_history), + SnapshotTurnState { + ends_mid_turn: true, + active_turn_id: None, + active_turn_start_index: None, + }, + ); +} + +#[tokio::test] +async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_history() { + let temp_dir = tempdir().expect("tempdir"); + let mut config = test_config(); + config.codex_home = temp_dir.path().join("codex-home"); + config.cwd = config.codex_home.clone(); + std::fs::create_dir_all(&config.codex_home).expect("create codex home"); + + let auth_manager = + AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let manager = ThreadManager::new( + &config, + auth_manager.clone(), + SessionSource::Exec, + CollaborationModesConfig::default(), + ); + + let source = manager + .resume_thread_with_history( + config.clone(), + InitialHistory::Forked(vec![ + RolloutItem::ResponseItem(user_msg("hello")), + RolloutItem::ResponseItem(assistant_msg("partial")), + ]), + auth_manager, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) + .await + .expect("create source thread from completed history"); + let source_path = source + .thread + .rollout_path() + .expect("source rollout path should exist"); + let source_history = RolloutRecorder::get_rollout_history(&source_path) + .await + .expect("read source rollout history"); + let source_snapshot_state = snapshot_turn_state(&source_history); + assert!(source_snapshot_state.ends_mid_turn); + let expected_turn_id = source_snapshot_state.active_turn_id.clone(); + assert_eq!(expected_turn_id, None); + + let forked = manager + .fork_thread( + ForkSnapshot::Interrupted, + config, + source_path, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) + .await + .expect("fork interrupted snapshot"); + let forked_path = forked + .thread + .rollout_path() + .expect("forked rollout path should exist"); + let history = RolloutRecorder::get_rollout_history(&forked_path) + .await + .expect("read forked rollout history"); + assert!(!snapshot_turn_state(&history).ends_mid_turn); + let rollout_items: Vec<_> = history + .get_rollout_items() + .into_iter() + .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) + .collect(); + let interrupted_marker_json = + serde_json::to_value(RolloutItem::ResponseItem(interrupted_turn_history_marker())) + .expect("serialize interrupted marker"); + let interrupted_abort_json = serde_json::to_value(RolloutItem::EventMsg( + EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: expected_turn_id, + reason: TurnAbortReason::Interrupted, + }), + )) + .expect("serialize interrupted abort event"); + assert_eq!( + rollout_items + .iter() + .filter(|item| { + serde_json::to_value(item).expect("serialize rollout item") + == interrupted_marker_json + }) + .count(), + 1, + ); + assert_eq!( + rollout_items + .iter() + .filter(|item| { + serde_json::to_value(item).expect("serialize rollout item") + == interrupted_abort_json + }) + .count(), + 1, + ); +} + +#[tokio::test] +async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { + let temp_dir = tempdir().expect("tempdir"); + let mut config = test_config(); + config.codex_home = temp_dir.path().join("codex-home"); + config.cwd = config.codex_home.clone(); + std::fs::create_dir_all(&config.codex_home).expect("create codex home"); + + let auth_manager = + AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let manager = ThreadManager::new( + &config, + auth_manager.clone(), + SessionSource::Exec, + CollaborationModesConfig::default(), + ); + + let source = manager + .resume_thread_with_history( + config.clone(), + InitialHistory::Forked(vec![ + RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-explicit".to_string(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + })), + RolloutItem::ResponseItem(user_msg("hello")), + RolloutItem::ResponseItem(assistant_msg("partial")), + ]), + auth_manager, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) + .await + .expect("create source thread from explicit partial history"); + let source_path = source + .thread + .rollout_path() + .expect("source rollout path should exist"); + let source_history = RolloutRecorder::get_rollout_history(&source_path) + .await + .expect("read source rollout history"); + let source_snapshot_state = snapshot_turn_state(&source_history); + assert_eq!( + source_snapshot_state, + SnapshotTurnState { + ends_mid_turn: true, + active_turn_id: Some("turn-explicit".to_string()), + active_turn_start_index: Some(1), + }, + ); + + let forked = manager + .fork_thread( + ForkSnapshot::Interrupted, + config, + source_path, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) + .await + .expect("fork interrupted snapshot"); + let forked_path = forked + .thread + .rollout_path() + .expect("forked rollout path should exist"); + let history = RolloutRecorder::get_rollout_history(&forked_path) + .await + .expect("read forked rollout history"); + let rollout_items: Vec<_> = history + .get_rollout_items() + .into_iter() + .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) + .collect(); + + assert!(rollout_items.iter().any(|item| { + matches!( + item, + RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: Some(turn_id), + reason: TurnAbortReason::Interrupted, + })) if turn_id == "turn-explicit" + ) + })); +} + +#[tokio::test] +async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_source() { + let temp_dir = tempdir().expect("tempdir"); + let mut config = test_config(); + config.codex_home = temp_dir.path().join("codex-home"); + config.cwd = config.codex_home.clone(); + std::fs::create_dir_all(&config.codex_home).expect("create codex home"); + + let auth_manager = + AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let manager = ThreadManager::new( + &config, + auth_manager.clone(), + SessionSource::Exec, + CollaborationModesConfig::default(), + ); + + let source = manager + .resume_thread_with_history( + config.clone(), + InitialHistory::Forked(vec![ + RolloutItem::ResponseItem(user_msg("hello")), + RolloutItem::ResponseItem(assistant_msg("partial")), + ]), + auth_manager, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) + .await + .expect("create source thread from partial history"); + let source_path = source + .thread + .rollout_path() + .expect("source rollout path should exist"); + let source_history = RolloutRecorder::get_rollout_history(&source_path) + .await + .expect("read source rollout history"); + assert!(snapshot_turn_state(&source_history).ends_mid_turn); + manager.remove_thread(&source.thread_id).await; + + let forked = manager + .fork_thread( + ForkSnapshot::Interrupted, + config.clone(), + source_path, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) + .await + .expect("fork interrupted snapshot"); + let forked_path = forked + .thread + .rollout_path() + .expect("forked rollout path should exist"); + let history = RolloutRecorder::get_rollout_history(&forked_path) + .await + .expect("read forked rollout history"); + assert!(!snapshot_turn_state(&history).ends_mid_turn); + + let forked_rollout_items: Vec<_> = history + .get_rollout_items() + .into_iter() + .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) + .collect(); + let interrupted_marker_json = + serde_json::to_value(RolloutItem::ResponseItem(interrupted_turn_history_marker())) + .expect("serialize interrupted marker"); + assert_eq!( + forked_rollout_items + .iter() + .filter(|item| { + serde_json::to_value(item).expect("serialize forked rollout item") + == interrupted_marker_json + }) + .count(), + 1, + ); + + manager.remove_thread(&forked.thread_id).await; + let reforked = manager + .fork_thread( + ForkSnapshot::Interrupted, + config, + forked_path, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) + .await + .expect("re-fork interrupted snapshot"); + let reforked_path = reforked + .thread + .rollout_path() + .expect("re-forked rollout path should exist"); + let reforked_history = RolloutRecorder::get_rollout_history(&reforked_path) + .await + .expect("read re-forked rollout history"); + let reforked_rollout_items: Vec<_> = reforked_history + .get_rollout_items() + .into_iter() + .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) + .collect(); + + assert_eq!( + reforked_rollout_items + .iter() + .filter(|item| { + serde_json::to_value(item).expect("serialize re-forked rollout item") + == interrupted_marker_json + }) + .count(), + 1, + ); + assert_eq!( + reforked_rollout_items + .iter() + .filter(|item| { + matches!( + item, + RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { + reason: TurnAbortReason::Interrupted, + .. + })) + ) + }) + .count(), + 1, + ); +} diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 0b7852af0e..2007aab55b 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -12,6 +12,7 @@ use super::compact::FIRST_REPLY; use super::compact::SUMMARY_TEXT; use anyhow::Result; use codex_core::CodexThread; +use codex_core::ForkSnapshot; use codex_core::ThreadManager; use codex_core::compact::SUMMARIZATION_PROMPT; use codex_core::config::Config; @@ -383,8 +384,13 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() let seeded_user_prefix = &first_request_user_texts[..first_turn_user_index]; let summary_after_second_compact = extract_summary_user_text(&requests[requests.len() - 3], SUMMARY_TEXT); - let mut expected_after_second_compact_user_texts = - vec!["AFTER_FORK".to_string(), summary_after_second_compact]; + let mut expected_after_second_compact_user_texts = vec![ + "hello world".to_string(), + "AFTER_COMPACT".to_string(), + "AFTER_RESUME".to_string(), + "AFTER_FORK".to_string(), + summary_after_second_compact, + ]; expected_after_second_compact_user_texts.extend_from_slice(seeded_user_prefix); expected_after_second_compact_user_texts.push("AFTER_COMPACT_2".to_string()); let final_user_texts = json_message_input_texts(&requests[requests.len() - 1], "user"); @@ -841,8 +847,14 @@ async fn fork_thread( path: std::path::PathBuf, nth_user_message: usize, ) -> Arc { - Box::pin(manager.fork_thread(nth_user_message, config.clone(), path, false, None)) - .await - .expect("fork conversation") - .thread + Box::pin(manager.fork_thread( + ForkSnapshot::TruncateBeforeNthUserMessage(nth_user_message), + config.clone(), + path, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + )) + .await + .expect("fork conversation") + .thread } diff --git a/codex-rs/core/tests/suite/fork_thread.rs b/codex-rs/core/tests/suite/fork_thread.rs index 96fef440c8..a16e460204 100644 --- a/codex-rs/core/tests/suite/fork_thread.rs +++ b/codex-rs/core/tests/suite/fork_thread.rs @@ -1,3 +1,4 @@ +use codex_core::ForkSnapshot; use codex_core::NewThread; use codex_core::parse_turn_item; use codex_protocol::items::TurnItem; @@ -110,7 +111,13 @@ async fn fork_thread_twice_drops_to_first_message() { thread: codex_fork1, .. } = thread_manager - .fork_thread(1, config_for_fork.clone(), base_path.clone(), false, None) + .fork_thread( + ForkSnapshot::TruncateBeforeNthUserMessage(1), + config_for_fork.clone(), + base_path.clone(), + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) .await .expect("fork 1"); @@ -129,7 +136,13 @@ async fn fork_thread_twice_drops_to_first_message() { thread: codex_fork2, .. } = thread_manager - .fork_thread(0, config_for_fork.clone(), fork1_path.clone(), false, None) + .fork_thread( + ForkSnapshot::TruncateBeforeNthUserMessage(0), + config_for_fork.clone(), + fork1_path.clone(), + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) .await .expect("fork 2"); diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index 2838233e7f..30180c8cf6 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use codex_core::ForkSnapshot; use codex_core::config::Constrained; use codex_execpolicy::Policy; use codex_protocol::models::DeveloperInstructions; @@ -419,7 +420,13 @@ async fn resume_and_fork_append_permissions_messages() -> Result<()> { fork_config.permissions.approval_policy = Constrained::allow_any(AskForApproval::UnlessTrusted); let forked = initial .thread_manager - .fork_thread(usize::MAX, fork_config, rollout_path, false, None) + .fork_thread( + ForkSnapshot::Interrupted, + fork_config, + rollout_path, + /*persist_extended_history*/ false, + /*parent_trace*/ None, + ) .await?; forked .thread diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index 55bb0ac284..0df7d583e5 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -531,7 +531,9 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { let script = "apply_patch <<'EOF'\n*** Begin Patch\n*** Add File: snapshot-apply.txt\n+hello from snapshot\n*** End Patch\nEOF\n"; let args = json!({ "command": script, - "timeout_ms": 1_000, + // The intercepted apply_patch path self-invokes codex, which can take + // longer than a second in Bazel macOS test environments. + "timeout_ms": 5_000, }); let call_id = "shell-snapshot-apply-patch"; let responses = vec![ diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 0d77e3c7f8..5b43840b49 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -57,6 +57,7 @@ use codex_app_server_protocol::RequestId; use codex_arg0::Arg0DispatchPaths; use codex_core::AuthManager; use codex_core::CodexAuth; +use codex_core::ForkSnapshot; use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::config::ConfigBuilder; @@ -2502,7 +2503,7 @@ impl App { ); let forked = thread_manager .fork_thread( - usize::MAX, + ForkSnapshot::Interrupted, config.clone(), target_session.path.clone(), /*persist_extended_history*/ false, @@ -2925,7 +2926,7 @@ impl App { match self .server .fork_thread( - usize::MAX, + ForkSnapshot::Interrupted, self.config.clone(), path.clone(), /*persist_extended_history*/ false, From db8bb7236d1266dd9596dc01cb46e414fd794538 Mon Sep 17 00:00:00 2001 From: alexsong-oai Date: Mon, 23 Mar 2026 19:08:30 -0700 Subject: [PATCH 06/55] Add plugin-creator as system skill (#15554) --- .../assets/samples/plugin-creator/SKILL.md | 148 +++++++++ .../samples/plugin-creator/agents/openai.yaml | 6 + .../assets/plugin-creator-small.svg | 3 + .../plugin-creator/assets/plugin-creator.png | Bin 0 -> 1563 bytes .../references/plugin-json-spec.md | 166 ++++++++++ .../scripts/create_basic_plugin.py | 291 ++++++++++++++++++ 6 files changed, 614 insertions(+) create mode 100644 codex-rs/skills/src/assets/samples/plugin-creator/SKILL.md create mode 100644 codex-rs/skills/src/assets/samples/plugin-creator/agents/openai.yaml create mode 100644 codex-rs/skills/src/assets/samples/plugin-creator/assets/plugin-creator-small.svg create mode 100644 codex-rs/skills/src/assets/samples/plugin-creator/assets/plugin-creator.png create mode 100644 codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md create mode 100755 codex-rs/skills/src/assets/samples/plugin-creator/scripts/create_basic_plugin.py diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/SKILL.md b/codex-rs/skills/src/assets/samples/plugin-creator/SKILL.md new file mode 100644 index 0000000000..28d47b7548 --- /dev/null +++ b/codex-rs/skills/src/assets/samples/plugin-creator/SKILL.md @@ -0,0 +1,148 @@ +--- +name: plugin-creator +description: Create and scaffold plugin directories for Codex with a required `.codex-plugin/plugin.json`, optional plugin folders/files, and baseline placeholders you can edit before publishing or testing. Use when Codex needs to create a new local plugin, add optional plugin structure, or generate or update repo-root `.agents/plugins/marketplace.json` entries for plugin ordering and availability metadata. +--- + +# Plugin Creator + +## Quick Start + +1. Run the scaffold script: + +```bash + # Plugin names are normalized to lower-case hyphen-case and must be <= 64 chars. + # The generated folder and plugin.json name are always the same. +# Run from repo root (or replace .agents/... with the absolute path to this SKILL). +# By default creates in /plugins/. +python3 .agents/skills/plugin-creator/scripts/create_basic_plugin.py +``` + +2. Open `/.codex-plugin/plugin.json` and replace `[TODO: ...]` placeholders. + +3. Generate or update the repo marketplace entry when the plugin should appear in Codex UI ordering: + +```bash +# marketplace.json always lives at /.agents/plugins/marketplace.json +python3 .agents/skills/plugin-creator/scripts/create_basic_plugin.py my-plugin --with-marketplace +``` + +4. Generate/adjust optional companion folders as needed: + +```bash +python3 .agents/skills/plugin-creator/scripts/create_basic_plugin.py my-plugin --path \ + --with-skills --with-hooks --with-scripts --with-assets --with-mcp --with-apps --with-marketplace +``` + +`` is the directory where the plugin folder `` will be created (for example `~/code/plugins`). + +## What this skill creates + +- Creates plugin root at `///`. +- Always creates `///.codex-plugin/plugin.json`. +- Fills the manifest with the full schema shape, placeholder values, and the complete `interface` section. +- Creates or updates `/.agents/plugins/marketplace.json` when `--with-marketplace` is set. + - If the marketplace file does not exist yet, seed top-level `name` plus `interface.displayName` placeholders before adding the first plugin entry. +- `` is normalized using skill-creator naming rules: + - `My Plugin` → `my-plugin` + - `My--Plugin` → `my-plugin` + - underscores, spaces, and punctuation are converted to `-` + - result is lower-case hyphen-delimited with consecutive hyphens collapsed +- Supports optional creation of: + - `skills/` + - `hooks/` + - `scripts/` + - `assets/` + - `.mcp.json` + - `.app.json` + +## Marketplace workflow + +- `marketplace.json` always lives at `/.agents/plugins/marketplace.json`. +- Marketplace root metadata supports top-level `name` plus optional `interface.displayName`. +- Treat plugin order in `plugins[]` as render order in Codex. Append new entries unless a user explicitly asks to reorder the list. +- `displayName` belongs inside the marketplace `interface` object, not individual `plugins[]` entries. +- Each generated marketplace entry must include all of: + - `policy.installation` + - `policy.authentication` + - `category` +- Default new entries to: + - `policy.installation: "AVAILABLE"` + - `policy.authentication: "ON_INSTALL"` +- Override defaults only when the user explicitly specifies another allowed value. +- Allowed `policy.installation` values: + - `NOT_AVAILABLE` + - `AVAILABLE` + - `INSTALLED_BY_DEFAULT` +- Allowed `policy.authentication` values: + - `ON_INSTALL` + - `ON_USE` +- Treat `policy.products` as an override. Omit it unless the user explicitly requests product gating. +- The generated plugin entry shape is: + +```json +{ + "name": "plugin-name", + "source": { + "source": "local", + "path": "./plugins/plugin-name" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Productivity" +} +``` + +- Use `--force` only when intentionally replacing an existing marketplace entry for the same plugin name. +- If `/.agents/plugins/marketplace.json` does not exist yet, create it with top-level `"name"`, an `"interface"` object containing `"displayName"`, and a `plugins` array, then add the new entry. + +- For a brand-new marketplace file, the root object should look like: + +```json +{ + "name": "[TODO: marketplace-name]", + "interface": { + "displayName": "[TODO: Marketplace Display Name]" + }, + "plugins": [ + { + "name": "plugin-name", + "source": { + "source": "local", + "path": "./plugins/plugin-name" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Productivity" + } + ] +} +``` + +## Required behavior + +- Outer folder name and `plugin.json` `"name"` are always the same normalized plugin name. +- Do not remove required structure; keep `.codex-plugin/plugin.json` present. +- Keep manifest values as placeholders until a human or follow-up step explicitly fills them. +- If creating files inside an existing plugin path, use `--force` only when overwrite is intentional. +- Preserve any existing marketplace `interface.displayName`. +- When generating marketplace entries, always write `policy.installation`, `policy.authentication`, and `category` even if their values are defaults. +- Add `policy.products` only when the user explicitly asks for that override. +- Keep marketplace `source.path` relative to repo root as `./plugins/`. + +## Reference to exact spec sample + +For the exact canonical sample JSON for both plugin manifests and marketplace entries, use: + +- `references/plugin-json-spec.md` + +## Validation + +After editing `SKILL.md`, run: + +```bash +python3 /scripts/quick_validate.py .agents/skills/plugin-creator +``` diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/agents/openai.yaml b/codex-rs/skills/src/assets/samples/plugin-creator/agents/openai.yaml new file mode 100644 index 0000000000..979b49859f --- /dev/null +++ b/codex-rs/skills/src/assets/samples/plugin-creator/agents/openai.yaml @@ -0,0 +1,6 @@ +interface: + display_name: "Plugin Creator" + short_description: "Scaffold plugins and marketplace entries" + default_prompt: "Use $plugin-creator to scaffold a plugin with placeholder plugin.json, optional structure, and a marketplace.json entry." + icon_small: "./assets/plugin-creator-small.svg" + icon_large: "./assets/plugin-creator.png" diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/assets/plugin-creator-small.svg b/codex-rs/skills/src/assets/samples/plugin-creator/assets/plugin-creator-small.svg new file mode 100644 index 0000000000..c6e4f67c62 --- /dev/null +++ b/codex-rs/skills/src/assets/samples/plugin-creator/assets/plugin-creator-small.svg @@ -0,0 +1,3 @@ + + + diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/assets/plugin-creator.png b/codex-rs/skills/src/assets/samples/plugin-creator/assets/plugin-creator.png new file mode 100644 index 0000000000000000000000000000000000000000..4f3d6d82fa78fbdce97af3c17f6a25c683aa3290 GIT binary patch literal 1563 zcmV+$2ITpPP)1#i4F!Dtb@FnH%p6OalPi4TDy zMcTdqNqhk%K`*Qbx>_X{K4hvjHd<=hH0fq{Gsiib9J_g*IrFfyGygA?&1Rpl>Iw2qhkb ze|)YB#{A1DHe~H{=}oVwFPPA*37~O^y*^+fpb`thKY1$)zGE1LrVQMsu}jdkKwuIF z!Y>Y|aerZBkl8tR2W{Vc1+7>_fXyPMKjmfh7264^#1Q8f8yfM)Gsj@1QV6g#maH@I zP%I~8Ek1&*Sz<4`O%n=C#xm?`ka z@f*eCnG*Imx=o;)FU8GPEG9%?fE(jGa4*wlOu)drquT{LDa8yFs(BEJAz$>R*gPVE zqU#+{sHQ>IV#p$cqEO9(Y=$ALItt^-S@v#+A*(J*LO`5i$f|}?GICI;ra;_c$ihcq zX6#?7t76DPMwx(OH38xkLlzp!uBppVEb<_JVaNhUVVGdAj!ZzYNQ3x|AuFRSya%H+ z?dbZhQ0jjifQKRr5)?-HzW@Ca#_cg2y7h{`C>$TMzDr80D_JB#g2uq?7v-;0HimyF z%!H*6!ecrjR#+G^-I_yvejNAeo`a<|YBMrB2#6*u;OYX9V;b-SyY zmSuIfx3`<{v8qoM4|80YXzgSg7L69b@21RF+`z))FOtR$uxAc?!1W*3JruXoD~WPVK^^v5W;X* zpdf_dzQ8~TL#;qS2tz&JK?uWJzP@9~zdK179`pAt+{P25gyAjUK={;^Ves}2U?==6 zNf;tu_96V2E4^TR<-qwx`(A%qKJ|LP3BwTiGa(EWhXUIZ!jHo1q$CkT~d#9UdUR zH40%o$QuZQi$i}mMj?#VDT6HUKah5JtJNrku|D+)DV1De$d*y~ogWBegVa9;v@4x` z{E^m^dv>Y3)-b^IHelgvz;bpZlf!BnL!=DS*wpgU)oZ_Yc0Tbalu)#Ku-%=0qSQQw zNPQo|`ICR&zKgduf@(L>gD6zp1DQ&W{*=*XftU-Bx z{{AAaUi%GI*nGtbL!>kzrBZ3#>yIi-O6)L1N+5D`wYCsZVu>MA1<7o7SqLbx#Skfh zh!xgWmj{RvYYdUEo2<$LK#4tu$n`2~?FwtF{91jn{I@yA@4gUs43X;z*=U8eRh}nB zZ@!E}7h*pdGS?vX=F0UC71dDKEG$029y`&?4tIz02f3n9Xus>kIZ7E~2%2`79eMr( zBrMmT%M$w#UDy9A6bf}v=-|G+IQzzn<BbTJjk&~1^x!BXwGidKmHbOD6^2FPh=j>?` zzdy/.agents/plugins/marketplace.json` +- Local plugin: `~/.agents/plugins/marketplace.json` + +```json +{ + "name": "openai-curated", + "interface": { + "displayName": "ChatGPT Official" + }, + "plugins": [ + { + "name": "linear", + "source": { + "source": "local", + "path": "./plugins/linear" + }, + "installPolicy": "AVAILABLE", + "authPolicy": "ON_INSTALL", + "category": "Productivity" + } + ] +} +``` + +## Marketplace field guide + +### Top-level fields + +- `name` (`string`): Marketplace identifier or catalog name. +- `interface` (`object`, optional): Marketplace presentation metadata. +- `plugins` (`array`): Ordered plugin entries. This order determines how Codex renders plugins. + +### `interface` fields + +- `displayName` (`string`, optional): User-facing marketplace title. + +### Plugin entry fields + +- `name` (`string`): Plugin identifier. Match the plugin folder name and `plugin.json` `name`. +- `source` (`object`): Plugin source descriptor. + - `source` (`string`): Use `local` for this repo workflow. + - `path` (`string`): Relative plugin path based on the marketplace root. + - Repo plugin: `./plugins/` + - Local plugin in `~/.agents/plugins/marketplace.json`: `./.codex/plugins/` +- `policy` (`object`): Marketplace policy block. Always include it. + - `installation` (`string`): Availability policy. + - Allowed values: `NOT_AVAILABLE`, `AVAILABLE`, `INSTALLED_BY_DEFAULT` + - Default for new entries: `AVAILABLE` + - `authentication` (`string`): Authentication timing policy. + - Allowed values: `ON_INSTALL`, `ON_USE` + - Default for new entries: `ON_INSTALL` + - `products` (`array` of `string`, optional): Product override for this plugin entry. Omit it unless product gating is explicitly requested. +- `category` (`string`): Display category bucket. Always include it. + +### Marketplace generation rules + +- `displayName` belongs under the top-level `interface` object, not individual plugin entries. +- When creating a new marketplace file from scratch, seed `interface.displayName` alongside top-level `name`. +- Always include `policy.installation`, `policy.authentication`, and `category` on every generated or updated plugin entry. +- Treat `policy.products` as an override and omit it unless explicitly requested. +- Append new entries unless the user explicitly requests reordering. +- Replace an existing entry for the same plugin only when overwrite is intentional. +- Choose marketplace location to match the plugin destination: + - Repo plugin: `/.agents/plugins/marketplace.json` + - Local plugin: `~/.agents/plugins/marketplace.json` diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/scripts/create_basic_plugin.py b/codex-rs/skills/src/assets/samples/plugin-creator/scripts/create_basic_plugin.py new file mode 100755 index 0000000000..5a61093164 --- /dev/null +++ b/codex-rs/skills/src/assets/samples/plugin-creator/scripts/create_basic_plugin.py @@ -0,0 +1,291 @@ +#!/usr/bin/env python3 +"""Scaffold a plugin directory and optionally update marketplace.json.""" + +from __future__ import annotations + +import argparse +import json +import re +from pathlib import Path +from typing import Any + + +MAX_PLUGIN_NAME_LENGTH = 64 +DEFAULT_PLUGIN_PARENT = Path.cwd() / "plugins" +DEFAULT_MARKETPLACE_PATH = Path.cwd() / ".agents" / "plugins" / "marketplace.json" +DEFAULT_INSTALL_POLICY = "AVAILABLE" +DEFAULT_AUTH_POLICY = "ON_INSTALL" +DEFAULT_CATEGORY = "Productivity" +DEFAULT_MARKETPLACE_DISPLAY_NAME = "[TODO: Marketplace Display Name]" +VALID_INSTALL_POLICIES = {"NOT_AVAILABLE", "AVAILABLE", "INSTALLED_BY_DEFAULT"} +VALID_AUTH_POLICIES = {"ON_INSTALL", "ON_USE"} + + +def normalize_plugin_name(plugin_name: str) -> str: + """Normalize a plugin name to lowercase hyphen-case.""" + normalized = plugin_name.strip().lower() + normalized = re.sub(r"[^a-z0-9]+", "-", normalized) + normalized = normalized.strip("-") + normalized = re.sub(r"-{2,}", "-", normalized) + return normalized + + +def validate_plugin_name(plugin_name: str) -> None: + if not plugin_name: + raise ValueError("Plugin name must include at least one letter or digit.") + if len(plugin_name) > MAX_PLUGIN_NAME_LENGTH: + raise ValueError( + f"Plugin name '{plugin_name}' is too long ({len(plugin_name)} characters). " + f"Maximum is {MAX_PLUGIN_NAME_LENGTH} characters." + ) + + +def build_plugin_json(plugin_name: str) -> dict: + return { + "name": plugin_name, + "version": "[TODO: 1.2.0]", + "description": "[TODO: Brief plugin description]", + "author": { + "name": "[TODO: Author Name]", + "email": "[TODO: author@example.com]", + "url": "[TODO: https://github.com/author]", + }, + "homepage": "[TODO: https://docs.example.com/plugin]", + "repository": "[TODO: https://github.com/author/plugin]", + "license": "[TODO: MIT]", + "keywords": ["[TODO: keyword1]", "[TODO: keyword2]"], + "skills": "[TODO: ./skills/]", + "hooks": "[TODO: ./hooks.json]", + "mcpServers": "[TODO: ./.mcp.json]", + "apps": "[TODO: ./.app.json]", + "interface": { + "displayName": "[TODO: Plugin Display Name]", + "shortDescription": "[TODO: Short description for subtitle]", + "longDescription": "[TODO: Long description for details page]", + "developerName": "[TODO: OpenAI]", + "category": "[TODO: Productivity]", + "capabilities": ["[TODO: Interactive]", "[TODO: Write]"], + "websiteURL": "[TODO: https://openai.com/]", + "privacyPolicyURL": "[TODO: https://openai.com/policies/row-privacy-policy/]", + "termsOfServiceURL": "[TODO: https://openai.com/policies/row-terms-of-use/]", + "defaultPrompt": [ + "[TODO: Summarize my inbox and draft replies for me.]", + "[TODO: Find open bugs and turn them into tickets.]", + "[TODO: Review today's meetings and flag gaps.]", + ], + "brandColor": "[TODO: #3B82F6]", + "composerIcon": "[TODO: ./assets/icon.png]", + "logo": "[TODO: ./assets/logo.png]", + "screenshots": [ + "[TODO: ./assets/screenshot1.png]", + "[TODO: ./assets/screenshot2.png]", + "[TODO: ./assets/screenshot3.png]", + ], + }, + } + + +def build_marketplace_entry( + plugin_name: str, + install_policy: str, + auth_policy: str, + category: str, +) -> dict[str, Any]: + return { + "name": plugin_name, + "source": { + "source": "local", + "path": f"./plugins/{plugin_name}", + }, + "policy": { + "installation": install_policy, + "authentication": auth_policy, + }, + "category": category, + } + + +def load_json(path: Path) -> dict[str, Any]: + with path.open() as handle: + return json.load(handle) + + +def build_default_marketplace() -> dict[str, Any]: + return { + "name": "[TODO: marketplace-name]", + "interface": { + "displayName": DEFAULT_MARKETPLACE_DISPLAY_NAME, + }, + "plugins": [], + } + + +def validate_marketplace_interface(payload: dict[str, Any]) -> None: + interface = payload.get("interface") + if interface is not None and not isinstance(interface, dict): + raise ValueError("marketplace.json field 'interface' must be an object.") + + +def update_marketplace_json( + marketplace_path: Path, + plugin_name: str, + install_policy: str, + auth_policy: str, + category: str, + force: bool, +) -> None: + if marketplace_path.exists(): + payload = load_json(marketplace_path) + else: + payload = build_default_marketplace() + + if not isinstance(payload, dict): + raise ValueError(f"{marketplace_path} must contain a JSON object.") + + validate_marketplace_interface(payload) + + plugins = payload.setdefault("plugins", []) + if not isinstance(plugins, list): + raise ValueError(f"{marketplace_path} field 'plugins' must be an array.") + + new_entry = build_marketplace_entry(plugin_name, install_policy, auth_policy, category) + + for index, entry in enumerate(plugins): + if isinstance(entry, dict) and entry.get("name") == plugin_name: + if not force: + raise FileExistsError( + f"Marketplace entry '{plugin_name}' already exists in {marketplace_path}. " + "Use --force to overwrite that entry." + ) + plugins[index] = new_entry + break + else: + plugins.append(new_entry) + + write_json(marketplace_path, payload, force=True) + + +def write_json(path: Path, data: dict, force: bool) -> None: + if path.exists() and not force: + raise FileExistsError(f"{path} already exists. Use --force to overwrite.") + path.parent.mkdir(parents=True, exist_ok=True) + with path.open("w") as handle: + json.dump(data, handle, indent=2) + handle.write("\n") + + +def create_stub_file(path: Path, payload: dict, force: bool) -> None: + if path.exists() and not force: + return + path.parent.mkdir(parents=True, exist_ok=True) + with path.open("w") as handle: + json.dump(payload, handle, indent=2) + handle.write("\n") + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description="Create a plugin skeleton with placeholder plugin.json." + ) + parser.add_argument("plugin_name") + parser.add_argument( + "--path", + default=str(DEFAULT_PLUGIN_PARENT), + help="Parent directory for plugin creation (defaults to /plugins)", + ) + parser.add_argument("--with-skills", action="store_true", help="Create skills/ directory") + parser.add_argument("--with-hooks", action="store_true", help="Create hooks/ directory") + parser.add_argument("--with-scripts", action="store_true", help="Create scripts/ directory") + parser.add_argument("--with-assets", action="store_true", help="Create assets/ directory") + parser.add_argument("--with-mcp", action="store_true", help="Create .mcp.json placeholder") + parser.add_argument("--with-apps", action="store_true", help="Create .app.json placeholder") + parser.add_argument( + "--with-marketplace", + action="store_true", + help="Create or update /.agents/plugins/marketplace.json", + ) + parser.add_argument( + "--marketplace-path", + default=str(DEFAULT_MARKETPLACE_PATH), + help="Path to marketplace.json (defaults to /.agents/plugins/marketplace.json)", + ) + parser.add_argument( + "--install-policy", + default=DEFAULT_INSTALL_POLICY, + choices=sorted(VALID_INSTALL_POLICIES), + help="Marketplace policy.installation value", + ) + parser.add_argument( + "--auth-policy", + default=DEFAULT_AUTH_POLICY, + choices=sorted(VALID_AUTH_POLICIES), + help="Marketplace policy.authentication value", + ) + parser.add_argument( + "--category", + default=DEFAULT_CATEGORY, + help="Marketplace category value", + ) + parser.add_argument("--force", action="store_true", help="Overwrite existing files") + return parser.parse_args() + + +def main() -> None: + args = parse_args() + raw_plugin_name = args.plugin_name + plugin_name = normalize_plugin_name(raw_plugin_name) + if plugin_name != raw_plugin_name: + print(f"Note: Normalized plugin name from '{raw_plugin_name}' to '{plugin_name}'.") + validate_plugin_name(plugin_name) + + plugin_root = (Path(args.path).expanduser().resolve() / plugin_name) + plugin_root.mkdir(parents=True, exist_ok=True) + + plugin_json_path = plugin_root / ".codex-plugin" / "plugin.json" + write_json(plugin_json_path, build_plugin_json(plugin_name), args.force) + + optional_directories = { + "skills": args.with_skills, + "hooks": args.with_hooks, + "scripts": args.with_scripts, + "assets": args.with_assets, + } + for folder, enabled in optional_directories.items(): + if enabled: + (plugin_root / folder).mkdir(parents=True, exist_ok=True) + + if args.with_mcp: + create_stub_file( + plugin_root / ".mcp.json", + {"mcpServers": {}}, + args.force, + ) + + if args.with_apps: + create_stub_file( + plugin_root / ".app.json", + { + "apps": {}, + }, + args.force, + ) + + if args.with_marketplace: + marketplace_path = Path(args.marketplace_path).expanduser().resolve() + update_marketplace_json( + marketplace_path, + plugin_name, + args.install_policy, + args.auth_policy, + args.category, + args.force, + ) + + print(f"Created plugin scaffold: {plugin_root}") + print(f"plugin manifest: {plugin_json_path}") + if args.with_marketplace: + print(f"marketplace manifest: {marketplace_path}") + + +if __name__ == "__main__": + main() From 2227248cd60ab35df5bd13b4f3f54e577564ce3c Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Mon, 23 Mar 2026 20:56:15 -0700 Subject: [PATCH 07/55] Extract landlock helpers into codex-sandboxing (#15592) ## Summary - add a new `codex-sandboxing` crate for sandboxing extraction work - move the pure Linux sandbox argv builders and their unit tests out of `codex-core` - keep `core::landlock` as the spawn wrapper and update direct callers to use `codex_sandboxing::landlock` ## Testing - `cargo test -p codex-sandboxing` - `cargo test -p codex-core landlock` - `cargo test -p codex-cli debug_sandbox` - `just argument-comment-lint` ## Notes - this is step 1 of the move plan aimed at minimizing per-PR diffs - no re-exports or no-op proxy methods were added --- codex-rs/Cargo.lock | 11 ++ codex-rs/Cargo.toml | 2 + codex-rs/cli/Cargo.toml | 1 + codex-rs/cli/src/debug_sandbox.rs | 2 +- codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/landlock.rs | 111 +---------------- codex-rs/core/src/sandboxing/mod.rs | 4 +- codex-rs/sandboxing/BUILD.bazel | 6 + codex-rs/sandboxing/Cargo.toml | 19 +++ codex-rs/sandboxing/src/landlock.rs | 112 ++++++++++++++++++ .../src/landlock_tests.rs | 0 codex-rs/sandboxing/src/lib.rs | 1 + 12 files changed, 158 insertions(+), 112 deletions(-) create mode 100644 codex-rs/sandboxing/BUILD.bazel create mode 100644 codex-rs/sandboxing/Cargo.toml create mode 100644 codex-rs/sandboxing/src/landlock.rs rename codex-rs/{core => sandboxing}/src/landlock_tests.rs (100%) create mode 100644 codex-rs/sandboxing/src/lib.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 778dee3ea7..f9a523dd9e 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1678,6 +1678,7 @@ dependencies = [ "codex-protocol", "codex-responses-api-proxy", "codex-rmcp-client", + "codex-sandboxing", "codex-state", "codex-stdio-to-uds", "codex-terminal-detection", @@ -1885,6 +1886,7 @@ dependencies = [ "codex-otel", "codex-protocol", "codex-rmcp-client", + "codex-sandboxing", "codex-secrets", "codex-shell-command", "codex-shell-escalation", @@ -2462,6 +2464,15 @@ dependencies = [ "which 8.0.0", ] +[[package]] +name = "codex-sandboxing" +version = "0.0.0" +dependencies = [ + "codex-protocol", + "pretty_assertions", + "serde_json", +] + [[package]] name = "codex-secrets" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 524b61e3b8..db203d83b5 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -42,6 +42,7 @@ members = [ "protocol", "rmcp-client", "responses-api-proxy", + "sandboxing", "stdio-to-uds", "otel", "tui", @@ -130,6 +131,7 @@ codex-process-hardening = { path = "process-hardening" } codex-protocol = { path = "protocol" } codex-responses-api-proxy = { path = "responses-api-proxy" } codex-rmcp-client = { path = "rmcp-client" } +codex-sandboxing = { path = "sandboxing" } codex-secrets = { path = "secrets" } codex-shell-command = { path = "shell-command" } codex-shell-escalation = { path = "shell-escalation" } diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index c2fd1300c6..7e703efe10 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -36,6 +36,7 @@ codex-mcp-server = { workspace = true } codex-protocol = { workspace = true } codex-responses-api-proxy = { workspace = true } codex-rmcp-client = { workspace = true } +codex-sandboxing = { workspace = true } codex-state = { workspace = true } codex-stdio-to-uds = { workspace = true } codex-terminal-detection = { workspace = true } diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index c65b6dcad5..71a2661017 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -11,7 +11,6 @@ use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; use codex_core::config::NetworkProxyAuditMetadata; use codex_core::exec_env::create_env; -use codex_core::landlock::create_linux_sandbox_command_args_for_policies; #[cfg(target_os = "macos")] use codex_core::seatbelt::create_seatbelt_command_args_for_policies_with_extensions; #[cfg(target_os = "macos")] @@ -19,6 +18,7 @@ use codex_core::spawn::CODEX_SANDBOX_ENV_VAR; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_protocol::config_types::SandboxMode; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; use codex_utils_cli::CliConfigOverrides; use tokio::process::Child; use tokio::process::Command as TokioCommand; diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 6386c8d424..9c20345170 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -48,6 +48,7 @@ codex-otel = { workspace = true } codex-artifacts = { workspace = true } codex-protocol = { workspace = true } codex-rmcp-client = { workspace = true } +codex-sandboxing = { workspace = true } codex-state = { workspace = true } codex-terminal-detection = { workspace = true } codex-utils-absolute-path = { workspace = true } diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 19b3f7c6af..c1dc4ba542 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -5,6 +5,8 @@ use crate::spawn::spawn_child_async; use codex_network_proxy::NetworkProxy; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_sandboxing::landlock::allow_network_for_proxy; +use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; @@ -59,112 +61,3 @@ where }) .await } - -pub(crate) fn allow_network_for_proxy(enforce_managed_network: bool) -> bool { - // When managed network requirements are active, request proxy-only - // networking from the Linux sandbox helper. Without managed requirements, - // preserve existing behavior. - enforce_managed_network -} - -/// Converts the sandbox policies into the CLI invocation for -/// `codex-linux-sandbox`. -/// -/// The helper performs the actual sandboxing (bubblewrap by default + seccomp) after -/// parsing these arguments. Policy JSON flags are emitted before helper feature -/// flags so the argv order matches the helper's CLI shape. See -/// `docs/linux_sandbox.md` for the Linux semantics. -#[allow(clippy::too_many_arguments)] -pub fn create_linux_sandbox_command_args_for_policies( - command: Vec, - command_cwd: &Path, - sandbox_policy: &SandboxPolicy, - file_system_sandbox_policy: &FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, - sandbox_policy_cwd: &Path, - use_legacy_landlock: bool, - allow_network_for_proxy: bool, -) -> Vec { - let sandbox_policy_json = serde_json::to_string(sandbox_policy) - .unwrap_or_else(|err| panic!("failed to serialize sandbox policy: {err}")); - let file_system_policy_json = serde_json::to_string(file_system_sandbox_policy) - .unwrap_or_else(|err| panic!("failed to serialize filesystem sandbox policy: {err}")); - let network_policy_json = serde_json::to_string(&network_sandbox_policy) - .unwrap_or_else(|err| panic!("failed to serialize network sandbox policy: {err}")); - let sandbox_policy_cwd = sandbox_policy_cwd - .to_str() - .unwrap_or_else(|| panic!("cwd must be valid UTF-8")) - .to_string(); - let command_cwd = command_cwd - .to_str() - .unwrap_or_else(|| panic!("command cwd must be valid UTF-8")) - .to_string(); - - let mut linux_cmd: Vec = vec![ - "--sandbox-policy-cwd".to_string(), - sandbox_policy_cwd, - "--command-cwd".to_string(), - command_cwd, - "--sandbox-policy".to_string(), - sandbox_policy_json, - "--file-system-sandbox-policy".to_string(), - file_system_policy_json, - "--network-sandbox-policy".to_string(), - network_policy_json, - ]; - if use_legacy_landlock { - linux_cmd.push("--use-legacy-landlock".to_string()); - } - if allow_network_for_proxy { - linux_cmd.push("--allow-network-for-proxy".to_string()); - } - linux_cmd.push("--".to_string()); - linux_cmd.extend(command); - linux_cmd -} - -/// Converts the sandbox cwd and execution options into the CLI invocation for -/// `codex-linux-sandbox`. -#[cfg(test)] -pub(crate) fn create_linux_sandbox_command_args( - command: Vec, - command_cwd: &Path, - sandbox_policy_cwd: &Path, - use_legacy_landlock: bool, - allow_network_for_proxy: bool, -) -> Vec { - let command_cwd = command_cwd - .to_str() - .unwrap_or_else(|| panic!("command cwd must be valid UTF-8")) - .to_string(); - let sandbox_policy_cwd = sandbox_policy_cwd - .to_str() - .unwrap_or_else(|| panic!("cwd must be valid UTF-8")) - .to_string(); - - let mut linux_cmd: Vec = vec![ - "--sandbox-policy-cwd".to_string(), - sandbox_policy_cwd, - "--command-cwd".to_string(), - command_cwd, - ]; - if use_legacy_landlock { - linux_cmd.push("--use-legacy-landlock".to_string()); - } - if allow_network_for_proxy { - linux_cmd.push("--allow-network-for-proxy".to_string()); - } - - // Separator so that command arguments starting with `-` are not parsed as - // options of the helper itself. - linux_cmd.push("--".to_string()); - - // Append the original tool command. - linux_cmd.extend(command); - - linux_cmd -} - -#[cfg(test)] -#[path = "landlock_tests.rs"] -mod tests; diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 277ff2b241..fd055d4de1 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -14,8 +14,6 @@ use crate::exec::ExecToolCallOutput; use crate::exec::SandboxType; use crate::exec::StdoutStream; use crate::exec::execute_exec_request; -use crate::landlock::allow_network_for_proxy; -use crate::landlock::create_linux_sandbox_command_args_for_policies; use crate::protocol::SandboxPolicy; #[cfg(target_os = "macos")] use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; @@ -40,6 +38,8 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::NetworkAccess; use codex_protocol::protocol::ReadOnlyAccess; +use codex_sandboxing::landlock::allow_network_for_proxy; +use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; use codex_utils_absolute_path::AbsolutePathBuf; use dunce::canonicalize; use macos_permissions::intersect_macos_seatbelt_profile_extensions; diff --git a/codex-rs/sandboxing/BUILD.bazel b/codex-rs/sandboxing/BUILD.bazel new file mode 100644 index 0000000000..b4b3377a79 --- /dev/null +++ b/codex-rs/sandboxing/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "sandboxing", + crate_name = "codex_sandboxing", +) diff --git a/codex-rs/sandboxing/Cargo.toml b/codex-rs/sandboxing/Cargo.toml new file mode 100644 index 0000000000..dc6f4b122a --- /dev/null +++ b/codex-rs/sandboxing/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "codex-sandboxing" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lib] +name = "codex_sandboxing" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-protocol = { workspace = true } +serde_json = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } diff --git a/codex-rs/sandboxing/src/landlock.rs b/codex-rs/sandboxing/src/landlock.rs new file mode 100644 index 0000000000..8636f9ed04 --- /dev/null +++ b/codex-rs/sandboxing/src/landlock.rs @@ -0,0 +1,112 @@ +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::SandboxPolicy; +use std::path::Path; +pub fn allow_network_for_proxy(enforce_managed_network: bool) -> bool { + // When managed network requirements are active, request proxy-only + // networking from the Linux sandbox helper. Without managed requirements, + // preserve existing behavior. + enforce_managed_network +} + +/// Converts the sandbox policies into the CLI invocation for +/// `codex-linux-sandbox`. +/// +/// The helper performs the actual sandboxing (bubblewrap by default + seccomp) after +/// parsing these arguments. Policy JSON flags are emitted before helper feature +/// flags so the argv order matches the helper's CLI shape. See +/// `docs/linux_sandbox.md` for the Linux semantics. +#[allow(clippy::too_many_arguments)] +pub fn create_linux_sandbox_command_args_for_policies( + command: Vec, + command_cwd: &Path, + sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + sandbox_policy_cwd: &Path, + use_legacy_landlock: bool, + allow_network_for_proxy: bool, +) -> Vec { + let sandbox_policy_json = serde_json::to_string(sandbox_policy) + .unwrap_or_else(|err| panic!("failed to serialize sandbox policy: {err}")); + let file_system_policy_json = serde_json::to_string(file_system_sandbox_policy) + .unwrap_or_else(|err| panic!("failed to serialize filesystem sandbox policy: {err}")); + let network_policy_json = serde_json::to_string(&network_sandbox_policy) + .unwrap_or_else(|err| panic!("failed to serialize network sandbox policy: {err}")); + let sandbox_policy_cwd = sandbox_policy_cwd + .to_str() + .unwrap_or_else(|| panic!("cwd must be valid UTF-8")) + .to_string(); + let command_cwd = command_cwd + .to_str() + .unwrap_or_else(|| panic!("command cwd must be valid UTF-8")) + .to_string(); + + let mut linux_cmd: Vec = vec![ + "--sandbox-policy-cwd".to_string(), + sandbox_policy_cwd, + "--command-cwd".to_string(), + command_cwd, + "--sandbox-policy".to_string(), + sandbox_policy_json, + "--file-system-sandbox-policy".to_string(), + file_system_policy_json, + "--network-sandbox-policy".to_string(), + network_policy_json, + ]; + if use_legacy_landlock { + linux_cmd.push("--use-legacy-landlock".to_string()); + } + if allow_network_for_proxy { + linux_cmd.push("--allow-network-for-proxy".to_string()); + } + linux_cmd.push("--".to_string()); + linux_cmd.extend(command); + linux_cmd +} + +/// Converts the sandbox cwd and execution options into the CLI invocation for +/// `codex-linux-sandbox`. +#[cfg_attr(not(test), allow(dead_code))] +fn create_linux_sandbox_command_args( + command: Vec, + command_cwd: &Path, + sandbox_policy_cwd: &Path, + use_legacy_landlock: bool, + allow_network_for_proxy: bool, +) -> Vec { + let command_cwd = command_cwd + .to_str() + .unwrap_or_else(|| panic!("command cwd must be valid UTF-8")) + .to_string(); + let sandbox_policy_cwd = sandbox_policy_cwd + .to_str() + .unwrap_or_else(|| panic!("cwd must be valid UTF-8")) + .to_string(); + + let mut linux_cmd: Vec = vec![ + "--sandbox-policy-cwd".to_string(), + sandbox_policy_cwd, + "--command-cwd".to_string(), + command_cwd, + ]; + if use_legacy_landlock { + linux_cmd.push("--use-legacy-landlock".to_string()); + } + if allow_network_for_proxy { + linux_cmd.push("--allow-network-for-proxy".to_string()); + } + + // Separator so that command arguments starting with `-` are not parsed as + // options of the helper itself. + linux_cmd.push("--".to_string()); + + // Append the original tool command. + linux_cmd.extend(command); + + linux_cmd +} + +#[cfg(test)] +#[path = "landlock_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/landlock_tests.rs b/codex-rs/sandboxing/src/landlock_tests.rs similarity index 100% rename from codex-rs/core/src/landlock_tests.rs rename to codex-rs/sandboxing/src/landlock_tests.rs diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs new file mode 100644 index 0000000000..517579e280 --- /dev/null +++ b/codex-rs/sandboxing/src/lib.rs @@ -0,0 +1 @@ +pub mod landlock; From 431af0807ce18b66643c28e1c9cd1d5d8f1e4a8e Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Mon, 23 Mar 2026 21:26:35 -0700 Subject: [PATCH 08/55] Move macOS sandbox builders into codex-sandboxing (#15593) ## Summary - move macOS permission merging/intersection logic and tests from `codex-core` into `codex-sandboxing` - move seatbelt policy builders, permissions logic, SBPL assets, and their tests into `codex-sandboxing` - keep `codex-core` owning only the seatbelt spawn wrapper and switch call sites to import the moved APIs directly ## Notes - no re-exports added - moved the seatbelt tests with the implementation so internal helpers could stay private - local verification is still finishing while this PR is open --- codex-rs/Cargo.lock | 7 + codex-rs/Cargo.toml | 2 +- codex-rs/cli/src/debug_sandbox.rs | 4 +- codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/sandboxing/mod.rs | 14 +- codex-rs/core/src/seatbelt.rs | 567 +----------------- .../runtimes/shell/unix_escalation_tests.rs | 4 +- codex-rs/sandboxing/BUILD.bazel | 5 + codex-rs/sandboxing/Cargo.toml | 7 + codex-rs/sandboxing/src/lib.rs | 5 + .../src}/macos_permissions.rs | 4 +- .../src}/macos_permissions_tests.rs | 0 ...estricted_read_only_platform_defaults.sbpl | 0 codex-rs/sandboxing/src/seatbelt.rs | 540 +++++++++++++++++ .../src/seatbelt_base_policy.sbpl | 0 .../src/seatbelt_network_policy.sbpl | 0 .../src/seatbelt_permissions.rs | 10 +- .../src/seatbelt_permissions_tests.rs | 0 .../src/seatbelt_tests.rs | 81 ++- 19 files changed, 650 insertions(+), 601 deletions(-) rename codex-rs/{core/src/sandboxing => sandboxing/src}/macos_permissions.rs (98%) rename codex-rs/{core/src/sandboxing => sandboxing/src}/macos_permissions_tests.rs (100%) rename codex-rs/{core => sandboxing}/src/restricted_read_only_platform_defaults.sbpl (100%) create mode 100644 codex-rs/sandboxing/src/seatbelt.rs rename codex-rs/{core => sandboxing}/src/seatbelt_base_policy.sbpl (100%) rename codex-rs/{core => sandboxing}/src/seatbelt_network_policy.sbpl (100%) rename codex-rs/{core => sandboxing}/src/seatbelt_permissions.rs (96%) rename codex-rs/{core => sandboxing}/src/seatbelt_permissions_tests.rs (100%) rename codex-rs/{core => sandboxing}/src/seatbelt_tests.rs (94%) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index f9a523dd9e..c14c633c43 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2468,9 +2468,16 @@ dependencies = [ name = "codex-sandboxing" version = "0.0.0" dependencies = [ + "codex-network-proxy", "codex-protocol", + "codex-utils-absolute-path", + "dirs", + "libc", "pretty_assertions", "serde_json", + "tempfile", + "tracing", + "url", ] [[package]] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index db203d83b5..e0c7633733 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -290,7 +290,7 @@ supports-color = "3.0.2" syntect = "5" sys-locale = "0.3.2" tempfile = "3.23.0" -tar = "0.4.44" +tar = "0.4.45" test-log = "0.2.19" textwrap = "0.16.2" thiserror = "2.0.17" diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 71a2661017..03226cced4 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -12,13 +12,13 @@ use codex_core::config::ConfigOverrides; use codex_core::config::NetworkProxyAuditMetadata; use codex_core::exec_env::create_env; #[cfg(target_os = "macos")] -use codex_core::seatbelt::create_seatbelt_command_args_for_policies_with_extensions; -#[cfg(target_os = "macos")] use codex_core::spawn::CODEX_SANDBOX_ENV_VAR; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_protocol::config_types::SandboxMode; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; +#[cfg(target_os = "macos")] +use codex_sandboxing::seatbelt::create_seatbelt_command_args_for_policies_with_extensions; use codex_utils_cli::CliConfigOverrides; use tokio::process::Child; use tokio::process::Command as TokioCommand; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e51bf45076..cd6b51b7e9 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -93,7 +93,6 @@ mod event_mapping; mod response_debug_context; pub mod review_format; pub mod review_prompts; -mod seatbelt_permissions; mod thread_manager; pub mod web_search; pub mod windows_sandbox_read_grants; diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index fd055d4de1..e9cf082ca4 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -6,8 +6,6 @@ sandbox placement and transformation of portable CommandSpec into a ready‑to‑spawn environment. */ -pub(crate) mod macos_permissions; - use crate::exec::ExecCapturePolicy; use crate::exec::ExecExpiration; use crate::exec::ExecToolCallOutput; @@ -16,10 +14,6 @@ use crate::exec::StdoutStream; use crate::exec::execute_exec_request; use crate::protocol::SandboxPolicy; #[cfg(target_os = "macos")] -use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; -#[cfg(target_os = "macos")] -use crate::seatbelt::create_seatbelt_command_args_for_policies_with_extensions; -#[cfg(target_os = "macos")] use crate::spawn::CODEX_SANDBOX_ENV_VAR; use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use crate::tools::sandboxing::SandboxablePreference; @@ -40,10 +34,14 @@ use codex_protocol::protocol::NetworkAccess; use codex_protocol::protocol::ReadOnlyAccess; use codex_sandboxing::landlock::allow_network_for_proxy; use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; +use codex_sandboxing::macos_permissions::intersect_macos_seatbelt_profile_extensions; +use codex_sandboxing::macos_permissions::merge_macos_seatbelt_profile_extensions; +#[cfg(target_os = "macos")] +use codex_sandboxing::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; +#[cfg(target_os = "macos")] +use codex_sandboxing::seatbelt::create_seatbelt_command_args_for_policies_with_extensions; use codex_utils_absolute_path::AbsolutePathBuf; use dunce::canonicalize; -use macos_permissions::intersect_macos_seatbelt_profile_extensions; -use macos_permissions::merge_macos_seatbelt_profile_extensions; use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 2d1e0a7b5d..f81016a990 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -1,40 +1,19 @@ #![cfg(target_os = "macos")] -use codex_network_proxy::NetworkProxy; -use codex_network_proxy::PROXY_URL_ENV_KEYS; -use codex_network_proxy::has_proxy_url_env_vars; -use codex_network_proxy::proxy_url_env_value; -use codex_utils_absolute_path::AbsolutePathBuf; -use std::collections::BTreeMap; -use std::collections::BTreeSet; -use std::collections::HashMap; -use std::ffi::CStr; -use std::path::Path; -use std::path::PathBuf; -use tokio::process::Child; -use tracing::warn; -use url::Url; - use crate::protocol::SandboxPolicy; -use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; -use crate::seatbelt_permissions::build_seatbelt_extensions; use crate::spawn::CODEX_SANDBOX_ENV_VAR; use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; +use codex_network_proxy::NetworkProxy; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; - -const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl"); -const MACOS_SEATBELT_NETWORK_POLICY: &str = include_str!("seatbelt_network_policy.sbpl"); -const MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS: &str = - include_str!("restricted_read_only_platform_defaults.sbpl"); - -/// When working with `sandbox-exec`, only consider `sandbox-exec` in `/usr/bin` -/// to defend against an attacker trying to inject a malicious version on the -/// PATH. If /usr/bin/sandbox-exec has been tampered with, then the attacker -/// already has root access. -pub const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec"; +use codex_sandboxing::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; +use codex_sandboxing::seatbelt::create_seatbelt_command_args_for_policies_with_extensions; +use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; +use tokio::process::Child; pub async fn spawn_command_under_seatbelt( command: Vec, @@ -45,12 +24,14 @@ pub async fn spawn_command_under_seatbelt( network: Option<&NetworkProxy>, mut env: HashMap, ) -> std::io::Result { - let args = create_seatbelt_command_args( + let args = create_seatbelt_command_args_for_policies_with_extensions( command, - sandbox_policy, + &FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, sandbox_policy_cwd), + NetworkSandboxPolicy::from(sandbox_policy), sandbox_policy_cwd, /*enforce_managed_network*/ false, network, + /*extensions*/ None, ); let arg0 = None; env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); @@ -66,529 +47,3 @@ pub async fn spawn_command_under_seatbelt( }) .await } - -fn is_loopback_host(host: &str) -> bool { - host.eq_ignore_ascii_case("localhost") || host == "127.0.0.1" || host == "::1" -} - -fn proxy_scheme_default_port(scheme: &str) -> u16 { - match scheme { - "https" => 443, - "socks5" | "socks5h" | "socks4" | "socks4a" => 1080, - _ => 80, - } -} - -fn proxy_loopback_ports_from_env(env: &HashMap) -> Vec { - let mut ports = BTreeSet::new(); - for key in PROXY_URL_ENV_KEYS { - let Some(proxy_url) = proxy_url_env_value(env, key) else { - continue; - }; - let trimmed = proxy_url.trim(); - if trimmed.is_empty() { - continue; - } - - let candidate = if trimmed.contains("://") { - trimmed.to_string() - } else { - format!("http://{trimmed}") - }; - let Ok(parsed) = Url::parse(&candidate) else { - continue; - }; - let Some(host) = parsed.host_str() else { - continue; - }; - if !is_loopback_host(host) { - continue; - } - - let scheme = parsed.scheme().to_ascii_lowercase(); - let port = parsed - .port() - .unwrap_or_else(|| proxy_scheme_default_port(scheme.as_str())); - ports.insert(port); - } - ports.into_iter().collect() -} - -#[derive(Debug, Default)] -struct ProxyPolicyInputs { - ports: Vec, - has_proxy_config: bool, - allow_local_binding: bool, - unix_domain_socket_policy: UnixDomainSocketPolicy, -} - -#[derive(Debug, Clone)] -// Keep allow-all and allowlist modes disjoint so we don't carry ignored state. -enum UnixDomainSocketPolicy { - AllowAll, - Restricted { allowed: Vec }, -} - -impl Default for UnixDomainSocketPolicy { - fn default() -> Self { - Self::Restricted { allowed: vec![] } - } -} - -#[derive(Debug, Clone)] -struct UnixSocketPathParam { - index: usize, - path: AbsolutePathBuf, -} - -fn proxy_policy_inputs(network: Option<&NetworkProxy>) -> ProxyPolicyInputs { - if let Some(network) = network { - let mut env = HashMap::new(); - network.apply_to_env(&mut env); - let unix_domain_socket_policy = if network.dangerously_allow_all_unix_sockets() { - UnixDomainSocketPolicy::AllowAll - } else { - let allowed = network - .allow_unix_sockets() - .iter() - .filter_map( - |socket_path| match normalize_path_for_sandbox(Path::new(socket_path)) { - Some(path) => Some((path.to_string_lossy().to_string(), path)), - None => { - warn!( - "ignoring network.allow_unix_sockets entry because it could not be normalized: {socket_path}" - ); - None - } - }, - ) - .collect::>() - .into_values() - .collect(); - UnixDomainSocketPolicy::Restricted { allowed } - }; - return ProxyPolicyInputs { - ports: proxy_loopback_ports_from_env(&env), - has_proxy_config: has_proxy_url_env_vars(&env), - allow_local_binding: network.allow_local_binding(), - unix_domain_socket_policy, - }; - } - - ProxyPolicyInputs::default() -} - -fn normalize_path_for_sandbox(path: &Path) -> Option { - // `AbsolutePathBuf::from_absolute_path()` normalizes relative paths against the current - // working directory, so keep the explicit check to avoid silently accepting relative entries. - if !path.is_absolute() { - return None; - } - - let absolute_path = AbsolutePathBuf::from_absolute_path(path).ok()?; - let normalized_path = absolute_path - .as_path() - .canonicalize() - .ok() - .and_then(|canonical_path| AbsolutePathBuf::from_absolute_path(canonical_path).ok()); - normalized_path.or(Some(absolute_path)) -} - -fn unix_socket_path_params(proxy: &ProxyPolicyInputs) -> Vec { - let mut deduped_paths: BTreeMap = BTreeMap::new(); - let UnixDomainSocketPolicy::Restricted { allowed } = &proxy.unix_domain_socket_policy else { - return vec![]; - }; - for path in allowed { - deduped_paths - .entry(path.to_string_lossy().to_string()) - .or_insert_with(|| path.clone()); - } - - deduped_paths - .into_values() - .enumerate() - .map(|(index, path)| UnixSocketPathParam { index, path }) - .collect() -} - -fn unix_socket_path_param_key(index: usize) -> String { - format!("UNIX_SOCKET_PATH_{index}") -} - -fn unix_socket_dir_params(proxy: &ProxyPolicyInputs) -> Vec<(String, PathBuf)> { - unix_socket_path_params(proxy) - .into_iter() - .map(|param| { - ( - unix_socket_path_param_key(param.index), - param.path.into_path_buf(), - ) - }) - .collect() -} - -/// Returns zero or more complete Seatbelt policy lines for unix socket rules. -/// When non-empty, the returned string is newline-terminated so callers can -/// append it directly to larger policy blocks. -fn unix_socket_policy(proxy: &ProxyPolicyInputs) -> String { - let socket_params = unix_socket_path_params(proxy); - let has_unix_socket_access = matches!( - proxy.unix_domain_socket_policy, - UnixDomainSocketPolicy::AllowAll - ) || !socket_params.is_empty(); - if !has_unix_socket_access { - return String::new(); - } - - let mut policy = String::new(); - policy.push_str("(allow system-socket (socket-domain AF_UNIX))\n"); - if matches!( - proxy.unix_domain_socket_policy, - UnixDomainSocketPolicy::AllowAll - ) { - // Keep AllowAll genuinely broad here; path qualifiers look narrower - // without a clear macOS behavioral benefit. - policy.push_str("(allow network-bind (local unix-socket))\n"); - policy.push_str("(allow network-outbound (remote unix-socket))\n"); - return policy; - } - - for param in socket_params { - let key = unix_socket_path_param_key(param.index); - // Use subpath so allowlists cover sockets created beneath approved directories. - policy.push_str(&format!( - "(allow network-bind (local unix-socket (subpath (param \"{key}\"))))\n" - )); - policy.push_str(&format!( - "(allow network-outbound (remote unix-socket (subpath (param \"{key}\"))))\n" - )); - } - policy -} - -#[cfg_attr(not(test), allow(dead_code))] -fn dynamic_network_policy( - sandbox_policy: &SandboxPolicy, - enforce_managed_network: bool, - proxy: &ProxyPolicyInputs, -) -> String { - dynamic_network_policy_for_network( - NetworkSandboxPolicy::from(sandbox_policy), - enforce_managed_network, - proxy, - ) -} - -fn dynamic_network_policy_for_network( - network_policy: NetworkSandboxPolicy, - enforce_managed_network: bool, - proxy: &ProxyPolicyInputs, -) -> String { - let should_use_restricted_network_policy = - !proxy.ports.is_empty() || proxy.has_proxy_config || enforce_managed_network; - if should_use_restricted_network_policy { - let mut policy = String::new(); - if proxy.allow_local_binding { - policy.push_str("; allow loopback local binding and loopback traffic\n"); - policy.push_str("(allow network-bind (local ip \"localhost:*\"))\n"); - policy.push_str("(allow network-inbound (local ip \"localhost:*\"))\n"); - policy.push_str("(allow network-outbound (remote ip \"localhost:*\"))\n"); - } - for port in &proxy.ports { - policy.push_str(&format!( - "(allow network-outbound (remote ip \"localhost:{port}\"))\n" - )); - } - let unix_socket_policy = unix_socket_policy(proxy); - if !unix_socket_policy.is_empty() { - policy.push_str("; allow unix domain sockets for local IPC\n"); - policy.push_str(&unix_socket_policy); - } - return format!("{policy}{MACOS_SEATBELT_NETWORK_POLICY}"); - } - - if proxy.has_proxy_config { - // Proxy configuration is present but we could not infer any valid loopback endpoints. - // Fail closed to avoid silently widening network access in proxy-enforced sessions. - return String::new(); - } - - if enforce_managed_network { - // Managed network requirements are active but no usable proxy endpoints - // are available. Fail closed for network access. - return String::new(); - } - - if network_policy.is_enabled() { - // No proxy env is configured: retain the existing full-network behavior. - format!( - "(allow network-outbound)\n(allow network-inbound)\n{MACOS_SEATBELT_NETWORK_POLICY}" - ) - } else { - String::new() - } -} - -#[cfg_attr(not(test), allow(dead_code))] -pub(crate) fn create_seatbelt_command_args( - command: Vec, - sandbox_policy: &SandboxPolicy, - sandbox_policy_cwd: &Path, - enforce_managed_network: bool, - network: Option<&NetworkProxy>, -) -> Vec { - create_seatbelt_command_args_with_extensions( - command, - sandbox_policy, - sandbox_policy_cwd, - enforce_managed_network, - network, - /*extensions*/ None, - ) -} - -fn root_absolute_path() -> AbsolutePathBuf { - match AbsolutePathBuf::from_absolute_path(Path::new("/")) { - Ok(path) => path, - Err(err) => panic!("root path must be absolute: {err}"), - } -} - -#[derive(Debug, Clone)] -struct SeatbeltAccessRoot { - root: AbsolutePathBuf, - excluded_subpaths: Vec, -} - -fn build_seatbelt_access_policy( - action: &str, - param_prefix: &str, - roots: Vec, -) -> (String, Vec<(String, PathBuf)>) { - let mut policy_components = Vec::new(); - let mut params = Vec::new(); - - for (index, access_root) in roots.into_iter().enumerate() { - let root = - normalize_path_for_sandbox(access_root.root.as_path()).unwrap_or(access_root.root); - let root_param = format!("{param_prefix}_{index}"); - params.push((root_param.clone(), root.into_path_buf())); - - if access_root.excluded_subpaths.is_empty() { - policy_components.push(format!("(subpath (param \"{root_param}\"))")); - continue; - } - - let mut require_parts = vec![format!("(subpath (param \"{root_param}\"))")]; - for (excluded_index, excluded_subpath) in - access_root.excluded_subpaths.into_iter().enumerate() - { - let excluded_subpath = - normalize_path_for_sandbox(excluded_subpath.as_path()).unwrap_or(excluded_subpath); - let excluded_param = format!("{param_prefix}_{index}_RO_{excluded_index}"); - params.push((excluded_param.clone(), excluded_subpath.into_path_buf())); - require_parts.push(format!( - "(require-not (subpath (param \"{excluded_param}\")))" - )); - } - policy_components.push(format!("(require-all {} )", require_parts.join(" "))); - } - - if policy_components.is_empty() { - (String::new(), Vec::new()) - } else { - ( - format!("(allow {action}\n{}\n)", policy_components.join(" ")), - params, - ) - } -} - -#[cfg_attr(not(test), allow(dead_code))] -pub(crate) fn create_seatbelt_command_args_with_extensions( - command: Vec, - sandbox_policy: &SandboxPolicy, - sandbox_policy_cwd: &Path, - enforce_managed_network: bool, - network: Option<&NetworkProxy>, - extensions: Option<&MacOsSeatbeltProfileExtensions>, -) -> Vec { - create_seatbelt_command_args_for_policies_with_extensions( - command, - &FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, sandbox_policy_cwd), - NetworkSandboxPolicy::from(sandbox_policy), - sandbox_policy_cwd, - enforce_managed_network, - network, - extensions, - ) -} - -pub fn create_seatbelt_command_args_for_policies_with_extensions( - command: Vec, - file_system_sandbox_policy: &FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, - sandbox_policy_cwd: &Path, - enforce_managed_network: bool, - network: Option<&NetworkProxy>, - extensions: Option<&MacOsSeatbeltProfileExtensions>, -) -> Vec { - let unreadable_roots = - file_system_sandbox_policy.get_unreadable_roots_with_cwd(sandbox_policy_cwd); - let (file_write_policy, file_write_dir_params) = - if file_system_sandbox_policy.has_full_disk_write_access() { - if unreadable_roots.is_empty() { - // Allegedly, this is more permissive than `(allow file-write*)`. - ( - r#"(allow file-write* (regex #"^/"))"#.to_string(), - Vec::new(), - ) - } else { - build_seatbelt_access_policy( - "file-write*", - "WRITABLE_ROOT", - vec![SeatbeltAccessRoot { - root: root_absolute_path(), - excluded_subpaths: unreadable_roots.clone(), - }], - ) - } - } else { - build_seatbelt_access_policy( - "file-write*", - "WRITABLE_ROOT", - file_system_sandbox_policy - .get_writable_roots_with_cwd(sandbox_policy_cwd) - .into_iter() - .map(|root| SeatbeltAccessRoot { - root: root.root, - excluded_subpaths: root.read_only_subpaths, - }) - .collect(), - ) - }; - - let (file_read_policy, file_read_dir_params) = - if file_system_sandbox_policy.has_full_disk_read_access() { - if unreadable_roots.is_empty() { - ( - "; allow read-only file operations\n(allow file-read*)".to_string(), - Vec::new(), - ) - } else { - let (policy, params) = build_seatbelt_access_policy( - "file-read*", - "READABLE_ROOT", - vec![SeatbeltAccessRoot { - root: root_absolute_path(), - excluded_subpaths: unreadable_roots, - }], - ); - ( - format!("; allow read-only file operations\n{policy}"), - params, - ) - } - } else { - let (policy, params) = build_seatbelt_access_policy( - "file-read*", - "READABLE_ROOT", - file_system_sandbox_policy - .get_readable_roots_with_cwd(sandbox_policy_cwd) - .into_iter() - .map(|root| SeatbeltAccessRoot { - excluded_subpaths: unreadable_roots - .iter() - .filter(|path| path.as_path().starts_with(root.as_path())) - .cloned() - .collect(), - root, - }) - .collect(), - ); - if policy.is_empty() { - (String::new(), params) - } else { - ( - format!("; allow read-only file operations\n{policy}"), - params, - ) - } - }; - - let proxy = proxy_policy_inputs(network); - let network_policy = - dynamic_network_policy_for_network(network_sandbox_policy, enforce_managed_network, &proxy); - let seatbelt_extensions = extensions.map_or_else( - || { - // Backward-compatibility default when no extension profile is provided. - build_seatbelt_extensions(&MacOsSeatbeltProfileExtensions::default()) - }, - build_seatbelt_extensions, - ); - - let include_platform_defaults = file_system_sandbox_policy.include_platform_defaults(); - let mut policy_sections = vec![ - MACOS_SEATBELT_BASE_POLICY.to_string(), - file_read_policy, - file_write_policy, - network_policy, - ]; - if include_platform_defaults { - policy_sections.push(MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS.to_string()); - } - if !seatbelt_extensions.policy.is_empty() { - policy_sections.push(seatbelt_extensions.policy.clone()); - } - - let full_policy = policy_sections.join("\n"); - - let dir_params = [ - file_read_dir_params, - file_write_dir_params, - macos_dir_params(), - unix_socket_dir_params(&proxy), - seatbelt_extensions.dir_params, - ] - .concat(); - - let mut seatbelt_args: Vec = vec!["-p".to_string(), full_policy]; - let definition_args = dir_params - .into_iter() - .map(|(key, value)| format!("-D{key}={value}", value = value.to_string_lossy())); - seatbelt_args.extend(definition_args); - seatbelt_args.push("--".to_string()); - seatbelt_args.extend(command); - seatbelt_args -} - -/// Wraps libc::confstr to return a String. -fn confstr(name: libc::c_int) -> Option { - let mut buf = vec![0_i8; (libc::PATH_MAX as usize) + 1]; - let len = unsafe { libc::confstr(name, buf.as_mut_ptr(), buf.len()) }; - if len == 0 { - return None; - } - // confstr guarantees NUL-termination when len > 0. - let cstr = unsafe { CStr::from_ptr(buf.as_ptr()) }; - cstr.to_str().ok().map(ToString::to_string) -} - -/// Wraps confstr to return a canonicalized PathBuf. -fn confstr_path(name: libc::c_int) -> Option { - let s = confstr(name)?; - let path = PathBuf::from(s); - path.canonicalize().ok().or(Some(path)) -} - -fn macos_dir_params() -> Vec<(String, PathBuf)> { - if let Some(p) = confstr_path(libc::_CS_DARWIN_USER_CACHE_DIR) { - return vec![("DARWIN_USER_CACHE_DIR".to_string(), p)]; - } - vec![] -} - -#[cfg(test)] -#[path = "seatbelt_tests.rs"] -mod tests; diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 37c1b53a13..3adee9c295 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -20,8 +20,6 @@ use crate::protocol::GranularApprovalConfig; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; use crate::sandboxing::SandboxPermissions; -#[cfg(target_os = "macos")] -use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; use crate::skills::SkillMetadata; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; @@ -40,6 +38,8 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SkillScope; +#[cfg(target_os = "macos")] +use codex_sandboxing::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; use codex_shell_escalation::EscalationExecution; use codex_shell_escalation::EscalationPermissions; use codex_shell_escalation::ExecResult; diff --git a/codex-rs/sandboxing/BUILD.bazel b/codex-rs/sandboxing/BUILD.bazel index b4b3377a79..57837fa809 100644 --- a/codex-rs/sandboxing/BUILD.bazel +++ b/codex-rs/sandboxing/BUILD.bazel @@ -3,4 +3,9 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "sandboxing", crate_name = "codex_sandboxing", + compile_data = [ + "src/restricted_read_only_platform_defaults.sbpl", + "src/seatbelt_base_policy.sbpl", + "src/seatbelt_network_policy.sbpl", + ], ) diff --git a/codex-rs/sandboxing/Cargo.toml b/codex-rs/sandboxing/Cargo.toml index dc6f4b122a..0e2ab443a0 100644 --- a/codex-rs/sandboxing/Cargo.toml +++ b/codex-rs/sandboxing/Cargo.toml @@ -12,8 +12,15 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } +dirs = { workspace = true } +libc = { workspace = true } serde_json = { workspace = true } +tracing = { workspace = true, features = ["log"] } +url = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index 517579e280..17291ec2ae 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -1 +1,6 @@ pub mod landlock; +pub mod macos_permissions; +#[cfg(target_os = "macos")] +pub mod seatbelt; +#[cfg(target_os = "macos")] +mod seatbelt_permissions; diff --git a/codex-rs/core/src/sandboxing/macos_permissions.rs b/codex-rs/sandboxing/src/macos_permissions.rs similarity index 98% rename from codex-rs/core/src/sandboxing/macos_permissions.rs rename to codex-rs/sandboxing/src/macos_permissions.rs index 1a409d4bdf..22314937ef 100644 --- a/codex-rs/core/src/sandboxing/macos_permissions.rs +++ b/codex-rs/sandboxing/src/macos_permissions.rs @@ -7,7 +7,7 @@ use codex_protocol::models::MacOsSeatbeltProfileExtensions; /// Merges macOS seatbelt profile extensions by taking the permissive union of /// each permission field. -pub(crate) fn merge_macos_seatbelt_profile_extensions( +pub fn merge_macos_seatbelt_profile_extensions( base: Option<&MacOsSeatbeltProfileExtensions>, permissions: Option<&MacOsSeatbeltProfileExtensions>, ) -> Option { @@ -38,7 +38,7 @@ pub(crate) fn merge_macos_seatbelt_profile_extensions( } } -pub(crate) fn intersect_macos_seatbelt_profile_extensions( +pub fn intersect_macos_seatbelt_profile_extensions( requested: Option, granted: Option, ) -> Option { diff --git a/codex-rs/core/src/sandboxing/macos_permissions_tests.rs b/codex-rs/sandboxing/src/macos_permissions_tests.rs similarity index 100% rename from codex-rs/core/src/sandboxing/macos_permissions_tests.rs rename to codex-rs/sandboxing/src/macos_permissions_tests.rs diff --git a/codex-rs/core/src/restricted_read_only_platform_defaults.sbpl b/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl similarity index 100% rename from codex-rs/core/src/restricted_read_only_platform_defaults.sbpl rename to codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs new file mode 100644 index 0000000000..e7cca3d2ac --- /dev/null +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -0,0 +1,540 @@ +use codex_network_proxy::NetworkProxy; +use codex_network_proxy::PROXY_URL_ENV_KEYS; +use codex_network_proxy::has_proxy_url_env_vars; +use codex_network_proxy::proxy_url_env_value; +use codex_protocol::models::MacOsSeatbeltProfileExtensions; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::SandboxPolicy; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::collections::HashMap; +use std::ffi::CStr; +use std::path::Path; +use std::path::PathBuf; +use tracing::warn; +use url::Url; + +use crate::seatbelt_permissions::build_seatbelt_extensions; + +const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl"); +const MACOS_SEATBELT_NETWORK_POLICY: &str = include_str!("seatbelt_network_policy.sbpl"); +const MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS: &str = + include_str!("restricted_read_only_platform_defaults.sbpl"); + +/// When working with `sandbox-exec`, only consider `sandbox-exec` in `/usr/bin` +/// to defend against an attacker trying to inject a malicious version on the +/// PATH. If /usr/bin/sandbox-exec has been tampered with, then the attacker +/// already has root access. +pub const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec"; + +fn is_loopback_host(host: &str) -> bool { + host.eq_ignore_ascii_case("localhost") || host == "127.0.0.1" || host == "::1" +} + +fn proxy_scheme_default_port(scheme: &str) -> u16 { + match scheme { + "https" => 443, + "socks5" | "socks5h" | "socks4" | "socks4a" => 1080, + _ => 80, + } +} + +fn proxy_loopback_ports_from_env(env: &HashMap) -> Vec { + let mut ports = BTreeSet::new(); + for key in PROXY_URL_ENV_KEYS { + let Some(proxy_url) = proxy_url_env_value(env, key) else { + continue; + }; + let trimmed = proxy_url.trim(); + if trimmed.is_empty() { + continue; + } + + let candidate = if trimmed.contains("://") { + trimmed.to_string() + } else { + format!("http://{trimmed}") + }; + let Ok(parsed) = Url::parse(&candidate) else { + continue; + }; + let Some(host) = parsed.host_str() else { + continue; + }; + if !is_loopback_host(host) { + continue; + } + + let scheme = parsed.scheme().to_ascii_lowercase(); + let port = parsed + .port() + .unwrap_or_else(|| proxy_scheme_default_port(scheme.as_str())); + ports.insert(port); + } + ports.into_iter().collect() +} + +#[derive(Debug, Default)] +struct ProxyPolicyInputs { + ports: Vec, + has_proxy_config: bool, + allow_local_binding: bool, + unix_domain_socket_policy: UnixDomainSocketPolicy, +} + +#[derive(Debug, Clone)] +// Keep allow-all and allowlist modes disjoint so we don't carry ignored state. +enum UnixDomainSocketPolicy { + AllowAll, + Restricted { allowed: Vec }, +} + +impl Default for UnixDomainSocketPolicy { + fn default() -> Self { + Self::Restricted { allowed: vec![] } + } +} + +#[derive(Debug, Clone)] +struct UnixSocketPathParam { + index: usize, + path: AbsolutePathBuf, +} + +fn proxy_policy_inputs(network: Option<&NetworkProxy>) -> ProxyPolicyInputs { + if let Some(network) = network { + let mut env = HashMap::new(); + network.apply_to_env(&mut env); + let unix_domain_socket_policy = if network.dangerously_allow_all_unix_sockets() { + UnixDomainSocketPolicy::AllowAll + } else { + let allowed = network + .allow_unix_sockets() + .iter() + .filter_map( + |socket_path| match normalize_path_for_sandbox(Path::new(socket_path)) { + Some(path) => Some((path.to_string_lossy().to_string(), path)), + None => { + warn!( + "ignoring network.allow_unix_sockets entry because it could not be normalized: {socket_path}" + ); + None + } + }, + ) + .collect::>() + .into_values() + .collect(); + UnixDomainSocketPolicy::Restricted { allowed } + }; + return ProxyPolicyInputs { + ports: proxy_loopback_ports_from_env(&env), + has_proxy_config: has_proxy_url_env_vars(&env), + allow_local_binding: network.allow_local_binding(), + unix_domain_socket_policy, + }; + } + + ProxyPolicyInputs::default() +} + +fn normalize_path_for_sandbox(path: &Path) -> Option { + // `AbsolutePathBuf::from_absolute_path()` normalizes relative paths against the current + // working directory, so keep the explicit check to avoid silently accepting relative entries. + if !path.is_absolute() { + return None; + } + + let absolute_path = AbsolutePathBuf::from_absolute_path(path).ok()?; + let normalized_path = absolute_path + .as_path() + .canonicalize() + .ok() + .and_then(|canonical_path| AbsolutePathBuf::from_absolute_path(canonical_path).ok()); + normalized_path.or(Some(absolute_path)) +} + +fn unix_socket_path_params(proxy: &ProxyPolicyInputs) -> Vec { + let mut deduped_paths: BTreeMap = BTreeMap::new(); + let UnixDomainSocketPolicy::Restricted { allowed } = &proxy.unix_domain_socket_policy else { + return vec![]; + }; + for path in allowed { + deduped_paths + .entry(path.to_string_lossy().to_string()) + .or_insert_with(|| path.clone()); + } + + deduped_paths + .into_values() + .enumerate() + .map(|(index, path)| UnixSocketPathParam { index, path }) + .collect() +} + +fn unix_socket_path_param_key(index: usize) -> String { + format!("UNIX_SOCKET_PATH_{index}") +} + +fn unix_socket_dir_params(proxy: &ProxyPolicyInputs) -> Vec<(String, PathBuf)> { + unix_socket_path_params(proxy) + .into_iter() + .map(|param| { + ( + unix_socket_path_param_key(param.index), + param.path.into_path_buf(), + ) + }) + .collect() +} + +/// Returns zero or more complete Seatbelt policy lines for unix socket rules. +/// When non-empty, the returned string is newline-terminated so callers can +/// append it directly to larger policy blocks. +fn unix_socket_policy(proxy: &ProxyPolicyInputs) -> String { + let socket_params = unix_socket_path_params(proxy); + let has_unix_socket_access = matches!( + proxy.unix_domain_socket_policy, + UnixDomainSocketPolicy::AllowAll + ) || !socket_params.is_empty(); + if !has_unix_socket_access { + return String::new(); + } + + let mut policy = String::new(); + policy.push_str("(allow system-socket (socket-domain AF_UNIX))\n"); + if matches!( + proxy.unix_domain_socket_policy, + UnixDomainSocketPolicy::AllowAll + ) { + // Keep AllowAll genuinely broad here; path qualifiers look narrower + // without a clear macOS behavioral benefit. + policy.push_str("(allow network-bind (local unix-socket))\n"); + policy.push_str("(allow network-outbound (remote unix-socket))\n"); + return policy; + } + + for param in socket_params { + let key = unix_socket_path_param_key(param.index); + // Use subpath so allowlists cover sockets created beneath approved directories. + policy.push_str(&format!( + "(allow network-bind (local unix-socket (subpath (param \"{key}\"))))\n" + )); + policy.push_str(&format!( + "(allow network-outbound (remote unix-socket (subpath (param \"{key}\"))))\n" + )); + } + policy +} + +#[cfg_attr(not(test), allow(dead_code))] +fn dynamic_network_policy( + sandbox_policy: &SandboxPolicy, + enforce_managed_network: bool, + proxy: &ProxyPolicyInputs, +) -> String { + dynamic_network_policy_for_network( + NetworkSandboxPolicy::from(sandbox_policy), + enforce_managed_network, + proxy, + ) +} + +fn dynamic_network_policy_for_network( + network_policy: NetworkSandboxPolicy, + enforce_managed_network: bool, + proxy: &ProxyPolicyInputs, +) -> String { + let should_use_restricted_network_policy = + !proxy.ports.is_empty() || proxy.has_proxy_config || enforce_managed_network; + if should_use_restricted_network_policy { + let mut policy = String::new(); + if proxy.allow_local_binding { + policy.push_str("; allow loopback local binding and loopback traffic\n"); + policy.push_str("(allow network-bind (local ip \"localhost:*\"))\n"); + policy.push_str("(allow network-inbound (local ip \"localhost:*\"))\n"); + policy.push_str("(allow network-outbound (remote ip \"localhost:*\"))\n"); + } + for port in &proxy.ports { + policy.push_str(&format!( + "(allow network-outbound (remote ip \"localhost:{port}\"))\n" + )); + } + let unix_socket_policy = unix_socket_policy(proxy); + if !unix_socket_policy.is_empty() { + policy.push_str("; allow unix domain sockets for local IPC\n"); + policy.push_str(&unix_socket_policy); + } + return format!("{policy}{MACOS_SEATBELT_NETWORK_POLICY}"); + } + + if proxy.has_proxy_config { + // Proxy configuration is present but we could not infer any valid loopback endpoints. + // Fail closed to avoid silently widening network access in proxy-enforced sessions. + return String::new(); + } + + if enforce_managed_network { + // Managed network requirements are active but no usable proxy endpoints + // are available. Fail closed for network access. + return String::new(); + } + + if network_policy.is_enabled() { + // No proxy env is configured: retain the existing full-network behavior. + format!( + "(allow network-outbound)\n(allow network-inbound)\n{MACOS_SEATBELT_NETWORK_POLICY}" + ) + } else { + String::new() + } +} + +fn root_absolute_path() -> AbsolutePathBuf { + match AbsolutePathBuf::from_absolute_path(Path::new("/")) { + Ok(path) => path, + Err(err) => panic!("root path must be absolute: {err}"), + } +} + +#[derive(Debug, Clone)] +struct SeatbeltAccessRoot { + root: AbsolutePathBuf, + excluded_subpaths: Vec, +} + +fn build_seatbelt_access_policy( + action: &str, + param_prefix: &str, + roots: Vec, +) -> (String, Vec<(String, PathBuf)>) { + let mut policy_components = Vec::new(); + let mut params = Vec::new(); + + for (index, access_root) in roots.into_iter().enumerate() { + let root = + normalize_path_for_sandbox(access_root.root.as_path()).unwrap_or(access_root.root); + let root_param = format!("{param_prefix}_{index}"); + params.push((root_param.clone(), root.into_path_buf())); + + if access_root.excluded_subpaths.is_empty() { + policy_components.push(format!("(subpath (param \"{root_param}\"))")); + continue; + } + + let mut require_parts = vec![format!("(subpath (param \"{root_param}\"))")]; + for (excluded_index, excluded_subpath) in + access_root.excluded_subpaths.into_iter().enumerate() + { + let excluded_subpath = + normalize_path_for_sandbox(excluded_subpath.as_path()).unwrap_or(excluded_subpath); + let excluded_param = format!("{param_prefix}_{index}_RO_{excluded_index}"); + params.push((excluded_param.clone(), excluded_subpath.into_path_buf())); + require_parts.push(format!( + "(require-not (subpath (param \"{excluded_param}\")))" + )); + } + policy_components.push(format!("(require-all {} )", require_parts.join(" "))); + } + + if policy_components.is_empty() { + (String::new(), Vec::new()) + } else { + ( + format!("(allow {action}\n{}\n)", policy_components.join(" ")), + params, + ) + } +} + +#[cfg_attr(not(test), allow(dead_code))] +fn create_seatbelt_command_args_with_extensions( + command: Vec, + sandbox_policy: &SandboxPolicy, + sandbox_policy_cwd: &Path, + enforce_managed_network: bool, + network: Option<&NetworkProxy>, + extensions: Option<&MacOsSeatbeltProfileExtensions>, +) -> Vec { + create_seatbelt_command_args_for_policies_with_extensions( + command, + &FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, sandbox_policy_cwd), + NetworkSandboxPolicy::from(sandbox_policy), + sandbox_policy_cwd, + enforce_managed_network, + network, + extensions, + ) +} + +pub fn create_seatbelt_command_args_for_policies_with_extensions( + command: Vec, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + sandbox_policy_cwd: &Path, + enforce_managed_network: bool, + network: Option<&NetworkProxy>, + extensions: Option<&MacOsSeatbeltProfileExtensions>, +) -> Vec { + let unreadable_roots = + file_system_sandbox_policy.get_unreadable_roots_with_cwd(sandbox_policy_cwd); + let (file_write_policy, file_write_dir_params) = + if file_system_sandbox_policy.has_full_disk_write_access() { + if unreadable_roots.is_empty() { + // Allegedly, this is more permissive than `(allow file-write*)`. + ( + r#"(allow file-write* (regex #"^/"))"#.to_string(), + Vec::new(), + ) + } else { + build_seatbelt_access_policy( + "file-write*", + "WRITABLE_ROOT", + vec![SeatbeltAccessRoot { + root: root_absolute_path(), + excluded_subpaths: unreadable_roots.clone(), + }], + ) + } + } else { + build_seatbelt_access_policy( + "file-write*", + "WRITABLE_ROOT", + file_system_sandbox_policy + .get_writable_roots_with_cwd(sandbox_policy_cwd) + .into_iter() + .map(|root| SeatbeltAccessRoot { + root: root.root, + excluded_subpaths: root.read_only_subpaths, + }) + .collect(), + ) + }; + + let (file_read_policy, file_read_dir_params) = + if file_system_sandbox_policy.has_full_disk_read_access() { + if unreadable_roots.is_empty() { + ( + "; allow read-only file operations\n(allow file-read*)".to_string(), + Vec::new(), + ) + } else { + let (policy, params) = build_seatbelt_access_policy( + "file-read*", + "READABLE_ROOT", + vec![SeatbeltAccessRoot { + root: root_absolute_path(), + excluded_subpaths: unreadable_roots, + }], + ); + ( + format!("; allow read-only file operations\n{policy}"), + params, + ) + } + } else { + let (policy, params) = build_seatbelt_access_policy( + "file-read*", + "READABLE_ROOT", + file_system_sandbox_policy + .get_readable_roots_with_cwd(sandbox_policy_cwd) + .into_iter() + .map(|root| SeatbeltAccessRoot { + excluded_subpaths: unreadable_roots + .iter() + .filter(|path| path.as_path().starts_with(root.as_path())) + .cloned() + .collect(), + root, + }) + .collect(), + ); + if policy.is_empty() { + (String::new(), params) + } else { + ( + format!("; allow read-only file operations\n{policy}"), + params, + ) + } + }; + + let proxy = proxy_policy_inputs(network); + let network_policy = + dynamic_network_policy_for_network(network_sandbox_policy, enforce_managed_network, &proxy); + let seatbelt_extensions = extensions.map_or_else( + || { + // Backward-compatibility default when no extension profile is provided. + build_seatbelt_extensions(&MacOsSeatbeltProfileExtensions::default()) + }, + build_seatbelt_extensions, + ); + + let include_platform_defaults = file_system_sandbox_policy.include_platform_defaults(); + let mut policy_sections = vec![ + MACOS_SEATBELT_BASE_POLICY.to_string(), + file_read_policy, + file_write_policy, + network_policy, + ]; + if include_platform_defaults { + policy_sections.push(MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS.to_string()); + } + if !seatbelt_extensions.policy.is_empty() { + policy_sections.push(seatbelt_extensions.policy.clone()); + } + + let full_policy = policy_sections.join("\n"); + + let dir_params = [ + file_read_dir_params, + file_write_dir_params, + macos_dir_params(), + unix_socket_dir_params(&proxy), + seatbelt_extensions.dir_params, + ] + .concat(); + + let mut seatbelt_args: Vec = vec!["-p".to_string(), full_policy]; + let definition_args = dir_params + .into_iter() + .map(|(key, value): (String, PathBuf)| { + format!("-D{key}={value}", value = value.to_string_lossy()) + }); + seatbelt_args.extend(definition_args); + seatbelt_args.push("--".to_string()); + seatbelt_args.extend(command); + seatbelt_args +} + +/// Wraps libc::confstr to return a String. +fn confstr(name: libc::c_int) -> Option { + let mut buf = vec![0_i8; (libc::PATH_MAX as usize) + 1]; + let len = unsafe { libc::confstr(name, buf.as_mut_ptr(), buf.len()) }; + if len == 0 { + return None; + } + // confstr guarantees NUL-termination when len > 0. + let cstr = unsafe { CStr::from_ptr(buf.as_ptr()) }; + cstr.to_str().ok().map(ToString::to_string) +} + +/// Wraps confstr to return a canonicalized PathBuf. +fn confstr_path(name: libc::c_int) -> Option { + let s = confstr(name)?; + let path = PathBuf::from(s); + path.canonicalize().ok().or(Some(path)) +} + +fn macos_dir_params() -> Vec<(String, PathBuf)> { + if let Some(p) = confstr_path(libc::_CS_DARWIN_USER_CACHE_DIR) { + return vec![("DARWIN_USER_CACHE_DIR".to_string(), p)]; + } + vec![] +} + +#[cfg(test)] +#[path = "seatbelt_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/seatbelt_base_policy.sbpl b/codex-rs/sandboxing/src/seatbelt_base_policy.sbpl similarity index 100% rename from codex-rs/core/src/seatbelt_base_policy.sbpl rename to codex-rs/sandboxing/src/seatbelt_base_policy.sbpl diff --git a/codex-rs/core/src/seatbelt_network_policy.sbpl b/codex-rs/sandboxing/src/seatbelt_network_policy.sbpl similarity index 100% rename from codex-rs/core/src/seatbelt_network_policy.sbpl rename to codex-rs/sandboxing/src/seatbelt_network_policy.sbpl diff --git a/codex-rs/core/src/seatbelt_permissions.rs b/codex-rs/sandboxing/src/seatbelt_permissions.rs similarity index 96% rename from codex-rs/core/src/seatbelt_permissions.rs rename to codex-rs/sandboxing/src/seatbelt_permissions.rs index 5cbfd65094..d53e7496ad 100644 --- a/codex-rs/core/src/seatbelt_permissions.rs +++ b/codex-rs/sandboxing/src/seatbelt_permissions.rs @@ -1,12 +1,10 @@ -#![cfg(target_os = "macos")] - use std::collections::BTreeSet; use std::path::PathBuf; -pub use codex_protocol::models::MacOsAutomationPermission; -pub use codex_protocol::models::MacOsContactsPermission; -pub use codex_protocol::models::MacOsPreferencesPermission; -pub use codex_protocol::models::MacOsSeatbeltProfileExtensions; +use codex_protocol::models::MacOsAutomationPermission; +use codex_protocol::models::MacOsContactsPermission; +use codex_protocol::models::MacOsPreferencesPermission; +use codex_protocol::models::MacOsSeatbeltProfileExtensions; #[derive(Debug, Clone, PartialEq, Eq, Default)] pub(crate) struct SeatbeltExtensionPolicy { diff --git a/codex-rs/core/src/seatbelt_permissions_tests.rs b/codex-rs/sandboxing/src/seatbelt_permissions_tests.rs similarity index 100% rename from codex-rs/core/src/seatbelt_permissions_tests.rs rename to codex-rs/sandboxing/src/seatbelt_permissions_tests.rs diff --git a/codex-rs/core/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs similarity index 94% rename from codex-rs/core/src/seatbelt_tests.rs rename to codex-rs/sandboxing/src/seatbelt_tests.rs index bab1362017..925285763e 100644 --- a/codex-rs/core/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -1,7 +1,7 @@ +use super::MACOS_PATH_TO_SEATBELT_EXECUTABLE; use super::MACOS_SEATBELT_BASE_POLICY; use super::ProxyPolicyInputs; use super::UnixDomainSocketPolicy; -use super::create_seatbelt_command_args; use super::create_seatbelt_command_args_for_policies_with_extensions; use super::create_seatbelt_command_args_with_extensions; use super::dynamic_network_policy; @@ -9,18 +9,18 @@ use super::macos_dir_params; use super::normalize_path_for_sandbox; use super::unix_socket_dir_params; use super::unix_socket_policy; -use crate::protocol::ReadOnlyAccess; -use crate::protocol::SandboxPolicy; -use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; -use crate::seatbelt_permissions::MacOsAutomationPermission; -use crate::seatbelt_permissions::MacOsContactsPermission; -use crate::seatbelt_permissions::MacOsPreferencesPermission; -use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; +use codex_protocol::models::MacOsAutomationPermission; +use codex_protocol::models::MacOsContactsPermission; +use codex_protocol::models::MacOsPreferencesPermission; +use codex_protocol::models::MacOsSeatbeltProfileExtensions; 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::protocol::ReadOnlyAccess; +use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::fs; @@ -105,7 +105,7 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { - value: crate::protocol::FileSystemSpecialPath::Root, + value: FileSystemSpecialPath::Root, }, access: FileSystemAccessMode::Write, }, @@ -277,12 +277,13 @@ sys.exit(0 if allowed else 13) #[test] fn seatbelt_args_without_extension_profile_keep_legacy_preferences_read_access() { let cwd = std::env::temp_dir(); - let args = create_seatbelt_command_args( + let args = create_seatbelt_command_args_with_extensions( vec!["echo".to_string(), "ok".to_string()], &SandboxPolicy::new_read_only_policy(), cwd.as_path(), - false, + /*enforce_managed_network*/ false, None, + /*extensions*/ None, ); let policy = &args[1]; assert!(policy.contains("(allow user-preference-read)")); @@ -295,7 +296,7 @@ fn seatbelt_legacy_workspace_write_nested_readable_root_stays_writable() { let cwd = tmp.path().join("workspace"); fs::create_dir_all(cwd.join("docs")).expect("create docs"); let docs = AbsolutePathBuf::from_absolute_path(cwd.join("docs")).expect("absolute docs"); - let args = create_seatbelt_command_args( + let args = create_seatbelt_command_args_with_extensions( vec!["/bin/true".to_string()], &SandboxPolicy::WorkspaceWrite { writable_roots: vec![], @@ -308,8 +309,9 @@ fn seatbelt_legacy_workspace_write_nested_readable_root_stays_writable() { exclude_slash_tmp: true, }, cwd.as_path(), - false, + /*enforce_managed_network*/ false, None, + /*extensions*/ None, ); let docs_param = format!("-DWRITABLE_ROOT_0_RO_0={}", docs.as_path().display()); @@ -636,7 +638,14 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { .iter() .map(std::string::ToString::to_string) .collect(); - let args = create_seatbelt_command_args(shell_command.clone(), &policy, &cwd, false, None); + let args = create_seatbelt_command_args_with_extensions( + shell_command.clone(), + &policy, + &cwd, + /*enforce_managed_network*/ false, + None, + /*extensions*/ None, + ); // Build the expected policy text using a raw string for readability. // Note that the policy includes: @@ -740,8 +749,14 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { .iter() .map(std::string::ToString::to_string) .collect(); - let write_hooks_file_args = - create_seatbelt_command_args(shell_command_git, &policy, &cwd, false, None); + let write_hooks_file_args = create_seatbelt_command_args_with_extensions( + shell_command_git, + &policy, + &cwd, + /*enforce_managed_network*/ false, + None, + /*extensions*/ None, + ); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&write_hooks_file_args) .current_dir(&cwd) @@ -771,8 +786,14 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { .iter() .map(std::string::ToString::to_string) .collect(); - let write_allowed_file_args = - create_seatbelt_command_args(shell_command_allowed, &policy, &cwd, false, None); + let write_allowed_file_args = create_seatbelt_command_args_with_extensions( + shell_command_allowed, + &policy, + &cwd, + /*enforce_managed_network*/ false, + None, + /*extensions*/ None, + ); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&write_allowed_file_args) .current_dir(&cwd) @@ -833,7 +854,14 @@ fn create_seatbelt_args_with_read_only_git_pointer_file() { .iter() .map(std::string::ToString::to_string) .collect(); - let args = create_seatbelt_command_args(shell_command, &policy, &cwd, false, None); + let args = create_seatbelt_command_args_with_extensions( + shell_command, + &policy, + &cwd, + /*enforce_managed_network*/ false, + None, + /*extensions*/ None, + ); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&args) @@ -863,8 +891,14 @@ fn create_seatbelt_args_with_read_only_git_pointer_file() { .iter() .map(std::string::ToString::to_string) .collect(); - let gitdir_args = - create_seatbelt_command_args(shell_command_gitdir, &policy, &cwd, false, None); + let gitdir_args = create_seatbelt_command_args_with_extensions( + shell_command_gitdir, + &policy, + &cwd, + /*enforce_managed_network*/ false, + None, + /*extensions*/ None, + ); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&gitdir_args) .current_dir(&cwd) @@ -921,12 +955,13 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { .iter() .map(std::string::ToString::to_string) .collect(); - let args = create_seatbelt_command_args( + let args = create_seatbelt_command_args_with_extensions( shell_command.clone(), &policy, vulnerable_root.as_path(), - false, + /*enforce_managed_network*/ false, None, + /*extensions*/ None, ); let tmpdir_env_var = std::env::var("TMPDIR") From c2410060ead90d7229158dbf2b8219c217b632e6 Mon Sep 17 00:00:00 2001 From: dhruvgupta-oai Date: Tue, 24 Mar 2026 00:41:38 -0400 Subject: [PATCH 09/55] [codex-cli][app-server] Update self-serve business usage limit copy in error returned (#15478) ## Summary - update the self-serve business usage-based limit message to direct users to their admin for additional credits - add a focused unit test for the self_serve_business_usage_based plan branch Added also: If you are at a rate limit but you still have credits, codex cli would tell you to switch the model. We shouldnt do this if you have credits so fixed this. ## Test - launched the source-built CLI and verified the updated message is shown for the self-serve business usage-based plan ![Test screenshot](https://raw.githubusercontent.com/openai/codex/5cc3c013ef17ac5c66dfd9395c0d3c4837602231/docs/images/self-serve-business-usage-limit.png) --- .github/workflows/rust-ci.yml | 2 +- .../app-server/tests/suite/v2/thread_start.rs | 22 +++- codex-rs/core/src/agent/control_tests.rs | 37 +------ codex-rs/core/src/config/config_tests.rs | 9 +- codex-rs/core/src/error.rs | 15 +++ .../src/tools/handlers/multi_agents_tests.rs | 101 ------------------ .../core/tests/suite/compact_resume_fork.rs | 35 +++--- codex-rs/core/tests/suite/shell_snapshot.rs | 29 ++++- codex-rs/tui/src/chatwidget.rs | 7 ++ .../tui/tests/suite/model_availability_nux.rs | 22 ++-- codex-rs/tui_app_server/src/chatwidget.rs | 7 ++ .../tests/suite/model_availability_nux.rs | 22 ++-- 12 files changed, 129 insertions(+), 179 deletions(-) diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 9db72b1d21..c203e2b742 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -547,7 +547,7 @@ jobs: tests: name: Tests — ${{ matrix.runner }} - ${{ matrix.target }}${{ matrix.remote_env == 'true' && ' (remote)' || '' }} runs-on: ${{ matrix.runs_on || matrix.runner }} - timeout-minutes: 30 + timeout-minutes: ${{ matrix.runner == 'windows-arm64' && 35 || 30 }} needs: changed if: ${{ needs.changed.outputs.codex == 'true' || needs.changed.outputs.workflows == 'true' || github.event_name == 'push' }} defaults: diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 37ca5a3903..1a73444bda 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -585,9 +585,10 @@ request_max_retries = 0 stream_max_retries = 0 [mcp_servers.required_broken] -command = "codex-definitely-not-a-real-binary" +{required_broken_transport} required = true -"# +"#, + required_broken_transport = broken_mcp_transport_toml() ), ) } @@ -615,8 +616,21 @@ request_max_retries = 0 stream_max_retries = 0 [mcp_servers.optional_broken] -command = "codex-definitely-not-a-real-binary" -"# +{optional_broken_transport} +"#, + optional_broken_transport = broken_mcp_transport_toml() ), ) } + +#[cfg(target_os = "windows")] +fn broken_mcp_transport_toml() -> &'static str { + r#"command = "cmd" +args = ["/C", "exit 1"]"# +} + +#[cfg(not(target_os = "windows"))] +fn broken_mcp_transport_toml() -> &'static str { + r#"command = "/bin/sh" +args = ["-c", "exit 1"]"# +} diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 2cf7398f7d..2da9db3985 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -29,6 +29,8 @@ use tempfile::TempDir; use tokio::time::Duration; use tokio::time::sleep; use tokio::time::timeout; + +const MULTI_AGENT_EVENTUAL_TIMEOUT: Duration = Duration::from_secs(5); use toml::Value as TomlValue; async fn test_config_with_cli_overrides( @@ -1019,7 +1021,7 @@ async fn multi_agent_v2_completion_sends_inter_agent_message_to_direct_parent() ) .await; - timeout(Duration::from_secs(2), async { + timeout(MULTI_AGENT_EVENTUAL_TIMEOUT, async { loop { let delivered = harness .manager @@ -1044,39 +1046,6 @@ async fn multi_agent_v2_completion_sends_inter_agent_message_to_direct_parent() }) .await .expect("completion watcher should send inter-agent communication"); - - let worker_thread = harness - .manager - .get_thread(worker_thread_id) - .await - .expect("worker thread should exist"); - let expected_message = InterAgentCommunication::new( - tester_path.clone(), - worker_path.clone(), - Vec::new(), - "done".to_string(), - ); - timeout(Duration::from_secs(2), async { - loop { - let history_items = worker_thread - .codex - .session - .clone_history() - .await - .raw_items() - .to_vec(); - if history_contains_assistant_inter_agent_communication( - &history_items, - &expected_message, - ) && !has_subagent_notification(&history_items) - { - break; - } - sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("worker should record assistant inter-agent message"); } #[tokio::test] diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 7786b30873..7d3af80249 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -5639,8 +5639,15 @@ fn write_fake_bwrap(contents: &str) -> tempfile::TempPath { use std::os::unix::fs::PermissionsExt; use tempfile::NamedTempFile; + // Bazel can mount the OS temp directory `noexec`, so prefer the current + // working directory for fake executables and fall back to the default temp + // dir outside that environment. + let temp_file = std::env::current_dir() + .ok() + .and_then(|dir| NamedTempFile::new_in(dir).ok()) + .unwrap_or_else(|| NamedTempFile::new().expect("temp file")); // Linux rejects exec-ing a file that is still open for writing. - let path = NamedTempFile::new().expect("temp file").into_temp_path(); + let path = temp_file.into_temp_path(); fs::write(&path, contents).expect("write fake bwrap"); let permissions = fs::Permissions::from_mode(0o755); fs::set_permissions(&path, permissions).expect("chmod fake bwrap"); diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 80d60619dd..b84180d160 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -460,6 +460,21 @@ impl std::fmt::Display for UsageLimitReachedError { "You've hit your usage limit.{}", retry_suffix(self.resets_at.as_ref()) ), + Some(PlanType::Unknown(plan)) + if plan.eq_ignore_ascii_case("self_serve_business_usage_based") => + { + match self + .rate_limits + .as_ref() + .and_then(|snapshot| snapshot.credits.as_ref()) + .map(|credits| credits.has_credits) + { + Some(true) => "You've hit your usage limit. Contact your admin to increase spend limits to continue." + .to_string(), + Some(false) | None => "You've hit your usage limit. Contact your admin to add credits to continue." + .to_string(), + } + } Some(PlanType::Unknown(_)) | None => format!( "You've hit your usage limit.{}", retry_suffix(self.resets_at.as_ref()) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 873b55d660..5afc3682a0 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -105,10 +105,6 @@ fn history_contains_inter_agent_communication( }) } -fn inter_agent_message_text(recipient: &str, content: &str) -> String { - format!("author: /root\nrecipient: {recipient}\nother_recipients: []\nContent: {content}") -} - #[derive(Clone, Copy)] struct NeverEndingTask; @@ -415,47 +411,6 @@ async fn multi_agent_v2_spawn_returns_path_and_send_input_accepts_relative_path( && communication.content == "continue" ) })); - - let child_thread = manager - .get_thread(child_thread_id) - .await - .expect("child thread should exist"); - let expected_communication = InterAgentCommunication::new( - AgentPath::root(), - AgentPath::try_from("/root/test_process").expect("agent path"), - Vec::new(), - "continue".to_string(), - ); - timeout(Duration::from_secs(2), async { - loop { - let history_items = child_thread - .codex - .session - .clone_history() - .await - .raw_items() - .to_vec(); - let recorded = - history_contains_inter_agent_communication(&history_items, &expected_communication); - let saw_user_message = history_items.iter().any(|item| { - matches!( - item, - ResponseItem::Message { role, content, .. } - if role == "user" - && content.iter().any(|content_item| matches!( - content_item, - ContentItem::InputText { text } if text == "continue" - )) - ) - }); - if recorded && !saw_user_message { - break; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("v2 send_input should record assistant envelope"); } #[tokio::test] @@ -492,10 +447,6 @@ async fn multi_agent_v2_send_input_accepts_structured_items() { .resolve_agent_reference(session.conversation_id, &turn.session_source, "worker") .await .expect("worker should resolve"); - let thread = manager - .get_thread(agent_id) - .await - .expect("worker thread should exist"); let invocation = invocation( session, turn, @@ -532,58 +483,6 @@ async fn multi_agent_v2_send_input_accepts_structured_items() { .into_iter() .find(|(id, op)| *id == agent_id && *op == expected); assert_eq!(captured, Some((agent_id, expected))); - - let expected_message = inter_agent_message_text( - "/root/worker", - "[mention:$drive](app://google_drive)\nread the folder", - ); - timeout(Duration::from_secs(2), async { - loop { - let history_items = thread - .codex - .session - .clone_history() - .await - .raw_items() - .to_vec(); - let recorded_assistant_envelope = history_items.iter().any(|item| { - matches!( - item, - ResponseItem::Message { role, content, .. } - if role == "assistant" - && content.iter().any(|content_item| matches!( - content_item, - ContentItem::OutputText { text } - if text == &expected_message - )) - ) - }); - let saw_user_message = history_items.iter().any(|item| { - matches!( - item, - ResponseItem::Message { role, content, .. } - if role == "user" - && content.iter().any(|content_item| matches!( - content_item, - ContentItem::InputText { text } - if text == "read the folder" - || text == "[mention:$drive](app://google_drive)\nread the folder" - )) - ) - }); - if !recorded_assistant_envelope && saw_user_message { - break; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("structured items should stay on the legacy user-input path"); - - let _ = thread - .submit(Op::Shutdown {}) - .await - .expect("shutdown should submit"); } #[tokio::test] diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 2007aab55b..f4f89281db 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -12,7 +12,6 @@ use super::compact::FIRST_REPLY; use super::compact::SUMMARY_TEXT; use anyhow::Result; use codex_core::CodexThread; -use codex_core::ForkSnapshot; use codex_core::ThreadManager; use codex_core::compact::SUMMARIZATION_PROMPT; use codex_core::config::Config; @@ -393,16 +392,26 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() ]; expected_after_second_compact_user_texts.extend_from_slice(seeded_user_prefix); expected_after_second_compact_user_texts.push("AFTER_COMPACT_2".to_string()); + let mut expected_fork_local_user_texts = vec![ + "AFTER_FORK".to_string(), + expected_after_second_compact_user_texts[4].clone(), + ]; + expected_fork_local_user_texts.extend_from_slice(seeded_user_prefix); + expected_fork_local_user_texts.push("AFTER_COMPACT_2".to_string()); let final_user_texts = json_message_input_texts(&requests[requests.len() - 1], "user"); let (final_last, final_prefix) = final_user_texts .split_last() .unwrap_or_else(|| panic!("after-second-resume request missing user messages")); assert_eq!(final_last, AFTER_SECOND_RESUME); - assert!( - final_prefix.starts_with(&expected_after_second_compact_user_texts), - "after-second-resume user texts should preserve post-compact user history prefix" - ); - let final_seeded_suffix = &final_prefix[expected_after_second_compact_user_texts.len()..]; + let matched_prefix_len = if final_prefix.starts_with(&expected_after_second_compact_user_texts) + { + expected_after_second_compact_user_texts.len() + } else if final_prefix.starts_with(&expected_fork_local_user_texts) { + expected_fork_local_user_texts.len() + } else { + panic!("after-second-resume user texts should preserve post-compact user history prefix"); + }; + let final_seeded_suffix = &final_prefix[matched_prefix_len..]; if seeded_user_prefix.is_empty() { assert!( final_seeded_suffix.is_empty(), @@ -847,14 +856,8 @@ async fn fork_thread( path: std::path::PathBuf, nth_user_message: usize, ) -> Arc { - Box::pin(manager.fork_thread( - ForkSnapshot::TruncateBeforeNthUserMessage(nth_user_message), - config.clone(), - path, - /*persist_extended_history*/ false, - /*parent_trace*/ None, - )) - .await - .expect("fork conversation") - .thread + Box::pin(manager.fork_thread(nth_user_message, config.clone(), path, false, None)) + .await + .expect("fork conversation") + .thread } diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index 0df7d583e5..0e044f3922 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -72,7 +72,7 @@ async fn wait_for_snapshot(codex_home: &Path) -> Result { } async fn wait_for_file_contents(path: &Path) -> Result { - let deadline = Instant::now() + Duration::from_secs(5); + let deadline = Instant::now() + Duration::from_secs(15); loop { match fs::read_to_string(path).await { Ok(contents) => return Ok(contents), @@ -575,7 +575,32 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { let snapshot_content = fs::read_to_string(&snapshot_path).await?; assert_posix_snapshot_sections(&snapshot_content); - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + let mut saw_patch_begin = false; + let mut patch_end = None; + wait_for_event(&codex, |ev| match ev { + EventMsg::PatchApplyBegin(begin) if begin.call_id == call_id => { + saw_patch_begin = true; + false + } + EventMsg::PatchApplyEnd(end) if end.call_id == call_id => { + patch_end = Some(end.clone()); + false + } + EventMsg::TurnComplete(_) => true, + _ => false, + }) + .await; + + assert!( + saw_patch_begin, + "expected apply_patch to emit PatchApplyBegin" + ); + let patch_end = patch_end.expect("expected apply_patch to emit PatchApplyEnd"); + assert!( + patch_end.success, + "expected apply_patch to finish successfully: stdout={:?} stderr={:?}", + patch_end.stdout, patch_end.stderr, + ); assert_eq!( wait_for_file_contents(&target).await?, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index ade9235b40..91f6159329 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2120,7 +2120,14 @@ impl ChatWidget { .map(|w| w.used_percent >= RATE_LIMIT_SWITCH_PROMPT_THRESHOLD) .unwrap_or(false)); + let has_workspace_credits = snapshot + .credits + .as_ref() + .map(|credits| credits.has_credits) + .unwrap_or(false); + if high_usage + && !has_workspace_credits && !self.rate_limit_switch_prompt_hidden() && self.current_model() != NUDGE_MODEL_SLUG && !matches!( diff --git a/codex-rs/tui/tests/suite/model_availability_nux.rs b/codex-rs/tui/tests/suite/model_availability_nux.rs index 512db97974..f1bee31606 100644 --- a/codex-rs/tui/tests/suite/model_availability_nux.rs +++ b/codex-rs/tui/tests/suite/model_availability_nux.rs @@ -139,23 +139,27 @@ trust_level = "trusted" let mut exit_rx = exit_rx; let writer_tx = session.writer_sender(); let interrupt_writer = writer_tx.clone(); - let interrupt_task = tokio::spawn(async move { - sleep(Duration::from_secs(2)).await; - for _ in 0..4 { - let _ = interrupt_writer.send(vec![3]).await; - sleep(Duration::from_millis(500)).await; - } - }); + let mut startup_ready = false; + let mut answered_cursor_query = false; let exit_code_result = timeout(Duration::from_secs(15), async { loop { select! { result = output_rx.recv() => match result { Ok(chunk) => { - if chunk.windows(4).any(|window| window == b"\x1b[6n") { + let has_cursor_query = chunk.windows(4).any(|window| window == b"\x1b[6n"); + if has_cursor_query { let _ = writer_tx.send(b"\x1b[1;1R".to_vec()).await; + answered_cursor_query = true; } output.extend_from_slice(&chunk); + if !startup_ready && answered_cursor_query && !has_cursor_query { + startup_ready = true; + for _ in 0..4 { + let _ = interrupt_writer.send(vec![3]).await; + sleep(Duration::from_millis(500)).await; + } + } } Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await, Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {} @@ -166,8 +170,6 @@ trust_level = "trusted" }) .await; - interrupt_task.abort(); - let exit_code = match exit_code_result { Ok(Ok(code)) => code, Ok(Err(err)) => return Err(err.into()), diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 9363e3e356..923e96b02b 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -2491,7 +2491,14 @@ impl ChatWidget { .map(|w| w.used_percent >= RATE_LIMIT_SWITCH_PROMPT_THRESHOLD) .unwrap_or(false)); + let has_workspace_credits = snapshot + .credits + .as_ref() + .map(|credits| credits.has_credits) + .unwrap_or(false); + if high_usage + && !has_workspace_credits && !self.rate_limit_switch_prompt_hidden() && self.current_model() != NUDGE_MODEL_SLUG && !matches!( diff --git a/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs b/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs index 512db97974..f1bee31606 100644 --- a/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs +++ b/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs @@ -139,23 +139,27 @@ trust_level = "trusted" let mut exit_rx = exit_rx; let writer_tx = session.writer_sender(); let interrupt_writer = writer_tx.clone(); - let interrupt_task = tokio::spawn(async move { - sleep(Duration::from_secs(2)).await; - for _ in 0..4 { - let _ = interrupt_writer.send(vec![3]).await; - sleep(Duration::from_millis(500)).await; - } - }); + let mut startup_ready = false; + let mut answered_cursor_query = false; let exit_code_result = timeout(Duration::from_secs(15), async { loop { select! { result = output_rx.recv() => match result { Ok(chunk) => { - if chunk.windows(4).any(|window| window == b"\x1b[6n") { + let has_cursor_query = chunk.windows(4).any(|window| window == b"\x1b[6n"); + if has_cursor_query { let _ = writer_tx.send(b"\x1b[1;1R".to_vec()).await; + answered_cursor_query = true; } output.extend_from_slice(&chunk); + if !startup_ready && answered_cursor_query && !has_cursor_query { + startup_ready = true; + for _ in 0..4 { + let _ = interrupt_writer.send(vec![3]).await; + sleep(Duration::from_millis(500)).await; + } + } } Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await, Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {} @@ -166,8 +170,6 @@ trust_level = "trusted" }) .await; - interrupt_task.abort(); - let exit_code = match exit_code_result { Ok(Ok(code)) => code, Ok(Err(err)) => return Err(err.into()), From a10960e41c02f1a1c2819d9384039a8b9b67480d Mon Sep 17 00:00:00 2001 From: Dominik Kundel Date: Mon, 23 Mar 2026 22:14:33 -0700 Subject: [PATCH 10/55] move imagegen skill into system skills (#15600) Add imagegen skill as built-in skill. Source: github.com/openai/skills --- .../src/assets/samples/imagegen/LICENSE.txt | 201 ++++ .../src/assets/samples/imagegen/SKILL.md | 279 ++++++ .../samples/imagegen/agents/openai.yaml | 6 + .../imagegen/assets/imagegen-small.svg | 5 + .../samples/imagegen/assets/imagegen.png | Bin 0 -> 1711 bytes .../assets/samples/imagegen/references/cli.md | 160 +++ .../imagegen/references/codex-network.md | 33 + .../samples/imagegen/references/image-api.md | 49 + .../samples/imagegen/references/prompting.md | 98 ++ .../imagegen/references/sample-prompts.md | 376 +++++++ .../samples/imagegen/scripts/image_gen.py | 926 ++++++++++++++++++ 11 files changed, 2133 insertions(+) create mode 100644 codex-rs/skills/src/assets/samples/imagegen/LICENSE.txt create mode 100644 codex-rs/skills/src/assets/samples/imagegen/SKILL.md create mode 100644 codex-rs/skills/src/assets/samples/imagegen/agents/openai.yaml create mode 100644 codex-rs/skills/src/assets/samples/imagegen/assets/imagegen-small.svg create mode 100644 codex-rs/skills/src/assets/samples/imagegen/assets/imagegen.png create mode 100644 codex-rs/skills/src/assets/samples/imagegen/references/cli.md create mode 100644 codex-rs/skills/src/assets/samples/imagegen/references/codex-network.md create mode 100644 codex-rs/skills/src/assets/samples/imagegen/references/image-api.md create mode 100644 codex-rs/skills/src/assets/samples/imagegen/references/prompting.md create mode 100644 codex-rs/skills/src/assets/samples/imagegen/references/sample-prompts.md create mode 100644 codex-rs/skills/src/assets/samples/imagegen/scripts/image_gen.py diff --git a/codex-rs/skills/src/assets/samples/imagegen/LICENSE.txt b/codex-rs/skills/src/assets/samples/imagegen/LICENSE.txt new file mode 100644 index 0000000000..13e25df86c --- /dev/null +++ b/codex-rs/skills/src/assets/samples/imagegen/LICENSE.txt @@ -0,0 +1,201 @@ +Apache License +Version 2.0, January 2004 +http://www.apache.org/licenses/ + +TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + +1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + +2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + +3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + +4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + +5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + +6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + +7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + +8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + +9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf of + any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + +END OF TERMS AND CONDITIONS + +APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don\'t include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + +Copyright [yyyy] [name of copyright owner] + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. diff --git a/codex-rs/skills/src/assets/samples/imagegen/SKILL.md b/codex-rs/skills/src/assets/samples/imagegen/SKILL.md new file mode 100644 index 0000000000..88195882ef --- /dev/null +++ b/codex-rs/skills/src/assets/samples/imagegen/SKILL.md @@ -0,0 +1,279 @@ +--- +name: "imagegen" +description: "Generate or edit raster images when the task benefits from AI-created bitmap visuals such as photos, illustrations, textures, sprites, mockups, or transparent-background cutouts. Use when Codex should create a brand-new image, transform an existing image, or derive visual variants from references, and the output should be a bitmap asset rather than repo-native code or vector. Do not use when the task is better handled by editing existing SVG/vector/code-native assets, extending an established icon or logo system, or building the visual directly in HTML/CSS/canvas." +--- + +# Image Generation Skill + +Generates or edits images for the current project (for example website assets, game assets, UI mockups, product mockups, wireframes, logo design, photorealistic images, or infographics). + +## Top-level modes and rules + +This skill has exactly two top-level modes: + +- **Default built-in tool mode (preferred):** built-in `image_gen` tool for normal image generation and editing. Does not require `OPENAI_API_KEY`. +- **Fallback CLI mode (explicit-only):** `scripts/image_gen.py` CLI. Use only when the user explicitly asks for the CLI path. Requires `OPENAI_API_KEY`. + +Within the explicit CLI fallback only, the CLI exposes three subcommands: + +- `generate` +- `edit` +- `generate-batch` + +Rules: +- Use the built-in `image_gen` tool by default for all normal image generation and editing requests. +- Never switch to CLI fallback automatically. +- If the built-in tool fails or is unavailable, tell the user the CLI fallback exists and that it requires `OPENAI_API_KEY`. Proceed only if the user explicitly asks for that fallback. +- If the user explicitly asks for CLI mode, use the bundled `scripts/image_gen.py` workflow. Do not create one-off SDK runners. +- Never modify `scripts/image_gen.py`. If something is missing, ask the user before doing anything else. + +Built-in save-path policy: +- In built-in tool mode, Codex saves generated images under `$CODEX_HOME/*` by default. +- Do not describe or rely on OS temp as the default built-in destination. +- Do not describe or rely on a destination-path argument (if any) on the built-in `image_gen` tool. If a specific location is needed, generate first and then move or copy the selected output from `$CODEX_HOME/generated_images/...`. +- Save-path precedence in built-in mode: + 1. If the user names a destination, move or copy the selected output there. + 2. If the image is meant for the current project, move or copy the final selected image into the workspace before finishing. + 3. If the image is only for preview or brainstorming, render it inline; the underlying file can remain at the default `$CODEX_HOME/*` path. +- Never leave a project-referenced asset only at the default `$CODEX_HOME/*` path. +- Do not overwrite an existing asset unless the user explicitly asked for replacement; otherwise create a sibling versioned filename such as `hero-v2.png` or `item-icon-edited.png`. + +Shared prompt guidance for both modes lives in `references/prompting.md` and `references/sample-prompts.md`. + +Fallback-only docs/resources for CLI mode: +- `references/cli.md` +- `references/image-api.md` +- `references/codex-network.md` +- `scripts/image_gen.py` + +## When to use +- Generate a new image (concept art, product shot, cover, website hero) +- Generate a new image using one or more reference images for style, composition, or mood +- Edit an existing image (inpainting, lighting or weather transformations, background replacement, object removal, compositing, transparent background) +- Produce many assets or variants for one task + +## When not to use +- Extending or matching an existing SVG/vector icon set, logo system, or illustration library inside the repo +- Creating simple shapes, diagrams, wireframes, or icons that are better produced directly in SVG, HTML/CSS, or canvas +- Making a small project-local asset edit when the source file already exists in an editable native format +- Any task where the user clearly wants deterministic code-native output instead of a generated bitmap + +## Decision tree + +Think about two separate questions: + +1. **Intent:** is this a new image or an edit of an existing image? +2. **Execution strategy:** is this one asset or many assets/variants? + +Intent: +- If the user wants to modify an existing image while preserving parts of it, treat the request as **edit**. +- If the user provides images only as references for style, composition, mood, or subject guidance, treat the request as **generate**. +- If the user provides no images, treat the request as **generate**. + +Built-in edit semantics: +- Built-in edit mode is for images already visible in the conversation context, such as attached images or images generated earlier in the thread. +- If the user wants to edit a local image file with the built-in tool, first load it with built-in `view_image` tool so the image is visible in the conversation context, then proceed with the built-in edit flow. +- Do not promise arbitrary filesystem-path editing through the built-in tool. +- If a local file still needs direct file-path control, masks, or other explicit CLI-only parameters, use the explicit CLI fallback only when the user asks for it. +- For edits, preserve invariants aggressively and save non-destructively by default. + +Execution strategy: +- In the built-in default path, produce many assets or variants by issuing one `image_gen` call per requested asset or variant. +- In the explicit CLI fallback path, use the CLI `generate-batch` subcommand only when the user explicitly chose CLI mode and needs many prompts/assets. + +Assume the user wants a new image unless they clearly ask to change an existing one. + +## Workflow +1. Decide the top-level mode: built-in by default, fallback CLI only if explicitly requested. +2. Decide the intent: `generate` or `edit`. +3. Decide whether the output is preview-only or meant to be consumed by the current project. +4. Decide the execution strategy: single asset vs repeated built-in calls vs CLI `generate-batch`. +5. Collect inputs up front: prompt(s), exact text (verbatim), constraints/avoid list, and any input images. +6. For every input image, label its role explicitly: + - reference image + - edit target + - supporting insert/style/compositing input +7. If the edit target is only on the local filesystem and you are staying on the built-in path, inspect it with `view_image` first so the image is available in conversation context. +8. If the user asked for a photo, illustration, sprite, product image, banner, or other explicitly raster-style asset, use `image_gen` rather than substituting SVG/HTML/CSS placeholders. If the request is for an icon, logo, or UI graphic that should match existing repo-native SVG/vector/code assets, prefer editing those directly instead. +9. Augment the prompt based on specificity: + - If the user's prompt is already specific and detailed, normalize it into a clear spec without adding creative requirements. + - If the user's prompt is generic, add tasteful augmentation only when it materially improves output quality. +10. Use the built-in `image_gen` tool by default. +11. If the user explicitly chooses the CLI fallback, then and only then use the fallback-only docs for quality, `input_fidelity`, masks, output format, output paths, and network setup. +12. Inspect outputs and validate: subject, style, composition, text accuracy, and invariants/avoid items. +13. Iterate with a single targeted change, then re-check. +14. For preview-only work, render the image inline; the underlying file may remain at the default `$CODEX_HOME/generated_images/...` path. +15. For project-bound work, move or copy the selected artifact into the workspace and update any consuming code or references. Never leave a project-referenced asset only at the default `$CODEX_HOME/generated_images/...` path. +16. For batches, persist only the selected finals in the workspace unless the user explicitly asked to keep discarded variants. +17. Always report the final saved path for any workspace-bound asset, plus the final prompt and whether the built-in tool or fallback CLI mode was used. + +## Prompt augmentation + +Reformat user prompts into a structured, production-oriented spec. Make the user's goal clearer and more actionable, but do not blindly add detail. + +Treat this as prompt-shaping guidance, not a closed schema. Use only the lines that help, and add a short extra labeled line when it materially improves clarity. + +### Specificity policy + +Use the user's prompt specificity to decide how much augmentation is appropriate: + +- If the prompt is already specific and detailed, preserve that specificity and only normalize/structure it. +- If the prompt is generic, you may add tasteful augmentation when it will materially improve the result. + +Allowed augmentations: +- composition or framing hints +- polish level or intended-use hints +- practical layout guidance +- reasonable scene concreteness that supports the stated request + +Not allowed augmentations: +- extra characters or objects that are not implied by the request +- brand names, slogans, palettes, or narrative beats that are not implied +- arbitrary side-specific placement unless the surrounding layout supports it + +## Use-case taxonomy (exact slugs) + +Classify each request into one of these buckets and keep the slug consistent across prompts and references. + +Generate: +- photorealistic-natural — candid/editorial lifestyle scenes with real texture and natural lighting. +- product-mockup — product/packaging shots, catalog imagery, merch concepts. +- ui-mockup — app/web interface mockups and wireframes; specify the desired fidelity. +- infographic-diagram — diagrams/infographics with structured layout and text. +- logo-brand — logo/mark exploration, vector-friendly. +- illustration-story — comics, children’s book art, narrative scenes. +- stylized-concept — style-driven concept art, 3D/stylized renders. +- historical-scene — period-accurate/world-knowledge scenes. + +Edit: +- text-localization — translate/replace in-image text, preserve layout. +- identity-preserve — try-on, person-in-scene; lock face/body/pose. +- precise-object-edit — remove/replace a specific element (including interior swaps). +- lighting-weather — time-of-day/season/atmosphere changes only. +- background-extraction — transparent background / clean cutout. +- style-transfer — apply reference style while changing subject/scene. +- compositing — multi-image insert/merge with matched lighting/perspective. +- sketch-to-render — drawing/line art to photoreal render. + +## Shared prompt schema + +Use the following labeled spec as shared prompt scaffolding for both top-level modes: + +```text +Use case: +Asset type: +Primary request: +Input images: (optional) +Scene/backdrop: +Subject:
+Style/medium: +Composition/framing: +Lighting/mood: +Color palette: +Materials/textures: +Text (verbatim): "" +Constraints: +Avoid: +``` + +Notes: +- `Asset type` and `Input images` are prompt scaffolding, not dedicated CLI flags. +- `Scene/backdrop` refers to the visual setting. It is not the same as the fallback CLI `background` parameter, which controls output transparency behavior. +- Fallback-only execution notes such as `Quality:`, `Input fidelity:`, masks, output format, and output paths belong in the explicit CLI path only. Do not treat them as built-in `image_gen` tool arguments. + +Augmentation rules: +- Keep it short. +- Add only the details needed to improve the prompt materially. +- For edits, explicitly list invariants (`change only X; keep Y unchanged`). +- If any critical detail is missing and blocks success, ask a question; otherwise proceed. + +## Examples + +### Generation example (hero image) +```text +Use case: product-mockup +Asset type: landing page hero +Primary request: a minimal hero image of a ceramic coffee mug +Style/medium: clean product photography +Composition/framing: wide composition with usable negative space for page copy if needed +Lighting/mood: soft studio lighting +Constraints: no logos, no text, no watermark +``` + +### Edit example (invariants) +```text +Use case: precise-object-edit +Asset type: product photo background replacement +Primary request: replace only the background with a warm sunset gradient +Constraints: change only the background; keep the product and its edges unchanged; no text; no watermark +``` + +## Prompting best practices +- Structure prompt as scene/backdrop -> subject -> details -> constraints. +- Include intended use (ad, UI mock, infographic) to set the mode and polish level. +- Use camera/composition language for photorealism. +- Only use SVG/vector stand-ins when the user explicitly asked for vector output or a non-image placeholder. +- Quote exact text and specify typography + placement. +- For tricky words, spell them letter-by-letter and require verbatim rendering. +- For multi-image inputs, reference images by index and describe how they should be used. +- For edits, repeat invariants every iteration to reduce drift. +- Iterate with single-change follow-ups. +- If the prompt is generic, add only the extra detail that will materially help. +- If the prompt is already detailed, normalize it instead of expanding it. +- For explicit CLI fallback only, see `references/cli.md` and `references/image-api.md` for `quality`, `input_fidelity`, masks, output format, and output-path guidance. + +More principles shared by both modes: `references/prompting.md`. +Copy/paste specs shared by both modes: `references/sample-prompts.md`. + +## Guidance by asset type +Asset-type templates (website assets, game assets, wireframes, logo) are consolidated in `references/sample-prompts.md`. + +## Fallback CLI mode only + +### Temp and output conventions +These conventions apply only to the explicit CLI fallback. They do not describe built-in `image_gen` output behavior. +- Use `tmp/imagegen/` for intermediate files (for example JSONL batches); delete them when done. +- Write final artifacts under `output/imagegen/`. +- Use `--out` or `--out-dir` to control output paths; keep filenames stable and descriptive. + +### Dependencies +Prefer `uv` for dependency management in this repo. + +Required Python package: +```bash +uv pip install openai +``` + +Optional for downscaling only: +```bash +uv pip install pillow +``` + +Portability note: +- If you are using the installed skill outside this repo, install dependencies into that environment with its package manager. +- In uv-managed environments, `uv pip install ...` remains the preferred path. + +### Environment +- `OPENAI_API_KEY` must be set for live API calls. +- Do not ask the user for `OPENAI_API_KEY` when using the built-in `image_gen` tool. +- Never ask the user to paste the full key in chat. Ask them to set it locally and confirm when ready. + +If the key is missing, give the user these steps: +1. Create an API key in the OpenAI platform UI: https://platform.openai.com/api-keys +2. Set `OPENAI_API_KEY` as an environment variable in their system. +3. Offer to guide them through setting the environment variable for their OS/shell if needed. + +If installation is not possible in this environment, tell the user which dependency is missing and how to install it into their active environment. + +### Script-mode notes +- CLI commands + examples: `references/cli.md` +- API parameter quick reference: `references/image-api.md` +- Network approvals / sandbox settings for CLI mode: `references/codex-network.md` + +## Reference map +- `references/prompting.md`: shared prompting principles for both modes. +- `references/sample-prompts.md`: shared copy/paste prompt recipes for both modes. +- `references/cli.md`: fallback-only CLI usage via `scripts/image_gen.py`. +- `references/image-api.md`: fallback-only API/CLI parameter reference. +- `references/codex-network.md`: fallback-only network/sandbox troubleshooting for CLI mode. +- `scripts/image_gen.py`: fallback-only CLI implementation. Do not load or use it unless the user explicitly chooses CLI mode. diff --git a/codex-rs/skills/src/assets/samples/imagegen/agents/openai.yaml b/codex-rs/skills/src/assets/samples/imagegen/agents/openai.yaml new file mode 100644 index 0000000000..c9cfddb144 --- /dev/null +++ b/codex-rs/skills/src/assets/samples/imagegen/agents/openai.yaml @@ -0,0 +1,6 @@ +interface: + display_name: "Image Gen" + short_description: "Generate or edit images for websites, games, and more" + icon_small: "./assets/imagegen-small.svg" + icon_large: "./assets/imagegen.png" + default_prompt: "Generate or edit the visual assets for this task with the built-in `image_gen` tool by default. First confirm that the task actually calls for a raster image; if the project already has SVG/vector/code-native assets and the user wants to extend or match those, do not use this skill. If the task includes reference images, treat them as references unless the user clearly wants an existing image modified. For multi-asset requests, loop built-in calls rather than treating batch as a separate top-level mode. Only use the fallback CLI if the user explicitly asks for it, and keep CLI-only controls such as `generate-batch`, `quality`, `input_fidelity`, masks, and output paths on that fallback path." diff --git a/codex-rs/skills/src/assets/samples/imagegen/assets/imagegen-small.svg b/codex-rs/skills/src/assets/samples/imagegen/assets/imagegen-small.svg new file mode 100644 index 0000000000..20128b2dba --- /dev/null +++ b/codex-rs/skills/src/assets/samples/imagegen/assets/imagegen-small.svg @@ -0,0 +1,5 @@ + + + + + diff --git a/codex-rs/skills/src/assets/samples/imagegen/assets/imagegen.png b/codex-rs/skills/src/assets/samples/imagegen/assets/imagegen.png new file mode 100644 index 0000000000000000000000000000000000000000..94b54541a9affd39a7aa09d0efd5bc6b712b723b GIT binary patch literal 1711 zcmV;g22lBlP)f8*GWodgmJ0xDHBC~1!*z7`}xiD+*{^u{6Wtr9m>;F?6e6%akq_R>NQ97`^h zS_ISw!l_acdMv2|s@jkWFo}~maqQ{5Ar6jXJ8OGpcRf3wwDM+c*~)zT-u&HNh6Xr2 z$-B~6@6U&DoN@F6Tx_gMyw!zZkYd0r7n}J1r^VmNyNO5=4Zu(bgOk|-gzyRH_#DA0 ze5a&Dv5qs&Z`LEHCLtt(GYpH}0a%=n6p=Gpix@{jKAv3Z&a&_|v3nYpI$$>Jn72{}2rewsoH7VU@@*oo2>*cKtL zTT?Bf&S2fJxIt9+2RUqgo>5nJ#ub| zQq}-0z927LghkRij)2h@&){gs3Jyeb=#8#z9#4y@PugZO5lp1xm|ls&O&Ie1Y;Y<& zDJqnH7*0nhk28A~+ z7<=9c&7_PLLIN%we2x!9dQ!nmCW_&I_i2rm5DevDWF6nVJEu$r(Gy$4m3QXlck9AP zAru+93XZ;0rxYWFoZJ6W>sd_Seeu8x=)gE3r*<#NO*egQ8QEe}jd4PT_P&B!&~Me- za;t<zMu9~wY)fm}Rzy9AItra{)#n*Ft zp#x)wPKN*4hndv~RP#6!#f883EA5>ZW2_J=urHpxtu%d61x_+_HXOVgZ*U1pUyntk z&{EQR=i5oZ_4zJ5T+-jH*0PkegJp0u{3nitpQ5+rnfUw7Y~EbH6KNj9MD{&AT0ew2 zu6@1iCZ~3%@lofp<0)5aAWWoVxbvz*TZ$@0sgS;wrx=Pokgrae&K<&7=9qkS3pxKD zT|-~ns+{fX2)!(eSEue($$FZ zBMl2V75M|_V)ta4Qo-2jF}VemhGKH4^?}?}Lf=PnuV)4kMCz`oE{PPST1J60(ckfX z`wz;Zu9>J=2o=)R_FFPl>7=J+9#`9L1x`v$La6e-A_}RI%DWbVD8;dmf{b2KNNy4| zDUO8nhUTRVlHvtHlCq-^sw=LvdncnnP^459LaiNYP5Gq|f*_@$5Ngt%i`|9aFkMPn zAtXXqA43HuOW7i%(8r)u<#$Y#vMI#vK86ZRlp=+A)yGhQX;MO7_c2spk`%T$bD5iR zcFpAW%f^kirm}vO`, `image_2.`, and so on. +- Downscaled copies use the default suffix `-web` unless you override it. + +## Common recipes + +Generate with augmentation fields: + +```bash +python "$IMAGE_GEN" generate \ + --prompt "A minimal hero image of a ceramic coffee mug" \ + --use-case "product-mockup" \ + --style "clean product photography" \ + --composition "wide product shot with usable negative space for page copy" \ + --constraints "no logos, no text" \ + --out output/imagegen/mug-hero.png +``` + +Generate + also write a downscaled copy for fast web loading: + +```bash +python "$IMAGE_GEN" generate \ + --prompt "A cozy alpine cabin at dawn" \ + --size 1024x1024 \ + --downscale-max-dim 1024 \ + --out output/imagegen/alpine-cabin.png +``` + +Generate multiple prompts concurrently (async batch): + +```bash +mkdir -p tmp/imagegen output/imagegen/batch +cat > tmp/imagegen/prompts.jsonl << 'EOF' +{"prompt":"Cavernous hangar interior with a compact shuttle parked near the center","use_case":"stylized-concept","composition":"wide-angle, low-angle","lighting":"volumetric light rays through drifting fog","constraints":"no logos or trademarks; no watermark","size":"1536x1024"} +{"prompt":"Gray wolf in profile in a snowy forest","use_case":"photorealistic-natural","composition":"eye-level","constraints":"no logos or trademarks; no watermark","size":"1024x1024"} +EOF + +python "$IMAGE_GEN" generate-batch \ + --input tmp/imagegen/prompts.jsonl \ + --out-dir output/imagegen/batch \ + --concurrency 5 + +rm -f tmp/imagegen/prompts.jsonl +``` + +Notes: +- `generate-batch` requires `--out-dir`. +- generate-batch requires --out-dir. +- Use `--concurrency` to control parallelism (default `5`). +- Per-job overrides are supported in JSONL (for example `size`, `quality`, `background`, `output_format`, `output_compression`, `moderation`, `n`, `model`, `out`, and prompt-augmentation fields). +- `--n` generates multiple variants for a single prompt; `generate-batch` is for many different prompts. +- In batch mode, per-job `out` is treated as a filename under `--out-dir`. + +## CLI notes +- Supported sizes: `1024x1024`, `1536x1024`, `1024x1536`, or `auto`. +- Transparent backgrounds require `output_format` to be `png` or `webp`. +- `--prompt-file`, `--output-compression`, `--moderation`, `--max-attempts`, `--fail-fast`, `--force`, and `--no-augment` are supported. +- This CLI is intended for GPT Image models. Do not assume older non-GPT image-model behavior applies here. + +## See also +- API parameter quick reference for fallback CLI mode: `references/image-api.md` +- Prompt examples shared across both top-level modes: `references/sample-prompts.md` +- Network/sandbox notes for fallback CLI mode: `references/codex-network.md` diff --git a/codex-rs/skills/src/assets/samples/imagegen/references/codex-network.md b/codex-rs/skills/src/assets/samples/imagegen/references/codex-network.md new file mode 100644 index 0000000000..249d628262 --- /dev/null +++ b/codex-rs/skills/src/assets/samples/imagegen/references/codex-network.md @@ -0,0 +1,33 @@ +# Codex network approvals / sandbox notes + +This file is for the fallback CLI mode only. Read it only after the user explicitly asks to use `scripts/image_gen.py`. + +This guidance is intentionally isolated from `SKILL.md` because it can vary by environment and may become stale. Prefer the defaults in your environment when in doubt. + +## Why am I asked to approve image generation calls? +The fallback CLI uses the OpenAI Image API, so it needs outbound network access. In many Codex setups, network access is disabled by default and/or the approval policy requires confirmation before networked commands run. + +## Important note about approvals vs network +- `--ask-for-approval never` suppresses approval prompts. +- It does **not** by itself enable network access. +- In `workspace-write`, network access still depends on your Codex configuration (for example `[sandbox_workspace_write] network_access = true`). + +## How do I reduce repeated approval prompts? +If you trust the repo and want fewer prompts, use a configuration or profile that both: +- enables network for the sandbox mode you plan to use +- sets an approval policy that matches your risk tolerance + +Example `~/.codex/config.toml` pattern: + +```toml +approval_policy = "on-request" +sandbox_mode = "workspace-write" + +[sandbox_workspace_write] +network_access = true +``` + +If you want quieter automation after network is enabled, you can choose a stricter approval policy, but do that intentionally and with care. + +## Safety note +Enabling network and reducing approvals lowers friction, but increases risk if you run untrusted code or work in an untrusted repository. diff --git a/codex-rs/skills/src/assets/samples/imagegen/references/image-api.md b/codex-rs/skills/src/assets/samples/imagegen/references/image-api.md new file mode 100644 index 0000000000..a2750c15c6 --- /dev/null +++ b/codex-rs/skills/src/assets/samples/imagegen/references/image-api.md @@ -0,0 +1,49 @@ +# Image API quick reference + +This file is for the fallback CLI mode only. Use it only after the user explicitly asks to use `scripts/image_gen.py` instead of the built-in `image_gen` tool. + +These parameters describe the Image API and bundled CLI fallback surface. Do not assume they are normal arguments on the built-in `image_gen` tool. + +## Scope +- This fallback CLI is intended for GPT Image models (`gpt-image-1.5`, `gpt-image-1`, and `gpt-image-1-mini`). +- The built-in `image_gen` tool and the fallback CLI do not expose the same controls. + +## Endpoints +- Generate: `POST /v1/images/generations` (`client.images.generate(...)`) +- Edit: `POST /v1/images/edits` (`client.images.edit(...)`) + +## Core parameters for GPT Image models +- `prompt`: text prompt +- `model`: image model +- `n`: number of images (1-10) +- `size`: `1024x1024`, `1536x1024`, `1024x1536`, or `auto` +- `quality`: `low`, `medium`, `high`, or `auto` +- `background`: output transparency behavior (`transparent`, `opaque`, or `auto`) for generated output; this is not the same thing as the prompt's visual scene/backdrop +- `output_format`: `png` (default), `jpeg`, `webp` +- `output_compression`: 0-100 (jpeg/webp only) +- `moderation`: `auto` (default) or `low` + +## Edit-specific parameters +- `image`: one or more input images. For GPT Image models, you can provide up to 16 images. +- `mask`: optional mask image +- `input_fidelity`: `low` (default) or `high` + +Model-specific note for `input_fidelity`: +- `gpt-image-1` and `gpt-image-1-mini` preserve all input images, but the first image gets richer textures and finer details. +- `gpt-image-1.5` preserves the first 5 input images with higher fidelity. + +## Output +- `data[]` list with `b64_json` per image +- The bundled `scripts/image_gen.py` CLI decodes `b64_json` and writes output files for you. + +## Limits and notes +- Input images and masks must be under 50MB. +- Use the edits endpoint when the user requests changes to an existing image. +- Masking is prompt-guided; exact shapes are not guaranteed. +- Large sizes and high quality increase latency and cost. +- High `input_fidelity` can materially increase input token usage. +- If a request fails because a specific option is unsupported by the selected GPT Image model, retry manually without that option. + +## Important boundary +- `quality`, `input_fidelity`, explicit masks, `background`, `output_format`, and related parameters are fallback-only execution controls. +- Do not assume they are built-in `image_gen` tool arguments. diff --git a/codex-rs/skills/src/assets/samples/imagegen/references/prompting.md b/codex-rs/skills/src/assets/samples/imagegen/references/prompting.md new file mode 100644 index 0000000000..8b6c684eef --- /dev/null +++ b/codex-rs/skills/src/assets/samples/imagegen/references/prompting.md @@ -0,0 +1,98 @@ +# Prompting best practices + +These prompting principles are shared by both top-level modes of the skill: +- built-in `image_gen` tool (default) +- explicit `scripts/image_gen.py` CLI fallback + +This file is about prompt structure, specificity, and iteration. Fallback-only execution controls such as `quality`, `input_fidelity`, masks, output format, and output paths live in the fallback docs. + +## Contents +- [Structure](#structure) +- [Specificity policy](#specificity-policy) +- [Allowed and disallowed augmentation](#allowed-and-disallowed-augmentation) +- [Composition and layout](#composition-and-layout) +- [Constraints and invariants](#constraints-and-invariants) +- [Text in images](#text-in-images) +- [Input images and references](#input-images-and-references) +- [Iterate deliberately](#iterate-deliberately) +- [Fallback-only execution controls](#fallback-only-execution-controls) +- [Use-case tips](#use-case-tips) +- [Where to find copy/paste recipes](#where-to-find-copypaste-recipes) + +## Structure +- Use a consistent order: scene/backdrop -> subject -> key details -> constraints -> output intent. +- Include intended use (ad, UI mock, infographic) to set the level of polish. +- For complex requests, use short labeled lines instead of one long paragraph. + +## Specificity policy +- If the user prompt is already specific and detailed, normalize it into a clean spec without adding creative requirements. +- If the prompt is generic, you may add tasteful detail when it materially improves the output. +- Treat examples in `sample-prompts.md` as fully-authored recipes, not as the default amount of augmentation to add to every request. + +## Allowed and disallowed augmentation + +Allowed augmentation for generic prompts: +- composition and framing cues +- intended-use or polish-level hints +- practical layout guidance +- reasonable scene concreteness that supports the request + +Do not add: +- extra characters, props, or objects that are not implied +- brand palettes, slogans, or story beats that are not implied +- arbitrary side-specific placement unless the surrounding layout supports it + +## Composition and layout +- Specify framing and viewpoint (close-up, wide, top-down) and placement only when it materially helps. +- Call out negative space if the asset clearly needs room for UI or copy. +- Avoid making left/right layout decisions unless the user or surrounding layout supports them. + +## Constraints and invariants +- State what must not change (`keep background unchanged`). +- For edits, say `change only X; keep Y unchanged` and repeat invariants on every iteration to reduce drift. + +## Text in images +- Put literal text in quotes or ALL CAPS and specify typography (font style, size, color, placement). +- Spell uncommon words letter-by-letter if accuracy matters. +- For in-image copy, require verbatim rendering and no extra characters. + +## Input images and references +- Do not assume that every provided image is an edit target. +- Label each image by index and role (`Image 1: edit target`, `Image 2: style reference`). +- If the user provides images for style, composition, or mood guidance and does not ask to modify them, treat the request as generation with references. +- If the user asks to preserve an existing image while changing specific parts, treat the request as an edit. +- For compositing, describe how the images interact (`place the subject from Image 2 into Image 1`). + +## Iterate deliberately +- Start with a clean base prompt, then make small single-change edits. +- Re-specify critical constraints when you iterate. +- Prefer one targeted follow-up at a time over rewriting the whole prompt. + +## Fallback-only execution controls +- `quality`, `input_fidelity`, explicit masks, output format, and output paths are fallback-only execution controls. +- Do not assume they are built-in `image_gen` tool arguments. +- If the user explicitly chooses CLI fallback, see `references/cli.md` and `references/image-api.md` for those controls. + +## Use-case tips +Generate: +- photorealistic-natural: Prompt as if a real photo is captured in the moment; use photography language (lens, lighting, framing); call for real texture; avoid over-stylized polish unless requested. +- product-mockup: Describe the product/packaging and materials; ensure clean silhouette and label clarity; if in-image text is needed, require verbatim rendering and specify typography. +- ui-mockup: Describe the target fidelity first (shippable mockup or low-fi wireframe), then focus on layout, hierarchy, and practical UI elements; avoid concept-art language. +- infographic-diagram: Define the audience and layout flow; label parts explicitly; require verbatim text. +- logo-brand: Keep it simple and scalable; ask for a strong silhouette and balanced negative space; avoid decorative flourishes unless requested. +- illustration-story: Define panels or scene beats; keep each action concrete. +- stylized-concept: Specify style cues, material finish, and rendering approach (3D, painterly, clay) without inventing new story elements. +- historical-scene: State the location/date and required period accuracy; constrain clothing, props, and environment to match the era. + +Edit: +- text-localization: Change only the text; preserve layout, typography, spacing, and hierarchy; no extra words or reflow unless needed. +- identity-preserve: Lock identity (face, body, pose, hair, expression); change only the specified elements; match lighting and shadows. +- precise-object-edit: Specify exactly what to remove/replace; preserve surrounding texture and lighting; keep everything else unchanged. +- lighting-weather: Change only environmental conditions (light, shadows, atmosphere, precipitation); keep geometry, framing, and subject identity. +- background-extraction: Request a clean cutout; crisp silhouette; no halos; preserve label text exactly; no restyling. +- style-transfer: Specify style cues to preserve (palette, texture, brushwork) and what must change; add `no extra elements` to prevent drift. +- compositing: Reference inputs by index; specify what moves where; match lighting, perspective, and scale; keep the base framing unchanged. +- sketch-to-render: Preserve layout, proportions, and perspective; choose materials and lighting that support the supplied sketch without adding new elements. + +## Where to find copy/paste recipes +For copy/paste prompt specs (examples only), see `references/sample-prompts.md`. This file focuses on principles, specificity, and iteration patterns. diff --git a/codex-rs/skills/src/assets/samples/imagegen/references/sample-prompts.md b/codex-rs/skills/src/assets/samples/imagegen/references/sample-prompts.md new file mode 100644 index 0000000000..e4b2b01da0 --- /dev/null +++ b/codex-rs/skills/src/assets/samples/imagegen/references/sample-prompts.md @@ -0,0 +1,376 @@ +# Sample prompts (copy/paste) + +These prompt recipes are shared across both top-level modes of the skill: +- built-in `image_gen` tool (default) +- explicit `scripts/image_gen.py` CLI fallback + +Use these as starting points. They are intentionally complete prompt recipes, not the default amount of augmentation to add to every user request. + +When adapting a user's prompt: +- keep user-provided requirements +- only add detail according to the specificity policy in `SKILL.md` +- do not treat every example below as permission to invent extra story elements + +The labeled lines are prompt scaffolding, not a closed schema. `Asset type` and `Input images` are prompt-only scaffolding; the CLI does not expose them as dedicated flags. + +Execution details such as explicit CLI flags, `quality`, `input_fidelity`, masks, output formats, and local output paths depend on mode. Use the built-in tool by default; only apply CLI-specific controls after the user explicitly opts into fallback mode. + +For prompting principles (structure, specificity, invariants, iteration), see `references/prompting.md`. + +## Generate + +### photorealistic-natural +``` +Use case: photorealistic-natural +Primary request: candid photo of an elderly sailor on a small fishing boat adjusting a net +Scene/backdrop: coastal water with soft haze +Subject: weathered skin with wrinkles and sun texture +Style/medium: photorealistic candid photo +Composition/framing: medium close-up, eye-level +Lighting/mood: soft coastal daylight, shallow depth of field, subtle film grain +Materials/textures: real skin texture, worn fabric, salt-worn wood +Constraints: natural color balance; no heavy retouching; no glamorization; no watermark +Avoid: studio polish; staged look +``` + +### product-mockup +``` +Use case: product-mockup +Primary request: premium product photo of a matte black shampoo bottle with a minimal label +Scene/backdrop: clean studio gradient from light gray to white +Subject: single bottle centered with subtle reflection +Style/medium: premium product photography +Composition/framing: centered, slight three-quarter angle, generous padding +Lighting/mood: softbox lighting, clean highlights, controlled shadows +Materials/textures: matte plastic, crisp label printing +Constraints: no logos or trademarks; no watermark +``` + +### ui-mockup +``` +Use case: ui-mockup +Primary request: mobile app home screen for a local farmers market with vendors and daily specials +Asset type: mobile app screen +Style/medium: realistic product UI, not concept art +Composition/framing: clean vertical mobile layout with clear hierarchy +Constraints: practical layout, clear typography, no logos or trademarks, no watermark +``` + +### infographic-diagram +``` +Use case: infographic-diagram +Primary request: detailed infographic of an automatic coffee machine flow +Scene/backdrop: clean, light neutral background +Subject: bean hopper -> grinder -> brew group -> boiler -> water tank -> drip tray +Style/medium: clean vector-like infographic with clear callouts and arrows +Composition/framing: vertical poster layout, top-to-bottom flow +Text (verbatim): "Bean Hopper", "Grinder", "Brew Group", "Boiler", "Water Tank", "Drip Tray" +Constraints: clear labels, strong contrast, no logos or trademarks, no watermark +``` + +### logo-brand +``` +Use case: logo-brand +Primary request: original logo for "Field & Flour", a local bakery +Style/medium: vector logo mark; flat colors; minimal +Composition/framing: single centered logo on a plain background with generous padding +Constraints: strong silhouette, balanced negative space; original design only; no gradients unless essential; no trademarks; no watermark +``` + +### illustration-story +``` +Use case: illustration-story +Primary request: 4-panel comic about a pet left alone at home +Scene/backdrop: cozy living room across panels +Subject: pet reacting to the owner leaving, then relaxing, then returning to a composed pose +Style/medium: comic illustration with clear panels +Composition/framing: 4 equal-sized vertical panels, readable actions per panel +Constraints: no text; no logos or trademarks; no watermark +``` + +### stylized-concept +``` +Use case: stylized-concept +Primary request: cavernous hangar interior with tall support beams and drifting fog +Scene/backdrop: industrial hangar interior, deep scale, light haze +Subject: compact shuttle parked near the center +Style/medium: cinematic concept art, industrial realism +Composition/framing: wide-angle, low-angle +Lighting/mood: volumetric light rays cutting through fog +Constraints: no logos or trademarks; no watermark +``` + +### historical-scene +``` +Use case: historical-scene +Primary request: outdoor crowd scene in Bethel, New York on August 16, 1969 +Scene/backdrop: open field with period-appropriate staging +Subject: crowd in period-accurate clothing, authentic environment +Style/medium: photorealistic photo +Composition/framing: wide shot, eye-level +Constraints: period-accurate details; no modern objects; no logos or trademarks; no watermark +``` + +## Asset type templates (taxonomy-aligned) + +### Website assets template +``` +Use case: +Asset type: +Primary request: +Scene/backdrop: +Subject:
+Style/medium: +Composition/framing: +Lighting/mood: +Color palette: +Constraints: +``` + +### Website assets example: minimal hero background +``` +Use case: stylized-concept +Asset type: landing page hero background +Primary request: minimal abstract background with a soft gradient and subtle texture +Style/medium: matte illustration / soft-rendered abstract background +Composition/framing: wide composition with usable negative space for page copy +Lighting/mood: gentle studio glow +Color palette: restrained neutral palette +Constraints: no text; no logos; no watermark +``` + +### Website assets example: feature section illustration +``` +Use case: stylized-concept +Asset type: feature section illustration +Primary request: simple abstract shapes suggesting connection and flow +Scene/backdrop: subtle light-gray backdrop with faint texture +Style/medium: flat illustration; soft shadows; restrained contrast +Composition/framing: centered cluster; open margins for UI +Color palette: muted neutral palette +Constraints: no text; no logos; no watermark +``` + +### Website assets example: blog header image +``` +Use case: photorealistic-natural +Asset type: blog header image +Primary request: overhead desk scene with notebook, pen, and coffee cup +Scene/backdrop: warm wooden tabletop +Style/medium: photorealistic photo +Composition/framing: wide crop with clean room for page copy +Lighting/mood: soft morning light +Constraints: no text; no logos; no watermark +``` + +### Game assets template +``` +Use case: stylized-concept +Asset type: +Primary request: +Scene/backdrop: (if applicable) +Subject:
+Style/medium: ; +Composition/framing: ; ; +Lighting/mood: