mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Always assign response item IDs (#34645)
## What changed - Assign IDs to client-created response items in every session, including streamed items, forked history, compaction results, and non-OpenAI provider requests. - Preserve server-provided IDs and stable generated IDs across persisted and resumed conversations while continuing to omit unprefixed server IDs from outbound requests. - Retire `features.item_ids` as a configurable feature. Existing configuration and CLI toggles remain accepted as compatibility no-ops. ## Testing - Cover default ID assignment, persistence and resume behavior, remote compaction, provider requests, WebSocket requests, and removed-feature compatibility. GitOrigin-RevId: 7a09e2d07d2606c2ef1e8e2283d36cb555557eae
This commit is contained in:
@@ -16,6 +16,8 @@ use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::strip_response_item_id;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use serde_json::Value;
|
||||
use std::path::Path;
|
||||
use tempfile::TempDir;
|
||||
@@ -89,7 +91,7 @@ async fn thread_inject_items_adds_raw_response_items_to_thread_history() -> Resu
|
||||
resumed_history
|
||||
.history
|
||||
.iter()
|
||||
.any(|item| matches!(item, RolloutItem::ResponseItem(response_item) if responses::strip_metadata(response_item.clone()) == injected_item)),
|
||||
.any(|item| matches!(item, RolloutItem::ResponseItem(response_item) if strip_response_item_id(responses::strip_metadata(response_item.clone())) == injected_item)),
|
||||
"injected item should be persisted in rollout history"
|
||||
);
|
||||
|
||||
@@ -116,7 +118,12 @@ async fn thread_inject_items_adds_raw_response_items_to_thread_history() -> Resu
|
||||
.await??;
|
||||
|
||||
let injected_value = serde_json::to_value(&injected_item)?;
|
||||
let model_input = response_mock.single_request().input();
|
||||
let model_input: Vec<Value> = response_mock
|
||||
.single_request()
|
||||
.input()
|
||||
.into_iter()
|
||||
.map(strip_response_item_ids_from_json)
|
||||
.collect();
|
||||
let environment_context_index =
|
||||
response_item_text_position(&model_input, "<environment_context>")
|
||||
.expect("environment context should be injected before the first user turn");
|
||||
@@ -247,11 +254,19 @@ async fn thread_inject_items_adds_raw_response_items_after_a_turn() -> Result<()
|
||||
let requests = response_mock.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
assert!(
|
||||
!requests[0].input().contains(&injected_value),
|
||||
!requests[0]
|
||||
.input()
|
||||
.into_iter()
|
||||
.map(strip_response_item_ids_from_json)
|
||||
.any(|item| item == injected_value),
|
||||
"injected item should not be sent before it is injected"
|
||||
);
|
||||
assert!(
|
||||
requests[1].input().contains(&injected_value),
|
||||
requests[1]
|
||||
.input()
|
||||
.into_iter()
|
||||
.map(strip_response_item_ids_from_json)
|
||||
.any(|item| item == injected_value),
|
||||
"injected item should be sent after being injected into existing history"
|
||||
);
|
||||
|
||||
|
||||
@@ -24,6 +24,7 @@ use codex_app_server_protocol::WebSearchAction;
|
||||
use codex_app_server_protocol::WebSearchItem;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
@@ -228,7 +229,9 @@ async fn standalone_web_search_round_trips_output() -> Result<()> {
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
responses::strip_metadata_from_json(requests[1].function_call_output(call_id)),
|
||||
strip_response_item_ids_from_json(responses::strip_metadata_from_json(
|
||||
requests[1].function_call_output(call_id),
|
||||
)),
|
||||
json!({
|
||||
"type": "function_call_output",
|
||||
"call_id": call_id,
|
||||
|
||||
@@ -4133,6 +4133,16 @@ mod tests {
|
||||
assert_eq!(overrides, vec!["features.enable_fanout=true".to_string(),]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn feature_toggles_accept_removed_item_ids_flag() {
|
||||
let toggles = FeatureToggles {
|
||||
enable: vec!["item_ids".to_string()],
|
||||
disable: Vec::new(),
|
||||
};
|
||||
let overrides = toggles.to_overrides().expect("valid features");
|
||||
assert_eq!(overrides, vec!["features.item_ids=true".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn feature_toggles_unknown_feature_errors() {
|
||||
let toggles = FeatureToggles {
|
||||
|
||||
@@ -56,6 +56,7 @@ use codex_thread_store::LocalThreadStore;
|
||||
use codex_thread_store::LocalThreadStoreConfig;
|
||||
use codex_thread_store::ThreadStore;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::responses::strip_response_item_ids;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::Duration;
|
||||
@@ -1340,8 +1341,8 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
|
||||
},
|
||||
];
|
||||
assert_eq!(
|
||||
history.raw_items(),
|
||||
&expected_history,
|
||||
strip_response_item_ids(history.raw_items()),
|
||||
strip_response_item_ids(&expected_history),
|
||||
"full-history forked child history should replace parent usage hints with the child subagent hint while filtering non-final assistant/tool chatter"
|
||||
);
|
||||
assert_eq!(
|
||||
|
||||
@@ -207,7 +207,6 @@ struct ModelClientState {
|
||||
enable_request_compression: bool,
|
||||
include_timing_metrics: bool,
|
||||
beta_features_header: Option<String>,
|
||||
item_ids_enabled: bool,
|
||||
concurrent_reasoning_summaries_enabled: bool,
|
||||
include_attestation: bool,
|
||||
attestation_provider: Option<Arc<dyn AttestationProvider>>,
|
||||
@@ -421,7 +420,6 @@ impl ModelClient {
|
||||
enable_request_compression: bool,
|
||||
include_timing_metrics: bool,
|
||||
beta_features_header: Option<String>,
|
||||
item_ids_enabled: bool,
|
||||
concurrent_reasoning_summaries_enabled: bool,
|
||||
attestation_provider: Option<Arc<dyn AttestationProvider>>,
|
||||
http_client_factory: HttpClientFactory,
|
||||
@@ -445,7 +443,6 @@ impl ModelClient {
|
||||
enable_request_compression,
|
||||
include_timing_metrics,
|
||||
beta_features_header,
|
||||
item_ids_enabled,
|
||||
concurrent_reasoning_summaries_enabled,
|
||||
include_attestation,
|
||||
attestation_provider,
|
||||
@@ -583,7 +580,7 @@ impl ModelClient {
|
||||
text,
|
||||
..
|
||||
} = request;
|
||||
self.prepare_response_items_for_request(&mut input, /*store*/ false);
|
||||
self.prepare_response_items_for_request(&mut input);
|
||||
let payload = ApiCompactionInput {
|
||||
model: &model,
|
||||
input: &input,
|
||||
@@ -908,20 +905,12 @@ impl ModelClient {
|
||||
Ok(request)
|
||||
}
|
||||
|
||||
fn prepare_response_items_for_request(&self, input: &mut [ResponseItem], store: bool) {
|
||||
for item in input.iter_mut() {
|
||||
fn prepare_response_items_for_request(&self, input: &mut [ResponseItem]) {
|
||||
for item in input {
|
||||
if item.id().is_some_and(|id| !id.is_prefixed()) {
|
||||
item.set_id(/*new_id*/ None);
|
||||
}
|
||||
}
|
||||
|
||||
if self.state.item_ids_enabled || store {
|
||||
return;
|
||||
}
|
||||
|
||||
for item in input {
|
||||
item.set_id(/*new_id*/ None);
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns whether the Responses-over-WebSocket transport is active for this session.
|
||||
@@ -1439,9 +1428,8 @@ impl ModelClientSession {
|
||||
service_tier.clone(),
|
||||
responses_metadata,
|
||||
)?;
|
||||
let store = request.store;
|
||||
self.client
|
||||
.prepare_response_items_for_request(&mut request.input, store);
|
||||
.prepare_response_items_for_request(&mut request.input);
|
||||
let request_session_telemetry =
|
||||
session_telemetry_for_request(session_telemetry, &request);
|
||||
let inference_trace_attempt = inference_trace.start_attempt();
|
||||
@@ -1620,10 +1608,9 @@ impl ModelClientSession {
|
||||
Some((response_id, items)) => (Some(response_id), Some(items)),
|
||||
None => (None, None),
|
||||
};
|
||||
let store = request.store;
|
||||
let original_item_ids = if let Some(incremental_items) = &mut incremental_items {
|
||||
self.client
|
||||
.prepare_response_items_for_request(incremental_items, store);
|
||||
.prepare_response_items_for_request(incremental_items);
|
||||
None
|
||||
} else {
|
||||
let original_item_ids = request
|
||||
@@ -1632,7 +1619,7 @@ impl ModelClientSession {
|
||||
.map(|item| item.id().cloned())
|
||||
.collect::<Vec<_>>();
|
||||
self.client
|
||||
.prepare_response_items_for_request(&mut request.input, store);
|
||||
.prepare_response_items_for_request(&mut request.input);
|
||||
Some(original_item_ids)
|
||||
};
|
||||
let ws_payload = ResponseCreateWsRequest {
|
||||
|
||||
@@ -104,7 +104,6 @@ fn test_model_client_with_thread_id(
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*attestation_provider*/ None,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
@@ -150,7 +149,6 @@ async fn compact_uses_bearer_after_agent_identity_session_fallback() -> anyhow::
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*attestation_provider*/ None,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
@@ -825,7 +823,6 @@ fn model_client_with_counting_attestation(
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
Some(Arc::new(CountingAttestationProvider {
|
||||
calls: attestation_calls.clone(),
|
||||
|
||||
@@ -368,7 +368,6 @@ async fn run_compact_task_inner_impl(
|
||||
}
|
||||
};
|
||||
sess.replace_compacted_history(
|
||||
turn_context.as_ref(),
|
||||
new_history,
|
||||
reference_context_item,
|
||||
world_state_baseline,
|
||||
|
||||
@@ -279,7 +279,6 @@ async fn run_remote_compact_task_inner_impl(
|
||||
});
|
||||
}
|
||||
sess.replace_compacted_history(
|
||||
compaction_turn_context.as_ref(),
|
||||
new_history,
|
||||
reference_context_item,
|
||||
world_state_baseline,
|
||||
|
||||
@@ -300,7 +300,6 @@ async fn run_remote_compact_task_inner_impl(
|
||||
});
|
||||
}
|
||||
sess.replace_compacted_history(
|
||||
compaction_turn_context.as_ref(),
|
||||
new_history,
|
||||
reference_context_item,
|
||||
world_state_baseline,
|
||||
|
||||
@@ -1324,9 +1324,7 @@ impl Session {
|
||||
}
|
||||
InitialHistory::Forked(mut rollout_items) => {
|
||||
let turn_context = self.new_default_turn().await;
|
||||
if turn_context.item_ids_enabled() {
|
||||
Self::assign_missing_rollout_response_item_ids(&mut rollout_items);
|
||||
}
|
||||
Self::assign_missing_rollout_response_item_ids(&mut rollout_items);
|
||||
self.apply_rollout_reconstruction(&turn_context, &rollout_items)
|
||||
.await;
|
||||
|
||||
@@ -2807,11 +2805,7 @@ impl Session {
|
||||
for item in items.to_mut() {
|
||||
item.set_turn_id_if_missing(&turn_context.sub_id);
|
||||
}
|
||||
if turn_context.item_ids_enabled() {
|
||||
Self::assign_missing_response_item_ids(items)
|
||||
} else {
|
||||
items
|
||||
}
|
||||
Self::assign_missing_response_item_ids(items)
|
||||
}
|
||||
|
||||
fn assign_missing_response_item_ids(items: Cow<'_, [ResponseItem]>) -> Cow<'_, [ResponseItem]> {
|
||||
@@ -3057,17 +3051,12 @@ impl Session {
|
||||
|
||||
pub(crate) async fn replace_compacted_history(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
items: Vec<ResponseItem>,
|
||||
reference_context_item: Option<TurnContextItem>,
|
||||
world_state_baseline: Option<Arc<WorldState>>,
|
||||
metadata: CompactedHistoryMetadata,
|
||||
) {
|
||||
let items = if turn_context.item_ids_enabled() {
|
||||
Self::assign_missing_response_item_ids(Cow::Owned(items)).into_owned()
|
||||
} else {
|
||||
items
|
||||
};
|
||||
let items = Self::assign_missing_response_item_ids(Cow::Owned(items)).into_owned();
|
||||
let compacted_item = CompactedItem {
|
||||
message: metadata.message,
|
||||
replacement_history: Some(items.clone()),
|
||||
@@ -3556,7 +3545,6 @@ impl Session {
|
||||
.await;
|
||||
let turn_context_item = turn_context.to_turn_context_item();
|
||||
self.replace_compacted_history(
|
||||
turn_context,
|
||||
context_items,
|
||||
Some(turn_context_item),
|
||||
Some(world_state),
|
||||
|
||||
@@ -522,13 +522,7 @@ impl Session {
|
||||
session_configuration.thread_source.as_ref(),
|
||||
Some(ThreadSource::Subagent)
|
||||
);
|
||||
if (config.features.enabled(Feature::ItemIds)
|
||||
|| matches!(
|
||||
session_configuration.history_mode,
|
||||
ThreadHistoryMode::Paginated
|
||||
))
|
||||
&& let InitialHistory::Forked(items) = &mut initial_history
|
||||
{
|
||||
if let InitialHistory::Forked(items) = &mut initial_history {
|
||||
Self::assign_missing_rollout_response_item_ids(items);
|
||||
}
|
||||
let multi_agent_version = multi_agent_version.map(OnceLock::from).unwrap_or_default();
|
||||
@@ -1120,11 +1114,6 @@ impl Session {
|
||||
config.features.enabled(Feature::EnableRequestCompression),
|
||||
config.features.enabled(Feature::RuntimeMetrics),
|
||||
Self::build_model_client_beta_features_header(config.as_ref()),
|
||||
/*item_ids_enabled*/ config.features.enabled(Feature::ItemIds)
|
||||
|| matches!(
|
||||
session_configuration.history_mode,
|
||||
ThreadHistoryMode::Paginated
|
||||
),
|
||||
/*concurrent_reasoning_summaries_enabled*/ config
|
||||
.features
|
||||
.enabled(Feature::ConcurrentReasoningSummaries),
|
||||
|
||||
@@ -67,7 +67,6 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::NonSteerableTurnKind;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::ThreadHistoryMode;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelections;
|
||||
use codex_protocol::request_permissions::PermissionGrantScope;
|
||||
use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
@@ -163,6 +162,8 @@ use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::responses::strip_metadata_from_items;
|
||||
use core_test_support::responses::strip_response_item_ids;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use core_test_support::test_codex::local;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::test_path_buf;
|
||||
@@ -261,9 +262,8 @@ fn assign_missing_response_item_ids_assigns_additional_tools_ids() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn paginated_turn_context_assigns_missing_response_item_ids_without_feature() {
|
||||
let (session, mut turn_context) = make_session_and_context().await;
|
||||
turn_context.history_mode = ThreadHistoryMode::Paginated;
|
||||
async fn default_turn_context_assigns_missing_response_item_ids() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let response_item = user_message("hello");
|
||||
|
||||
let items = session.prepare_conversation_items_for_history(
|
||||
@@ -516,7 +516,6 @@ fn test_model_client_session() -> crate::client::ModelClientSession {
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*attestation_provider*/ None,
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
@@ -1844,8 +1843,10 @@ async fn record_initial_history_reconstructs_resumed_transcript() {
|
||||
#[tokio::test]
|
||||
async fn record_conversation_items_stamps_missing_turn_id_and_preserves_existing_turn_id() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let fresh_item = user_message("fresh");
|
||||
let mut fresh_item = user_message("fresh");
|
||||
fresh_item.set_id(Some(ResponseItemId::with_suffix("msg", "fresh")));
|
||||
let mut existing_item = assistant_message("existing");
|
||||
existing_item.set_id(Some(ResponseItemId::with_suffix("msg", "existing")));
|
||||
existing_item.set_turn_id_if_missing("older-turn");
|
||||
|
||||
session
|
||||
@@ -1918,8 +1919,8 @@ async fn record_inter_agent_communication_sets_turn_id_in_rollout_and_resume() {
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
session.clone_history().await.raw_items(),
|
||||
std::slice::from_ref(&expected_item)
|
||||
strip_response_item_ids(session.clone_history().await.raw_items()),
|
||||
strip_response_item_ids(std::slice::from_ref(&expected_item))
|
||||
);
|
||||
|
||||
session.flush_rollout().await.expect("rollout should flush");
|
||||
@@ -1949,8 +1950,8 @@ async fn record_inter_agent_communication_sets_turn_id_in_rollout_and_resume() {
|
||||
RolloutItem::ResponseItem(expected_item.clone()),
|
||||
];
|
||||
assert_eq!(
|
||||
serde_json::to_value(persisted_items).unwrap(),
|
||||
serde_json::to_value(expected_persisted_items).unwrap()
|
||||
strip_response_item_ids_from_json(serde_json::to_value(persisted_items).unwrap()),
|
||||
strip_response_item_ids_from_json(serde_json::to_value(expected_persisted_items).unwrap())
|
||||
);
|
||||
|
||||
let (resumed_session, _resumed_turn_context) = make_session_and_context().await;
|
||||
@@ -1958,8 +1959,8 @@ async fn record_inter_agent_communication_sets_turn_id_in_rollout_and_resume() {
|
||||
.record_initial_history(InitialHistory::Resumed(resumed))
|
||||
.await;
|
||||
assert_eq!(
|
||||
resumed_session.clone_history().await.raw_items(),
|
||||
std::slice::from_ref(&expected_item)
|
||||
strip_response_item_ids(resumed_session.clone_history().await.raw_items()),
|
||||
strip_response_item_ids(std::slice::from_ref(&expected_item))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1968,9 +1969,7 @@ async fn record_inter_agent_communication_preserves_item_id_in_rollout_and_resum
|
||||
let (mut session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
Vec::new(),
|
||||
|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
},
|
||||
|_| {},
|
||||
)
|
||||
.await;
|
||||
let rollout_path =
|
||||
@@ -2017,9 +2016,7 @@ async fn record_inter_agent_communication_preserves_item_id_in_rollout_and_resum
|
||||
make_session_and_context_with_auth_and_config_and_rx(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
Vec::new(),
|
||||
|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
},
|
||||
|_| {},
|
||||
)
|
||||
.await;
|
||||
resumed_session
|
||||
@@ -2040,9 +2037,7 @@ async fn prepares_image_failures_before_history_insertion() {
|
||||
let (session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
Vec::new(),
|
||||
|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
},
|
||||
|_| {},
|
||||
)
|
||||
.await;
|
||||
let item = ResponseItem::FunctionCallOutput {
|
||||
@@ -2274,7 +2269,10 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only()
|
||||
let initial_context = build_initial_context(&session, &turn_context).await;
|
||||
expected.extend(initial_context);
|
||||
let history_after_seed = session.clone_history().await;
|
||||
assert_eq!(expected, history_after_seed.raw_items());
|
||||
assert_eq!(
|
||||
strip_response_item_ids(&expected),
|
||||
strip_response_item_ids(history_after_seed.raw_items())
|
||||
);
|
||||
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&step_context)
|
||||
@@ -2880,7 +2878,10 @@ async fn record_initial_history_reconstructs_forked_transcript() {
|
||||
.await;
|
||||
|
||||
let history = session.state.lock().await.clone_history();
|
||||
assert_eq!(expected, history.raw_items());
|
||||
assert_eq!(
|
||||
strip_response_item_ids(&expected),
|
||||
strip_response_item_ids(history.raw_items())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -2888,9 +2889,7 @@ async fn start_new_context_window_assigns_and_persists_item_ids() {
|
||||
let (mut session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
Vec::new(),
|
||||
|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
},
|
||||
|_| {},
|
||||
)
|
||||
.await;
|
||||
let rollout_path =
|
||||
@@ -2941,9 +2940,7 @@ async fn record_initial_history_assigns_and_persists_id_for_forked_response_item
|
||||
let (mut session, _turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
Vec::new(),
|
||||
|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
},
|
||||
|_| {},
|
||||
)
|
||||
.await;
|
||||
let rollout_path =
|
||||
@@ -3329,6 +3326,7 @@ async fn thread_rollback_fails_without_persisted_thread_history() {
|
||||
let initial_context = build_initial_context(&sess, &tc).await;
|
||||
sess.record_conversation_items(tc.as_ref(), &initial_context)
|
||||
.await;
|
||||
let history_before_rollback = sess.clone_history().await;
|
||||
|
||||
handlers::thread_rollback(&sess, "sub-1".to_string(), /*num_turns*/ 1).await;
|
||||
|
||||
@@ -3341,7 +3339,10 @@ async fn thread_rollback_fails_without_persisted_thread_history() {
|
||||
error_event.codex_error_info,
|
||||
Some(CodexErrorInfo::ThreadRollbackFailed)
|
||||
);
|
||||
assert_eq!(sess.clone_history().await.raw_items(), initial_context);
|
||||
assert_eq!(
|
||||
sess.clone_history().await.raw_items(),
|
||||
history_before_rollback.raw_items()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -3755,6 +3756,7 @@ async fn thread_rollback_fails_when_turn_in_progress() {
|
||||
let initial_context = build_initial_context(&sess, &tc).await;
|
||||
sess.record_conversation_items(tc.as_ref(), &initial_context)
|
||||
.await;
|
||||
let history_before_rollback = sess.clone_history().await;
|
||||
|
||||
*sess.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
|
||||
handlers::thread_rollback(&sess, "sub-1".to_string(), /*num_turns*/ 1).await;
|
||||
@@ -3766,7 +3768,7 @@ async fn thread_rollback_fails_when_turn_in_progress() {
|
||||
);
|
||||
|
||||
let history = sess.clone_history().await;
|
||||
assert_eq!(initial_context, history.raw_items());
|
||||
assert_eq!(history_before_rollback.raw_items(), history.raw_items());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -3776,6 +3778,7 @@ async fn thread_rollback_fails_when_num_turns_is_zero() {
|
||||
let initial_context = build_initial_context(&sess, &tc).await;
|
||||
sess.record_conversation_items(tc.as_ref(), &initial_context)
|
||||
.await;
|
||||
let history_before_rollback = sess.clone_history().await;
|
||||
|
||||
handlers::thread_rollback(&sess, "sub-1".to_string(), /*num_turns*/ 0).await;
|
||||
|
||||
@@ -3787,7 +3790,7 @@ async fn thread_rollback_fails_when_num_turns_is_zero() {
|
||||
);
|
||||
|
||||
let history = sess.clone_history().await;
|
||||
assert_eq!(initial_context, history.raw_items());
|
||||
assert_eq!(history_before_rollback.raw_items(), history.raw_items());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -5465,7 +5468,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
config.features.enabled(Feature::EnableRequestCompression),
|
||||
config.features.enabled(Feature::RuntimeMetrics),
|
||||
Session::build_model_client_beta_features_header(config.as_ref()),
|
||||
/*item_ids_enabled*/ config.features.enabled(Feature::ItemIds),
|
||||
/*concurrent_reasoning_summaries_enabled*/
|
||||
config
|
||||
.features
|
||||
@@ -7625,7 +7627,6 @@ where
|
||||
config.features.enabled(Feature::EnableRequestCompression),
|
||||
config.features.enabled(Feature::RuntimeMetrics),
|
||||
Session::build_model_client_beta_features_header(config.as_ref()),
|
||||
/*item_ids_enabled*/ config.features.enabled(Feature::ItemIds),
|
||||
/*concurrent_reasoning_summaries_enabled*/
|
||||
config
|
||||
.features
|
||||
@@ -8946,7 +8947,10 @@ async fn record_context_updates_and_set_reference_context_item_injects_full_cont
|
||||
.await;
|
||||
let history = session.clone_history().await;
|
||||
let initial_context = build_initial_context(&session, &turn_context).await;
|
||||
assert_eq!(history.raw_items().to_vec(), initial_context);
|
||||
assert_eq!(
|
||||
strip_response_item_ids(history.raw_items()),
|
||||
strip_response_item_ids(&initial_context)
|
||||
);
|
||||
|
||||
let current_context = session.reference_context_item().await;
|
||||
assert_eq!(
|
||||
@@ -8996,7 +9000,10 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co
|
||||
let mut expected_history = vec![compacted_summary];
|
||||
let initial_context = build_initial_context(&session, &turn_context).await;
|
||||
expected_history.extend(initial_context);
|
||||
assert_eq!(history.raw_items().to_vec(), expected_history);
|
||||
assert_eq!(
|
||||
strip_response_item_ids(history.raw_items()),
|
||||
strip_response_item_ids(&expected_history)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -9748,7 +9755,8 @@ async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input()
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
};
|
||||
assert!(
|
||||
strip_metadata_from_items(history.raw_items()).contains(&expected),
|
||||
strip_response_item_ids(&strip_metadata_from_items(history.raw_items()))
|
||||
.contains(&expected),
|
||||
"expected pending input to be persisted into history on turn completion"
|
||||
);
|
||||
|
||||
|
||||
@@ -1930,7 +1930,9 @@ fn assign_missing_streamed_response_item_id(
|
||||
return;
|
||||
}
|
||||
|
||||
let active_item_id = active_item.map(|item| ResponseItemId::from_server(item.id()));
|
||||
let active_item_id = active_item
|
||||
.map(|item| ResponseItemId::from_server(item.id()))
|
||||
.filter(|item_id| !item_id.is_empty());
|
||||
item.set_id(active_item_id);
|
||||
Session::assign_missing_response_item_id(item);
|
||||
}
|
||||
@@ -2051,9 +2053,7 @@ async fn try_run_sampling_request(
|
||||
match event {
|
||||
ResponseEvent::Created => {}
|
||||
ResponseEvent::OutputItemDone(mut item) => {
|
||||
if turn_context.item_ids_enabled() {
|
||||
assign_missing_streamed_response_item_id(&mut item, active_item.as_ref());
|
||||
}
|
||||
assign_missing_streamed_response_item_id(&mut item, active_item.as_ref());
|
||||
if let Some((_, mut consumer)) = active_tool_argument_diff_consumer.take()
|
||||
&& let Ok(Some(event)) = consumer.finish()
|
||||
{
|
||||
@@ -2148,9 +2148,7 @@ async fn try_run_sampling_request(
|
||||
}
|
||||
}
|
||||
ResponseEvent::OutputItemAdded(mut item) => {
|
||||
if turn_context.item_ids_enabled() {
|
||||
assign_missing_streamed_response_item_id(&mut item, /*active_item*/ None);
|
||||
}
|
||||
assign_missing_streamed_response_item_id(&mut item, /*active_item*/ None);
|
||||
if let ResponseItem::CustomToolCall {
|
||||
call_id,
|
||||
name,
|
||||
|
||||
@@ -162,11 +162,6 @@ enum TurnMultiAgentRuntime {
|
||||
}
|
||||
|
||||
impl TurnContext {
|
||||
pub(crate) fn item_ids_enabled(&self) -> bool {
|
||||
self.config.features.enabled(Feature::ItemIds)
|
||||
|| matches!(self.history_mode, ThreadHistoryMode::Paginated)
|
||||
}
|
||||
|
||||
pub(crate) fn collaboration_mode(&self) -> CollaborationMode {
|
||||
CollaborationMode {
|
||||
mode: self.mode,
|
||||
|
||||
@@ -31,6 +31,7 @@ use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::PathExt;
|
||||
use core_test_support::responses::mount_models_once;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::time::Duration;
|
||||
use tempfile::tempdir;
|
||||
@@ -1782,8 +1783,9 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor
|
||||
rollout_items
|
||||
.iter()
|
||||
.filter(|item| {
|
||||
serde_json::to_value(item).expect("serialize rollout item")
|
||||
== interrupted_marker_json
|
||||
strip_response_item_ids_from_json(
|
||||
serde_json::to_value(item).expect("serialize rollout item"),
|
||||
) == interrupted_marker_json
|
||||
})
|
||||
.count(),
|
||||
1,
|
||||
@@ -1986,8 +1988,9 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_
|
||||
forked_rollout_items
|
||||
.iter()
|
||||
.filter(|item| {
|
||||
serde_json::to_value(item).expect("serialize forked rollout item")
|
||||
== interrupted_marker_json
|
||||
strip_response_item_ids_from_json(
|
||||
serde_json::to_value(item).expect("serialize forked rollout item"),
|
||||
) == interrupted_marker_json
|
||||
})
|
||||
.count(),
|
||||
1,
|
||||
@@ -2021,8 +2024,9 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_
|
||||
reforked_rollout_items
|
||||
.iter()
|
||||
.filter(|item| {
|
||||
serde_json::to_value(item).expect("serialize re-forked rollout item")
|
||||
== interrupted_marker_json
|
||||
strip_response_item_ids_from_json(
|
||||
serde_json::to_value(item).expect("serialize re-forked rollout item"),
|
||||
) == interrupted_marker_json
|
||||
})
|
||||
.count(),
|
||||
1,
|
||||
|
||||
@@ -183,6 +183,8 @@ mod tests {
|
||||
use codex_tools::ExtensionTurnItem;
|
||||
use codex_utils_absolute_path::test_support::PathExt;
|
||||
use codex_utils_absolute_path::test_support::test_path_buf;
|
||||
use core_test_support::responses::strip_response_item_id;
|
||||
use core_test_support::responses::strip_response_item_ids;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use tokio::sync::Mutex;
|
||||
@@ -357,7 +359,10 @@ mod tests {
|
||||
let EventMsg::RawResponseItem(raw_history_item) = raw_history_event.msg else {
|
||||
panic!("expected raw response item event");
|
||||
};
|
||||
assert_eq!(raw_history_item.item, expected_history_item);
|
||||
assert_eq!(
|
||||
strip_response_item_id(raw_history_item.item),
|
||||
expected_history_item
|
||||
);
|
||||
let step_context = StepContext::for_test(Arc::clone(&turn));
|
||||
let invocation = ToolInvocation {
|
||||
session,
|
||||
@@ -397,8 +402,8 @@ mod tests {
|
||||
expected_sandbox_cwds
|
||||
);
|
||||
assert_eq!(
|
||||
captured_call.conversation_history.items(),
|
||||
std::slice::from_ref(&expected_history_item)
|
||||
strip_response_item_ids(captured_call.conversation_history.items()),
|
||||
vec![expected_history_item]
|
||||
);
|
||||
match captured_call.payload {
|
||||
ToolPayload::Function { arguments } => {
|
||||
|
||||
@@ -26,6 +26,7 @@ use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_tools::default_namespace_description;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
@@ -439,13 +440,13 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow
|
||||
let value: serde_json::Value =
|
||||
serde_json::from_str(&text).expect("extension tool output should be json");
|
||||
assert_eq!(
|
||||
value,
|
||||
json!({
|
||||
strip_response_item_ids_from_json(value),
|
||||
strip_response_item_ids_from_json(json!({
|
||||
"arguments": { "message": "hello" },
|
||||
"callId": "call-extension",
|
||||
"conversationHistory": [expected_history_item],
|
||||
"ok": true,
|
||||
})
|
||||
}))
|
||||
);
|
||||
}
|
||||
other => panic!("expected function call output, got {other:?}"),
|
||||
|
||||
@@ -5,6 +5,7 @@ use similar::TextDiff;
|
||||
use std::sync::OnceLock;
|
||||
|
||||
use crate::responses::ResponsesRequest;
|
||||
use crate::responses::strip_response_item_ids_from_json;
|
||||
use codex_protocol::protocol::APPS_INSTRUCTIONS_OPEN_TAG;
|
||||
use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG;
|
||||
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
|
||||
@@ -25,6 +26,7 @@ pub struct ContextSnapshotOptions {
|
||||
render_mode: ContextSnapshotRenderMode,
|
||||
strip_capability_instructions: bool,
|
||||
strip_agents_md_user_context: bool,
|
||||
strip_response_item_ids: bool,
|
||||
}
|
||||
|
||||
impl Default for ContextSnapshotOptions {
|
||||
@@ -33,6 +35,7 @@ impl Default for ContextSnapshotOptions {
|
||||
render_mode: ContextSnapshotRenderMode::RedactedText,
|
||||
strip_capability_instructions: false,
|
||||
strip_agents_md_user_context: false,
|
||||
strip_response_item_ids: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -52,6 +55,11 @@ impl ContextSnapshotOptions {
|
||||
self.strip_agents_md_user_context = true;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn strip_response_item_ids(mut self) -> Self {
|
||||
self.strip_response_item_ids = true;
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
pub fn format_request_input_snapshot(
|
||||
@@ -267,6 +275,11 @@ fn format_request_body_snapshot(
|
||||
options: &ContextSnapshotOptions,
|
||||
) -> String {
|
||||
let mut body = crate::responses::strip_metadata_from_json(request.body_json());
|
||||
if options.strip_response_item_ids
|
||||
&& let Some(input) = body.get_mut("input")
|
||||
{
|
||||
*input = strip_response_item_ids_from_json(std::mem::take(input));
|
||||
}
|
||||
canonicalize_json_snapshot_value(&mut body, options);
|
||||
serde_json::to_string_pretty(&body).expect("request body should serialize")
|
||||
}
|
||||
|
||||
@@ -109,6 +109,43 @@ pub fn strip_metadata_from_items(items: &[ResponseItem]) -> Vec<ResponseItem> {
|
||||
items.iter().cloned().map(strip_metadata).collect()
|
||||
}
|
||||
|
||||
/// Returns a response item without its Responses API item ID for semantic assertions.
|
||||
pub fn strip_response_item_id(mut item: ResponseItem) -> ResponseItem {
|
||||
item.set_id(/*new_id*/ None);
|
||||
item
|
||||
}
|
||||
|
||||
/// Returns response items without their Responses API item IDs for semantic assertions.
|
||||
pub fn strip_response_item_ids(items: &[ResponseItem]) -> Vec<ResponseItem> {
|
||||
items.iter().cloned().map(strip_response_item_id).collect()
|
||||
}
|
||||
|
||||
/// Returns JSON without IDs on recognized Responses API items.
|
||||
pub fn strip_response_item_ids_from_json(value: Value) -> Value {
|
||||
match value {
|
||||
Value::Array(values) => Value::Array(
|
||||
values
|
||||
.into_iter()
|
||||
.map(strip_response_item_ids_from_json)
|
||||
.collect(),
|
||||
),
|
||||
Value::Object(mut map) => {
|
||||
let is_response_item =
|
||||
serde_json::from_value::<ResponseItem>(Value::Object(map.clone()))
|
||||
.is_ok_and(|item| !matches!(item, ResponseItem::Other));
|
||||
if is_response_item {
|
||||
map.remove("id");
|
||||
}
|
||||
Value::Object(
|
||||
map.into_iter()
|
||||
.map(|(key, value)| (key, strip_response_item_ids_from_json(value)))
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
value => value,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns JSON without internal transport metadata for semantic assertions.
|
||||
pub fn strip_metadata_from_json(value: Value) -> Value {
|
||||
match value {
|
||||
|
||||
@@ -130,7 +130,6 @@ async fn responses_stream_includes_subagent_header_on_review() {
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*attestation_provider*/ None,
|
||||
config.http_client_factory(),
|
||||
@@ -266,7 +265,6 @@ async fn responses_stream_includes_subagent_header_on_other() {
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*attestation_provider*/ None,
|
||||
config.http_client_factory(),
|
||||
@@ -387,7 +385,6 @@ async fn responses_respects_model_info_overrides_from_config() {
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*attestation_provider*/ None,
|
||||
config.http_client_factory(),
|
||||
|
||||
@@ -257,6 +257,7 @@ async fn websocket_v2_test_codex_shell_chain() -> Result<()> {
|
||||
|
||||
let call_id = "shell-command-call";
|
||||
let mut shell_command_call = ev_shell_command_call(call_id, "echo websocket");
|
||||
shell_command_call["item"]["id"] = serde_json::json!("fc_shell_command_call");
|
||||
shell_command_call["item"]["internal_chat_message_metadata_passthrough"] =
|
||||
serde_json::json!({"turn_id": "turn-123"});
|
||||
let server = start_websocket_server(vec![vec![
|
||||
|
||||
@@ -71,6 +71,7 @@ use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::sse_failed;
|
||||
use core_test_support::responses::strip_metadata_from_json;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use core_test_support::responses_metadata as test_responses_metadata;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
@@ -250,7 +251,7 @@ async fn openai_stateless_responses_requests_preserve_item_turn_metadata_across_
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn non_openai_responses_requests_omit_item_passthrough_metadata() {
|
||||
async fn non_openai_responses_requests_include_item_ids_without_passthrough_metadata() {
|
||||
let server = MockServer::start().await;
|
||||
let response_mock = mount_sse_once(
|
||||
&server,
|
||||
@@ -299,8 +300,8 @@ async fn non_openai_responses_requests_omit_item_passthrough_metadata() {
|
||||
"input item should omit internal chat message metadata passthrough: {item}"
|
||||
);
|
||||
assert!(
|
||||
item.get("id").is_none(),
|
||||
"input item should omit generated IDs: {item}"
|
||||
item.get("id").and_then(serde_json::Value::as_str).is_some(),
|
||||
"input item should include a generated ID: {item}"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -433,9 +434,7 @@ async fn response_item_ids_persist_across_resume_and_preserve_server_ids() -> an
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
});
|
||||
let mut builder = test_codex();
|
||||
let initial = builder.build(&server).await?;
|
||||
let home = Arc::clone(&initial.home);
|
||||
let rollout_path = initial
|
||||
@@ -451,9 +450,6 @@ async fn response_item_ids_persist_across_resume_and_preserve_server_ids() -> an
|
||||
})
|
||||
.await;
|
||||
|
||||
builder = builder.with_config(|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
});
|
||||
let resumed = builder.resume(&server, home, rollout_path).await?;
|
||||
resumed.submit_turn("after resume").await?;
|
||||
|
||||
@@ -532,9 +528,7 @@ async fn synthetic_call_output_id_is_stable_across_resumes() -> anyhow::Result<(
|
||||
)
|
||||
.await;
|
||||
let codex_home = Arc::new(TempDir::new()?);
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
});
|
||||
let mut builder = test_codex();
|
||||
let first = builder
|
||||
.resume(&server, Arc::clone(&codex_home), session_path.clone())
|
||||
.await?;
|
||||
@@ -550,9 +544,6 @@ async fn synthetic_call_output_id_is_stable_across_resumes() -> anyhow::Result<(
|
||||
"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?;
|
||||
|
||||
@@ -606,7 +597,6 @@ async fn response_item_ids_are_sent_for_all_remote_v2_compaction_requests() -> a
|
||||
let test = test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
let _ = config.features.enable(Feature::RemoteCompactionV2);
|
||||
})
|
||||
.build(&server)
|
||||
@@ -1503,7 +1493,6 @@ async fn send_request_with_provider(provider: ModelProviderInfo) {
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ config.features.enabled(Feature::ItemIds),
|
||||
/*concurrent_reasoning_summaries_enabled*/
|
||||
config
|
||||
.features
|
||||
@@ -3236,7 +3225,6 @@ async fn azure_responses_request_includes_store_and_prefixed_item_ids() {
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*attestation_provider*/ None,
|
||||
config.http_client_factory(),
|
||||
@@ -4107,7 +4095,9 @@ async fn history_dedupes_streamed_and_final_messages_across_turns() {
|
||||
let tail_len = r3_tail_expected.as_array().unwrap().len();
|
||||
let actual_tail = &r3_input_array[r3_input_array.len() - tail_len..];
|
||||
assert_eq!(
|
||||
strip_metadata_from_json(serde_json::Value::Array(actual_tail.to_vec())),
|
||||
strip_response_item_ids_from_json(strip_metadata_from_json(serde_json::Value::Array(
|
||||
actual_tail.to_vec(),
|
||||
))),
|
||||
r3_tail_expected,
|
||||
"request 3 tail mismatch",
|
||||
);
|
||||
|
||||
@@ -161,8 +161,7 @@ async fn responses_websocket_streams_request() {
|
||||
|
||||
let harness = websocket_harness(&server).await;
|
||||
let mut client_session = harness.client.new_session();
|
||||
let mut prompt = prompt_with_input(vec![message_item("hello")]);
|
||||
prompt.input[0].set_id(Some(ResponseItemId::with_suffix("msg", "existing")));
|
||||
let prompt = prompt_with_input(vec![message_item("hello")]);
|
||||
|
||||
stream_until_complete(&mut client_session, &harness, &prompt).await;
|
||||
|
||||
@@ -174,7 +173,6 @@ async fn responses_websocket_streams_request() {
|
||||
assert_eq!(body["model"].as_str(), Some(MODEL));
|
||||
assert_eq!(body["stream"], serde_json::Value::Bool(true));
|
||||
assert_eq!(body["input"].as_array().map(Vec::len), Some(1));
|
||||
assert_eq!(body["input"][0].get("id"), None);
|
||||
let handshake = server.single_handshake();
|
||||
assert_eq!(
|
||||
handshake.header(OPENAI_BETA_HEADER),
|
||||
@@ -225,7 +223,7 @@ async fn responses_websocket_omits_unprefixed_item_ids_without_mutating_prompt()
|
||||
websocket_provider(&server),
|
||||
/*runtime_metrics_enabled*/ false,
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*enabled_features*/ &[Feature::ItemIds],
|
||||
/*enabled_features*/ &[],
|
||||
)
|
||||
.await;
|
||||
let mut client_session = harness.client.new_session();
|
||||
@@ -2389,7 +2387,6 @@ async fn websocket_harness_with_provider_options(
|
||||
/*enable_request_compression*/ false,
|
||||
runtime_metrics_enabled,
|
||||
/*beta_features_header*/ None,
|
||||
/*item_ids_enabled*/ config.features.enabled(Feature::ItemIds),
|
||||
/*concurrent_reasoning_summaries_enabled*/
|
||||
config
|
||||
.features
|
||||
|
||||
@@ -34,6 +34,7 @@ use core_test_support::hooks::trust_discovered_hooks;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::ev_reasoning_item;
|
||||
use core_test_support::responses::mount_models_once;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::local_selections;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
@@ -1225,6 +1226,7 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() {
|
||||
values
|
||||
.iter()
|
||||
.filter_map(|value| {
|
||||
let value = strip_response_item_ids_from_json(value.clone());
|
||||
if value
|
||||
.get("type")
|
||||
.and_then(|ty| ty.as_str())
|
||||
@@ -1251,9 +1253,9 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() {
|
||||
return None;
|
||||
}
|
||||
if role == Some("user") {
|
||||
return strip_agents_parts_from_user_message(value);
|
||||
return strip_agents_parts_from_user_message(&value);
|
||||
}
|
||||
Some(value.clone())
|
||||
Some(value)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -4078,7 +4080,8 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.model_provider = model_provider;
|
||||
set_test_compact_prompt(config);
|
||||
config.model_auto_compact_token_limit = Some(200);
|
||||
// Leave enough headroom for per-item request metadata before the second compaction.
|
||||
config.model_auto_compact_token_limit = Some(300);
|
||||
});
|
||||
let codex = builder.build(&server).await.unwrap().codex;
|
||||
|
||||
|
||||
@@ -872,7 +872,7 @@ async fn assert_remote_manual_compact_request_parity(
|
||||
&normal_request,
|
||||
"Remote /responses/compact Request",
|
||||
&compact_request,
|
||||
&ContextSnapshotOptions::default(),
|
||||
&ContextSnapshotOptions::default().strip_response_item_ids(),
|
||||
)
|
||||
);
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::hooks::trust_discovered_hooks;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::ResponseMock;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodexHarness;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
@@ -762,17 +763,17 @@ fn compact_request_view(body: &Value, mode: Mode) -> Value {
|
||||
}
|
||||
|
||||
let mut selected = selected_request_fields(body, SelectedFieldsMode::Compact);
|
||||
selected["input"] = normalize_value(Value::Array(input));
|
||||
selected["input"] = normalize_value(strip_response_item_ids_from_json(Value::Array(input)));
|
||||
canonical_json(&normalize_value(selected))
|
||||
}
|
||||
|
||||
fn follow_up_request_view(body: &Value) -> Value {
|
||||
let mut selected = selected_request_fields(body, SelectedFieldsMode::FollowUp);
|
||||
selected["input"] = normalize_value(
|
||||
selected["input"] = normalize_value(strip_response_item_ids_from_json(
|
||||
body.get("input")
|
||||
.cloned()
|
||||
.expect("follow-up request should include input"),
|
||||
);
|
||||
));
|
||||
canonical_json(&normalize_value(selected))
|
||||
}
|
||||
|
||||
@@ -800,7 +801,9 @@ fn replacement_history_from_rollout(path: &Path) -> Result<Value> {
|
||||
}
|
||||
let replacement_history =
|
||||
replacement_history.expect("expected compacted rollout replacement history");
|
||||
Ok(canonical_json(&normalize_value(replacement_history)))
|
||||
Ok(canonical_json(&normalize_value(
|
||||
strip_response_item_ids_from_json(replacement_history),
|
||||
)))
|
||||
}
|
||||
|
||||
fn write_manual_compact_hooks(home: &Path) {
|
||||
|
||||
@@ -16,6 +16,8 @@ use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::responses::strip_metadata;
|
||||
use core_test_support::responses::strip_response_item_id;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::local_selections;
|
||||
@@ -181,7 +183,7 @@ async fn copy_paste_local_image_persists_rollout_request_shape() -> anyhow::Resu
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
};
|
||||
|
||||
assert_eq!(responses::strip_metadata(actual), expected);
|
||||
assert_eq!(strip_response_item_id(strip_metadata(actual)), expected);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -272,7 +274,7 @@ async fn drag_drop_image_persists_rollout_request_shape() -> anyhow::Result<()>
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
};
|
||||
|
||||
assert_eq!(responses::strip_metadata(actual), expected);
|
||||
assert_eq!(strip_response_item_id(strip_metadata(actual)), expected);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -253,12 +253,7 @@ async fn missing_streamed_reasoning_id_is_reused_for_completion() -> anyhow::Res
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let TestCodex { codex, .. } = test_codex()
|
||||
.with_config(|config| {
|
||||
let _ = config.features.enable(Feature::ItemIds);
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
let TestCodex { codex, .. } = test_codex().build_with_auto_env(&server).await?;
|
||||
|
||||
let mut reasoning_added = ev_reasoning_item_added("unused", &[]);
|
||||
reasoning_added["item"]
|
||||
|
||||
@@ -681,11 +681,6 @@ async fn generated_image_is_replayed_for_image_capable_models() -> Result<()> {
|
||||
1,
|
||||
"expected generated image history to be replayed as an image_generation_call"
|
||||
);
|
||||
assert_eq!(
|
||||
image_generation_calls[0]["id"].as_str(),
|
||||
None,
|
||||
"expected the image generation call id to be omitted"
|
||||
);
|
||||
assert_eq!(
|
||||
image_generation_calls[0]["result"].as_str(),
|
||||
Some("Zm9v"),
|
||||
@@ -786,11 +781,6 @@ async fn model_change_from_generated_image_to_text_preserves_prior_generated_ima
|
||||
image_generation_calls.len() == 1,
|
||||
"second request should preserve the generated image call for text-only models"
|
||||
);
|
||||
assert_eq!(
|
||||
image_generation_calls[0]["id"].as_str(),
|
||||
None,
|
||||
"second request should omit the generated image call id"
|
||||
);
|
||||
assert_eq!(
|
||||
image_generation_calls[0]["result"].as_str(),
|
||||
Some(""),
|
||||
|
||||
@@ -26,6 +26,7 @@ use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::responses::strip_metadata_from_json;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::local_selections;
|
||||
@@ -55,10 +56,10 @@ fn text_user_input_parts(texts: Vec<String>) -> serde_json::Value {
|
||||
})
|
||||
}
|
||||
|
||||
fn assert_eq_without_metadata(left: serde_json::Value, right: serde_json::Value) {
|
||||
fn assert_eq_without_metadata_or_item_ids(left: serde_json::Value, right: serde_json::Value) {
|
||||
assert_eq!(
|
||||
strip_metadata_from_json(left),
|
||||
strip_metadata_from_json(right)
|
||||
strip_response_item_ids_from_json(strip_metadata_from_json(left)),
|
||||
strip_response_item_ids_from_json(strip_metadata_from_json(right))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -392,15 +393,18 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests
|
||||
Some("input_text"),
|
||||
"expected environment context bundled after UI message in cached contextual message"
|
||||
);
|
||||
assert_eq_without_metadata(input1[2].clone(), text_user_input("hello 1".to_string()));
|
||||
assert_eq_without_metadata_or_item_ids(
|
||||
input1[2].clone(),
|
||||
text_user_input("hello 1".to_string()),
|
||||
);
|
||||
|
||||
let body2 = req2.single_request().body_json();
|
||||
let input2 = body2["input"].as_array().expect("input array");
|
||||
assert_eq_without_metadata(
|
||||
assert_eq_without_metadata_or_item_ids(
|
||||
serde_json::Value::Array(input2[..input1.len()].to_vec()),
|
||||
serde_json::Value::Array(input1.to_vec()),
|
||||
);
|
||||
assert_eq_without_metadata(
|
||||
assert_eq_without_metadata_or_item_ids(
|
||||
input2[input1.len()].clone(),
|
||||
text_user_input("hello 2".to_string()),
|
||||
);
|
||||
@@ -533,7 +537,7 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an
|
||||
expected_body2.push(expected_permissions_msg_2);
|
||||
expected_body2.push(expected_env_msg_2);
|
||||
expected_body2.push(expected_user_message_2);
|
||||
assert_eq_without_metadata(
|
||||
assert_eq_without_metadata_or_item_ids(
|
||||
body2["input"].clone(),
|
||||
serde_json::Value::Array(expected_body2),
|
||||
);
|
||||
@@ -823,7 +827,7 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Res
|
||||
expected_body2.push(expected_permissions_update_msg);
|
||||
expected_body2.push(expected_env_msg_2);
|
||||
expected_body2.push(expected_user_message_2);
|
||||
assert_eq_without_metadata(
|
||||
assert_eq_without_metadata_or_item_ids(
|
||||
body2["input"].clone(),
|
||||
serde_json::Value::Array(expected_body2),
|
||||
);
|
||||
@@ -955,7 +959,7 @@ async fn send_user_turn_with_no_changes_does_not_send_environment_context() -> a
|
||||
expected_contextual_user_msg_1.clone(),
|
||||
expected_user_message_1.clone(),
|
||||
]);
|
||||
assert_eq_without_metadata(body1["input"].clone(), expected_input_1);
|
||||
assert_eq_without_metadata_or_item_ids(body1["input"].clone(), expected_input_1);
|
||||
|
||||
let expected_user_message_2 = text_user_input("hello 2".to_string());
|
||||
let expected_input_2 = serde_json::Value::Array(vec![
|
||||
@@ -964,7 +968,7 @@ async fn send_user_turn_with_no_changes_does_not_send_environment_context() -> a
|
||||
expected_user_message_1,
|
||||
expected_user_message_2,
|
||||
]);
|
||||
assert_eq_without_metadata(body2["input"].clone(), expected_input_2);
|
||||
assert_eq_without_metadata_or_item_ids(body2["input"].clone(), expected_input_2);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -1094,7 +1098,7 @@ async fn send_user_turn_with_changes_sends_environment_context() -> anyhow::Resu
|
||||
expected_contextual_user_msg_1.clone(),
|
||||
expected_user_message_1.clone(),
|
||||
]);
|
||||
assert_eq_without_metadata(body1["input"].clone(), expected_input_1);
|
||||
assert_eq_without_metadata_or_item_ids(body1["input"].clone(), expected_input_1);
|
||||
|
||||
let body1_input = body1["input"].as_array().expect("input array");
|
||||
let expected_settings_update_msg = body2["input"][body1_input.len()].clone();
|
||||
@@ -1139,7 +1143,7 @@ async fn send_user_turn_with_changes_sends_environment_context() -> anyhow::Resu
|
||||
expected_env_update_msg,
|
||||
expected_user_message_2,
|
||||
]);
|
||||
assert_eq_without_metadata(body2["input"].clone(), expected_input_2);
|
||||
assert_eq_without_metadata_or_item_ids(body2["input"].clone(), expected_input_2);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses::strip_metadata;
|
||||
use core_test_support::responses::strip_response_item_id;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
@@ -53,7 +54,11 @@ async fn build_prompt_input_includes_context_and_user_message() -> Result<()> {
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
};
|
||||
assert_eq!(
|
||||
input.last().cloned().map(strip_metadata),
|
||||
input
|
||||
.last()
|
||||
.cloned()
|
||||
.map(strip_metadata)
|
||||
.map(strip_response_item_id),
|
||||
Some(expected_user_message)
|
||||
);
|
||||
assert!(input.iter().any(|item| {
|
||||
|
||||
@@ -31,6 +31,7 @@ use core_test_support::responses::sse;
|
||||
use core_test_support::responses::sse_response;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::responses::strip_metadata_from_json;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::local_selections;
|
||||
@@ -1435,7 +1436,9 @@ async fn encrypted_multi_agent_v2_spawn_sends_agent_message_to_child() -> Result
|
||||
sleep(Duration::from_millis(10)).await;
|
||||
};
|
||||
assert_eq!(
|
||||
strip_metadata_from_json(Value::Array(child_request.inputs_of_type("agent_message"))),
|
||||
strip_response_item_ids_from_json(strip_metadata_from_json(Value::Array(
|
||||
child_request.inputs_of_type("agent_message"),
|
||||
))),
|
||||
Value::Array(vec![json!({
|
||||
"type": "agent_message",
|
||||
"author": "/root",
|
||||
@@ -1623,7 +1626,9 @@ async fn plaintext_multi_agent_v2_completion_sends_agent_message(
|
||||
.pop()
|
||||
.expect("agent message request");
|
||||
assert_eq!(
|
||||
strip_metadata_from_json(Value::Array(request.inputs_of_type("agent_message"))),
|
||||
strip_response_item_ids_from_json(strip_metadata_from_json(Value::Array(
|
||||
request.inputs_of_type("agent_message"),
|
||||
))),
|
||||
Value::Array(vec![json!({
|
||||
"type": "agent_message",
|
||||
"author": "/root/worker",
|
||||
|
||||
@@ -27,6 +27,7 @@ use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_sandbox;
|
||||
use core_test_support::test_codex::local;
|
||||
@@ -226,9 +227,12 @@ async fn namespaced_custom_tool_call_preserves_namespace_through_dispatch_and_re
|
||||
.map(str::to_string)
|
||||
.expect("custom tool call should include turn metadata");
|
||||
assert_eq!(
|
||||
(custom_tool_calls, request.custom_tool_call_output(call_id),),
|
||||
(
|
||||
vec![json!({
|
||||
strip_response_item_ids_from_json(Value::Array(custom_tool_calls)),
|
||||
strip_response_item_ids_from_json(request.custom_tool_call_output(call_id)),
|
||||
),
|
||||
(
|
||||
Value::Array(vec![json!({
|
||||
"type": "custom_tool_call",
|
||||
"call_id": call_id,
|
||||
"namespace": namespace,
|
||||
@@ -237,7 +241,7 @@ async fn namespaced_custom_tool_call_preserves_namespace_through_dispatch_and_re
|
||||
"internal_chat_message_metadata_passthrough": {
|
||||
"turn_id": turn_id,
|
||||
},
|
||||
})],
|
||||
})]),
|
||||
json!({
|
||||
"type": "custom_tool_call_output",
|
||||
"call_id": call_id,
|
||||
|
||||
@@ -206,7 +206,7 @@ pub enum Feature {
|
||||
ImageGeneration,
|
||||
/// Removed compatibility flag for always-on centralized image preparation.
|
||||
ResizeAllImages,
|
||||
/// Generate Responses API item IDs for client-created history items.
|
||||
/// Removed compatibility flag for always-on response item IDs.
|
||||
ItemIds,
|
||||
/// Request sequential cutoff reasoning summary delivery.
|
||||
ConcurrentReasoningSummaries,
|
||||
@@ -483,7 +483,7 @@ impl Features {
|
||||
"tool_search" | "tool_search_always_defer_mcp_tools" | "apps_mcp_path_override" => {
|
||||
continue;
|
||||
}
|
||||
"image_detail_original" | "resize_all_images" => {
|
||||
"image_detail_original" | "resize_all_images" | "item_ids" => {
|
||||
continue;
|
||||
}
|
||||
"plugin_hooks" => {
|
||||
@@ -1198,8 +1198,8 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
FeatureSpec {
|
||||
id: Feature::ItemIds,
|
||||
key: "item_ids",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
stage: Stage::Removed,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ConcurrentReasoningSummaries,
|
||||
|
||||
@@ -274,6 +274,23 @@ fn from_sources_ignores_removed_resize_all_images_feature_key() {
|
||||
assert_eq!(features, Features::with_defaults());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_sources_ignores_removed_item_ids_feature_key() {
|
||||
let features_toml = FeaturesToml::from(BTreeMap::from([("item_ids".to_string(), false)]));
|
||||
|
||||
let features = Features::from_sources(
|
||||
FeatureConfigSource {
|
||||
features: Some(&features_toml),
|
||||
..Default::default()
|
||||
},
|
||||
FeatureConfigSource::default(),
|
||||
FeatureOverrides::default(),
|
||||
);
|
||||
|
||||
assert_eq!(features, Features::with_defaults());
|
||||
assert_eq!(features.enabled(Feature::ItemIds), true);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_sources_ignores_removed_undo_feature_key() {
|
||||
let features_toml = FeaturesToml::from(BTreeMap::from([("undo".to_string(), true)]));
|
||||
|
||||
@@ -260,7 +260,6 @@ impl MemoryStartupContext {
|
||||
config.features.enabled(Feature::EnableRequestCompression),
|
||||
config.features.enabled(Feature::RuntimeMetrics),
|
||||
/*beta_features_header*/ None,
|
||||
config.features.enabled(Feature::ItemIds),
|
||||
/*concurrent_reasoning_summaries_enabled*/ false,
|
||||
/*attestation_provider*/ None,
|
||||
config.http_client_factory(),
|
||||
|
||||
Reference in New Issue
Block a user