mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Preserve client metadata through queued submissions (#59826)
## Why Queued inputs previously stored and dispatched without the submitting client's Responses API metadata, losing attribution such as `client_type` when the queued turn started. ## What changed - Accept `responsesapiClientMetadata` when adding or updating queued submissions and expose it on returned queue items. - Persist metadata with the input and forward it when starting a selected item or dispatching an idle queue, including after restoration. - Preserve existing metadata when an update omits it or supplies an empty map; replace it when a non-empty map is supplied. - Keep stored inputs readable by older queue workers and update generated protocol artifacts. ## Testing Extend queue service and app-server tests to cover metadata round trips, preservation and replacement on edits, compatibility with older readers, and forwarding during selected, idle, and restored dispatch. GitOrigin-RevId: 508043b1d3d0298c9af2be8e05b6999de46596a1
This commit is contained in:
@@ -42,6 +42,7 @@ use uuid::Uuid;
|
||||
pub struct QueuedItem {
|
||||
pub id: String,
|
||||
pub input: TurnInput,
|
||||
pub responsesapi_client_metadata: Option<HashMap<String, String>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
@@ -266,12 +267,22 @@ impl QueuedItemService {
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
input: TurnInput,
|
||||
) -> Result<QueuedItem, QueueServiceError> {
|
||||
self.enqueue_with_metadata(thread_id, input, /*responsesapi_client_metadata*/ None)
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn enqueue_with_metadata(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
input: TurnInput,
|
||||
responsesapi_client_metadata: Option<HashMap<String, String>>,
|
||||
) -> Result<QueuedItem, QueueServiceError> {
|
||||
let input = prepare_queued_user_input(input).await?;
|
||||
let payload = serde_json::to_string(&input)?;
|
||||
let payload = queued_input_payload(&input, responsesapi_client_metadata.as_ref())?;
|
||||
let item = {
|
||||
let _dispatch_guard = self.dispatch_guard(thread_id).await;
|
||||
let item = queued_item_from_record(self.queue.enqueue(thread_id, payload).await?)?;
|
||||
let item = QueuedItem::try_from(self.queue.enqueue(thread_id, payload).await?)?;
|
||||
self.emit_changed(thread_id);
|
||||
item
|
||||
};
|
||||
@@ -294,7 +305,7 @@ impl QueuedItemService {
|
||||
.list_page(thread_id, offset, limit)
|
||||
.await?
|
||||
.into_iter()
|
||||
.map(queued_item_from_record)
|
||||
.map(QueuedItem::try_from)
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -302,29 +313,50 @@ impl QueuedItemService {
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
queued_item_id: String,
|
||||
mut input: TurnInput,
|
||||
input: TurnInput,
|
||||
) -> Result<Option<QueuedItem>, QueueServiceError> {
|
||||
self.update_with_metadata(
|
||||
thread_id,
|
||||
queued_item_id,
|
||||
input,
|
||||
/*responsesapi_client_metadata*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Omitted or empty metadata preserves the input's attribution, not the current controller's.
|
||||
pub async fn update_with_metadata(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
queued_item_id: String,
|
||||
mut input: TurnInput,
|
||||
mut responsesapi_client_metadata: Option<HashMap<String, String>>,
|
||||
) -> Result<Option<QueuedItem>, QueueServiceError> {
|
||||
responsesapi_client_metadata =
|
||||
responsesapi_client_metadata.filter(|metadata| !metadata.is_empty());
|
||||
let _dispatch_guard = self.dispatch_guard(thread_id).await;
|
||||
let previous = self
|
||||
.list(thread_id)
|
||||
.await?
|
||||
.into_iter()
|
||||
.find(|item| item.id == queued_item_id);
|
||||
if let TurnInput::UserInput { client_id, .. } = &mut input {
|
||||
*client_id = self
|
||||
.list(thread_id)
|
||||
.await?
|
||||
.into_iter()
|
||||
.find_map(|item| match item {
|
||||
QueuedItem {
|
||||
id,
|
||||
input: TurnInput::UserInput { client_id, .. },
|
||||
} if id == queued_item_id => client_id,
|
||||
_ => None,
|
||||
});
|
||||
*client_id = previous.as_ref().and_then(|item| match &item.input {
|
||||
TurnInput::UserInput { client_id, .. } => client_id.clone(),
|
||||
_ => None,
|
||||
});
|
||||
}
|
||||
if responsesapi_client_metadata.is_none() {
|
||||
responsesapi_client_metadata =
|
||||
previous.and_then(|item| item.responsesapi_client_metadata);
|
||||
}
|
||||
let input = prepare_queued_user_input(input).await?;
|
||||
let payload = serde_json::to_string(&input)?;
|
||||
let payload = queued_input_payload(&input, responsesapi_client_metadata.as_ref())?;
|
||||
let item = self
|
||||
.queue
|
||||
.update(thread_id, queued_item_id, payload)
|
||||
.await?
|
||||
.map(queued_item_from_record)
|
||||
.map(QueuedItem::try_from)
|
||||
.transpose()?;
|
||||
if item.is_some() {
|
||||
self.emit_changed(thread_id);
|
||||
@@ -389,12 +421,15 @@ impl QueuedItemService {
|
||||
return Err(QueueServiceError::InvalidInput);
|
||||
};
|
||||
let submission = thread
|
||||
.start_turn_if_idle(TurnInputRequest::new(input).with_trace(trace).on_start(
|
||||
TurnStartOptions {
|
||||
turn_trigger: Some("queue".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
))
|
||||
.start_turn_if_idle(
|
||||
TurnInputRequest::new(input)
|
||||
.with_responses_metadata(item.responsesapi_client_metadata)
|
||||
.with_trace(trace)
|
||||
.on_start(TurnStartOptions {
|
||||
turn_trigger: Some("queue".to_string()),
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
if matches!(submission, StartIfIdleSubmission::Started { .. }) {
|
||||
self.delete_locked(thread_id, queued_item_id).await?;
|
||||
@@ -422,25 +457,29 @@ impl QueuedItemService {
|
||||
};
|
||||
let queued_item_id = record.id.clone();
|
||||
|
||||
let input = match serde_json::from_str::<TurnInput>(&record.payload) {
|
||||
Ok(input) => input,
|
||||
let item = match QueuedItem::try_from(record) {
|
||||
Ok(item) => item,
|
||||
Err(error) => {
|
||||
tracing::warn!(%queued_item_id, %error, "discarding invalid queued item");
|
||||
self.delete_locked(thread_id, queued_item_id).await?;
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if !matches!(input, TurnInput::UserInput { .. }) {
|
||||
if !matches!(item.input, TurnInput::UserInput { .. }) {
|
||||
tracing::warn!(%queued_item_id, "discarding non-user queued input");
|
||||
self.delete_locked(thread_id, queued_item_id).await?;
|
||||
continue;
|
||||
}
|
||||
|
||||
match thread
|
||||
.start_turn_if_idle(TurnInputRequest::new(input).on_start(TurnStartOptions {
|
||||
turn_trigger: Some("queue".to_string()),
|
||||
..Default::default()
|
||||
}))
|
||||
.start_turn_if_idle(
|
||||
TurnInputRequest::new(item.input)
|
||||
.with_responses_metadata(item.responsesapi_client_metadata)
|
||||
.on_start(TurnStartOptions {
|
||||
turn_trigger: Some("queue".to_string()),
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(StartIfIdleSubmission::Started { .. }) => {
|
||||
@@ -566,11 +605,34 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
fn queued_item_from_record(
|
||||
record: QueuedUserSubmissionRecord,
|
||||
) -> Result<QueuedItem, QueueServiceError> {
|
||||
Ok(QueuedItem {
|
||||
id: record.id,
|
||||
input: serde_json::from_str::<TurnInput>(&record.payload)?,
|
||||
})
|
||||
fn queued_input_payload(
|
||||
input: &TurnInput,
|
||||
metadata: Option<&HashMap<String, String>>,
|
||||
) -> Result<String, QueueServiceError> {
|
||||
let Some(metadata) = metadata else {
|
||||
return Ok(serde_json::to_string(input)?);
|
||||
};
|
||||
let mut payload = serde_json::to_value(input)?;
|
||||
// Keep TurnInput's wire shape so older readers can still execute the input.
|
||||
payload["UserInput"]["responsesapi_client_metadata"] = serde_json::to_value(metadata)?;
|
||||
Ok(serde_json::to_string(&payload)?)
|
||||
}
|
||||
|
||||
impl TryFrom<QueuedUserSubmissionRecord> for QueuedItem {
|
||||
type Error = QueueServiceError;
|
||||
|
||||
fn try_from(record: QueuedUserSubmissionRecord) -> Result<Self, Self::Error> {
|
||||
let mut payload: serde_json::Value = serde_json::from_str(&record.payload)?;
|
||||
let responsesapi_client_metadata = payload
|
||||
.get_mut("UserInput")
|
||||
.and_then(serde_json::Value::as_object_mut)
|
||||
.and_then(|input| input.remove("responsesapi_client_metadata"))
|
||||
.map(serde_json::from_value)
|
||||
.transpose()?;
|
||||
Ok(QueuedItem {
|
||||
id: record.id,
|
||||
input: serde_json::from_value(payload)?,
|
||||
responsesapi_client_metadata,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use codex_protocol::models::AudioReference;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
@@ -316,11 +317,28 @@ async fn queued_input_and_unique_event_ids_round_trip() -> anyhow::Result<()> {
|
||||
let (queue, _home) = test_queue().await?;
|
||||
let sink = Arc::new(RecordingEventSink::default());
|
||||
let event_sink: Arc<dyn ExtensionEventSink> = sink.clone();
|
||||
let service = QueuedItemService::new(queue, Weak::new(), event_sink);
|
||||
let service = QueuedItemService::new(Arc::clone(&queue), Weak::new(), event_sink);
|
||||
let thread_id = ThreadId::new();
|
||||
let input = structured_user_input("structured message");
|
||||
|
||||
let first = service.enqueue(thread_id, input.clone()).await?;
|
||||
let first = service
|
||||
.enqueue_with_metadata(
|
||||
thread_id,
|
||||
input.clone(),
|
||||
Some(HashMap::from([(
|
||||
"client_type".to_string(),
|
||||
"ios_app".to_string(),
|
||||
)])),
|
||||
)
|
||||
.await?;
|
||||
// Old workers still read TurnInput directly from the stored payload.
|
||||
let records = queue
|
||||
.list_page(thread_id, /*offset*/ 0, /*limit*/ 1)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
input,
|
||||
serde_json::from_str::<TurnInput>(&records[0].payload)?
|
||||
);
|
||||
let second = service.enqueue(thread_id, user_input("next")).await?;
|
||||
let TurnInput::UserInput {
|
||||
client_id: Some(generated_client_id),
|
||||
@@ -362,7 +380,14 @@ async fn editing_reordering_and_deleting_preserve_queue_identity() -> anyhow::Re
|
||||
let service = QueuedItemService::new(queue, Weak::new(), Arc::new(NoopExtensionEventSink));
|
||||
let thread_id = ThreadId::new();
|
||||
let first = service
|
||||
.enqueue(thread_id, structured_user_input("first"))
|
||||
.enqueue_with_metadata(
|
||||
thread_id,
|
||||
structured_user_input("first"),
|
||||
Some(HashMap::from([(
|
||||
"client_type".to_string(),
|
||||
"desktop_app".to_string(),
|
||||
)])),
|
||||
)
|
||||
.await?;
|
||||
let second = service.enqueue(thread_id, user_input("second")).await?;
|
||||
let edited = service
|
||||
@@ -374,6 +399,21 @@ async fn editing_reordering_and_deleting_preserve_queue_identity() -> anyhow::Re
|
||||
anyhow::bail!("edited queue item does not contain user input");
|
||||
};
|
||||
assert_eq!(Some("stable-client-message"), client_id.as_deref());
|
||||
assert_eq!(
|
||||
first.responsesapi_client_metadata,
|
||||
edited.responsesapi_client_metadata
|
||||
);
|
||||
let mobile_metadata = HashMap::from([("client_type".to_string(), "ios_app".to_string())]);
|
||||
let edited = service
|
||||
.update_with_metadata(
|
||||
thread_id,
|
||||
first.id.clone(),
|
||||
user_input("edited on phone"),
|
||||
Some(mobile_metadata.clone()),
|
||||
)
|
||||
.await?
|
||||
.context("queued item missing")?;
|
||||
assert_eq!(Some(mobile_metadata), edited.responsesapi_client_metadata);
|
||||
assert!(matches!(
|
||||
service.reorder(thread_id, vec![first.id.clone()]).await,
|
||||
Err(QueueServiceError::Storage(
|
||||
@@ -403,7 +443,14 @@ async fn starting_a_selected_item_preserves_the_remaining_queue() -> anyhow::Res
|
||||
let service = QueuedItemService::new(queue, Weak::new(), Arc::new(NoopExtensionEventSink));
|
||||
let first = service.enqueue(thread_id, user_input("first")).await?;
|
||||
let second = service
|
||||
.enqueue(thread_id, structured_user_input("second"))
|
||||
.enqueue_with_metadata(
|
||||
thread_id,
|
||||
structured_user_input("second"),
|
||||
Some(HashMap::from([(
|
||||
"client_type".to_string(),
|
||||
"ios_app".to_string(),
|
||||
)])),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let submission = service
|
||||
@@ -432,6 +479,13 @@ async fn starting_a_selected_item_preserves_the_remaining_queue() -> anyhow::Res
|
||||
.last()
|
||||
.map(String::as_str)
|
||||
);
|
||||
let body = response.single_request().body_json();
|
||||
let metadata: serde_json::Value = serde_json::from_str(
|
||||
body["client_metadata"]["x-codex-turn-metadata"]
|
||||
.as_str()
|
||||
.context("missing turn metadata")?,
|
||||
)?;
|
||||
assert_eq!(metadata["client_type"], "ios_app");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1107,7 +1161,8 @@ async fn invalid_head_is_skipped_and_a_live_user_turn_is_accepted() -> anyhow::R
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn resumed_idle_dispatches_input_without_a_loaded_manager() -> anyhow::Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
responses::mount_sse_once(&server, responses::sse_completed("resumed-turn")).await;
|
||||
let response =
|
||||
responses::mount_sse_once(&server, responses::sse_completed("resumed-turn")).await;
|
||||
let test = test_codex().build_with_auto_env(&server).await?;
|
||||
let thread_id = test.session_configured.thread_id;
|
||||
let queue = loaded_thread_queue(&test)?;
|
||||
@@ -1116,7 +1171,14 @@ async fn resumed_idle_dispatches_input_without_a_loaded_manager() -> anyhow::Res
|
||||
Weak::new(),
|
||||
Arc::new(NoopExtensionEventSink),
|
||||
)
|
||||
.enqueue(thread_id, user_input("queued while unloaded"))
|
||||
.enqueue_with_metadata(
|
||||
thread_id,
|
||||
user_input("queued while unloaded"),
|
||||
Some(HashMap::from([(
|
||||
"client_type".to_string(),
|
||||
"ios_app".to_string(),
|
||||
)])),
|
||||
)
|
||||
.await?;
|
||||
let service = QueuedItemService::new(
|
||||
queue,
|
||||
@@ -1125,5 +1187,17 @@ async fn resumed_idle_dispatches_input_without_a_loaded_manager() -> anyhow::Res
|
||||
);
|
||||
emit_idle(&service, thread_id).await;
|
||||
assert!(service.list(thread_id).await?.is_empty());
|
||||
wait_for_event_match(test.codex.as_ref(), |event| match event {
|
||||
EventMsg::TurnComplete(_) => Some(()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
let body = response.single_request().body_json();
|
||||
let metadata: serde_json::Value = serde_json::from_str(
|
||||
body["client_metadata"]["x-codex-turn-metadata"]
|
||||
.as_str()
|
||||
.context("missing turn metadata")?,
|
||||
)?;
|
||||
assert_eq!(metadata["client_type"], "ios_app");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user