mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
core: stabilize synthesized call output IDs
This commit is contained in:
@@ -1622,6 +1622,43 @@ fn normalize_adds_missing_output_for_function_call_inserts_output() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn for_prompt_assigns_stable_id_to_synthetic_output_without_reordering_history() {
|
||||
let items = vec![
|
||||
ResponseItem::FunctionCall {
|
||||
id: Some("fc_existing".to_string()),
|
||||
name: "do_it".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: "call-x".to_string(),
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: Some("msg_later".to_string()),
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "later turn".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
},
|
||||
];
|
||||
|
||||
let first = create_history_with_items(items.clone()).for_prompt(&default_input_modalities());
|
||||
let second = create_history_with_items(items).for_prompt(&default_input_modalities());
|
||||
|
||||
assert_eq!(first, second);
|
||||
let [
|
||||
ResponseItem::FunctionCall { .. },
|
||||
ResponseItem::FunctionCallOutput { id: Some(id), .. },
|
||||
ResponseItem::Message { .. },
|
||||
] = first.as_slice()
|
||||
else {
|
||||
panic!("expected the synthetic output between its call and the later message");
|
||||
};
|
||||
assert!(id.starts_with("fco_"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_adds_missing_output_for_tool_search_call() {
|
||||
let items = vec![ResponseItem::ToolSearchCall {
|
||||
|
||||
@@ -4,12 +4,15 @@ use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use std::collections::HashSet;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::util::error_or_panic;
|
||||
use tracing::info;
|
||||
|
||||
const IMAGE_CONTENT_OMITTED_PLACEHOLDER: &str =
|
||||
"image content omitted because you do not support image input";
|
||||
// Changing this value would change model-visible IDs and invalidate prompt caches.
|
||||
const SYNTHETIC_OUTPUT_ID_NAMESPACE: Uuid = Uuid::from_u128(0x90d38d3e_6a5b_4d52_bfe2_2f1e634bfac4);
|
||||
|
||||
pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
let mut function_output_ids = HashSet::new();
|
||||
@@ -40,14 +43,14 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
|
||||
for (idx, item) in items.iter().enumerate() {
|
||||
match item {
|
||||
ResponseItem::FunctionCall { call_id, .. }
|
||||
ResponseItem::FunctionCall { id, call_id, .. }
|
||||
if !function_output_ids.contains(call_id.as_str()) =>
|
||||
{
|
||||
info!("Function call output is missing for call id: {call_id}");
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::FunctionCallOutput {
|
||||
id: None,
|
||||
id: synthetic_output_id("fco", id.as_deref()),
|
||||
call_id: call_id.clone(),
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
@@ -55,6 +58,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
));
|
||||
}
|
||||
ResponseItem::ToolSearchCall {
|
||||
id,
|
||||
call_id: Some(call_id),
|
||||
..
|
||||
} if !tool_search_output_ids.contains(call_id.as_str()) => {
|
||||
@@ -62,7 +66,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::ToolSearchOutput {
|
||||
id: None,
|
||||
id: synthetic_output_id("tso", id.as_deref()),
|
||||
call_id: Some(call_id.clone()),
|
||||
status: "completed".to_string(),
|
||||
execution: "client".to_string(),
|
||||
@@ -71,7 +75,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
},
|
||||
));
|
||||
}
|
||||
ResponseItem::CustomToolCall { call_id, .. }
|
||||
ResponseItem::CustomToolCall { id, call_id, .. }
|
||||
if !custom_tool_output_ids.contains(call_id.as_str()) =>
|
||||
{
|
||||
error_or_panic(format!(
|
||||
@@ -80,7 +84,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::CustomToolCallOutput {
|
||||
id: None,
|
||||
id: synthetic_output_id("ctco", id.as_deref()),
|
||||
call_id: call_id.clone(),
|
||||
name: None,
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
@@ -90,6 +94,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
}
|
||||
// LocalShellCall is represented in upstream streams by a FunctionCallOutput
|
||||
ResponseItem::LocalShellCall {
|
||||
id,
|
||||
call_id: Some(call_id),
|
||||
..
|
||||
} if !function_output_ids.contains(call_id.as_str()) => {
|
||||
@@ -99,7 +104,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
missing_outputs_to_insert.push((
|
||||
idx,
|
||||
ResponseItem::FunctionCallOutput {
|
||||
id: None,
|
||||
id: synthetic_output_id("fco", id.as_deref()),
|
||||
call_id: call_id.clone(),
|
||||
output: FunctionCallOutputPayload::from_text("aborted".to_string()),
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
@@ -121,6 +126,15 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec<ResponseItem>) {
|
||||
}
|
||||
}
|
||||
|
||||
fn synthetic_output_id(prefix: &str, item_id: Option<&str>) -> Option<String> {
|
||||
let source_id = item_id.filter(|id| !id.is_empty())?;
|
||||
let name = format!("{prefix}:{source_id}");
|
||||
Some(format!(
|
||||
"{prefix}_{}",
|
||||
Uuid::new_v5(&SYNTHETIC_OUTPUT_ID_NAMESPACE, name.as_bytes())
|
||||
))
|
||||
}
|
||||
|
||||
pub(crate) fn remove_orphan_outputs(items: &mut Vec<ResponseItem>) {
|
||||
let function_call_ids: HashSet<String> = items
|
||||
.iter()
|
||||
|
||||
@@ -363,6 +363,107 @@ async fn response_item_ids_persist_across_resume_and_preserve_server_ids() -> an
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn synthetic_call_output_id_is_stable_across_resumes() -> anyhow::Result<()> {
|
||||
let function_call_id = "missing-output-call";
|
||||
let thread_id = ThreadId::default();
|
||||
let rollout = vec![
|
||||
RolloutLine {
|
||||
timestamp: "2024-01-01T00:00:00.000Z".to_string(),
|
||||
item: RolloutItem::SessionMeta(SessionMetaLine {
|
||||
meta: SessionMeta {
|
||||
session_id: thread_id.into(),
|
||||
id: thread_id,
|
||||
parent_thread_id: None,
|
||||
timestamp: "2024-01-01T00:00:00Z".to_string(),
|
||||
cwd: ".".into(),
|
||||
originator: "test_originator".to_string(),
|
||||
cli_version: "test_version".to_string(),
|
||||
model_provider: Some("test-provider".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
git: None,
|
||||
}),
|
||||
},
|
||||
RolloutLine {
|
||||
timestamp: "2024-01-01T00:00:01.000Z".to_string(),
|
||||
item: RolloutItem::ResponseItem(ResponseItem::FunctionCall {
|
||||
id: Some("fc_existing".to_string()),
|
||||
name: "do_it".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: function_call_id.to_string(),
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
}),
|
||||
},
|
||||
];
|
||||
let tmpdir = TempDir::new()?;
|
||||
let session_path = tmpdir.path().join("normalized-call-output-item-id.jsonl");
|
||||
let mut file = std::fs::File::create(&session_path)?;
|
||||
for line in rollout {
|
||||
writeln!(file, "{}", serde_json::to_string(&line)?)?;
|
||||
}
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let response_mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let codex_home = Arc::new(TempDir::new()?);
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
});
|
||||
let first = builder
|
||||
.resume(&server, Arc::clone(&codex_home), session_path.clone())
|
||||
.await?;
|
||||
|
||||
first.submit_turn("first resume").await?;
|
||||
first.codex.submit(Op::Shutdown).await?;
|
||||
wait_for_event(&first.codex, |event| {
|
||||
matches!(event, EventMsg::ShutdownComplete)
|
||||
})
|
||||
.await;
|
||||
assert!(
|
||||
!std::fs::read_to_string(&session_path)?.contains("\"type\":\"function_call_output\""),
|
||||
"prompt-only repair should not be persisted to the rollout"
|
||||
);
|
||||
|
||||
builder = builder.with_config(|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
});
|
||||
let second = builder.resume(&server, codex_home, session_path).await?;
|
||||
second.submit_turn("second resume").await?;
|
||||
|
||||
let requests = response_mock.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let first_output = requests[0].function_call_output(function_call_id);
|
||||
let first_output_id = first_output
|
||||
.get("id")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.expect("reconstructed output should have an item ID")
|
||||
.to_string();
|
||||
let first_output_uuid = first_output_id
|
||||
.strip_prefix("fco_")
|
||||
.expect("synthetic output should use the Responses API prefix");
|
||||
assert_eq!(
|
||||
Uuid::parse_str(first_output_uuid)?.get_version(),
|
||||
Some(uuid::Version::Sha1)
|
||||
);
|
||||
assert_eq!(
|
||||
requests[1]
|
||||
.function_call_output(function_call_id)
|
||||
.get("id")
|
||||
.and_then(serde_json::Value::as_str),
|
||||
Some(first_output_id.as_str())
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn response_item_ids_are_sent_for_all_remote_v2_compaction_requests() -> anyhow::Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
Reference in New Issue
Block a user