mirror of
https://github.com/openai/codex.git
synced 2026-10-24 18:18:41 +00:00
Move Guardian approval reason caps into context composition (#56477)
## What changed Apply the 512-token approval reason cap in `guardian-context` when composing synchronous full and delta reviews, before request budget enforcement. Remove truncation from the requesting host and preserve exact action JSON. ## Testing Extend the planned-action budget test to cover long approval reasons in both synchronous presentations, verifying that optional tool descriptions can be omitted without changing action arguments. GitOrigin-RevId: 8cdb6aadd5f50194f63ebd2402193e69bed127cc
This commit is contained in:
Generated
+1
@@ -3586,6 +3586,7 @@ dependencies = [
|
||||
"codex-history",
|
||||
"codex-protocol",
|
||||
"codex-protocol-test-support",
|
||||
"codex-utils-output-truncation",
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
|
||||
@@ -27,9 +27,7 @@ use crate::context::NodeReplReviewEvidenceMode;
|
||||
use crate::context::is_guardian_context_message;
|
||||
use crate::context::node_repl_review_evidence_mode;
|
||||
use crate::session::session::Session;
|
||||
use codex_utils_output_truncation::TruncationPolicy;
|
||||
use codex_utils_output_truncation::approx_bytes_for_tokens;
|
||||
use codex_utils_output_truncation::truncate_text;
|
||||
|
||||
use super::ApprovalRequestReasons;
|
||||
use super::GUARDIAN_MAX_NODE_REPL_TOOL_RESULT_TOKENS;
|
||||
@@ -38,7 +36,6 @@ use super::GuardianApprovalRequest;
|
||||
use super::GuardianReviewContext;
|
||||
use super::approval_request::format_guardian_action_pretty;
|
||||
|
||||
const GUARDIAN_MAX_APPROVAL_REASON_TOKENS: usize = 512;
|
||||
// Bound both JSON and permission evidence without restricting manual approvals.
|
||||
const MAX_GUARDIAN_ENVIRONMENT_ID_BYTES: usize = 256;
|
||||
pub(super) const GUARDIAN_TRANSCRIPT_START: &str = ">>> TRANSCRIPT START\n";
|
||||
@@ -152,12 +149,7 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn(
|
||||
| GuardianApprovalRequest::McpToolCall { .. }
|
||||
| GuardianApprovalRequest::RequestPermissions { .. } => PlannedActionKind::Command,
|
||||
},
|
||||
reason: reasons.retry.or(reasons.approval).map(|reason| {
|
||||
truncate_text(
|
||||
&reason,
|
||||
TruncationPolicy::Tokens(GUARDIAN_MAX_APPROVAL_REASON_TOKENS),
|
||||
)
|
||||
}),
|
||||
reason: reasons.retry.or(reasons.approval),
|
||||
};
|
||||
let permissions = parent_context
|
||||
.map(|context| {
|
||||
|
||||
@@ -16,6 +16,7 @@ workspace = true
|
||||
codex-context-fragments = { workspace = true }
|
||||
codex-history = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-utils-output-truncation = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
|
||||
|
||||
@@ -17,6 +17,8 @@ use codex_protocol::models::ImageReference;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::TruncationPolicy;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_output_truncation::TruncationPolicy as OutputTruncationPolicy;
|
||||
use codex_utils_output_truncation::truncate_text;
|
||||
|
||||
use crate::ActionPresentation;
|
||||
use crate::BudgetPriority;
|
||||
@@ -31,6 +33,8 @@ use crate::TranscriptRecord;
|
||||
use crate::TruncationObservation;
|
||||
use crate::retained_instructions::RetainedInstructionKind;
|
||||
|
||||
const MAX_APPROVAL_REASON_TOKENS: usize = 512;
|
||||
|
||||
/// Consumer framing after the host has selected a full or delta transcript.
|
||||
pub enum ContextPresentation<'a> {
|
||||
SyncFull { session_id: &'a str },
|
||||
@@ -148,7 +152,7 @@ impl CollectedContext {
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Frames the selected transcript while preserving structured text through admission.
|
||||
/// Applies consumer caps and frames the selected transcript before request admission.
|
||||
pub fn compose(
|
||||
self,
|
||||
presentation: ContextPresentation<'_>,
|
||||
@@ -380,7 +384,15 @@ impl CollectedContext {
|
||||
SectionDelivery::user_content(items),
|
||||
)
|
||||
}
|
||||
ContextSection::PlannedAction(planned) => {
|
||||
ContextSection::PlannedAction(mut planned) => {
|
||||
if action != ActionPresentation::Async {
|
||||
planned.reason = planned.reason.map(|reason| {
|
||||
truncate_text(
|
||||
&reason,
|
||||
OutputTruncationPolicy::Tokens(MAX_APPROVAL_REASON_TOKENS),
|
||||
)
|
||||
});
|
||||
}
|
||||
let mut items = planned
|
||||
.render(action)
|
||||
.into_iter()
|
||||
|
||||
@@ -261,50 +261,73 @@ fn recovery_shortens_older_history_only_after_optional_evidence() {
|
||||
|
||||
#[test]
|
||||
fn planned_action_budget_omits_descriptions_without_changing_arguments() {
|
||||
let reason = format!("reason-start {} reason-end", "x".repeat(/*n*/ 10_000));
|
||||
let action = crate::PlannedAction {
|
||||
json: r#"{"tool":"write_record","arguments":{"description":"required payload"}}"#
|
||||
.to_owned(),
|
||||
kind: crate::PlannedActionKind::Command,
|
||||
reason: None,
|
||||
reason: Some(reason),
|
||||
tool_descriptions: Some("optional tool description ".repeat(/*n*/ 100)),
|
||||
};
|
||||
let required = action.render(crate::ActionPresentation::SyncFull);
|
||||
let context = crate::CollectedContext {
|
||||
sections: vec![crate::ContextSection::PlannedAction(action)],
|
||||
}
|
||||
.compose(
|
||||
crate::ContextPresentation::SyncFull {
|
||||
session_id: "review",
|
||||
},
|
||||
crate::PreparedTranscript {
|
||||
items: Vec::new(),
|
||||
omission_note: None,
|
||||
truncations: Vec::new(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
let mut required_items = required.iter().map(|item| text(item)).collect::<Vec<_>>();
|
||||
required_items.push(text("evidence omitted"));
|
||||
let budget = crate::estimate_input_tokens(&user_message(required_items.clone())) + 100;
|
||||
let selected = context
|
||||
.enforce_budget(
|
||||
RequestBudget {
|
||||
max_input_tokens: budget,
|
||||
existing_context_tokens: 0,
|
||||
for (presentation, action_presentation) in [
|
||||
(
|
||||
crate::ContextPresentation::SyncFull {
|
||||
session_id: "review",
|
||||
},
|
||||
crate::ActionPresentation::SyncFull,
|
||||
),
|
||||
(
|
||||
crate::ContextPresentation::SyncDelta {
|
||||
session_id: "review",
|
||||
},
|
||||
crate::ActionPresentation::SyncDelta,
|
||||
),
|
||||
] {
|
||||
let mut bounded = action.clone();
|
||||
bounded.reason = bounded.reason.map(|reason| {
|
||||
codex_utils_output_truncation::truncate_text(
|
||||
&reason,
|
||||
codex_utils_output_truncation::TruncationPolicy::Tokens(/*tokens*/ 512),
|
||||
)
|
||||
});
|
||||
let required = bounded.render(action_presentation);
|
||||
let context = crate::CollectedContext {
|
||||
sections: vec![crate::ContextSection::PlannedAction(action.clone())],
|
||||
}
|
||||
.compose(
|
||||
presentation,
|
||||
crate::PreparedTranscript {
|
||||
items: Vec::new(),
|
||||
omission_note: None,
|
||||
truncations: Vec::new(),
|
||||
},
|
||||
"evidence omitted".to_owned(),
|
||||
HistoryTruncation::Preserve,
|
||||
)
|
||||
.unwrap();
|
||||
// The sync preamble is also required and remains ahead of the action.
|
||||
let messages = selected.into_messages();
|
||||
let ResponseItem::Message { content, .. } = &messages[0] else {
|
||||
panic!("expected user evidence");
|
||||
};
|
||||
assert_eq!(
|
||||
&content[content.len() - required_items.len()..],
|
||||
required_items
|
||||
);
|
||||
let mut required_items = required.iter().map(|item| text(item)).collect::<Vec<_>>();
|
||||
required_items.push(text("evidence omitted"));
|
||||
let budget = crate::estimate_input_tokens(&user_message(required_items.clone()))
|
||||
+ section_tokens(&context.sections[0])
|
||||
+ 100;
|
||||
let selected = context
|
||||
.enforce_budget(
|
||||
RequestBudget {
|
||||
max_input_tokens: budget,
|
||||
existing_context_tokens: 0,
|
||||
},
|
||||
"evidence omitted".to_owned(),
|
||||
HistoryTruncation::Preserve,
|
||||
)
|
||||
.unwrap();
|
||||
// The sync preamble is also required and remains ahead of the action.
|
||||
let messages = selected.into_messages();
|
||||
let ResponseItem::Message { content, .. } = &messages[0] else {
|
||||
panic!("expected user evidence");
|
||||
};
|
||||
assert_eq!(
|
||||
&content[content.len() - required_items.len()..],
|
||||
required_items
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -162,7 +162,7 @@ pub struct SectionInput<'a> {
|
||||
pub root_conversation: &'a [GuardianRootMessage],
|
||||
/// Bounded, role-labeled answers selected from the host-owned context snapshot.
|
||||
pub trusted_user_answers: &'a [String],
|
||||
/// Exact action JSON and reason, already bounded by the requesting host.
|
||||
/// Exact action JSON and optional reason; sync composition bounds the reason.
|
||||
pub planned_action: Option<&'a PlannedAction>,
|
||||
/// Sync-only restrictions resolved from the parent execution environment.
|
||||
pub permissions: Option<&'a PermissionContext>,
|
||||
|
||||
Reference in New Issue
Block a user