mirror of
https://github.com/openai/codex.git
synced 2026-09-10 20:26:47 +00:00
[codex] add code-mode host failure supervision hooks (#30110)
## Why A process host should be discarded and rebuilt after critical actor or V8 failure, while the existing in-process production path must keep its current cell-error semantics. This change establishes that failure boundary without adding the host process or remote client. ## What changed - add optional task-failure supervision to the transport-neutral code-mode session runtime - report Tokio cell-actor failures and V8 runtime-thread panics to a host-provided fail-stop handler - preserve the existing handler-less in-process behavior - make host-owned cell ID allocation fail before numeric wraparound ## Follow-up The V8 panic signal surfaced here should also be consumed by the `InProcessCodeModeSession` manager in a future change so it can fail the affected cell. This PR intentionally leaves the handler-less in-process behavior unchanged while putting the required panic tracking in place. ## Stack This is **2 of 4** in the process-owned code-mode session stack. - #30108 is merged into `main` - The next PR targets this branch ## Validation - `just test -p codex-code-mode` — 53 passed - `just argument-comment-lint -p codex-code-mode` - `just fix -p codex-code-mode`
This commit is contained in:
@@ -24,6 +24,7 @@ pub(crate) use self::types::SessionRuntimeDelegate;
|
||||
pub(crate) use self::types::ToolDefinition;
|
||||
pub(crate) use self::types::ToolKind;
|
||||
pub(crate) use self::types::ToolName;
|
||||
use crate::TaskFailureHandler;
|
||||
use crate::cell_actor::CellActor;
|
||||
use crate::cell_actor::CellError;
|
||||
use crate::cell_actor::CellEventFuture;
|
||||
@@ -46,11 +47,19 @@ struct Inner<D: SessionRuntimeDelegate> {
|
||||
cell_tasks: TaskTracker,
|
||||
shutdown_token: CancellationToken,
|
||||
delegate: Arc<D>,
|
||||
task_failure_handler: Option<TaskFailureHandler>,
|
||||
next_cell_id: AtomicU64,
|
||||
}
|
||||
|
||||
impl<D: SessionRuntimeDelegate> SessionRuntime<D> {
|
||||
pub(crate) fn new(delegate: Arc<D>) -> Self {
|
||||
Self::new_with_task_failure_handler(delegate, /*task_failure_handler*/ None)
|
||||
}
|
||||
|
||||
pub(crate) fn new_with_task_failure_handler(
|
||||
delegate: Arc<D>,
|
||||
task_failure_handler: Option<TaskFailureHandler>,
|
||||
) -> Self {
|
||||
Self {
|
||||
inner: Arc::new(Inner {
|
||||
stored_values: Mutex::new(HashMap::new()),
|
||||
@@ -58,6 +67,7 @@ impl<D: SessionRuntimeDelegate> SessionRuntime<D> {
|
||||
cell_tasks: TaskTracker::new(),
|
||||
shutdown_token: CancellationToken::new(),
|
||||
delegate,
|
||||
task_failure_handler,
|
||||
next_cell_id: AtomicU64::new(1),
|
||||
}),
|
||||
}
|
||||
@@ -71,7 +81,7 @@ impl<D: SessionRuntimeDelegate> SessionRuntime<D> {
|
||||
if self.inner.shutdown_token.is_cancelled() {
|
||||
return Err(Error::ShuttingDown);
|
||||
}
|
||||
let cell_id = self.allocate_cell_id();
|
||||
let cell_id = self.allocate_cell_id()?;
|
||||
let initial_event = self
|
||||
.start_cell(cell_id.clone(), request, initial_observe_mode)
|
||||
.await?;
|
||||
@@ -133,13 +143,14 @@ impl<D: SessionRuntimeDelegate> SessionRuntime<D> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn allocate_cell_id(&self) -> CellId {
|
||||
CellId::new(
|
||||
self.inner
|
||||
.next_cell_id
|
||||
.fetch_add(1, Ordering::Relaxed)
|
||||
.to_string(),
|
||||
)
|
||||
fn allocate_cell_id(&self) -> Result<CellId, Error> {
|
||||
self.inner
|
||||
.next_cell_id
|
||||
.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |next_cell_id| {
|
||||
next_cell_id.checked_add(1)
|
||||
})
|
||||
.map(|cell_id| CellId::new(cell_id.to_string()))
|
||||
.map_err(|_| Error::CellIdSpaceExhausted)
|
||||
}
|
||||
|
||||
async fn start_cell(
|
||||
@@ -167,10 +178,21 @@ impl<D: SessionRuntimeDelegate> SessionRuntime<D> {
|
||||
host,
|
||||
initial_observe_mode,
|
||||
cell_state,
|
||||
self.inner.task_failure_handler.clone(),
|
||||
)
|
||||
.map_err(Error::Runtime)?;
|
||||
cells.insert(cell_id.clone(), handle);
|
||||
self.inner.cell_tasks.spawn(task);
|
||||
let task = self.inner.cell_tasks.spawn(task);
|
||||
if let Some(task_failure_handler) = self.inner.task_failure_handler.clone() {
|
||||
let failed_cell_id = cell_id.clone();
|
||||
let _failure_watcher = self.inner.cell_tasks.spawn(async move {
|
||||
if let Err(err) = task.await {
|
||||
task_failure_handler(format!(
|
||||
"code-mode cell {failed_cell_id} task failed: {err}"
|
||||
));
|
||||
}
|
||||
});
|
||||
}
|
||||
drop(cells);
|
||||
Ok(map_actor_event(cell_id, initial_event))
|
||||
}
|
||||
|
||||
@@ -15,6 +15,8 @@ use crate::cell_actor::CompletionCommit;
|
||||
|
||||
struct RecordingDelegate;
|
||||
|
||||
struct PanickingClosedDelegate;
|
||||
|
||||
impl SessionRuntimeDelegate for RecordingDelegate {
|
||||
async fn invoke_tool(
|
||||
&self,
|
||||
@@ -37,6 +39,62 @@ impl SessionRuntimeDelegate for RecordingDelegate {
|
||||
fn cell_closed(&self, _cell_id: &CellId) {}
|
||||
}
|
||||
|
||||
impl SessionRuntimeDelegate for PanickingClosedDelegate {
|
||||
async fn invoke_tool(
|
||||
&self,
|
||||
_invocation: NestedToolCall,
|
||||
_cancellation_token: CancellationToken,
|
||||
) -> Result<JsonValue, String> {
|
||||
Ok(JsonValue::Null)
|
||||
}
|
||||
|
||||
async fn notify(
|
||||
&self,
|
||||
_call_id: String,
|
||||
_cell_id: CellId,
|
||||
_text: String,
|
||||
_cancellation_token: CancellationToken,
|
||||
) -> Result<(), String> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn cell_closed(&self, _cell_id: &CellId) {
|
||||
panic!("cell close panic probe");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reports_cell_actor_panics_to_the_owner() {
|
||||
let (failure_tx, mut failure_rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
let runtime = SessionRuntime::new_with_task_failure_handler(
|
||||
Arc::new(PanickingClosedDelegate),
|
||||
Some(Arc::new(move |reason| {
|
||||
let _ = failure_tx.send(reason);
|
||||
})),
|
||||
);
|
||||
let started = runtime
|
||||
.execute(
|
||||
execute_request(r#"text("done");"#),
|
||||
ObserveMode::YieldAfter(Duration::from_secs(1)),
|
||||
)
|
||||
.await
|
||||
.expect("start cell");
|
||||
assert_eq!(
|
||||
started.initial_event().await,
|
||||
Ok(CellEvent::Completed {
|
||||
content_items: vec![OutputItem::Text {
|
||||
text: "done".to_string(),
|
||||
}],
|
||||
error_text: None,
|
||||
})
|
||||
);
|
||||
runtime.shutdown().await.expect("shutdown runtime");
|
||||
let failure = failure_rx
|
||||
.try_recv()
|
||||
.expect("shutdown should wait for the cell failure watcher");
|
||||
assert!(failure.contains("code-mode cell 1 task failed"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn termination_rejects_a_waiting_store_commit_before_the_next_cell_can_load_it() {
|
||||
let runtime = SessionRuntime::new(Arc::new(RecordingDelegate));
|
||||
@@ -118,6 +176,26 @@ fn execute_request(source: &str) -> CreateCellRequest {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cell_id_allocation_fails_before_wrapping() {
|
||||
let runtime = SessionRuntime::new(Arc::new(RecordingDelegate));
|
||||
runtime
|
||||
.inner
|
||||
.next_cell_id
|
||||
.store(u64::MAX, Ordering::Relaxed);
|
||||
|
||||
assert_eq!(
|
||||
runtime
|
||||
.execute(
|
||||
execute_request(r#"text("unreachable");"#),
|
||||
ObserveMode::YieldAfter(Duration::from_secs(1)),
|
||||
)
|
||||
.await
|
||||
.err(),
|
||||
Some(Error::CellIdSpaceExhausted)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
|
||||
@@ -138,6 +138,7 @@ pub(crate) trait SessionRuntimeDelegate: Send + Sync + 'static {
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
pub(crate) enum Error {
|
||||
ShuttingDown,
|
||||
CellIdSpaceExhausted,
|
||||
DuplicateCell(CellId),
|
||||
MissingCell(CellId),
|
||||
BusyObserver(CellId),
|
||||
@@ -150,6 +151,9 @@ impl fmt::Display for Error {
|
||||
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
Self::ShuttingDown => formatter.write_str("code mode session is shutting down"),
|
||||
Self::CellIdSpaceExhausted => {
|
||||
formatter.write_str("code mode session exhausted its cell ID space")
|
||||
}
|
||||
Self::DuplicateCell(cell_id) => write!(formatter, "exec cell {cell_id} already exists"),
|
||||
Self::MissingCell(cell_id) => write!(formatter, "exec cell {cell_id} not found"),
|
||||
Self::BusyObserver(cell_id) => {
|
||||
|
||||
Reference in New Issue
Block a user