fix: truncate long approval prefixes when rendering (#8734)
Fixes inscrutable multiline approval requests: <img width="686" height="844" alt="image" src="https://github.com/user-attachments/assets/cf9493dc-79e6-4168-8020-0ef0fe676d5e" />
This commit is contained in:
parent
dc1a568dc7
commit
8b4d27dfcd
6 changed files with 127 additions and 6 deletions
|
|
@ -461,9 +461,13 @@ fn exec_options(
|
|||
.chain(
|
||||
proposed_execpolicy_amendment
|
||||
.filter(|_| features.enabled(Feature::ExecPolicy))
|
||||
.map(|prefix| {
|
||||
.and_then(|prefix| {
|
||||
let rendered_prefix = strip_bash_lc_and_escape(prefix.command());
|
||||
ApprovalOption {
|
||||
if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(ApprovalOption {
|
||||
label: format!(
|
||||
"Yes, and don't ask again for commands that start with `{rendered_prefix}`"
|
||||
),
|
||||
|
|
@ -474,7 +478,7 @@ fn exec_options(
|
|||
),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
|
||||
}
|
||||
})
|
||||
}),
|
||||
)
|
||||
.chain([ApprovalOption {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,16 @@
|
|||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: contents
|
||||
---
|
||||
|
||||
|
||||
Would you like to run the following command?
|
||||
|
||||
$ python - <<'PY'
|
||||
print('hello')
|
||||
PY
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
|
@ -2465,6 +2465,48 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
// Snapshot test: approval modal with a proposed execpolicy prefix that is multi-line;
|
||||
// we should not offer adding it to execpolicy.
|
||||
#[tokio::test]
|
||||
async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
|
||||
-> anyhow::Result<()> {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.config.approval_policy.set(AskForApproval::OnRequest)?;
|
||||
|
||||
let script = "python - <<'PY'\nprint('hello')\nPY".to_string();
|
||||
let command = vec!["bash".into(), "-lc".into(), script];
|
||||
let ev = ExecApprovalRequestEvent {
|
||||
call_id: "call-approve-cmd-multiline-trunc".into(),
|
||||
turn_id: "turn-approve-cmd-multiline-trunc".into(),
|
||||
command: command.clone(),
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
id: "sub-approve-multiline-trunc".into(),
|
||||
msg: EventMsg::ExecApprovalRequest(ev),
|
||||
});
|
||||
|
||||
let width = 100;
|
||||
let height = chat.desired_height(width);
|
||||
let mut terminal =
|
||||
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 (multiline prefix)");
|
||||
let contents = terminal.backend().vt100().screen().contents();
|
||||
assert!(!contents.contains("don't ask again"));
|
||||
assert_snapshot!(
|
||||
"approval_modal_exec_multiline_prefix_no_execpolicy",
|
||||
contents
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// Snapshot test: patch approval modal
|
||||
#[tokio::test]
|
||||
async fn approval_modal_patch_snapshot() -> anyhow::Result<()> {
|
||||
|
|
|
|||
|
|
@ -461,9 +461,13 @@ fn exec_options(
|
|||
.chain(
|
||||
proposed_execpolicy_amendment
|
||||
.filter(|_| features.enabled(Feature::ExecPolicy))
|
||||
.map(|prefix| {
|
||||
.and_then(|prefix| {
|
||||
let rendered_prefix = strip_bash_lc_and_escape(prefix.command());
|
||||
ApprovalOption {
|
||||
if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(ApprovalOption {
|
||||
label: format!(
|
||||
"Yes, and don't ask again for commands that start with `{rendered_prefix}`"
|
||||
),
|
||||
|
|
@ -474,7 +478,7 @@ fn exec_options(
|
|||
),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
|
||||
}
|
||||
})
|
||||
}),
|
||||
)
|
||||
.chain([ApprovalOption {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,16 @@
|
|||
---
|
||||
source: tui2/src/chatwidget/tests.rs
|
||||
expression: contents
|
||||
---
|
||||
|
||||
|
||||
Would you like to run the following command?
|
||||
|
||||
$ python - <<'PY'
|
||||
print('hello')
|
||||
PY
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
|
@ -2123,6 +2123,45 @@ async fn approval_modal_exec_without_reason_snapshot() {
|
|||
);
|
||||
}
|
||||
|
||||
// Snapshot test: approval modal with a proposed execpolicy prefix that is multi-line;
|
||||
// we should not offer adding it to execpolicy.
|
||||
#[tokio::test]
|
||||
async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||
|
||||
let script = "python - <<'PY'\nprint('hello')\nPY".to_string();
|
||||
let command = vec!["bash".into(), "-lc".into(), script];
|
||||
let ev = ExecApprovalRequestEvent {
|
||||
call_id: "call-approve-cmd-multiline-trunc".into(),
|
||||
turn_id: "turn-approve-cmd-multiline-trunc".into(),
|
||||
command: command.clone(),
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
id: "sub-approve-multiline-trunc".into(),
|
||||
msg: EventMsg::ExecApprovalRequest(ev),
|
||||
});
|
||||
|
||||
let width = 100;
|
||||
let height = chat.desired_height(width);
|
||||
let mut terminal =
|
||||
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 (multiline prefix)");
|
||||
let contents = terminal.backend().vt100().screen().contents();
|
||||
assert!(!contents.contains("don't ask again"));
|
||||
assert_snapshot!(
|
||||
"approval_modal_exec_multiline_prefix_no_execpolicy",
|
||||
contents
|
||||
);
|
||||
}
|
||||
|
||||
// Snapshot test: patch approval modal
|
||||
#[tokio::test]
|
||||
async fn approval_modal_patch_snapshot() {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue