improved behavior for dont ask again for this prefix

This commit is contained in:
kevin zhao
2025-12-01 14:08:34 -05:00
parent b492d9369e
commit 109dc3a178
7 changed files with 99 additions and 69 deletions

View File

@@ -160,14 +160,21 @@ fn requirement_from_decision(
/// unrolled command (multi-part scripts shouldnt 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) async fn create_approval_requirement_for_command(
@@ -193,7 +200,7 @@ pub(crate) async 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 {
@@ -597,8 +604,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);
}
}

View File

@@ -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));
}
}

View File

@@ -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

View File

@@ -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

View File

@@ -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,
]
}

View File

@@ -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 "

View File

@@ -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");