mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
Wire bounded output collection into the code mode runtime (#57850)
## Why Bounding collected output also requires limiting the raw event backlog between V8 and the cell actor. Previously, runtime events and cell output accumulated in an unbounded channel and vector. ## What changed - Select the event transport and `OutputCollector` from the cell's `output_truncation` mode. Server mode uses an ordered, bounded channel with backpressure; client mode retains untruncated output. - Transfer the collector with each observation and restore it on failed delivery, preserving retained media and omission metadata. - Include server-mode script errors in the observation's output budget. Build completion output only after completion wins against termination, so a rejected error cannot evict output returned by termination. - Keep the public service on client-side truncation. ## Testing Add regression coverage for channel backpressure, event ordering, receiver closure, failed observation delivery, termination during completion commit, and oversized errors across UTF-8 and omission boundaries. GitOrigin-RevId: 7604b60416e33c636ca41138be65dc0fd452774a
This commit is contained in:
@@ -40,7 +40,7 @@ impl CellHost for PanickingCallbackHost {
|
||||
async fn commit_completion(
|
||||
&self,
|
||||
_stored_value_writes: HashMap<String, Arc<JsonValue>>,
|
||||
_event: CellEvent,
|
||||
_event: impl FnOnce() -> CellEvent + Send,
|
||||
_cell_state: Arc<CellState>,
|
||||
) -> CompletionCommit {
|
||||
panic!("unexpected completion commit");
|
||||
|
||||
@@ -1,13 +1,9 @@
|
||||
use codex_code_mode_protocol::CodeModeToolKind;
|
||||
use codex_code_mode_protocol::ExecuteRequest;
|
||||
use codex_code_mode_protocol::FunctionCallOutputContentItem;
|
||||
use codex_code_mode_protocol::ImageDetail;
|
||||
use codex_code_mode_protocol::ToolDefinition;
|
||||
use codex_protocol::ToolName;
|
||||
|
||||
use crate::session_runtime::CreateCellRequest as CellRequest;
|
||||
use crate::session_runtime::ImageDetail as CellImageDetail;
|
||||
use crate::session_runtime::OutputItem as CellOutputItem;
|
||||
use crate::session_runtime::ToolKind as CellToolKind;
|
||||
|
||||
pub(super) fn runtime_request(request: CellRequest) -> ExecuteRequest {
|
||||
@@ -44,21 +40,3 @@ pub(super) fn cell_tool_kind(kind: CodeModeToolKind) -> CellToolKind {
|
||||
CodeModeToolKind::Freeform => CellToolKind::Freeform,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn output_item(item: FunctionCallOutputContentItem) -> CellOutputItem {
|
||||
match item {
|
||||
FunctionCallOutputContentItem::InputText { text } => CellOutputItem::Text { text },
|
||||
FunctionCallOutputContentItem::InputImage { image_url, detail } => CellOutputItem::Image {
|
||||
image_url,
|
||||
detail: detail.map(|detail| match detail {
|
||||
ImageDetail::Auto => CellImageDetail::Auto,
|
||||
ImageDetail::Low => CellImageDetail::Low,
|
||||
ImageDetail::High => CellImageDetail::High,
|
||||
ImageDetail::Original => CellImageDetail::Original,
|
||||
}),
|
||||
},
|
||||
FunctionCallOutputContentItem::InputAudio { audio_url } => {
|
||||
CellOutputItem::Audio { audio_url }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,6 @@ use self::callbacks::report_task_result;
|
||||
use self::callbacks::spawn_notification;
|
||||
use self::callbacks::spawn_tool;
|
||||
use self::conversions::cell_tool_kind;
|
||||
use self::conversions::output_item;
|
||||
use self::conversions::runtime_request;
|
||||
use self::types::CellCommand;
|
||||
pub(crate) use self::types::CellError;
|
||||
@@ -31,16 +30,18 @@ pub(crate) use self::types::CompletionCommit;
|
||||
use self::types::CompletionDelivery;
|
||||
use self::types::ObservationDelivery;
|
||||
use crate::TaskFailureHandler;
|
||||
use crate::output::OutputCollector;
|
||||
use crate::runtime::PendingRuntimeMode;
|
||||
use crate::runtime::RuntimeCommand;
|
||||
use crate::runtime::RuntimeControlCommand;
|
||||
use crate::runtime::RuntimeEvent;
|
||||
use crate::runtime::RuntimeEventReceiver;
|
||||
use crate::runtime::runtime_event_channel;
|
||||
use crate::runtime::spawn_runtime;
|
||||
use crate::session_runtime::CellEvent;
|
||||
use crate::session_runtime::CreateCellRequest as CellRequest;
|
||||
use crate::session_runtime::Observation;
|
||||
use crate::session_runtime::ObserveMode;
|
||||
use crate::session_runtime::OutputItem;
|
||||
use crate::session_runtime::ToolName as CellToolName;
|
||||
|
||||
pub(crate) struct CellActor;
|
||||
@@ -61,7 +62,8 @@ impl CellActor {
|
||||
),
|
||||
String,
|
||||
> {
|
||||
let (event_tx, event_rx) = mpsc::unbounded_channel();
|
||||
let (event_tx, event_rx) = runtime_event_channel(request.output_truncation);
|
||||
let output = OutputCollector::new(request.output_truncation);
|
||||
let (command_tx, command_rx) = mpsc::unbounded_channel();
|
||||
let (initial_response_tx, initial_response_rx) = oneshot::channel();
|
||||
let (runtime_tx, runtime_control_tx, runtime_terminate_handle) = spawn_runtime(
|
||||
@@ -79,6 +81,7 @@ impl CellActor {
|
||||
runtime_control_tx,
|
||||
runtime_terminate_handle,
|
||||
cell_state,
|
||||
output,
|
||||
},
|
||||
event_rx,
|
||||
command_rx,
|
||||
@@ -99,6 +102,7 @@ struct CellContext {
|
||||
runtime_control_tx: std::sync::mpsc::Sender<RuntimeControlCommand>,
|
||||
runtime_terminate_handle: v8::IsolateHandle,
|
||||
cell_state: Arc<CellState>,
|
||||
output: OutputCollector,
|
||||
}
|
||||
|
||||
struct Observer {
|
||||
@@ -109,7 +113,7 @@ struct Observer {
|
||||
async fn run_cell<H: CellHost>(
|
||||
host: Arc<H>,
|
||||
context: CellContext,
|
||||
mut event_rx: mpsc::UnboundedReceiver<RuntimeEvent>,
|
||||
mut event_rx: RuntimeEventReceiver,
|
||||
command_rx: mpsc::UnboundedReceiver<CellCommand>,
|
||||
initial_observer: Observer,
|
||||
task_failure_handler: Option<TaskFailureHandler>,
|
||||
@@ -119,10 +123,10 @@ async fn run_cell<H: CellHost>(
|
||||
runtime_control_tx,
|
||||
runtime_terminate_handle,
|
||||
cell_state,
|
||||
mut output,
|
||||
} = context;
|
||||
let cancellation_token = cell_state.cancellation_token();
|
||||
let callback_cancellation_token = cancellation_token.child_token();
|
||||
let mut content_items = Vec::new();
|
||||
let mut pending_tool_call_ids = Vec::new();
|
||||
let mut pending_frontier_ready = false;
|
||||
let mut yield_signal = Some(initial_observer.observation.yield_signal.clone());
|
||||
@@ -164,7 +168,7 @@ async fn run_cell<H: CellHost>(
|
||||
&cell_state,
|
||||
observer.take().map(|observer| observer.response_tx),
|
||||
CellEvent::Terminated {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
content_items: output.take(),
|
||||
},
|
||||
);
|
||||
break;
|
||||
@@ -202,27 +206,8 @@ async fn run_cell<H: CellHost>(
|
||||
continue;
|
||||
}
|
||||
if matches!(mode, ObserveMode::PendingFrontier) && pending_frontier_ready {
|
||||
pending_frontier_ready = false;
|
||||
match send_cell_event(
|
||||
response_tx,
|
||||
CellEvent::Pending {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
pending_tool_call_ids: std::mem::take(&mut pending_tool_call_ids),
|
||||
},
|
||||
) {
|
||||
Ok(()) => {}
|
||||
Err(CellEvent::Pending {
|
||||
content_items: undelivered_items,
|
||||
pending_tool_call_ids: undelivered_tool_call_ids,
|
||||
}) => {
|
||||
content_items = undelivered_items;
|
||||
pending_tool_call_ids = undelivered_tool_call_ids;
|
||||
pending_frontier_ready = true;
|
||||
}
|
||||
Err(event) => {
|
||||
panic!("pending delivery returned an unexpected event: {event:?}")
|
||||
}
|
||||
}
|
||||
pending_frontier_ready =
|
||||
!send_pending(&mut output, response_tx, &mut pending_tool_call_ids);
|
||||
continue;
|
||||
}
|
||||
yield_signal = Some(observation.yield_signal.clone());
|
||||
@@ -259,19 +244,13 @@ async fn run_cell<H: CellHost>(
|
||||
} => {
|
||||
yield_timer = None;
|
||||
yield_signal = None;
|
||||
restore_undelivered_yield(
|
||||
send_observer_event(
|
||||
observer.take(),
|
||||
CellEvent::Yielded {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
},
|
||||
),
|
||||
&mut content_items,
|
||||
);
|
||||
if let Some(observer) = observer.take() {
|
||||
send_yield(&mut output, observer.response_tx);
|
||||
}
|
||||
}
|
||||
maybe_event = async {
|
||||
if runtime_closed {
|
||||
std::future::pending::<Option<RuntimeEvent>>().await
|
||||
std::future::pending().await
|
||||
} else {
|
||||
event_rx.recv().await
|
||||
}
|
||||
@@ -292,7 +271,7 @@ async fn run_cell<H: CellHost>(
|
||||
&cell_state,
|
||||
observer.take().map(|observer| observer.response_tx),
|
||||
CellEvent::Terminated {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
content_items: output.take(),
|
||||
},
|
||||
);
|
||||
break;
|
||||
@@ -313,22 +292,17 @@ async fn run_cell<H: CellHost>(
|
||||
task_failure_handler.as_ref(),
|
||||
)
|
||||
.await;
|
||||
let event = CellEvent::Completed {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
result_buffered_for: std::time::Duration::ZERO,
|
||||
error_text: Some("exec runtime ended unexpectedly".to_string()),
|
||||
};
|
||||
let rejected_event = match host
|
||||
host
|
||||
.commit_completion(
|
||||
HashMap::new(),
|
||||
event,
|
||||
|| {
|
||||
take_completed_event(&mut output, Some(
|
||||
"exec runtime ended unexpectedly".to_string(),
|
||||
))
|
||||
},
|
||||
Arc::clone(&cell_state),
|
||||
)
|
||||
.await
|
||||
{
|
||||
CompletionCommit::Committed => None,
|
||||
CompletionCommit::Rejected(event) => Some(event),
|
||||
};
|
||||
.await;
|
||||
match cell_state.deliver_completion(
|
||||
observer.take().map(|observer| observer.response_tx),
|
||||
) {
|
||||
@@ -339,7 +313,7 @@ async fn run_cell<H: CellHost>(
|
||||
&cell_state,
|
||||
response_tx,
|
||||
CellEvent::Terminated {
|
||||
content_items: rejected_completion_content(rejected_event),
|
||||
content_items: output.take(),
|
||||
},
|
||||
);
|
||||
break;
|
||||
@@ -356,59 +330,29 @@ async fn run_cell<H: CellHost>(
|
||||
}
|
||||
RuntimeEvent::Pending => {
|
||||
runtime_paused = true;
|
||||
if matches!(
|
||||
observer.as_ref().map(|observer| observer.observation.mode),
|
||||
Some(ObserveMode::PendingFrontier)
|
||||
) {
|
||||
if let Some(observer) = observer.take_if(|observer| {
|
||||
matches!(observer.observation.mode, ObserveMode::PendingFrontier)
|
||||
}) {
|
||||
yield_timer = None;
|
||||
yield_signal = None;
|
||||
pending_frontier_ready = false;
|
||||
match send_observer_event(
|
||||
observer.take(),
|
||||
CellEvent::Pending {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
pending_tool_call_ids: std::mem::take(
|
||||
&mut pending_tool_call_ids,
|
||||
),
|
||||
},
|
||||
) {
|
||||
Ok(()) => {}
|
||||
Err(CellEvent::Pending {
|
||||
content_items: undelivered_items,
|
||||
pending_tool_call_ids: undelivered_tool_call_ids,
|
||||
}) => {
|
||||
content_items = undelivered_items;
|
||||
pending_tool_call_ids = undelivered_tool_call_ids;
|
||||
pending_frontier_ready = true;
|
||||
}
|
||||
Err(event) => {
|
||||
panic!("pending delivery returned an unexpected event: {event:?}")
|
||||
}
|
||||
}
|
||||
pending_frontier_ready =
|
||||
!send_pending(&mut output, observer.response_tx, &mut pending_tool_call_ids);
|
||||
} else {
|
||||
pending_tool_call_ids.clear();
|
||||
let _ = runtime_control_tx.send(RuntimeControlCommand::Continue);
|
||||
runtime_paused = false;
|
||||
}
|
||||
}
|
||||
RuntimeEvent::ContentItem(item) => content_items.push(output_item(item)),
|
||||
RuntimeEvent::ContentItem(item) => {
|
||||
output.push(item);
|
||||
}
|
||||
RuntimeEvent::YieldRequested => {
|
||||
let yield_observer = matches!(
|
||||
observer.as_ref().map(|observer| observer.observation.mode),
|
||||
Some(ObserveMode::YieldAfter(_))
|
||||
);
|
||||
if yield_observer {
|
||||
if let Some(observer) = observer.take_if(|observer| {
|
||||
matches!(observer.observation.mode, ObserveMode::YieldAfter(_))
|
||||
}) {
|
||||
yield_timer = None;
|
||||
yield_signal = None;
|
||||
restore_undelivered_yield(
|
||||
send_observer_event(
|
||||
observer.take(),
|
||||
CellEvent::Yielded {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
},
|
||||
),
|
||||
&mut content_items,
|
||||
);
|
||||
send_yield(&mut output, observer.response_tx);
|
||||
}
|
||||
}
|
||||
RuntimeEvent::Notify { call_id, text } => {
|
||||
@@ -456,7 +400,7 @@ async fn run_cell<H: CellHost>(
|
||||
&cell_state,
|
||||
observer.take().map(|observer| observer.response_tx),
|
||||
CellEvent::Terminated {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
content_items: output.take(),
|
||||
},
|
||||
);
|
||||
break;
|
||||
@@ -469,22 +413,13 @@ async fn run_cell<H: CellHost>(
|
||||
task_failure_handler.as_ref(),
|
||||
)
|
||||
.await;
|
||||
let event = CellEvent::Completed {
|
||||
content_items: std::mem::take(&mut content_items),
|
||||
result_buffered_for: std::time::Duration::ZERO,
|
||||
error_text,
|
||||
};
|
||||
let rejected_event = match host
|
||||
host
|
||||
.commit_completion(
|
||||
stored_value_writes,
|
||||
event,
|
||||
|| take_completed_event(&mut output, error_text),
|
||||
Arc::clone(&cell_state),
|
||||
)
|
||||
.await
|
||||
{
|
||||
CompletionCommit::Committed => None,
|
||||
CompletionCommit::Rejected(event) => Some(event),
|
||||
};
|
||||
.await;
|
||||
match cell_state.deliver_completion(
|
||||
observer.take().map(|observer| observer.response_tx),
|
||||
) {
|
||||
@@ -495,7 +430,7 @@ async fn run_cell<H: CellHost>(
|
||||
&cell_state,
|
||||
response_tx,
|
||||
CellEvent::Terminated {
|
||||
content_items: rejected_completion_content(rejected_event),
|
||||
content_items: output.take(),
|
||||
},
|
||||
);
|
||||
break;
|
||||
@@ -539,11 +474,14 @@ async fn run_cell<H: CellHost>(
|
||||
host.closed().await;
|
||||
}
|
||||
|
||||
fn send_observer_event(observer: Option<Observer>, event: CellEvent) -> Result<(), CellEvent> {
|
||||
let Some(observer) = observer else {
|
||||
return Err(event);
|
||||
};
|
||||
send_cell_event(observer.response_tx, event)
|
||||
/// Construct completion only after it has won the terminal decision.
|
||||
fn take_completed_event(output: &mut OutputCollector, error_text: Option<String>) -> CellEvent {
|
||||
let error_text = error_text.map(|error_text| output.collect_error(error_text));
|
||||
CellEvent::Completed {
|
||||
content_items: output.take(),
|
||||
error_text,
|
||||
result_buffered_for: std::time::Duration::ZERO,
|
||||
}
|
||||
}
|
||||
|
||||
fn send_cell_event(
|
||||
@@ -557,25 +495,42 @@ fn send_cell_event(
|
||||
}
|
||||
}
|
||||
|
||||
fn restore_undelivered_yield(delivery: Result<(), CellEvent>, content_items: &mut Vec<OutputItem>) {
|
||||
match delivery {
|
||||
Ok(()) => {}
|
||||
Err(CellEvent::Yielded {
|
||||
content_items: mut undelivered_items,
|
||||
}) => {
|
||||
undelivered_items.append(content_items);
|
||||
*content_items = undelivered_items;
|
||||
}
|
||||
Err(event) => panic!("yield delivery returned an unexpected event: {event:?}"),
|
||||
}
|
||||
fn send_yield(
|
||||
output: &mut OutputCollector,
|
||||
response_tx: oneshot::Sender<Result<CellEvent, CellError>>,
|
||||
) {
|
||||
output.try_deliver(|content_items| {
|
||||
send_cell_event(response_tx, CellEvent::Yielded { content_items }).map_err(|event| {
|
||||
let CellEvent::Yielded { content_items } = event else {
|
||||
unreachable!("yield delivery returned a different event");
|
||||
};
|
||||
content_items
|
||||
})
|
||||
});
|
||||
}
|
||||
|
||||
fn rejected_completion_content(event: Option<CellEvent>) -> Vec<OutputItem> {
|
||||
match event {
|
||||
Some(CellEvent::Completed { content_items, .. }) => content_items,
|
||||
None => Vec::new(),
|
||||
Some(event) => panic!("completion commit rejected an unexpected event: {event:?}"),
|
||||
}
|
||||
fn send_pending(
|
||||
output: &mut OutputCollector,
|
||||
response_tx: oneshot::Sender<Result<CellEvent, CellError>>,
|
||||
pending_tool_call_ids: &mut Vec<String>,
|
||||
) -> bool {
|
||||
output.try_deliver(|content_items| {
|
||||
let pending = CellEvent::Pending {
|
||||
content_items,
|
||||
pending_tool_call_ids: std::mem::take(pending_tool_call_ids),
|
||||
};
|
||||
send_cell_event(response_tx, pending).map_err(|event| {
|
||||
let CellEvent::Pending {
|
||||
content_items,
|
||||
pending_tool_call_ids: returned_ids,
|
||||
} = event
|
||||
else {
|
||||
unreachable!("pending delivery returned a different event");
|
||||
};
|
||||
*pending_tool_call_ids = returned_ids;
|
||||
content_items
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn finish_termination(
|
||||
|
||||
@@ -5,8 +5,9 @@ use std::sync::atomic::Ordering;
|
||||
use std::sync::mpsc as std_mpsc;
|
||||
use std::time::Duration;
|
||||
|
||||
use codex_code_mode_protocol::CodeModeOutputTruncation;
|
||||
use codex_code_mode_protocol::ExecuteRequest;
|
||||
use codex_code_mode_protocol::FunctionCallOutputContentItem;
|
||||
use codex_code_mode_protocol::host::BOUNDED_OUTPUT_MAX_IMAGE_BYTES;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value as JsonValue;
|
||||
use tokio::sync::mpsc;
|
||||
@@ -14,9 +15,14 @@ use tokio::sync::oneshot;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use super::*;
|
||||
use crate::runtime::RuntimeEventSender;
|
||||
use crate::session_runtime::OutputItem;
|
||||
|
||||
struct TestHost;
|
||||
#[derive(Default)]
|
||||
struct TestHost {
|
||||
commit_started: CancellationToken,
|
||||
resume_commit: Option<CancellationToken>,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct RecordingHost {
|
||||
@@ -44,9 +50,13 @@ impl CellHost for TestHost {
|
||||
async fn commit_completion(
|
||||
&self,
|
||||
_stored_value_writes: HashMap<String, Arc<JsonValue>>,
|
||||
event: CellEvent,
|
||||
event: impl FnOnce() -> CellEvent + Send,
|
||||
cell_state: Arc<CellState>,
|
||||
) -> CompletionCommit {
|
||||
self.commit_started.cancel();
|
||||
if let Some(resume) = &self.resume_commit {
|
||||
resume.cancelled().await;
|
||||
}
|
||||
cell_state.commit_completion(event, || {})
|
||||
}
|
||||
|
||||
@@ -75,7 +85,7 @@ impl CellHost for RecordingHost {
|
||||
async fn commit_completion(
|
||||
&self,
|
||||
_stored_value_writes: HashMap<String, Arc<JsonValue>>,
|
||||
event: CellEvent,
|
||||
event: impl FnOnce() -> CellEvent + Send,
|
||||
cell_state: Arc<CellState>,
|
||||
) -> CompletionCommit {
|
||||
cell_state.commit_completion(event, || {})
|
||||
@@ -85,7 +95,7 @@ impl CellHost for RecordingHost {
|
||||
}
|
||||
|
||||
struct CellActorHarness {
|
||||
event_tx: mpsc::UnboundedSender<RuntimeEvent>,
|
||||
event_tx: RuntimeEventSender,
|
||||
handle: CellHandle,
|
||||
yield_signal: CancellationToken,
|
||||
initial_event_rx: oneshot::Receiver<Result<CellEvent, CellError>>,
|
||||
@@ -95,7 +105,7 @@ struct CellActorHarness {
|
||||
}
|
||||
|
||||
fn spawn_cell_actor_harness(initial_observe_mode: ObserveMode) -> CellActorHarness {
|
||||
spawn_cell_actor_harness_with_host(initial_observe_mode, Arc::new(TestHost))
|
||||
spawn_cell_actor_harness_with_host(initial_observe_mode, Arc::new(TestHost::default()))
|
||||
}
|
||||
|
||||
fn spawn_cell_actor_harness_with_host<H: CellHost>(
|
||||
@@ -105,6 +115,7 @@ fn spawn_cell_actor_harness_with_host<H: CellHost>(
|
||||
spawn_cell_actor_harness_with_host_and_failure_handler(
|
||||
initial_observe_mode,
|
||||
host,
|
||||
CodeModeOutputTruncation::Client,
|
||||
/*task_failure_handler*/ None,
|
||||
)
|
||||
}
|
||||
@@ -112,9 +123,11 @@ fn spawn_cell_actor_harness_with_host<H: CellHost>(
|
||||
fn spawn_cell_actor_harness_with_host_and_failure_handler<H: CellHost>(
|
||||
initial_observe_mode: ObserveMode,
|
||||
host: Arc<H>,
|
||||
output_truncation: CodeModeOutputTruncation,
|
||||
task_failure_handler: Option<TaskFailureHandler>,
|
||||
) -> CellActorHarness {
|
||||
let (event_tx, event_rx) = mpsc::unbounded_channel();
|
||||
let (event_tx, event_rx) = runtime_event_channel(output_truncation);
|
||||
let output = OutputCollector::new(output_truncation);
|
||||
let (command_tx, command_rx) = mpsc::unbounded_channel();
|
||||
let (initial_event_tx, initial_event_rx) = oneshot::channel();
|
||||
let (runtime_event_tx, runtime_event_rx) = mpsc::unbounded_channel();
|
||||
@@ -143,6 +156,7 @@ fn spawn_cell_actor_harness_with_host_and_failure_handler<H: CellHost>(
|
||||
runtime_control_tx,
|
||||
runtime_terminate_handle,
|
||||
cell_state,
|
||||
output,
|
||||
},
|
||||
event_rx,
|
||||
command_rx,
|
||||
@@ -169,7 +183,8 @@ async fn unexpected_runtime_thread_exit_is_reported_to_the_session_owner() {
|
||||
let (failure_tx, mut failure_rx) = mpsc::unbounded_channel();
|
||||
let harness = spawn_cell_actor_harness_with_host_and_failure_handler(
|
||||
ObserveMode::YieldAfter(Duration::from_secs(60)),
|
||||
Arc::new(TestHost),
|
||||
Arc::new(TestHost::default()),
|
||||
CodeModeOutputTruncation::Client,
|
||||
Some(Arc::new(move |reason| {
|
||||
let _ = failure_tx.send(reason);
|
||||
})),
|
||||
@@ -193,19 +208,74 @@ async fn unexpected_runtime_thread_exit_is_reported_to_the_session_owner() {
|
||||
harness.task.await.expect("cell task");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn termination_during_completion_commit_preserves_output_and_discards_losing_error() {
|
||||
for mode in [
|
||||
CodeModeOutputTruncation::Client,
|
||||
CodeModeOutputTruncation::Server,
|
||||
] {
|
||||
// The actor has received Result but cannot commit until the host releases
|
||||
// the session store. Termination must decide which output is published.
|
||||
let resume_commit = CancellationToken::new();
|
||||
let host = Arc::new(TestHost {
|
||||
resume_commit: Some(resume_commit.clone()),
|
||||
..TestHost::default()
|
||||
});
|
||||
let harness = spawn_cell_actor_harness_with_host_and_failure_handler(
|
||||
ObserveMode::YieldAfter(Duration::from_secs(60)),
|
||||
Arc::clone(&host),
|
||||
mode,
|
||||
/*task_failure_handler*/ None,
|
||||
);
|
||||
// Simulate V8's dedicated thread, where server-mode sends can block
|
||||
// while the event queue is full.
|
||||
let event_tx = harness.event_tx.clone();
|
||||
tokio::task::spawn_blocking(move || {
|
||||
event_tx
|
||||
.blocking_send(RuntimeEvent::ContentItem(OutputItem::InputText {
|
||||
text: "output that must survive termination".to_string(),
|
||||
}))
|
||||
.expect("output");
|
||||
event_tx
|
||||
.blocking_send(RuntimeEvent::Result {
|
||||
stored_value_writes: HashMap::new(),
|
||||
error_text: Some("uncommitted error".repeat(128 * 1024)),
|
||||
})
|
||||
.expect("completion");
|
||||
})
|
||||
.await
|
||||
.expect("runtime producer");
|
||||
tokio::time::timeout(Duration::from_secs(5), host.commit_started.cancelled())
|
||||
.await
|
||||
.expect("commit reached");
|
||||
let termination = harness.handle.terminate();
|
||||
resume_commit.cancel();
|
||||
let mut expected_output = OutputCollector::new(mode);
|
||||
expected_output.push(OutputItem::InputText {
|
||||
text: "output that must survive termination".to_string(),
|
||||
});
|
||||
let expected = Ok(CellEvent::Terminated {
|
||||
content_items: expected_output,
|
||||
});
|
||||
assert_eq!(termination.await, expected, "{mode:?}");
|
||||
assert_eq!(harness.initial_event_rx.await.expect("observer"), expected);
|
||||
harness.task.await.expect("cell task");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn runtime_thread_panic_remains_a_cell_error_without_owner_supervision() {
|
||||
let harness = spawn_cell_actor_harness(ObserveMode::YieldAfter(Duration::from_secs(60)));
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::ThreadPanicked)
|
||||
.blocking_send(RuntimeEvent::ThreadPanicked)
|
||||
.expect("runtime panic event");
|
||||
drop(harness.event_tx);
|
||||
|
||||
assert_eq!(
|
||||
harness.initial_event_rx.await.expect("initial event"),
|
||||
Ok(CellEvent::Completed {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(Vec::new()),
|
||||
result_buffered_for: Duration::ZERO,
|
||||
error_text: Some("exec runtime ended unexpectedly".to_string()),
|
||||
})
|
||||
@@ -226,20 +296,21 @@ async fn wait_for_notification(host: &RecordingHost) {
|
||||
#[tokio::test]
|
||||
async fn yield_timer_preempts_buffered_runtime_output() {
|
||||
let harness = spawn_cell_actor_harness(ObserveMode::YieldAfter(Duration::ZERO));
|
||||
harness.event_tx.send(RuntimeEvent::Started).unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::ContentItem(
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "queued output".to_string(),
|
||||
},
|
||||
))
|
||||
.blocking_send(RuntimeEvent::Started)
|
||||
.unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::ContentItem(OutputItem::InputText {
|
||||
text: "queued output".to_string(),
|
||||
}))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
harness.initial_event_rx.await.unwrap(),
|
||||
Ok(CellEvent::Yielded {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(Vec::new()),
|
||||
})
|
||||
);
|
||||
|
||||
@@ -248,9 +319,9 @@ async fn yield_timer_preempts_buffered_runtime_output() {
|
||||
assert_eq!(
|
||||
termination.await,
|
||||
Ok(CellEvent::Terminated {
|
||||
content_items: vec![OutputItem::Text {
|
||||
content_items: OutputCollector::Client(vec![OutputItem::InputText {
|
||||
text: "queued output".to_string(),
|
||||
}],
|
||||
}]),
|
||||
})
|
||||
);
|
||||
harness.task.await.unwrap();
|
||||
@@ -259,19 +330,23 @@ async fn yield_timer_preempts_buffered_runtime_output() {
|
||||
#[tokio::test]
|
||||
async fn queued_termination_preempts_unobserved_runtime_completion() {
|
||||
let harness = spawn_cell_actor_harness(ObserveMode::YieldAfter(Duration::ZERO));
|
||||
harness.event_tx.send(RuntimeEvent::Started).unwrap();
|
||||
harness.event_tx.send(RuntimeEvent::YieldRequested).unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::ContentItem(
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "queued output".to_string(),
|
||||
},
|
||||
))
|
||||
.blocking_send(RuntimeEvent::Started)
|
||||
.unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::Result {
|
||||
.blocking_send(RuntimeEvent::YieldRequested)
|
||||
.unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::ContentItem(OutputItem::InputText {
|
||||
text: "queued output".to_string(),
|
||||
}))
|
||||
.unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::Result {
|
||||
stored_value_writes: HashMap::new(),
|
||||
error_text: None,
|
||||
})
|
||||
@@ -280,9 +355,9 @@ async fn queued_termination_preempts_unobserved_runtime_completion() {
|
||||
let termination = harness.handle.terminate();
|
||||
|
||||
let terminated = Ok(CellEvent::Terminated {
|
||||
content_items: vec![OutputItem::Text {
|
||||
content_items: OutputCollector::Client(vec![OutputItem::InputText {
|
||||
text: "queued output".to_string(),
|
||||
}],
|
||||
}]),
|
||||
});
|
||||
assert_eq!(termination.await, terminated.clone());
|
||||
assert_eq!(harness.initial_event_rx.await.unwrap(), terminated);
|
||||
@@ -296,7 +371,10 @@ async fn observation_dropped_before_dequeue_does_not_consume_output() {
|
||||
ObserveMode::YieldAfter(Duration::from_secs(60)),
|
||||
Arc::clone(&host),
|
||||
);
|
||||
harness.event_tx.send(RuntimeEvent::YieldRequested).unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::YieldRequested)
|
||||
.unwrap();
|
||||
assert!(harness.initial_event_rx.await.unwrap().is_ok());
|
||||
|
||||
drop(
|
||||
@@ -306,16 +384,17 @@ async fn observation_dropped_before_dequeue_does_not_consume_output() {
|
||||
);
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::ContentItem(
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "survives pre-dequeue cancellation".to_string(),
|
||||
},
|
||||
))
|
||||
.blocking_send(RuntimeEvent::ContentItem(OutputItem::InputText {
|
||||
text: "survives pre-dequeue cancellation".to_string(),
|
||||
}))
|
||||
.unwrap();
|
||||
harness.event_tx.send(RuntimeEvent::YieldRequested).unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::Notify {
|
||||
.blocking_send(RuntimeEvent::YieldRequested)
|
||||
.unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::Notify {
|
||||
call_id: "after-dropped-command".to_string(),
|
||||
text: "barrier".to_string(),
|
||||
})
|
||||
@@ -328,9 +407,9 @@ async fn observation_dropped_before_dequeue_does_not_consume_output() {
|
||||
.observe(ObserveMode::YieldAfter(Duration::ZERO))
|
||||
.await,
|
||||
Ok(CellEvent::Yielded {
|
||||
content_items: vec![OutputItem::Text {
|
||||
content_items: OutputCollector::Client(vec![OutputItem::InputText {
|
||||
text: "survives pre-dequeue cancellation".to_string(),
|
||||
}],
|
||||
}]),
|
||||
})
|
||||
);
|
||||
|
||||
@@ -339,7 +418,7 @@ async fn observation_dropped_before_dequeue_does_not_consume_output() {
|
||||
assert_eq!(
|
||||
termination.await,
|
||||
Ok(CellEvent::Terminated {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(Vec::new()),
|
||||
})
|
||||
);
|
||||
harness.task.await.unwrap();
|
||||
@@ -352,7 +431,10 @@ async fn dropped_yield_observer_preserves_output_for_the_next_observation() {
|
||||
ObserveMode::YieldAfter(Duration::from_secs(60)),
|
||||
Arc::clone(&host),
|
||||
);
|
||||
harness.event_tx.send(RuntimeEvent::YieldRequested).unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::YieldRequested)
|
||||
.unwrap();
|
||||
assert!(harness.initial_event_rx.await.unwrap().is_ok());
|
||||
|
||||
let dropped_observation = harness
|
||||
@@ -368,16 +450,17 @@ async fn dropped_yield_observer_preserves_output_for_the_next_observation() {
|
||||
drop(dropped_observation);
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::ContentItem(
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "survives active cancellation".to_string(),
|
||||
},
|
||||
))
|
||||
.blocking_send(RuntimeEvent::ContentItem(OutputItem::InputText {
|
||||
text: "survives active cancellation".to_string(),
|
||||
}))
|
||||
.unwrap();
|
||||
harness.event_tx.send(RuntimeEvent::YieldRequested).unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::Notify {
|
||||
.blocking_send(RuntimeEvent::YieldRequested)
|
||||
.unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::Notify {
|
||||
call_id: "after-dropped-observer".to_string(),
|
||||
text: "barrier".to_string(),
|
||||
})
|
||||
@@ -390,9 +473,9 @@ async fn dropped_yield_observer_preserves_output_for_the_next_observation() {
|
||||
.observe(ObserveMode::YieldAfter(Duration::ZERO))
|
||||
.await,
|
||||
Ok(CellEvent::Yielded {
|
||||
content_items: vec![OutputItem::Text {
|
||||
content_items: OutputCollector::Client(vec![OutputItem::InputText {
|
||||
text: "survives active cancellation".to_string(),
|
||||
}],
|
||||
}]),
|
||||
})
|
||||
);
|
||||
|
||||
@@ -401,7 +484,7 @@ async fn dropped_yield_observer_preserves_output_for_the_next_observation() {
|
||||
assert_eq!(
|
||||
termination.await,
|
||||
Ok(CellEvent::Terminated {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(Vec::new()),
|
||||
})
|
||||
);
|
||||
harness.task.await.unwrap();
|
||||
@@ -414,7 +497,10 @@ async fn dropped_pending_observer_preserves_the_frontier_for_the_next_observatio
|
||||
ObserveMode::YieldAfter(Duration::from_secs(60)),
|
||||
Arc::clone(&host),
|
||||
);
|
||||
harness.event_tx.send(RuntimeEvent::YieldRequested).unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::YieldRequested)
|
||||
.unwrap();
|
||||
assert!(harness.initial_event_rx.await.unwrap().is_ok());
|
||||
|
||||
let dropped_observation = harness.handle.observe(ObserveMode::PendingFrontier);
|
||||
@@ -423,9 +509,16 @@ async fn dropped_pending_observer_preserves_the_frontier_for_the_next_observatio
|
||||
Err(CellError::Busy)
|
||||
);
|
||||
drop(dropped_observation);
|
||||
let item = OutputItem::InputText {
|
||||
text: "before frontier".to_owned(),
|
||||
};
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::ToolCall {
|
||||
.blocking_send(RuntimeEvent::ContentItem(item.clone()))
|
||||
.unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::ToolCall {
|
||||
id: "tool-1".to_string(),
|
||||
name: codex_protocol::ToolName {
|
||||
name: "echo".to_string(),
|
||||
@@ -435,10 +528,13 @@ async fn dropped_pending_observer_preserves_the_frontier_for_the_next_observatio
|
||||
input: Some(serde_json::json!({})),
|
||||
})
|
||||
.unwrap();
|
||||
harness.event_tx.send(RuntimeEvent::Pending).unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.send(RuntimeEvent::Notify {
|
||||
.blocking_send(RuntimeEvent::Pending)
|
||||
.unwrap();
|
||||
harness
|
||||
.event_tx
|
||||
.blocking_send(RuntimeEvent::Notify {
|
||||
call_id: "after-dropped-pending".to_string(),
|
||||
text: "barrier".to_string(),
|
||||
})
|
||||
@@ -448,7 +544,7 @@ async fn dropped_pending_observer_preserves_the_frontier_for_the_next_observatio
|
||||
assert_eq!(
|
||||
harness.handle.observe(ObserveMode::PendingFrontier).await,
|
||||
Ok(CellEvent::Pending {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(vec![item]),
|
||||
pending_tool_call_ids: vec!["tool-1".to_string()],
|
||||
})
|
||||
);
|
||||
@@ -462,7 +558,7 @@ async fn dropped_pending_observer_preserves_the_frontier_for_the_next_observatio
|
||||
assert_eq!(
|
||||
termination.await,
|
||||
Ok(CellEvent::Terminated {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(Vec::new()),
|
||||
})
|
||||
);
|
||||
harness.task.await.unwrap();
|
||||
@@ -472,12 +568,12 @@ async fn dropped_pending_observer_preserves_the_frontier_for_the_next_observatio
|
||||
async fn only_the_first_termination_claims_a_buffered_completion() {
|
||||
let cell_state = CellState::new(CancellationToken::new());
|
||||
let completion = CellEvent::Completed {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(Vec::new()),
|
||||
result_buffered_for: Duration::ZERO,
|
||||
error_text: None,
|
||||
};
|
||||
assert_eq!(
|
||||
cell_state.commit_completion(completion.clone(), || {}),
|
||||
cell_state.commit_completion(|| completion.clone(), || {}),
|
||||
CompletionCommit::Committed
|
||||
);
|
||||
assert!(matches!(
|
||||
@@ -493,7 +589,7 @@ async fn only_the_first_termination_claims_a_buffered_completion() {
|
||||
assert_eq!(first_termination.await, Ok(completion.clone()));
|
||||
assert_eq!(
|
||||
cell_state.finish_termination(CellEvent::Terminated {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(Vec::new()),
|
||||
}),
|
||||
Some(completion)
|
||||
);
|
||||
@@ -504,20 +600,17 @@ async fn termination_claim_prevents_stored_value_commit() {
|
||||
let cell_state = CellState::new(CancellationToken::new());
|
||||
let termination = cell_state.request_termination();
|
||||
let mut commit_ran = false;
|
||||
let completion = CellEvent::Completed {
|
||||
content_items: Vec::new(),
|
||||
result_buffered_for: Duration::ZERO,
|
||||
error_text: None,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
cell_state.commit_completion(completion.clone(), || commit_ran = true),
|
||||
CompletionCommit::Rejected(completion)
|
||||
cell_state.commit_completion(
|
||||
|| panic!("terminated cell must not construct completion output"),
|
||||
|| commit_ran = true
|
||||
),
|
||||
CompletionCommit::Rejected
|
||||
);
|
||||
assert!(!commit_ran);
|
||||
|
||||
let terminated = CellEvent::Terminated {
|
||||
content_items: Vec::new(),
|
||||
content_items: OutputCollector::Client(Vec::new()),
|
||||
};
|
||||
assert_eq!(
|
||||
cell_state.finish_termination(terminated.clone()),
|
||||
@@ -530,14 +623,14 @@ async fn termination_claim_prevents_stored_value_commit() {
|
||||
async fn failed_completion_delivery_rebuffers_the_event() {
|
||||
let cell_state = CellState::new(CancellationToken::new());
|
||||
let completion = |result_buffered_for| CellEvent::Completed {
|
||||
content_items: vec![OutputItem::Text {
|
||||
content_items: OutputCollector::Client(vec![OutputItem::InputText {
|
||||
text: "done".to_string(),
|
||||
}],
|
||||
}]),
|
||||
result_buffered_for,
|
||||
error_text: None,
|
||||
};
|
||||
assert_eq!(
|
||||
cell_state.commit_completion(completion(Duration::ZERO), || {}),
|
||||
cell_state.commit_completion(|| completion(Duration::ZERO), || {}),
|
||||
CompletionCommit::Committed
|
||||
);
|
||||
let (response_tx, response_rx) = oneshot::channel();
|
||||
@@ -565,3 +658,49 @@ async fn failed_completion_delivery_rebuffers_the_event() {
|
||||
));
|
||||
assert_eq!(response_rx.try_recv(), Ok(Ok(completion(2 * interval))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_yield_restores_image_allocation_and_omission_order() {
|
||||
let mut output = OutputCollector::new(CodeModeOutputTruncation::Server);
|
||||
let image_url = "i".repeat(BOUNDED_OUTPUT_MAX_IMAGE_BYTES);
|
||||
let pointer = image_url.as_ptr();
|
||||
output.push(OutputItem::InputImage {
|
||||
image_url,
|
||||
detail: None,
|
||||
});
|
||||
output.push(OutputItem::InputImage {
|
||||
image_url: "excess".to_string(),
|
||||
detail: None,
|
||||
});
|
||||
let (tx, rx) = oneshot::channel();
|
||||
drop(rx);
|
||||
send_yield(&mut output, tx);
|
||||
|
||||
output.push(OutputItem::InputText {
|
||||
text: "after".to_string(),
|
||||
});
|
||||
let (tx, mut rx) = oneshot::channel();
|
||||
send_yield(&mut output, tx);
|
||||
let CellEvent::Yielded { content_items } = rx.try_recv().unwrap().unwrap() else {
|
||||
panic!("expected a yield");
|
||||
};
|
||||
let items = crate::output::render_output(
|
||||
content_items.into_collected_output(),
|
||||
/*max_output_tokens*/ None,
|
||||
);
|
||||
let [
|
||||
OutputItem::InputImage { image_url, .. },
|
||||
OutputItem::InputText { text: notice },
|
||||
OutputItem::InputText { text },
|
||||
] = items.as_slice()
|
||||
else {
|
||||
panic!("expected image, omission notice, and subsequent text");
|
||||
};
|
||||
assert_eq!(image_url.as_ptr(), pointer);
|
||||
assert!(notice.contains("1 images"), "{notice}");
|
||||
assert_eq!(text, "after");
|
||||
assert_eq!(
|
||||
output.take(),
|
||||
OutputCollector::new(CodeModeOutputTruncation::Server)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -55,7 +55,7 @@ pub(crate) trait CellHost: Send + Sync + 'static {
|
||||
fn commit_completion(
|
||||
&self,
|
||||
stored_value_writes: HashMap<String, Arc<JsonValue>>,
|
||||
event: CellEvent,
|
||||
event: impl FnOnce() -> CellEvent + Send,
|
||||
cell_state: Arc<CellState>,
|
||||
) -> impl Future<Output = CompletionCommit> + Send;
|
||||
|
||||
@@ -136,7 +136,7 @@ pub(crate) enum CompletionDelivery {
|
||||
#[derive(Debug, PartialEq)]
|
||||
pub(crate) enum CompletionCommit {
|
||||
Committed,
|
||||
Rejected(CellEvent),
|
||||
Rejected,
|
||||
}
|
||||
|
||||
pub(crate) enum ObservationDelivery {
|
||||
@@ -200,7 +200,7 @@ impl CellState {
|
||||
|
||||
pub(crate) fn commit_completion(
|
||||
&self,
|
||||
event: CellEvent,
|
||||
event: impl FnOnce() -> CellEvent,
|
||||
commit: impl FnOnce(),
|
||||
) -> CompletionCommit {
|
||||
let mut phase = self
|
||||
@@ -208,11 +208,13 @@ impl CellState {
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
if !matches!(*phase, CellPhase::Running) || self.cancellation_token.is_cancelled() {
|
||||
return CompletionCommit::Rejected(event);
|
||||
return CompletionCommit::Rejected;
|
||||
}
|
||||
commit();
|
||||
*phase = CellPhase::Completed {
|
||||
event,
|
||||
// Construct terminal output only after completion wins. In bounded
|
||||
// mode, an error may evict user output; termination must not see it.
|
||||
event: event(),
|
||||
completed_at: Instant::now(),
|
||||
};
|
||||
CompletionCommit::Committed
|
||||
|
||||
Reference in New Issue
Block a user