mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
fix(core): restore thread_source in x-codex-turn-metadata (#29455)
## Description Restore `thread_source` in `x-codex-turn-metadata`. Inadvertently removed `thread_source` from `x-codex-turn-metadata` in https://github.com/openai/codex/pull/27122 - didn't realize it was a top-level thread app-server API field, not passed in `responsesapi_client_metadata`. This also reserves the key so `responsesapi_client_metadata` cannot override it.
This commit is contained in:
@@ -63,7 +63,10 @@ async fn turn_start_forwards_client_metadata_to_responses_request_v2() -> Result
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams::default())
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
thread_source: Some(ThreadSource::Feature("automation".to_string())),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let thread_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
@@ -75,7 +78,6 @@ async fn turn_start_forwards_client_metadata_to_responses_request_v2() -> Result
|
||||
let client_metadata = HashMap::from([
|
||||
("fiber_run_id".to_string(), "fiber-start-123".to_string()),
|
||||
("origin".to_string(), "gaas".to_string()),
|
||||
("thread_source".to_string(), "client-supplied".to_string()),
|
||||
]);
|
||||
let turn_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
@@ -110,7 +112,7 @@ async fn turn_start_forwards_client_metadata_to_responses_request_v2() -> Result
|
||||
.expect("x-codex-turn-metadata header should be present");
|
||||
assert_eq!(metadata["fiber_run_id"].as_str(), Some("fiber-start-123"));
|
||||
assert_eq!(metadata["origin"].as_str(), Some("gaas"));
|
||||
assert_eq!(metadata["thread_source"].as_str(), Some("client-supplied"));
|
||||
assert_eq!(metadata["thread_source"].as_str(), Some("automation"));
|
||||
assert_eq!(metadata["turn_id"].as_str(), Some(turn.id.as_str()));
|
||||
assert!(metadata.get("installation_id").is_some());
|
||||
assert!(metadata.get("session_id").is_some());
|
||||
@@ -561,7 +563,10 @@ async fn turn_start_forwards_client_metadata_to_responses_websocket_request_body
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams::default())
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
thread_source: Some(ThreadSource::Feature("automation".to_string())),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let thread_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
@@ -619,6 +624,7 @@ async fn turn_start_forwards_client_metadata_to_responses_websocket_request_body
|
||||
.expect("websocket x-codex-turn-metadata client metadata should be present");
|
||||
assert_eq!(metadata["fiber_run_id"].as_str(), Some("fiber-start-123"));
|
||||
assert_eq!(metadata["origin"].as_str(), Some("gaas"));
|
||||
assert_eq!(metadata["thread_source"].as_str(), Some("automation"));
|
||||
assert_eq!(metadata["turn_id"].as_str(), Some(turn.id.as_str()));
|
||||
assert!(metadata.get("session_id").is_some());
|
||||
assert_eq!(
|
||||
|
||||
@@ -10,6 +10,7 @@ use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::InternalSessionSource;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
use codex_utils_string::to_ascii_json_string;
|
||||
use http::HeaderMap as ApiHeaderMap;
|
||||
use http::HeaderValue;
|
||||
@@ -34,6 +35,7 @@ pub(crate) const TURN_STARTED_AT_UNIX_MS_KEY: &str = "turn_started_at_unix_ms";
|
||||
pub(crate) const FORKED_FROM_THREAD_ID_KEY: &str = "forked_from_thread_id";
|
||||
pub(crate) const PARENT_THREAD_ID_KEY: &str = "parent_thread_id";
|
||||
pub(crate) const SUBAGENT_KIND_KEY: &str = "subagent_kind";
|
||||
pub(crate) const THREAD_SOURCE_KEY: &str = "thread_source";
|
||||
pub(crate) const SANDBOX_KEY: &str = "sandbox";
|
||||
pub(crate) const WORKSPACES_KEY: &str = "workspaces";
|
||||
|
||||
@@ -56,6 +58,7 @@ const RESERVED_METADATA_KEYS: &[&str] = &[
|
||||
FORKED_FROM_THREAD_ID_KEY,
|
||||
PARENT_THREAD_ID_KEY,
|
||||
SUBAGENT_KIND_KEY,
|
||||
THREAD_SOURCE_KEY,
|
||||
SANDBOX_KEY,
|
||||
WORKSPACES_KEY,
|
||||
];
|
||||
@@ -143,6 +146,7 @@ pub struct CodexResponsesMetadata {
|
||||
pub(crate) parent_thread_id: Option<ThreadId>,
|
||||
pub(crate) subagent_header: Option<String>,
|
||||
pub(crate) subagent_kind: Option<String>,
|
||||
pub(crate) thread_source: Option<ThreadSource>,
|
||||
pub(crate) sandbox: Option<String>,
|
||||
pub(crate) workspaces: BTreeMap<String, TurnMetadataWorkspace>,
|
||||
pub(crate) turn_started_at_unix_ms: Option<i64>,
|
||||
@@ -167,6 +171,7 @@ impl CodexResponsesMetadata {
|
||||
parent_thread_id: None,
|
||||
subagent_header: None,
|
||||
subagent_kind: None,
|
||||
thread_source: None,
|
||||
sandbox: None,
|
||||
workspaces: BTreeMap::new(),
|
||||
turn_started_at_unix_ms: None,
|
||||
@@ -268,6 +273,7 @@ impl CodexResponsesMetadata {
|
||||
forked_from_thread_id: self.forked_from_thread_id,
|
||||
parent_thread_id: self.parent_thread_id,
|
||||
subagent_kind: self.subagent_kind.as_deref(),
|
||||
thread_source: self.thread_source.as_ref(),
|
||||
sandbox: self.sandbox.as_deref(),
|
||||
workspaces: non_empty_workspaces(&self.workspaces),
|
||||
turn_started_at_unix_ms: self.turn_started_at_unix_ms,
|
||||
@@ -354,6 +360,8 @@ struct CodexTurnMetadataPayload<'a> {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
subagent_kind: Option<&'a str>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
thread_source: Option<&'a ThreadSource>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
sandbox: Option<&'a str>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
workspaces: Option<&'a BTreeMap<String, TurnMetadataWorkspace>>,
|
||||
|
||||
@@ -75,9 +75,12 @@ pub(super) async fn spawn_review_thread(
|
||||
.model_reasoning_summary
|
||||
.unwrap_or(model_info.default_reasoning_summary);
|
||||
let session_source = parent_turn_context.session_source.clone();
|
||||
let forked_from_thread_id = {
|
||||
let (forked_from_thread_id, thread_source) = {
|
||||
let state = sess.state.lock().await;
|
||||
state.session_configuration.forked_from_thread_id
|
||||
(
|
||||
state.session_configuration.forked_from_thread_id,
|
||||
state.session_configuration.thread_source.clone(),
|
||||
)
|
||||
};
|
||||
|
||||
let per_turn_config = Arc::new(per_turn_config);
|
||||
@@ -88,6 +91,7 @@ pub(super) async fn spawn_review_thread(
|
||||
forked_from_thread_id,
|
||||
parent_turn_context.parent_thread_id,
|
||||
&session_source,
|
||||
thread_source,
|
||||
review_turn_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
parent_turn_context.cwd.clone(),
|
||||
|
||||
@@ -536,6 +536,7 @@ impl Session {
|
||||
session_configuration.forked_from_thread_id,
|
||||
session_configuration.parent_thread_id,
|
||||
&session_configuration.session_source,
|
||||
session_configuration.thread_source.clone(),
|
||||
sub_id.clone(),
|
||||
cwd.clone(),
|
||||
&session_configuration.permission_profile(),
|
||||
|
||||
@@ -25,6 +25,7 @@ use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
const MODEL_KEY: &str = "model";
|
||||
@@ -91,6 +92,7 @@ pub(crate) struct TurnMetadataState {
|
||||
parent_thread_id: Option<ThreadId>,
|
||||
subagent_header: Option<String>,
|
||||
subagent_kind: Option<String>,
|
||||
thread_source: Option<ThreadSource>,
|
||||
turn_id: String,
|
||||
sandbox: Option<String>,
|
||||
enriched_workspaces: Arc<RwLock<Option<BTreeMap<String, TurnMetadataWorkspace>>>>,
|
||||
@@ -108,6 +110,7 @@ impl TurnMetadataState {
|
||||
forked_from_thread_id: Option<ThreadId>,
|
||||
parent_thread_id: Option<ThreadId>,
|
||||
session_source: &SessionSource,
|
||||
thread_source: Option<ThreadSource>,
|
||||
turn_id: String,
|
||||
cwd: AbsolutePathBuf,
|
||||
permission_profile: &PermissionProfile,
|
||||
@@ -132,6 +135,7 @@ impl TurnMetadataState {
|
||||
parent_thread_id,
|
||||
subagent_header: subagent_header_value(session_source),
|
||||
subagent_kind: subagent_metadata_kind(session_source),
|
||||
thread_source,
|
||||
turn_id,
|
||||
sandbox,
|
||||
enriched_workspaces: Arc::new(RwLock::new(None)),
|
||||
@@ -225,6 +229,7 @@ impl TurnMetadataState {
|
||||
parent_thread_id: self.parent_thread_id,
|
||||
subagent_header: self.subagent_header.clone(),
|
||||
subagent_kind: self.subagent_kind.clone(),
|
||||
thread_source: self.thread_source.clone(),
|
||||
sandbox: self.sandbox.clone(),
|
||||
workspaces: self.current_workspaces(),
|
||||
turn_started_at_unix_ms: self.current_turn_started_at_unix_ms(),
|
||||
|
||||
@@ -13,6 +13,7 @@ use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::PathExt;
|
||||
@@ -187,6 +188,7 @@ fn turn_metadata_state_uses_platform_sandbox_tag() {
|
||||
/*forked_from_thread_id*/ None,
|
||||
/*parent_thread_id*/ None,
|
||||
&SessionSource::Exec,
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -229,6 +231,7 @@ fn turn_metadata_state_includes_root_fork_lineage() {
|
||||
Some(source_thread_id),
|
||||
/*parent_thread_id*/ None,
|
||||
&SessionSource::Exec,
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -267,6 +270,7 @@ fn turn_metadata_state_includes_thread_spawn_subagent_parent_without_fork() {
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
}),
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -305,6 +309,7 @@ fn turn_metadata_state_includes_forked_thread_spawn_subagent_lineage() {
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
}),
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -349,6 +354,7 @@ fn turn_metadata_state_includes_known_parent_for_non_thread_spawn_subagents_with
|
||||
/*forked_from_thread_id*/ None,
|
||||
Some(parent_thread_id),
|
||||
&SessionSource::SubAgent(subagent_source),
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd.clone(),
|
||||
&permission_profile,
|
||||
@@ -380,6 +386,7 @@ fn turn_metadata_state_includes_turn_started_at_unix_ms_after_start() {
|
||||
/*forked_from_thread_id*/ None,
|
||||
/*parent_thread_id*/ None,
|
||||
&SessionSource::Exec,
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -409,6 +416,7 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta(
|
||||
/*forked_from_thread_id*/ None,
|
||||
/*parent_thread_id*/ None,
|
||||
&SessionSource::Exec,
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -457,6 +465,7 @@ fn turn_metadata_state_marks_user_input_requested_during_turn_only_for_mcp_reque
|
||||
/*forked_from_thread_id*/ None,
|
||||
/*parent_thread_id*/ None,
|
||||
&SessionSource::Exec,
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -509,6 +518,7 @@ fn turn_metadata_state_ignores_client_reserved_metadata_before_start() {
|
||||
/*forked_from_thread_id*/ None,
|
||||
/*parent_thread_id*/ None,
|
||||
&SessionSource::Exec,
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -562,6 +572,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
}),
|
||||
Some(ThreadSource::Feature("automation".to_string())),
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -637,7 +648,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
|
||||
Some("55555555-5555-4555-8555-555555555555")
|
||||
);
|
||||
assert_eq!(json["subagent_kind"].as_str(), Some("thread_spawn"));
|
||||
assert_eq!(json["thread_source"].as_str(), Some("client-supplied"));
|
||||
assert_eq!(json["thread_source"].as_str(), Some("automation"));
|
||||
assert_eq!(json["turn_id"].as_str(), Some("turn-a"));
|
||||
assert!(json.get("request_kind").is_none());
|
||||
assert!(json.get(WINDOW_ID_KEY).is_none());
|
||||
@@ -650,6 +661,10 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
|
||||
let model_request_json: Value =
|
||||
serde_json::from_str(&model_request_header).expect("model request json");
|
||||
assert_eq!(model_request_json["request_kind"].as_str(), Some("turn"));
|
||||
assert_eq!(
|
||||
model_request_json["thread_source"].as_str(),
|
||||
Some("automation")
|
||||
);
|
||||
assert_eq!(
|
||||
model_request_json[INSTALLATION_ID_KEY].as_str(),
|
||||
Some("installation-a")
|
||||
@@ -679,6 +694,7 @@ fn turn_metadata_state_overlays_compaction_only_on_compaction_requests() {
|
||||
/*forked_from_thread_id*/ None,
|
||||
/*parent_thread_id*/ None,
|
||||
&SessionSource::Exec,
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&permission_profile,
|
||||
@@ -741,6 +757,7 @@ async fn turn_metadata_state_preserves_lineage_after_git_enrichment() {
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
}),
|
||||
/*thread_source*/ None,
|
||||
"turn-a".to_string(),
|
||||
repo_path,
|
||||
&permission_profile,
|
||||
|
||||
Reference in New Issue
Block a user