codex: address PR review feedback (#14710)

This commit is contained in:
Eric Traut
2026-03-15 13:14:34 -06:00
parent 602d74c9bd
commit b0651b3d80
2 changed files with 73 additions and 3 deletions

View File

@@ -1827,6 +1827,12 @@ impl App {
self.active_thread_rx = None;
self.primary_thread_id = None;
self.pending_primary_updates.clear();
self.pending_exec_approval_request_ids.clear();
self.pending_patch_approval_request_ids.clear();
self.pending_elicitation_request_ids.clear();
self.pending_permissions_request_ids.clear();
self.pending_user_input_request_ids.clear();
self.pending_dynamic_tool_request_ids.clear();
self.chat_widget.set_pending_thread_approvals(Vec::new());
self.sync_active_agent_label();
}
@@ -4781,6 +4787,59 @@ mod tests {
.expect("listener task drop notification should succeed");
}
#[tokio::test]
async fn reset_thread_event_state_clears_pending_request_id_maps() {
let mut app = make_test_app().await;
app.pending_exec_approval_request_ids
.insert("exec".to_string(), RequestId::Integer(1));
app.pending_patch_approval_request_ids
.insert("patch".to_string(), RequestId::Integer(2));
app.pending_elicitation_request_ids.insert(
(
"turn".to_string(),
RequestId::String("elicitation".to_string()),
),
RequestId::Integer(3),
);
app.pending_permissions_request_ids
.insert("permissions".to_string(), RequestId::Integer(4));
app.pending_user_input_request_ids
.insert("turn".to_string(), VecDeque::from([RequestId::Integer(5)]));
app.pending_dynamic_tool_request_ids
.insert("tool".to_string(), RequestId::Integer(6));
app.reset_thread_event_state();
assert!(app.pending_exec_approval_request_ids.is_empty());
assert!(app.pending_patch_approval_request_ids.is_empty());
assert!(app.pending_elicitation_request_ids.is_empty());
assert!(app.pending_permissions_request_ids.is_empty());
assert!(app.pending_user_input_request_ids.is_empty());
assert!(app.pending_dynamic_tool_request_ids.is_empty());
}
#[tokio::test]
async fn unsubscribe_thread_via_app_server_errors_when_thread_is_not_loaded() {
let mut app = make_test_app().await;
let app_server = start_test_app_server(app.config.clone()).await;
let thread_id = ThreadId::new();
let err = app
.unsubscribe_thread_via_app_server(&app_server, thread_id)
.await
.expect_err("unsubscribing an unknown thread should fail");
assert_eq!(
err,
format!("thread {thread_id} is not loaded by the app server")
);
app_server
.shutdown()
.await
.expect("shutdown should complete");
}
#[tokio::test]
async fn enqueue_thread_event_does_not_block_when_channel_full() -> Result<()> {
let mut app = make_test_app().await;

View File

@@ -62,6 +62,7 @@ use codex_app_server_protocol::ThreadStartParams;
use codex_app_server_protocol::ThreadStartResponse;
use codex_app_server_protocol::ThreadUnsubscribeParams;
use codex_app_server_protocol::ThreadUnsubscribeResponse;
use codex_app_server_protocol::ThreadUnsubscribeStatus;
use codex_app_server_protocol::ToolRequestUserInputResponse as ApiToolRequestUserInputResponse;
use codex_app_server_protocol::TurnInterruptParams;
use codex_app_server_protocol::TurnInterruptResponse;
@@ -293,7 +294,7 @@ impl App {
app_server_client: &AppServerClient,
thread_id: ThreadId,
) -> Result<(), String> {
let _: ThreadUnsubscribeResponse = send_request_with_response(
let response: ThreadUnsubscribeResponse = send_request_with_response(
app_server_client,
ClientRequest::ThreadUnsubscribe {
request_id: self.next_app_server_request_id(),
@@ -304,8 +305,18 @@ impl App {
"thread/unsubscribe",
)
.await?;
self.active_turn_ids.remove(&thread_id);
Ok(())
match response.status {
ThreadUnsubscribeStatus::Unsubscribed => {
self.active_turn_ids.remove(&thread_id);
Ok(())
}
ThreadUnsubscribeStatus::NotLoaded => Err(format!(
"thread {thread_id} is not loaded by the app server"
)),
ThreadUnsubscribeStatus::NotSubscribed => Err(format!(
"thread {thread_id} is not subscribed by the app server"
)),
}
}
pub(super) async fn submit_app_server_op(