mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Revert "Make auto-review on-request prompt more proactive" (#30508)
Reverts openai/codex#26496
This commit is contained in:
committed by
GitHub
Unverified
parent
8dac605901
commit
ccdfb4f342
@@ -20,8 +20,6 @@ const APPROVAL_POLICY_UNLESS_TRUSTED: &str =
|
||||
include_str!("../templates/permissions/approval_policy/unless_trusted.md");
|
||||
const APPROVAL_POLICY_ON_REQUEST_RULE: &str =
|
||||
include_str!("../templates/permissions/approval_policy/on_request.md");
|
||||
const APPROVAL_POLICY_ON_REQUEST_AUTO_REVIEW: &str =
|
||||
include_str!("../templates/permissions/approval_policy/on_request_auto_review.md");
|
||||
const APPROVAL_POLICY_ON_REQUEST_RULE_REQUEST_PERMISSION: &str =
|
||||
include_str!("../templates/permissions/approval_policy/on_request_rule_request_permission.md");
|
||||
const AUTO_REVIEW_APPROVAL_SUFFIX: &str = "`approvals_reviewer` is `auto_review`: Sandbox escalations with require_escalated will be reviewed for compliance with the policy. If a rejection happens, you should proceed only with a materially safer alternative, or inform the user of the risk and send a final message to ask for approval.";
|
||||
@@ -208,8 +206,6 @@ fn approval_text(
|
||||
let on_request_instructions = || {
|
||||
let on_request_rule = if exec_permission_approvals_enabled {
|
||||
APPROVAL_POLICY_ON_REQUEST_RULE_REQUEST_PERMISSION.to_string()
|
||||
} else if approvals_reviewer == ApprovalsReviewer::AutoReview {
|
||||
APPROVAL_POLICY_ON_REQUEST_AUTO_REVIEW.to_string()
|
||||
} else {
|
||||
APPROVAL_POLICY_ON_REQUEST_RULE.to_string()
|
||||
};
|
||||
|
||||
@@ -237,98 +237,17 @@ fn on_request_includes_tool_guidance_alongside_inline_permission_guidance_when_b
|
||||
|
||||
#[test]
|
||||
fn auto_review_approvals_append_auto_review_specific_guidance() {
|
||||
let mut exec_policy = Policy::empty();
|
||||
exec_policy
|
||||
.add_prefix_rule(&["git".to_string(), "pull".to_string()], Decision::Allow)
|
||||
.expect("add rule");
|
||||
let text = approval_text(
|
||||
AskForApproval::OnRequest,
|
||||
ApprovalsReviewer::AutoReview,
|
||||
&exec_policy,
|
||||
&Policy::empty(),
|
||||
/*exec_permission_approvals_enabled*/ false,
|
||||
/*request_permissions_tool_enabled*/ false,
|
||||
);
|
||||
|
||||
assert!(text.contains("`approvals_reviewer` is `auto_review`"));
|
||||
assert!(text.contains("materially safer alternative"));
|
||||
assert!(!text.contains("`approvals_reviewer` is `guardian_subagent`"));
|
||||
assert!(text.contains(
|
||||
"When the sandbox is likely to block a command needed for the task, request escalation up front"
|
||||
));
|
||||
assert!(text.contains(
|
||||
"When unsure, prefer requesting escalation unnecessarily over failing to request it when needed"
|
||||
));
|
||||
assert!(
|
||||
text.contains(
|
||||
"Request escalation for commands that need write access outside writable roots"
|
||||
)
|
||||
);
|
||||
assert!(text.contains(
|
||||
"Request escalation for git operations that may write lock files, such as updating the index or refs"
|
||||
));
|
||||
assert!(!text.contains("Request escalation for GUI commands"));
|
||||
assert!(!text.contains("Use escalation when it is the direct or most reliable way"));
|
||||
assert!(!text.contains("Do not spend extra turns running likely-to-fail sandbox probes first"));
|
||||
assert!(text.contains("Do not include a `prefix_rule` parameter."));
|
||||
assert!(text.contains(
|
||||
"Request escalation for commands that may need network access, including HTTP calls, package registries, internal services, data-service APIs, remote queries, data fetches, or live probes"
|
||||
));
|
||||
assert!(text.contains(
|
||||
"Request escalation for commands that may need remote authentication, cluster, cloud, or database access"
|
||||
));
|
||||
assert!(text.contains(
|
||||
"Request escalation for commands that may need process, cache, or other environment access outside the sandbox"
|
||||
));
|
||||
assert!(
|
||||
text.contains(
|
||||
"including DNS, connection, authentication, retry, or service endpoint errors"
|
||||
)
|
||||
);
|
||||
assert!(text.contains(
|
||||
"Request escalation before potentially destructive actions, such as `rm` or `git reset`"
|
||||
));
|
||||
assert!(text.contains(
|
||||
"If a command may be hanging on sandbox-blocked access, stop after a short timeout and rerun with `require_escalated`"
|
||||
));
|
||||
assert!(!text.contains("Be judicious with escalating"));
|
||||
assert!(text.contains("Approved command prefixes"));
|
||||
assert!(text.contains(r#"["git", "pull"]"#));
|
||||
assert!(!text.contains("prefix_rule guidance"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normal_on_request_keeps_user_approval_wording() {
|
||||
let text = approval_text(
|
||||
AskForApproval::OnRequest,
|
||||
ApprovalsReviewer::User,
|
||||
&Policy::empty(),
|
||||
/*exec_permission_approvals_enabled*/ false,
|
||||
/*request_permissions_tool_enabled*/ false,
|
||||
);
|
||||
|
||||
assert!(text.contains("approved by the user"));
|
||||
assert!(text.contains("Be judicious with escalating"));
|
||||
assert!(!text.contains(
|
||||
"When the sandbox is likely to block a command needed for the task, request escalation up front"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_review_on_request_with_inline_permission_requests_keeps_suffix() {
|
||||
let text = approval_text(
|
||||
AskForApproval::OnRequest,
|
||||
ApprovalsReviewer::AutoReview,
|
||||
&Policy::empty(),
|
||||
/*exec_permission_approvals_enabled*/ true,
|
||||
/*request_permissions_tool_enabled*/ false,
|
||||
);
|
||||
|
||||
assert!(text.contains("with_additional_permissions"));
|
||||
assert!(text.contains("`approvals_reviewer` is `auto_review`"));
|
||||
assert!(text.contains("materially safer alternative"));
|
||||
assert!(!text.contains(
|
||||
"When the sandbox is likely to block a command needed for the task, request escalation up front"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1,45 +0,0 @@
|
||||
# Escalation Requests
|
||||
|
||||
Commands are run outside the sandbox after approval. The command string is split into independent command segments at shell control operators, including but not limited to:
|
||||
|
||||
- Pipes: |
|
||||
- Logical operators: &&, ||
|
||||
- Command separators: ;
|
||||
- Subshell boundaries: (...), $()
|
||||
|
||||
Each resulting segment is evaluated independently for sandbox restrictions and approval requirements.
|
||||
|
||||
Example:
|
||||
|
||||
git pull | tee output.txt
|
||||
|
||||
This is treated as two command segments:
|
||||
|
||||
["git", "pull"]
|
||||
|
||||
["tee", "output.txt"]
|
||||
|
||||
Commands that use more advanced shell features like redirection (>, >>, <), substitutions ($(...), ...), environment variables (FOO=bar), or wildcard patterns (*, ?) require care because each independent command segment is evaluated separately.
|
||||
|
||||
## How to request escalation
|
||||
|
||||
IMPORTANT: To request approval to execute a command that will require escalated privileges:
|
||||
|
||||
- Provide the `sandbox_permissions` parameter with the value `"require_escalated"`
|
||||
- Include a concise `justification` parameter that explains why escalated privileges are needed.
|
||||
- Do not include a `prefix_rule` parameter.
|
||||
|
||||
## When to request escalation
|
||||
|
||||
While commands are running inside the sandbox, here are some scenarios that justify escalation:
|
||||
|
||||
- When the sandbox is likely to block a command needed for the task, request escalation up front.
|
||||
- Request escalation for commands that need write access outside writable roots, such as tests that write to `/var`.
|
||||
- Request escalation for git operations that may write lock files, such as updating the index or refs.
|
||||
- Request escalation for commands that may need network access, including HTTP calls, package registries, internal services, data-service APIs, remote queries, data fetches, or live probes.
|
||||
- Request escalation for commands that may need remote authentication, cluster, cloud, or database access.
|
||||
- Request escalation for commands that may need process, cache, or other environment access outside the sandbox.
|
||||
- If a sandboxed attempt fails with sandboxing or likely network symptoms, including DNS, connection, authentication, retry, or service endpoint errors, rerun with `sandbox_permissions` set to `"require_escalated"` and include `justification`.
|
||||
- If a command may be hanging on sandbox-blocked access, stop after a short timeout and rerun with `require_escalated`.
|
||||
- Request escalation before potentially destructive actions, such as `rm` or `git reset`, that the user did not explicitly ask for.
|
||||
- When unsure, prefer requesting escalation unnecessarily over failing to request it when needed.
|
||||
Reference in New Issue
Block a user