mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(core): add metadata field to ResponseItem (#28355)
## Description This PR adds an optional `metadata` field to `ResponseItem` for Responses API calls. Only mechanical plumbing, no actual values populated and sent yet. Turns out just adding a new field to `ResponseItem` has quite a large blast radius already. This change is backwards compatible because `metadata` is optional and omitted when absent, so existing response items and rollout history without it still deserialize and requests that do not set it keep the same wire shape. For provider compatibility, we strip out `metadata` before non-OpenAI Responses requests so Azure and AWS Bedrock never see this field. My followup PR here will actually make use of it to start storing and passing along `turn_id`: https://github.com/openai/codex/pull/28360 ## What changed - Added `ResponseItemMetadata` with optional `turn_id`, plus optional `metadata` on Responses API item variants and inter-agent communication. - Preserved item metadata through response-item rewrites such as truncation, missing tool-output synthesis, compaction history rebuilding, visible-history conversion, rollout/resume, and generated app-server schemas/types. - Strip item metadata from non-OpenAI Responses requests while preserving it for OpenAI-shaped requests. - Updated the mechanical fixture/test construction churn required by the new optional field.
This commit is contained in:
committed by
GitHub
Unverified
parent
bef99f861b
commit
040dafa32d
@@ -181,6 +181,7 @@ use uuid::Uuid;
|
||||
|
||||
use crate::client::ModelClient;
|
||||
use crate::codex_thread::ThreadConfigSnapshot;
|
||||
#[cfg(test)]
|
||||
use crate::compact::collect_user_messages;
|
||||
use crate::config::Config;
|
||||
use crate::config::Constrained;
|
||||
|
||||
@@ -296,7 +296,7 @@ impl Session {
|
||||
// prompt shape.
|
||||
// TODO(ccunningham): if we drop support for None replacement_history compaction items,
|
||||
// we can get rid of this second loop entirely and just build `history` directly in the first loop.
|
||||
let user_messages = collect_user_messages(history.raw_items());
|
||||
let user_messages = compact::collect_user_messages(history.raw_items());
|
||||
let rebuilt = compact::build_compacted_history(
|
||||
Vec::new(),
|
||||
&user_messages,
|
||||
|
||||
@@ -20,6 +20,7 @@ fn user_message(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,6 +32,7 @@ fn assistant_message(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -49,6 +51,7 @@ fn inter_agent_assistant_message(text: &str) -> ResponseItem {
|
||||
text: serde_json::to_string(&communication).unwrap(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -103,6 +103,7 @@ use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::models::ResponseItemMetadata;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::CodexErrorInfo;
|
||||
use codex_protocol::protocol::CompactedItem;
|
||||
@@ -189,6 +190,7 @@ fn user_message(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -200,6 +202,7 @@ fn assistant_message(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -259,6 +262,7 @@ fn skill_message(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1598,6 +1602,9 @@ async fn reconstruct_history_uses_replacement_history_verbatim() {
|
||||
text: "summary".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: Some(ResponseItemMetadata {
|
||||
turn_id: Some("compact-turn".to_string()),
|
||||
}),
|
||||
};
|
||||
let replacement_history = vec![
|
||||
summary_item.clone(),
|
||||
@@ -1608,6 +1615,7 @@ async fn reconstruct_history_uses_replacement_history_verbatim() {
|
||||
text: "stale developer instructions".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
},
|
||||
];
|
||||
let rollout_items = vec![RolloutItem::Compacted(CompactedItem {
|
||||
@@ -1669,32 +1677,35 @@ async fn resize_all_images_prepares_failures_before_history_insertion() {
|
||||
]),
|
||||
success: Some(true),
|
||||
},
|
||||
metadata: None,
|
||||
};
|
||||
|
||||
session
|
||||
.record_conversation_items(turn_context.as_ref(), std::slice::from_ref(&item))
|
||||
.await;
|
||||
|
||||
let expected = vec![ResponseItem::FunctionCallOutput {
|
||||
call_id: "call-1".to_string(),
|
||||
output: FunctionCallOutputPayload {
|
||||
body: FunctionCallOutputBody::ContentItems(vec![
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "before".to_string(),
|
||||
},
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "image content omitted because it could not be processed".to_string(),
|
||||
},
|
||||
FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "https://example.com/image.png".to_string(),
|
||||
detail: Some(ImageDetail::High),
|
||||
},
|
||||
]),
|
||||
success: Some(true),
|
||||
},
|
||||
metadata: None,
|
||||
}];
|
||||
assert_eq!(
|
||||
session.state.lock().await.clone_history().raw_items(),
|
||||
&[ResponseItem::FunctionCallOutput {
|
||||
call_id: "call-1".to_string(),
|
||||
output: FunctionCallOutputPayload {
|
||||
body: FunctionCallOutputBody::ContentItems(vec![
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "before".to_string(),
|
||||
},
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "image content omitted because it could not be processed".to_string(),
|
||||
},
|
||||
FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "https://example.com/image.png".to_string(),
|
||||
detail: Some(ImageDetail::High),
|
||||
},
|
||||
]),
|
||||
success: Some(true),
|
||||
},
|
||||
}]
|
||||
expected.as_slice()
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1721,6 +1732,7 @@ async fn resize_all_images_prepares_resumed_history_before_installing_it() {
|
||||
},
|
||||
],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
|
||||
session
|
||||
@@ -1745,6 +1757,7 @@ async fn resize_all_images_prepares_resumed_history_before_installing_it() {
|
||||
},
|
||||
],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
@@ -1860,7 +1873,8 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only()
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await;
|
||||
expected.extend(session.build_initial_context(&turn_context).await);
|
||||
let initial_context = session.build_initial_context(&turn_context).await;
|
||||
expected.extend(initial_context);
|
||||
let history_after_seed = session.clone_history().await;
|
||||
assert_eq!(expected, history_after_seed.raw_items());
|
||||
|
||||
@@ -7662,6 +7676,7 @@ async fn build_initial_context_omits_default_image_save_location_with_image_hist
|
||||
status: "completed".to_string(),
|
||||
revised_prompt: Some("a tiny blue square".to_string()),
|
||||
result: "Zm9v".to_string(),
|
||||
metadata: None,
|
||||
}],
|
||||
/*reference_context_item*/ None,
|
||||
)
|
||||
@@ -7914,6 +7929,7 @@ async fn handle_output_item_done_records_image_save_history_message() {
|
||||
status: "completed".to_string(),
|
||||
revised_prompt: Some("a tiny blue square".to_string()),
|
||||
result: "Zm9v".to_string(),
|
||||
metadata: None,
|
||||
};
|
||||
|
||||
let mut ctx = HandleOutputCtx {
|
||||
@@ -7944,7 +7960,8 @@ async fn handle_output_item_done_records_image_save_history_message() {
|
||||
image_output_path.display(),
|
||||
),
|
||||
);
|
||||
assert_eq!(history.raw_items(), &[image_message, item]);
|
||||
let expected = vec![image_message, item];
|
||||
assert_eq!(history.raw_items(), expected.as_slice());
|
||||
assert_eq!(
|
||||
std::fs::read(&expected_saved_path).expect("saved file"),
|
||||
b"foo"
|
||||
@@ -7969,6 +7986,7 @@ async fn handle_output_item_done_skips_image_save_message_when_save_fails() {
|
||||
status: "completed".to_string(),
|
||||
revised_prompt: Some("broken payload".to_string()),
|
||||
result: "_-8".to_string(),
|
||||
metadata: None,
|
||||
};
|
||||
|
||||
let mut ctx = HandleOutputCtx {
|
||||
@@ -7985,7 +8003,8 @@ async fn handle_output_item_done_skips_image_save_message_when_save_fails() {
|
||||
.expect("image generation item should still complete");
|
||||
|
||||
let history = session.clone_history().await;
|
||||
assert_eq!(history.raw_items(), &[item]);
|
||||
let expected = vec![item];
|
||||
assert_eq!(history.raw_items(), expected.as_slice());
|
||||
assert!(!expected_saved_path.exists());
|
||||
}
|
||||
|
||||
@@ -8128,6 +8147,7 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co
|
||||
text: format!("{}\nsummary", crate::compact::SUMMARY_PREFIX),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
session
|
||||
.record_conversation_items(&turn_context, std::slice::from_ref(&compacted_summary))
|
||||
@@ -8152,7 +8172,8 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co
|
||||
|
||||
let history = session.clone_history().await;
|
||||
let mut expected_history = vec![compacted_summary];
|
||||
expected_history.extend(session.build_initial_context(&turn_context).await);
|
||||
let initial_context = session.build_initial_context(&turn_context).await;
|
||||
expected_history.extend(initial_context);
|
||||
assert_eq!(history.raw_items().to_vec(), expected_history);
|
||||
}
|
||||
|
||||
@@ -8725,6 +8746,7 @@ async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input()
|
||||
text: "late pending input".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
assert!(
|
||||
history.raw_items().iter().any(|item| item == &expected),
|
||||
@@ -9157,6 +9179,7 @@ async fn abort_empty_active_turn_preserves_pending_input() {
|
||||
text: "late pending input".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
let turn_state = {
|
||||
let mut active = sess.active_turn.lock().await;
|
||||
@@ -9415,6 +9438,7 @@ async fn tool_calls_reopen_mailbox_delivery_for_current_turn() {
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: "call-1".to_string(),
|
||||
metadata: None,
|
||||
};
|
||||
let mut ctx = HandleOutputCtx {
|
||||
sess: Arc::clone(&sess),
|
||||
@@ -9542,6 +9566,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() {
|
||||
call_id: "call-1".to_string(),
|
||||
name: "shell_command".to_string(),
|
||||
input: "{}".to_string(),
|
||||
metadata: None,
|
||||
};
|
||||
|
||||
let call = ToolRouter::build_tool_call(item.clone())
|
||||
@@ -9626,6 +9651,7 @@ async fn sample_rollout(
|
||||
text: "first user".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
live_history.record_items(
|
||||
std::iter::once(&user1),
|
||||
@@ -9640,6 +9666,7 @@ async fn sample_rollout(
|
||||
text: "assistant reply one".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
live_history.record_items(
|
||||
std::iter::once(&assistant1),
|
||||
@@ -9667,6 +9694,7 @@ async fn sample_rollout(
|
||||
text: "second user".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
live_history.record_items(
|
||||
std::iter::once(&user2),
|
||||
@@ -9681,6 +9709,7 @@ async fn sample_rollout(
|
||||
text: "assistant reply two".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
live_history.record_items(
|
||||
std::iter::once(&assistant2),
|
||||
@@ -9708,6 +9737,7 @@ async fn sample_rollout(
|
||||
text: "third user".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
live_history.record_items(
|
||||
std::iter::once(&user3),
|
||||
@@ -9722,6 +9752,7 @@ async fn sample_rollout(
|
||||
text: "assistant reply three".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
};
|
||||
live_history.record_items(
|
||||
std::iter::once(&assistant3),
|
||||
@@ -9867,6 +9898,7 @@ while :; do sleep 1; done"#,
|
||||
})
|
||||
.to_string(),
|
||||
call_id: "shell-cleanup-call".to_string(),
|
||||
metadata: None,
|
||||
};
|
||||
let call = ToolRouter::build_tool_call(item)?
|
||||
.expect("shell command response item should build a tool call");
|
||||
|
||||
@@ -536,6 +536,7 @@ async fn process_compacted_history_preserves_separate_guardian_developer_message
|
||||
text: "stale developer message".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -544,6 +545,7 @@ async fn process_compacted_history_preserves_separate_guardian_developer_message
|
||||
text: "summary".to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
},
|
||||
],
|
||||
InitialContextInjection::BeforeLastUserMessage,
|
||||
|
||||
@@ -1978,7 +1978,7 @@ async fn try_run_sampling_request(
|
||||
| ResponseItem::WebSearchCall { .. }
|
||||
| ResponseItem::ImageGenerationCall { .. }
|
||||
| ResponseItem::Compaction { .. }
|
||||
| ResponseItem::CompactionTrigger
|
||||
| ResponseItem::CompactionTrigger { .. }
|
||||
| ResponseItem::ContextCompaction { .. }
|
||||
| ResponseItem::Other => false,
|
||||
};
|
||||
|
||||
@@ -33,6 +33,7 @@ fn assistant_output_text(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
phase: None,
|
||||
metadata: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user