From 2fde968de8bc9949f79d2f5565422f865b8f0fcc Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 30 Sep 2026 22:52:00 +0000 Subject: [PATCH] Preserve local launch permissions across TUI sessions and reconnects (#49809) ## Why Explicit launch permissions such as `--yolo` can be lost during reconnect recovery, causing subsequent input to use different approval and sandbox settings. ## What changed Remember explicit local launch approval policies, approvals reviewers, and permission profiles at startup and when creating new sessions. Restore the approvals reviewer independently of profile overrides during reconnect recovery. Keep constrained profiles that cannot be expressed through legacy turn parameters under server control. ## Testing Add regression coverage for `--yolo` across new sessions and reconnects, later permission-picker choices, reviewer-only overrides, and server-owned constrained profiles. Extend the reconnect integration test to submit the preserved draft and verify its approval and sandbox settings. GitOrigin-RevId: 7a77dc41b2a357ab17fec9ba6cb2361f4a8f4aa5 --- codex-rs/tui/src/app/agents_overview.rs | 3 + codex-rs/tui/src/app/reconnect.rs | 5 ++ codex-rs/tui/src/app/session_lifecycle.rs | 1 + codex-rs/tui/src/app/startup.rs | 37 ++++++++++ .../src/app/tests/active_reconnect_tests.rs | 16 +++++ .../tui/src/app/tests/new_session_tests.rs | 72 +++++++++++++++++++ codex-rs/tui/src/app/tests/turn_submission.rs | 32 +++++++++ codex-rs/tui/tests/suite/reconnect.rs | 70 ++++++++++++++++-- 8 files changed, 229 insertions(+), 7 deletions(-) diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index f7639dd86b..341e116648 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -775,6 +775,9 @@ impl App { .set_workspace_roots(self.config.permissions.workspace_roots().to_vec()); } self.config = destination_config; + if is_new_session { + self.remember_launch_permissions(); + } if !read_only { let approval = self.config.permissions.approval_policy.value(); if self diff --git a/codex-rs/tui/src/app/reconnect.rs b/codex-rs/tui/src/app/reconnect.rs index 132f4f9388..bdb643db84 100644 --- a/codex-rs/tui/src/app/reconnect.rs +++ b/codex-rs/tui/src/app/reconnect.rs @@ -147,6 +147,11 @@ impl App { { session.approval_policy = policy; } + if let Some(reviewer) = self.runtime_approvals_reviewer_override + && reviewer == cached.approvals_reviewer + { + session.approvals_reviewer = reviewer; + } if let Some(profile) = &self.runtime_permission_profile_override && profile.permission_profile == cached.permission_profile && profile.active_permission_profile == cached.active_permission_profile diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index 4ffd85ed63..c1bdc28b26 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -1038,6 +1038,7 @@ impl App { self.local_settings = self.local_settings.reloaded(&config); self.refresh_server_version_overview_notice(CODEX_CLI_VERSION); self.config = config; + self.remember_launch_permissions(); let name_error = if let Some(name) = new_thread_name { match app_server diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index ce40f68450..0714428e9d 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -3,6 +3,7 @@ //! Owns the main app run loop from app-server bootstrap through terminal shutdown. Startup input //! remains isolated from protected interactive requests until the initialized composer owns it. //! Queued resume history replaces the provisional loading frame only when it is ready to render. +//! Explicit local launch permissions remain runtime overrides across new sessions and reconnects. use super::reconnect::ReconnectState; use super::*; @@ -121,6 +122,41 @@ pub(super) fn startup_model( } impl App { + /// Keep explicit local launch choices available for new sessions and reconnect recovery. + pub(super) fn remember_launch_permissions(&mut self) { + if self.app_server_target.thread_params_mode() + == crate::app_server_session::ThreadParamsMode::Remote + { + return; + } + let selected = crate::resume_permissions::ResumePermissions::from_overrides( + &self.config, + &self.harness_overrides, + ); + if selected.approval_policy { + self.runtime_approval_policy_override = Some(RuntimeApprovalPolicyOverride::Explicit( + self.config.permissions.approval_policy.value().into(), + )); + } + if selected.approvals_reviewer { + self.runtime_approvals_reviewer_override = Some(self.config.approvals_reviewer); + } + if selected.profile { + let profile = RuntimePermissionProfileOverride::from_config(&self.config); + // The server owns constrained profiles that legacy turn parameters cannot express. + self.runtime_permission_profile_override = + (profile.active_permission_profile.is_some() + || crate::app_server_session::turn_permissions_overrides( + TurnPermissionsOverride::LegacySandbox( + self.config.permissions.effective_permission_profile(), + ), + self.config.cwd.as_path(), + ) + .is_ok()) + .then_some(profile); + } + } + /// Keep the provisional loading frame until queued history reaches the owned transcript. /// Visible startup decisions and the agent overview must still render immediately. pub(super) fn render_startup_frame( @@ -889,6 +925,7 @@ See the Codex keymap documentation for supported actions and examples." #[cfg(test)] _test_codex_home: None, }; + app.remember_launch_permissions(); if !tui.is_terminal_focused() { app.recap.note_focus_lost(Instant::now()); } diff --git a/codex-rs/tui/src/app/tests/active_reconnect_tests.rs b/codex-rs/tui/src/app/tests/active_reconnect_tests.rs index de130ba8d3..7723ab1b6b 100644 --- a/codex-rs/tui/src/app/tests/active_reconnect_tests.rs +++ b/codex-rs/tui/src/app/tests/active_reconnect_tests.rs @@ -11,6 +11,22 @@ use tokio::net::TcpListener; use super::disconnect::serve_reconnect_requests; +#[tokio::test] +async fn reconnect_restores_launch_reviewer_without_a_profile_override() -> Result<()> { + let (mut app, _events, _ops) = make_test_app_with_channels().await; + let mut cached = test_thread_session(ThreadId::new(), app.config.cwd.to_path_buf()); + cached.approvals_reviewer = ApprovalsReviewer::AutoReview; + app.primary_thread_id = Some(cached.thread_id); + app.config.approvals_reviewer = ApprovalsReviewer::AutoReview; + app.harness_overrides.approvals_reviewer = Some(ApprovalsReviewer::AutoReview); + app.remember_launch_permissions(); + let mut resumed = cached.clone(); + resumed.approvals_reviewer = ApprovalsReviewer::User; + app.restore_runtime_permissions(&mut resumed, &cached); + assert_eq!(resumed, cached); + Ok(()) +} + #[tokio::test] async fn reconnect_restores_history_permissions_and_resumes_unsent_input() -> Result<()> { for (recovered_queue, edit_offline, resume_error_code, deferred_notice, notice_enabled) in [ diff --git a/codex-rs/tui/src/app/tests/new_session_tests.rs b/codex-rs/tui/src/app/tests/new_session_tests.rs index e77967d796..99c8addc39 100644 --- a/codex-rs/tui/src/app/tests/new_session_tests.rs +++ b/codex-rs/tui/src/app/tests/new_session_tests.rs @@ -6,6 +6,78 @@ use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; use pretty_assertions::assert_eq; +#[tokio::test] +async fn new_sessions_preserve_yolo_launch_and_later_permission_choices() -> Result<()> { + let (mut app, _events, _ops) = make_test_app_with_channels().await; + app.harness_overrides.approval_policy = Some(AskForApproval::Never.to_core()); + app.harness_overrides.sandbox_mode = + Some(codex_protocol::config_types::SandboxMode::DangerFullAccess); + let (mut server, requests, proxy) = start_recording_app_server( + &app.config, + /*blocked_thread_list*/ None, + /*failed_thread_name*/ None, + ) + .await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + let mut loaded_config = app.config.clone(); + loaded_config + .permissions + .approval_policy + .set(AskForApproval::OnRequest.to_core())?; + loaded_config + .permissions + .set_permission_profile(PermissionProfile::read_only())?; + let loaded = server.start_thread(&loaded_config).await?; + app.select_agents_overview_thread(&mut tui, &mut server, loaded.session.thread_id) + .await?; + for picker_change in [false, true] { + if picker_change { + app.chat_widget + .set_approval_policy(AskForApproval::OnRequest); + app.chat_widget + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + PermissionProfile::read_only(), + ActivePermissionProfile::new(":read-only"), + ))?; + app.adopt_server_permissions(); + } + app.start_fresh_session( + &mut tui, + &mut server, + /*session_start_source*/ None, + /*initial_user_message*/ None, + /*new_thread_name*/ None, + ) + .await; + let cached = app.primary_session_configured.as_ref().unwrap().clone(); + let mut resumed = cached.clone(); + resumed.approval_policy = AskForApproval::OnRequest; + resumed.permission_profile = PermissionProfile::read_only(); + resumed.active_permission_profile = None; + app.restore_runtime_permissions(&mut resumed, &cached); + assert_eq!(resumed, cached); + } + let params = recorded_params(&requests, "thread/start"); + assert_eq!( + params + .iter() + .skip(1) + .map(|params| serde_json::json!({ + "approvalPolicy": params["approvalPolicy"], + "sandbox": params["sandbox"], + "permissions": params["permissions"], + })) + .collect::>(), + vec![ + serde_json::json!({"approvalPolicy": "never", "sandbox": "danger-full-access", "permissions": null}), + serde_json::json!({"approvalPolicy": "on-request", "sandbox": null, "permissions": ":read-only"}), + ] + ); + server.shutdown().await?; + proxy.await??; + Ok(()) +} + #[tokio::test] async fn new_session_preserves_vim_line_yank() -> Result<()> { let (mut app, _events, _ops) = make_test_app_with_channels().await; diff --git a/codex-rs/tui/src/app/tests/turn_submission.rs b/codex-rs/tui/src/app/tests/turn_submission.rs index 6fd095fbb8..15030ebae3 100644 --- a/codex-rs/tui/src/app/tests/turn_submission.rs +++ b/codex-rs/tui/src/app/tests/turn_submission.rs @@ -358,6 +358,38 @@ async fn misalignment_policy_in_parent_stops_active_side_conversation() -> Resul Ok(()) } +#[tokio::test] +async fn launch_permissions_preserve_server_owned_constrained_profile() -> Result<()> { + let (mut app, _events, _ops) = make_test_app_with_channels().await; + app.harness_overrides.sandbox_mode = + Some(codex_protocol::config_types::SandboxMode::DangerFullAccess); + app.config + .permissions + .set_permission_profile(PermissionProfile::Disabled)?; + app.remember_launch_permissions(); + let mut profile = PermissionProfile::workspace_write(); + let PermissionProfile::Managed { + file_system: ManagedFileSystemPermissions::Restricted { entries, .. }, + .. + } = &mut profile + else { + unreachable!("workspace-write uses managed permissions"); + }; + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: "**/*.secret".into(), + }, + access: FileSystemAccessMode::Deny, + missing_path_behavior: None, + }); + app.config.permissions.set_permission_profile(profile)?; + app.harness_overrides.sandbox_mode = + Some(codex_protocol::config_types::SandboxMode::WorkspaceWrite); + app.remember_launch_permissions(); + assert_eq!(app.runtime_permission_profile_override, None); + Ok(()) +} + #[tokio::test] async fn unsupported_legacy_permissions_are_shown_without_exiting() -> Result<()> { let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; diff --git a/codex-rs/tui/tests/suite/reconnect.rs b/codex-rs/tui/tests/suite/reconnect.rs index e62dc3dbb3..609abe7910 100644 --- a/codex-rs/tui/tests/suite/reconnect.rs +++ b/codex-rs/tui/tests/suite/reconnect.rs @@ -16,10 +16,13 @@ use tokio_tungstenite::tungstenite::Message; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Result<()> { - let repo_root = codex_utils_cargo_bin::repo_root()?; + core_test_support::skip_if_sandbox!(Ok(())); + // Use a workspace without project-specific permission requirements. + let workspace = tempfile::tempdir()?; + let workspace_path = workspace.path().canonicalize()?; // macOS's default temporary directory leaves too little room for the control socket path. let codex_home = tempfile::tempdir_in("/tmp")?; - write_test_config(codex_home.path(), &repo_root)?; + write_test_config(codex_home.path(), &workspace_path)?; let config_path = codex_home.path().join("config.toml"); let config = std::fs::read_to_string(&config_path)?; std::fs::write( @@ -31,11 +34,15 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re let listener = UnixListener::bind(socket.as_path())?; let (disconnect_tx, mut disconnect_rx) = tokio::sync::oneshot::channel(); let (restore_tx, restore_rx) = tokio::sync::oneshot::channel(); - let server_cwd = repo_root.clone(); + let (complete_tx, mut complete_rx) = tokio::sync::oneshot::channel(); + let (submitted_tx, submitted_rx) = tokio::sync::oneshot::channel(); + let server_cwd = workspace_path.clone(); let id = "00000000-0000-0000-0000-000000000001"; let server = tokio::spawn(async move { let mut methods = Vec::new(); let mut restore_rx = Some(restore_rx); + let mut recovered_completed = false; + let mut submitted_tx = Some(submitted_tx); let thread = json!({ "id": id, "sessionId": id, "preview": "", "ephemeral": false, "modelProvider": "openai", "createdAt": 1, "updatedAt": 2, @@ -53,6 +60,16 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re }; loop { let frame = tokio::select! { + _ = &mut complete_rx, if connection == 6 && !recovered_completed => { + recovered_completed = true; + socket.send(Message::Text(json!({ + "method": "turn/completed", "params": { + "threadId": id, "turn": {"id": "running", "status": "completed", "error": null, + "items": [{"type": "agentMessage", "id": "completed-item", "text": "Recovered turn completed."}]} + } + }).to_string().into())).await?; + continue; + } _ = &mut disconnect_rx, if connection == 0 => { socket.close(/*msg*/ None).await?; break; @@ -105,9 +122,34 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re } "configRequirements/read" => json!({"requirements": null}), "thread/start" | "thread/resume" => { + let starting = request.method == "thread/start"; + if !starting { + let params = request.params.as_ref().unwrap(); + assert_eq!( + json!([ + params["approvalPolicy"], + params["sandbox"], + params["permissions"] + ]), + json!([null, null, null]) + ); + } json!({"thread": thread, "model": "gpt-5.6-terra", "modelProvider": "openai", - "cwd": server_cwd, "approvalPolicy": "never", "approvalsReviewer": "user", - "sandbox": {"type": "readOnly"}, "reasoningEffort": null}) + "cwd": server_cwd, "approvalPolicy": if starting { "never" } else { "on-request" }, "approvalsReviewer": "user", + "sandbox": {"type": if starting { "dangerFullAccess" } else { "readOnly" }}, "reasoningEffort": null}) + } + "turn/start" => { + let params = request.params.as_ref().unwrap(); + assert_eq!(params["input"][0]["text"], "preserved-draft!"); + assert_eq!( + json!([ + params["approvalPolicy"], + params["sandboxPolicy"], + params["permissions"] + ]), + json!(["never", {"type": "dangerFullAccess"}, null]) + ); + json!({"turn": {"id": "fresh", "items": [], "status": "inProgress", "error": null}}) } "thread/read" => json!({"thread": thread}), "thread/goal/get" => json!({"goal": null}), @@ -132,6 +174,9 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re .into(), )) .await?; + if request.method == "turn/start" { + submitted_tx.take().unwrap().send(()).unwrap(); + } if connection == 6 && request.method == "thread/resume" { // Keep the recovered turn running: its output must appear without waiting // for turn/completed or rebuilding the transcript. @@ -152,7 +197,8 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re } Ok::<_, anyhow::Error>(methods) }); - let mut terminal = PtyCodex::start(&repo_root, codex_home, &["--no-alt-screen"])?; + let mut terminal = + PtyCodex::start(&workspace_path, codex_home, &["--yolo", "--no-alt-screen"])?; terminal.wait_for_startup()?; let mut disconnect_tx = Some(disconnect_tx); let mut restore_tx = Some(restore_tx); @@ -199,6 +245,10 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re "draft and notification did not remain visible after recovery; screen:\n{}", terminal.screen_contents() ); + complete_tx.send(()).unwrap(); + terminal.wait_for_screen("Recovered turn completed.")?; + terminal.write_input(b"\r")?; + tokio::time::timeout(Duration::from_secs(/*secs*/ 10), submitted_rx).await??; drop(terminal); let methods = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), server).await???; assert_eq!( @@ -208,6 +258,12 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re .count(), 6 ); - assert!(!methods.iter().any(|method| method == "turn/start")); + assert_eq!( + methods + .iter() + .filter(|method| *method == "turn/start") + .count(), + 1 + ); Ok(()) }