mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Persist session IDs across thread resume (#29327)
## Summary
A cold-resumed subagent kept its durable thread ID but could receive a
new session ID, splitting one agent tree across multiple sessions after
a restart.
Persist the root session ID in every rollout `SessionMeta`, carry it
through thread creation, and restore it before initializing the resumed
`Session` and `AgentControl`.
## Behavior
For a nested agent tree:
```text
root session R
parent thread P
child thread C
```
The child rollout stores:
```text
session_id: R
parent_thread_id: P
id: C
```
After a cold resume, the child still belongs to root session `R` while
its immediate parent remains `P`. The integration coverage uses distinct
values for all three IDs so it catches restoring the session from
`parent_thread_id`.
## Legacy rollouts
Previous rollouts have `id` but no `session_id`. `SessionMetaLine`
deserialization treats a missing `session_id` as `id`, keeping those
files readable, listable, and resumable. When a legacy subagent is
resumed through its root, that synthesized child ID no longer overrides
the inherited root-scoped `AgentControl`. New rollouts always persist
the explicit root session ID.
This commit is contained in:
@@ -200,6 +200,7 @@ impl ExternalAgentSessionImporter {
|
||||
};
|
||||
let now = Utc::now();
|
||||
let create_params = CreateThreadParams {
|
||||
session_id: thread_id.into(),
|
||||
thread_id,
|
||||
extra_config: None,
|
||||
forked_from_id: None,
|
||||
|
||||
@@ -1004,6 +1004,7 @@ mod thread_processor_behavior_tests {
|
||||
let timestamp = "2025-09-05T16:53:11.850Z".to_string();
|
||||
|
||||
let session_meta = SessionMeta {
|
||||
session_id: conversation_id.into(),
|
||||
id: conversation_id,
|
||||
timestamp: timestamp.clone(),
|
||||
model_provider: None,
|
||||
@@ -1060,6 +1061,7 @@ mod thread_processor_behavior_tests {
|
||||
let timestamp = "2025-09-05T16:53:11.850Z".to_string();
|
||||
|
||||
let session_meta = SessionMeta {
|
||||
session_id: parent_thread_id.into(),
|
||||
id: conversation_id,
|
||||
timestamp: timestamp.clone(),
|
||||
source: SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
@@ -1110,6 +1112,7 @@ mod thread_processor_behavior_tests {
|
||||
let timestamp = "2025-09-05T16:53:11.850Z".to_string();
|
||||
|
||||
let session_meta = SessionMeta {
|
||||
session_id: conversation_id.into(),
|
||||
id: conversation_id,
|
||||
forked_from_id: Some(forked_from_id),
|
||||
timestamp: timestamp.clone(),
|
||||
|
||||
@@ -13,6 +13,7 @@ fn extract_conversation_summary_prefers_plain_user_messages() -> Result<()> {
|
||||
|
||||
let head = vec![
|
||||
json!({
|
||||
"session_id": conversation_id.to_string(),
|
||||
"id": conversation_id.to_string(),
|
||||
"timestamp": timestamp,
|
||||
"cwd": "/",
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use anyhow::Result;
|
||||
use codex_protocol::SessionId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::GitInfo;
|
||||
@@ -127,11 +128,12 @@ pub fn create_fake_rollout_with_source(
|
||||
model_provider,
|
||||
git_info,
|
||||
source,
|
||||
/*session_id*/ None,
|
||||
/*parent_thread_id*/ None,
|
||||
)
|
||||
}
|
||||
|
||||
/// Create a minimal rollout file with an explicit session source and control parent.
|
||||
/// Create a minimal rollout file with an explicit root session and control parent.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn create_fake_parented_rollout_with_source(
|
||||
codex_home: &Path,
|
||||
@@ -141,6 +143,7 @@ pub fn create_fake_parented_rollout_with_source(
|
||||
model_provider: Option<&str>,
|
||||
git_info: Option<GitInfo>,
|
||||
source: SessionSource,
|
||||
session_id: SessionId,
|
||||
parent_thread_id: ThreadId,
|
||||
) -> Result<String> {
|
||||
create_fake_rollout_with_source_and_parent_thread_id(
|
||||
@@ -151,6 +154,7 @@ pub fn create_fake_parented_rollout_with_source(
|
||||
model_provider,
|
||||
git_info,
|
||||
source,
|
||||
Some(session_id),
|
||||
Some(parent_thread_id),
|
||||
)
|
||||
}
|
||||
@@ -164,11 +168,13 @@ fn create_fake_rollout_with_source_and_parent_thread_id(
|
||||
model_provider: Option<&str>,
|
||||
git_info: Option<GitInfo>,
|
||||
source: SessionSource,
|
||||
session_id: Option<SessionId>,
|
||||
parent_thread_id: Option<ThreadId>,
|
||||
) -> Result<String> {
|
||||
let uuid = Uuid::new_v4();
|
||||
let uuid_str = uuid.to_string();
|
||||
let conversation_id = ThreadId::from_string(&uuid_str)?;
|
||||
let session_id = session_id.unwrap_or_else(|| conversation_id.into());
|
||||
|
||||
let file_path = rollout_path(codex_home, filename_ts, &uuid_str);
|
||||
let dir = file_path
|
||||
@@ -178,6 +184,7 @@ fn create_fake_rollout_with_source_and_parent_thread_id(
|
||||
|
||||
// Build JSONL lines
|
||||
let meta = SessionMeta {
|
||||
session_id,
|
||||
id: conversation_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id,
|
||||
@@ -264,6 +271,7 @@ pub fn create_fake_rollout_with_text_elements(
|
||||
|
||||
// Build JSONL lines
|
||||
let meta = SessionMeta {
|
||||
session_id: conversation_id.into(),
|
||||
id: conversation_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id: None,
|
||||
|
||||
@@ -121,6 +121,7 @@ async fn get_conversation_summary_by_thread_id_reads_pathless_store_thread() ->
|
||||
let thread_id = ThreadId::from_string("00000000-0000-4000-8000-000000000125")?;
|
||||
store
|
||||
.create_thread(CreateThreadParams {
|
||||
session_id: thread_id.into(),
|
||||
thread_id,
|
||||
extra_config: None,
|
||||
forked_from_id: None,
|
||||
|
||||
@@ -300,7 +300,7 @@ async fn review_start_sends_parent_lineage_in_turn_metadata_for_thread_fork_v2()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_sends_other_subagent_lineage_after_cold_thread_resume_v2() -> Result<()> {
|
||||
async fn turn_start_sends_nested_subagent_lineage_after_cold_thread_resume_v2() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
@@ -321,6 +321,8 @@ async fn turn_start_sends_other_subagent_lineage_after_cold_thread_resume_v2() -
|
||||
/*supports_websockets*/ false,
|
||||
)?;
|
||||
|
||||
let root_thread_id = CoreThreadId::new();
|
||||
let root_thread_id_str = root_thread_id.to_string();
|
||||
let parent_thread_id = CoreThreadId::new();
|
||||
let parent_thread_id_str = parent_thread_id.to_string();
|
||||
let subagent_thread_id = create_fake_parented_rollout_with_source(
|
||||
@@ -331,6 +333,7 @@ async fn turn_start_sends_other_subagent_lineage_after_cold_thread_resume_v2() -
|
||||
Some("mock_provider"),
|
||||
/*git_info*/ None,
|
||||
SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())),
|
||||
root_thread_id.into(),
|
||||
parent_thread_id,
|
||||
)?;
|
||||
|
||||
@@ -350,6 +353,7 @@ async fn turn_start_sends_other_subagent_lineage_after_cold_thread_resume_v2() -
|
||||
.await??;
|
||||
let ThreadResumeResponse { thread, .. } = to_response::<ThreadResumeResponse>(resume_resp)?;
|
||||
assert_eq!(thread.id, subagent_thread_id);
|
||||
assert_eq!(thread.session_id, root_thread_id_str);
|
||||
assert_eq!(thread.parent_thread_id, Some(parent_thread_id_str.clone()));
|
||||
assert_eq!(
|
||||
thread.source,
|
||||
@@ -390,6 +394,10 @@ async fn turn_start_sends_other_subagent_lineage_after_cold_thread_resume_v2() -
|
||||
Some(parent_thread_id_str.as_str())
|
||||
);
|
||||
assert_eq!(metadata["subagent_kind"].as_str(), Some("guardian"));
|
||||
assert_eq!(
|
||||
metadata["session_id"].as_str(),
|
||||
Some(thread.session_id.as_str())
|
||||
);
|
||||
assert_eq!(metadata["thread_id"].as_str(), Some(thread.id.as_str()));
|
||||
assert_eq!(metadata["turn_id"].as_str(), Some(turn.id.as_str()));
|
||||
assert!(metadata.get("forked_from_thread_id").is_none());
|
||||
|
||||
@@ -147,6 +147,7 @@ async fn thread_delete_with_non_local_thread_store_does_not_create_local_persist
|
||||
let unloaded_thread_id = ThreadId::from_string(&Uuid::new_v4().to_string())?;
|
||||
thread_store
|
||||
.create_thread(StoreCreateThreadParams {
|
||||
session_id: unloaded_thread_id.into(),
|
||||
thread_id: unloaded_thread_id,
|
||||
extra_config: None,
|
||||
forked_from_id: None,
|
||||
|
||||
@@ -1252,6 +1252,7 @@ async fn thread_list_filters_by_subagent_variant() -> Result<()> {
|
||||
Some("mock_provider"),
|
||||
/*git_info*/ None,
|
||||
CoreSessionSource::SubAgent(SubAgentSource::Review),
|
||||
parent_thread_id.into(),
|
||||
parent_thread_id,
|
||||
)?;
|
||||
let compact_id = create_fake_rollout_with_source(
|
||||
|
||||
@@ -1357,6 +1357,7 @@ async fn seed_pathless_store_thread(
|
||||
) -> Result<()> {
|
||||
store
|
||||
.create_thread(CreateThreadParams {
|
||||
session_id: thread_id.into(),
|
||||
thread_id,
|
||||
extra_config: None,
|
||||
forked_from_id: None,
|
||||
|
||||
@@ -2054,6 +2054,7 @@ stream_max_retries = 0
|
||||
let rollout_dir = rollout_path.parent().expect("rollout parent directory");
|
||||
std::fs::create_dir_all(rollout_dir)?;
|
||||
let session_meta = SessionMeta {
|
||||
session_id: conversation_id.into(),
|
||||
id: conversation_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id: None,
|
||||
@@ -2735,6 +2736,7 @@ async fn thread_resume_rejects_mismatched_path_for_running_thread_id() -> Result
|
||||
"timestamp": "2025-01-01T00:00:00Z",
|
||||
"type": "session_meta",
|
||||
"payload": {
|
||||
"session_id": thread_uuid,
|
||||
"id": thread_uuid,
|
||||
"timestamp": "2025-01-01T00:00:00Z",
|
||||
"cwd": codex_home.path(),
|
||||
|
||||
@@ -208,6 +208,7 @@ async fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> {
|
||||
let parent_thread_id = ThreadId::from_string("00000000-0000-4000-8000-000000000127")?;
|
||||
store
|
||||
.create_thread(CreateThreadParams {
|
||||
session_id: thread_id.into(),
|
||||
thread_id,
|
||||
extra_config: None,
|
||||
forked_from_id: Some(parent_thread_id),
|
||||
|
||||
Reference in New Issue
Block a user