This PR does two things because after I got deep into the first one I started pulling on the thread to the second: - Makes `ConversationManager` the place where all in-memory conversations are created and stored. Previously, `MessageProcessor` in the `codex-mcp-server` crate was doing this via its `session_map`, but this is something that should be done in `codex-core`. - It unwinds the `ctrl_c: tokio::sync::Notify` that was threaded throughout our code. I think this made sense at one time, but now that we handle Ctrl-C within the TUI and have a proper `Op::Interrupt` event, I don't think this was quite right, so I removed it. For `codex exec` and `codex proto`, we now use `tokio::signal::ctrl_c()` directly, but we no longer make `Notify` a field of `Codex` or `CodexConversation`. Changes of note: - Adds the files `conversation_manager.rs` and `codex_conversation.rs` to `codex-core`. - `Codex` and `CodexSpawnOk` are no longer exported from `codex-core`: other crates must use `CodexConversation` instead (which is created via `ConversationManager`). - `core/src/codex_wrapper.rs` has been deleted in favor of `ConversationManager`. - `ConversationManager::new_conversation()` returns `NewConversation`, which is in line with the `new_conversation` tool we want to add to the MCP server. Note `NewConversation` includes `SessionConfiguredEvent`, so we eliminate checks in cases like `codex-rs/core/tests/client.rs` to verify `SessionConfiguredEvent` is the first event because that is now internal to `ConversationManager`. - Quite a bit of code was deleted from `codex-rs/mcp-server/src/message_processor.rs` since it no longer has to manage multiple conversations itself: it goes through `ConversationManager` instead. - `core/tests/live_agent.rs` has been deleted because I had to update a bunch of tests and all the tests in here were ignored, and I don't think anyone ever ran them, so this was just technical debt, at this point. - Removed `notify_on_sigint()` from `util.rs` (and in a follow-up, I hope to refactor the blandly-named `util.rs` into more descriptive files). - In general, I started replacing local variables named `codex` as `conversation`, where appropriate, though admittedly I didn't do it through all the integration tests because that would have added a lot of noise to this PR. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2240). * #2264 * #2263 * __->__ #2240
107 lines
3.8 KiB
Rust
107 lines
3.8 KiB
Rust
#![allow(clippy::expect_used)]
|
||
|
||
use tempfile::TempDir;
|
||
|
||
use codex_core::CodexConversation;
|
||
use codex_core::config::Config;
|
||
use codex_core::config::ConfigOverrides;
|
||
use codex_core::config::ConfigToml;
|
||
|
||
/// Returns a default `Config` whose on-disk state is confined to the provided
|
||
/// temporary directory. Using a per-test directory keeps tests hermetic and
|
||
/// avoids clobbering a developer’s real `~/.codex`.
|
||
pub fn load_default_config_for_test(codex_home: &TempDir) -> Config {
|
||
Config::load_from_base_config_with_overrides(
|
||
ConfigToml::default(),
|
||
ConfigOverrides::default(),
|
||
codex_home.path().to_path_buf(),
|
||
)
|
||
.expect("defaults for test should always succeed")
|
||
}
|
||
|
||
/// Builds an SSE stream body from a JSON fixture.
|
||
///
|
||
/// The fixture must contain an array of objects where each object represents a
|
||
/// single SSE event with at least a `type` field matching the `event:` value.
|
||
/// Additional fields become the JSON payload for the `data:` line. An object
|
||
/// with only a `type` field results in an event with no `data:` section. This
|
||
/// makes it trivial to extend the fixtures as OpenAI adds new event kinds or
|
||
/// fields.
|
||
pub fn load_sse_fixture(path: impl AsRef<std::path::Path>) -> String {
|
||
let events: Vec<serde_json::Value> =
|
||
serde_json::from_reader(std::fs::File::open(path).expect("read fixture"))
|
||
.expect("parse JSON fixture");
|
||
events
|
||
.into_iter()
|
||
.map(|e| {
|
||
let kind = e
|
||
.get("type")
|
||
.and_then(|v| v.as_str())
|
||
.expect("fixture event missing type");
|
||
if e.as_object().map(|o| o.len() == 1).unwrap_or(false) {
|
||
format!("event: {kind}\n\n")
|
||
} else {
|
||
format!("event: {kind}\ndata: {e}\n\n")
|
||
}
|
||
})
|
||
.collect()
|
||
}
|
||
|
||
/// Same as [`load_sse_fixture`], but replaces the placeholder `__ID__` in the
|
||
/// fixture template with the supplied identifier before parsing. This lets a
|
||
/// single JSON template be reused by multiple tests that each need a unique
|
||
/// `response_id`.
|
||
pub fn load_sse_fixture_with_id(path: impl AsRef<std::path::Path>, id: &str) -> String {
|
||
let raw = std::fs::read_to_string(path).expect("read fixture template");
|
||
let replaced = raw.replace("__ID__", id);
|
||
let events: Vec<serde_json::Value> =
|
||
serde_json::from_str(&replaced).expect("parse JSON fixture");
|
||
events
|
||
.into_iter()
|
||
.map(|e| {
|
||
let kind = e
|
||
.get("type")
|
||
.and_then(|v| v.as_str())
|
||
.expect("fixture event missing type");
|
||
if e.as_object().map(|o| o.len() == 1).unwrap_or(false) {
|
||
format!("event: {kind}\n\n")
|
||
} else {
|
||
format!("event: {kind}\ndata: {e}\n\n")
|
||
}
|
||
})
|
||
.collect()
|
||
}
|
||
|
||
pub async fn wait_for_event<F>(
|
||
codex: &CodexConversation,
|
||
predicate: F,
|
||
) -> codex_core::protocol::EventMsg
|
||
where
|
||
F: FnMut(&codex_core::protocol::EventMsg) -> bool,
|
||
{
|
||
use tokio::time::Duration;
|
||
wait_for_event_with_timeout(codex, predicate, Duration::from_secs(1)).await
|
||
}
|
||
|
||
pub async fn wait_for_event_with_timeout<F>(
|
||
codex: &CodexConversation,
|
||
mut predicate: F,
|
||
wait_time: tokio::time::Duration,
|
||
) -> codex_core::protocol::EventMsg
|
||
where
|
||
F: FnMut(&codex_core::protocol::EventMsg) -> bool,
|
||
{
|
||
use tokio::time::Duration;
|
||
use tokio::time::timeout;
|
||
loop {
|
||
// Allow a bit more time to accommodate async startup work (e.g. config IO, tool discovery)
|
||
let ev = timeout(wait_time.max(Duration::from_secs(5)), codex.next_event())
|
||
.await
|
||
.expect("timeout waiting for event")
|
||
.expect("stream ended unexpectedly");
|
||
if predicate(&ev.msg) {
|
||
return ev.msg;
|
||
}
|
||
}
|
||
}
|