codex: address PR review feedback (#14710)

This commit is contained in:
Eric Traut
2026-03-14 15:19:19 -06:00
parent cc439296a3
commit 3515d9b034
4 changed files with 57 additions and 8 deletions

View File

@@ -703,7 +703,7 @@ pub(crate) struct App {
active_turn_ids: HashMap<ThreadId, String>,
pending_exec_approval_request_ids: HashMap<String, RequestId>,
pending_patch_approval_request_ids: HashMap<String, RequestId>,
pending_elicitation_request_ids: HashMap<(String, String), RequestId>,
pending_elicitation_request_ids: HashMap<(String, RequestId), RequestId>,
pending_permissions_request_ids: HashMap<String, RequestId>,
pending_user_input_request_ids: HashMap<String, RequestId>,
pending_dynamic_tool_request_ids: HashMap<String, RequestId>,

View File

@@ -553,7 +553,10 @@ impl App {
content,
meta,
} => {
let key = (server_name.clone(), request_id.to_string());
let key = (
server_name.clone(),
mcp_request_id_to_app_server_request_id(&request_id),
);
let Some(server_request_id) = self.pending_elicitation_request_ids.remove(&key)
else {
return Err(format!(
@@ -769,7 +772,7 @@ impl App {
}
ServerRequest::McpServerElicitationRequest { request_id, params } => {
self.pending_elicitation_request_ids.insert(
(params.server_name.clone(), request_id.to_string()),
(params.server_name.clone(), request_id.clone()),
request_id.clone(),
);
}
@@ -1439,6 +1442,15 @@ fn review_decision_to_file_change_decision(
}
}
fn mcp_request_id_to_app_server_request_id(
request_id: &codex_protocol::mcp::RequestId,
) -> RequestId {
match request_id {
codex_protocol::mcp::RequestId::String(value) => RequestId::String(value.clone()),
codex_protocol::mcp::RequestId::Integer(value) => RequestId::Integer(*value),
}
}
fn permission_grant_scope_to_api(
scope: PermissionGrantScope,
) -> codex_app_server_protocol::PermissionGrantScope {

View File

@@ -13,6 +13,7 @@ use serde::de::DeserializeOwned;
use crate::LoginStatus;
#[derive(Debug, PartialEq)]
pub(crate) enum AuthCommand {
StartApiKey { api_key: String },
StartChatgpt,

View File

@@ -721,12 +721,24 @@ impl AuthModeWidget {
}
pub(crate) fn apply_chatgpt_login_started(&mut self, login_id: String, auth_url: String) {
let mut sign_in_state = self.sign_in_state.write().unwrap();
let still_waiting_for_login = matches!(
&*sign_in_state,
SignInState::ChatGptContinueInBrowser(ContinueInBrowserState { login_id: None, .. })
);
if !still_waiting_for_login {
drop(sign_in_state);
let _ = self
.auth_command_tx
.send(AuthCommand::CancelChatgpt { login_id });
return;
}
self.error = None;
*self.sign_in_state.write().unwrap() =
SignInState::ChatGptContinueInBrowser(ContinueInBrowserState {
auth_url,
login_id: Some(login_id),
});
*sign_in_state = SignInState::ChatGptContinueInBrowser(ContinueInBrowserState {
auth_url,
login_id: Some(login_id),
});
self.request_frame.schedule_frame();
}
@@ -930,6 +942,30 @@ mod tests {
assert_eq!(found, url, "OSC 8 hyperlink should cover the full URL");
}
#[test]
fn chatgpt_login_start_after_cancel_requests_server_cancel_without_reopening_ui() {
let (auth_command_tx, mut auth_command_rx) = tokio::sync::mpsc::unbounded_channel();
let mut widget = auth_widget(Some(ForcedLoginMethod::Chatgpt), SignInOption::ChatGpt);
widget.auth_command_tx = auth_command_tx;
*widget.sign_in_state.write().unwrap() = SignInState::PickMode;
widget.apply_chatgpt_login_started(
"login-123".to_string(),
"https://auth.example.com/login?state=abc123".to_string(),
);
assert!(matches!(
&*widget.sign_in_state.read().unwrap(),
SignInState::PickMode
));
assert_eq!(
auth_command_rx.try_recv().unwrap(),
AuthCommand::CancelChatgpt {
login_id: "login-123".to_string()
}
);
}
#[test]
fn device_code_login_pending_snapshot() {
let mut widget = auth_widget(None, SignInOption::DeviceCode);