mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
store and expose parent_thread_id on Threads (#25113)
## Why This PR https://github.com/openai/codex/pull/24161#discussion_r3325692763 revealed a subagent data modeling issue, where we overloaded `forked_from_id` to also mean `parent_thread_id`. That's incorrect since guardian and review subagents can be a subagent and NOT fork the main thread's history. The solution here is to explicitly store a new `parent_thread_id` on `SessionMeta`, alongside `forked_from_id` which already exists. While we're at it, also expose it in the app-server protocol on the `Thread` object. A thread->subagent relationship and a fork of thread history are orthogonal concepts. ## What Changed - Added top-level `parent_thread_id` persistence on `SessionMeta` and runtime/session plumbing through `SessionConfiguredEvent`, `CodexSpawnArgs`, `SessionConfiguration`, `ThreadConfigSnapshot`, `TurnContext`, and `ModelClient`. - Made turn metadata, request headers, analytics, and subagent-start events read the separate runtime/top-level parent field instead of deriving general parent lineage from `SessionSource` or `forked_from_thread_id`. - Passed parent lineage separately at delegated subagent, review, guardian, agent-job, and multi-agent spawn construction sites; copied-history fork lineage remains derived only from `InitialHistory`. - Persisted and exposed parent lineage through rollout/thread-store projections and app-server v2 `Thread.parentThreadId`. - Updated app-server README text and regenerated app-server schema fixtures for the additive `parentThreadId` response field.
This commit is contained in:
@@ -38,6 +38,7 @@ pub use responses::create_final_assistant_message_sse_response;
|
||||
pub use responses::create_request_permissions_sse_response;
|
||||
pub use responses::create_request_user_input_sse_response;
|
||||
pub use responses::create_shell_command_sse_response;
|
||||
pub use rollout::create_fake_parented_rollout_with_source;
|
||||
pub use rollout::create_fake_rollout;
|
||||
pub use rollout::create_fake_rollout_with_source;
|
||||
pub use rollout::create_fake_rollout_with_text_elements;
|
||||
|
||||
@@ -118,6 +118,53 @@ pub fn create_fake_rollout_with_source(
|
||||
model_provider: Option<&str>,
|
||||
git_info: Option<GitInfo>,
|
||||
source: SessionSource,
|
||||
) -> Result<String> {
|
||||
create_fake_rollout_with_source_and_parent_thread_id(
|
||||
codex_home,
|
||||
filename_ts,
|
||||
meta_rfc3339,
|
||||
preview,
|
||||
model_provider,
|
||||
git_info,
|
||||
source,
|
||||
/*parent_thread_id*/ None,
|
||||
)
|
||||
}
|
||||
|
||||
/// Create a minimal rollout file with an explicit session source and control parent.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn create_fake_parented_rollout_with_source(
|
||||
codex_home: &Path,
|
||||
filename_ts: &str,
|
||||
meta_rfc3339: &str,
|
||||
preview: &str,
|
||||
model_provider: Option<&str>,
|
||||
git_info: Option<GitInfo>,
|
||||
source: SessionSource,
|
||||
parent_thread_id: ThreadId,
|
||||
) -> Result<String> {
|
||||
create_fake_rollout_with_source_and_parent_thread_id(
|
||||
codex_home,
|
||||
filename_ts,
|
||||
meta_rfc3339,
|
||||
preview,
|
||||
model_provider,
|
||||
git_info,
|
||||
source,
|
||||
Some(parent_thread_id),
|
||||
)
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn create_fake_rollout_with_source_and_parent_thread_id(
|
||||
codex_home: &Path,
|
||||
filename_ts: &str,
|
||||
meta_rfc3339: &str,
|
||||
preview: &str,
|
||||
model_provider: Option<&str>,
|
||||
git_info: Option<GitInfo>,
|
||||
source: SessionSource,
|
||||
parent_thread_id: Option<ThreadId>,
|
||||
) -> Result<String> {
|
||||
let uuid = Uuid::new_v4();
|
||||
let uuid_str = uuid.to_string();
|
||||
@@ -133,6 +180,7 @@ pub fn create_fake_rollout_with_source(
|
||||
let meta = SessionMeta {
|
||||
id: conversation_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id,
|
||||
timestamp: meta_rfc3339.to_string(),
|
||||
cwd: PathBuf::from("/"),
|
||||
originator: "codex".to_string(),
|
||||
@@ -217,6 +265,7 @@ pub fn create_fake_rollout_with_text_elements(
|
||||
let meta = SessionMeta {
|
||||
id: conversation_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id: None,
|
||||
timestamp: meta_rfc3339.to_string(),
|
||||
cwd: PathBuf::from("/"),
|
||||
originator: "codex".to_string(),
|
||||
|
||||
@@ -124,6 +124,7 @@ async fn get_conversation_summary_by_thread_id_reads_pathless_store_thread() ->
|
||||
.create_thread(CreateThreadParams {
|
||||
thread_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id: None,
|
||||
source: SessionSource::Cli,
|
||||
thread_source: None,
|
||||
base_instructions: BaseInstructions::default(),
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use anyhow::Result;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::create_fake_parented_rollout_with_source;
|
||||
use app_test_support::create_fake_rollout;
|
||||
use app_test_support::create_fake_rollout_with_source;
|
||||
use app_test_support::to_response;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
@@ -9,6 +9,7 @@ use codex_app_server_protocol::ReviewDelivery;
|
||||
use codex_app_server_protocol::ReviewStartParams;
|
||||
use codex_app_server_protocol::ReviewStartResponse;
|
||||
use codex_app_server_protocol::ReviewTarget;
|
||||
use codex_app_server_protocol::SessionSource as ApiSessionSource;
|
||||
use codex_app_server_protocol::ThreadForkParams;
|
||||
use codex_app_server_protocol::ThreadForkResponse;
|
||||
use codex_app_server_protocol::ThreadResumeParams;
|
||||
@@ -198,7 +199,7 @@ async fn turn_start_sends_fork_lineage_in_turn_metadata_for_thread_fork_v2() ->
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn review_start_sends_fork_lineage_in_turn_metadata_for_thread_fork_v2() -> Result<()> {
|
||||
async fn review_start_sends_parent_lineage_in_turn_metadata_for_thread_fork_v2() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let review_payload = serde_json::json!({
|
||||
@@ -276,8 +277,9 @@ async fn review_start_sends_fork_lineage_in_turn_metadata_for_thread_fork_v2() -
|
||||
request.header("x-openai-subagent").as_deref(),
|
||||
Some("review")
|
||||
);
|
||||
assert!(metadata.get("forked_from_thread_id").is_none());
|
||||
assert_eq!(
|
||||
metadata["forked_from_thread_id"].as_str(),
|
||||
metadata["parent_thread_id"].as_str(),
|
||||
Some(review_thread_id.as_str())
|
||||
);
|
||||
let review_request_thread_id = metadata["thread_id"]
|
||||
@@ -297,7 +299,7 @@ async fn review_start_sends_fork_lineage_in_turn_metadata_for_thread_fork_v2() -
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_sends_subagent_lineage_after_cold_thread_resume_v2() -> Result<()> {
|
||||
async fn turn_start_sends_other_subagent_lineage_after_cold_thread_resume_v2() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
@@ -320,20 +322,15 @@ async fn turn_start_sends_subagent_lineage_after_cold_thread_resume_v2() -> Resu
|
||||
|
||||
let parent_thread_id = CoreThreadId::new();
|
||||
let parent_thread_id_str = parent_thread_id.to_string();
|
||||
let subagent_thread_id = create_fake_rollout_with_source(
|
||||
let subagent_thread_id = create_fake_parented_rollout_with_source(
|
||||
codex_home.path(),
|
||||
"2025-01-05T12-00-00",
|
||||
"2025-01-05T12:00:00Z",
|
||||
"Saved subagent message",
|
||||
Some("mock_provider"),
|
||||
/*git_info*/ None,
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth: 1,
|
||||
agent_path: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
}),
|
||||
SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())),
|
||||
parent_thread_id,
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
@@ -352,6 +349,11 @@ async fn turn_start_sends_subagent_lineage_after_cold_thread_resume_v2() -> Resu
|
||||
.await??;
|
||||
let ThreadResumeResponse { thread, .. } = to_response::<ThreadResumeResponse>(resume_resp)?;
|
||||
assert_eq!(thread.id, subagent_thread_id);
|
||||
assert_eq!(thread.parent_thread_id, Some(parent_thread_id_str.clone()));
|
||||
assert_eq!(
|
||||
thread.source,
|
||||
ApiSessionSource::SubAgent(SubAgentSource::Other("guardian".to_string()))
|
||||
);
|
||||
|
||||
let turn_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
@@ -386,7 +388,7 @@ async fn turn_start_sends_subagent_lineage_after_cold_thread_resume_v2() -> Resu
|
||||
metadata["parent_thread_id"].as_str(),
|
||||
Some(parent_thread_id_str.as_str())
|
||||
);
|
||||
assert_eq!(metadata["subagent_kind"].as_str(), Some("thread_spawn"));
|
||||
assert_eq!(metadata["subagent_kind"].as_str(), Some("guardian"));
|
||||
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());
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use anyhow::Result;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::create_fake_parented_rollout_with_source;
|
||||
use app_test_support::create_fake_rollout;
|
||||
use app_test_support::create_fake_rollout_with_source;
|
||||
use app_test_support::create_final_assistant_message_sse_response;
|
||||
@@ -1049,7 +1050,7 @@ async fn thread_list_filters_by_subagent_variant() -> Result<()> {
|
||||
|
||||
let parent_thread_id = ThreadId::from_string(&Uuid::new_v4().to_string())?;
|
||||
|
||||
let review_id = create_fake_rollout_with_source(
|
||||
let review_id = create_fake_parented_rollout_with_source(
|
||||
codex_home.path(),
|
||||
"2025-02-02T09-00-00",
|
||||
"2025-02-02T09:00:00Z",
|
||||
@@ -1057,6 +1058,7 @@ async fn thread_list_filters_by_subagent_variant() -> Result<()> {
|
||||
Some("mock_provider"),
|
||||
/*git_info*/ None,
|
||||
CoreSessionSource::SubAgent(SubAgentSource::Review),
|
||||
parent_thread_id,
|
||||
)?;
|
||||
let compact_id = create_fake_rollout_with_source(
|
||||
codex_home.path(),
|
||||
@@ -1109,6 +1111,10 @@ async fn thread_list_filters_by_subagent_variant() -> Result<()> {
|
||||
.map(|thread| thread.id.as_str())
|
||||
.collect();
|
||||
assert_eq!(review_ids, vec![review_id.as_str()]);
|
||||
assert_eq!(
|
||||
review.data[0].parent_thread_id,
|
||||
Some(parent_thread_id.to_string())
|
||||
);
|
||||
|
||||
let compact = list_threads(
|
||||
&mut mcp,
|
||||
|
||||
@@ -1358,6 +1358,7 @@ async fn seed_pathless_store_thread(
|
||||
.create_thread(CreateThreadParams {
|
||||
thread_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id: None,
|
||||
source: ProtocolSessionSource::Cli,
|
||||
thread_source: None,
|
||||
base_instructions: BaseInstructions::default(),
|
||||
|
||||
@@ -1791,6 +1791,7 @@ stream_max_retries = 0
|
||||
let session_meta = SessionMeta {
|
||||
id: conversation_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id: None,
|
||||
timestamp: "2025-01-05T12:00:00Z".to_string(),
|
||||
cwd: repo_path.clone(),
|
||||
originator: "codex".to_string(),
|
||||
|
||||
@@ -211,6 +211,7 @@ async fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> {
|
||||
.create_thread(CreateThreadParams {
|
||||
thread_id,
|
||||
forked_from_id: Some(parent_thread_id),
|
||||
parent_thread_id: None,
|
||||
source: SessionSource::Cli,
|
||||
thread_source: None,
|
||||
base_instructions: BaseInstructions::default(),
|
||||
|
||||
Reference in New Issue
Block a user