diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index e1bfe11e9a..633ef1ad1f 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -140,6 +140,8 @@ use prompt::build_guardian_approval_request_items; #[cfg(test)] use prompt::build_guardian_prompt_items; #[cfg(test)] +use prompt::build_guardian_transcript_sync_items; +#[cfg(test)] use prompt::collect_guardian_transcript_entries; #[cfg(test)] use prompt::guardian_output_schema; diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index b5154de2fa..814dd5958c 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -64,6 +64,7 @@ pub(crate) struct GuardianPromptItems { pub(crate) transcript_cursor: GuardianTranscriptCursor, pub(crate) reviewed_action_truncated: bool, pub(crate) has_transcript_update: bool, + pub(crate) has_pending_tool_call: bool, } pub(crate) struct GuardianApprovalPromptItems { @@ -117,6 +118,7 @@ async fn build_guardian_transcript_items( delta_headings: GuardianPromptHeadings, ) -> GuardianPromptItems { let history = session.clone_history().await; + let has_pending_tool_call = has_pending_guardian_tool_call(history.raw_items()); let transcript_entries = collect_guardian_transcript_entries(history.raw_items()); let transcript_cursor = GuardianTranscriptCursor { parent_history_version: history.history_version(), @@ -185,6 +187,7 @@ async fn build_guardian_transcript_items( transcript_cursor, reviewed_action_truncated: false, has_transcript_update, + has_pending_tool_call, } } @@ -572,6 +575,27 @@ pub(crate) fn collect_guardian_transcript_entries( entries } +fn has_pending_guardian_tool_call(items: &[ResponseItem]) -> bool { + let mut call_ids = std::collections::HashSet::new(); + let mut completed_call_ids = std::collections::HashSet::new(); + for item in items { + match item { + ResponseItem::FunctionCall { call_id, .. } + | ResponseItem::CustomToolCall { call_id, .. } => { + call_ids.insert(call_id); + } + ResponseItem::FunctionCallOutput { call_id, .. } + | ResponseItem::CustomToolCallOutput { call_id, .. } => { + completed_call_ids.insert(call_id); + } + _ => {} + } + } + call_ids + .iter() + .any(|call_id| !completed_call_ids.contains(call_id)) +} + pub(crate) fn guardian_truncate_text(content: &str, token_cap: usize) -> (String, bool) { if content.is_empty() { return (String::new(), false); diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 4362785d3b..916e57a287 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -788,12 +788,14 @@ async fn run_review_on_session( params.request.clone(), ) .await?; + let initial_transcript_cursor = + (!prompt_items.has_pending_tool_call).then_some(prompt_items.transcript_cursor); ( GuardianApprovalPromptItems { items: prompt_items.items, reviewed_action_truncated: prompt_items.reviewed_action_truncated, }, - Some(prompt_items.transcript_cursor), + initial_transcript_cursor, ) } else { sync_parent_transcript_to_session(review_session, params.parent_session.as_ref()) @@ -819,7 +821,7 @@ async fn run_review_on_session( Ok(prompt_items) => prompt_items, Err(err) => { return ( - GuardianReviewSessionOutcome::PromptBuildFailed(err.into()), + GuardianReviewSessionOutcome::PromptBuildFailed(err), false, analytics_result, ); @@ -911,6 +913,9 @@ async fn sync_parent_transcript_to_session( super::prompt::GuardianPromptMode::Delta { cursor } }); let prompt_items = build_guardian_transcript_sync_items(parent_session, prompt_mode).await; + if prompt_items.has_pending_tool_call { + return Ok(false); + } if Some(prompt_items.transcript_cursor) == last_synced_transcript_cursor { return Ok(false); } @@ -954,7 +959,6 @@ fn response_item_from_text_inputs( id: None, role: "user".to_string(), content, - end_turn: None, phase: None, } } diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index dabe3fdc09..b0431a9482 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -723,6 +723,45 @@ fn collect_guardian_transcript_entries_excludes_dangling_tool_calls() { ); } +#[tokio::test(flavor = "current_thread")] +async fn build_guardian_transcript_items_flags_pending_tool_calls() -> anyhow::Result<()> { + let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; + session + .record_into_history( + &[ + ResponseItem::FunctionCall { + id: None, + name: "read_file".to_string(), + namespace: None, + arguments: "{\"path\":\"README.md\"}".to_string(), + call_id: "pending-call".to_string(), + }, + ResponseItem::FunctionCall { + id: None, + name: "read_file".to_string(), + namespace: None, + arguments: "{\"path\":\"Cargo.toml\"}".to_string(), + call_id: "completed-call".to_string(), + }, + ResponseItem::FunctionCallOutput { + call_id: "completed-call".to_string(), + output: codex_protocol::models::FunctionCallOutputPayload::from_text( + "workspace manifest".to_string(), + ), + }, + ], + turn.as_ref(), + ) + .await; + + let prompt = build_guardian_transcript_sync_items(&session, GuardianPromptMode::Full).await; + + assert!(prompt.has_pending_tool_call); + assert_eq!(prompt.transcript_cursor.transcript_entry_count, 2); + + Ok(()) +} + #[tokio::test] async fn build_guardian_approval_request_items_are_skinny() -> anyhow::Result<()> { let (session, _) = crate::session::tests::make_session_and_context().await; @@ -1806,7 +1845,7 @@ async fn proactive_guardian_sync_compacts_trunk_before_review_request() -> anyho let (session, turn) = guardian_test_session_and_turn_with_config(server.uri().as_str(), |config| { - config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent; + config.approvals_reviewer = ApprovalsReviewer::AutoReview; config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); config.model_provider.name = "OpenAI-compatible guardian compact test".to_string(); config.compact_prompt = Some(SUMMARIZATION_PROMPT.to_string()); @@ -1831,7 +1870,7 @@ async fn proactive_guardian_sync_compacts_trunk_before_review_request() -> anyho /*external_cancel*/ None, ) .await; - let GuardianReviewOutcome::Completed(Ok(first_assessment)) = first_outcome else { + let (GuardianReviewOutcome::Completed(first_assessment), _) = first_outcome else { panic!("expected first guardian assessment"); }; assert_eq!(first_assessment.outcome, GuardianAssessmentOutcome::Allow); @@ -1860,7 +1899,6 @@ async fn proactive_guardian_sync_compacts_trunk_before_review_request() -> anyho content: vec![ContentItem::InputText { text: "Please push the docs fix now.".to_string(), }], - end_turn: None, phase: None, }, ResponseItem::Message { @@ -1869,7 +1907,6 @@ async fn proactive_guardian_sync_compacts_trunk_before_review_request() -> anyho content: vec![ContentItem::OutputText { text: "I need approval to push the docs fix.".to_string(), }], - end_turn: None, phase: None, }, ], @@ -1911,7 +1948,7 @@ async fn proactive_guardian_sync_compacts_trunk_before_review_request() -> anyho /*external_cancel*/ None, ) .await; - let GuardianReviewOutcome::Completed(Ok(second_assessment)) = second_outcome else { + let (GuardianReviewOutcome::Completed(second_assessment), _) = second_outcome else { panic!("expected second guardian assessment"); }; assert_eq!(second_assessment.outcome, GuardianAssessmentOutcome::Allow); @@ -2150,7 +2187,6 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a text: "Please inspect pending changes before pushing." .to_string(), }], - end_turn: None, phase: None, }, ResponseItem::Message { @@ -2159,7 +2195,6 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a content: vec![ContentItem::OutputText { text: "I need approval to run git diff.".to_string(), }], - end_turn: None, phase: None, }, ], @@ -2221,7 +2256,6 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a content: vec![ContentItem::InputText { text: "Now inspect whether pushing is safe.".to_string(), }], - end_turn: None, phase: None, }, ResponseItem::Message { @@ -2231,7 +2265,6 @@ async fn guardian_parallel_reviews_fork_from_last_committed_trunk_history() -> a text: "I need approval to push after the diff check." .to_string(), }], - end_turn: None, phase: None, }, ], diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 05e34efc8c..c744ed934f 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -4,6 +4,7 @@ // user-visible output must go through the appropriate abstraction (e.g., // the TUI or the tracing stack). #![deny(clippy::print_stdout, clippy::print_stderr)] +#![recursion_limit = "256"] mod apply_patch; mod apps;