mirror of
https://github.com/openai/codex.git
synced 2026-10-26 18:38:47 +00:00
Replace Guardian tickets with parent response IDs (#43002)
## What changed Capture the ID from `response.created` and pass it as `parent_response_id` in Guardian review and classifier request metadata. Clear the current response ID before sampling retries so tool reviews cannot inherit a failed response's ID. Request Guardian credits with `guardian_credits_requested` on eligible parent requests. Remove the ticket protocol, transport attachment, and ticket-specific body redaction. Keep ordinary app-server client `parent_response_id` values inside `x-codex-turn-metadata` rather than promoting them to Guardian request metadata. ## Testing Update coverage for HTTP fallback, WebSocket review reuse, missing response IDs, classifier transport retries, and code-mode yield/resume attribution. Extend app-server tests to verify client metadata placement over HTTP and WebSocket. GitOrigin-RevId: c1665e56382ecec8d260fc2ca0ecd5e20b0b2391
This commit is contained in:
@@ -1104,14 +1104,24 @@ impl ModelClient {
|
||||
&& provider.aws.is_none()
|
||||
}
|
||||
|
||||
fn set_guardian_ticket_request(
|
||||
fn set_guardian_metadata(
|
||||
&self,
|
||||
metadata: &mut Option<HashMap<String, String>>,
|
||||
parent_response_id: Option<&str>,
|
||||
auth: Option<&CodexAuth>,
|
||||
endpoint: ResponsesEndpoint,
|
||||
) {
|
||||
if let Some(metadata) = metadata.as_mut() {
|
||||
metadata.remove("guardian_ticket_requested");
|
||||
metadata.remove("guardian_credits_requested");
|
||||
metadata.remove("parent_response_id");
|
||||
}
|
||||
if endpoint == ResponsesEndpoint::Guardian
|
||||
&& let Some(parent_response_id) = parent_response_id
|
||||
{
|
||||
metadata.get_or_insert_with(HashMap::new).insert(
|
||||
"parent_response_id".to_owned(),
|
||||
parent_response_id.to_owned(),
|
||||
);
|
||||
}
|
||||
if self.free_guardian_enabled
|
||||
&& endpoint == ResponsesEndpoint::Responses
|
||||
@@ -1125,7 +1135,7 @@ impl ModelClient {
|
||||
{
|
||||
metadata
|
||||
.get_or_insert_with(HashMap::new)
|
||||
.insert("guardian_ticket_requested".to_owned(), "true".to_owned());
|
||||
.insert("guardian_credits_requested".to_owned(), "true".to_owned());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1351,7 +1361,6 @@ impl ModelClientSession {
|
||||
},
|
||||
compression,
|
||||
turn_state: Some(Arc::clone(&self.turn_state)),
|
||||
guardian_ticket: responses_metadata.guardian_ticket.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1641,8 +1650,9 @@ impl ModelClientSession {
|
||||
service_tier.clone(),
|
||||
responses_metadata,
|
||||
)?;
|
||||
self.client.set_guardian_ticket_request(
|
||||
self.client.set_guardian_metadata(
|
||||
&mut request.client_metadata,
|
||||
responses_metadata.parent_response_id.as_deref(),
|
||||
client_setup.auth.as_ref(),
|
||||
endpoint,
|
||||
);
|
||||
@@ -1900,8 +1910,9 @@ impl ModelClientSession {
|
||||
),
|
||||
..ResponseCreateWsRequest::from(&request)
|
||||
};
|
||||
self.client.set_guardian_ticket_request(
|
||||
self.client.set_guardian_metadata(
|
||||
&mut ws_payload.client_metadata,
|
||||
responses_metadata.parent_response_id.as_deref(),
|
||||
client_setup.auth.as_ref(),
|
||||
endpoint,
|
||||
);
|
||||
@@ -1922,7 +1933,6 @@ impl ModelClientSession {
|
||||
ws_request,
|
||||
self.websocket_session.connection_reused(),
|
||||
Some(Arc::clone(&self.turn_state)),
|
||||
responses_metadata.guardian_ticket.as_ref(),
|
||||
)
|
||||
.await;
|
||||
if let Some(original_item_ids) = original_item_ids {
|
||||
|
||||
@@ -844,9 +844,7 @@ async fn response_stream_records_last_model_feedback_ids() {
|
||||
.set_default();
|
||||
|
||||
let api_stream = futures::stream::iter([
|
||||
Ok(ResponseEvent::Created {
|
||||
guardian_ticket: None,
|
||||
}),
|
||||
Ok(ResponseEvent::Created { response_id: None }),
|
||||
Ok(ResponseEvent::Completed {
|
||||
response_id: "resp-123".to_string(),
|
||||
token_usage: None,
|
||||
@@ -1062,9 +1060,7 @@ async fn dropped_backpressured_response_stream_traces_cancelled_partial_output()
|
||||
let backpressured_item_yielded = Arc::new(Notify::new());
|
||||
let mut events = VecDeque::new();
|
||||
for _ in 0..super::RESPONSE_STREAM_CHANNEL_CAPACITY {
|
||||
events.push_back(ResponseEvent::Created {
|
||||
guardian_ticket: None,
|
||||
});
|
||||
events.push_back(ResponseEvent::Created { response_id: None });
|
||||
}
|
||||
events.push_back(ResponseEvent::OutputItemDone(output_message(
|
||||
"1",
|
||||
|
||||
@@ -88,8 +88,8 @@ const GUARDIAN_RECENT_ENTRY_LIMIT: usize = 40;
|
||||
/// step-scoped things past their lifetime (like MCP bindings)
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct GuardianReviewContext {
|
||||
/// Ticket from the response currently handled in this execution context.
|
||||
pub(crate) guardian_ticket: Option<codex_protocol::guardian_ticket::GuardianTicket>,
|
||||
/// The response currently handled in this execution context.
|
||||
pub(crate) parent_response_id: Option<String>,
|
||||
turn: Arc<TurnContext>,
|
||||
environments: TurnEnvironmentSnapshot,
|
||||
// Model and reasoning inputs are carried for the follow-up Guardian and V2 migrations.
|
||||
@@ -109,11 +109,10 @@ impl GuardianReviewContext {
|
||||
settings: &ResolvedStepSettings,
|
||||
) -> Self {
|
||||
Self {
|
||||
guardian_ticket: turn
|
||||
parent_response_id: turn
|
||||
.extension_data
|
||||
.get::<codex_protocol::guardian_ticket::GuardianTicket>()
|
||||
.as_deref()
|
||||
.cloned(),
|
||||
.get::<codex_api::ResponseId>()
|
||||
.map(|id| id.0.clone()),
|
||||
environments: turn.environments.clone(),
|
||||
model_info: Arc::clone(&settings.model_info),
|
||||
reasoning_effort: settings.reasoning_effort().cloned(),
|
||||
@@ -136,12 +135,11 @@ impl GuardianReviewContext {
|
||||
impl From<&Arc<StepContext>> for GuardianReviewContext {
|
||||
fn from(step: &Arc<StepContext>) -> Self {
|
||||
Self {
|
||||
guardian_ticket: step
|
||||
parent_response_id: step
|
||||
.turn
|
||||
.extension_data
|
||||
.get::<codex_protocol::guardian_ticket::GuardianTicket>()
|
||||
.as_deref()
|
||||
.cloned(),
|
||||
.get::<codex_api::ResponseId>()
|
||||
.map(|id| id.0.clone()),
|
||||
turn: Arc::clone(&step.turn),
|
||||
environments: step.environments.clone(),
|
||||
model_info: Arc::clone(&step.settings.model_info),
|
||||
@@ -156,11 +154,10 @@ impl From<&Arc<StepContext>> for GuardianReviewContext {
|
||||
impl From<Arc<TurnContext>> for GuardianReviewContext {
|
||||
fn from(turn: Arc<TurnContext>) -> Self {
|
||||
Self {
|
||||
guardian_ticket: turn
|
||||
parent_response_id: turn
|
||||
.extension_data
|
||||
.get::<codex_protocol::guardian_ticket::GuardianTicket>()
|
||||
.as_deref()
|
||||
.cloned(),
|
||||
.get::<codex_api::ResponseId>()
|
||||
.map(|id| id.0.clone()),
|
||||
environments: turn.environments.clone(),
|
||||
model_info: Arc::clone(turn.model_info()),
|
||||
reasoning_effort: turn.reasoning_effort().cloned(),
|
||||
|
||||
@@ -1300,8 +1300,14 @@ async fn run_review_on_session(
|
||||
}),
|
||||
..Default::default()
|
||||
})
|
||||
.with_responses_metadata(
|
||||
params
|
||||
.parent_context
|
||||
.parent_response_id
|
||||
.as_ref()
|
||||
.map(|id| HashMap::from([("parent_response_id".to_owned(), id.clone())])),
|
||||
)
|
||||
.on_start(TurnStartOptions {
|
||||
guardian_ticket: params.parent_context.guardian_ticket.clone(),
|
||||
final_output_json_schema: Some(params.schema.clone()),
|
||||
service_tier: None,
|
||||
parent_turn_id: Some(parent_turn.sub_id.clone()),
|
||||
|
||||
@@ -58,8 +58,7 @@ pub(crate) const WORKSPACES_KEY: &str = "workspaces";
|
||||
// App-server clients can specify additional metadata in the `responsesapi_client_metadata` param
|
||||
// when submitting a turn, but they must not override fields owned by core.
|
||||
const RESERVED_METADATA_KEYS: &[&str] = &[
|
||||
codex_protocol::guardian_ticket::GUARDIAN_TICKET_METADATA_KEY,
|
||||
"guardian_ticket_requested",
|
||||
"guardian_credits_requested",
|
||||
INSTALLATION_ID_KEY,
|
||||
X_CODEX_INSTALLATION_ID_HEADER,
|
||||
SESSION_ID_KEY,
|
||||
@@ -219,8 +218,8 @@ pub(crate) enum TurnToolSource {
|
||||
/// truth.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct CodexResponsesMetadata {
|
||||
/// Opaque runtime receipt; deliberately omitted from metadata projections.
|
||||
pub(crate) guardian_ticket: Option<codex_protocol::guardian_ticket::GuardianTicket>,
|
||||
/// Guardian parent reference; projected only onto a Guardian request.
|
||||
pub(crate) parent_response_id: Option<String>,
|
||||
pub(crate) installation_id: String,
|
||||
pub(crate) session_id: String,
|
||||
pub(crate) thread_id: String,
|
||||
@@ -260,7 +259,7 @@ impl CodexResponsesMetadata {
|
||||
window_id: String,
|
||||
) -> Self {
|
||||
Self {
|
||||
guardian_ticket: None,
|
||||
parent_response_id: None,
|
||||
installation_id,
|
||||
session_id,
|
||||
thread_id,
|
||||
|
||||
@@ -137,7 +137,6 @@ pub(super) async fn spawn_review_thread(
|
||||
extension_data.insert(parent_turn_context.skills_snapshot().as_ref().clone());
|
||||
|
||||
let review_turn_context = TurnContext {
|
||||
guardian_ticket: None,
|
||||
sub_id: review_turn_id.clone(),
|
||||
trace_id: current_span_trace_id(),
|
||||
realtime_active: parent_turn_context.realtime_active,
|
||||
|
||||
@@ -619,7 +619,6 @@ impl Session {
|
||||
request_kind,
|
||||
);
|
||||
CodexResponsesMetadata {
|
||||
guardian_ticket: turn_context.guardian_ticket.clone(),
|
||||
window_number: Some(window_number),
|
||||
context_window_id: Some(context_window_id),
|
||||
history_ingest_requested: turn_context
|
||||
|
||||
@@ -1437,10 +1437,10 @@ async fn run_sampling_request(
|
||||
let mut original_input = None;
|
||||
let mut executed_tool_calls_by_output = HashMap::new();
|
||||
loop {
|
||||
// A retry must not lend a previous response's ticket to the next tool call.
|
||||
// A retry must not attribute the next tool call to the previous response.
|
||||
turn_context
|
||||
.extension_data
|
||||
.remove::<codex_protocol::guardian_ticket::GuardianTicket>();
|
||||
.remove::<codex_api::ResponseId>();
|
||||
let prompt_input = if let Some(input) = initial_input.take() {
|
||||
input
|
||||
} else {
|
||||
@@ -2376,9 +2376,11 @@ async fn try_run_sampling_request(
|
||||
record_turn_ttft_metric(&turn_context, &event).await;
|
||||
|
||||
match event {
|
||||
ResponseEvent::Created { guardian_ticket } => {
|
||||
if let Some(ticket) = guardian_ticket {
|
||||
turn_context.extension_data.insert(ticket);
|
||||
ResponseEvent::Created { response_id } => {
|
||||
if let Some(response_id) = response_id {
|
||||
turn_context
|
||||
.extension_data
|
||||
.insert(codex_api::ResponseId(response_id));
|
||||
}
|
||||
}
|
||||
ResponseEvent::OutputItemDone(mut item) => {
|
||||
|
||||
@@ -185,7 +185,6 @@ impl std::fmt::Debug for TurnEnvironment {
|
||||
/// Request options for one turn, separate from persistent thread settings.
|
||||
#[derive(Default)]
|
||||
pub(crate) struct NewTurnContextOptions {
|
||||
pub(crate) guardian_ticket: Option<codex_protocol::guardian_ticket::GuardianTicket>,
|
||||
pub(crate) final_output_json_schema: Option<Value>,
|
||||
pub(crate) cyber_access_program: Option<CyberAccessProgram>,
|
||||
}
|
||||
@@ -193,8 +192,6 @@ pub(crate) struct NewTurnContextOptions {
|
||||
/// The context needed for a single turn of the thread.
|
||||
#[derive(Debug)]
|
||||
pub struct TurnContext {
|
||||
/// Receipt selected for this Guardian turn, separate from its own responses.
|
||||
pub(crate) guardian_ticket: Option<codex_protocol::guardian_ticket::GuardianTicket>,
|
||||
pub(crate) sub_id: String,
|
||||
pub(crate) trace_id: Option<String>,
|
||||
pub(crate) realtime_active: bool,
|
||||
@@ -540,7 +537,6 @@ impl TurnContext {
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
available_models,
|
||||
unified_exec_shell_mode: self.unified_exec_shell_mode.clone(),
|
||||
guardian_ticket: self.guardian_ticket.clone(),
|
||||
final_output_json_schema: self.final_output_json_schema.clone(),
|
||||
dynamic_tools: self.dynamic_tools.clone(),
|
||||
turn_metadata_state: self.turn_metadata_state.clone(),
|
||||
@@ -785,7 +781,6 @@ impl Session {
|
||||
let extension_data = Arc::new(codex_extension_api::ExtensionData::new(sub_id.clone()));
|
||||
extension_data.insert(skills_snapshot);
|
||||
TurnContext {
|
||||
guardian_ticket: None,
|
||||
sub_id,
|
||||
trace_id: current_span_trace_id(),
|
||||
realtime_active: false,
|
||||
@@ -1040,7 +1035,6 @@ impl Session {
|
||||
turn_context.extension_data.insert(trusted_plugin_roots);
|
||||
turn_context.realtime_active = self.conversation.running_state().await.is_some();
|
||||
|
||||
turn_context.guardian_ticket = options.guardian_ticket;
|
||||
turn_context.final_output_json_schema = options.final_output_json_schema;
|
||||
if turn_context.config.model_provider_id == codex_model_provider_info::OPENAI_PROVIDER_ID {
|
||||
turn_context.cyber_access_program = options.cyber_access_program;
|
||||
|
||||
@@ -121,7 +121,6 @@ impl PreparedTurnInputSettings {
|
||||
kind: TurnStartKind,
|
||||
) -> CodexResult<Option<Arc<TurnContext>>> {
|
||||
let TurnStartOptions {
|
||||
guardian_ticket,
|
||||
turn_trigger,
|
||||
final_output_json_schema,
|
||||
service_tier,
|
||||
@@ -139,7 +138,6 @@ impl PreparedTurnInputSettings {
|
||||
updates.service_tier_for_turn = service_tier;
|
||||
|
||||
let options = NewTurnContextOptions {
|
||||
guardian_ticket,
|
||||
final_output_json_schema,
|
||||
cyber_access_program,
|
||||
};
|
||||
|
||||
@@ -473,7 +473,6 @@ impl Session {
|
||||
.new_turn_with_default_settings(
|
||||
sub_id,
|
||||
NewTurnContextOptions {
|
||||
guardian_ticket: start_options.guardian_ticket,
|
||||
final_output_json_schema: start_options.final_output_json_schema,
|
||||
cyber_access_program: start_options.cyber_access_program,
|
||||
},
|
||||
|
||||
@@ -393,6 +393,14 @@ impl TurnMetadataState {
|
||||
.read()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone();
|
||||
// Extract Guardian's internal parent before filtering configured metadata keys.
|
||||
// Ordinary app-server client metadata stays in `extra`.
|
||||
let parent_response_id =
|
||||
if self.subagent_header.as_deref() == Some(crate::guardian::GUARDIAN_REVIEWER_NAME) {
|
||||
extra.remove("parent_response_id")
|
||||
} else {
|
||||
None
|
||||
};
|
||||
for key in self
|
||||
.responses_api_metadata
|
||||
.read()
|
||||
@@ -402,6 +410,7 @@ impl TurnMetadataState {
|
||||
extra.remove(key);
|
||||
}
|
||||
let mut metadata = CodexResponsesMetadata {
|
||||
parent_response_id,
|
||||
turn_id: Some(self.turn_id.clone()),
|
||||
agent_name: Some(self.agent_name.clone()),
|
||||
forked_from_thread_id: self.forked_from_thread_id,
|
||||
|
||||
@@ -29,9 +29,7 @@ async fn turn_timing_state_records_ttft_only_once_per_turn() {
|
||||
state.mark_turn_started(Instant::now()).await;
|
||||
assert_eq!(
|
||||
state
|
||||
.record_ttft_for_response_event(&ResponseEvent::Created {
|
||||
guardian_ticket: None
|
||||
})
|
||||
.record_ttft_for_response_event(&ResponseEvent::Created { response_id: None })
|
||||
.await,
|
||||
None
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user