mirror of
https://github.com/openai/codex.git
synced 2026-09-17 12:23:33 +00:00
Merge 4c82f9a609 into sapling-pr-archive-bolinfest
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -1189,6 +1189,7 @@ dependencies = [
|
||||
"anyhow",
|
||||
"clap",
|
||||
"codex-core",
|
||||
"codex-execpolicy2",
|
||||
"libc",
|
||||
"path-absolutize",
|
||||
"pretty_assertions",
|
||||
|
||||
@@ -67,6 +67,7 @@ codex-chatgpt = { path = "chatgpt" }
|
||||
codex-common = { path = "common" }
|
||||
codex-core = { path = "core" }
|
||||
codex-exec = { path = "exec" }
|
||||
codex-execpolicy2 = { path = "execpolicy2" }
|
||||
codex-feedback = { path = "feedback" }
|
||||
codex-file-search = { path = "file-search" }
|
||||
codex-git = { path = "utils/git" }
|
||||
|
||||
@@ -531,9 +531,11 @@ pub struct AccountUpdatedNotification {
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct Turn {
|
||||
pub id: String,
|
||||
/// This is currently only populated for resumed threads.
|
||||
/// TODO: properly populate items for all turns.
|
||||
pub items: Vec<ThreadItem>,
|
||||
#[serde(flatten)]
|
||||
pub status: TurnStatus,
|
||||
pub error: Option<TurnError>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
@@ -544,12 +546,12 @@ pub struct TurnError {
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
#[serde(tag = "status", rename_all = "camelCase")]
|
||||
#[ts(tag = "status", export_to = "v2/")]
|
||||
pub enum TurnStatus {
|
||||
Completed,
|
||||
Interrupted,
|
||||
Failed,
|
||||
Failed { error: TurnError },
|
||||
InProgress,
|
||||
}
|
||||
|
||||
@@ -865,8 +867,6 @@ pub struct Usage {
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct TurnCompletedNotification {
|
||||
pub turn: Turn,
|
||||
// TODO: should usage be stored on the Turn object, and we return that instead?
|
||||
pub usage: Usage,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
|
||||
@@ -46,6 +46,7 @@ use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::UserInput as V2UserInput;
|
||||
use codex_protocol::ConversationId;
|
||||
use codex_protocol::protocol::Event;
|
||||
@@ -509,10 +510,9 @@ impl CodexClient {
|
||||
ServerNotification::TurnCompleted(payload) => {
|
||||
if payload.turn.id == turn_id {
|
||||
println!("\n< turn/completed notification: {:?}", payload.turn.status);
|
||||
if let Some(error) = payload.turn.error {
|
||||
if let TurnStatus::Failed { error } = &payload.turn.status {
|
||||
println!("[turn error] {}", error.message);
|
||||
}
|
||||
println!("< usage: {:?}", payload.usage);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use crate::codex_message_processor::ApiVersion;
|
||||
use crate::codex_message_processor::PendingInterrupts;
|
||||
use crate::codex_message_processor::TurnSummary;
|
||||
use crate::codex_message_processor::TurnSummaryStore;
|
||||
use crate::outgoing_message::OutgoingMessageSender;
|
||||
use codex_app_server_protocol::AccountRateLimitsUpdatedNotification;
|
||||
use codex_app_server_protocol::AgentMessageDeltaNotification;
|
||||
@@ -26,7 +28,11 @@ use codex_app_server_protocol::SandboxCommandAssessment as V2SandboxCommandAsses
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequestPayload;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::Turn;
|
||||
use codex_app_server_protocol::TurnCompletedNotification;
|
||||
use codex_app_server_protocol::TurnError;
|
||||
use codex_app_server_protocol::TurnInterruptResponse;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_core::CodexConversation;
|
||||
use codex_core::parse_command::shlex_join;
|
||||
use codex_core::protocol::ApplyPatchApprovalRequestEvent;
|
||||
@@ -54,10 +60,14 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
conversation: Arc<CodexConversation>,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
pending_interrupts: PendingInterrupts,
|
||||
turn_summary_store: TurnSummaryStore,
|
||||
api_version: ApiVersion,
|
||||
) {
|
||||
let Event { id: event_id, msg } = event;
|
||||
match msg {
|
||||
EventMsg::TaskComplete(_ev) => {
|
||||
handle_turn_complete(conversation_id, event_id, &outgoing, &turn_summary_store).await;
|
||||
}
|
||||
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id,
|
||||
changes,
|
||||
@@ -191,6 +201,9 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
.await;
|
||||
}
|
||||
}
|
||||
EventMsg::Error(ev) => {
|
||||
handle_error(conversation_id, ev.message, &turn_summary_store).await;
|
||||
}
|
||||
EventMsg::EnteredReviewMode(review_request) => {
|
||||
let notification = ItemStartedNotification {
|
||||
item: ThreadItem::CodeReview {
|
||||
@@ -326,12 +339,79 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
handle_turn_interrupted(conversation_id, event_id, &outgoing, &turn_summary_store)
|
||||
.await;
|
||||
}
|
||||
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
async fn emit_turn_completed_with_status(
|
||||
event_id: String,
|
||||
status: TurnStatus,
|
||||
outgoing: &OutgoingMessageSender,
|
||||
) {
|
||||
let notification = TurnCompletedNotification {
|
||||
turn: Turn {
|
||||
id: event_id,
|
||||
items: vec![],
|
||||
status,
|
||||
},
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::TurnCompleted(notification))
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn find_and_remove_turn_summary(
|
||||
conversation_id: ConversationId,
|
||||
turn_summary_store: &TurnSummaryStore,
|
||||
) -> TurnSummary {
|
||||
let mut map = turn_summary_store.lock().await;
|
||||
map.remove(&conversation_id).unwrap_or_default()
|
||||
}
|
||||
|
||||
async fn handle_turn_complete(
|
||||
conversation_id: ConversationId,
|
||||
event_id: String,
|
||||
outgoing: &OutgoingMessageSender,
|
||||
turn_summary_store: &TurnSummaryStore,
|
||||
) {
|
||||
let turn_summary = find_and_remove_turn_summary(conversation_id, turn_summary_store).await;
|
||||
|
||||
let status = if let Some(message) = turn_summary.last_error_message {
|
||||
TurnStatus::Failed {
|
||||
error: TurnError { message },
|
||||
}
|
||||
} else {
|
||||
TurnStatus::Completed
|
||||
};
|
||||
|
||||
emit_turn_completed_with_status(event_id, status, outgoing).await;
|
||||
}
|
||||
|
||||
async fn handle_turn_interrupted(
|
||||
conversation_id: ConversationId,
|
||||
event_id: String,
|
||||
outgoing: &OutgoingMessageSender,
|
||||
turn_summary_store: &TurnSummaryStore,
|
||||
) {
|
||||
find_and_remove_turn_summary(conversation_id, turn_summary_store).await;
|
||||
|
||||
emit_turn_completed_with_status(event_id, TurnStatus::Interrupted, outgoing).await;
|
||||
}
|
||||
|
||||
async fn handle_error(
|
||||
conversation_id: ConversationId,
|
||||
message: String,
|
||||
turn_summary_store: &TurnSummaryStore,
|
||||
) {
|
||||
let mut map = turn_summary_store.lock().await;
|
||||
map.entry(conversation_id).or_default().last_error_message = Some(message);
|
||||
}
|
||||
|
||||
async fn on_patch_approval_response(
|
||||
event_id: String,
|
||||
receiver: oneshot::Receiver<JsonValue>,
|
||||
@@ -536,13 +616,140 @@ async fn construct_mcp_tool_call_end_notification(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::CHANNEL_CAPACITY;
|
||||
use crate::outgoing_message::OutgoingMessage;
|
||||
use crate::outgoing_message::OutgoingMessageSender;
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use anyhow::bail;
|
||||
use codex_core::protocol::McpInvocation;
|
||||
use mcp_types::CallToolResult;
|
||||
use mcp_types::ContentBlock;
|
||||
use mcp_types::TextContent;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::collections::HashMap;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::mpsc;
|
||||
|
||||
fn new_turn_summary_store() -> TurnSummaryStore {
|
||||
Arc::new(Mutex::new(HashMap::new()))
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_handle_error_records_message() -> Result<()> {
|
||||
let conversation_id = ConversationId::new();
|
||||
let turn_summary_store = new_turn_summary_store();
|
||||
|
||||
handle_error(conversation_id, "boom".to_string(), &turn_summary_store).await;
|
||||
|
||||
let turn_summary = find_and_remove_turn_summary(conversation_id, &turn_summary_store).await;
|
||||
assert_eq!(turn_summary.last_error_message, Some("boom".to_string()));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_handle_turn_complete_emits_completed_without_error() -> Result<()> {
|
||||
let conversation_id = ConversationId::new();
|
||||
let event_id = "complete1".to_string();
|
||||
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
|
||||
let outgoing = Arc::new(OutgoingMessageSender::new(tx));
|
||||
let turn_summary_store = new_turn_summary_store();
|
||||
|
||||
handle_turn_complete(
|
||||
conversation_id,
|
||||
event_id.clone(),
|
||||
&outgoing,
|
||||
&turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
|
||||
let msg = rx
|
||||
.recv()
|
||||
.await
|
||||
.ok_or_else(|| anyhow!("should send one notification"))?;
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, event_id);
|
||||
assert_eq!(n.turn.status, TurnStatus::Completed);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
assert!(rx.try_recv().is_err(), "no extra messages expected");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_handle_turn_interrupted_emits_interrupted_with_error() -> Result<()> {
|
||||
let conversation_id = ConversationId::new();
|
||||
let event_id = "interrupt1".to_string();
|
||||
let turn_summary_store = new_turn_summary_store();
|
||||
handle_error(conversation_id, "oops".to_string(), &turn_summary_store).await;
|
||||
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
|
||||
let outgoing = Arc::new(OutgoingMessageSender::new(tx));
|
||||
|
||||
handle_turn_interrupted(
|
||||
conversation_id,
|
||||
event_id.clone(),
|
||||
&outgoing,
|
||||
&turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
|
||||
let msg = rx
|
||||
.recv()
|
||||
.await
|
||||
.ok_or_else(|| anyhow!("should send one notification"))?;
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, event_id);
|
||||
assert_eq!(n.turn.status, TurnStatus::Interrupted);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
assert!(rx.try_recv().is_err(), "no extra messages expected");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_handle_turn_complete_emits_failed_with_error() -> Result<()> {
|
||||
let conversation_id = ConversationId::new();
|
||||
let event_id = "complete_err1".to_string();
|
||||
let turn_summary_store = new_turn_summary_store();
|
||||
handle_error(conversation_id, "bad".to_string(), &turn_summary_store).await;
|
||||
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
|
||||
let outgoing = Arc::new(OutgoingMessageSender::new(tx));
|
||||
|
||||
handle_turn_complete(
|
||||
conversation_id,
|
||||
event_id.clone(),
|
||||
&outgoing,
|
||||
&turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
|
||||
let msg = rx
|
||||
.recv()
|
||||
.await
|
||||
.ok_or_else(|| anyhow!("should send one notification"))?;
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, event_id);
|
||||
assert_eq!(
|
||||
n.turn.status,
|
||||
TurnStatus::Failed {
|
||||
error: TurnError {
|
||||
message: "bad".to_string(),
|
||||
}
|
||||
}
|
||||
);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
assert!(rx.try_recv().is_err(), "no extra messages expected");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_construct_mcp_tool_call_begin_notification_with_args() {
|
||||
@@ -572,6 +779,105 @@ mod tests {
|
||||
assert_eq!(notification, expected);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_handle_turn_complete_emits_error_multiple_turns() -> Result<()> {
|
||||
// Conversation A will have two turns; Conversation B will have one turn.
|
||||
let conversation_a = ConversationId::new();
|
||||
let conversation_b = ConversationId::new();
|
||||
let turn_summary_store = new_turn_summary_store();
|
||||
|
||||
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
|
||||
let outgoing = Arc::new(OutgoingMessageSender::new(tx));
|
||||
|
||||
// Turn 1 on conversation A
|
||||
let a_turn1 = "a_turn1".to_string();
|
||||
handle_error(conversation_a, "a1".to_string(), &turn_summary_store).await;
|
||||
handle_turn_complete(
|
||||
conversation_a,
|
||||
a_turn1.clone(),
|
||||
&outgoing,
|
||||
&turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
|
||||
// Turn 1 on conversation B
|
||||
let b_turn1 = "b_turn1".to_string();
|
||||
handle_error(conversation_b, "b1".to_string(), &turn_summary_store).await;
|
||||
handle_turn_complete(
|
||||
conversation_b,
|
||||
b_turn1.clone(),
|
||||
&outgoing,
|
||||
&turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
|
||||
// Turn 2 on conversation A
|
||||
let a_turn2 = "a_turn2".to_string();
|
||||
handle_turn_complete(
|
||||
conversation_a,
|
||||
a_turn2.clone(),
|
||||
&outgoing,
|
||||
&turn_summary_store,
|
||||
)
|
||||
.await;
|
||||
|
||||
// Verify: A turn 1
|
||||
let msg = rx
|
||||
.recv()
|
||||
.await
|
||||
.ok_or_else(|| anyhow!("should send first notification"))?;
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, a_turn1);
|
||||
assert_eq!(
|
||||
n.turn.status,
|
||||
TurnStatus::Failed {
|
||||
error: TurnError {
|
||||
message: "a1".to_string(),
|
||||
}
|
||||
}
|
||||
);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
|
||||
// Verify: B turn 1
|
||||
let msg = rx
|
||||
.recv()
|
||||
.await
|
||||
.ok_or_else(|| anyhow!("should send second notification"))?;
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, b_turn1);
|
||||
assert_eq!(
|
||||
n.turn.status,
|
||||
TurnStatus::Failed {
|
||||
error: TurnError {
|
||||
message: "b1".to_string(),
|
||||
}
|
||||
}
|
||||
);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
|
||||
// Verify: A turn 2
|
||||
let msg = rx
|
||||
.recv()
|
||||
.await
|
||||
.ok_or_else(|| anyhow!("should send third notification"))?;
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, a_turn2);
|
||||
assert_eq!(n.turn.status, TurnStatus::Completed);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
|
||||
assert!(rx.try_recv().is_err(), "no extra messages expected");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_construct_mcp_tool_call_begin_notification_without_args() {
|
||||
let begin_event = McpToolCallBeginEvent {
|
||||
|
||||
@@ -155,6 +155,14 @@ use uuid::Uuid;
|
||||
type PendingInterruptQueue = Vec<(RequestId, ApiVersion)>;
|
||||
pub(crate) type PendingInterrupts = Arc<Mutex<HashMap<ConversationId, PendingInterruptQueue>>>;
|
||||
|
||||
/// Per-conversation accumulation of the latest states e.g. error message while a turn runs.
|
||||
#[derive(Default, Clone)]
|
||||
pub(crate) struct TurnSummary {
|
||||
pub(crate) last_error_message: Option<String>,
|
||||
}
|
||||
|
||||
pub(crate) type TurnSummaryStore = Arc<Mutex<HashMap<ConversationId, TurnSummary>>>;
|
||||
|
||||
// Duration before a ChatGPT login attempt is abandoned.
|
||||
const LOGIN_CHATGPT_TIMEOUT: Duration = Duration::from_secs(10 * 60);
|
||||
struct ActiveLogin {
|
||||
@@ -179,6 +187,7 @@ pub(crate) struct CodexMessageProcessor {
|
||||
active_login: Arc<Mutex<Option<ActiveLogin>>>,
|
||||
// Queue of pending interrupt requests per conversation. We reply when TurnAborted arrives.
|
||||
pending_interrupts: PendingInterrupts,
|
||||
turn_summary_store: TurnSummaryStore,
|
||||
pending_fuzzy_searches: Arc<Mutex<HashMap<String, Arc<AtomicBool>>>>,
|
||||
feedback: CodexFeedback,
|
||||
}
|
||||
@@ -231,6 +240,7 @@ impl CodexMessageProcessor {
|
||||
conversation_listeners: HashMap::new(),
|
||||
active_login: Arc::new(Mutex::new(None)),
|
||||
pending_interrupts: Arc::new(Mutex::new(HashMap::new())),
|
||||
turn_summary_store: Arc::new(Mutex::new(HashMap::new())),
|
||||
pending_fuzzy_searches: Arc::new(Mutex::new(HashMap::new())),
|
||||
feedback,
|
||||
}
|
||||
@@ -2390,9 +2400,6 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
};
|
||||
|
||||
// Keep a copy of v2 inputs for the notification payload.
|
||||
let v2_inputs_for_notif = params.input.clone();
|
||||
|
||||
// Map v2 input items to core input items.
|
||||
let mapped_items: Vec<CoreInputItem> = params
|
||||
.input
|
||||
@@ -2432,12 +2439,8 @@ impl CodexMessageProcessor {
|
||||
Ok(turn_id) => {
|
||||
let turn = Turn {
|
||||
id: turn_id.clone(),
|
||||
items: vec![ThreadItem::UserMessage {
|
||||
id: turn_id,
|
||||
content: v2_inputs_for_notif,
|
||||
}],
|
||||
items: vec![],
|
||||
status: TurnStatus::InProgress,
|
||||
error: None,
|
||||
};
|
||||
|
||||
let response = TurnStartResponse { turn: turn.clone() };
|
||||
@@ -2498,7 +2501,6 @@ impl CodexMessageProcessor {
|
||||
id: turn_id.clone(),
|
||||
items,
|
||||
status: TurnStatus::InProgress,
|
||||
error: None,
|
||||
};
|
||||
let response = TurnStartResponse { turn: turn.clone() };
|
||||
self.outgoing.send_response(request_id, response).await;
|
||||
@@ -2618,6 +2620,7 @@ impl CodexMessageProcessor {
|
||||
|
||||
let outgoing_for_task = self.outgoing.clone();
|
||||
let pending_interrupts = self.pending_interrupts.clone();
|
||||
let turn_summary_store = self.turn_summary_store.clone();
|
||||
let api_version_for_task = api_version;
|
||||
tokio::spawn(async move {
|
||||
loop {
|
||||
@@ -2674,6 +2677,7 @@ impl CodexMessageProcessor {
|
||||
conversation.clone(),
|
||||
outgoing_for_task.clone(),
|
||||
pending_interrupts.clone(),
|
||||
turn_summary_store.clone(),
|
||||
api_version_for_task,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -5,14 +5,17 @@ use app_test_support::McpProcess;
|
||||
use app_test_support::create_mock_chat_completions_server;
|
||||
use app_test_support::create_shell_sse_response;
|
||||
use app_test_support::to_response;
|
||||
use codex_app_server_protocol::JSONRPCNotification;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_app_server_protocol::TurnCompletedNotification;
|
||||
use codex_app_server_protocol::TurnInterruptParams;
|
||||
use codex_app_server_protocol::TurnInterruptResponse;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::UserInput as V2UserInput;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::timeout;
|
||||
@@ -99,7 +102,18 @@ async fn turn_interrupt_aborts_running_turn() -> Result<()> {
|
||||
.await??;
|
||||
let _resp: TurnInterruptResponse = to_response::<TurnInterruptResponse>(interrupt_resp)?;
|
||||
|
||||
// No fields to assert on; successful deserialization confirms proper response shape.
|
||||
let completed_notif: JSONRPCNotification = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
let completed: TurnCompletedNotification = serde_json::from_value(
|
||||
completed_notif
|
||||
.params
|
||||
.expect("turn/completed params must be present"),
|
||||
)?;
|
||||
assert_eq!(completed.turn.status, TurnStatus::Interrupted);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -14,9 +14,11 @@ use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_app_server_protocol::TurnCompletedNotification;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::TurnStartedNotification;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::UserInput as V2UserInput;
|
||||
use codex_core::protocol_config_types::ReasoningEffort;
|
||||
use codex_core::protocol_config_types::ReasoningSummary;
|
||||
@@ -118,13 +120,17 @@ async fn turn_start_emits_notifications_and_accepts_model_override() -> Result<(
|
||||
)
|
||||
.await??;
|
||||
|
||||
// And we should ultimately get a task_complete without having to add a
|
||||
// legacy conversation listener explicitly (auto-attached by thread/start).
|
||||
let _task_complete: JSONRPCNotification = timeout(
|
||||
let completed_notif: JSONRPCNotification = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
let completed: TurnCompletedNotification = serde_json::from_value(
|
||||
completed_notif
|
||||
.params
|
||||
.expect("turn/completed params must be present"),
|
||||
)?;
|
||||
assert_eq!(completed.turn.status, TurnStatus::Completed);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -274,6 +280,11 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> {
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
// Second turn with approval_policy=never should not elicit approval
|
||||
let second_turn_id = mcp
|
||||
@@ -302,6 +313,11 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> {
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -12,6 +12,8 @@ use serde::Serialize;
|
||||
|
||||
#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)]
|
||||
pub struct CreditStatusDetails {
|
||||
#[serde(rename = "has_credits")]
|
||||
pub has_credits: bool,
|
||||
#[serde(rename = "unlimited")]
|
||||
pub unlimited: bool,
|
||||
#[serde(
|
||||
@@ -38,8 +40,9 @@ pub struct CreditStatusDetails {
|
||||
}
|
||||
|
||||
impl CreditStatusDetails {
|
||||
pub fn new(unlimited: bool) -> CreditStatusDetails {
|
||||
pub fn new(has_credits: bool, unlimited: bool) -> CreditStatusDetails {
|
||||
CreditStatusDetails {
|
||||
has_credits,
|
||||
unlimited,
|
||||
balance: None,
|
||||
approx_local_messages: None,
|
||||
|
||||
@@ -313,7 +313,7 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
id: Feature::RemoteCompaction,
|
||||
key: "remote_compaction",
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ParallelToolCalls,
|
||||
|
||||
@@ -14,6 +14,7 @@ workspace = true
|
||||
anyhow = { workspace = true }
|
||||
clap = { workspace = true, features = ["derive"] }
|
||||
codex-core = { workspace = true }
|
||||
codex-execpolicy2 = { workspace = true }
|
||||
libc = { workspace = true }
|
||||
path-absolutize = { workspace = true }
|
||||
rmcp = { workspace = true, default-features = false, features = [
|
||||
|
||||
@@ -59,11 +59,12 @@ use std::path::Path;
|
||||
|
||||
use clap::Parser;
|
||||
use clap::Subcommand;
|
||||
use codex_execpolicy2::Decision;
|
||||
use tracing_subscriber::EnvFilter;
|
||||
use tracing_subscriber::{self};
|
||||
|
||||
use crate::posix::escalate_protocol::EscalateAction;
|
||||
use crate::posix::escalate_server::EscalateServer;
|
||||
use crate::posix::escalate_server::ExecPolicyOutcome;
|
||||
|
||||
mod escalate_client;
|
||||
mod escalate_protocol;
|
||||
@@ -71,16 +72,28 @@ mod escalate_server;
|
||||
mod mcp;
|
||||
mod socket;
|
||||
|
||||
fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> EscalateAction {
|
||||
fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> ExecPolicyOutcome {
|
||||
// TODO: execpolicy
|
||||
if file == Path::new("/opt/homebrew/bin/gh")
|
||||
if file.ends_with("/git") {
|
||||
ExecPolicyOutcome {
|
||||
decision: Decision::Prompt,
|
||||
run_with_escalated_permissions: false,
|
||||
}
|
||||
} else if file == Path::new("/opt/homebrew/bin/gh")
|
||||
&& let [_, arg1, arg2, ..] = argv
|
||||
&& arg1 == "issue"
|
||||
&& arg2 == "list"
|
||||
{
|
||||
return EscalateAction::Escalate;
|
||||
ExecPolicyOutcome {
|
||||
decision: Decision::Allow,
|
||||
run_with_escalated_permissions: true,
|
||||
}
|
||||
} else {
|
||||
ExecPolicyOutcome {
|
||||
decision: Decision::Allow,
|
||||
run_with_escalated_permissions: false,
|
||||
}
|
||||
}
|
||||
EscalateAction::Run
|
||||
}
|
||||
|
||||
#[derive(Parser)]
|
||||
|
||||
@@ -98,5 +98,8 @@ pub(crate) async fn run(file: String, argv: Vec<String>) -> anyhow::Result<i32>
|
||||
|
||||
Err(err.into())
|
||||
}
|
||||
EscalateAction::Deny => {
|
||||
todo!("handle EscalateAction::Deny")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,6 +34,8 @@ pub(super) enum EscalateAction {
|
||||
Run,
|
||||
/// The command should be escalated to the server for execution.
|
||||
Escalate,
|
||||
/// The command should be denied.
|
||||
Deny,
|
||||
}
|
||||
|
||||
/// The client sends this to the server to forward its open FDs.
|
||||
|
||||
@@ -12,6 +12,7 @@ use codex_core::exec::SandboxType;
|
||||
use codex_core::exec::process_exec_tool_call;
|
||||
use codex_core::get_platform_sandbox;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use codex_execpolicy2::Decision as ExecPolicyDecision;
|
||||
use tokio::process::Command;
|
||||
|
||||
use crate::posix::escalate_protocol::BASH_EXEC_WRAPPER_ENV_VAR;
|
||||
@@ -30,7 +31,13 @@ use crate::posix::socket::AsyncSocket;
|
||||
/// `argv` is the argv, including the program name (`argv[0]`).
|
||||
/// `workdir` is the absolute, canonical path to the working directory in which to execute the
|
||||
/// command.
|
||||
pub(crate) type ExecPolicy = fn(file: &Path, argv: &[String], workdir: &Path) -> EscalateAction;
|
||||
pub(crate) type ExecPolicy = fn(file: &Path, argv: &[String], workdir: &Path) -> ExecPolicyOutcome;
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct ExecPolicyOutcome {
|
||||
pub(crate) decision: ExecPolicyDecision,
|
||||
pub(crate) run_with_escalated_permissions: bool,
|
||||
}
|
||||
|
||||
pub(crate) struct EscalateServer {
|
||||
bash_path: PathBuf,
|
||||
@@ -132,8 +139,27 @@ async fn handle_escalate_session_with_policy(
|
||||
} = socket.receive::<EscalateRequest>().await?;
|
||||
let file = PathBuf::from(&file).absolutize()?.into_owned();
|
||||
let workdir = PathBuf::from(&workdir).absolutize()?.into_owned();
|
||||
let action = policy(file.as_path(), &argv, &workdir);
|
||||
tracing::debug!("decided {action:?} for {file:?} {argv:?} {workdir:?}");
|
||||
let outcome = policy(file.as_path(), &argv, &workdir);
|
||||
|
||||
tracing::debug!("decided {outcome:?} for {file:?} {argv:?} {workdir:?}");
|
||||
let ExecPolicyOutcome {
|
||||
decision,
|
||||
run_with_escalated_permissions,
|
||||
} = outcome;
|
||||
let action = match decision {
|
||||
ExecPolicyDecision::Allow => {
|
||||
if run_with_escalated_permissions {
|
||||
EscalateAction::Escalate
|
||||
} else {
|
||||
EscalateAction::Run
|
||||
}
|
||||
}
|
||||
ExecPolicyDecision::Prompt => {
|
||||
todo!("prompting not yet implemented")
|
||||
}
|
||||
ExecPolicyDecision::Forbidden => EscalateAction::Deny,
|
||||
};
|
||||
|
||||
match action {
|
||||
EscalateAction::Run => {
|
||||
socket
|
||||
@@ -195,6 +221,9 @@ async fn handle_escalate_session_with_policy(
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
EscalateAction::Deny => {
|
||||
todo!("denying not yet implemented");
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -211,7 +240,10 @@ mod tests {
|
||||
let (server, client) = AsyncSocket::pair()?;
|
||||
let server_task = tokio::spawn(handle_escalate_session_with_policy(
|
||||
server,
|
||||
|_file, _argv, _workdir| EscalateAction::Run,
|
||||
|_file, _argv, _workdir| ExecPolicyOutcome {
|
||||
decision: ExecPolicyDecision::Allow,
|
||||
run_with_escalated_permissions: false,
|
||||
},
|
||||
));
|
||||
|
||||
client
|
||||
@@ -238,7 +270,10 @@ mod tests {
|
||||
let (server, client) = AsyncSocket::pair()?;
|
||||
let server_task = tokio::spawn(handle_escalate_session_with_policy(
|
||||
server,
|
||||
|_file, _argv, _workdir| EscalateAction::Escalate,
|
||||
|_file, _argv, _workdir| ExecPolicyOutcome {
|
||||
decision: ExecPolicyDecision::Allow,
|
||||
run_with_escalated_permissions: true,
|
||||
},
|
||||
));
|
||||
|
||||
client
|
||||
|
||||
@@ -4,21 +4,12 @@ What Codex is allowed to do is governed by a combination of **sandbox modes** (w
|
||||
|
||||
### Approval policies
|
||||
|
||||
Codex starts conservatively. Until you explicitly tell it a workspace is trusted, the CLI defaults to **read-only sandboxing** with the `read-only` approval preset. Codex can inspect files and answer questions, but every edit or command requires approval.
|
||||
Codex starts conservatively. Until you explicitly tell it a working directory is trusted, the CLI defaults to **read-only**. Codex can inspect files and answer questions, but every edit or command requires approval.
|
||||
|
||||
When you mark a workspace as trusted (for example via the onboarding prompt or `/approvals` → “Trust this directory”), Codex upgrades the default preset to **Auto**: sandboxed writes inside the workspace with `AskForApproval::OnRequest`. Codex only interrupts you when it needs to leave the workspace or rerun something outside the sandbox.
|
||||
When you mark a working directory as trusted (for example via the onboarding prompt or `/approvals` → “Trust this directory”), Codex upgrades the default preset to **Agent**, which allows writes inside the workspace. Codex only interrupts you when it needs to leave the workspace or rerun something outside the sandbox. Note that the workspace includes the working directory plus temporary directories like `/tmp`. Use `/status` to confirm the exact writable roots.
|
||||
|
||||
If you want maximum guardrails for a trusted repo, switch back to Read Only from the `/approvals` picker. If you truly need hands-off automation, use `Full Access`—but be deliberate, because that skips both the sandbox and approvals.
|
||||
|
||||
#### Defaults and recommendations
|
||||
|
||||
- Every session starts in a sandbox. Until a repo is trusted, Codex enforces read-only access and will prompt before any write or command.
|
||||
- Marking a repo as trusted switches the default preset to Auto (`workspace-write` + `ask-for-approval on-request`) so Codex can keep iterating locally without nagging you.
|
||||
- The workspace always includes the current directory plus temporary directories like `/tmp`. Use `/status` to confirm the exact writable roots.
|
||||
- You can override the defaults from the command line at any time:
|
||||
- `codex --sandbox read-only --ask-for-approval on-request`
|
||||
- `codex --sandbox workspace-write --ask-for-approval on-request`
|
||||
|
||||
### Can I run without ANY approvals?
|
||||
|
||||
Yes, you can disable all approval prompts with `--ask-for-approval never`. This option works with all `--sandbox` modes, so you still have full control over Codex's level of autonomy. It will make its best attempt with whatever constraints you provide.
|
||||
@@ -63,22 +54,32 @@ approval_policy = "never"
|
||||
sandbox_mode = "read-only"
|
||||
```
|
||||
|
||||
### Sandbox mechanics by platform {#platform-sandboxing-details}
|
||||
### Sandbox mechanics by platform
|
||||
|
||||
The mechanism Codex uses to enforce the sandbox policy depends on your OS:
|
||||
|
||||
- **macOS 12+** uses **Apple Seatbelt**. Codex invokes `sandbox-exec` with a profile that corresponds to the selected `--sandbox` mode, constraining filesystem and network access at the OS level.
|
||||
- **Linux** combines **Landlock** and **seccomp** APIs to approximate the same guarantees. Kernel support is required; older kernels may not expose the necessary features.
|
||||
- **Windows (experimental)**:
|
||||
- Launches commands inside a restricted token derived from an AppContainer profile.
|
||||
- Grants only specifically requested filesystem capabilities by attaching capability SIDs to that profile.
|
||||
- Disables outbound network access by overriding proxy-related environment variables and inserting stub executables for common network tools.
|
||||
#### macOS 12+
|
||||
|
||||
Windows sandbox support remains highly experimental. It cannot prevent file writes, deletions, or creations in any directory where the Everyone SID already has write permissions (for example, world-writable folders).
|
||||
Uses **Apple Seatbelt**. Codex invokes `sandbox-exec` with a profile that corresponds to the selected `--sandbox` mode, constraining filesystem and network access at the OS level.
|
||||
|
||||
#### Linux
|
||||
|
||||
Combines **Landlock** and **seccomp** APIs to approximate the same guarantees. Kernel support is required; older kernels may not expose the necessary features.
|
||||
|
||||
In containerized Linux environments (for example Docker), sandboxing may not work when the host or container configuration does not expose Landlock/seccomp. In those cases, configure the container to provide the isolation you need and run Codex with `--sandbox danger-full-access` (or the shorthand `--dangerously-bypass-approvals-and-sandbox`) inside that container.
|
||||
|
||||
### Experimenting with the Codex Sandbox
|
||||
#### Windows
|
||||
|
||||
Windows sandbox support remains experimental. How it works:
|
||||
|
||||
- Launches commands inside a restricted token derived from an AppContainer profile.
|
||||
- Grants only specifically requested filesystem capabilities by attaching capability SIDs to that profile.
|
||||
- Disables outbound network access by overriding proxy-related environment variables and inserting stub executables for common network tools.
|
||||
|
||||
Its primary limitation is that it cannot prevent file writes, deletions, or creations in any directory where the Everyone SID already has write permissions (for example, world-writable folders).
|
||||
See more discussion and limitations at [Windows Sandbox Security Details](./windows_sandbox_security.md).
|
||||
|
||||
## Experimenting with the Codex Sandbox
|
||||
|
||||
To test how commands behave under Codex's sandbox, use the CLI helpers:
|
||||
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
# Windows Sandbox Status
|
||||
# Windows Sandbox Security Details
|
||||
|
||||
For overall context on sandboxing in Codex, see [sandbox.md](./sandbox.md).
|
||||
|
||||
## Implementation Overview
|
||||
|
||||
|
||||
Reference in New Issue
Block a user