mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
fix: clarify Guardian retry messages
Co-authored-by: Codex noreply@openai.com
This commit is contained in:
co-authored by
Codex noreply@openai.com
parent
e014da9666
commit
518962f57d
@@ -59,8 +59,8 @@ const GUARDIAN_TIMEOUT_INSTRUCTIONS: &str = concat!(
|
||||
);
|
||||
|
||||
const GUARDIAN_RETRY_WARNING: &str = concat!(
|
||||
"Automatic approval review hit a retryable reviewer availability failure. ",
|
||||
"Retrying once before failing closed."
|
||||
"Automatic approval review is temporarily unavailable. ",
|
||||
"Retrying once before blocking the action.",
|
||||
);
|
||||
|
||||
pub(crate) fn new_guardian_review_id() -> String {
|
||||
@@ -86,7 +86,7 @@ pub(crate) async fn guardian_rejection_message(session: &Session, review_id: &st
|
||||
|| rationale.starts_with("Automatic approval review could not")
|
||||
{
|
||||
format!(
|
||||
"This action was blocked because automatic approval review could not complete safely.\nReason: {rationale}\n{GUARDIAN_REJECTION_INSTRUCTIONS}",
|
||||
"Automatic approval review could not complete, so this action was blocked.\nReason: {rationale}\n{GUARDIAN_REJECTION_INSTRUCTIONS}",
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
@@ -155,12 +155,14 @@ impl GuardianReviewError {
|
||||
|
||||
fn retry_reason(&self) -> String {
|
||||
match self {
|
||||
Self::Session { message } => format!(
|
||||
"Previous automatic approval review failed due to reviewer capacity, transport, or session error: {message}. Retry once before treating this as a failed-closed review."
|
||||
),
|
||||
Self::Session { .. } => concat!(
|
||||
"The previous automatic approval review was temporarily unavailable. ",
|
||||
"Retry the review once before blocking the action."
|
||||
)
|
||||
.to_string(),
|
||||
Self::Timeout => concat!(
|
||||
"Previous automatic approval review timed out. ",
|
||||
"Retry once before treating this as a failed-closed review."
|
||||
"The previous automatic approval review timed out. ",
|
||||
"Retry the review once before blocking the action."
|
||||
)
|
||||
.to_string(),
|
||||
Self::PromptBuild { .. } | Self::Parse { .. } | Self::Cancelled => {
|
||||
@@ -532,12 +534,12 @@ async fn run_guardian_review(
|
||||
}
|
||||
GuardianReviewError::Session { message } => {
|
||||
format!(
|
||||
"Automatic approval review failed because the reviewer session returned an error: {message}"
|
||||
"Automatic approval review failed because the reviewer returned an error: {message}"
|
||||
)
|
||||
}
|
||||
GuardianReviewError::Parse { message } => {
|
||||
format!(
|
||||
"Automatic approval review returned an unreadable assessment: {message}"
|
||||
"Automatic approval review failed because the reviewer returned an unreadable response: {message}"
|
||||
)
|
||||
}
|
||||
GuardianReviewError::Timeout | GuardianReviewError::Cancelled => {
|
||||
@@ -679,11 +681,15 @@ async fn run_guardian_review_session_with_retry(
|
||||
)
|
||||
.await;
|
||||
|
||||
let retry_reason = match retry_reason {
|
||||
Some(retry_reason) => format!("{retry_reason}\n\n{}", error.retry_reason()),
|
||||
None => error.retry_reason(),
|
||||
};
|
||||
run_guardian_review_session(
|
||||
session,
|
||||
turn,
|
||||
request,
|
||||
Some(error.retry_reason()),
|
||||
Some(retry_reason),
|
||||
schema,
|
||||
external_cancel,
|
||||
)
|
||||
|
||||
@@ -2074,7 +2074,7 @@ async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() ->
|
||||
guardian_rejection_message(session.as_ref(), "review-shell-guardian-error").await;
|
||||
assert!(
|
||||
rejection_message
|
||||
.contains("Reason: Automatic approval review failed because the reviewer session")
|
||||
.contains("Reason: Automatic approval review failed because the reviewer returned")
|
||||
&& rejection_message.contains(error_message),
|
||||
"rejection message should include guardian rationale: {rejection_message}"
|
||||
);
|
||||
@@ -2086,6 +2086,8 @@ async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() ->
|
||||
async fn guardian_review_retries_reviewer_availability_failure() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
const PRIOR_RETRY_REASON: &str = "The command is being retried without sandboxing.";
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let guardian_assessment = serde_json::json!({
|
||||
"risk_level": "low",
|
||||
@@ -2128,12 +2130,19 @@ async fn guardian_review_retries_reviewer_availability_failure() -> anyhow::Resu
|
||||
additional_permissions: None,
|
||||
justification: Some("Inspect repo state before proceeding.".to_string()),
|
||||
},
|
||||
/*retry_reason*/ None,
|
||||
/*retry_reason*/ Some(PRIOR_RETRY_REASON.to_string()),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(decision, ReviewDecision::Approved);
|
||||
assert_eq!(request_log.requests().len(), 2);
|
||||
let requests = request_log.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let retry_request = requests[1].body_json().to_string();
|
||||
assert!(retry_request.contains(PRIOR_RETRY_REASON));
|
||||
assert!(retry_request.contains(
|
||||
"The previous automatic approval review was temporarily unavailable. Retry the review once before blocking the action."
|
||||
));
|
||||
assert!(!retry_request.contains("reviewer capacity temporarily unavailable"));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user