mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
improved behavior for dont ask again for this prefix
This commit is contained in:
@@ -160,14 +160,21 @@ fn requirement_from_decision(
|
||||
/// unrolled command (multi-part scripts shouldn’t be whitelisted via prefix) and
|
||||
/// when execpolicy feature is enabled.
|
||||
fn allow_prefix_if_applicable(
|
||||
policy: &Policy,
|
||||
commands: &[Vec<String>],
|
||||
features: &Features,
|
||||
) -> Option<Vec<String>> {
|
||||
if features.enabled(Feature::ExecPolicy) && commands.len() == 1 {
|
||||
Some(commands[0].clone())
|
||||
} else {
|
||||
None
|
||||
if !features.enabled(Feature::ExecPolicy) {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Only offer a prefix when the prompt is driven by heuristics (policy has no matches).
|
||||
// For multi-command scripts, choose the first command segment that also has no policy
|
||||
// match (i.e. the first segment that could plausibly be the prompt trigger).
|
||||
commands
|
||||
.iter()
|
||||
.find(|cmd| matches!(policy.check(cmd), Evaluation::NoMatch))
|
||||
.cloned()
|
||||
}
|
||||
|
||||
pub(crate) fn create_approval_requirement_for_command(
|
||||
@@ -192,7 +199,7 @@ pub(crate) fn create_approval_requirement_for_command(
|
||||
) {
|
||||
ApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: allow_prefix_if_applicable(&commands, features),
|
||||
allow_prefix: allow_prefix_if_applicable(policy, &commands, features),
|
||||
}
|
||||
} else {
|
||||
ApprovalRequirement::Skip {
|
||||
@@ -588,8 +595,35 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
requirement,
|
||||
ApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: None,
|
||||
allow_prefix: Some(vec!["python".to_string()]),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allow_prefix_uses_first_no_match_in_multi_command_scripts() {
|
||||
let policy_src = r#"prefix_rule(pattern=["python"], decision="allow")"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.codexpolicy", policy_src)
|
||||
.expect("parse policy");
|
||||
let policy = parser.build();
|
||||
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python && echo ok".to_string(),
|
||||
];
|
||||
|
||||
let requirement = create_approval_requirement_for_command(
|
||||
&policy,
|
||||
&Features::with_defaults(),
|
||||
&command,
|
||||
AskForApproval::UnlessTrusted,
|
||||
&SandboxPolicy::ReadOnly,
|
||||
SandboxPermissions::UseDefault,
|
||||
);
|
||||
|
||||
assert_eq!(requirement, ApprovalRequirement::Skip);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -471,29 +471,30 @@ impl ApprovalOption {
|
||||
}
|
||||
|
||||
fn exec_options(allow_prefix: Option<Vec<String>>) -> Vec<ApprovalOption> {
|
||||
vec![
|
||||
ApprovalOption {
|
||||
label: "Yes, proceed".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::Approved),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Yes, and don't ask again this session".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
|
||||
},
|
||||
]
|
||||
.into_iter()
|
||||
.chain(allow_prefix.map(|prefix| ApprovalOption {
|
||||
label: "Yes, and don't ask again for commands with this prefix".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::ApprovedAllowPrefix {
|
||||
allow_prefix: prefix,
|
||||
}),
|
||||
let allow_prefix_label = allow_prefix.as_ref().map(|prefix| {
|
||||
let rendered_prefix = strip_bash_lc_and_escape(prefix);
|
||||
format!("Yes, and don't ask again for `{rendered_prefix}` commands")
|
||||
});
|
||||
|
||||
vec![ApprovalOption {
|
||||
label: "Yes, proceed".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::Approved),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
|
||||
}))
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
|
||||
}]
|
||||
.into_iter()
|
||||
.chain(
|
||||
allow_prefix
|
||||
.zip(allow_prefix_label)
|
||||
.map(|(prefix, label)| ApprovalOption {
|
||||
label,
|
||||
decision: ApprovalDecision::Review(ReviewDecision::ApprovedAllowPrefix {
|
||||
allow_prefix: prefix,
|
||||
}),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
|
||||
}),
|
||||
)
|
||||
.chain([ApprovalOption {
|
||||
label: "No, and tell Codex what to do differently".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::Abort),
|
||||
@@ -687,7 +688,6 @@ mod tests {
|
||||
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx_raw);
|
||||
let mut view = ApprovalOverlay::new(make_exec_request(), tx);
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE));
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
|
||||
assert!(
|
||||
@@ -702,6 +702,6 @@ mod tests {
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert_eq!(decision, Some(ReviewDecision::ApprovedForSession));
|
||||
assert_eq!(decision, Some(ReviewDecision::Approved));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,14 +4,12 @@ expression: terminal.backend().vt100().screen().contents()
|
||||
---
|
||||
Would you like to run the following command?
|
||||
|
||||
Reason: this is a test reason such as one that would be produced by the
|
||||
model
|
||||
Reason: this is a test reason such as one that would be produced by the model
|
||||
|
||||
$ echo hello world
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. Yes, and don't ask again this session (a)
|
||||
3. Yes, and don't ask again for commands with this prefix (p)
|
||||
4. No, and tell Codex what to do differently (esc)
|
||||
2. Yes, and don't ask again for `echo hello world` commands (p)
|
||||
3. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
||||
@@ -7,8 +7,7 @@ expression: terminal.backend().vt100().screen().contents()
|
||||
$ echo hello world
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. Yes, and don't ask again this session (a)
|
||||
3. Yes, and don't ask again for commands with this prefix (p)
|
||||
4. No, and tell Codex what to do differently (esc)
|
||||
2. Yes, and don't ask again for `echo hello world` commands (p)
|
||||
3. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
||||
@@ -3,7 +3,7 @@ source: tui/src/chatwidget/tests.rs
|
||||
expression: "format!(\"{buf:?}\")"
|
||||
---
|
||||
Buffer {
|
||||
area: Rect { x: 0, y: 0, width: 80, height: 14 },
|
||||
area: Rect { x: 0, y: 0, width: 80, height: 13 },
|
||||
content: [
|
||||
" ",
|
||||
" ",
|
||||
@@ -15,8 +15,7 @@ Buffer {
|
||||
" $ echo hello world ",
|
||||
" ",
|
||||
"› 1. Yes, proceed (y) ",
|
||||
" 2. Yes, and don't ask again this session (a) ",
|
||||
" 3. No, and tell Codex what to do differently (esc) ",
|
||||
" 2. No, and tell Codex what to do differently (esc) ",
|
||||
" ",
|
||||
" Press enter to confirm or esc to cancel ",
|
||||
],
|
||||
@@ -30,10 +29,8 @@ Buffer {
|
||||
x: 7, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 9, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 21, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 44, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 45, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 48, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 51, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 2, y: 13, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 48, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 51, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 2, y: 12, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
]
|
||||
}
|
||||
|
||||
@@ -2,18 +2,16 @@
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: terminal.backend()
|
||||
---
|
||||
" "
|
||||
" "
|
||||
" Would you like to run the following command? "
|
||||
" "
|
||||
" Reason: this is a test reason such as one that would be produced by the "
|
||||
" model "
|
||||
" "
|
||||
" $ echo 'hello world' "
|
||||
" "
|
||||
"› 1. Yes, proceed (y) "
|
||||
" 2. Yes, and don't ask again this session (a) "
|
||||
" 3. Yes, and don't ask again for commands with this prefix (p) "
|
||||
" 4. No, and tell Codex what to do differently (esc) "
|
||||
" "
|
||||
" Press enter to confirm or esc to cancel "
|
||||
" "
|
||||
" "
|
||||
" Would you like to run the following command? "
|
||||
" "
|
||||
" Reason: this is a test reason such as one that would be produced by the model "
|
||||
" "
|
||||
" $ echo 'hello world' "
|
||||
" "
|
||||
"› 1. Yes, proceed (y) "
|
||||
" 2. Yes, and don't ask again for `echo 'hello world'` commands (p) "
|
||||
" 3. No, and tell Codex what to do differently (esc) "
|
||||
" "
|
||||
" Press enter to confirm or esc to cancel "
|
||||
|
||||
@@ -1981,11 +1981,12 @@ fn approval_modal_exec_snapshot() {
|
||||
});
|
||||
// Render to a fixed-size test terminal and snapshot.
|
||||
// Call desired_height first and use that exact height for rendering.
|
||||
let height = chat.desired_height(80);
|
||||
let width = 100;
|
||||
let height = chat.desired_height(width);
|
||||
let mut terminal =
|
||||
crate::custom_terminal::Terminal::with_options(VT100Backend::new(80, height))
|
||||
crate::custom_terminal::Terminal::with_options(VT100Backend::new(width, height))
|
||||
.expect("create terminal");
|
||||
let viewport = Rect::new(0, 0, 80, height);
|
||||
let viewport = Rect::new(0, 0, width, height);
|
||||
terminal.set_viewport_area(viewport);
|
||||
|
||||
terminal
|
||||
@@ -2027,10 +2028,11 @@ fn approval_modal_exec_without_reason_snapshot() {
|
||||
msg: EventMsg::ExecApprovalRequest(ev),
|
||||
});
|
||||
|
||||
let height = chat.desired_height(80);
|
||||
let width = 100;
|
||||
let height = chat.desired_height(width);
|
||||
let mut terminal =
|
||||
ratatui::Terminal::new(VT100Backend::new(80, height)).expect("create terminal");
|
||||
terminal.set_viewport_area(Rect::new(0, 0, 80, height));
|
||||
ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal");
|
||||
terminal.set_viewport_area(Rect::new(0, 0, width, height));
|
||||
terminal
|
||||
.draw(|f| chat.render(f.area(), f.buffer_mut()))
|
||||
.expect("draw approval modal (no reason)");
|
||||
@@ -2242,9 +2244,11 @@ fn status_widget_and_approval_modal_snapshot() {
|
||||
});
|
||||
|
||||
// Render at the widget's desired height and snapshot.
|
||||
let height = chat.desired_height(80);
|
||||
let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height))
|
||||
let width: u16 = 100;
|
||||
let height = chat.desired_height(width);
|
||||
let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(width, height))
|
||||
.expect("create terminal");
|
||||
terminal.set_viewport_area(Rect::new(0, 0, width, height));
|
||||
terminal
|
||||
.draw(|f| chat.render(f.area(), f.buffer_mut()))
|
||||
.expect("draw status + approval modal");
|
||||
|
||||
Reference in New Issue
Block a user