mirror of
https://github.com/openai/codex.git
synced 2026-11-11 21:10:39 +00:00
Retain assistant context for Guardian authorization reviews (#47582)
## Why Ordinary user replies can depend on preceding assistant messages. Losing those messages during compaction can leave Guardian without the context needed to interpret a reply's scope. ## What changed - Retain original assistant messages, including commentary, with their recorded order across checkpoints and resume. Exclude compaction output from this evidence. - Include assistant context in local and delegated reviews as untrusted context, with separate storage limits and lower budget priority than user instructions. - Prefer assistant context preceding ordinary replies when selecting bounded root evidence, and report unavailable context without inferring what a reply refers to. - Preserve inherited context scope and remove assistant evidence beyond rollback boundaries, including during rollout migration. ## Testing Extend coverage for compaction and resume, checkpoint migration, queued-input ordering and rollback, role labeling, and evidence budgets that preserve user restrictions while dropping assistant context. GitOrigin-RevId: 886189b0be29ffc3d324b56f5d0b673d6b8d84da
This commit is contained in:
@@ -158,6 +158,19 @@ impl RollbackPlanner {
|
||||
match &line.item {
|
||||
RolloutItem::SessionMeta(_) => self.record_boundaries[index] = None,
|
||||
RolloutItem::ResponseItem(response) => {
|
||||
if matches!(&response.item, ResponseItem::Message { role, .. } if role == "assistant")
|
||||
|| matches!(&response.item, ResponseItem::FunctionCall { .. })
|
||||
{
|
||||
self.retained_fact_sources.push(RetainedFactSource {
|
||||
record_index: index,
|
||||
turn_id: response.turn_id().unwrap_or_default().to_owned(),
|
||||
acceptance_order: response
|
||||
.metadata
|
||||
.as_ref()
|
||||
.filter(|metadata| !metadata.inherited_user_message)
|
||||
.and_then(|metadata| metadata.user_input_order),
|
||||
});
|
||||
}
|
||||
if let Some(boundary) = paired_delivery_boundary {
|
||||
self.record_boundaries[index] = Some(boundary);
|
||||
self.boundaries[boundary].message_id = response.id().cloned();
|
||||
|
||||
@@ -1399,7 +1399,8 @@ async fn assert_migrated_evidence_order(steer_order: Option<u64>) {
|
||||
{"order": if steer_order.is_some() { 2 } else { 1 }, "turn_id": "shared-turn", "call_id": "before", "questions": [{"question": "Publish?", "answer": "Only privately."}]},
|
||||
{"order": 3, "turn_id": "shared-turn", "call_id": "after", "questions": [{"question": "Publish the README?", "answer": "Do not publish it."}]}
|
||||
],
|
||||
"incomplete": false, "user_messages_incomplete": false, "next_order": 4
|
||||
"incomplete": false, "user_messages_incomplete": false, "next_order": 4,
|
||||
"assistant_messages": [], "assistant_messages_incomplete": true
|
||||
});
|
||||
checkpoint.retained_context =
|
||||
Some(serde_json::from_value(retained.clone()).expect("retained fixture"));
|
||||
@@ -1456,6 +1457,18 @@ async fn assert_migrated_evidence_order(steer_order: Option<u64>) {
|
||||
rollout_response_item(initial.clone()),
|
||||
user_message(INITIAL),
|
||||
RolloutItem::RetainedContext(answers[0].clone()),
|
||||
RolloutItem::ResponseItem(codex_rollout::ResponseItemEnvelope {
|
||||
item: serde_json::from_value(json!({
|
||||
"type": "message", "id": "late-assistant", "role": "assistant",
|
||||
"content": [{"type": "output_text", "text": "Late assistant question?"}],
|
||||
"internal_chat_message_metadata_passthrough": {"turn_id": "shared-turn"}
|
||||
}))
|
||||
.expect("assistant message"),
|
||||
metadata: Some(
|
||||
serde_json::from_value(json!({"user_input_order": 2}))
|
||||
.expect("assistant order"),
|
||||
),
|
||||
}),
|
||||
RolloutItem::Compacted(before_steer),
|
||||
RolloutItem::ResponseItem(codex_rollout::ResponseItemEnvelope {
|
||||
item: steer,
|
||||
@@ -1487,6 +1500,13 @@ async fn assert_migrated_evidence_order(steer_order: Option<u64>) {
|
||||
.await
|
||||
.expect("migrate same-turn rollback");
|
||||
let migrated = read_rollout(&path);
|
||||
assert_eq!(
|
||||
migrated.iter().any(|line| matches!(&line.item,
|
||||
RolloutItem::ResponseItem(envelope)
|
||||
if envelope.item.id().is_some_and(|id| id.as_str() == "late-assistant")
|
||||
)),
|
||||
steer_order.is_none()
|
||||
);
|
||||
let checkpoints = migrated
|
||||
.iter()
|
||||
.filter_map(|line| match &line.item {
|
||||
|
||||
Reference in New Issue
Block a user