codex: address PR review feedback (#14710)

This commit is contained in:
Eric Traut
2026-03-14 17:21:44 -06:00
parent badad98b5a
commit 920a13fefd
4 changed files with 75 additions and 6 deletions

View File

@@ -7550,6 +7550,46 @@ smart_approvals = true
.expect("shutdown should complete");
}
#[tokio::test]
async fn override_turn_context_and_reload_user_config_use_runtime_path() {
let mut app = make_test_app().await;
let app_server = start_test_app_server(app.config.clone()).await;
let event = app
.thread_start_via_app_server(&app_server, &app.config.clone())
.await
.expect("thread/start should succeed");
assert!(
app.submit_app_server_op(
&app_server,
event.session_id,
Op::OverrideTurnContext {
cwd: None,
approval_policy: Some(AskForApproval::OnFailure),
approvals_reviewer: None,
sandbox_policy: None,
windows_sandbox_level: None,
model: None,
effort: None,
summary: None,
service_tier: None,
collaboration_mode: None,
personality: None,
}
)
.await
);
assert!(
app.submit_app_server_op(&app_server, event.session_id, Op::ReloadUserConfig)
.await
);
app_server
.shutdown()
.await
.expect("shutdown should complete");
}
#[tokio::test]
async fn clear_only_ui_reset_preserves_chat_session_state() {
let mut app = make_test_app().await;

View File

@@ -657,10 +657,6 @@ impl App {
.await?;
let _ = config;
}
Op::ReloadUserConfig | Op::OverrideTurnContext { .. } => {
// TODO(app-server): add a thread-scoped override/context refresh API so the TUI
// does not need to treat these as local-only state updates.
}
Op::Compact => {
let _: ThreadCompactStartResponse = send_request_with_response(
app_server_client,
@@ -728,9 +724,11 @@ impl App {
| Op::Undo
| Op::DropMemories
| Op::UpdateMemories
| Op::ReloadUserConfig
| Op::RunUserShellCommand { .. }
| Op::GetHistoryEntryRequest { .. }
| Op::ListMcpTools => {
| Op::ListMcpTools
| Op::OverrideTurnContext { .. } => {
// TODO(app-server): migrate these legacy-only TUI features once app-server grows
// equivalent APIs. Until then, keep routing the still-emitted TUI ops through the
// shared in-process thread runtime so existing behavior does not regress.

View File

@@ -799,6 +799,11 @@ impl AuthModeWidget {
self.request_frame.schedule_frame();
}
pub(crate) fn show_continue_in_browser_error(&mut self, message: String) {
self.error = Some(message);
self.request_frame.schedule_frame();
}
pub(crate) fn show_device_code_login_error(&mut self, message: String) {
*self.sign_in_state.write().unwrap() = SignInState::PickMode;
self.error = Some(message);
@@ -966,6 +971,32 @@ mod tests {
);
}
#[test]
fn continue_in_browser_error_keeps_active_login_state() {
let mut widget = auth_widget(Some(ForcedLoginMethod::Chatgpt), SignInOption::ChatGpt);
*widget.sign_in_state.write().unwrap() =
SignInState::ChatGptContinueInBrowser(ContinueInBrowserState {
auth_url: String::new(),
login_id: None,
});
widget.apply_chatgpt_login_started(
"login-123".to_string(),
"https://auth.example.com/login?state=abc123".to_string(),
);
widget.show_continue_in_browser_error("Failed to open browser".to_string());
assert_eq!(widget.error.as_deref(), Some("Failed to open browser"));
assert!(matches!(
&*widget.sign_in_state.read().unwrap(),
SignInState::ChatGptContinueInBrowser(ContinueInBrowserState {
auth_url,
login_id: Some(login_id),
}) if auth_url == "https://auth.example.com/login?state=abc123"
&& login_id == "login-123"
));
}
#[test]
fn device_code_login_pending_snapshot() {
let mut widget = auth_widget(None, SignInOption::DeviceCode);

View File

@@ -555,7 +555,7 @@ async fn handle_auth_command(
Ok(LoginAccountResponse::Chatgpt { login_id, auth_url }) => {
auth_widget.apply_chatgpt_login_started(login_id, auth_url.clone());
if let Err(err) = webbrowser::open(&auth_url) {
auth_widget.show_login_request_error(format!(
auth_widget.show_continue_in_browser_error(format!(
"Failed to open browser for {auth_url}: {err}"
));
}