openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-19 17:28:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove obsolete Guardian context capture mode branches (#47690)

Browse Source 
## What changed

Remove the session-level capture mode and make thread-owned retained context the default throughout history management. Simplify input ordering, compaction identity preservation, fork handling, and approval freshness checks by removing legacy capture branches.

Keep checkpoint compatibility review separate: unknown or incompatible checkpoints can still use legacy review while retaining user evidence. Set the already-removed `guardianv2.thread_context` feature's default to false and remove redundant opt-out test variants.

## Testing

Update checkpoint tests to exercise legacy review through restored checkpoint data. Adapt existing compaction, resume, rollback, and fork tests to use unconditional context capture.

GitOrigin-RevId: ab23a0d7aca71d5a103458cff8b0c68137dd9708
This commit is contained in:
felixxia-oai
2026-09-23 22:51:13 +00:00
committed by copyberry
parent 920e1a34dc
commit 7b66c21f58
24 changed files with 213 additions and 469 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/tests/suite/v2/guardian_v2_history_tests.rs
+54 -134
View File
@@ -64,12 +64,6 @@ enum EvidenceSize {
OversizedInstruction,
}
#[derive(Clone, Copy)]
enum ContextPath {
Legacy,
ThreadOwned,
}
#[derive(Clone, Copy)]
enum CheckpointReuse {
Enabled,
@@ -85,30 +79,22 @@ enum ReviewCheckpoint {
EmptyReviewerHash,
}
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::EmptyContent; "empty checkpoint fails closed")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::DifferentReviewerHash; "different sync hash preserves retained evidence")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::UnknownReviewer; "unknown sync hash preserves retained evidence")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::EmptyReviewerHash; "empty sync hash preserves retained evidence")]
#[test_case(ContextPath::Legacy, CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::DifferentReviewerHash; "ignored opt-out preserves sync compatibility")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Disabled, Some("matching"), Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "disabled Luna reuse requires sync")]
#[test_case(ContextPath::Legacy, CheckpointReuse::Disabled, Some("matching"), Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "ignored opt-out with disabled Luna reuse requires sync")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("matching"), 0, EvidenceSize::OversizedInstruction, ReviewCheckpoint::Valid; "instruction budget preserves fresh low score")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "compatible checkpoint")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "incompatible checkpoint")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), None, 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "unknown Luna compatibility")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some(""), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "empty Luna compatibility")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, None, Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "unknown producer preserves retained evidence")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some(""), Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "empty producer preserves retained evidence")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("matching"), 140, EvidenceSize::Normal, ReviewCheckpoint::Valid; "source call evicted")]
#[test_case(ContextPath::ThreadOwned, CheckpointReuse::Enabled, Some("matching"), Some("matching"), 0, EvidenceSize::OversizedAnswer, ReviewCheckpoint::Valid; "incomplete answers reject fresh low score")]
#[test_case(ContextPath::Legacy, CheckpointReuse::Enabled, Some("matching"), Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "ignored opt-out retains answers across resume")]
#[test_case(ContextPath::Legacy, CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "ignored opt-out with incompatible checkpoint requires sync")]
#[test_case(ContextPath::Legacy, CheckpointReuse::Enabled, Some("matching"), None, 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "ignored opt-out with unknown Luna compatibility requires sync")]
#[test_case(ContextPath::Legacy, CheckpointReuse::Enabled, Some("matching"), Some("matching"), 140, EvidenceSize::Normal, ReviewCheckpoint::Valid; "ignored opt-out preserves answers after source call eviction")]
#[test_case(ContextPath::Legacy, CheckpointReuse::Enabled, Some("matching"), Some("matching"), 0, EvidenceSize::OversizedAnswer, ReviewCheckpoint::Valid; "ignored opt-out rejects incomplete answers")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::EmptyContent; "empty checkpoint fails closed")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::DifferentReviewerHash; "different sync hash preserves retained evidence")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::UnknownReviewer; "unknown sync hash preserves retained evidence")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::EmptyReviewerHash; "empty sync hash preserves retained evidence")]
#[test_case(CheckpointReuse::Disabled, Some("matching"), Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "disabled Luna reuse requires sync")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("matching"), 0, EvidenceSize::OversizedInstruction, ReviewCheckpoint::Valid; "instruction budget preserves fresh low score")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "compatible checkpoint")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("different"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "incompatible checkpoint")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), None, 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "unknown Luna compatibility")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some(""), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "empty Luna compatibility")]
#[test_case(CheckpointReuse::Enabled, None, Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "unknown producer preserves retained evidence")]
#[test_case(CheckpointReuse::Enabled, Some(""), Some("matching"), 0, EvidenceSize::Normal, ReviewCheckpoint::Valid; "empty producer preserves retained evidence")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("matching"), 140, EvidenceSize::Normal, ReviewCheckpoint::Valid; "source call evicted")]
#[test_case(CheckpointReuse::Enabled, Some("matching"), Some("matching"), 0, EvidenceSize::OversizedAnswer, ReviewCheckpoint::Valid; "incomplete answers reject fresh low score")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn guardians_retain_evidence_after_compaction_and_resume(
requested_context_path: ContextPath,
checkpoint_reuse: CheckpointReuse,
parent_hash: Option<&str>,
luna_hash: Option<&str>,
@@ -118,16 +104,13 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
) -> Result<()> {
skip_if_no_network!(Ok(()));
// Both values of the retired config setting use thread-owned context.
let context_path = ContextPath::ThreadOwned;
const RESTRICTION: &str = "Only publish to a private repository.";
const EVIDENCE: &str = "repository is private";
const SUMMARY: &str = "Repository inspection was summarized.";
let reuse_parent_compaction = matches!(checkpoint_reuse, CheckpointReuse::Enabled);
let compatible =
reuse_parent_compaction && parent_hash == Some("matching") && luna_hash == parent_hash;
let requires_sync = matches!(context_path, ContextPath::ThreadOwned) && !compatible;
let requires_sync = !compatible;
let oversized_instruction = matches!(evidence_size, EvidenceSize::OversizedInstruction);
let reviewer_hash = match review_checkpoint {
ReviewCheckpoint::DifferentReviewerHash => Some("different-reviewer"),
@@ -136,8 +119,7 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
ReviewCheckpoint::Valid | ReviewCheckpoint::EmptyContent => Some("matching"),
};
// Sync review accepts any hash metadata; only the checkpoint payload must be usable.
let reject_sync_checkpoint = matches!(context_path, ContextPath::ThreadOwned)
&& matches!(review_checkpoint, ReviewCheckpoint::EmptyContent);
let reject_sync_checkpoint = matches!(review_checkpoint, ReviewCheckpoint::EmptyContent);
let answer = match evidence_size {
EvidenceSize::Normal | EvidenceSize::OversizedInstruction => {
USER_INPUT_RESTRICTION.to_owned()
@@ -165,10 +147,7 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
| ReviewCheckpoint::UnknownReviewer
| ReviewCheckpoint::EmptyReviewerHash => {}
}
let rejects_incomplete_score = matches!(
(context_path, evidence_size),
(ContextPath::ThreadOwned, EvidenceSize::OversizedAnswer)
);
let rejects_incomplete_score = matches!(evidence_size, EvidenceSize::OversizedAnswer);
let classifier = Arc::new(MockResponsesState {
luna_score: if rejects_incomplete_score || oversized_instruction || requires_sync {
0.0
@@ -273,11 +252,7 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
});
let (mcp_url, mcp_server) = start_mcp_server(/*sensitive_action*/ None).await?;
let codex_home = TempDir::new()?;
let thread_context_enabled = match requested_context_path {
ContextPath::Legacy => false,
ContextPath::ThreadOwned => true,
};
let mut mock_config = MockResponsesConfig::new(&responses_url)
let mock_config = MockResponsesConfig::new(&responses_url)
.with_provider_name("OpenAI")
.with_provider_config("requires_openai_auth = true\nsupports_websockets = false")
.with_root_config("approvals_reviewer = \"auto_review\"\nmodel_auto_compact_token_limit = 1000000")
@@ -285,12 +260,10 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
.enable_feature(Feature::GuardianApproval)
.disable_feature(Feature::EnableRequestCompression)
.disable_feature(Feature::TokenBudget)
.disable_feature(Feature::GuardianReuseParentCompaction)
.with_extra_config(&format!(
"[mcp_servers.{TEST_SERVER_NAME}]\nurl = \"{mcp_url}/mcp\"\ndefault_tools_approval_mode = \"prompt\"\n\n[features.guardianv2]\nenabled = true\nthread_context = {thread_context_enabled}\npersist_scores = true\nreuse_parent_compaction = {reuse_parent_compaction}\n\n[features.guardianv2.review_scope]\ncomputer_use_only = false"
"[mcp_servers.{TEST_SERVER_NAME}]\nurl = \"{mcp_url}/mcp\"\ndefault_tools_approval_mode = \"prompt\"\n\n[features.guardianv2]\nenabled = true\npersist_scores = true\nreuse_parent_compaction = {reuse_parent_compaction}\n\n[features.guardianv2.review_scope]\ncomputer_use_only = false"
));
if matches!(context_path, ContextPath::ThreadOwned) {
mock_config = mock_config.disable_feature(Feature::GuardianReuseParentCompaction);
}
mock_config.write(codex_home.path())?;
let config = load_default_config_for_test(&codex_home).await;
let models = [
@@ -587,16 +560,10 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
}));
{
assert!(sync_text.contains(RESTRICTION));
if matches!(context_path, ContextPath::ThreadOwned) {
assert!(
!sync_text.contains(&expected_output),
"raw tool result must not survive the parent checkpoint"
);
} else if tool_traffic == 0 {
assert!(sync_text.contains(EVIDENCE));
assert!(sync_text.contains(&format!("tool {TEST_TOOL_NAME} result:")));
assert!(sync_text.contains(&expected_output));
}
assert!(
!sync_text.contains(&expected_output),
"raw tool result must not survive the parent checkpoint"
);
assert!(sync_text.contains(">>> TRANSCRIPT START"));
assert!(!sync_text.contains(">>> TRANSCRIPT DELTA START"));
// The endpoint receives the original evidence. The returned checkpoint is
@@ -619,61 +586,31 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
"{compact_output}"
);
assert!(parent_items.contains(&checkpoint));
if matches!(context_path, ContextPath::ThreadOwned) {
assert!(
content.contains(RESTRICTION),
"retained instruction must survive"
);
assert!(
!transcript.contains(&expected_output),
"raw tool result must not survive the parent checkpoint"
);
assert!(transcript.contains(RESTRICTION));
assert!(!transcript.contains(SUMMARY));
} else {
assert!(
transcript.contains(RESTRICTION),
"retained user restriction missing: {transcript}"
);
if tool_traffic == 0 {
assert!(transcript.contains(&format!("tool {TEST_TOOL_NAME} call:")));
assert!(transcript.contains(&format!("tool {TEST_TOOL_NAME} result:")));
assert!(transcript.contains(&expected_output));
} else {
assert!(!transcript.contains("tool request_user_input call:"));
}
}
assert!(
content.contains(RESTRICTION),
"retained instruction must survive"
);
assert!(
!transcript.contains(&expected_output),
"raw tool result must not survive the parent checkpoint"
);
assert!(transcript.contains(RESTRICTION));
assert!(!transcript.contains(SUMMARY));
}
}
for (consumer, text) in [("async", &content), ("sync", &sync_text)] {
if matches!(context_path, ContextPath::ThreadOwned) || index == 0 {
let answers = text
.split_once(">>> TRUSTED USER ANSWERS START")
.unwrap_or_else(|| {
panic!("missing {consumer} answers at step {index}: {text}")
})
.1;
match evidence_size {
EvidenceSize::Normal | EvidenceSize::OversizedInstruction => {
assert!(answers.contains("assistant: Can I keep using the browser?"));
assert!(answers.contains(&format!("user: {USER_INPUT_RESTRICTION}")));
}
EvidenceSize::OversizedAnswer => match context_path {
ContextPath::ThreadOwned => {
assert!(
answers.contains("some verified user answers are unavailable")
);
}
ContextPath::Legacy => {
assert!(answers.contains("<truncated omitted_approx_tokens="));
assert!(
!answers.contains("some verified user answers are unavailable")
);
}
},
let answers = text
.split_once(">>> TRUSTED USER ANSWERS START")
.unwrap_or_else(|| panic!("missing {consumer} answers at step {index}: {text}"))
.1;
match evidence_size {
EvidenceSize::Normal | EvidenceSize::OversizedInstruction => {
assert!(answers.contains("assistant: Can I keep using the browser?"));
assert!(answers.contains(&format!("user: {USER_INPUT_RESTRICTION}")));
}
EvidenceSize::OversizedAnswer => {
assert!(answers.contains("some verified user answers are unavailable"));
}
} else {
assert!(!text.contains(">>> TRUSTED USER ANSWERS START"));
}
}
}
@@ -739,14 +676,12 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
app_server.shutdown_gracefully().await?;
let rollout = std::fs::read_to_string(thread.path.as_ref().expect("saved rollout path"))?;
if matches!(context_path, ContextPath::ThreadOwned) {
for line in rollout
.lines()
.filter_map(|line| serde_json::from_str::<Value>(line).ok())
{
if line["type"] == "compacted" {
assert!(line["payload"]["guardian_history"].is_null());
}
for line in rollout
.lines()
.filter_map(|line| serde_json::from_str::<Value>(line).ok())
{
if line["type"] == "compacted" {
assert!(line["payload"]["guardian_history"].is_null());
}
}
let items = rollout
@@ -758,8 +693,8 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
.iter()
.filter(|line| matches!(line.item, RolloutItem::RetainedContext(_)))
.count(),
usize::from(matches!(context_path, ContextPath::ThreadOwned)),
"only the enabled path may persist a retained-answer event",
1,
"verified answers must be persisted",
);
for line in &items {
if let RolloutItem::Compacted(checkpoint) = &line.item {
@@ -780,22 +715,7 @@ async fn guardians_retain_evidence_after_compaction_and_resume(
}
}
}
if matches!(context_path, ContextPath::Legacy) {
for line in &items {
if let RolloutItem::Compacted(checkpoint) = &line.item {
assert_eq!(
checkpoint
.retained_context
.as_ref()
.expect("retained context checkpoint")
.verified_answers()
.count(),
0,
"flag-off compaction must not populate retained answers",
);
}
}
}
mcp_server.abort();
responses_server.abort();
Ok(())
codex-rs/core/src/agent/control/spawn.rs
+6 -19
View File
@@ -15,7 +15,6 @@ use crate::context::ContextualUserFragment;
use crate::context::CurrentTimeReminder;
use crate::context::CurrentTimeUnavailable;
use crate::context::DeveloperInstructions;
use crate::context::GuardianContextMode;
use crate::context::ManagedDeveloperInstructions;
use crate::context::MultiAgentModeInstructions;
use crate::context::MultiAgentRoleInstructions;
@@ -118,11 +117,7 @@ fn keep_forked_rollout_item(item: &RolloutItem, preserve_context_baselines: bool
}
}
fn retain_forked_developer_message(
item: &mut ResponseItem,
usage_hint_texts: &[String],
context_mode: GuardianContextMode,
) -> bool {
fn retain_forked_developer_message(item: &mut ResponseItem, usage_hint_texts: &[String]) -> bool {
if !matches!(item, ResponseItem::Message { role, .. } if role == "developer") {
return true;
}
@@ -131,9 +126,7 @@ fn retain_forked_developer_message(
return false;
};
content.retain(|content_item| {
if context_mode == GuardianContextMode::ThreadOwned
&& content_item.kind().0 == "guardian.approved_action"
{
if content_item.kind().0 == "guardian.approved_action" {
return false;
}
let ContentItem::InputText { text } = content_item.content() else {
@@ -141,10 +134,8 @@ fn retain_forked_developer_message(
};
!(MultiAgentRoleInstructions::matches_text(text)
|| (context_mode == GuardianContextMode::ThreadOwned
&& text.starts_with(
crate::guardian::AUTO_REVIEW_DENIED_ACTION_APPROVAL_DEVELOPER_PREFIX,
))
|| text
.starts_with(crate::guardian::AUTO_REVIEW_DENIED_ACTION_APPROVAL_DEVELOPER_PREFIX)
|| MultiAgentModeInstructions::matches_text(text)
|| CurrentTimeReminder::matches_text(text)
|| CurrentTimeUnavailable::matches_text(text)
@@ -980,14 +971,12 @@ impl LocalAgentControl {
break;
}
}
let context_mode = GuardianContextMode::ThreadOwned;
let mut replaced_parent_developer_instructions = false;
// Scrub inherited hints and replace only the parent's developer-instruction fragment.
// Compaction stores response items separately, so sanitize both top-level messages and
// compacted replacement histories with the same policy.
let retain_forked_item = |envelope: &mut ResponseItemEnvelope, replaced: &mut bool| {
if context_mode == GuardianContextMode::ThreadOwned
&& multi_agent_version == MultiAgentVersion::V2
if multi_agent_version == MultiAgentVersion::V2
&& matches!(&envelope.item, ResponseItem::Message { role, .. } if role == "user" || role == "assistant")
{
// Persist the scope of every inherited conversational message, including the suffix
@@ -1011,7 +1000,6 @@ impl LocalAgentControl {
if !retain_forked_developer_message(
response_item,
&multi_agent_v2_usage_hint_texts_to_filter,
context_mode,
) {
return false;
}
@@ -1097,8 +1085,7 @@ impl LocalAgentControl {
compacted.guardian_history = None;
// Only V2 fetches root authorization live. Its local scope starts known-empty;
// V1 must remain incomplete when inherited authorization has been stripped.
compacted.retained_context = (context_mode == GuardianContextMode::ThreadOwned
&& multi_agent_version == MultiAgentVersion::V2)
compacted.retained_context = (multi_agent_version == MultiAgentVersion::V2)
.then(codex_history::RetainedContext::default);
if let Some(replacement_history) = compacted.replacement_history.as_mut() {
// Matches before this checkpoint cannot survive its replacement history.
codex-rs/core/src/agent/control_tests.rs
+2 -12
View File
@@ -1927,17 +1927,11 @@ async fn spawn_agent_without_fork_from_paginated_parent_stays_fresh_and_paginate
.expect("parent shutdown should submit");
}
#[test_case::test_case(true; "thread context enabled")]
#[test_case::test_case(false; "thread context disabled")]
#[tokio::test]
async fn spawn_agent_fork_sanitizes_inherited_compaction_metadata(thread_context_enabled: bool) {
async fn spawn_agent_fork_sanitizes_inherited_compaction_metadata() {
let mut harness = AgentControlHarness::new().await;
let _ = harness.config.features.disable(Feature::MultiAgentV2);
harness
.config
.features
.set_enabled(Feature::GuardianThreadContext, thread_context_enabled)
.expect("test context mode");
let (parent_thread_id, parent_thread) = harness.start_paginated_thread().await;
let parent_resume_metadata = codex_history::CompactionResumeMetadata {
multi_agent_version: Some(MultiAgentVersion::V2),
@@ -2178,9 +2172,6 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
let harness = AgentControlHarness::new().await;
let mut parent_config = harness.config.clone();
let _ = parent_config.features.enable(Feature::MultiAgentV2);
let _ = parent_config
.features
.enable(Feature::GuardianThreadContext);
parent_config.developer_instructions = Some("Parent developer instructions.".to_string());
parent_config.multi_agent_v2.root_agent_usage_hint_text =
Some("Parent root guidance.".to_string());
@@ -2188,7 +2179,6 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() {
Some("Parent subagent guidance.".to_string());
let mut child_config = harness.config.clone();
let _ = child_config.features.enable(Feature::MultiAgentV2);
let _ = child_config.features.enable(Feature::GuardianThreadContext);
child_config.developer_instructions = Some("Child developer instructions.".to_string());
child_config.multi_agent_v2.subagent_developer_instructions =
Some("Child developer instructions.".to_string());
codex-rs/core/src/compact.rs
+1 -19
View File
@@ -1,4 +1,3 @@
use crate::context::GuardianContextMode;
use std::sync::Arc;
use std::time::Instant;
@@ -358,12 +357,7 @@ async fn run_compact_task_inner_impl(
get_last_assistant_message_from_turn(history_snapshot.raw_items()).unwrap_or_default()
};
let summary_text = format!("{SUMMARY_PREFIX}\n{summary_suffix}");
let identity = if sess.guardian_context_mode == GuardianContextMode::ThreadOwned {
CompactedMessageIdentity::Preserve
} else {
CompactedMessageIdentity::Regenerate
};
let user_messages = collect_annotated_user_messages(history_items, identity);
let user_messages = collect_annotated_user_messages(history_items);
let mut new_history = build_compacted_history(Vec::new(), &user_messages, &summary_text);
if let Some(summary_item) = new_history.last_mut() {
@@ -552,24 +546,12 @@ pub(crate) fn collect_user_messages(items: &[ResponseItem]) -> Vec<CompactedUser
.collect()
}
pub(crate) enum CompactedMessageIdentity {
Preserve,
Regenerate,
}
pub(crate) fn collect_annotated_user_messages(
items: &[ResponseItemEnvelope],
identity: CompactedMessageIdentity,
) -> Vec<CompactedUserMessage> {
items
.iter()
.filter_map(|envelope| compacted_user_message(&envelope.item, envelope.metadata.clone()))
.map(|mut message| {
if matches!(identity, CompactedMessageIdentity::Regenerate) {
message.id = None;
}
message
})
.collect()
}
codex-rs/core/src/compact_tests.rs
+1 -1
View File
@@ -309,7 +309,7 @@ fn collect_annotated_user_messages_extracts_user_text_only() {
ResponseItemEnvelope::new(ResponseItem::Other),
];
let collected = collect_annotated_user_messages(&items, CompactedMessageIdentity::Preserve);
let collected = collect_annotated_user_messages(&items);
assert_eq!(
vec![CompactedUserMessage {
codex-rs/core/src/context/guardian_context_mode.rs
+4 -5
View File
@@ -1,14 +1,14 @@
//! Session capture policy and the reviewer policy carried by each history snapshot.
//! Reviewer policy carried by each history snapshot.
//! Unknown or incompatible checkpoints keep legacy review alongside retained user evidence.
use codex_extension_api::ConversationHistorySnapshot;
use codex_history::ResponseItemEnvelope;
/// Selects legacy compatibility or thread-owned evidence.
/// Selects checkpoint compatibility review or thread-owned evidence.
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
pub enum GuardianContextMode {
#[default]
Legacy,
#[default]
ThreadOwned,
}
@@ -23,14 +23,13 @@ impl GuardianContextMode {
}
pub(crate) fn for_checkpoint(
self,
items: &[ResponseItemEnvelope],
reviewer_compaction_hash: Option<&str>,
) -> Self {
if codex_history::CompactionCheckpoint::latest(items)
.is_none_or(|checkpoint| checkpoint.is_compatible_with(reviewer_compaction_hash))
{
self
Self::ThreadOwned
} else {
Self::Legacy
}
codex-rs/core/src/context_manager/history.rs
+26 -51
View File
@@ -84,8 +84,6 @@ pub(crate) struct ContextManager {
review_history: Option<TranscriptHistory>,
/// Host facts independent of the model window; snapshots share immutable state.
retained_context: Arc<RetainedContext>,
/// Capture follows the session flag, including while an older checkpoint uses legacy review.
guardian_context_mode: GuardianContextMode,
/// Reviewer policy travels with the history snapshot, independently of capture.
guardian_review_mode: GuardianContextMode,
retain_inherited_user_messages: bool,
@@ -115,7 +113,6 @@ struct SharedConversationHistory {
items: Arc<Vec<ResponseItemEnvelope>>,
review_history: Option<TranscriptHistory>,
retained_context: Arc<RetainedContext>,
guardian_context_mode: GuardianContextMode,
guardian_review_mode: GuardianContextMode,
history_version: u64,
user_message_revision: u64,
@@ -134,8 +131,7 @@ impl ConversationHistorySnapshot for SharedConversationHistory {
}
fn retained_context(&self) -> Option<&RetainedContext> {
(self.guardian_context_mode == GuardianContextMode::ThreadOwned)
.then_some(&self.retained_context)
Some(&self.retained_context)
}
fn uses_parent_context_for_review(&self) -> bool {
@@ -191,8 +187,7 @@ impl ContextManager {
items: Arc::new(Vec::new()),
review_history: None,
retained_context: Arc::default(),
guardian_context_mode: GuardianContextMode::Legacy,
guardian_review_mode: GuardianContextMode::Legacy,
guardian_review_mode: GuardianContextMode::ThreadOwned,
retain_inherited_user_messages: false,
history_version: 0,
reset_version: 0,
@@ -210,7 +205,6 @@ impl ContextManager {
items: Arc::clone(&self.items),
review_history: self.review_history.clone(),
retained_context: Arc::clone(&self.retained_context),
guardian_context_mode: self.guardian_context_mode,
guardian_review_mode: self.guardian_review_mode,
history_version: self.history_version,
user_message_revision: self.user_message_revision,
@@ -221,16 +215,9 @@ impl ContextManager {
&self.retained_context
}
pub(crate) fn with_guardian_context_mode(
guardian_context_mode: GuardianContextMode,
source: &SessionSource,
) -> Self {
pub(crate) fn for_session(source: &SessionSource) -> Self {
Self {
guardian_context_mode,
guardian_review_mode: guardian_context_mode,
retain_inherited_user_messages: guardian_context_mode
== GuardianContextMode::ThreadOwned
&& !source.is_non_root_agent(),
retain_inherited_user_messages: !source.is_non_root_agent(),
..Self::new()
}
}
@@ -314,32 +301,26 @@ impl ContextManager {
})
});
self.guardian_review_mode = if requires_parent_context {
self.guardian_context_mode
GuardianContextMode::ThreadOwned
} else {
self.guardian_context_mode
.for_checkpoint(&self.items, reviewer_compaction_hash)
GuardianContextMode::for_checkpoint(&self.items, reviewer_compaction_hash)
};
self.restore_retained_context(retained_context);
if self.guardian_context_mode == GuardianContextMode::ThreadOwned {
// Older retained checkpoints cleared oversized instructions. Recover their
// bounded root excerpts before discarding the legacy source transcript.
let items = &self.items;
Arc::make_mut(&mut self.retained_context).recover_user_message_excerpts(|id| {
// Prefer the backup over a compacted copy that retains the original ID.
let original = checkpoint
.into_iter()
.flat_map(|checkpoint| &checkpoint.0)
.chain(items.iter().map(|envelope| &envelope.item))
.find(|item| item.id().is_some_and(|item_id| item_id.as_str() == id));
let Some(TurnItem::UserMessage(original)) = original.and_then(parse_turn_item)
else {
return None;
};
Some(
guardian_truncate_text(&original.message(), GUARDIAN_MAX_ROOT_MESSAGE_TOKENS).0,
)
});
}
// Older retained checkpoints cleared oversized instructions. Recover their
// bounded root excerpts before discarding the legacy source transcript.
let items = &self.items;
Arc::make_mut(&mut self.retained_context).recover_user_message_excerpts(|id| {
// Prefer the backup over a compacted copy that retains the original ID.
let original = checkpoint
.into_iter()
.flat_map(|checkpoint| &checkpoint.0)
.chain(items.iter().map(|envelope| &envelope.item))
.find(|item| item.id().is_some_and(|item_id| item_id.as_str() == id));
let Some(TurnItem::UserMessage(original)) = original.and_then(parse_turn_item) else {
return None;
};
Some(guardian_truncate_text(&original.message(), GUARDIAN_MAX_ROOT_MESSAGE_TOKENS).0)
});
let retain_legacy_authorization = self.retain_inherited_user_messages
&& self.retained_context.has_missing_user_messages()
&& (checkpoint.is_some()
@@ -347,9 +328,8 @@ impl ContextManager {
// Preserve its surviving instructions without treating an opaque checkpoint's
// partial model window as a complete compatibility transcript.
|| codex_history::CompactionCheckpoint::latest(&self.items).is_none());
if (self.guardian_review_mode == GuardianContextMode::ThreadOwned
&& !retain_legacy_authorization)
|| (self.guardian_context_mode == GuardianContextMode::Legacy && checkpoint.is_none())
if self.guardian_review_mode == GuardianContextMode::ThreadOwned
&& !retain_legacy_authorization
{
self.review_history = None;
return;
@@ -486,8 +466,7 @@ impl ContextManager {
review_history.record(&processed.item);
}
Arc::make_mut(&mut self.items).push(processed);
if self.guardian_context_mode == GuardianContextMode::ThreadOwned
&& let Some(metadata) = metadata
if let Some(metadata) = metadata
&& Arc::make_mut(&mut self.retained_context).record_sender_user_messages(metadata)
{
self.user_message_revision = self.user_message_revision.saturating_add(1);
@@ -613,9 +592,7 @@ impl ContextManager {
reviewer_compaction_hash: Option<&str>,
) -> bool {
let promoted = self.guardian_review_mode == GuardianContextMode::Legacy
&& self
.guardian_context_mode
.for_checkpoint(&items, reviewer_compaction_hash)
&& GuardianContextMode::for_checkpoint(&items, reviewer_compaction_hash)
== GuardianContextMode::ThreadOwned;
if promoted {
self.guardian_review_mode = GuardianContextMode::ThreadOwned;
@@ -698,9 +675,7 @@ impl ContextManager {
self.trim_pre_turn_context_updates(&snapshot, first_instruction_turn_idx, cut_idx);
let mut retained_items = snapshot[..cut_idx].to_vec();
if self.guardian_context_mode == GuardianContextMode::ThreadOwned
&& let Some(boundary) = source.acceptance_order()
{
if let Some(boundary) = source.acceptance_order() {
// A later assistant item may have finished before an earlier-accepted
// steer was persisted. Drop its raw source too, so recovery cannot
// reintroduce context removed at the retained rollback boundary.
codex-rs/core/src/context_manager/history_tests.rs
+8 -32
View File
@@ -170,10 +170,7 @@ fn conversation_history_snapshot_binds_review_mode_and_hash_to_the_latest_item(
..Default::default()
}),
};
let mut history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&codex_protocol::protocol::SessionSource::Cli,
);
let mut history = ContextManager::for_session(&codex_protocol::protocol::SessionSource::Cli);
history.replace_annotated(vec![checkpoint.clone()]);
history.restore_review_context(
/*retained_context*/ None,
@@ -257,10 +254,7 @@ fn conversation_history_snapshot_binds_review_mode_and_hash_to_the_latest_item(
}]
}); "retained assistant context")]
fn checkpoint_retained_evidence_survives_legacy_review(saved_context: serde_json::Value) {
let mut history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&codex_protocol::protocol::SessionSource::Cli,
);
let mut history = ContextManager::for_session(&codex_protocol::protocol::SessionSource::Cli);
history.replace_annotated(vec![ResponseItemEnvelope::new(
serde_json::from_value(serde_json::json!({
"type": "compaction", "id": "unknown", "encrypted_content": "opaque checkpoint"
@@ -296,10 +290,7 @@ fn checkpoint_retained_evidence_survives_legacy_review(saved_context: serde_json
#[test]
fn checkpoint_replayed_messages_keep_legacy_review_when_the_source_survives() {
let mut history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&codex_protocol::protocol::SessionSource::Cli,
);
let mut history = ContextManager::for_session(&codex_protocol::protocol::SessionSource::Cli);
history.replace_annotated(vec![ResponseItemEnvelope::new(
serde_json::from_value(serde_json::json!({
"type": "compaction", "id": "unknown", "encrypted_content": "opaque checkpoint"
@@ -345,10 +336,7 @@ fn plaintext_checkpoint_without_backup_preserves_root_instructions(
.expect("old instruction without ordering metadata");
let saved_context: Option<RetainedContext> =
saved_context.map(|value| serde_json::from_value(value).expect("legacy retained context"));
let mut history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&SessionSource::Cli,
);
let mut history = ContextManager::for_session(&SessionSource::Cli);
history.replace(vec![instruction]);
history.restore_review_context(
saved_context.as_ref(),
@@ -369,10 +357,7 @@ fn plaintext_checkpoint_without_backup_preserves_root_instructions(
}),
}];
history.replace_compacted(compacted.clone(), Some("reviewer"));
let mut resumed = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&SessionSource::Cli,
);
let mut resumed = ContextManager::for_session(&SessionSource::Cli);
resumed.replace_annotated(compacted.clone());
resumed.restore_review_context(
Some(history.retained_context()),
@@ -429,10 +414,7 @@ fn legacy_checkpoint_rollback_keeps_answers_before_a_same_turn_steer() {
.verified_answers()
.cloned()
.collect::<Vec<_>>();
let mut history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&SessionSource::Cli,
);
let mut history = ContextManager::for_session(&SessionSource::Cli);
history.replace_annotated(checkpoint.replacement_history.take().unwrap());
history.restore_review_context(
checkpoint.retained_context.as_ref(),
@@ -1541,10 +1523,7 @@ fn drop_last_n_user_turns_preserves_prefix() {
// A steered message shares its source turn, but rollback must keep the earlier
// instruction and answer as complete evidence, including after the next compaction.
let mut history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&codex_protocol::protocol::SessionSource::Exec,
);
let mut history = ContextManager::for_session(&codex_protocol::protocol::SessionSource::Exec);
let mut expected = None;
for (id, text) in [
("restriction", "Never publish publicly."),
@@ -1594,10 +1573,7 @@ fn drop_last_n_user_turns_preserves_prefix() {
#[test]
fn rollback_removes_assistant_sources_recorded_ahead_of_queued_input() {
let mut history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&codex_protocol::protocol::SessionSource::Exec,
);
let mut history = ContextManager::for_session(&codex_protocol::protocol::SessionSource::Exec);
let original = user_msg("Staging only.");
let items = [
(original.clone(), 0),
codex-rs/core/src/context_manager/history_user_authorization.rs
+15 -23
View File
@@ -7,7 +7,6 @@ use std::sync::Arc;
use super::ContextManager;
use crate::compact::is_summary_message;
use crate::context::GuardianContextMode;
use crate::context::is_contextual_user_fragment;
use crate::event_mapping::parse_turn_item;
use crate::guardian::GUARDIAN_MAX_ROOT_MESSAGE_TOKENS;
@@ -66,20 +65,18 @@ impl ContextManager {
pub(crate) fn restore_retained_context(&mut self, checkpoint: Option<&RetainedContext>) {
Arc::make_mut(&mut self.retained_context).restore(checkpoint, &self.items);
if self.guardian_context_mode == GuardianContextMode::ThreadOwned {
let items = Arc::clone(&self.items);
for envelope in items.iter().filter(|envelope| {
envelope
.metadata
.as_ref()
.is_some_and(|metadata| metadata.delivered_assistant_message.is_some())
}) {
self.record_retained_message(
&envelope.item,
envelope.metadata.as_ref(),
RetainedMessageSource::Checkpoint,
);
}
let items = Arc::clone(&self.items);
for envelope in items.iter().filter(|envelope| {
envelope
.metadata
.as_ref()
.is_some_and(|metadata| metadata.delivered_assistant_message.is_some())
}) {
self.record_retained_message(
&envelope.item,
envelope.metadata.as_ref(),
RetainedMessageSource::Checkpoint,
);
}
if self.retain_inherited_user_messages
&& !self.retained_context.has_inherited_user_messages()
@@ -108,9 +105,8 @@ impl ContextManager {
metadata: Option<&CodexHarnessMetadata>,
source: RetainedMessageSource,
) {
if self.guardian_context_mode == GuardianContextMode::ThreadOwned
&& let Some(text) =
metadata.and_then(|metadata| metadata.delivered_assistant_message.as_ref())
if let Some(text) =
metadata.and_then(|metadata| metadata.delivered_assistant_message.as_ref())
&& let ResponseItem::FunctionCallOutput {
call_id: Some(call_id),
..
@@ -147,11 +143,7 @@ impl ContextManager {
return;
}
let inherited = metadata.is_some_and(|metadata| metadata.inherited_user_message);
if self.guardian_context_mode == GuardianContextMode::Legacy {
if !is_assistant {
Arc::make_mut(&mut self.retained_context).mark_user_messages_incomplete();
}
} else if (!inherited || self.retain_inherited_user_messages)
if (!inherited || self.retain_inherited_user_messages)
&& let ResponseItem::Message {
content,
internal_chat_message_metadata_passthrough,
codex-rs/core/src/guardian/review.rs
-1
View File
@@ -4,7 +4,6 @@
#[path = "review_request.rs"]
mod request;
use crate::context::GuardianContextMode;
use codex_analytics::GuardianApprovalRequestSource;
use codex_analytics::GuardianReviewAnalyticsResult;
use codex_core_plugins::PluginCommandAttribution;
codex-rs/core/src/guardian/review_request.rs
+10 -11
View File
@@ -163,19 +163,18 @@ impl ReviewHost for super::super::runtime::ReviewRuntime {
let root_authorization_version = prepared.root_authorization_version;
let user_message_revision = prepared.user_message_revision;
if matches!(&outcome, GuardianReviewOutcome::Completed(assessment) if assessment.outcome == GuardianAssessmentOutcome::Allow)
&& ((session.guardian_context_mode == GuardianContextMode::ThreadOwned
&& (root_authorization_version
&& ((root_authorization_version
!= session
.services
.agent_control
.get_guardian_package(session.thread_id)
.await
.map(|snapshot| snapshot.authorization_version)
|| user_message_revision
!= session
.services
.agent_control
.get_guardian_package(session.thread_id)
.conversation_history_snapshot()
.await
.map(|snapshot| snapshot.authorization_version)
|| user_message_revision
!= session
.conversation_history_snapshot()
.await
.user_message_revision()))
.user_message_revision())
|| self.history_reset.is_cancelled()
|| cancellation.is_cancelled())
{
codex-rs/core/src/session/guardian_checkpoint_tests.rs
+11 -9
View File
@@ -14,8 +14,7 @@ use test_case::test_case;
#[test_case(GuardianContextMode::ThreadOwned; "thread_owned")]
#[tokio::test]
async fn guardian_checkpoint_preserves_live_context_without_storage(mode: GuardianContextMode) {
let (mut session, turn) = make_session_and_context().await;
session.guardian_context_mode = mode;
let (session, turn) = make_session_and_context().await;
// This session has no live store. A checkpoint must still capture the complete context.
assert!(session.live_thread().is_none());
let instruction: ResponseItem = serde_json::from_value(json!({
@@ -30,7 +29,7 @@ async fn guardian_checkpoint_preserves_live_context_without_storage(mode: Guardi
let world_state = WorldStateSnapshot::from(baseline.as_object().unwrap());
{
let mut state = session.state.lock().await;
state.history = ContextManager::with_guardian_context_mode(mode, &SessionSource::default());
state.history = ContextManager::for_session(&SessionSource::default());
state
.history
.record_items([&instruction], turn.model_info().truncation_policy.into());
@@ -41,8 +40,13 @@ async fn guardian_checkpoint_preserves_live_context_without_storage(mode: Guardi
"type": "compaction", "id": "compaction-1", "encrypted_content": "summary",
}))
.unwrap();
state.history.replace_compacted(
vec![compacted.into()],
let retained = state.history.retained_context().clone();
state.history.replace_annotated(vec![compacted.into()]);
state.history.restore_review_context(
Some(&retained),
Some(&codex_history::GuardianHistoryCheckpoint(vec![
instruction.clone(),
])),
/*reviewer_compaction_hash*/ None,
);
}
@@ -88,10 +92,8 @@ async fn guardian_checkpoint_preserves_live_context_without_storage(mode: Guardi
.replace_history(Vec::new(), /*reference_context_item*/ None)
.await;
// Replay into a fresh session so preserved live state cannot mask missing checkpoint data.
let (mut fork, _) = make_session_and_context().await;
fork.guardian_context_mode = mode;
fork.state.lock().await.history =
ContextManager::with_guardian_context_mode(mode, &SessionSource::default());
let (fork, _) = make_session_and_context().await;
fork.state.lock().await.history = ContextManager::for_session(&SessionSource::default());
fork.record_initial_history(InitialHistory::Forked(items))
.await;
let restored = fork.clone_history().await;
codex-rs/core/src/session/mod.rs
+32 -40
View File
@@ -24,7 +24,6 @@ use crate::config::ManagedFeatures;
use crate::config::resolve_tool_suggest_config_from_layer_stack;
use crate::context::ContextualUserFragment;
use crate::context::DeveloperInstructions;
use crate::context::GuardianContextMode;
use crate::context::GuardianPolicy;
use crate::context::ManagedDeveloperInstructions;
use crate::context::ModelSwitchInstructions;
@@ -1742,14 +1741,9 @@ impl Session {
.zip(metadata)
.map(|(item, metadata)| ResponseItemEnvelope { item, metadata })
.collect();
let reviewer_compaction_hash =
if self.guardian_context_mode == crate::context::GuardianContextMode::ThreadOwned {
let context = crate::guardian::GuardianReviewContext::from(turn_context);
let (_, reviewer) = crate::guardian::resolve_review_model(self, &context).await;
reviewer.comp_hash.clone()
} else {
None
};
let context = crate::guardian::GuardianReviewContext::from(turn_context);
let (_, reviewer) = crate::guardian::resolve_review_model(self, &context).await;
let reviewer_compaction_hash = reviewer.comp_hash.clone();
{
let mut state = self.state.lock().await;
state.replace_annotated_history(
@@ -3613,39 +3607,37 @@ impl Session {
state
.current_time_reminder
.note_recorded_items(&response_items);
if self.guardian_context_mode == GuardianContextMode::ThreadOwned {
let pending_orders = turn_context
.extension_data
.get::<retained_context::PendingAssistantMessageOrders>();
for envelope in &mut items {
if envelope
let pending_orders = turn_context
.extension_data
.get::<retained_context::PendingAssistantMessageOrders>();
for envelope in &mut items {
if envelope
.metadata
.as_ref()
.is_some_and(|metadata| metadata.compaction_output)
{
continue;
}
if matches!(&envelope.item, ResponseItem::Message { role, .. } if role == "assistant")
|| matches!(&envelope.item, ResponseItem::FunctionCall { .. })
|| crate::context::is_user_authorization_message(&envelope.item)
{
let message_order = pending_orders.as_ref().and_then(|orders| {
orders
.0
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.remove(envelope.item.id()?.as_str())
});
// Preserve input acceptance and source-message start order.
// Synthetic messages still receive their order here.
envelope
.metadata
.as_ref()
.is_some_and(|metadata| metadata.compaction_output)
{
continue;
}
if matches!(&envelope.item, ResponseItem::Message { role, .. } if role == "assistant")
|| matches!(&envelope.item, ResponseItem::FunctionCall { .. })
|| crate::context::is_user_authorization_message(&envelope.item)
{
let message_order = pending_orders.as_ref().and_then(|orders| {
orders
.0
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.remove(envelope.item.id()?.as_str())
.get_or_insert_default()
.user_input_order
.get_or_insert_with(|| {
message_order.unwrap_or_else(|| state.history.reserve_input_order())
});
// Preserve input acceptance and source-message start order.
// Synthetic messages still receive their order here.
envelope
.metadata
.get_or_insert_default()
.user_input_order
.get_or_insert_with(|| {
message_order.unwrap_or_else(|| state.history.reserve_input_order())
});
}
}
}
state
codex-rs/core/src/session/retained_context.rs
+5 -11
View File
@@ -4,7 +4,6 @@ use std::collections::HashMap;
use std::sync::Mutex;
use std::sync::PoisonError;
use crate::context::GuardianContextMode;
use codex_history::RetainedContextEvent;
use codex_history::RolloutItem;
use codex_protocol::models::ResponseItem;
@@ -25,10 +24,9 @@ impl Session {
turn_context: &TurnContext,
item: &ResponseItem,
) {
if self.guardian_context_mode == GuardianContextMode::ThreadOwned
&& let ResponseItem::Message {
id: Some(id), role, ..
} = item
if let ResponseItem::Message {
id: Some(id), role, ..
} = item
&& role == "assistant"
{
let mut state = self.state.lock().await;
@@ -49,12 +47,8 @@ impl Session {
}
}
/// Legacy mode neither reserves a sequence nor takes the session-state lock.
pub(crate) async fn reserve_user_input_order(&self) -> Option<u64> {
if self.guardian_context_mode == GuardianContextMode::Legacy {
return None;
}
Some(self.state.lock().await.history.reserve_input_order())
pub(crate) async fn reserve_user_input_order(&self) -> u64 {
self.state.lock().await.history.reserve_input_order()
}
pub(crate) async fn record_retained_context(&self, mut event: RetainedContextEvent) {
codex-rs/core/src/session/rollout_reconstruction.rs
+3 -15
View File
@@ -1,5 +1,4 @@
use super::*;
use crate::context::GuardianContextMode;
use crate::context::world_state::WorldStateSnapshot;
use crate::context_manager::is_user_turn_boundary;
use codex_history::ResponseItemEnvelope;
@@ -393,10 +392,7 @@ impl Session {
.unwrap_or(u64::MAX);
// Build model-visible history from the selected compaction and its newer suffix.
let mut history = ContextManager::with_guardian_context_mode(
self.guardian_context_mode,
&turn_context.session_source,
);
let mut history = ContextManager::for_session(&turn_context.session_source);
let mut saw_legacy_compaction_without_replacement_history = false;
if let Some(checkpoint) = history_checkpoint
&& let Some(items) = &checkpoint.compacted.replacement_history
@@ -445,16 +441,8 @@ impl Session {
// prompt shape.
// TODO(ccunningham): if we drop support for None replacement_history compaction items,
// we can get rid of this second loop entirely and just build `history` directly in the first loop.
let identity =
if self.guardian_context_mode == GuardianContextMode::ThreadOwned {
compact::CompactedMessageIdentity::Preserve
} else {
compact::CompactedMessageIdentity::Regenerate
};
let user_messages = compact::collect_annotated_user_messages(
history.annotated_items(),
identity,
);
let user_messages =
compact::collect_annotated_user_messages(history.annotated_items());
let rebuilt = compact::build_compacted_history(
Vec::new(),
&user_messages,
codex-rs/core/src/session/rollout_reconstruction_tests.rs
+6 -15
View File
@@ -1,5 +1,4 @@
use super::*;
use crate::context::GuardianContextMode;
use super::tests::build_world_state_from_turn_context;
use super::tests::make_session_and_context;
@@ -33,12 +32,7 @@ use uuid::Uuid;
#[tokio::test]
async fn recorded_questions_share_queued_input_order_across_resume() {
let (mut session, turn) = make_session_and_context().await;
session.guardian_context_mode = GuardianContextMode::ThreadOwned;
session.state.lock().await.history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&SessionSource::default(),
);
let (session, turn) = make_session_and_context().await;
let question = |call_id: &str| {
serde_json::from_value::<ResponseItem>(json!({
"type": "function_call", "call_id": call_id,
@@ -64,7 +58,7 @@ async fn recorded_questions_share_queued_input_order_across_resume() {
ResponseItemEnvelope {
item: user_message("Yes."),
metadata: Some(codex_history::CodexHarnessMetadata {
user_input_order: reply_order,
user_input_order: Some(reply_order),
..Default::default()
}),
},
@@ -109,17 +103,14 @@ async fn recorded_questions_share_queued_input_order_across_resume() {
.collect::<Vec<_>>(),
vec![Some(0), Some(2), Some(1), None]
);
assert_eq!(session.reserve_user_input_order().await, Some(3));
assert_eq!(session.reserve_user_input_order().await, 3);
}
#[tokio::test]
async fn sender_context_follows_its_delivery_through_checkpoint_and_rollback() {
let (mut session, turn_context) = make_session_and_context().await;
session.guardian_context_mode = GuardianContextMode::ThreadOwned;
let mut live = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&SessionSource::default(),
);
let (session, turn_context) = make_session_and_context().await;
let mut live = ContextManager::for_session(&SessionSource::default());
let mut items = Vec::new();
let mut snapshots = Vec::new();
for index in 0..2 {
codex-rs/core/src/session/session.rs
+1 -7
View File
@@ -10,7 +10,6 @@ use crate::agent::api::AgentControl;
use crate::agents_md_manager::AgentsMdManager;
use crate::agents_md_manager::SessionInstructions;
use crate::config::ConstraintError;
use crate::context::GuardianContextMode;
use crate::environment_selection::ThreadEnvironments;
use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::hook_mcp_executor::CoreHookMcpExecutor;
@@ -71,7 +70,6 @@ pub(crate) struct Session {
/// The set of enabled features should be invariant for the lifetime of the
/// session.
pub(super) features: ManagedFeatures,
pub(crate) guardian_context_mode: GuardianContextMode,
pub(super) isolation: codex_extension_api::SessionIsolation,
pub(crate) tool_policy: Arc<codex_extension_api::ToolPolicy>,
pub(crate) windows_sandbox_proxy_settings_mode:
@@ -988,8 +986,6 @@ impl Session {
thread_id.to_string(),
thread_extension_init,
);
// Capture is unconditional; replay selects reviewer policy from the saved checkpoint.
let guardian_context_mode = GuardianContextMode::ThreadOwned;
thread_extension_data.insert(crate::context::GuardianReviewEvidence::default());
// Kick off independent async setup tasks in parallel to reduce startup latency.
//
@@ -1485,8 +1481,7 @@ impl Session {
let mut state = SessionState::new_with_auto_compact_window_ids(
session_configuration.clone(),
initial_auto_compact_window_ids,
ContextManager::with_guardian_context_mode(
guardian_context_mode,
ContextManager::for_session(
&session_configuration.session_source,
),
);
@@ -1766,7 +1761,6 @@ impl Session {
thread_settings_persistence: Semaphore::new(/*permits*/ 1),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
guardian_context_mode,
isolation,
tool_policy,
windows_sandbox_proxy_settings_mode,
codex-rs/core/src/session/tests.rs
+4 -11
View File
@@ -18,7 +18,6 @@ use crate::config::ConfigBuilder;
use crate::config::ConfigOverrides;
use crate::context::ContextualUserFragment;
use crate::context::DeveloperInstructions;
use crate::context::GuardianContextMode;
use crate::context::TurnAborted;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::environment_selection::ThreadEnvironments;
@@ -6402,10 +6401,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
);
let mut state = SessionState::new(session_configuration.clone());
state.history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&session_configuration.session_source,
);
state.history = ContextManager::for_session(&session_configuration.session_source);
let (environment_manager, resolved_environments) =
resolved_environments_for_configuration(&session_configuration, &default_environments)
.await;
@@ -6551,7 +6547,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
thread_settings_persistence: Semaphore::new(/*permits*/ 1),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
guardian_context_mode: GuardianContextMode::ThreadOwned,
isolation: codex_extension_api::SessionIsolation::Inherit,
tool_policy: Arc::default(),
windows_sandbox_proxy_settings_mode:
@@ -8674,10 +8670,7 @@ where
);
let mut state = SessionState::new(session_configuration.clone());
state.history = ContextManager::with_guardian_context_mode(
GuardianContextMode::ThreadOwned,
&session_configuration.session_source,
);
state.history = ContextManager::for_session(&session_configuration.session_source);
let (environment_manager, resolved_turn_environments) =
resolved_environments_for_configuration(&session_configuration, &default_environments)
.await;
@@ -8822,7 +8815,7 @@ where
thread_settings_persistence: Semaphore::new(/*permits*/ 1),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
guardian_context_mode: GuardianContextMode::ThreadOwned,
isolation: codex_extension_api::SessionIsolation::Inherit,
tool_policy: Arc::default(),
windows_sandbox_proxy_settings_mode:
codex-rs/core/src/session/turn_input.rs
+8 -10
View File
@@ -18,7 +18,6 @@ use super::session::SessionSettingsUpdate;
use super::thread_settings;
use super::turn_context::NewTurnContextOptions;
use super::turn_context::TurnContext;
use crate::context::GuardianContextMode;
use crate::state::ActiveTurn;
use crate::state::TurnState;
use crate::tasks::RegularTask;
@@ -691,7 +690,7 @@ impl Session {
TurnInput::UserInput {
content: std::mem::take(content),
client_id: client_id.clone(),
acceptance_order: self.reserve_user_input_order().await,
acceptance_order: Some(self.reserve_user_input_order().await),
}
}
input => pending_turn_input(self, input.clone(), active_turn_id).await,
@@ -731,7 +730,7 @@ async fn pending_turn_input(
SubmittedTurnInput::UserInput { content, client_id } => TurnInput::UserInput {
content,
client_id,
acceptance_order: session.reserve_user_input_order().await,
acceptance_order: Some(session.reserve_user_input_order().await),
},
SubmittedTurnInput::ResponseItem(mut item)
if matches!(
@@ -740,15 +739,14 @@ async fn pending_turn_input(
) =>
{
Session::assign_missing_response_item_id(&mut item);
let metadata = if session.guardian_context_mode == GuardianContextMode::ThreadOwned
&& let Some(messages) = session
.services
.local_agent_runtime
.capture_sender_user_messages(&item, session.thread_id, turn_id)
.await
let metadata = if let Some(messages) = session
.services
.local_agent_runtime
.capture_sender_user_messages(&item, session.thread_id, turn_id)
.await
{
Some(CodexHarnessMetadata {
user_input_order: session.reserve_user_input_order().await,
user_input_order: Some(session.reserve_user_input_order().await),
sender_user_messages: Some(Box::new(messages)),
..Default::default()
})
codex-rs/core/src/state/turn.rs
+2 -2
View File
@@ -108,8 +108,8 @@ pub(crate) struct TurnState {
/// Host receipt metadata follows the response through the asynchronous tool waiter.
pub(crate) struct AcceptedUserInputResponse {
pub(crate) response: RequestUserInputResponse,
/// Absent in legacy mode, which does not reserve or persist acceptance order.
pub(crate) acceptance_order: Option<u64>,
/// Order reserved when the host accepts this response.
pub(crate) acceptance_order: u64,
}
pub(crate) struct PendingRequestPermissions {
codex-rs/core/src/tools/handlers/request_user_input.rs
+3 -6
View File
@@ -1,4 +1,3 @@
use crate::context::GuardianContextMode;
use crate::function_tool::FunctionCallError;
use crate::tools::context::FunctionToolOutput;
use crate::tools::context::ToolInvocation;
@@ -101,10 +100,8 @@ impl RequestUserInputHandler {
"failed to serialize {REQUEST_USER_INPUT_TOOL_NAME} response: {err}"
))
})?;
// Capture follows the session flag even while an older checkpoint uses legacy review.
if turn.config.features.enabled(Feature::GuardianApproval)
&& session.guardian_context_mode == GuardianContextMode::ThreadOwned
{
// Persist answers even while an older checkpoint needs compatibility review.
if turn.config.features.enabled(Feature::GuardianApproval) {
let user_input = questions
.iter()
.filter_map(|question| {
@@ -142,7 +139,7 @@ impl RequestUserInputHandler {
call_id,
questions: user_input,
},
acceptance_order: accepted.acceptance_order,
acceptance_order: Some(accepted.acceptance_order),
})
.await;
}
codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs
+10 -33
View File
@@ -1893,15 +1893,6 @@ async fn contributor_uses_model_defaults_and_preserves_local_overrides() -> Resu
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result<()> {
assert_luna_pool_context(/*thread_context_enabled*/ true).await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn deprecated_opt_out_still_samples_with_thread_owned_context() -> Result<()> {
assert_luna_pool_context(/*thread_context_enabled*/ false).await
}
async fn assert_luna_pool_context(thread_context_enabled: bool) -> Result<()> {
skip_if_no_network!(Ok(()));
let conversation_history = vec![
@@ -1946,7 +1937,7 @@ async fn assert_luna_pool_context(thread_context_enabled: bool) -> Result<()> {
conversation_history,
r#"{"path":"README.md"}"#,
Some(TEST_GUARDIAN_POLICY),
&format!("[features.guardianv2]\nthread_context = {thread_context_enabled}\n"),
"",
/*model_defaults*/ None,
)
.await?;
@@ -2028,6 +2019,7 @@ async fn assert_luna_pool_context(thread_context_enabled: bool) -> Result<()> {
},
{"type": "input_text", "text": ">>> APPROVAL REQUEST END\n"},
]);
assert_eq!(request["input"][2]["content"], expected_content);
let score = tokio::time::timeout(ASYNC_TEST_TIMEOUT, async {
loop {
@@ -2413,21 +2405,9 @@ async fn cached_score_survives_compaction_and_internal_context_but_not_user_inpu
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn incompatible_compaction_blocks_cached_score_and_initial_cua_allowance() -> Result<()> {
assert_compaction_approval_policy(/*thread_context_enabled*/ true).await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn deprecated_opt_out_still_rejects_incompatible_compaction() -> Result<()> {
assert_compaction_approval_policy(/*thread_context_enabled*/ false).await
}
async fn assert_compaction_approval_policy(thread_context_enabled: bool) -> Result<()> {
skip_if_no_network!(Ok(()));
let fixture = GuardianFailureFixture::with_config(&format!(
"[features.guardianv2]\nthread_context = {thread_context_enabled}\n"
))
.await?;
let fixture = GuardianFailureFixture::with_config("").await?;
let thread_store = fixture.test.codex.thread_extension_data();
set_cached_score(
thread_store,
@@ -2481,7 +2461,7 @@ async fn assert_compaction_approval_policy(thread_context_enabled: bool) -> Resu
/*index*/ 1,
score_authorization,
);
// No new sample runs: only the enabled path rejects cached and initial-call approvals.
// No new sample runs: the live checkpoint rejects cached and initial-call approvals.
for (computer_use_only, prompt) in [
(false, "review action"),
(
@@ -2964,25 +2944,22 @@ async fn contributor_sends_compacted_conversation_history_to_luna() -> Result<()
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn contributor_reuses_the_latest_compatible_parent_compaction() -> Result<()> {
assert_parent_compaction_reuse(/*thread_context_enabled*/ true).await
assert_parent_compaction_reuse(/*parent_context_for_review*/ true).await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn legacy_contributor_reuses_the_latest_compatible_parent_compaction() -> Result<()> {
assert_parent_compaction_reuse(/*thread_context_enabled*/ false).await
assert_parent_compaction_reuse(/*parent_context_for_review*/ false).await
}
async fn assert_parent_compaction_reuse(thread_context_enabled: bool) -> Result<()> {
async fn assert_parent_compaction_reuse(parent_context_for_review: bool) -> Result<()> {
skip_if_no_network!(Ok(()));
let thread_server = responses::start_mock_server().await;
let test = test_codex()
.with_config(move |config| {
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
config
.features
.set_enabled(Feature::GuardianThreadContext, thread_context_enabled)
.expect("test context mode");
})
.with_pre_build_hook(|home| {
std::fs::write(
@@ -3074,7 +3051,7 @@ async fn assert_parent_compaction_reuse(thread_context_enabled: bool) -> Result<
retained,
current: conversation_history,
compaction_model_hash: parent_model.comp_hash.clone(),
retained_context: thread_context_enabled.then(codex_history::RetainedContext::default),
retained_context: parent_context_for_review.then(codex_history::RetainedContext::default),
};
thread_store.insert(parent_model);
@@ -3170,7 +3147,7 @@ async fn assert_parent_compaction_reuse(thread_context_enabled: bool) -> Result<
conversation_history: Arc::new(TestRetainedHistory {
current: TestConversationHistory(vec![latest_compaction, oversized_compaction]),
retained: Vec::new(),
retained_context: thread_context_enabled
retained_context: parent_context_for_review
.then(codex_history::RetainedContext::default),
compaction_model_hash: thread_store
.get::<ModelInfo>()
codex-rs/features/src/lib.rs
+1 -1
View File
@@ -1646,7 +1646,7 @@ pub const FEATURES: &[FeatureSpec] = &[
id: Feature::GuardianThreadContext,
key: "guardianv2.thread_context",
stage: Stage::Removed,
default_enabled: true,
default_enabled: false,
},
FeatureSpec {
id: Feature::GuardianReuseParentCompaction,
codex-rs/features/src/tests.rs
-1
View File
@@ -207,7 +207,6 @@ fn guardian_thread_context_is_ignored_with_a_migration_notice() {
},
FeatureOverrides::default(),
);
assert!(features.enabled(Feature::GuardianThreadContext));
assert_eq!(
features
.legacy_feature_usages()