clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-06-01 19:02:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: include thread ID in MCP turn metadata (#21329)

Browse Source 
## Why

MCP tool calls already include `session_id` in `x-codex-turn-metadata`,
but descendant threads intentionally share that value with the root
thread. Consumers that need to correlate work at the concrete thread
level also need the current `thread_id`.

## What changed

- add `thread_id` to `x-codex-turn-metadata` while preserving
`session_id` as the shared session identity
- thread the two identities separately through normal turns and spawned
review threads
- add regression coverage for resumed sessions, reserved metadata
fields, and deferred MCP tool calls

## Verification

- added focused coverage in `core/src/session/tests.rs`,
`core/src/turn_metadata_tests.rs`, and `core/tests/suite/search_tool.rs`
This commit is contained in:
jif-oai
2026-05-06 11:36:15 +02:00
committed by GitHub
parent b5e965e1d7
commit fe24a180ab
7 changed files with 48 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
codex-rs/core/src/session/review.rs
View File

@@ -102,7 +102,8 @@ pub(super) async fn spawn_review_thread(
let per_turn_config = Arc::new(per_turn_config);
let review_turn_id = sub_id.to_string();
let turn_metadata_state = Arc::new(TurnMetadataState::new(
sess.conversation_id.to_string(),
sess.session_id().to_string(),
sess.thread_id().to_string(),
parent_turn_context.thread_source,
review_turn_id.clone(),
parent_turn_context.cwd.clone(),

10
codex-rs/core/src/session/session.rs
View File

@@ -325,6 +325,16 @@ pub(crate) struct AppServerClientMetadata {
}
impl Session {
/// Returns the concrete identity for this thread.
pub(crate) fn thread_id(&self) -> ThreadId {
self.conversation_id
}
/// Returns the identity shared by the root thread and all descendant threads.
pub(crate) fn session_id(&self) -> SessionId {
self.services.agent_control.session_id()
}
#[instrument(name = "session_init", level = "info", skip_all)]
#[allow(clippy::too_many_arguments)]
#[expect(

14
codex-rs/core/src/session/tests.rs
View File

@@ -3777,6 +3777,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
let turn_environments = turn_environments_for_tests(&environment, &session_configuration.cwd);
let turn_context = Session::make_turn_context(
thread_id,
SessionId::from(thread_id),
Some(Arc::clone(&auth_manager)),
&session_telemetry,
session_configuration.provider.clone(),
@@ -4057,10 +4058,8 @@ async fn resumed_root_session_uses_thread_id_as_session_id() {
.await
.expect("resume should succeed");
assert_eq!(
session.services.agent_control.session_id(),
SessionId::from(thread_id)
);
assert_eq!(session.thread_id(), thread_id);
assert_eq!(session.session_id(), SessionId::from(thread_id));
let event = rx_event.recv().await.expect("session configured event");
let EventMsg::SessionConfigured(event) = event.msg else {
@@ -4094,10 +4093,8 @@ async fn resumed_subagent_session_keeps_inherited_session_id() {
.await
.expect("resume should succeed");
assert_eq!(
session.services.agent_control.session_id(),
parent_session_id
);
assert_eq!(session.thread_id(), thread_id);
assert_eq!(session.session_id(), parent_session_id);
let event = rx_event.recv().await.expect("session configured event");
let EventMsg::SessionConfigured(event) = event.msg else {
@@ -5463,6 +5460,7 @@ where
let turn_environments = turn_environments_for_tests(&environment, &session_configuration.cwd);
let turn_context = Arc::new(Session::make_turn_context(
thread_id,
SessionId::from(thread_id),
Some(Arc::clone(&auth_manager)),
&session_telemetry,
session_configuration.provider.clone(),

10
codex-rs/core/src/session/turn_context.rs
View File

@@ -4,6 +4,7 @@ use crate::config::GhostSnapshotConfig;
use crate::environment_selection::ResolvedTurnEnvironments;
use codex_model_provider::SharedModelProvider;
use codex_model_provider::create_model_provider;
use codex_protocol::SessionId;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::protocol::ThreadSource;
use codex_protocol::protocol::TurnEnvironmentSelection;
@@ -440,7 +441,8 @@ impl Session {
#[allow(clippy::too_many_arguments)]
pub(crate) fn make_turn_context(
conversation_id: ThreadId,
thread_id: ThreadId,
session_id: SessionId,
auth_manager: Option<Arc<AuthManager>>,
session_telemetry: &SessionTelemetry,
provider: ModelProviderInfo,
@@ -522,7 +524,8 @@ impl Session {
let per_turn_config = Arc::new(per_turn_config);
let turn_metadata_state = Arc::new(TurnMetadataState::new(
conversation_id.to_string(),
session_id.to_string(),
thread_id.to_string(),
session_configuration.thread_source,
sub_id.clone(),
cwd.clone(),
@@ -718,7 +721,8 @@ impl Session {
);
let goal_tools_supported = !per_turn_config.ephemeral && self.state_db().is_some();
let mut turn_context: TurnContext = Self::make_turn_context(
self.conversation_id,
self.thread_id(),
self.session_id(),
Some(Arc::clone(&self.services.auth_manager)),
&self.services.session_telemetry,
session_configuration.provider.clone(),

9
codex-rs/core/src/turn_metadata.rs
View File

@@ -69,6 +69,8 @@ pub(crate) struct TurnMetadataBag {
#[serde(default, skip_serializing_if = "Option::is_none")]
session_id: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
thread_id: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
thread_source: Option<ThreadSource>,
#[serde(default, skip_serializing_if = "Option::is_none")]
turn_id: Option<String>,
@@ -115,6 +117,7 @@ fn merge_turn_metadata(
fn build_turn_metadata_bag(
session_id: Option<String>,
thread_id: Option<String>,
thread_source: Option<ThreadSource>,
turn_id: Option<String>,
sandbox: Option<String>,
@@ -130,6 +133,7 @@ fn build_turn_metadata_bag(
TurnMetadataBag {
session_id,
thread_id,
thread_source,
turn_id,
workspaces,
@@ -159,6 +163,7 @@ pub async fn build_turn_metadata_header(
build_turn_metadata_bag(
/*session_id*/ None,
/*thread_id*/ None,
/*thread_source*/ None,
/*turn_id*/ None,
sandbox.map(ToString::to_string),
@@ -185,8 +190,10 @@ pub(crate) struct TurnMetadataState {
}
impl TurnMetadataState {
#[allow(clippy::too_many_arguments)]
pub(crate) fn new(
session_id: String,
thread_id: String,
thread_source: Option<ThreadSource>,
turn_id: String,
cwd: AbsolutePathBuf,
@@ -205,6 +212,7 @@ impl TurnMetadataState {
);
let base_metadata = build_turn_metadata_bag(
Some(session_id),
Some(thread_id),
thread_source,
Some(turn_id),
sandbox,
@@ -320,6 +328,7 @@ impl TurnMetadataState {
let enriched_metadata = build_turn_metadata_bag(
state.base_metadata.session_id.clone(),
state.base_metadata.thread_id.clone(),
state.base_metadata.thread_source,
state.base_metadata.turn_id.clone(),
state.base_metadata.sandbox.clone(),

10
codex-rs/core/src/turn_metadata_tests.rs
View File

@@ -94,6 +94,7 @@ fn turn_metadata_state_uses_platform_sandbox_tag() {
let state = TurnMetadataState::new(
"session-a".to_string(),
"thread-a".to_string(),
Some(ThreadSource::User),
"turn-a".to_string(),
cwd,
@@ -106,11 +107,13 @@ fn turn_metadata_state_uses_platform_sandbox_tag() {
let json: Value = serde_json::from_str(&header).expect("json");
let sandbox_name = json.get("sandbox").and_then(Value::as_str);
let session_id = json.get("session_id").and_then(Value::as_str);
let thread_id = json.get("thread_id").and_then(Value::as_str);
let thread_source = json.get("thread_source").and_then(Value::as_str);
let expected_sandbox = sandbox_tag(&sandbox_policy, WindowsSandboxLevel::Disabled);
assert_eq!(sandbox_name, Some(expected_sandbox));
assert_eq!(session_id, Some("session-a"));
assert_eq!(thread_id, Some("thread-a"));
assert_eq!(thread_source, Some("user"));
assert!(json.get("session_source").is_none());
}
@@ -122,6 +125,7 @@ fn turn_metadata_state_uses_explicit_subagent_thread_source() {
let permission_profile = PermissionProfile::read_only();
let state = TurnMetadataState::new(
"session-a".to_string(),
"thread-a".to_string(),
Some(ThreadSource::Subagent),
"turn-a".to_string(),
cwd,
@@ -145,6 +149,7 @@ fn turn_metadata_state_includes_turn_started_at_unix_ms_after_start() {
let state = TurnMetadataState::new(
"session-a".to_string(),
"thread-a".to_string(),
Some(ThreadSource::User),
"turn-a".to_string(),
cwd,
@@ -171,6 +176,7 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta(
let state = TurnMetadataState::new(
"session-a".to_string(),
"thread-a".to_string(),
/*thread_source*/ None,
"turn-a".to_string(),
cwd,
@@ -215,6 +221,7 @@ fn turn_metadata_state_ignores_client_turn_started_at_unix_ms_before_start() {
let state = TurnMetadataState::new(
"session-a".to_string(),
"thread-a".to_string(),
Some(ThreadSource::User),
"turn-a".to_string(),
cwd,
@@ -241,6 +248,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
let state = TurnMetadataState::new(
"session-a".to_string(),
"thread-a".to_string(),
Some(ThreadSource::User),
"turn-a".to_string(),
cwd,
@@ -257,6 +265,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
"client-supplied".to_string(),
),
("session_id".to_string(), "client-supplied".to_string()),
("thread_id".to_string(), "client-supplied".to_string()),
("thread_source".to_string(), "client-supplied".to_string()),
(
"turn_started_at_unix_ms".to_string(),
@@ -275,6 +284,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
assert_eq!(json["model"].as_str(), Some("client-supplied"));
assert_eq!(json["reasoning_effort"].as_str(), Some("client-supplied"));
assert_eq!(json["session_id"].as_str(), Some("session-a"));
assert_eq!(json["thread_id"].as_str(), Some("thread-a"));
assert_eq!(json["thread_source"].as_str(), Some("user"));
assert_eq!(json["turn_id"].as_str(), Some("turn-a"));
assert_eq!(