mirror of
https://github.com/openai/codex.git
synced 2026-11-04 00:24:43 +00:00
Forward app-server turn clientMetadata to Responses (#16009)
## Summary
App-server v2 already receives turn-scoped `clientMetadata`, but the
Rust app-server was dropping it before the outbound Responses request.
This change keeps the fix lightweight by threading that metadata through
the existing turn-metadata path rather than inventing a new transport.
## What we're trying to do and why
We want turn-scoped metadata from the app-server protocol layer,
especially fields like Hermes/GAAS run IDs, to survive all the way to
the actual Responses API request so it is visible in downstream
websocket request logging and analytics.
The specific bug was:
- app-server protocol uses camelCase `clientMetadata`
- Responses transport already has an existing turn metadata carrier:
`x-codex-turn-metadata`
- websocket transport already rewrites that header into
`request.request_body.client_metadata["x-codex-turn-metadata"]`
- but the Rust app-server never parsed or stored `clientMetadata`, so
nothing from the app-server request was making it into that existing
path
This PR fixes that without adding a new header or a second metadata
channel.
## How we did it
### Protocol surface
- Add optional `clientMetadata` to v2 `TurnStartParams` and
`TurnSteerParams`
- Regenerate the JSON schema / TypeScript fixtures
- Update app-server docs to describe the field and its behavior
### Runtime plumbing
- Add a dedicated core op for app-server user input carrying turn-scoped
metadata: `Op::UserInputWithClientMetadata`
- Wire `turn/start` and `turn/steer` through that op / signature path
instead of dropping the metadata at the message-processor boundary
- Store the metadata in `TurnMetadataState`
### Transport behavior
- Reuse the existing serialized `x-codex-turn-metadata` payload
- Merge the new app-server `clientMetadata` into that JSON additively
- Do **not** replace built-in reserved fields already present in the
turn metadata payload
- Keep websocket behavior unchanged at the outer shape level: it still
sends only `client_metadata["x-codex-turn-metadata"]`, but that JSON
string now contains the merged fields
- Keep HTTP fallback behavior unchanged except that the existing
`x-codex-turn-metadata` header now includes the merged fields too
### Request shape before / after
Before, a websocket `response.create` looked like:
```json
{
"type": "response.create",
"client_metadata": {
"x-codex-turn-metadata": "{\"session_id\":\"...\",\"turn_id\":\"...\"}"
}
}
```
Even if the app-server caller supplied `clientMetadata`, it was not
represented there.
After, the same request shape is preserved, but the serialized payload
now includes the new turn-scoped fields:
```json
{
"type": "response.create",
"client_metadata": {
"x-codex-turn-metadata": "{\"session_id\":\"...\",\"turn_id\":\"...\",\"fiber_run_id\":\"fiber-start-123\",\"origin\":\"gaas\"}"
}
}
```
## Validation
### Targeted tests added / updated
- protocol round-trip coverage for `clientMetadata` on `turn/start` and
`turn/steer`
- protocol round-trip coverage for `Op::UserInputWithClientMetadata`
- `TurnMetadataState` merge test proving client metadata is added
without overwriting reserved built-in fields
- websocket request-shape test proving outbound `response.create`
contains merged metadata inside
`client_metadata["x-codex-turn-metadata"]`
- app-server integration tests proving:
- `turn/start` forwards `clientMetadata` into the outbound Responses
request path
- websocket warmup + real turn request both behave correctly
- `turn/steer` updates the follow-up request metadata
### Commands run
- `just write-app-server-schema`
- `cargo test -p codex-app-server-protocol`
- `cargo test -p codex-protocol`
- `cargo test -p codex-core
turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields
--lib`
- `cargo test -p codex-core --test all
responses_websocket_preserves_custom_turn_metadata_fields`
- `cargo test -p codex-app-server --test all client_metadata`
- `cargo test -p codex-app-server --test all
turn_start_forwards_client_metadata_to_responses_websocket_request_body_v2
-- --nocapture`
- `just fmt`
- `just fix -p codex-core -p codex-protocol -p codex-app-server-protocol
-p codex-app-server`
- `just fix -p codex-exec -p codex-tui-app-server`
- `just argument-comment-lint`
### Full suite note
`cargo test` in `codex-rs` still fails in:
-
`suite::v2::turn_interrupt::turn_interrupt_resolves_pending_command_approval_request`
I verified that same failure on a clean detached `HEAD` worktree with an
isolated `CARGO_TARGET_DIR`, so it is not caused by this patch.
This commit is contained in:
1 parent
244b15c95d
commit
a92a5085bd
51 files changed
+867
-45
No files matched your search
@@ -425,6 +425,7 @@ async fn send_input_submits_user_message() {
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
},
|
||||
);
|
||||
let captured = harness
|
||||
@@ -571,6 +572,7 @@ async fn spawn_agent_creates_thread_and_sends_prompt() {
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
},
|
||||
);
|
||||
let captured = harness
|
||||
@@ -678,6 +680,7 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
},
|
||||
);
|
||||
let captured = harness
|
||||
|
||||
@@ -763,8 +763,11 @@ impl Codex {
|
||||
&self,
|
||||
input: Vec<UserInput>,
|
||||
expected_turn_id: Option<&str>,
|
||||
responsesapi_client_metadata: Option<HashMap<String, String>>,
|
||||
) -> Result<String, SteerInputError> {
|
||||
self.session.steer_input(input, expected_turn_id).await
|
||||
self.session
|
||||
.steer_input(input, expected_turn_id, responsesapi_client_metadata)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn set_app_server_client_info(
|
||||
@@ -2264,6 +2267,7 @@ impl Session {
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
@@ -4136,6 +4140,7 @@ impl Session {
|
||||
&self,
|
||||
input: Vec<UserInput>,
|
||||
expected_turn_id: Option<&str>,
|
||||
responsesapi_client_metadata: Option<HashMap<String, String>>,
|
||||
) -> Result<String, SteerInputError> {
|
||||
if input.is_empty() {
|
||||
return Err(SteerInputError::EmptyInput);
|
||||
@@ -4174,6 +4179,15 @@ impl Session {
|
||||
None => return Err(SteerInputError::NoActiveTurn(input)),
|
||||
}
|
||||
|
||||
if let Some(responsesapi_client_metadata) = responsesapi_client_metadata
|
||||
&& let Some((_, active_task)) = active_turn.tasks.first()
|
||||
{
|
||||
active_task
|
||||
.turn_context
|
||||
.turn_metadata_state
|
||||
.set_responsesapi_client_metadata(responsesapi_client_metadata);
|
||||
}
|
||||
|
||||
let mut turn_state = active_turn.turn_state.lock().await;
|
||||
turn_state.push_pending_input(input.into());
|
||||
turn_state.accept_mailbox_delivery_for_current_turn();
|
||||
@@ -4942,7 +4956,7 @@ mod handlers {
|
||||
}
|
||||
|
||||
pub async fn user_input_or_turn(sess: &Arc<Session>, sub_id: String, op: Op) {
|
||||
let (items, updates) = match op {
|
||||
let (items, updates, responsesapi_client_metadata) = match op {
|
||||
Op::UserTurn {
|
||||
cwd,
|
||||
approval_policy,
|
||||
@@ -4983,17 +4997,20 @@ mod handlers {
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
},
|
||||
None,
|
||||
)
|
||||
}
|
||||
Op::UserInput {
|
||||
items,
|
||||
final_output_json_schema,
|
||||
responsesapi_client_metadata,
|
||||
} => (
|
||||
items,
|
||||
SessionSettingsUpdate {
|
||||
final_output_json_schema: Some(final_output_json_schema),
|
||||
..Default::default()
|
||||
},
|
||||
responsesapi_client_metadata,
|
||||
),
|
||||
_ => unreachable!(),
|
||||
};
|
||||
@@ -5005,11 +5022,20 @@ mod handlers {
|
||||
sess.maybe_emit_unknown_model_warning_for_turn(current_context.as_ref())
|
||||
.await;
|
||||
match sess
|
||||
.steer_input(items.clone(), /*expected_turn_id*/ None)
|
||||
.steer_input(
|
||||
items.clone(),
|
||||
/*expected_turn_id*/ None,
|
||||
responsesapi_client_metadata.clone(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(_) => current_context.session_telemetry.user_prompt(&items),
|
||||
Err(SteerInputError::NoActiveTurn(items)) => {
|
||||
if let Some(responsesapi_client_metadata) = responsesapi_client_metadata {
|
||||
current_context
|
||||
.turn_metadata_state
|
||||
.set_responsesapi_client_metadata(responsesapi_client_metadata);
|
||||
}
|
||||
current_context.session_telemetry.user_prompt(&items);
|
||||
sess.refresh_mcp_servers_if_requested(¤t_context)
|
||||
.await;
|
||||
|
||||
@@ -185,6 +185,7 @@ pub(crate) async fn run_codex_thread_one_shot(
|
||||
io.submit(Op::UserInput {
|
||||
items: input,
|
||||
final_output_json_schema,
|
||||
responsesapi_client_metadata: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -1331,6 +1331,7 @@ async fn fork_startup_context_then_first_turn_diff_snapshot() -> anyhow::Result<
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&initial.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
@@ -1386,6 +1387,7 @@ async fn fork_startup_context_then_first_turn_diff_snapshot() -> anyhow::Result<
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&forked.thread, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
@@ -3279,6 +3281,7 @@ fn op_kind_distinguishes_turn_ops() {
|
||||
Op::UserInput {
|
||||
items: vec![],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
}
|
||||
.kind(),
|
||||
"user_input"
|
||||
@@ -4857,7 +4860,9 @@ async fn steer_input_requires_active_turn() {
|
||||
}];
|
||||
|
||||
let err = sess
|
||||
.steer_input(input, /*expected_turn_id*/ None)
|
||||
.steer_input(
|
||||
input, /*expected_turn_id*/ None, /*responsesapi_client_metadata*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("steering without active turn should fail");
|
||||
|
||||
@@ -4886,7 +4891,11 @@ async fn steer_input_enforces_expected_turn_id() {
|
||||
text_elements: Vec::new(),
|
||||
}];
|
||||
let err = sess
|
||||
.steer_input(steer_input, Some("different-turn-id"))
|
||||
.steer_input(
|
||||
steer_input,
|
||||
Some("different-turn-id"),
|
||||
/*responsesapi_client_metadata*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("mismatched expected turn id should fail");
|
||||
|
||||
@@ -4928,7 +4937,11 @@ async fn steer_input_rejects_non_regular_turns() {
|
||||
text_elements: Vec::new(),
|
||||
}];
|
||||
let err = sess
|
||||
.steer_input(steer_input, /*expected_turn_id*/ None)
|
||||
.steer_input(
|
||||
steer_input,
|
||||
/*expected_turn_id*/ None,
|
||||
/*responsesapi_client_metadata*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("steering a non-regular turn should fail");
|
||||
|
||||
@@ -4960,7 +4973,11 @@ async fn steer_input_returns_active_turn_id() {
|
||||
text_elements: Vec::new(),
|
||||
}];
|
||||
let turn_id = sess
|
||||
.steer_input(steer_input, Some(&tc.sub_id))
|
||||
.steer_input(
|
||||
steer_input,
|
||||
Some(&tc.sub_id),
|
||||
/*responsesapi_client_metadata*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("steering with matching expected turn id should succeed");
|
||||
|
||||
@@ -5147,6 +5164,7 @@ async fn steered_input_reopens_mailbox_delivery_for_current_turn() {
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
Some(&tc.sub_id),
|
||||
/*responsesapi_client_metadata*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("steered input should be accepted");
|
||||
@@ -5191,6 +5209,7 @@ async fn stale_defer_mailbox_delivery_does_not_override_steered_input() {
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
Some(&tc.sub_id),
|
||||
/*responsesapi_client_metadata*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("steered input should be accepted");
|
||||
|
||||
@@ -23,6 +23,7 @@ use codex_protocol::protocol::TokenUsage;
|
||||
use codex_protocol::protocol::W3cTraceContext;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use rmcp::model::ReadResourceRequestParams;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::watch;
|
||||
@@ -97,8 +98,11 @@ impl CodexThread {
|
||||
&self,
|
||||
input: Vec<UserInput>,
|
||||
expected_turn_id: Option<&str>,
|
||||
responsesapi_client_metadata: Option<HashMap<String, String>>,
|
||||
) -> Result<String, SteerInputError> {
|
||||
self.codex.steer_input(input, expected_turn_id).await
|
||||
self.codex
|
||||
.steer_input(input, expected_turn_id, responsesapi_client_metadata)
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn set_app_server_client_info(
|
||||
|
||||
@@ -2004,6 +2004,7 @@ async fn send_input_accepts_structured_items() {
|
||||
},
|
||||
],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
};
|
||||
let captured = manager
|
||||
.captured_ops()
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
@@ -6,6 +7,7 @@ use std::sync::Mutex;
|
||||
use std::sync::RwLock;
|
||||
|
||||
use serde::Serialize;
|
||||
use serde_json::Value;
|
||||
use tokio::task::JoinHandle;
|
||||
|
||||
use crate::sandbox_tags::sandbox_tag;
|
||||
@@ -69,6 +71,20 @@ impl TurnMetadataBag {
|
||||
}
|
||||
}
|
||||
|
||||
fn merge_responsesapi_client_metadata(
|
||||
header: &str,
|
||||
responsesapi_client_metadata: Option<&HashMap<String, String>>,
|
||||
) -> Option<String> {
|
||||
let responsesapi_client_metadata = responsesapi_client_metadata?;
|
||||
let mut metadata = serde_json::from_str::<serde_json::Map<String, Value>>(header).ok()?;
|
||||
for (key, value) in responsesapi_client_metadata {
|
||||
metadata
|
||||
.entry(key.clone())
|
||||
.or_insert_with(|| Value::String(value.clone()));
|
||||
}
|
||||
serde_json::to_string(&metadata).ok()
|
||||
}
|
||||
|
||||
fn build_turn_metadata_bag(
|
||||
session_id: Option<String>,
|
||||
turn_id: Option<String>,
|
||||
@@ -129,6 +145,7 @@ pub(crate) struct TurnMetadataState {
|
||||
base_metadata: TurnMetadataBag,
|
||||
base_header: String,
|
||||
enriched_header: Arc<RwLock<Option<String>>>,
|
||||
responsesapi_client_metadata: Arc<RwLock<Option<HashMap<String, String>>>>,
|
||||
enrichment_task: Arc<Mutex<Option<JoinHandle<()>>>>,
|
||||
}
|
||||
|
||||
@@ -159,21 +176,30 @@ impl TurnMetadataState {
|
||||
base_metadata,
|
||||
base_header,
|
||||
enriched_header: Arc::new(RwLock::new(None)),
|
||||
responsesapi_client_metadata: Arc::new(RwLock::new(None)),
|
||||
enrichment_task: Arc::new(Mutex::new(None)),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn current_header_value(&self) -> Option<String> {
|
||||
if let Some(header) = self
|
||||
let header = if let Some(header) = self
|
||||
.enriched_header
|
||||
.read()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.as_ref()
|
||||
.cloned()
|
||||
{
|
||||
return Some(header);
|
||||
}
|
||||
Some(self.base_header.clone())
|
||||
header
|
||||
} else {
|
||||
self.base_header.clone()
|
||||
};
|
||||
let responsesapi_client_metadata = self
|
||||
.responsesapi_client_metadata
|
||||
.read()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone();
|
||||
merge_responsesapi_client_metadata(&header, responsesapi_client_metadata.as_ref())
|
||||
.or(Some(header))
|
||||
}
|
||||
|
||||
pub(crate) fn current_meta_value(&self) -> Option<serde_json::Value> {
|
||||
@@ -181,6 +207,17 @@ impl TurnMetadataState {
|
||||
.and_then(|header| serde_json::from_str(&header).ok())
|
||||
}
|
||||
|
||||
pub(crate) fn set_responsesapi_client_metadata(
|
||||
&self,
|
||||
responsesapi_client_metadata: HashMap<String, String>,
|
||||
) {
|
||||
*self
|
||||
.responsesapi_client_metadata
|
||||
.write()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner) =
|
||||
Some(responsesapi_client_metadata);
|
||||
}
|
||||
|
||||
pub(crate) fn spawn_git_enrichment_task(&self) {
|
||||
if self.repo_root.is_none() {
|
||||
return;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use super::*;
|
||||
|
||||
use serde_json::Value;
|
||||
use std::collections::HashMap;
|
||||
use tempfile::TempDir;
|
||||
use tokio::process::Command;
|
||||
|
||||
@@ -83,3 +84,29 @@ fn turn_metadata_state_uses_platform_sandbox_tag() {
|
||||
assert_eq!(sandbox_name, Some(expected_sandbox));
|
||||
assert_eq!(session_id, Some("session-a"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
let cwd = temp_dir.path().to_path_buf();
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
|
||||
let state = TurnMetadataState::new(
|
||||
"session-a".to_string(),
|
||||
"turn-a".to_string(),
|
||||
cwd,
|
||||
&sandbox_policy,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
);
|
||||
state.set_responsesapi_client_metadata(HashMap::from([
|
||||
("fiber_run_id".to_string(), "fiber-123".to_string()),
|
||||
("session_id".to_string(), "client-supplied".to_string()),
|
||||
]));
|
||||
|
||||
let header = state.current_header_value().expect("header");
|
||||
let json: Value = serde_json::from_str(&header).expect("json");
|
||||
|
||||
assert_eq!(json["fiber_run_id"].as_str(), Some("fiber-123"));
|
||||
assert_eq!(json["session_id"].as_str(), Some("session-a"));
|
||||
assert_eq!(json["turn_id"].as_str(), Some("turn-a"));
|
||||
}
|
||||
Reference in new issue
Block a user