mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
code-mode: linearize cell terminal state (#29286)
## Summary - Introduce a single cell terminal-state machine for completion and termination. - Make stored-value commits atomic with the winning terminal outcome. - Buffer terminal results for later observation and cover termination-before-commit behavior. ## Why Completion, termination, observation, and stored-value updates must agree on one linearized outcome under cancellation races. ## Impact Terminal delivery becomes deterministic and terminated cells cannot commit state after termination wins. ## Validation - Focused terminal-state regression passed. - Stack-tip validation: `just test -p codex-code-mode -p codex-code-mode-protocol` (70 passed). - Parent branch: `cconger/code-mode-runtime-compact-03b-session-runtime`.
This commit is contained in:
committed by
GitHub
Unverified
parent
63f009e9da
commit
f774455c3a
@@ -29,7 +29,9 @@ use crate::cell_actor::CellError;
|
||||
use crate::cell_actor::CellEventFuture;
|
||||
use crate::cell_actor::CellHandle;
|
||||
use crate::cell_actor::CellHost;
|
||||
use crate::cell_actor::CellState;
|
||||
use crate::cell_actor::CellToolCall;
|
||||
use crate::cell_actor::CompletionCommit;
|
||||
|
||||
type RuntimeEventFuture = Pin<Box<dyn Future<Output = Result<CellEvent, Error>> + Send + 'static>>;
|
||||
|
||||
@@ -165,9 +167,15 @@ impl<D: SessionRuntimeDelegate> SessionRuntime<D> {
|
||||
if cells.contains_key(&cell_id) {
|
||||
return Err(Error::DuplicateCell(cell_id));
|
||||
}
|
||||
let (handle, initial_event, task) =
|
||||
CellActor::prepare(request, stored_values, host, initial_observe_mode)
|
||||
.map_err(Error::Runtime)?;
|
||||
let cell_state = Arc::new(CellState::new(CancellationToken::new()));
|
||||
let (handle, initial_event, task) = CellActor::prepare(
|
||||
request,
|
||||
stored_values,
|
||||
host,
|
||||
initial_observe_mode,
|
||||
cell_state,
|
||||
)
|
||||
.map_err(Error::Runtime)?;
|
||||
cells.insert(cell_id.clone(), handle);
|
||||
drop(cells);
|
||||
tokio::spawn(task);
|
||||
@@ -251,12 +259,21 @@ impl<D: SessionRuntimeDelegate> CellHost for RuntimeCellHost<D> {
|
||||
.await
|
||||
}
|
||||
|
||||
async fn commit_stored_values(&self, stored_value_writes: HashMap<String, JsonValue>) {
|
||||
self.inner
|
||||
.stored_values
|
||||
.lock()
|
||||
.await
|
||||
.extend(stored_value_writes);
|
||||
async fn commit_completion(
|
||||
&self,
|
||||
stored_value_writes: HashMap<String, JsonValue>,
|
||||
event: CellEvent,
|
||||
cell_state: Arc<CellState>,
|
||||
) -> CompletionCommit {
|
||||
let cancellation_token = cell_state.cancellation_token();
|
||||
let mut stored_values = tokio::select! {
|
||||
biased;
|
||||
_ = cancellation_token.cancelled() => {
|
||||
return CompletionCommit::Rejected(event);
|
||||
}
|
||||
stored_values = self.inner.stored_values.lock() => stored_values,
|
||||
};
|
||||
cell_state.commit_completion(event, || stored_values.extend(stored_value_writes))
|
||||
}
|
||||
|
||||
async fn closed(&self) {
|
||||
@@ -276,3 +293,7 @@ fn actor_error(cell_id: &CellId, error: CellError) -> Error {
|
||||
CellError::Closed => Error::ClosedCell(cell_id.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
use std::collections::HashMap;
|
||||
use std::future::Future;
|
||||
use std::sync::Arc;
|
||||
use std::task::Context;
|
||||
use std::task::Poll;
|
||||
use std::task::Waker;
|
||||
use std::time::Duration;
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value as JsonValue;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use super::*;
|
||||
use crate::cell_actor::CompletionCommit;
|
||||
|
||||
struct RecordingDelegate;
|
||||
|
||||
impl SessionRuntimeDelegate for RecordingDelegate {
|
||||
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) {}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn termination_rejects_a_waiting_store_commit_before_the_next_cell_can_load_it() {
|
||||
let runtime = SessionRuntime::new(Arc::new(RecordingDelegate));
|
||||
let cell_state = Arc::new(CellState::new(CancellationToken::new()));
|
||||
let host = RuntimeCellHost {
|
||||
cell_id: CellId::new("terminating-writer"),
|
||||
inner: Arc::clone(&runtime.inner),
|
||||
};
|
||||
let completion = CellEvent::Completed {
|
||||
content_items: vec![OutputItem::Text {
|
||||
text: "uncommitted output".to_string(),
|
||||
}],
|
||||
error_text: None,
|
||||
};
|
||||
|
||||
let stored_values = runtime.inner.stored_values.lock().await;
|
||||
let commit = host.commit_completion(
|
||||
HashMap::from([(
|
||||
"candidate".to_string(),
|
||||
JsonValue::String("lost".to_string()),
|
||||
)]),
|
||||
completion.clone(),
|
||||
Arc::clone(&cell_state),
|
||||
);
|
||||
tokio::pin!(commit);
|
||||
let waker = Waker::noop();
|
||||
let mut context = Context::from_waker(waker);
|
||||
assert!(matches!(commit.as_mut().poll(&mut context), Poll::Pending));
|
||||
|
||||
let termination = cell_state.request_termination();
|
||||
drop(stored_values);
|
||||
assert_eq!(commit.await, CompletionCommit::Rejected(completion));
|
||||
let terminated = CellEvent::Terminated {
|
||||
content_items: Vec::new(),
|
||||
};
|
||||
assert_eq!(
|
||||
cell_state.finish_termination(terminated.clone()),
|
||||
Some(terminated.clone())
|
||||
);
|
||||
assert_eq!(termination.await, Ok(terminated));
|
||||
assert!(
|
||||
!runtime
|
||||
.inner
|
||||
.stored_values
|
||||
.lock()
|
||||
.await
|
||||
.contains_key("candidate")
|
||||
);
|
||||
|
||||
let reader = runtime
|
||||
.execute(
|
||||
CreateCellRequest {
|
||||
tool_call_id: "reader".to_string(),
|
||||
enabled_tools: Vec::new(),
|
||||
source: r#"text(String(load("candidate")));"#.to_string(),
|
||||
},
|
||||
ObserveMode::YieldAfter(Duration::from_secs(1)),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
reader.initial_event().await,
|
||||
Ok(CellEvent::Completed {
|
||||
content_items: vec![OutputItem::Text {
|
||||
text: "undefined".to_string(),
|
||||
}],
|
||||
error_text: None,
|
||||
})
|
||||
);
|
||||
runtime.shutdown().await.unwrap();
|
||||
}
|
||||
Reference in New Issue
Block a user