mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
handle pending tool calls in cursor
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
],
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user