openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-29 19:08:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Scope TUI approval requests to their threads (#39372)

Browse Source 
## Why

Approval and item IDs can collide across concurrent threads. Tracking pending app-server approvals by ID alone can therefore resolve, submit, or dismiss a request from the wrong thread.

## What changed

- Key command execution, file change, and permissions approvals by both thread ID and approval ID.
- Route approval responses through the originating thread and require the thread to match when processing resolution notifications or dismissing approval prompts.
- Canonicalize parseable thread IDs so equivalent UUID representations still match.

## Testing

- Cover colliding approval IDs across threads, mismatched resolution notifications, prompt dismissal, and response routing for primary and background threads.

GitOrigin-RevId: c2d899a1e4b6a6413c78a0595a5311f5fabb9815
This commit is contained in:
Eric Traut
2026-08-19 02:28:42 +00:00
committed by copyberry
parent fde2156057
commit 8843960ba0
9 changed files with 358 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/app/app_server_events.rs
+9 -3
View File
@@ -116,16 +116,22 @@ impl App {
}
match &notification {
ServerNotification::ServerRequestResolved(notification) => {
let notification_thread_id =
codex_protocol::ThreadId::from_string(&notification.thread_id).ok();
self.pending_primary_events.retain(|event| {
!matches!(event, ThreadBufferedEvent::Request(request)
if request.id() == &notification.request_id)
if request.id() == &notification.request_id
&& server_request_thread_id(request) == notification_thread_id)
});
for requests in self.agents_overview.dispatched_requests.values_mut() {
if let Some(thread_id) = notification_thread_id
&& let Some(requests) =
self.agents_overview.dispatched_requests.get_mut(&thread_id)
{
requests.retain(|request| request.id() != &notification.request_id);
}
if let Some(request) = self
.pending_app_server_requests
.resolve_notification(&notification.request_id)
.resolve_notification(&notification.thread_id, &notification.request_id)
{
self.chat_widget.dismiss_app_server_request(&request);
if self.startup_pending_protected_request {
codex-rs/tui/src/app/app_server_requests.rs
+218 -86
View File
@@ -50,12 +50,15 @@ pub(super) struct UnsupportedAppServerRequest {
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum ResolvedAppServerRequest {
ExecApproval {
thread_id: String,
id: String,
},
FileChangeApproval {
thread_id: String,
id: String,
},
PermissionsApproval {
thread_id: String,
id: String,
},
UserInput {
@@ -69,14 +72,20 @@ pub(crate) enum ResolvedAppServerRequest {
#[derive(Debug, Default)]
pub(super) struct PendingAppServerRequests {
exec_approvals: HashMap<String, AppServerRequestId>,
file_change_approvals: HashMap<String, AppServerRequestId>,
permissions_approvals: HashMap<String, AppServerRequestId>,
exec_approvals: HashMap<(String, String), AppServerRequestId>,
file_change_approvals: HashMap<(String, String), AppServerRequestId>,
permissions_approvals: HashMap<(String, String), AppServerRequestId>,
user_inputs: HashMap<String, VecDeque<PendingUserInputRequest>>,
mcp_requests: HashMap<McpRequestKey, AppServerRequestId>,
}
impl PendingAppServerRequests {
fn canonical_thread_id(thread_id: &str) -> String {
codex_protocol::ThreadId::from_string(thread_id)
.map(|thread_id| thread_id.to_string())
.unwrap_or_else(|_| thread_id.to_string())
}
pub(super) fn clear(&mut self) {
self.exec_approvals.clear();
self.file_change_approvals.clear();
@@ -95,12 +104,20 @@ impl PendingAppServerRequests {
.approval_id
.clone()
.unwrap_or_else(|| params.item_id.clone());
self.exec_approvals.insert(approval_id, request_id.clone());
self.exec_approvals.insert(
(Self::canonical_thread_id(&params.thread_id), approval_id),
request_id.clone(),
);
None
}
ServerRequest::FileChangeRequestApproval { request_id, params } => {
self.file_change_approvals
.insert(params.item_id.clone(), request_id.clone());
self.file_change_approvals.insert(
(
Self::canonical_thread_id(&params.thread_id),
params.item_id.clone(),
),
request_id.clone(),
);
None
}
ServerRequest::PermissionsRequestApproval { request_id, params } => {
@@ -116,8 +133,13 @@ impl PendingAppServerRequests {
message: format!("failed to localize requested filesystem paths: {err}"),
});
}
self.permissions_approvals
.insert(params.item_id.clone(), request_id.clone());
self.permissions_approvals.insert(
(
Self::canonical_thread_id(&params.thread_id),
params.item_id.clone(),
),
request_id.clone(),
);
None
}
ServerRequest::ToolRequestUserInput { request_id, params } => {
@@ -178,16 +200,18 @@ impl PendingAppServerRequests {
pub(super) fn take_resolution<T>(
&mut self,
thread_id: &str,
op: T,
) -> Result<Option<AppServerRequestResolution>, String>
where
T: Into<AppCommand>,
{
let thread_id = Self::canonical_thread_id(thread_id);
let op: AppCommand = op.into();
let resolution = match &op {
AppCommand::ExecApproval { id, decision, .. } => self
.exec_approvals
.remove(id)
.remove(&(thread_id, id.clone()))
.map(|request_id| {
Ok::<AppServerRequestResolution, String>(AppServerRequestResolution {
request_id,
@@ -204,7 +228,7 @@ impl PendingAppServerRequests {
.transpose()?,
AppCommand::PatchApproval { id, decision } => self
.file_change_approvals
.remove(id)
.remove(&(thread_id, id.clone()))
.map(|request_id| {
Ok::<AppServerRequestResolution, String>(AppServerRequestResolution {
request_id,
@@ -219,7 +243,7 @@ impl PendingAppServerRequests {
.transpose()?,
AppCommand::RequestPermissionsResponse { id, response } => self
.permissions_approvals
.remove(id)
.remove(&(thread_id, id.clone()))
.map(|request_id| {
Ok::<AppServerRequestResolution, String>(AppServerRequestResolution {
request_id,
@@ -280,33 +304,38 @@ impl PendingAppServerRequests {
pub(super) fn resolve_notification(
&mut self,
thread_id: &str,
request_id: &AppServerRequestId,
) -> Option<ResolvedAppServerRequest> {
if let Some(id) = self
.exec_approvals
.iter()
.find_map(|(id, value)| (value == request_id).then(|| id.clone()))
{
self.exec_approvals.remove(&id);
return Some(ResolvedAppServerRequest::ExecApproval { id });
let thread_id = Self::canonical_thread_id(thread_id);
if let Some(key) = self.exec_approvals.iter().find_map(|(key, value)| {
(key.0 == thread_id && value == request_id).then(|| key.clone())
}) {
self.exec_approvals.remove(&key);
return Some(ResolvedAppServerRequest::ExecApproval {
thread_id: key.0,
id: key.1,
});
}
if let Some(id) = self
.file_change_approvals
.iter()
.find_map(|(id, value)| (value == request_id).then(|| id.clone()))
{
self.file_change_approvals.remove(&id);
return Some(ResolvedAppServerRequest::FileChangeApproval { id });
if let Some(key) = self.file_change_approvals.iter().find_map(|(key, value)| {
(key.0 == thread_id && value == request_id).then(|| key.clone())
}) {
self.file_change_approvals.remove(&key);
return Some(ResolvedAppServerRequest::FileChangeApproval {
thread_id: key.0,
id: key.1,
});
}
if let Some(id) = self
.permissions_approvals
.iter()
.find_map(|(id, value)| (value == request_id).then(|| id.clone()))
{
self.permissions_approvals.remove(&id);
return Some(ResolvedAppServerRequest::PermissionsApproval { id });
if let Some(key) = self.permissions_approvals.iter().find_map(|(key, value)| {
(key.0 == thread_id && value == request_id).then(|| key.clone())
}) {
self.permissions_approvals.remove(&key);
return Some(ResolvedAppServerRequest::PermissionsApproval {
thread_id: key.0,
id: key.1,
});
}
if let Some(pending) = self.remove_user_input_request(request_id) {
@@ -475,11 +504,14 @@ mod tests {
assert_eq!(pending.note_server_request(&request), None);
let resolution = pending
.take_resolution(&Op::ExecApproval {
id: "approval-1".to_string(),
turn_id: None,
decision: CommandExecutionApprovalDecision::Accept,
})
.take_resolution(
"thread-1",
&Op::ExecApproval {
id: "approval-1".to_string(),
turn_id: None,
decision: CommandExecutionApprovalDecision::Accept,
},
)
.expect("resolution should serialize")
.expect("request should be pending");
@@ -487,6 +519,83 @@ mod tests {
assert_eq!(resolution.result, json!({ "decision": "accept" }));
}
#[test]
fn colliding_approvals_resolve_only_on_their_own_thread() {
let approvals = [
(
"item/commandExecution/requestApproval",
Op::ExecApproval {
id: "shared-id".to_string(),
turn_id: None,
decision: CommandExecutionApprovalDecision::Accept,
},
),
(
"item/fileChange/requestApproval",
Op::PatchApproval {
id: "shared-id".to_string(),
decision: FileChangeApprovalDecision::Accept,
},
),
(
"item/permissions/requestApproval",
Op::RequestPermissionsResponse {
id: "shared-id".to_string(),
response: codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: codex_protocol::request_permissions::PermissionGrantScope::Turn,
strict_auto_review: false,
},
},
),
];
for (method, op) in approvals {
let mut pending = PendingAppServerRequests::default();
let thread_ids = [
codex_protocol::ThreadId::new().to_string(),
codex_protocol::ThreadId::new().to_string(),
];
for (index, thread_id) in thread_ids.iter().enumerate() {
let wire_thread_id = if index == 0 {
thread_id.to_ascii_uppercase()
} else {
thread_id.replace('-', "")
};
let request: ServerRequest = serde_json::from_value(json!({
"method": method,
"id": index + 1,
"params": {
"threadId": wire_thread_id,
"turnId": "turn-1",
"itemId": "shared-id",
"startedAtMs": 0,
"cwd": if cfg!(windows) { r"C:\tmp" } else { "/tmp" },
"permissions": {},
},
}))
.expect("approval request should deserialize");
assert_eq!(pending.note_server_request(&request), None);
assert!(pending.contains_server_request(&request));
}
assert_eq!(
pending.resolve_notification(&thread_ids[1], &AppServerRequestId::Integer(1)),
None
);
for (thread_id, request_id) in thread_ids.iter().zip(1..=2) {
let resolution = pending
.take_resolution(thread_id, &op)
.expect("approval resolution should serialize")
.expect("approval should remain pending on its own thread");
assert_eq!(
resolution.request_id,
AppServerRequestId::Integer(request_id)
);
}
}
}
#[test]
fn rejects_permissions_with_paths_that_cannot_be_localized() {
let mut pending = PendingAppServerRequests::default();
@@ -587,22 +696,25 @@ mod tests {
);
let permissions = pending
.take_resolution(&Op::RequestPermissionsResponse {
id: "perm-1".to_string(),
response: codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: RequestPermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path(read_path)]),
Some(vec![absolute_path(write_path)]),
)),
.take_resolution(
"thread-1",
&Op::RequestPermissionsResponse {
id: "perm-1".to_string(),
response: codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: RequestPermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path(read_path)]),
Some(vec![absolute_path(write_path)]),
)),
},
scope: codex_protocol::request_permissions::PermissionGrantScope::Session,
strict_auto_review: false,
},
scope: codex_protocol::request_permissions::PermissionGrantScope::Session,
strict_auto_review: false,
},
})
)
.expect("permissions response should serialize")
.expect("permissions request should be pending");
assert_eq!(permissions.request_id, AppServerRequestId::Integer(7));
@@ -640,18 +752,21 @@ mod tests {
);
let user_input = pending
.take_resolution(&Op::UserInputAnswer {
id: "turn-2".to_string(),
response: ToolRequestUserInputResponse {
answers: std::iter::once((
"question".to_string(),
ToolRequestUserInputAnswer {
answers: vec!["yes".to_string()],
},
))
.collect(),
.take_resolution(
"thread-1",
&Op::UserInputAnswer {
id: "turn-2".to_string(),
response: ToolRequestUserInputResponse {
answers: std::iter::once((
"question".to_string(),
ToolRequestUserInputAnswer {
answers: vec!["yes".to_string()],
},
))
.collect(),
},
},
})
)
.expect("user input response should serialize")
.expect("user input request should be pending");
assert_eq!(user_input.request_id, AppServerRequestId::Integer(8));
@@ -697,13 +812,16 @@ mod tests {
);
let resolution = pending
.take_resolution(&Op::ResolveElicitation {
server_name: "example".to_string(),
request_id: AppServerRequestId::Integer(12),
decision: McpServerElicitationAction::Accept,
content: Some(json!({ "answer": "yes" })),
meta: Some(json!({ "source": "tui" })),
})
.take_resolution(
"thread-1",
&Op::ResolveElicitation {
server_name: "example".to_string(),
request_id: AppServerRequestId::Integer(12),
decision: McpServerElicitationAction::Accept,
content: Some(json!({ "answer": "yes" })),
meta: Some(json!({ "source": "tui" })),
},
)
.expect("elicitation response should serialize")
.expect("elicitation request should be pending");
@@ -779,10 +897,13 @@ mod tests {
);
let resolution = pending
.take_resolution(&Op::PatchApproval {
id: "patch-1".to_string(),
decision: FileChangeApprovalDecision::Cancel,
})
.take_resolution(
"thread-1",
&Op::PatchApproval {
id: "patch-1".to_string(),
decision: FileChangeApprovalDecision::Cancel,
},
)
.expect("resolution should serialize")
.expect("request should be pending");
@@ -793,11 +914,12 @@ mod tests {
#[test]
fn resolve_notification_returns_resolved_exec_request() {
let mut pending = PendingAppServerRequests::default();
let thread_id = codex_protocol::ThreadId::new().to_string();
assert_eq!(
pending.note_server_request(&ServerRequest::CommandExecutionRequestApproval {
request_id: AppServerRequestId::Integer(41),
params: CommandExecutionRequestApprovalParams {
thread_id: "thread-1".to_string(),
thread_id: thread_id.to_ascii_uppercase(),
turn_id: "turn-1".to_string(),
item_id: "call-1".to_string(),
started_at_ms: 0,
@@ -818,13 +940,17 @@ mod tests {
);
assert_eq!(
pending.resolve_notification(&AppServerRequestId::Integer(41)),
pending.resolve_notification(
&thread_id.replace('-', ""),
&AppServerRequestId::Integer(41)
),
Some(ResolvedAppServerRequest::ExecApproval {
thread_id: thread_id.clone(),
id: "approval-1".to_string(),
})
);
assert_eq!(
pending.resolve_notification(&AppServerRequestId::Integer(41)),
pending.resolve_notification(&thread_id, &AppServerRequestId::Integer(41)),
None
);
}
@@ -855,7 +981,7 @@ mod tests {
);
assert_eq!(
pending.resolve_notification(&AppServerRequestId::Integer(12)),
pending.resolve_notification("thread-1", &AppServerRequestId::Integer(12)),
Some(ResolvedAppServerRequest::McpElicitation {
server_name: "example".to_string(),
request_id: AppServerRequestId::Integer(12),
@@ -879,7 +1005,7 @@ mod tests {
});
assert_eq!(
pending.resolve_notification(&AppServerRequestId::Integer(8)),
pending.resolve_notification("thread-1", &AppServerRequestId::Integer(8)),
Some(ResolvedAppServerRequest::UserInput {
call_id: "tool-1".to_string(),
})
@@ -907,17 +1033,23 @@ mod tests {
answers: HashMap::new(),
};
let first_response = pending
.take_resolution(&Op::UserInputAnswer {
id: "turn-1".to_string(),
response: response.clone(),
})
.take_resolution(
"thread-1",
&Op::UserInputAnswer {
id: "turn-1".to_string(),
response: response.clone(),
},
)
.expect("user input response should serialize")
.expect("first user input request should be pending");
let second_response = pending
.take_resolution(&Op::UserInputAnswer {
id: "turn-1".to_string(),
response,
})
.take_resolution(
"thread-1",
&Op::UserInputAnswer {
id: "turn-1".to_string(),
response,
},
)
.expect("user input response should serialize")
.expect("second user input request should be pending");
codex-rs/tui/src/app/tests.rs
+73 -19
View File
@@ -377,7 +377,30 @@ async fn enqueue_primary_thread_session_replays_buffered_approval_after_attach()
.note_server_request(&approval_request),
None
);
app.enqueue_primary_thread_request(approval_request).await?;
app.agents_overview
.dispatched_requests
.insert(thread_id, vec![approval_request.clone()]);
app.enqueue_primary_thread_request(approval_request.clone())
.await?;
let (mut app_server, requests, proxy) = session_lifecycle_requests::start_recording_app_server(
&app.config,
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
)
.await?;
app.handle_app_server_event(
&app_server,
codex_app_server_client::AppServerEvent::ServerNotification(Box::new(
ServerNotification::ServerRequestResolved(
codex_app_server_protocol::ServerRequestResolvedNotification {
thread_id: ThreadId::new().to_string(),
request_id: AppServerRequestId::Integer(1),
},
),
)),
)
.await;
assert_eq!(app.agents_overview.dispatched_requests[&thread_id].len(), 1);
app.enqueue_primary_thread_session(
test_thread_session(thread_id, test_path_buf("/tmp/project")),
Vec::new(),
@@ -404,21 +427,49 @@ async fn enqueue_primary_thread_session_replays_buffered_approval_after_attach()
));
app.handle_thread_event_now(event);
app.chat_widget
.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
let background_thread_id = ThreadId::new();
let mut background_request = approval_request;
let ServerRequest::CommandExecutionRequestApproval { request_id, params } =
&mut background_request
else {
unreachable!("expected a command approval request");
};
*request_id = AppServerRequestId::Integer(2);
params.thread_id = background_thread_id.to_string();
app.pending_app_server_requests
.note_server_request(&background_request);
app.enqueue_thread_request(background_thread_id, background_request)
.await?;
while let Ok(app_event) = app_event_rx.try_recv() {
if let AppEvent::SubmitThreadOp {
thread_id: op_thread_id,
..
} = app_event
{
assert_eq!(op_thread_id, thread_id);
return Ok(());
}
for expected_thread_id in [thread_id, background_thread_id] {
assert!(app.chat_widget.has_active_view());
app.chat_widget
.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
let (actual_thread_id, op) = std::iter::from_fn(|| app_event_rx.try_recv().ok())
.find_map(|event| match event {
AppEvent::SubmitThreadOp { thread_id, op } => Some((thread_id, op)),
_ => None,
})
.expect("approval should submit a thread-scoped operation");
assert_eq!(actual_thread_id, expected_thread_id);
app.submit_thread_op(&mut app_server, expected_thread_id, op)
.await?;
}
assert!(!app.chat_widget.has_active_view());
panic!("expected approval action to submit a thread-scoped op");
app_server.shutdown().await?;
proxy.await??;
assert_eq!(
requests
.lock()
.expect("request recorder lock")
.iter()
.filter(|request| request.method == "server/request/response")
.map(|request| request.id.clone())
.collect::<Vec<_>>(),
(1..=2).map(AppServerRequestId::Integer).collect::<Vec<_>>()
);
Ok(())
}
#[tokio::test]
@@ -445,7 +496,7 @@ async fn resolved_buffered_approval_does_not_become_actionable_after_drain() ->
let resolved = app
.pending_app_server_requests
.resolve_notification(&AppServerRequestId::Integer(1))
.resolve_notification(&thread_id.to_string(), &AppServerRequestId::Integer(1))
.expect("matching app-server request should resolve");
app.chat_widget.dismiss_app_server_request(&resolved);
while app_event_rx.try_recv().is_ok() {}
@@ -4874,11 +4925,14 @@ async fn background_side_cleanup_removes_local_state_and_ignores_late_events() -
.await;
let resolution = app
.pending_app_server_requests
.take_resolution(Op::ExecApproval {
id: "approval-1".to_string(),
turn_id: None,
decision: codex_app_server_protocol::CommandExecutionApprovalDecision::Accept,
})
.take_resolution(
&side_thread_id.to_string(),
Op::ExecApproval {
id: "approval-1".to_string(),
turn_id: None,
decision: codex_app_server_protocol::CommandExecutionApprovalDecision::Accept,
},
)
.expect("approval resolution should serialize");
assert_eq!(resolution, None);
Ok(())
codex-rs/tui/src/app/tests/session_lifecycle_requests.rs
+13 -2
View File
@@ -64,7 +64,7 @@ fn take_backfill_counts(requests: &RecordedRequests) -> (usize, usize) {
}
/// Starts an embedded app server behind a loopback WebSocket proxy that records JSON-RPC methods.
async fn start_recording_app_server(
pub(super) async fn start_recording_app_server(
config: &Config,
blocked_thread_list: Option<(ThreadId, oneshot::Sender<()>, oneshot::Receiver<()>)>,
failed_thread_name: Option<&'static str>,
@@ -255,7 +255,18 @@ async fn start_recording_app_server_with_history(
)?)
.await?;
}
JSONRPCMessage::Response(_) | JSONRPCMessage::Error(_) => {}
JSONRPCMessage::Response(response) => {
request_sink
.lock()
.expect("request recorder lock")
.push(JSONRPCRequest {
id: response.id,
method: "server/request/response".to_string(),
params: Some(response.result),
trace: None,
});
}
JSONRPCMessage::Error(_) => {}
}
}
embedded.shutdown().await?;
codex-rs/tui/src/app/thread_routing.rs
+1 -1
View File
@@ -876,7 +876,7 @@ impl App {
) -> Result<bool> {
let Some(resolution) = self
.pending_app_server_requests
.take_resolution(op)
.take_resolution(&thread_id.to_string(), op)
.map_err(|err| color_eyre::eyre::eyre!(err))?
else {
return Ok(false);
codex-rs/tui/src/bottom_pane/approval_overlay.rs
+18 -8
View File
@@ -120,7 +120,7 @@ pub(crate) struct McpElicitationApprovalRequest {
}
impl ApprovalRequest {
fn thread_id(&self) -> ThreadId {
pub(super) fn thread_id(&self) -> ThreadId {
match self {
ApprovalRequest::Exec(request) => request.thread_id,
ApprovalRequest::Permissions(request) => request.thread_id,
@@ -142,16 +142,16 @@ impl ApprovalRequest {
match (self, request) {
(
ApprovalRequest::Exec(request),
ResolvedAppServerRequest::ExecApproval { id: resolved_id },
) => request.id == *resolved_id,
ResolvedAppServerRequest::ExecApproval { thread_id, id },
) => request.thread_id.to_string() == *thread_id && request.id == *id,
(
ApprovalRequest::Permissions(request),
ResolvedAppServerRequest::PermissionsApproval { id },
) => request.call_id == *id,
ResolvedAppServerRequest::PermissionsApproval { thread_id, id },
) => request.thread_id.to_string() == *thread_id && request.call_id == *id,
(
ApprovalRequest::ApplyPatch(request),
ResolvedAppServerRequest::FileChangeApproval { id: resolved_id },
) => request.id == *resolved_id,
ResolvedAppServerRequest::FileChangeApproval { thread_id, id },
) => request.thread_id.to_string() == *thread_id && request.id == *id,
(
ApprovalRequest::McpElicitation(request),
ResolvedAppServerRequest::McpElicitation {
@@ -1475,10 +1475,20 @@ mod tests {
fn resolved_request_dismisses_overlay_without_emitting_abort() {
let (tx, mut rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx);
let mut view = make_overlay(make_exec_request(), tx, Features::with_defaults());
let request = make_exec_request();
let thread_id = request.thread_id();
let mut view = make_overlay(request, tx, Features::with_defaults());
assert!(
!view.dismiss_app_server_request(&ResolvedAppServerRequest::ExecApproval {
thread_id: ThreadId::new().to_string(),
id: "test".to_string(),
})
);
assert!(!view.is_complete());
assert!(
view.dismiss_app_server_request(&ResolvedAppServerRequest::ExecApproval {
thread_id: thread_id.to_string(),
id: "test".to_string(),
})
);
codex-rs/tui/src/bottom_pane/mod.rs
+7 -2
View File
@@ -2018,7 +2018,7 @@ mod tests {
}
fn dismiss_app_server_request(&mut self, request: &ResolvedAppServerRequest) -> bool {
let ResolvedAppServerRequest::ExecApproval { id } = request else {
let ResolvedAppServerRequest::ExecApproval { id, .. } = request else {
return false;
};
if self.dismiss_exec_id != Some(id.as_str()) {
@@ -2262,10 +2262,13 @@ mod tests {
let mut pane = test_pane(tx);
let now = Instant::now();
pane.last_composer_activity_at = Some(now);
pane.push_approval_request(exec_request(), &features);
let request = exec_request();
let thread_id = request.thread_id();
pane.push_approval_request(request, &features);
assert!(
pane.dismiss_app_server_request(&ResolvedAppServerRequest::ExecApproval {
thread_id: thread_id.to_string(),
id: "1".to_string(),
})
);
@@ -2294,6 +2297,7 @@ mod tests {
assert!(
pane.dismiss_app_server_request(&ResolvedAppServerRequest::ExecApproval {
thread_id: "thread-1".to_string(),
id: "request-1".to_string(),
})
);
@@ -2323,6 +2327,7 @@ mod tests {
assert!(
!pane.dismiss_app_server_request(&ResolvedAppServerRequest::ExecApproval {
thread_id: "thread-1".to_string(),
id: "request-1".to_string(),
})
);
codex-rs/tui/src/chatwidget.rs
+13 -1
View File
@@ -1032,7 +1032,19 @@ impl ChatWidget {
pub(crate) fn dismiss_app_server_request(&mut self, request: &ResolvedAppServerRequest) {
// A remotely resolved request must not remain user-actionable. It may be
// materialized in the bottom pane or still deferred behind active streaming.
let removed_deferred = self.interrupts.remove_resolved_prompt(request);
let request_thread_id = match request {
ResolvedAppServerRequest::ExecApproval { thread_id, .. }
| ResolvedAppServerRequest::FileChangeApproval { thread_id, .. }
| ResolvedAppServerRequest::PermissionsApproval { thread_id, .. } => {
Some(thread_id.as_str())
}
ResolvedAppServerRequest::UserInput { .. }
| ResolvedAppServerRequest::McpElicitation { .. } => None,
};
let removed_deferred = request_thread_id.is_none_or(|request_thread_id| {
self.thread_id
.is_some_and(|thread_id| thread_id.to_string() == request_thread_id)
}) && self.interrupts.remove_resolved_prompt(request);
let removed_visible = self.bottom_pane.dismiss_app_server_request(request);
if removed_deferred || removed_visible {
self.request_redraw();
codex-rs/tui/src/chatwidget/interrupts.rs
+6 -3
View File
@@ -126,11 +126,11 @@ impl QueuedInterrupt {
fn matches_resolved_prompt(&self, request: &ResolvedAppServerRequest) -> bool {
match self {
QueuedInterrupt::ExecApproval(ev) => {
matches!(request, ResolvedAppServerRequest::ExecApproval { id }
matches!(request, ResolvedAppServerRequest::ExecApproval { id, .. }
if ev.effective_approval_id() == id.as_str())
}
QueuedInterrupt::ApplyPatchApproval(ev) => {
matches!(request, ResolvedAppServerRequest::FileChangeApproval { id }
matches!(request, ResolvedAppServerRequest::FileChangeApproval { id, .. }
if ev.call_id == id.as_str())
}
QueuedInterrupt::Elicitation { request_id, params } => {
@@ -140,7 +140,7 @@ impl QueuedInterrupt {
} if params.server_name == server_name.as_str() && request_id == resolved_request_id)
}
QueuedInterrupt::RequestPermissions(ev) => {
matches!(request, ResolvedAppServerRequest::PermissionsApproval { id }
matches!(request, ResolvedAppServerRequest::PermissionsApproval { id, .. }
if ev.call_id == id.as_str())
}
QueuedInterrupt::RequestUserInput(ev) => {
@@ -242,6 +242,7 @@ mod tests {
assert!(
!manager.remove_resolved_prompt(&ResolvedAppServerRequest::ExecApproval {
thread_id: "thread-1".to_string(),
id: "call".to_string(),
})
);
@@ -249,6 +250,7 @@ mod tests {
assert!(
manager.remove_resolved_prompt(&ResolvedAppServerRequest::ExecApproval {
thread_id: "thread-1".to_string(),
id: "approval".to_string(),
})
);
@@ -263,6 +265,7 @@ mod tests {
assert!(
!manager.remove_resolved_prompt(&ResolvedAppServerRequest::ExecApproval {
thread_id: "thread-1".to_string(),
id: "call".to_string(),
})
);