mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Fail closed on unbounded Guardian parent compactions (#41152)
## Why Guardian classification must not proceed after silently dropping a parent compaction that cannot fit within its configured size limit. ## What changed - Distinguish an absent or unusable parent compaction from serialization and size-limit failures. - Record a fail-closed score and failed classification when the latest encrypted parent compaction cannot be serialized or exceeds the complete-item byte budget. - Preserve omission of compactions with missing or empty encrypted content without falling back to older context. ## Testing Cover compactions at the exact byte limit, one byte over it, and oversized passthrough metadata. GitOrigin-RevId: 8e607c88f10003cdfab273963bc73c848b09c821
This commit is contained in:
@@ -442,49 +442,25 @@ impl GuardianV2Extension {
|
||||
.enable_image_capture();
|
||||
}
|
||||
input.thread_store.insert(guardian_config.clone());
|
||||
let latest_parent_compaction = if guardian_config.reuse_parent_compaction {
|
||||
input
|
||||
.conversation_history
|
||||
.items()
|
||||
.filter(|item| {
|
||||
matches!(
|
||||
item,
|
||||
ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. }
|
||||
)
|
||||
})
|
||||
.last()
|
||||
let parent_compaction = if guardian_config.reuse_parent_compaction {
|
||||
match encrypted_parent_compaction(
|
||||
input.conversation_history.items(),
|
||||
guardian_config.max_parent_compaction_tokens,
|
||||
) {
|
||||
Ok(compaction) => compaction,
|
||||
Err(_) => {
|
||||
Self::record_fail_closed_score(input.thread_store, sampled_at);
|
||||
record_classification(
|
||||
metrics.as_deref(),
|
||||
classification_started_at.elapsed(),
|
||||
"failure",
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let parent_compaction = latest_parent_compaction.and_then(|item| {
|
||||
encrypted_parent_compaction(
|
||||
std::iter::once(item),
|
||||
guardian_config.max_parent_compaction_tokens,
|
||||
)
|
||||
});
|
||||
if parent_compaction.is_none()
|
||||
&& latest_parent_compaction.is_some_and(|item| match item {
|
||||
ResponseItem::Compaction {
|
||||
id: Some(_),
|
||||
encrypted_content,
|
||||
..
|
||||
}
|
||||
| ResponseItem::ContextCompaction {
|
||||
id: Some(_),
|
||||
encrypted_content: Some(encrypted_content),
|
||||
..
|
||||
} => !encrypted_content.is_empty(),
|
||||
_ => false,
|
||||
})
|
||||
{
|
||||
Self::record_fail_closed_score(input.thread_store, sampled_at);
|
||||
record_classification(
|
||||
metrics.as_deref(),
|
||||
classification_started_at.elapsed(),
|
||||
"failure",
|
||||
);
|
||||
return;
|
||||
}
|
||||
let call_id = input.call_id.to_owned();
|
||||
let mcp_tool = input.mcp_tool.cloned();
|
||||
let action = GuardianAction {
|
||||
@@ -754,21 +730,32 @@ impl GuardianV2Extension {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Eq, PartialEq)]
|
||||
enum ParentCompactionError {
|
||||
Serialization,
|
||||
Oversized,
|
||||
}
|
||||
|
||||
// An unusable latest compaction must never fall back to an older one. Missing
|
||||
// encrypted content can be omitted; content that cannot be bounded rejects the sample.
|
||||
fn encrypted_parent_compaction<'a>(
|
||||
items: impl Iterator<Item = &'a ResponseItem>,
|
||||
max_parent_compaction_tokens: usize,
|
||||
) -> Option<ResponseItem> {
|
||||
) -> Result<Option<ResponseItem>, ParentCompactionError> {
|
||||
let max_compaction_bytes = TruncationPolicy::Tokens(max_parent_compaction_tokens).byte_budget();
|
||||
let item = items
|
||||
let Some(item) = items
|
||||
.filter(|item| {
|
||||
matches!(
|
||||
item,
|
||||
ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. }
|
||||
)
|
||||
})
|
||||
.last()?;
|
||||
.last()
|
||||
else {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
match item {
|
||||
let encrypted_content = match item {
|
||||
ResponseItem::Compaction {
|
||||
id: Some(_),
|
||||
encrypted_content,
|
||||
@@ -778,14 +765,18 @@ fn encrypted_parent_compaction<'a>(
|
||||
id: Some(_),
|
||||
encrypted_content: Some(encrypted_content),
|
||||
..
|
||||
} if !encrypted_content.is_empty()
|
||||
&& serde_json::to_vec(item)
|
||||
.is_ok_and(|serialized| serialized.len() <= max_compaction_bytes) =>
|
||||
{
|
||||
Some(item.clone())
|
||||
}
|
||||
_ => None,
|
||||
} => encrypted_content,
|
||||
_ => return Ok(None),
|
||||
};
|
||||
if encrypted_content.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
let serialized = serde_json::to_vec(item).map_err(|_| ParentCompactionError::Serialization)?;
|
||||
if serialized.len() > max_compaction_bytes {
|
||||
return Err(ParentCompactionError::Oversized);
|
||||
}
|
||||
|
||||
Ok(Some(item.clone()))
|
||||
}
|
||||
|
||||
/// Installs feature-gated Guardian V2 tool classification for each thread.
|
||||
|
||||
@@ -57,6 +57,7 @@ use serde_json::json;
|
||||
|
||||
use super::GuardianV2Extension;
|
||||
use super::GuardianV2ScoreProgress;
|
||||
use super::ParentCompactionError;
|
||||
use super::StrictReviewReason;
|
||||
use super::encrypted_parent_compaction;
|
||||
use super::should_classify_tool;
|
||||
@@ -645,14 +646,14 @@ fn encrypted_parent_compaction_preserves_the_latest_valid_item() {
|
||||
[&older, &latest].into_iter(),
|
||||
DEFAULT_PARENT_COMPACTION_TOKENS,
|
||||
),
|
||||
Some(latest.clone())
|
||||
Ok(Some(latest.clone()))
|
||||
);
|
||||
assert_eq!(
|
||||
encrypted_parent_compaction(
|
||||
[&latest, &older].into_iter(),
|
||||
DEFAULT_PARENT_COMPACTION_TOKENS,
|
||||
),
|
||||
Some(older)
|
||||
Ok(Some(older))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -697,7 +698,7 @@ fn encrypted_parent_compaction_rejects_invalid_latest_item() {
|
||||
[&older, latest].into_iter(),
|
||||
DEFAULT_PARENT_COMPACTION_TOKENS,
|
||||
),
|
||||
None,
|
||||
Ok(None),
|
||||
"an unusable latest summary must not resurrect older context"
|
||||
);
|
||||
}
|
||||
@@ -736,7 +737,7 @@ fn encrypted_parent_compaction_rejects_oversized_latest_item() -> Result<()> {
|
||||
assert_eq!(serde_json::to_vec(&*item)?.len(), max_compaction_bytes);
|
||||
assert_eq!(
|
||||
encrypted_parent_compaction(std::iter::once(&*item), DEFAULT_PARENT_COMPACTION_TOKENS,),
|
||||
Some(item.clone())
|
||||
Ok(Some(item.clone()))
|
||||
);
|
||||
|
||||
let mut oversized = item.clone();
|
||||
@@ -759,7 +760,7 @@ fn encrypted_parent_compaction_rejects_oversized_latest_item() -> Result<()> {
|
||||
[&*item, &oversized].into_iter(),
|
||||
DEFAULT_PARENT_COMPACTION_TOKENS,
|
||||
),
|
||||
None,
|
||||
Err(ParentCompactionError::Oversized),
|
||||
"an oversized latest summary must not resurrect older context"
|
||||
);
|
||||
}
|
||||
@@ -780,7 +781,7 @@ fn encrypted_parent_compaction_rejects_oversized_latest_item() -> Result<()> {
|
||||
[&bounded[0], &oversized_metadata].into_iter(),
|
||||
DEFAULT_PARENT_COMPACTION_TOKENS,
|
||||
),
|
||||
None,
|
||||
Err(ParentCompactionError::Oversized),
|
||||
"oversized passthrough metadata must not bypass the complete-item limit"
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user