diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index be02916487..f3e74ef9d2 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -67,7 +67,14 @@ pub(crate) async fn build_guardian_prompt_items( request: GuardianApprovalRequest, ) -> serde_json::Result> { let history = session.clone_history().await; - let transcript_entries = collect_guardian_transcript_entries(history.raw_items()); + let history_items = history.raw_items(); + let start_index = session + .guardian_review_session + .parent_history_boundary() + .await + .map(|boundary| boundary.min(history_items.len())) + .unwrap_or_default(); + let transcript_entries = collect_guardian_transcript_entries(&history_items[start_index..]); let planned_action_json = format_guardian_action_pretty(&request)?; let (transcript_entries, omission_note) = diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 3a491f6efb..23eed5153d 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -52,6 +52,14 @@ fn guardian_risk_level_str(level: GuardianRiskLevel) -> &'static str { } } +async fn mark_terminal_guardian_review_boundary(session: &Session) { + let boundary = session.clone_history().await.raw_items().len(); + session + .guardian_review_session + .set_parent_history_boundary(Some(boundary)) + .await; +} + /// Whether this turn should route `on-request` approval prompts through the /// guardian reviewer instead of surfacing them to the user. ARC may still /// block actions earlier in the flow. @@ -101,6 +109,7 @@ async fn run_guardian_review( .as_ref() .is_some_and(CancellationToken::is_cancelled) { + mark_terminal_guardian_review_boundary(session.as_ref()).await; session .send_event( turn.as_ref(), @@ -151,6 +160,7 @@ async fn run_guardian_review( evidence: vec![], }, GuardianReviewOutcome::Aborted => { + mark_terminal_guardian_review_boundary(session.as_ref()).await; session .send_event( turn.as_ref(), @@ -187,6 +197,7 @@ async fn run_guardian_review( } else { GuardianAssessmentStatus::Denied }; + mark_terminal_guardian_review_boundary(session.as_ref()).await; session .send_event( turn.as_ref(), @@ -249,14 +260,15 @@ pub(crate) async fn review_approval_request_with_cancel( /// it is pinned to a read-only sandbox with `approval_policy = never` and /// nonessential agent features disabled. When the cached trunk session is idle, /// later approvals append onto that same guardian conversation to preserve a -/// stable prompt-cache key. If the trunk is already busy, the review runs in an -/// ephemeral fork from the last committed trunk rollout so parallel approvals -/// do not block each other or mutate the cached thread. The trunk is recreated -/// when the effective review-session config changes, and any future compaction -/// must continue to preserve the guardian policy as exact top-level developer -/// context. It may still reuse the parent's managed-network allowlist for -/// read-only checks, but it intentionally runs without inherited exec-policy -/// rules. +/// stable prompt-cache key. That cached trunk also carries the parent-history +/// checkpoint used to slice future guardian transcript evidence. If the trunk +/// is already busy, the review runs in an ephemeral fork from the last +/// committed trunk rollout so parallel approvals do not block each other or +/// mutate the cached thread. The trunk is recreated when the effective +/// review-session config changes, and any future compaction must continue to +/// preserve the guardian policy as exact top-level developer context. It may +/// still reuse the parent's managed-network allowlist for read-only checks, but +/// it intentionally runs without inherited exec-policy rules. pub(super) async fn run_guardian_review_session( session: Arc, turn: Arc, diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 50bf2ed843..679a6cc94f 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -89,6 +89,10 @@ struct GuardianReviewSession { has_prior_review: AtomicBool, review_lock: Mutex<()>, last_committed_rollout_items: Mutex>>, + // Parent-session history index captured after the latest terminal guardian + // review. Future guardian prompts use it to slice parent transcript + // evidence without persisting extra rollout metadata. + parent_history_boundary: Mutex>, } struct EphemeralReviewCleanup { @@ -155,6 +159,14 @@ impl GuardianReviewSessionReuseKey { } impl GuardianReviewSession { + async fn parent_history_boundary(&self) -> Option { + *self.parent_history_boundary.lock().await + } + + async fn set_parent_history_boundary(&self, boundary: Option) { + *self.parent_history_boundary.lock().await = boundary; + } + async fn shutdown(&self) { self.cancel_token.cancel(); let _ = self.codex.shutdown_and_wait().await; @@ -228,6 +240,21 @@ impl Drop for EphemeralReviewCleanup { } impl GuardianReviewSessionManager { + pub(crate) async fn parent_history_boundary(&self) -> Option { + let trunk = self.state.lock().await.trunk.clone(); + match trunk { + Some(trunk) => trunk.parent_history_boundary().await, + None => None, + } + } + + pub(crate) async fn set_parent_history_boundary(&self, boundary: Option) { + let trunk = self.state.lock().await.trunk.clone(); + if let Some(trunk) = trunk { + trunk.set_parent_history_boundary(boundary).await; + } + } + pub(crate) async fn shutdown(&self) { let (review_session, ephemeral_reviews) = { let mut state = self.state.lock().await; @@ -251,6 +278,7 @@ impl GuardianReviewSessionManager { let deadline = tokio::time::Instant::now() + GUARDIAN_REVIEW_TIMEOUT; let next_reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(¶ms.spawn_config); let mut stale_trunk_to_shutdown = None; + let mut spawned_replacement_trunk = false; let trunk_candidate = match run_before_review_deadline( deadline, params.external_cancel.as_ref(), @@ -288,6 +316,7 @@ impl GuardianReviewSessionManager { } Err(outcome) => return outcome, }; + spawned_replacement_trunk = true; state.trunk = Some(Arc::clone(&review_session)); } @@ -296,6 +325,14 @@ impl GuardianReviewSessionManager { Err(outcome) => return outcome, }; + if spawned_replacement_trunk + && let (Some(stale_trunk), Some(trunk)) = + (stale_trunk_to_shutdown.as_ref(), trunk_candidate.as_ref()) + { + let boundary = stale_trunk.parent_history_boundary().await; + trunk.set_parent_history_boundary(boundary).await; + } + if let Some(review_session) = stale_trunk_to_shutdown { review_session.shutdown_in_background(); } @@ -356,6 +393,7 @@ impl GuardianReviewSessionManager { has_prior_review: AtomicBool::new(false), review_lock: Mutex::new(()), last_committed_rollout_items: Mutex::new(None), + parent_history_boundary: Mutex::new(None), })); } @@ -375,6 +413,7 @@ impl GuardianReviewSessionManager { has_prior_review: AtomicBool::new(false), review_lock: Mutex::new(()), last_committed_rollout_items: Mutex::new(None), + parent_history_boundary: Mutex::new(None), })); } @@ -483,6 +522,7 @@ async fn spawn_guardian_review_session( has_prior_review: AtomicBool::new(has_prior_review), review_lock: Mutex::new(()), last_committed_rollout_items: Mutex::new(None), + parent_history_boundary: Mutex::new(None), }) } diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 89c22528e8..3fcf2ca821 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -119,6 +119,16 @@ async fn seed_guardian_parent_history(session: &Arc, turn: &Arc String { + items + .iter() + .map(|item| match item { + codex_protocol::user_input::UserInput::Text { text, .. } => text.as_str(), + other => panic!("expected text-only guardian prompt item, got {other:?}"), + }) + .collect::() +} + fn guardian_snapshot_options() -> ContextSnapshotOptions { ContextSnapshotOptions::default() .strip_capability_instructions() @@ -431,6 +441,178 @@ async fn routes_approval_to_guardian_requires_auto_only_review_policy() { assert!(routes_approval_to_guardian(&turn)); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn approved_guardian_review_moves_parent_transcript_boundary() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-guardian-approved"), + ev_assistant_message( + "msg-guardian-approved", + "{\"risk_level\":\"low\",\"risk_score\":5,\"rationale\":\"approved\",\"evidence\":[]}", + ), + ev_completed("resp-guardian-approved"), + ]), + ) + .await; + + let (session, turn) = guardian_test_session_and_turn(&server).await; + seed_guardian_parent_history(&session, &turn).await; + + let decision = review_approval_request( + &session, + &turn, + GuardianApprovalRequest::Shell { + id: "shell-approved-1".to_string(), + command: vec!["git".to_string(), "push".to_string()], + cwd: PathBuf::from("/repo/codex-rs/core"), + sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Need to push the first docs fix.".to_string()), + }, + None, + ) + .await; + assert_eq!(decision, ReviewDecision::Approved); + + session + .record_into_history( + &[ + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "Push the release branch too.".to_string(), + }], + end_turn: None, + phase: None, + }, + ResponseItem::Message { + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: "I now need approval to push the release branch.".to_string(), + }], + end_turn: None, + phase: None, + }, + ], + turn.as_ref(), + ) + .await; + + let prompt = build_guardian_prompt_items( + session.as_ref(), + None, + GuardianApprovalRequest::Shell { + id: "shell-approved-2".to_string(), + command: vec!["git".to_string(), "push".to_string(), "origin".to_string()], + cwd: PathBuf::from("/repo/codex-rs/core"), + sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Need to push the release branch.".to_string()), + }, + ) + .await?; + let prompt_text = guardian_prompt_text(&prompt); + + assert!(!prompt_text.contains("Please check the repo visibility")); + assert!(!prompt_text.contains("The repo is public; I now need approval")); + assert!(prompt_text.contains("Push the release branch too.")); + assert!(prompt_text.contains("I now need approval to push the release branch.")); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn denied_guardian_review_moves_parent_transcript_boundary() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-guardian-denied"), + ev_assistant_message( + "msg-guardian-denied", + "{\"risk_level\":\"high\",\"risk_score\":95,\"rationale\":\"denied\",\"evidence\":[]}", + ), + ev_completed("resp-guardian-denied"), + ]), + ) + .await; + + let (session, turn) = guardian_test_session_and_turn(&server).await; + seed_guardian_parent_history(&session, &turn).await; + + let decision = review_approval_request( + &session, + &turn, + GuardianApprovalRequest::Shell { + id: "shell-denied-1".to_string(), + command: vec!["git".to_string(), "push".to_string(), "--force".to_string()], + cwd: PathBuf::from("/repo/codex-rs/core"), + sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Need to force push the first docs fix.".to_string()), + }, + None, + ) + .await; + assert_eq!(decision, ReviewDecision::Denied); + + session + .record_into_history( + &[ + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "Push the hotfix branch instead.".to_string(), + }], + end_turn: None, + phase: None, + }, + ResponseItem::Message { + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: "I need approval to push the hotfix branch.".to_string(), + }], + end_turn: None, + phase: None, + }, + ], + turn.as_ref(), + ) + .await; + + let prompt = build_guardian_prompt_items( + session.as_ref(), + None, + GuardianApprovalRequest::Shell { + id: "shell-denied-2".to_string(), + command: vec!["git".to_string(), "push".to_string(), "hotfix".to_string()], + cwd: PathBuf::from("/repo/codex-rs/core"), + sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Need to push the hotfix branch.".to_string()), + }, + ) + .await?; + let prompt_text = guardian_prompt_text(&prompt); + + assert!(!prompt_text.contains("Please check the repo visibility")); + assert!(!prompt_text.contains("The repo is public; I now need approval")); + assert!(prompt_text.contains("Push the hotfix branch instead.")); + assert!(prompt_text.contains("I need approval to push the hotfix branch.")); + + Ok(()) +} + #[test] fn build_guardian_transcript_reserves_separate_budget_for_tool_evidence() { let repeated = "signal ".repeat(8_000);