mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
Centralize synchronous Guardian prompt preparation (#57790)
## What changed - Separate evidence collection from prompt preparation and pass reviewer history, model limits, and image preparation policy through shared `guardian-context` APIs. - Detect captured images before composition and measure reviewer history tokens only when images and a valid context window require it. - Share instruction and native-message delivery filtering across budget checks, including restoration of retained evidence after history changes. GitOrigin-RevId: 65ef53cf0bc22e1bde9abfb7a9ffce65cfab6f1c
This commit is contained in:
@@ -85,9 +85,7 @@ pub(crate) async fn check_pending(session: &Session, turn: &TurnContext) -> Code
|
||||
session.services.image_store.as_ref(),
|
||||
)
|
||||
.await;
|
||||
context.retain_new_instructions(&history);
|
||||
context
|
||||
.retain_new_native_messages(&history)
|
||||
codex_guardian_context::prepare_sync_delivery(&mut context, &history)
|
||||
.map_err(|error| CodexErr::InvalidRequest(error.to_string()))?;
|
||||
if context.estimated_tokens().saturating_add(minimum_prefix) > maximum {
|
||||
context
|
||||
@@ -155,9 +153,7 @@ pub(crate) async fn finalize(
|
||||
.await;
|
||||
// Use the history that will actually reach the model. Recompute after every
|
||||
// compaction retry; evidence removed by compaction must be delivered again.
|
||||
context.retain_new_instructions(&history);
|
||||
context
|
||||
.retain_new_native_messages(&history)
|
||||
codex_guardian_context::prepare_sync_delivery(&mut context, &history)
|
||||
.map_err(|error| CodexErr::InvalidRequest(error.to_string()))?;
|
||||
let prompt = build_prompt(
|
||||
history
|
||||
|
||||
@@ -208,7 +208,7 @@ use prompt::GuardianTranscriptCursor;
|
||||
#[cfg(test)]
|
||||
use prompt::build_guardian_prompt_items;
|
||||
#[cfg(test)]
|
||||
use prompt::build_guardian_prompt_items_with_parent_turn;
|
||||
use prompt::collect_guardian_prompt_items_with_parent_turn;
|
||||
#[cfg(test)]
|
||||
use prompt::render_guardian_transcript_entries;
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -15,6 +15,7 @@ use codex_guardian_context::SectionInput;
|
||||
pub(crate) use codex_guardian_context::TranscriptCursor as GuardianTranscriptCursor;
|
||||
pub(crate) use codex_guardian_context::TranscriptMode as GuardianPromptMode;
|
||||
use codex_guardian_context::default_registry;
|
||||
#[cfg(test)]
|
||||
use codex_guardian_reviewer::GuardianPromptItems;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
|
||||
@@ -56,7 +57,7 @@ pub(crate) async fn build_guardian_prompt_items(
|
||||
request: GuardianApprovalRequest,
|
||||
mode: GuardianPromptMode,
|
||||
) -> anyhow::Result<GuardianPromptItems> {
|
||||
build_guardian_prompt_items_with_parent_turn(
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
session,
|
||||
GuardianTranscriptHistory::Retained(session.conversation_history_snapshot().await.as_ref()),
|
||||
/*parent_context*/ None,
|
||||
@@ -68,10 +69,28 @@ pub(crate) async fn build_guardian_prompt_items(
|
||||
mode,
|
||||
/*reviewed_node_repl_evidence_sequence*/ 0,
|
||||
)
|
||||
.await
|
||||
.await?;
|
||||
codex_guardian_reviewer::compose_guardian_prompt(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) async fn build_guardian_prompt_items_with_parent_turn(
|
||||
/// Captured request sources and host cursor facts, before request-size preparation.
|
||||
pub(super) struct CollectedGuardianPrompt {
|
||||
pub(super) sections: CollectedContext,
|
||||
pub(super) profile: ContextProfile,
|
||||
pub(super) mode: GuardianPromptMode,
|
||||
pub(super) history_version: u64,
|
||||
pub(super) session_id: String,
|
||||
pub(super) node_repl_evidence_sequence: u64,
|
||||
}
|
||||
|
||||
pub(super) async fn collect_guardian_prompt_items_with_parent_turn(
|
||||
session: &Session,
|
||||
transcript_history: GuardianTranscriptHistory<'_>,
|
||||
parent_context: Option<&GuardianReviewContext>,
|
||||
@@ -79,7 +98,7 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn(
|
||||
request: GuardianApprovalRequest,
|
||||
mode: GuardianPromptMode,
|
||||
reviewed_node_repl_evidence_sequence: u64,
|
||||
) -> anyhow::Result<GuardianPromptItems> {
|
||||
) -> anyhow::Result<CollectedGuardianPrompt> {
|
||||
let (history, use_parent_checkpoint) = match transcript_history {
|
||||
GuardianTranscriptHistory::Retained(history) => (history, false),
|
||||
GuardianTranscriptHistory::LoadedParentCheckpoint(history) => (history, true),
|
||||
@@ -170,18 +189,18 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn(
|
||||
permissions.as_ref(),
|
||||
node_repl_context.as_ref(),
|
||||
)?;
|
||||
codex_guardian_reviewer::compose_guardian_prompt(
|
||||
Ok(CollectedGuardianPrompt {
|
||||
sections,
|
||||
&profile,
|
||||
profile,
|
||||
mode,
|
||||
if use_parent_checkpoint {
|
||||
history_version: if use_parent_checkpoint {
|
||||
history.history_version()
|
||||
} else {
|
||||
history.review_history_version()
|
||||
},
|
||||
&session.thread_id.to_string(),
|
||||
session_id: session.thread_id.to_string(),
|
||||
node_repl_evidence_sequence,
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/// Exercises the sync profile through the host's existing transcript tests.
|
||||
|
||||
@@ -79,14 +79,16 @@ pub(crate) async fn prepare_prompt(
|
||||
/*include_internal*/ true,
|
||||
)?;
|
||||
let mut context = retained.context.clone();
|
||||
context.retain_new_instructions(
|
||||
codex_guardian_context::prepare_sync_delivery(
|
||||
&mut context,
|
||||
&history
|
||||
.for_prompt_annotated(
|
||||
&model.input_modalities,
|
||||
session.services.image_store.as_ref(),
|
||||
)
|
||||
.await,
|
||||
);
|
||||
)
|
||||
.map_err(|error| CodexErr::InvalidRequest(error.to_string()))?;
|
||||
let context = context
|
||||
.enforce_budget(
|
||||
RequestBudget {
|
||||
|
||||
@@ -8,12 +8,14 @@ pub use setup::PreparedGuardianContext;
|
||||
pub use setup::prepare_review_prewarm;
|
||||
pub(crate) use setup::run_guardian_review_session;
|
||||
|
||||
#[path = "review_session_prompt.rs"]
|
||||
mod prompt_builder;
|
||||
|
||||
#[path = "review_session_context.rs"]
|
||||
mod context_policy;
|
||||
use codex_guardian_reviewer::ReviewContextPolicy;
|
||||
use codex_guardian_reviewer::ReviewDeadline;
|
||||
|
||||
use std::borrow::Cow;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
@@ -42,11 +44,8 @@ use codex_protocol::config_types::AutoCompactTokenLimitScope;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ImageDetail;
|
||||
use codex_protocol::models::ImageReference;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::GuardianScope;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_protocol::protocol::CodexErrorInfo;
|
||||
use codex_protocol::protocol::Event;
|
||||
@@ -71,9 +70,6 @@ use crate::context::GuardianFollowupReviewReminder;
|
||||
use crate::context::GuardianNodeReplPolicy;
|
||||
use crate::context_manager::ContextManager;
|
||||
use crate::environment_selection::TurnEnvironmentSnapshot;
|
||||
use crate::image_preparation::ImagePreparationMode;
|
||||
use crate::image_preparation::resize_image;
|
||||
use crate::image_preparation::unified_image_budget_enabled;
|
||||
use crate::session::SessionIo;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
@@ -84,7 +80,6 @@ use codex_protocol::turn_input::TurnInputMode;
|
||||
use codex_protocol::turn_input::TurnInputRequest;
|
||||
use codex_protocol::turn_input::TurnInputSubmission;
|
||||
use codex_thread_store::PersistContext;
|
||||
use codex_tools::normalize_output_image_detail;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
use super::ApprovalRequestReasons;
|
||||
@@ -97,7 +92,6 @@ use super::prompt::GuardianPromptMode;
|
||||
#[cfg(test)]
|
||||
use super::prompt::GuardianTranscriptCursor;
|
||||
use super::prompt::GuardianTranscriptHistory;
|
||||
use super::prompt::build_guardian_prompt_items_with_parent_turn;
|
||||
use super::review::guardian_review_session_config;
|
||||
pub(crate) use super::reviewer_config::build_guardian_review_session_config;
|
||||
use codex_guardian_reviewer::run_before_review_deadline;
|
||||
@@ -473,86 +467,17 @@ async fn run_review_on_session(
|
||||
};
|
||||
analytics_result.had_prior_review_context =
|
||||
Some(had_prior_review_context(&prompt_mode));
|
||||
let mut prompt_items = build_guardian_prompt_items_with_parent_turn(
|
||||
params.parent_session.as_ref(),
|
||||
let prompt_items = prompt_builder::build_guardian_prompt_items(
|
||||
params,
|
||||
review_session,
|
||||
&model_info,
|
||||
&reviewer_history,
|
||||
history,
|
||||
Some(¶ms.parent_context),
|
||||
params.reasons.clone(),
|
||||
params.request.clone(),
|
||||
prompt_mode,
|
||||
last_admitted_node_repl_response_sequence,
|
||||
)
|
||||
.await?;
|
||||
|
||||
if prompt_items
|
||||
.context
|
||||
.section_costs()
|
||||
.any(|(_, cost)| cost.image_count > 0)
|
||||
{
|
||||
let reviewer_history = review_session.session.clone_history().await;
|
||||
let context_window = model_info.resolved_context_window().map(|supported| {
|
||||
params
|
||||
.spawn_config
|
||||
.model_context_window
|
||||
.unwrap_or(supported)
|
||||
.min(supported)
|
||||
.saturating_mul(model_info.effective_context_window_percent.clamp(0, 100))
|
||||
/ 100
|
||||
});
|
||||
let context_window = context_window.filter(|limit| {
|
||||
*limit > 0
|
||||
&& !model_info.used_fallback_model_metadata
|
||||
&& model_info.input_modalities.contains(&InputModality::Image)
|
||||
});
|
||||
let features = ¶ms.spawn_config.features;
|
||||
let mode = if unified_image_budget_enabled(features, &model_info) {
|
||||
ImagePreparationMode::UnifiedBudget
|
||||
} else {
|
||||
ImagePreparationMode::DetailBased
|
||||
};
|
||||
codex_guardian_context::prepare_sync_images(
|
||||
&mut prompt_items.context,
|
||||
reviewer_history.raw_items(),
|
||||
context_window,
|
||||
|image, detail| {
|
||||
*detail = match normalize_output_image_detail(&model_info, *detail) {
|
||||
_ if mode == ImagePreparationMode::UnifiedBudget => {
|
||||
Some(ImageDetail::Original)
|
||||
}
|
||||
Some(ImageDetail::Low) => Some(ImageDetail::High),
|
||||
detail => detail,
|
||||
};
|
||||
match image {
|
||||
ImageReference::Inline { image_url } => {
|
||||
match resize_image(image_url, detail, mode) {
|
||||
Ok(Some(prepared)) => {
|
||||
Some(Cow::Owned(ImageReference::Inline {
|
||||
image_url: prepared.into_data_url(),
|
||||
}))
|
||||
}
|
||||
Ok(None) => Some(Cow::Borrowed(image)),
|
||||
Err(error) => {
|
||||
warn!(%error, "failed to prepare guardian review image");
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
ImageReference::File { .. } => Some(Cow::Borrowed(image)),
|
||||
}
|
||||
},
|
||||
crate::context_manager::estimate_item_token_count,
|
||||
|| async {
|
||||
let base_instructions =
|
||||
review_session.session.get_base_instructions().await;
|
||||
reviewer_history
|
||||
.estimate_token_count_with_base_instructions(&base_instructions)
|
||||
.unwrap_or(i64::MAX)
|
||||
.max(review_session.session.get_total_token_usage().await)
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
let items = match prompt_items.context.clone().into_user_inputs() {
|
||||
Ok(items) => items,
|
||||
Err(codex_guardian_context::SectionError::UnsupportedDelivery {
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
//! Captures synchronous evidence once for the common preparation pass.
|
||||
//! Reviewer-history token measurement is needed only for captured images.
|
||||
|
||||
use codex_guardian_reviewer::GuardianPromptItems;
|
||||
use codex_protocol::models::ImageDetail;
|
||||
use codex_protocol::models::ImageReference;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_tools::normalize_output_image_detail;
|
||||
use std::borrow::Cow;
|
||||
use tracing::warn;
|
||||
|
||||
use super::super::prompt::GuardianPromptMode;
|
||||
use super::super::prompt::GuardianTranscriptHistory;
|
||||
use super::super::prompt::collect_guardian_prompt_items_with_parent_turn;
|
||||
use super::GuardianReviewSession;
|
||||
use super::GuardianReviewSessionParams;
|
||||
use crate::context_manager::ContextManager;
|
||||
use crate::image_preparation::ImagePreparationMode;
|
||||
use crate::image_preparation::resize_image;
|
||||
use crate::image_preparation::unified_image_budget_enabled;
|
||||
|
||||
pub(super) async fn build_guardian_prompt_items(
|
||||
params: &GuardianReviewSessionParams,
|
||||
reviewer: &GuardianReviewSession,
|
||||
model_info: &ModelInfo,
|
||||
reviewer_history: &ContextManager,
|
||||
history: GuardianTranscriptHistory<'_>,
|
||||
mode: GuardianPromptMode,
|
||||
reviewed_node_repl_evidence_sequence: u64,
|
||||
) -> anyhow::Result<GuardianPromptItems> {
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
params.parent_session.as_ref(),
|
||||
history,
|
||||
Some(¶ms.parent_context),
|
||||
params.reasons.clone(),
|
||||
params.request.clone(),
|
||||
mode,
|
||||
reviewed_node_repl_evidence_sequence,
|
||||
)
|
||||
.await?;
|
||||
let context_window = model_info.resolved_context_window().map(|supported| {
|
||||
params
|
||||
.spawn_config
|
||||
.model_context_window
|
||||
.unwrap_or(supported)
|
||||
.min(supported)
|
||||
.saturating_mul(model_info.effective_context_window_percent.clamp(0, 100))
|
||||
/ 100
|
||||
});
|
||||
let context_window =
|
||||
context_window.filter(|limit| *limit > 0 && !model_info.used_fallback_model_metadata);
|
||||
let image_mode = if unified_image_budget_enabled(¶ms.spawn_config.features, model_info) {
|
||||
ImagePreparationMode::UnifiedBudget
|
||||
} else {
|
||||
ImagePreparationMode::DetailBased
|
||||
};
|
||||
let history_tokens = if context_window.is_some() && collected.sections.has_images() {
|
||||
let base_instructions = reviewer.session.get_base_instructions().await;
|
||||
reviewer_history
|
||||
.estimate_token_count_with_base_instructions(&base_instructions)
|
||||
.unwrap_or(i64::MAX)
|
||||
.max(reviewer.session.get_total_token_usage().await)
|
||||
} else {
|
||||
i64::MAX
|
||||
};
|
||||
let image_preparer = ReviewImagePreparer {
|
||||
model_info,
|
||||
mode: image_mode,
|
||||
};
|
||||
codex_guardian_reviewer::compose_guardian_prompt_with_preparation(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
codex_guardian_context::SyncPreparation {
|
||||
reviewer_history: reviewer_history.annotated_items(),
|
||||
context_window,
|
||||
history_tokens,
|
||||
supports_images: model_info.input_modalities.contains(&InputModality::Image),
|
||||
image_preparer: &image_preparer,
|
||||
estimate_tokens: crate::context_manager::estimate_item_token_count,
|
||||
},
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
struct ReviewImagePreparer<'a> {
|
||||
model_info: &'a ModelInfo,
|
||||
mode: ImagePreparationMode,
|
||||
}
|
||||
|
||||
impl codex_guardian_context::SyncImagePreparer for ReviewImagePreparer<'_> {
|
||||
fn prepare<'image>(
|
||||
&self,
|
||||
image: &'image ImageReference,
|
||||
detail: &mut Option<ImageDetail>,
|
||||
) -> Option<Cow<'image, ImageReference>> {
|
||||
*detail = match normalize_output_image_detail(self.model_info, *detail) {
|
||||
_ if self.mode == ImagePreparationMode::UnifiedBudget => Some(ImageDetail::Original),
|
||||
Some(ImageDetail::Low) => Some(ImageDetail::High),
|
||||
detail => detail,
|
||||
};
|
||||
match image {
|
||||
ImageReference::Inline { image_url } => {
|
||||
match resize_image(image_url, detail, self.mode) {
|
||||
Ok(Some(prepared)) => Some(Cow::Owned(ImageReference::Inline {
|
||||
image_url: prepared.into_data_url(),
|
||||
})),
|
||||
Ok(None) => Some(Cow::Borrowed(image)),
|
||||
Err(error) => {
|
||||
warn!(%error, "failed to prepare guardian review image");
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
ImageReference::File { .. } => Some(Cow::Borrowed(image)),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -408,7 +408,7 @@ async fn build_guardian_prompt_prefers_retry_reason_over_approval_reason() -> an
|
||||
seed_guardian_parent_history(&session, &turn).await;
|
||||
let context = GuardianReviewContext::from(&turn);
|
||||
|
||||
let prompt = build_guardian_prompt_items_with_parent_turn(
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
session.as_ref(),
|
||||
GuardianTranscriptHistory::Retained(session.conversation_history_snapshot().await.as_ref()),
|
||||
Some(&context),
|
||||
@@ -431,6 +431,14 @@ async fn build_guardian_prompt_prefers_retry_reason_over_approval_reason() -> an
|
||||
/*reviewed_node_repl_evidence_sequence*/ 0,
|
||||
)
|
||||
.await?;
|
||||
let prompt = codex_guardian_reviewer::compose_guardian_prompt(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
)?;
|
||||
|
||||
let text = guardian_prompt_text(&prompt.context.into_user_inputs()?);
|
||||
assert!(text.contains("Retry reason:\nThe sandbox blocked the initial command.\n\n"));
|
||||
@@ -450,7 +458,7 @@ async fn build_guardian_prompt_truncates_oversized_approval_reason() -> anyhow::
|
||||
codex_utils_output_truncation::TruncationPolicy::Tokens(/*tokens*/ 512),
|
||||
);
|
||||
|
||||
let prompt = build_guardian_prompt_items_with_parent_turn(
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
session.as_ref(),
|
||||
GuardianTranscriptHistory::Retained(session.conversation_history_snapshot().await.as_ref()),
|
||||
Some(&context),
|
||||
@@ -473,6 +481,14 @@ async fn build_guardian_prompt_truncates_oversized_approval_reason() -> anyhow::
|
||||
/*reviewed_node_repl_evidence_sequence*/ 0,
|
||||
)
|
||||
.await?;
|
||||
let prompt = codex_guardian_reviewer::compose_guardian_prompt(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
)?;
|
||||
|
||||
let items = prompt.context.into_user_inputs()?;
|
||||
let reason_item = items
|
||||
@@ -545,7 +561,7 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re
|
||||
seed_guardian_parent_history(&session, &turn).await;
|
||||
let context = GuardianReviewContext::from(&turn);
|
||||
|
||||
let prompt = build_guardian_prompt_items_with_parent_turn(
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
session.as_ref(),
|
||||
GuardianTranscriptHistory::Retained(session.conversation_history_snapshot().await.as_ref()),
|
||||
Some(&context),
|
||||
@@ -568,6 +584,14 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re
|
||||
/*reviewed_node_repl_evidence_sequence*/ 0,
|
||||
)
|
||||
.await?;
|
||||
let prompt = codex_guardian_reviewer::compose_guardian_prompt(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
)?;
|
||||
|
||||
let text = guardian_prompt_text(&prompt.context.into_user_inputs()?);
|
||||
assert!(text.contains("PARENT TURN PERMISSION CONTEXT START"));
|
||||
@@ -673,7 +697,7 @@ async fn approval_permissions_use_the_owning_environment() -> anyhow::Result<()>
|
||||
environment_id,
|
||||
);
|
||||
let is_windows = request.target_environment_id() == Some("windows");
|
||||
let prompt = build_guardian_prompt_items_with_parent_turn(
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
&session,
|
||||
GuardianTranscriptHistory::Retained(
|
||||
session.conversation_history_snapshot().await.as_ref(),
|
||||
@@ -685,6 +709,14 @@ async fn approval_permissions_use_the_owning_environment() -> anyhow::Result<()>
|
||||
/*reviewed_node_repl_evidence_sequence*/ 0,
|
||||
)
|
||||
.await?;
|
||||
let prompt = codex_guardian_reviewer::compose_guardian_prompt(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
)?;
|
||||
let text = guardian_prompt_text(&prompt.context.into_user_inputs()?);
|
||||
assert!(text.contains(&format!(
|
||||
"The active permission profile for environment {environment_id:?}"
|
||||
@@ -763,7 +795,7 @@ async fn guardian_mcp_uses_thread_permissions_for_an_unavailable_captured_enviro
|
||||
&turn.initial_settings,
|
||||
&captured,
|
||||
);
|
||||
let prompt = build_guardian_prompt_items_with_parent_turn(
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
&session,
|
||||
GuardianTranscriptHistory::Retained(session.conversation_history_snapshot().await.as_ref()),
|
||||
Some(&context),
|
||||
@@ -773,6 +805,14 @@ async fn guardian_mcp_uses_thread_permissions_for_an_unavailable_captured_enviro
|
||||
/*reviewed_node_repl_evidence_sequence*/ 0,
|
||||
)
|
||||
.await?;
|
||||
let prompt = codex_guardian_reviewer::compose_guardian_prompt(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
)?;
|
||||
let text = guardian_prompt_text(&prompt.context.into_user_inputs()?);
|
||||
assert!(text.contains(captured_root.join("private").to_string_lossy().as_ref()));
|
||||
assert!(!text.contains(original_root.join("private").to_string_lossy().as_ref()));
|
||||
@@ -1273,7 +1313,7 @@ async fn build_guardian_prompt_items_keeps_required_node_repl_reviews_generic()
|
||||
seed_guardian_parent_history(&session, &turn).await;
|
||||
let context = GuardianReviewContext::from(&turn);
|
||||
|
||||
let prompt = build_guardian_prompt_items_with_parent_turn(
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
session.as_ref(),
|
||||
GuardianTranscriptHistory::Retained(session.conversation_history_snapshot().await.as_ref()),
|
||||
Some(&context),
|
||||
@@ -1286,6 +1326,14 @@ async fn build_guardian_prompt_items_keeps_required_node_repl_reviews_generic()
|
||||
/*reviewed_node_repl_evidence_sequence*/ 0,
|
||||
)
|
||||
.await?;
|
||||
let prompt = codex_guardian_reviewer::compose_guardian_prompt(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
)?;
|
||||
|
||||
let text = guardian_prompt_text(&prompt.context.into_user_inputs()?);
|
||||
assert!(text.contains("Assess the exact planned action below."));
|
||||
@@ -1312,7 +1360,7 @@ async fn build_guardian_prompt_items_keeps_other_requests_generic() -> anyhow::R
|
||||
guardian_mcp_request("another_server", "js"),
|
||||
guardian_exec_command_request("shell-1"),
|
||||
] {
|
||||
let prompt = build_guardian_prompt_items_with_parent_turn(
|
||||
let collected = collect_guardian_prompt_items_with_parent_turn(
|
||||
session.as_ref(),
|
||||
GuardianTranscriptHistory::Retained(
|
||||
session.conversation_history_snapshot().await.as_ref(),
|
||||
@@ -1324,6 +1372,14 @@ async fn build_guardian_prompt_items_keeps_other_requests_generic() -> anyhow::R
|
||||
/*reviewed_node_repl_evidence_sequence*/ 0,
|
||||
)
|
||||
.await?;
|
||||
let prompt = codex_guardian_reviewer::compose_guardian_prompt(
|
||||
collected.sections,
|
||||
&collected.profile,
|
||||
collected.mode,
|
||||
collected.history_version,
|
||||
&collected.session_id,
|
||||
collected.node_repl_evidence_sequence,
|
||||
)?;
|
||||
|
||||
let text = guardian_prompt_text(&prompt.context.into_user_inputs()?);
|
||||
assert!(text.contains("Assess the exact planned action below."));
|
||||
|
||||
@@ -97,6 +97,7 @@ pub use pool::SessionDisposition;
|
||||
pub use prompt::GUARDIAN_TRANSCRIPT_START;
|
||||
pub use prompt::GuardianPromptItems;
|
||||
pub use prompt::compose_guardian_prompt;
|
||||
pub use prompt::compose_guardian_prompt_with_preparation;
|
||||
|
||||
pub use review::ReviewHost;
|
||||
pub use review_action::ReviewAction;
|
||||
|
||||
@@ -25,6 +25,50 @@ pub fn compose_guardian_prompt(
|
||||
session_id: &str,
|
||||
node_repl_evidence_sequence: u64,
|
||||
) -> anyhow::Result<GuardianPromptItems> {
|
||||
let (offset, presentation, transcript_cursor) =
|
||||
select_transcript(§ions, mode, history_version, session_id);
|
||||
let context =
|
||||
codex_guardian_context::prepare_sync_context(sections, profile, offset, presentation)?;
|
||||
Ok(GuardianPromptItems {
|
||||
context,
|
||||
transcript_cursor,
|
||||
node_repl_evidence_sequence,
|
||||
})
|
||||
}
|
||||
|
||||
/// Prepares and composes any sync request after the host resolves reviewer/model facts.
|
||||
pub async fn compose_guardian_prompt_with_preparation(
|
||||
sections: CollectedContext,
|
||||
profile: &ContextProfile,
|
||||
mode: GuardianPromptMode,
|
||||
history_version: u64,
|
||||
session_id: &str,
|
||||
node_repl_evidence_sequence: u64,
|
||||
preparation: codex_guardian_context::SyncPreparation<'_>,
|
||||
) -> anyhow::Result<GuardianPromptItems> {
|
||||
let (offset, presentation, transcript_cursor) =
|
||||
select_transcript(§ions, mode, history_version, session_id);
|
||||
let context = codex_guardian_context::prepare_sync_context_with_facts(
|
||||
sections,
|
||||
profile,
|
||||
offset,
|
||||
presentation,
|
||||
preparation,
|
||||
)
|
||||
.await?;
|
||||
Ok(GuardianPromptItems {
|
||||
context,
|
||||
transcript_cursor,
|
||||
node_repl_evidence_sequence,
|
||||
})
|
||||
}
|
||||
|
||||
fn select_transcript<'a>(
|
||||
sections: &CollectedContext,
|
||||
mode: GuardianPromptMode,
|
||||
history_version: u64,
|
||||
session_id: &'a str,
|
||||
) -> (usize, ContextPresentation<'a>, GuardianTranscriptCursor) {
|
||||
let (selection, transcript_cursor) =
|
||||
mode.select(sections.transcript_entries(), history_version);
|
||||
let (offset, presentation) = match selection {
|
||||
@@ -33,13 +77,7 @@ pub fn compose_guardian_prompt(
|
||||
(offset, ContextPresentation::SyncDelta { session_id })
|
||||
}
|
||||
};
|
||||
let context =
|
||||
codex_guardian_context::prepare_sync_context(sections, profile, offset, presentation)?;
|
||||
Ok(GuardianPromptItems {
|
||||
context,
|
||||
transcript_cursor,
|
||||
node_repl_evidence_sequence,
|
||||
})
|
||||
(offset, presentation, transcript_cursor)
|
||||
}
|
||||
|
||||
pub fn guardian_truncate_text(content: &str, token_cap: usize) -> (String, bool) {
|
||||
|
||||
@@ -137,6 +137,41 @@ pub struct ComposedContext {
|
||||
}
|
||||
|
||||
impl CollectedContext {
|
||||
/// Reports captured images without preparing or copying their payloads.
|
||||
pub fn has_images(&self) -> bool {
|
||||
let has_inline_image = |items: &[UserInput]| {
|
||||
items.iter().any(|item| {
|
||||
matches!(item, UserInput::Image { image, .. } if matches!(image, ImageReference::Inline { .. }))
|
||||
})
|
||||
};
|
||||
self.sections.iter().any(|section| match section {
|
||||
ContextSection::TranscriptImages(images) => !images.images.is_empty(),
|
||||
ContextSection::NodeReplEvidence(evidence) => match &evidence.content {
|
||||
crate::node_repl::NodeReplEvidenceContent::Collected {
|
||||
responses, mode, ..
|
||||
} => {
|
||||
*mode == crate::NodeReplReviewEvidenceMode::Multimodal
|
||||
&& responses
|
||||
.iter()
|
||||
.any(|response| has_inline_image(&response.items))
|
||||
}
|
||||
crate::node_repl::NodeReplEvidenceContent::Prepared(items) => {
|
||||
has_inline_image(items)
|
||||
}
|
||||
},
|
||||
ContextSection::ConversationTranscript { .. }
|
||||
| ContextSection::RootConversation { .. }
|
||||
| ContextSection::TrustedUserAnswers { .. }
|
||||
| ContextSection::RetainedUserInstructions { .. }
|
||||
| ContextSection::SenderUserMessages { .. }
|
||||
| ContextSection::PlannedAction(_)
|
||||
| ContextSection::PreviousReviews(_)
|
||||
| ContextSection::TrustedTool(_)
|
||||
| ContextSection::TrustedSkills(_)
|
||||
| ContextSection::PermissionContext { .. } => false,
|
||||
})
|
||||
}
|
||||
|
||||
/// All collected entries, before the host applies retention or a delta cursor.
|
||||
pub fn transcript_entries(&self) -> &[ConversationTranscriptEntry] {
|
||||
self.sections
|
||||
|
||||
@@ -98,7 +98,11 @@ pub use composition::ComposedContext;
|
||||
pub use composition::ContextPresentation;
|
||||
pub use composition::PreparedTranscript;
|
||||
pub use preparation::ContextProfile;
|
||||
pub use preparation::SyncImagePreparer;
|
||||
pub use preparation::SyncPreparation;
|
||||
pub use preparation::prepare_sync_context;
|
||||
pub use preparation::prepare_sync_context_with_facts;
|
||||
pub use preparation::prepare_sync_delivery;
|
||||
pub use preparation::prepare_sync_images;
|
||||
pub use preparation::root::PreparedRootContext;
|
||||
pub use preparation::root::prepare_root_context;
|
||||
|
||||
@@ -18,6 +18,7 @@ use crate::RetainedInstructionItem;
|
||||
use crate::SectionError;
|
||||
use crate::retained_instructions::RetainedInstructionKind;
|
||||
use crate::retained_instructions::USER_OMISSION;
|
||||
use codex_history::ResponseItemEnvelope;
|
||||
use codex_protocol::models::ImageDetail;
|
||||
use codex_protocol::models::ImageReference;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
@@ -257,3 +258,66 @@ pub async fn prepare_sync_images<'a, HistoryCost>(
|
||||
context.retain_images(|_, _| false);
|
||||
}
|
||||
}
|
||||
|
||||
/// Applies existing instruction and native-message delivery checks in their original order.
|
||||
pub fn prepare_sync_delivery(
|
||||
context: &mut ComposedContext,
|
||||
reviewer_history: &[ResponseItemEnvelope],
|
||||
) -> Result<(), SectionError> {
|
||||
context.retain_new_instructions(reviewer_history);
|
||||
context.retain_new_native_messages(reviewer_history)
|
||||
}
|
||||
|
||||
/// Host policy for preparing an image for the review model.
|
||||
/// Implementations borrow unchanged images, own resized images, and omit rejected images.
|
||||
pub trait SyncImagePreparer: Sync {
|
||||
fn prepare<'image>(
|
||||
&self,
|
||||
image: &'image ImageReference,
|
||||
detail: &mut Option<ImageDetail>,
|
||||
) -> Option<Cow<'image, ImageReference>>;
|
||||
}
|
||||
|
||||
/// Model and reviewer facts resolved before preparing any synchronous request.
|
||||
/// Image capability is independent of the model context window used for all evidence.
|
||||
pub struct SyncPreparation<'a> {
|
||||
pub reviewer_history: &'a [ResponseItemEnvelope],
|
||||
pub context_window: Option<i64>,
|
||||
pub history_tokens: i64,
|
||||
pub supports_images: bool,
|
||||
pub image_preparer: &'a dyn SyncImagePreparer,
|
||||
pub estimate_tokens: fn(&ResponseItem) -> i64,
|
||||
}
|
||||
|
||||
/// Prepares every synchronous request with host facts available before selection.
|
||||
/// Existing entrypoints and prepare-then-compose ordering stay unchanged.
|
||||
/// Image admission uses the exact framed cost, as in the existing image policy.
|
||||
pub async fn prepare_sync_context_with_facts(
|
||||
context: CollectedContext,
|
||||
profile: &ContextProfile,
|
||||
entry_number_offset: usize,
|
||||
presentation: ContextPresentation<'_>,
|
||||
preparation: SyncPreparation<'_>,
|
||||
) -> Result<ComposedContext, SectionError> {
|
||||
let mut context = prepare_sync_context(context, profile, entry_number_offset, presentation)?;
|
||||
if context
|
||||
.section_costs()
|
||||
.any(|(_, cost)| cost.image_count > 0)
|
||||
{
|
||||
prepare_sync_images(
|
||||
&mut context,
|
||||
preparation
|
||||
.reviewer_history
|
||||
.iter()
|
||||
.map(|envelope| &envelope.item),
|
||||
preparation
|
||||
.context_window
|
||||
.filter(|_| preparation.supports_images),
|
||||
|image, detail| preparation.image_preparer.prepare(image, detail),
|
||||
preparation.estimate_tokens,
|
||||
|| std::future::ready(preparation.history_tokens),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Ok(context)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user