mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
fix: chatwidget was not honoring approval_id for an ExecApprovalRequestEvent
This commit is contained in:
@@ -111,11 +111,12 @@ impl PendingInteractiveReplayState {
|
||||
pub(super) fn note_event(&mut self, event: &Event) {
|
||||
match &event.msg {
|
||||
EventMsg::ExecApprovalRequest(ev) => {
|
||||
self.exec_approval_call_ids.insert(ev.call_id.clone());
|
||||
let approval_id = ev.effective_approval_id();
|
||||
self.exec_approval_call_ids.insert(approval_id.clone());
|
||||
self.exec_approval_call_ids_by_turn_id
|
||||
.entry(ev.turn_id.clone())
|
||||
.or_default()
|
||||
.push(ev.call_id.clone());
|
||||
.push(approval_id);
|
||||
}
|
||||
EventMsg::ExecCommandBegin(ev) => {
|
||||
self.exec_approval_call_ids.remove(&ev.call_id);
|
||||
@@ -173,11 +174,12 @@ impl PendingInteractiveReplayState {
|
||||
pub(super) fn note_evicted_event(&mut self, event: &Event) {
|
||||
match &event.msg {
|
||||
EventMsg::ExecApprovalRequest(ev) => {
|
||||
self.exec_approval_call_ids.remove(&ev.call_id);
|
||||
let approval_id = ev.effective_approval_id();
|
||||
self.exec_approval_call_ids.remove(&approval_id);
|
||||
Self::remove_call_id_from_turn_map_entry(
|
||||
&mut self.exec_approval_call_ids_by_turn_id,
|
||||
&ev.turn_id,
|
||||
&ev.call_id,
|
||||
&approval_id,
|
||||
);
|
||||
}
|
||||
EventMsg::ApplyPatchApprovalRequest(ev) => {
|
||||
@@ -218,7 +220,9 @@ impl PendingInteractiveReplayState {
|
||||
|
||||
pub(super) fn should_replay_snapshot_event(&self, event: &Event) -> bool {
|
||||
match &event.msg {
|
||||
EventMsg::ExecApprovalRequest(ev) => self.exec_approval_call_ids.contains(&ev.call_id),
|
||||
EventMsg::ExecApprovalRequest(ev) => self
|
||||
.exec_approval_call_ids
|
||||
.contains(&ev.effective_approval_id()),
|
||||
EventMsg::ApplyPatchApprovalRequest(ev) => {
|
||||
self.patch_approval_call_ids.contains(&ev.call_id)
|
||||
}
|
||||
@@ -362,7 +366,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_event_snapshot_drops_resolved_exec_approval_after_outbound_approval_call_id() {
|
||||
fn thread_event_snapshot_drops_resolved_exec_approval_after_outbound_approval_id() {
|
||||
let mut store = ThreadEventStore::new(8);
|
||||
store.push_event(Event {
|
||||
id: "ev-1".to_string(),
|
||||
@@ -384,7 +388,7 @@ mod tests {
|
||||
});
|
||||
|
||||
store.note_outbound_op(&Op::ExecApproval {
|
||||
id: "call-1".to_string(),
|
||||
id: "approval-1".to_string(),
|
||||
turn_id: Some("turn-1".to_string()),
|
||||
decision: codex_protocol::protocol::ReviewDecision::Approved,
|
||||
});
|
||||
|
||||
@@ -2569,7 +2569,7 @@ impl ChatWidget {
|
||||
self.notify(Notification::ExecApprovalRequested { command });
|
||||
|
||||
let request = ApprovalRequest::Exec {
|
||||
id: ev.call_id,
|
||||
id: ev.effective_approval_id(),
|
||||
command: ev.command,
|
||||
reason: ev.reason,
|
||||
network_approval_context: ev.network_approval_context,
|
||||
|
||||
@@ -2811,6 +2811,43 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_approval_uses_approval_id_when_present() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "sub-short".into(),
|
||||
msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
|
||||
call_id: "call-parent".into(),
|
||||
approval_id: Some("approval-subcommand".into()),
|
||||
turn_id: "turn-short".into(),
|
||||
command: vec!["bash".into(), "-lc".into(), "echo hello world".into()],
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
reason: Some(
|
||||
"this is a test reason such as one that would be produced by the model".into(),
|
||||
),
|
||||
network_approval_context: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
additional_permissions: None,
|
||||
parsed_cmd: vec![],
|
||||
}),
|
||||
});
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
|
||||
|
||||
let mut found = false;
|
||||
while let Ok(app_ev) = rx.try_recv() {
|
||||
if let AppEvent::CodexOp(Op::ExecApproval { id, decision, .. }) = app_ev {
|
||||
assert_eq!(id, "approval-subcommand");
|
||||
assert_matches!(decision, codex_protocol::protocol::ReviewDecision::Approved);
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(found, "expected ExecApproval op to be sent");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_approval_decision_truncates_multiline_and_long_commands() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
Reference in New Issue
Block a user