mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Celia ChenandGitHub
641d5268fa
Problem:
1. turn id is constructed in-memory;
2. on resuming threads, turn_id might not be unique;
3. client cannot no the boundary of a turn from rollout files easily.
This PR does three things:
1. persist `task_started` and `task_complete` events;
1. persist `turn_id` in rollout turn events;
5. generate turn_id as unique uuids instead of incrementing it in
memory.
This helps us resolve the issue of clients wanting to have unique turn
ids for resuming a thread, and knowing the boundry of each turn in
rollout files.
example debug logs
```
2026-02-11T00:32:10.746876Z DEBUG codex_app_server_protocol::protocol::thread_history: built turn from rollout items turn_index=8 turn=Turn { id: "019c4a07-d809-74c3-bc4b-fd9618487b4b", items: [UserMessage { id: "item-24", content: [Text { text: "hi", text_elements: [] }] }, AgentMessage { id: "item-25", text: "Hi. I’m in the workspace with your current changes loaded and ready. Send the next task and I’ll execute it end-to-end." }], status: Completed, error: None }
2026-02-11T00:32:10.746888Z DEBUG codex_app_server_protocol::protocol::thread_history: built turn from rollout items turn_index=9 turn=Turn { id: "019c4a18-1004-76c0-a0fb-a77610f6a9b8", items: [UserMessage { id: "item-26", content: [Text { text: "hello", text_elements: [] }] }, AgentMessage { id: "item-27", text: "Hello. Ready for the next change in `codex-rs`; I can continue from the current in-progress diff or start a new task." }], status: Completed, error: None }
2026-02-11T00:32:10.746899Z DEBUG codex_app_server_protocol::protocol::thread_history: built turn from rollout items turn_index=10 turn=Turn { id: "019c4a19-41f0-7db0-ad78-74f1503baeb8", items: [UserMessage { id: "item-28", content: [Text { text: "hello", text_elements: [] }] }, AgentMessage { id: "item-29", text: "Hello. Send the specific change you want in `codex-rs`, and I’ll implement it and run the required checks." }], status: Completed, error: None }
```
backward compatibility:
if you try to resume an old session without task_started and
task_complete event populated, the following happens:
- If you resume and do nothing: those reconstructed historical IDs can
differ next time you resume.
- If you resume and send a new turn: the new turn gets a fresh UUID from
live submission flow and is persisted, so that new turn’s ID is stable
on later resumes.
I think this behavior is fine, because we only care about deterministic
turn id once a turn is triggered.
88 lines
3.2 KiB
Rust
88 lines
3.2 KiB
Rust
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
|
|
|
use codex_core::AuthManager;
|
|
use codex_core::CodexAuth;
|
|
use codex_core::NewThread;
|
|
use codex_core::ThreadManager;
|
|
use codex_core::protocol::EventMsg;
|
|
use codex_core::protocol::InitialHistory;
|
|
use codex_core::protocol::ResumedHistory;
|
|
use codex_core::protocol::RolloutItem;
|
|
use codex_core::protocol::TurnContextItem;
|
|
use codex_core::protocol::WarningEvent;
|
|
use codex_protocol::ThreadId;
|
|
use core::time::Duration;
|
|
use core_test_support::load_default_config_for_test;
|
|
use core_test_support::wait_for_event;
|
|
use tempfile::TempDir;
|
|
|
|
fn resume_history(
|
|
config: &codex_core::config::Config,
|
|
previous_model: &str,
|
|
rollout_path: &std::path::Path,
|
|
) -> InitialHistory {
|
|
let turn_ctx = TurnContextItem {
|
|
turn_id: None,
|
|
cwd: config.cwd.clone(),
|
|
approval_policy: config.approval_policy.value(),
|
|
sandbox_policy: config.sandbox_policy.get().clone(),
|
|
model: previous_model.to_string(),
|
|
personality: None,
|
|
collaboration_mode: None,
|
|
effort: config.model_reasoning_effort,
|
|
summary: config.model_reasoning_summary,
|
|
user_instructions: None,
|
|
developer_instructions: None,
|
|
final_output_json_schema: None,
|
|
truncation_policy: None,
|
|
};
|
|
|
|
InitialHistory::Resumed(ResumedHistory {
|
|
conversation_id: ThreadId::default(),
|
|
history: vec![RolloutItem::TurnContext(turn_ctx)],
|
|
rollout_path: rollout_path.to_path_buf(),
|
|
})
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn emits_warning_when_resumed_model_differs() {
|
|
// Arrange a config with a current model and a prior rollout recorded under a different model.
|
|
let home = TempDir::new().expect("tempdir");
|
|
let mut config = load_default_config_for_test(&home).await;
|
|
config.model = Some("current-model".to_string());
|
|
// Ensure cwd is absolute (the helper sets it to the temp dir already).
|
|
assert!(config.cwd.is_absolute());
|
|
|
|
let rollout_path = home.path().join("rollout.jsonl");
|
|
std::fs::write(&rollout_path, "").expect("create rollout placeholder");
|
|
|
|
let initial_history = resume_history(&config, "previous-model", &rollout_path);
|
|
|
|
let thread_manager = ThreadManager::with_models_provider(
|
|
CodexAuth::from_api_key("test"),
|
|
config.model_provider.clone(),
|
|
);
|
|
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("test"));
|
|
|
|
// Act: resume the conversation.
|
|
let NewThread {
|
|
thread: conversation,
|
|
..
|
|
} = thread_manager
|
|
.resume_thread_with_history(config, initial_history, auth_manager)
|
|
.await
|
|
.expect("resume conversation");
|
|
|
|
// Assert: a Warning event is emitted describing the model mismatch.
|
|
let warning = wait_for_event(&conversation, |ev| matches!(ev, EventMsg::Warning(_))).await;
|
|
let EventMsg::Warning(WarningEvent { message }) = warning else {
|
|
panic!("expected warning event");
|
|
};
|
|
assert!(message.contains("previous-model"));
|
|
assert!(message.contains("current-model"));
|
|
|
|
// Drain the TurnComplete/Shutdown window to avoid leaking tasks between tests.
|
|
// The warning is emitted during initialization, so a short sleep is sufficient.
|
|
tokio::time::sleep(Duration::from_millis(50)).await;
|
|
}
|