diff --git a/.github/actions/setup-bazel-ci/action.yml b/.github/actions/setup-bazel-ci/action.yml index 881209fd81..bb757aab91 100644 --- a/.github/actions/setup-bazel-ci/action.yml +++ b/.github/actions/setup-bazel-ci/action.yml @@ -33,7 +33,7 @@ runs: run: Copy-Item (Get-Command dotslash).Source -Destination "$env:LOCALAPPDATA\Microsoft\WindowsApps\dotslash.exe" - name: Set up Bazel - uses: bazelbuild/setup-bazelisk@b39c379c82683a5f25d34f0d062761f62693e0b2 # v3 + uses: bazel-contrib/setup-bazel@c5acdfb288317d0b5c0bbd7a396a3dc868bb0f86 # 0.19.0 - name: Configure Bazel repository cache id: configure_bazel_repository_cache diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c745106cb..bb5ed5b5bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,11 +45,16 @@ jobs: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail - # Use a rust-release version that includes all native binaries. - CODEX_VERSION=0.115.0 + # Use a recent successful rust-release run that published the full + # cross-platform native payload required by the npm package layout. + # Passing the workflow URL directly avoids relying on old rust-v* + # branches remaining discoverable via `gh run list --branch ...`. + CODEX_VERSION=0.125.0 + WORKFLOW_URL="https://github.com/openai/codex/actions/runs/24901475298" OUTPUT_DIR="${RUNNER_TEMP}" python3 ./scripts/stage_npm_packages.py \ --release-version "$CODEX_VERSION" \ + --workflow-url "$WORKFLOW_URL" \ --package codex \ --output-dir "$OUTPUT_DIR" PACK_OUTPUT="${OUTPUT_DIR}/codex-npm-${CODEX_VERSION}.tgz" diff --git a/.github/workflows/rusty-v8-release.yml b/.github/workflows/rusty-v8-release.yml index 29e7b3b1ae..ee92eff4fa 100644 --- a/.github/workflows/rusty-v8-release.yml +++ b/.github/workflows/rusty-v8-release.yml @@ -78,7 +78,9 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - name: Set up Bazel - uses: bazelbuild/setup-bazelisk@b39c379c82683a5f25d34f0d062761f62693e0b2 # v3 + uses: ./.github/actions/setup-bazel-ci + with: + target: ${{ matrix.target }} - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 diff --git a/.github/workflows/v8-canary.yml b/.github/workflows/v8-canary.yml index f5aa1d7c67..119d042275 100644 --- a/.github/workflows/v8-canary.yml +++ b/.github/workflows/v8-canary.yml @@ -3,6 +3,7 @@ name: v8-canary on: pull_request: paths: + - ".github/actions/setup-bazel-ci/**" - ".github/scripts/rusty_v8_bazel.py" - ".github/workflows/rusty-v8-release.yml" - ".github/workflows/v8-canary.yml" @@ -16,6 +17,7 @@ on: branches: - main paths: + - ".github/actions/setup-bazel-ci/**" - ".github/scripts/rusty_v8_bazel.py" - ".github/workflows/rusty-v8-release.yml" - ".github/workflows/v8-canary.yml" @@ -75,7 +77,9 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - name: Set up Bazel - uses: bazelbuild/setup-bazelisk@b39c379c82683a5f25d34f0d062761f62693e0b2 # v3 + uses: ./.github/actions/setup-bazel-ci + with: + target: ${{ matrix.target }} - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index e02f563a6b..b27c6b3cf9 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -7,6 +7,7 @@ use crate::error_code::INPUT_TOO_LARGE_ERROR_CODE; use crate::error_code::INTERNAL_ERROR_CODE; use crate::error_code::INVALID_PARAMS_ERROR_CODE; use crate::error_code::INVALID_REQUEST_ERROR_CODE; +use crate::error_code::invalid_params; use crate::fuzzy_file_search::FuzzyFileSearchSession; use crate::fuzzy_file_search::run_fuzzy_file_search; use crate::fuzzy_file_search::start_fuzzy_file_search_session; @@ -540,7 +541,6 @@ struct ListenerTaskContext { outgoing: Arc, pending_thread_unloads: Arc>>, analytics_events_client: AnalyticsEventsClient, - general_analytics_enabled: bool, thread_watch_manager: ThreadWatchManager, fallback_model_provider: String, codex_home: PathBuf, @@ -727,14 +727,12 @@ impl CodexMessageProcessor { error: &JSONRPCErrorError, error_type: Option, ) { - if self.config.features.enabled(Feature::GeneralAnalytics) { - self.analytics_events_client.track_error_response( - request_id.connection_id.0, - request_id.request_id.clone(), - error.clone(), - error_type, - ); - } + self.analytics_events_client.track_error_response( + request_id.connection_id.0, + request_id.request_id.clone(), + error.clone(), + error_type, + ); } async fn load_thread( @@ -1365,7 +1363,7 @@ impl CodexMessageProcessor { self.config.cli_auth_credentials_store_mode, ) { Ok(()) => { - self.auth_manager.reload(); + self.auth_manager.reload().await; Ok(()) } Err(err) => Err(JSONRPCErrorError { @@ -1377,31 +1375,16 @@ impl CodexMessageProcessor { } async fn login_api_key_v2(&self, request_id: ConnectionRequestId, params: LoginApiKeyParams) { - match self.login_api_key_common(¶ms).await { - Ok(()) => { - let response = codex_app_server_protocol::LoginAccountResponse::ApiKey {}; - self.outgoing.send_response(request_id, response).await; + let result = self + .login_api_key_common(¶ms) + .await + .map(|()| LoginAccountResponse::ApiKey {}); + let logged_in = result.is_ok(); + self.outgoing.send_result(request_id, result).await; - let payload_login_completed = AccountLoginCompletedNotification { - login_id: None, - success: true, - error: None, - }; - self.outgoing - .send_server_notification(ServerNotification::AccountLoginCompleted( - payload_login_completed, - )) - .await; - - self.outgoing - .send_server_notification(ServerNotification::AccountUpdated( - self.current_account_updated_notification(), - )) - .await; - } - Err(error) => { - self.outgoing.send_error(request_id, error).await; - } + if logged_in { + self.send_login_success_notifications(/*login_id*/ None) + .await; } } @@ -1464,202 +1447,143 @@ impl CodexMessageProcessor { } async fn login_chatgpt_v2(&self, request_id: ConnectionRequestId) { - match self.login_chatgpt_common().await { - Ok(opts) => match run_login_server(opts) { - Ok(server) => { - let login_id = Uuid::new_v4(); - let shutdown_handle = server.cancel_handle(); + let result = self.login_chatgpt_response().await; + self.outgoing.send_result(request_id, result).await; + } - // Replace active login if present. - { - let mut guard = self.active_login.lock().await; - if let Some(existing) = guard.take() { - drop(existing); - } - *guard = Some(ActiveLogin::Browser { - shutdown_handle: shutdown_handle.clone(), - login_id, - }); - } + async fn login_chatgpt_response(&self) -> Result { + let opts = self.login_chatgpt_common().await?; + let server = run_login_server(opts) + .map_err(|err| internal_error(format!("failed to start login server: {err}")))?; + let login_id = Uuid::new_v4(); + let shutdown_handle = server.cancel_handle(); - // Spawn background task to monitor completion. - let outgoing_clone = self.outgoing.clone(); - let active_login = self.active_login.clone(); - let auth_manager = self.auth_manager.clone(); - let config_manager = self.config_manager.clone(); - let chatgpt_base_url = self.config.chatgpt_base_url.clone(); - let auth_url = server.auth_url.clone(); - tokio::spawn(async move { - let (success, error_msg) = match tokio::time::timeout( - LOGIN_CHATGPT_TIMEOUT, - server.block_until_done(), - ) - .await - { - Ok(Ok(())) => (true, None), - Ok(Err(err)) => (false, Some(format!("Login server error: {err}"))), - Err(_elapsed) => { - shutdown_handle.shutdown(); - (false, Some("Login timed out".to_string())) - } - }; - - let payload_v2 = AccountLoginCompletedNotification { - login_id: Some(login_id.to_string()), - success, - error: error_msg, - }; - outgoing_clone - .send_server_notification(ServerNotification::AccountLoginCompleted( - payload_v2, - )) - .await; - - if success { - auth_manager.reload(); - config_manager.replace_cloud_requirements_loader( - auth_manager.clone(), - chatgpt_base_url, - ); - config_manager - .sync_default_client_residency_requirement() - .await; - - // Notify clients with the actual current auth mode. - let auth = auth_manager.auth_cached(); - let payload_v2 = AccountUpdatedNotification { - auth_mode: auth.as_ref().map(CodexAuth::api_auth_mode), - plan_type: auth.as_ref().and_then(CodexAuth::account_plan_type), - }; - outgoing_clone - .send_server_notification(ServerNotification::AccountUpdated( - payload_v2, - )) - .await; - } - - // Clear the active login if it matches this attempt. It may have been replaced or cancelled. - let mut guard = active_login.lock().await; - if guard.as_ref().map(ActiveLogin::login_id) == Some(login_id) { - *guard = None; - } - }); - - let response = codex_app_server_protocol::LoginAccountResponse::Chatgpt { - login_id: login_id.to_string(), - auth_url, - }; - self.outgoing.send_response(request_id, response).await; - } - Err(err) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to start login server: {err}"), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - } - }, - Err(err) => { - self.outgoing.send_error(request_id, err).await; + // Replace active login if present. + { + let mut guard = self.active_login.lock().await; + if let Some(existing) = guard.take() { + drop(existing); } + *guard = Some(ActiveLogin::Browser { + shutdown_handle: shutdown_handle.clone(), + login_id, + }); } + + let outgoing_clone = self.outgoing.clone(); + let active_login = self.active_login.clone(); + let auth_manager = self.auth_manager.clone(); + let config_manager = self.config_manager.clone(); + let chatgpt_base_url = self.config.chatgpt_base_url.clone(); + let auth_url = server.auth_url.clone(); + tokio::spawn(async move { + let (success, error_msg) = match tokio::time::timeout( + LOGIN_CHATGPT_TIMEOUT, + server.block_until_done(), + ) + .await + { + Ok(Ok(())) => (true, None), + Ok(Err(err)) => (false, Some(format!("Login server error: {err}"))), + Err(_elapsed) => { + shutdown_handle.shutdown(); + (false, Some("Login timed out".to_string())) + } + }; + + Self::send_chatgpt_login_completion_notifications( + &outgoing_clone, + auth_manager, + config_manager, + chatgpt_base_url, + login_id, + success, + error_msg, + ) + .await; + + // Clear the active login if it matches this attempt. It may have been replaced or cancelled. + let mut guard = active_login.lock().await; + if guard.as_ref().map(ActiveLogin::login_id) == Some(login_id) { + *guard = None; + } + }); + + Ok(LoginAccountResponse::Chatgpt { + login_id: login_id.to_string(), + auth_url, + }) } async fn login_chatgpt_device_code_v2(&self, request_id: ConnectionRequestId) { - match self.login_chatgpt_common().await { - Ok(opts) => match request_device_code(&opts).await { - Ok(device_code) => { - let login_id = Uuid::new_v4(); - let cancel = CancellationToken::new(); + let result = self.login_chatgpt_device_code_response().await; + self.outgoing.send_result(request_id, result).await; + } - { - let mut guard = self.active_login.lock().await; - if let Some(existing) = guard.take() { - drop(existing); - } - *guard = Some(ActiveLogin::DeviceCode { - cancel: cancel.clone(), - login_id, - }); - } + async fn login_chatgpt_device_code_response( + &self, + ) -> Result { + let opts = self.login_chatgpt_common().await?; + let device_code = request_device_code(&opts) + .await + .map_err(Self::login_chatgpt_device_code_start_error)?; + let login_id = Uuid::new_v4(); + let cancel = CancellationToken::new(); - let verification_url = device_code.verification_url.clone(); - let user_code = device_code.user_code.clone(); - let response = - codex_app_server_protocol::LoginAccountResponse::ChatgptDeviceCode { - login_id: login_id.to_string(), - verification_url, - user_code, - }; - self.outgoing.send_response(request_id, response).await; - - let outgoing_clone = self.outgoing.clone(); - let active_login = self.active_login.clone(); - let auth_manager = self.auth_manager.clone(); - let config_manager = self.config_manager.clone(); - let chatgpt_base_url = self.config.chatgpt_base_url.clone(); - tokio::spawn(async move { - let (success, error_msg) = tokio::select! { - _ = cancel.cancelled() => { - (false, Some("Login was not completed".to_string())) - } - r = complete_device_code_login(opts, device_code) => { - match r { - Ok(()) => (true, None), - Err(err) => (false, Some(err.to_string())), - } - } - }; - - let payload_v2 = AccountLoginCompletedNotification { - login_id: Some(login_id.to_string()), - success, - error: error_msg, - }; - outgoing_clone - .send_server_notification(ServerNotification::AccountLoginCompleted( - payload_v2, - )) - .await; - - if success { - auth_manager.reload(); - config_manager.replace_cloud_requirements_loader( - auth_manager.clone(), - chatgpt_base_url, - ); - config_manager - .sync_default_client_residency_requirement() - .await; - - let auth = auth_manager.auth_cached(); - let payload_v2 = AccountUpdatedNotification { - auth_mode: auth.as_ref().map(CodexAuth::api_auth_mode), - plan_type: auth.as_ref().and_then(CodexAuth::account_plan_type), - }; - outgoing_clone - .send_server_notification(ServerNotification::AccountUpdated( - payload_v2, - )) - .await; - } - - let mut guard = active_login.lock().await; - if guard.as_ref().map(ActiveLogin::login_id) == Some(login_id) { - *guard = None; - } - }); - } - Err(err) => { - let error = Self::login_chatgpt_device_code_start_error(err); - self.outgoing.send_error(request_id, error).await; - } - }, - Err(err) => { - self.outgoing.send_error(request_id, err).await; + { + let mut guard = self.active_login.lock().await; + if let Some(existing) = guard.take() { + drop(existing); } + *guard = Some(ActiveLogin::DeviceCode { + cancel: cancel.clone(), + login_id, + }); } + + let verification_url = device_code.verification_url.clone(); + let user_code = device_code.user_code.clone(); + + let outgoing_clone = self.outgoing.clone(); + let active_login = self.active_login.clone(); + let auth_manager = self.auth_manager.clone(); + let config_manager = self.config_manager.clone(); + let chatgpt_base_url = self.config.chatgpt_base_url.clone(); + tokio::spawn(async move { + let (success, error_msg) = tokio::select! { + _ = cancel.cancelled() => { + (false, Some("Login was not completed".to_string())) + } + r = complete_device_code_login(opts, device_code) => { + match r { + Ok(()) => (true, None), + Err(err) => (false, Some(err.to_string())), + } + } + }; + + Self::send_chatgpt_login_completion_notifications( + &outgoing_clone, + auth_manager, + config_manager, + chatgpt_base_url, + login_id, + success, + error_msg, + ) + .await; + + let mut guard = active_login.lock().await; + if guard.as_ref().map(ActiveLogin::login_id) == Some(login_id) { + *guard = None; + } + }); + + Ok(LoginAccountResponse::ChatgptDeviceCode { + login_id: login_id.to_string(), + verification_url, + user_code, + }) } async fn cancel_login_chatgpt_common( @@ -1682,25 +1606,22 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: CancelLoginAccountParams, ) { + let result = self.cancel_login_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn cancel_login_response( + &self, + params: CancelLoginAccountParams, + ) -> Result { let login_id = params.login_id; - match Uuid::parse_str(&login_id) { - Ok(uuid) => { - let status = match self.cancel_login_chatgpt_common(uuid).await { - Ok(()) => CancelLoginAccountStatus::Canceled, - Err(CancelLoginError::NotFound) => CancelLoginAccountStatus::NotFound, - }; - let response = CancelLoginAccountResponse { status }; - self.outgoing.send_response(request_id, response).await; - } - Err(_) => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("invalid login id: {login_id}"), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - } - } + let uuid = Uuid::parse_str(&login_id) + .map_err(|_| invalid_request(format!("invalid login id: {login_id}")))?; + let status = match self.cancel_login_chatgpt_common(uuid).await { + Ok(()) => CancelLoginAccountStatus::Canceled, + Err(CancelLoginError::NotFound) => CancelLoginAccountStatus::NotFound, + }; + Ok(CancelLoginAccountResponse { status }) } async fn login_chatgpt_auth_tokens( @@ -1710,18 +1631,31 @@ impl CodexMessageProcessor { chatgpt_account_id: String, chatgpt_plan_type: Option, ) { + let result = self + .login_chatgpt_auth_tokens_response(access_token, chatgpt_account_id, chatgpt_plan_type) + .await; + let logged_in = result.is_ok(); + self.outgoing.send_result(request_id, result).await; + + if logged_in { + self.send_login_success_notifications(/*login_id*/ None) + .await; + } + } + + async fn login_chatgpt_auth_tokens_response( + &self, + access_token: String, + chatgpt_account_id: String, + chatgpt_plan_type: Option, + ) -> Result { if matches!( self.config.forced_login_method, Some(ForcedLoginMethod::Api) ) { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "External ChatGPT auth is disabled. Use API key login instead." - .to_string(), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; + return Err(invalid_request( + "External ChatGPT auth is disabled. Use API key login instead.", + )); } // Cancel any active login attempt to avoid persisting managed auth state. @@ -1735,32 +1669,19 @@ impl CodexMessageProcessor { if let Some(expected_workspace) = self.config.forced_chatgpt_workspace_id.as_deref() && chatgpt_account_id != expected_workspace { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!( - "External auth must use workspace {expected_workspace}, but received {chatgpt_account_id:?}." - ), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; + return Err(invalid_request(format!( + "External auth must use workspace {expected_workspace}, but received {chatgpt_account_id:?}." + ))); } - if let Err(err) = login_with_chatgpt_auth_tokens( + login_with_chatgpt_auth_tokens( &self.config.codex_home, &access_token, &chatgpt_account_id, chatgpt_plan_type.as_deref(), - ) { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to set external auth: {err}"), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; - } - self.auth_manager.reload(); + ) + .map_err(|err| internal_error(format!("failed to set external auth: {err}")))?; + self.auth_manager.reload().await; self.config_manager.replace_cloud_requirements_loader( self.auth_manager.clone(), self.config.chatgpt_base_url.clone(), @@ -1769,12 +1690,12 @@ impl CodexMessageProcessor { .sync_default_client_residency_requirement() .await; - self.outgoing - .send_response(request_id, LoginAccountResponse::ChatgptAuthTokens {}) - .await; + Ok(LoginAccountResponse::ChatgptAuthTokens {}) + } + async fn send_login_success_notifications(&self, login_id: Option) { let payload_login_completed = AccountLoginCompletedNotification { - login_id: None, + login_id: login_id.map(|id| id.to_string()), success: true, error: None, }; @@ -1791,6 +1712,43 @@ impl CodexMessageProcessor { .await; } + async fn send_chatgpt_login_completion_notifications( + outgoing: &OutgoingMessageSender, + auth_manager: Arc, + config_manager: ConfigManager, + chatgpt_base_url: String, + login_id: Uuid, + success: bool, + error_msg: Option, + ) { + let payload_v2 = AccountLoginCompletedNotification { + login_id: Some(login_id.to_string()), + success, + error: error_msg, + }; + outgoing + .send_server_notification(ServerNotification::AccountLoginCompleted(payload_v2)) + .await; + + if success { + auth_manager.reload().await; + config_manager + .replace_cloud_requirements_loader(auth_manager.clone(), chatgpt_base_url); + config_manager + .sync_default_client_residency_requirement() + .await; + + let auth = auth_manager.auth_cached(); + let payload_v2 = AccountUpdatedNotification { + auth_mode: auth.as_ref().map(CodexAuth::api_auth_mode), + plan_type: auth.as_ref().and_then(CodexAuth::account_plan_type), + }; + outgoing + .send_server_notification(ServerNotification::AccountUpdated(payload_v2)) + .await; + } + } + async fn logout_common(&self) -> std::result::Result, JSONRPCErrorError> { // Cancel any active login attempt. { @@ -1820,23 +1778,24 @@ impl CodexMessageProcessor { } async fn logout_v2(&self, request_id: ConnectionRequestId) { - match self.logout_common().await { - Ok(current_auth_method) => { - self.outgoing - .send_response(request_id, LogoutAccountResponse {}) - .await; - - let payload_v2 = AccountUpdatedNotification { - auth_mode: current_auth_method, + let result = self.logout_common().await; + let account_updated = + result + .as_ref() + .ok() + .cloned() + .map(|auth_mode| AccountUpdatedNotification { + auth_mode, plan_type: None, - }; - self.outgoing - .send_server_notification(ServerNotification::AccountUpdated(payload_v2)) - .await; - } - Err(error) => { - self.outgoing.send_error(request_id, error).await; - } + }); + self.outgoing + .send_result(request_id, result.map(|_| LogoutAccountResponse {})) + .await; + + if let Some(payload) = account_updated { + self.outgoing + .send_server_notification(ServerNotification::AccountUpdated(payload)) + .await; } } @@ -1919,6 +1878,14 @@ impl CodexMessageProcessor { } async fn get_account(&self, request_id: ConnectionRequestId, params: GetAccountParams) { + let result = self.get_account_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn get_account_response( + &self, + params: GetAccountParams, + ) -> Result { let do_refresh = params.refresh_token; self.refresh_token_if_requested(do_refresh).await; @@ -1930,43 +1897,35 @@ impl CodexMessageProcessor { let account_state = match provider.account_state() { Ok(account_state) => account_state, Err(ProviderAccountError::MissingChatgptAccountDetails) => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "email and plan type are required for chatgpt authentication" - .to_string(), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; + return Err(invalid_request( + "email and plan type are required for chatgpt authentication", + )); } }; let account = account_state.account.map(Account::from); - let response = GetAccountResponse { + Ok(GetAccountResponse { account, requires_openai_auth: account_state.requires_openai_auth, - }; - self.outgoing.send_response(request_id, response).await; + }) } async fn get_account_rate_limits(&self, request_id: ConnectionRequestId) { - match self.fetch_account_rate_limits().await { - Ok((rate_limits, rate_limits_by_limit_id)) => { - let response = GetAccountRateLimitsResponse { - rate_limits: rate_limits.into(), - rate_limits_by_limit_id: Some( - rate_limits_by_limit_id - .into_iter() - .map(|(limit_id, snapshot)| (limit_id, snapshot.into())) - .collect(), - ), - }; - self.outgoing.send_response(request_id, response).await; - } - Err(error) => { - self.outgoing.send_error(request_id, error).await; - } - } + let result = + self.fetch_account_rate_limits() + .await + .map( + |(rate_limits, rate_limits_by_limit_id)| GetAccountRateLimitsResponse { + rate_limits: rate_limits.into(), + rate_limits_by_limit_id: Some( + rate_limits_by_limit_id + .into_iter() + .map(|(limit_id, snapshot)| (limit_id, snapshot.into())) + .collect(), + ), + }, + ); + self.outgoing.send_result(request_id, result).await; } async fn send_add_credits_nudge_email( @@ -1974,16 +1933,11 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: SendAddCreditsNudgeEmailParams, ) { - match self.send_add_credits_nudge_email_inner(params).await { - Ok(status) => { - self.outgoing - .send_response(request_id, SendAddCreditsNudgeEmailResponse { status }) - .await; - } - Err(error) => { - self.outgoing.send_error(request_id, error).await; - } - } + let result = self + .send_add_credits_nudge_email_inner(params) + .await + .map(|status| SendAddCreditsNudgeEmailResponse { status }); + self.outgoing.send_result(request_id, result).await; } async fn send_add_credits_nudge_email_inner( @@ -2111,18 +2065,24 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: CommandExecParams, ) { + let result = self + .exec_one_off_command_inner(request_id.clone(), params) + .await + .map(|()| None::); + self.send_optional_result(request_id, result).await; + } + + async fn exec_one_off_command_inner( + &self, + request_id: ConnectionRequestId, + params: CommandExecParams, + ) -> Result<(), JSONRPCErrorError> { tracing::debug!("ExecOneOffCommand params: {params:?}"); let request = request_id.clone(); if params.command.is_empty() { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "command must not be empty".to_string(), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; + return Err(invalid_request("command must not be empty")); } let CommandExecParams { @@ -2142,43 +2102,25 @@ impl CodexMessageProcessor { permission_profile, } = params; if sandbox_policy.is_some() && permission_profile.is_some() { - self.send_invalid_request_error( - request_id, - "`permissionProfile` cannot be combined with `sandboxPolicy`".to_string(), - ) - .await; - return; + return Err(invalid_request( + "`permissionProfile` cannot be combined with `sandboxPolicy`", + )); } if size.is_some() && !tty { - let error = JSONRPCErrorError { - code: INVALID_PARAMS_ERROR_CODE, - message: "command/exec size requires tty: true".to_string(), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; + return Err(invalid_params("command/exec size requires tty: true")); } if disable_output_cap && output_bytes_cap.is_some() { - let error = JSONRPCErrorError { - code: INVALID_PARAMS_ERROR_CODE, - message: "command/exec cannot set both outputBytesCap and disableOutputCap" - .to_string(), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; + return Err(invalid_params( + "command/exec cannot set both outputBytesCap and disableOutputCap", + )); } if disable_timeout && timeout_ms.is_some() { - let error = JSONRPCErrorError { - code: INVALID_PARAMS_ERROR_CODE, - message: "command/exec cannot set both timeoutMs and disableTimeout".to_string(), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; + return Err(invalid_params( + "command/exec cannot set both timeoutMs and disableTimeout", + )); } let cwd = cwd.map_or_else(|| self.config.cwd.clone(), |cwd| self.config.cwd.join(cwd)); @@ -2202,15 +2144,9 @@ impl CodexMessageProcessor { Some(timeout_ms) => match u64::try_from(timeout_ms) { Ok(timeout_ms) => Some(timeout_ms), Err(_) => { - let error = JSONRPCErrorError { - code: INVALID_PARAMS_ERROR_CODE, - message: format!( - "command/exec timeoutMs must be non-negative, got {timeout_ms}" - ), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; + return Err(invalid_params(format!( + "command/exec timeoutMs must be non-negative, got {timeout_ms}" + ))); } }, None => None, @@ -2230,13 +2166,9 @@ impl CodexMessageProcessor { { Ok(started) => Some(started), Err(err) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to start managed network proxy: {err}"), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; + return Err(internal_error(format!( + "failed to start managed network proxy: {err}" + ))); } }, None => None, @@ -2301,68 +2233,33 @@ impl CodexMessageProcessor { &file_system_sandbox_policy, network_sandbox_policy, ); - match self - .config + self.config .permissions .permission_profile .can_set(&effective_permission_profile) - { - Ok(()) => effective_permission_profile, - Err(err) => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("invalid permission profile: {err}"), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; - } - } + .map_err(|err| invalid_request(format!("invalid permission profile: {err}")))?; + effective_permission_profile } else if let Some(policy) = sandbox_policy.map(|policy| policy.to_core()) { - match self - .config + self.config .permissions .can_set_legacy_sandbox_policy(&policy, &sandbox_cwd) - { - Ok(()) => { - let file_system_sandbox_policy = - codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, &sandbox_cwd); - let network_sandbox_policy = - codex_protocol::permissions::NetworkSandboxPolicy::from(&policy); - let permission_profile = - codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement( - codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy( - &policy, - ), - &file_system_sandbox_policy, - network_sandbox_policy, - ); - if let Err(err) = self - .config - .permissions - .permission_profile - .can_set(&permission_profile) - { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("invalid sandbox policy: {err}"), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; - } - permission_profile - } - Err(err) => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("invalid sandbox policy: {err}"), - data: None, - }; - self.outgoing.send_error(request, error).await; - return; - } - } + .map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?; + let file_system_sandbox_policy = + codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, &sandbox_cwd); + let network_sandbox_policy = + codex_protocol::permissions::NetworkSandboxPolicy::from(&policy); + let permission_profile = + codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement( + codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy(&policy), + &file_system_sandbox_policy, + network_sandbox_policy, + ); + self.config + .permissions + .permission_profile + .can_set(&permission_profile) + .map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?; + permission_profile } else { self.config.permissions.permission_profile() }; @@ -2374,49 +2271,32 @@ impl CodexMessageProcessor { let use_legacy_landlock = self.config.features.use_legacy_landlock(); let size = match size.map(crate::command_exec::terminal_size_from_protocol) { Some(Ok(size)) => Some(size), - Some(Err(error)) => { - self.outgoing.send_error(request, error).await; - return; - } + Some(Err(error)) => return Err(error), None => None, }; - match codex_core::exec::build_exec_request( + let exec_request = codex_core::exec::build_exec_request( exec_params, &effective_permission_profile, &sandbox_cwd, &codex_linux_sandbox_exe, use_legacy_landlock, - ) { - Ok(exec_request) => { - if let Err(error) = self - .command_exec_manager - .start(StartCommandExecParams { - outgoing, - request_id: request_for_task, - process_id, - exec_request, - started_network_proxy: started_network_proxy_for_task, - tty, - stream_stdin, - stream_stdout_stderr, - output_bytes_cap, - size, - }) - .await - { - self.outgoing.send_error(request, error).await; - } - } - Err(err) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("exec failed: {err}"), - data: None, - }; - self.outgoing.send_error(request, error).await; - } - } + ) + .map_err(|err| internal_error(format!("exec failed: {err}")))?; + self.command_exec_manager + .start(StartCommandExecParams { + outgoing, + request_id: request_for_task, + process_id, + exec_request, + started_network_proxy: started_network_proxy_for_task, + tty, + stream_stdin, + stream_stdout_stderr, + output_bytes_cap, + size, + }) + .await } fn preserve_configured_deny_read_restrictions( @@ -2432,14 +2312,11 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: CommandExecWriteParams, ) { - match self + let result = self .command_exec_manager .write(request_id.clone(), params) - .await - { - Ok(response) => self.outgoing.send_response(request_id, response).await, - Err(error) => self.outgoing.send_error(request_id, error).await, - } + .await; + self.outgoing.send_result(request_id, result).await; } async fn command_exec_resize( @@ -2447,14 +2324,11 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: CommandExecResizeParams, ) { - match self + let result = self .command_exec_manager .resize(request_id.clone(), params) - .await - { - Ok(response) => self.outgoing.send_response(request_id, response).await, - Err(error) => self.outgoing.send_error(request_id, error).await, - } + .await; + self.outgoing.send_result(request_id, result).await; } async fn command_exec_terminate( @@ -2462,14 +2336,11 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: CommandExecTerminateParams, ) { - match self + let result = self .command_exec_manager .terminate(request_id.clone(), params) - .await - { - Ok(response) => self.outgoing.send_response(request_id, response).await, - Err(error) => self.outgoing.send_error(request_id, error).await, - } + .await; + self.outgoing.send_result(request_id, result).await; } async fn thread_start( @@ -2548,7 +2419,6 @@ impl CodexMessageProcessor { outgoing: Arc::clone(&self.outgoing), pending_thread_unloads: Arc::clone(&self.pending_thread_unloads), analytics_events_client: self.analytics_events_client.clone(), - general_analytics_enabled: self.config.features.enabled(Feature::GeneralAnalytics), thread_watch_manager: self.thread_watch_manager.clone(), fallback_model_provider: self.config.model_provider_id.clone(), codex_home: self.config.codex_home.to_path_buf(), @@ -2887,17 +2757,15 @@ impl CodexMessageProcessor { permission_profile, reasoning_effort: config_snapshot.reasoning_effort, }; - if listener_task_context.general_analytics_enabled { - listener_task_context - .analytics_events_client - .track_response( - request_id.connection_id.0, - ClientResponse::ThreadStart { - request_id: request_id.request_id.clone(), - response: response.clone(), - }, - ); - } + listener_task_context + .analytics_events_client + .track_response( + request_id.connection_id.0, + ClientResponse::ThreadStart { + request_id: request_id.request_id.clone(), + response: response.clone(), + }, + ); listener_task_context .outgoing @@ -4703,15 +4571,13 @@ impl CodexMessageProcessor { permission_profile, reasoning_effort: session_configured.reasoning_effort, }; - if self.config.features.enabled(Feature::GeneralAnalytics) { - self.analytics_events_client.track_response( - request_id.connection_id.0, - ClientResponse::ThreadResume { - request_id: request_id.request_id.clone(), - response: response.clone(), - }, - ); - } + self.analytics_events_client.track_response( + request_id.connection_id.0, + ClientResponse::ThreadResume { + request_id: request_id.request_id.clone(), + response: response.clone(), + }, + ); let connection_id = request_id.connection_id; let token_usage_thread = include_turns.then(|| response.thread.clone()); @@ -5434,15 +5300,13 @@ impl CodexMessageProcessor { permission_profile, reasoning_effort: session_configured.reasoning_effort, }; - if self.config.features.enabled(Feature::GeneralAnalytics) { - self.analytics_events_client.track_response( - request_id.connection_id.0, - ClientResponse::ThreadFork { - request_id: request_id.request_id.clone(), - response: response.clone(), - }, - ); - } + self.analytics_events_client.track_response( + request_id.connection_id.0, + ClientResponse::ThreadFork { + request_id: request_id.request_id.clone(), + response: response.clone(), + }, + ); let connection_id = request_id.connection_id; let token_usage_thread = include_turns.then(|| response.thread.clone()); @@ -6326,6 +6190,22 @@ impl CodexMessageProcessor { }); } + async fn send_optional_result( + &self, + request_id: ConnectionRequestId, + result: Result, JSONRPCErrorError>, + ) where + T: serde::Serialize, + { + match result { + Ok(Some(response)) => self.outgoing.send_response(request_id, response).await, + Ok(None) => {} + Err(error) => { + self.outgoing.send_error(request_id, error).await; + } + } + } + async fn send_invalid_request_error(&self, request_id: ConnectionRequestId, message: String) { let error = JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, @@ -6335,6 +6215,15 @@ impl CodexMessageProcessor { self.outgoing.send_error(request_id, error).await; } + async fn send_internal_error(&self, request_id: ConnectionRequestId, message: String) { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message, + data: None, + }; + self.outgoing.send_error(request_id, error).await; + } + fn input_too_large_error(actual_chars: usize) -> JSONRPCErrorError { JSONRPCErrorError { code: INVALID_PARAMS_ERROR_CODE, @@ -6357,41 +6246,6 @@ impl CodexMessageProcessor { Ok(()) } - async fn send_internal_error(&self, request_id: ConnectionRequestId, message: String) { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message, - data: None, - }; - self.outgoing.send_error(request_id, error).await; - } - - async fn send_marketplace_error( - &self, - request_id: ConnectionRequestId, - err: MarketplaceError, - action: &str, - ) { - match err { - MarketplaceError::MarketplaceNotFound { .. } => { - self.send_invalid_request_error(request_id, err.to_string()) - .await; - } - MarketplaceError::Io { .. } => { - self.send_internal_error(request_id, format!("failed to {action}: {err}")) - .await; - } - MarketplaceError::InvalidMarketplaceFile { .. } - | MarketplaceError::PluginNotFound { .. } - | MarketplaceError::PluginNotAvailable { .. } - | MarketplaceError::PluginsDisabled - | MarketplaceError::InvalidPlugin(_) => { - self.send_invalid_request_error(request_id, err.to_string()) - .await; - } - } - } - async fn wait_for_thread_shutdown(thread: &Arc) -> ThreadShutdownResult { match tokio::time::timeout(Duration::from_secs(10), thread.shutdown_and_wait()).await { Ok(Ok(())) => ThreadShutdownResult::Complete, @@ -6470,34 +6324,33 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: ThreadUnsubscribeParams, ) { - let thread_id = match ThreadId::from_string(¶ms.thread_id) { - Ok(id) => id, - Err(err) => { - self.send_invalid_request_error(request_id, format!("invalid thread id: {err}")) - .await; - return; - } - }; + let result = self + .thread_unsubscribe_response(params, request_id.connection_id) + .await; + self.outgoing.send_result(request_id, result).await; + } + + async fn thread_unsubscribe_response( + &self, + params: ThreadUnsubscribeParams, + connection_id: ConnectionId, + ) -> Result { + let thread_id = ThreadId::from_string(¶ms.thread_id) + .map_err(|err| invalid_request(format!("invalid thread id: {err}")))?; if self.thread_manager.get_thread(thread_id).await.is_err() { // Reconcile stale app-server bookkeeping when the thread has already been // removed from the core manager. This keeps loaded-status/subscription state // consistent with the source of truth before reporting NotLoaded. self.finalize_thread_teardown(thread_id).await; - self.outgoing - .send_response( - request_id, - ThreadUnsubscribeResponse { - status: ThreadUnsubscribeStatus::NotLoaded, - }, - ) - .await; - return; + return Ok(ThreadUnsubscribeResponse { + status: ThreadUnsubscribeStatus::NotLoaded, + }); }; let was_subscribed = self .thread_state_manager - .unsubscribe_connection_from_thread(thread_id, request_id.connection_id) + .unsubscribe_connection_from_thread(thread_id, connection_id) .await; let status = if was_subscribed { @@ -6505,9 +6358,7 @@ impl CodexMessageProcessor { } else { ThreadUnsubscribeStatus::NotSubscribed }; - self.outgoing - .send_response(request_id, ThreadUnsubscribeResponse { status }) - .await; + Ok(ThreadUnsubscribeResponse { status }) } async fn prepare_thread_for_archive(&self, thread_id: ThreadId) { @@ -6601,6 +6452,16 @@ impl CodexMessageProcessor { config: Config, environment_manager: Arc, ) { + let result = Self::apps_list_response(&outgoing, params, config, environment_manager).await; + outgoing.send_result(request_id, result).await; + } + + async fn apps_list_response( + outgoing: &Arc, + params: AppsListParams, + config: Config, + environment_manager: Arc, + ) -> Result { let AppsListParams { cursor, limit, @@ -6610,15 +6471,7 @@ impl CodexMessageProcessor { let start = match cursor { Some(cursor) => match cursor.parse::() { Ok(idx) => idx, - Err(_) => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("invalid cursor: {cursor}"), - data: None, - }; - outgoing.send_error(request_id, error).await; - return; - } + Err(_) => return Err(invalid_request(format!("invalid cursor: {cursor}"))), }, None => 0, }; @@ -6672,7 +6525,7 @@ impl CodexMessageProcessor { accessible_loaded, all_loaded, ) { - apps_list_helpers::send_app_list_updated_notification(&outgoing, merged.clone()) + apps_list_helpers::send_app_list_updated_notification(outgoing, merged.clone()) .await; last_notified_apps = Some(merged); } @@ -6682,25 +6535,13 @@ impl CodexMessageProcessor { let result = match tokio::time::timeout_at(app_list_deadline, rx.recv()).await { Ok(Some(result)) => result, Ok(None) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: "failed to load app lists".to_string(), - data: None, - }; - outgoing.send_error(request_id, error).await; - return; + return Err(internal_error("failed to load app lists")); } Err(_) => { let timeout_seconds = APP_LIST_LOAD_TIMEOUT.as_secs(); - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!( - "timed out waiting for app lists after {timeout_seconds} seconds" - ), - data: None, - }; - outgoing.send_error(request_id, error).await; - return; + return Err(internal_error(format!( + "timed out waiting for app lists after {timeout_seconds} seconds" + ))); } }; @@ -6710,26 +6551,14 @@ impl CodexMessageProcessor { accessible_loaded = true; } AppListLoadResult::Accessible(Err(err)) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: err, - data: None, - }; - outgoing.send_error(request_id, error).await; - return; + return Err(internal_error(err)); } AppListLoadResult::Directory(Ok(connectors)) => { all_connectors = Some(connectors); all_loaded = true; } AppListLoadResult::Directory(Err(err)) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: err, - data: None, - }; - outgoing.send_error(request_id, error).await; - return; + return Err(internal_error(err)); } } @@ -6759,27 +6588,26 @@ impl CodexMessageProcessor { all_loaded, ) && last_notified_apps.as_ref() != Some(&merged) { - apps_list_helpers::send_app_list_updated_notification(&outgoing, merged.clone()) + apps_list_helpers::send_app_list_updated_notification(outgoing, merged.clone()) .await; last_notified_apps = Some(merged.clone()); } if accessible_loaded && all_loaded { - match apps_list_helpers::paginate_apps(merged.as_slice(), start, limit) { - Ok(response) => { - outgoing.send_response(request_id, response).await; - return; - } - Err(error) => { - outgoing.send_error(request_id, error).await; - return; - } - } + return apps_list_helpers::paginate_apps(merged.as_slice(), start, limit); } } } async fn skills_list(&self, request_id: ConnectionRequestId, params: SkillsListParams) { + let result = self.skills_list_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn skills_list_response( + &self, + params: SkillsListParams, + ) -> Result { let SkillsListParams { cwds, force_reload, @@ -6804,17 +6632,13 @@ impl CodexMessageProcessor { let mut valid_extra_roots = Vec::new(); for root in entry.extra_user_roots { - let Ok(root) = AbsolutePathBuf::from_absolute_path_checked(root.as_path()) else { - self.send_invalid_request_error( - request_id, - format!( + let root = + AbsolutePathBuf::from_absolute_path_checked(root.as_path()).map_err(|_| { + invalid_request(format!( "skills/list perCwdExtraUserRoots extraUserRoots paths must be absolute: {}", root.display() - ), - ) - .await; - return; - }; + )) + })?; valid_extra_roots.push(root); } extra_roots_by_cwd @@ -6823,13 +6647,7 @@ impl CodexMessageProcessor { .extend(valid_extra_roots); } - let config = match self.load_latest_config(/*fallback_cwd*/ None).await { - Ok(config) => config, - Err(error) => { - self.outgoing.send_error(request_id, error).await; - return; - } - }; + let config = self.load_latest_config(/*fallback_cwd*/ None).await?; let auth = self.auth_manager.auth().await; let workspace_codex_plugins_enabled = self .workspace_codex_plugins_enabled(&config, auth.as_ref()) @@ -6908,9 +6726,7 @@ impl CodexMessageProcessor { errors, }); } - self.outgoing - .send_response(request_id, SkillsListResponse { data }) - .await; + Ok(SkillsListResponse { data }) } async fn marketplace_remove( &self, @@ -6923,27 +6739,16 @@ impl CodexMessageProcessor { marketplace_name: params.marketplace_name, }, ) - .await; - - match result { - Ok(outcome) => { - self.outgoing - .send_response( - request_id, - MarketplaceRemoveResponse { - marketplace_name: outcome.marketplace_name, - installed_root: outcome.removed_installed_root, - }, - ) - .await; - } - Err(MarketplaceRemoveError::InvalidRequest(message)) => { - self.send_invalid_request_error(request_id, message).await; - } - Err(MarketplaceRemoveError::Internal(message)) => { - self.send_internal_error(request_id, message).await; - } - } + .await + .map(|outcome| MarketplaceRemoveResponse { + marketplace_name: outcome.marketplace_name, + installed_root: outcome.removed_installed_root, + }) + .map_err(|err| match err { + MarketplaceRemoveError::InvalidRequest(message) => invalid_request(message), + MarketplaceRemoveError::Internal(message) => internal_error(message), + }); + self.outgoing.send_result(request_id, result).await; } async fn marketplace_upgrade( @@ -6951,53 +6756,38 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: MarketplaceUpgradeParams, ) { - let config = match self.load_latest_config(/*fallback_cwd*/ None).await { - Ok(config) => config, - Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; - } - }; + let result = self.marketplace_upgrade_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn marketplace_upgrade_response( + &self, + params: MarketplaceUpgradeParams, + ) -> Result { + let config = self.load_latest_config(/*fallback_cwd*/ None).await?; let plugins_manager = self.thread_manager.plugins_manager(); let MarketplaceUpgradeParams { marketplace_name } = params; - let result = tokio::task::spawn_blocking(move || { + let outcome = tokio::task::spawn_blocking(move || { plugins_manager .upgrade_configured_marketplaces_for_config(&config, marketplace_name.as_deref()) }) - .await; + .await + .map_err(|err| internal_error(format!("failed to upgrade marketplaces: {err}")))? + .map_err(invalid_request)?; - match result { - Ok(Ok(outcome)) => { - self.outgoing - .send_response( - request_id, - MarketplaceUpgradeResponse { - selected_marketplaces: outcome.selected_marketplaces, - upgraded_roots: outcome.upgraded_roots, - errors: outcome - .errors - .into_iter() - .map(|err| MarketplaceUpgradeErrorInfo { - marketplace_name: err.marketplace_name, - message: err.message, - }) - .collect(), - }, - ) - .await; - } - Ok(Err(message)) => { - self.send_invalid_request_error(request_id, message).await; - } - Err(err) => { - self.send_internal_error( - request_id, - format!("failed to upgrade marketplaces: {err}"), - ) - .await; - } - } + Ok(MarketplaceUpgradeResponse { + selected_marketplaces: outcome.selected_marketplaces, + upgraded_roots: outcome.upgraded_roots, + errors: outcome + .errors + .into_iter() + .map(|err| MarketplaceUpgradeErrorInfo { + marketplace_name: err.marketplace_name, + message: err.message, + }) + .collect(), + }) } async fn marketplace_add(&self, request_id: ConnectionRequestId, params: MarketplaceAddParams) { @@ -7009,28 +6799,17 @@ impl CodexMessageProcessor { sparse_paths: params.sparse_paths.unwrap_or_default(), }, ) - .await; - - match result { - Ok(outcome) => { - self.outgoing - .send_response( - request_id, - MarketplaceAddResponse { - marketplace_name: outcome.marketplace_name, - installed_root: outcome.installed_root, - already_added: outcome.already_added, - }, - ) - .await; - } - Err(MarketplaceAddError::InvalidRequest(message)) => { - self.send_invalid_request_error(request_id, message).await; - } - Err(MarketplaceAddError::Internal(message)) => { - self.send_internal_error(request_id, message).await; - } - } + .await + .map(|outcome| MarketplaceAddResponse { + marketplace_name: outcome.marketplace_name, + installed_root: outcome.installed_root, + already_added: outcome.already_added, + }) + .map_err(|err| match err { + MarketplaceAddError::InvalidRequest(message) => invalid_request(message), + MarketplaceAddError::Internal(message) => internal_error(message), + }); + self.outgoing.send_result(request_id, result).await; } async fn skills_config_write( @@ -7038,6 +6817,14 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: SkillsConfigWriteParams, ) { + let result = self.skills_config_write_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn skills_config_write_response( + &self, + params: SkillsConfigWriteParams, + ) -> Result { let SkillsConfigWriteParams { path, name, @@ -7052,43 +6839,24 @@ impl CodexMessageProcessor { ConfigEdit::SetSkillConfigByName { name, enabled } } _ => { - let error = JSONRPCErrorError { - code: INVALID_PARAMS_ERROR_CODE, - message: "skills/config/write requires exactly one of path or name".to_string(), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; + return Err(invalid_params( + "skills/config/write requires exactly one of path or name", + )); } }; let edits = vec![edit]; - let result = ConfigEditsBuilder::new(&self.config.codex_home) + ConfigEditsBuilder::new(&self.config.codex_home) .with_edits(edits) .apply() - .await; - - match result { - Ok(()) => { + .await + .map(|()| { self.thread_manager.plugins_manager().clear_cache(); self.thread_manager.skills_manager().clear_cache(); - self.outgoing - .send_response( - request_id, - SkillsConfigWriteResponse { - effective_enabled: enabled, - }, - ) - .await; - } - Err(err) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to update skill settings: {err}"), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - } - } + SkillsConfigWriteResponse { + effective_enabled: enabled, + } + }) + .map_err(|err| internal_error(format!("failed to update skill settings: {err}"))) } async fn turn_start( @@ -7272,15 +7040,13 @@ impl CodexMessageProcessor { }; let response = TurnStartResponse { turn }; - if self.config.features.enabled(Feature::GeneralAnalytics) { - self.analytics_events_client.track_response( - request_id.connection_id.0, - ClientResponse::TurnStart { - request_id: request_id.request_id.clone(), - response: response.clone(), - }, - ); - } + self.analytics_events_client.track_response( + request_id.connection_id.0, + ClientResponse::TurnStart { + request_id: request_id.request_id.clone(), + response: response.clone(), + }, + ); self.outgoing.send_response(request_id, response).await; } Err(err) => { @@ -7406,15 +7172,13 @@ impl CodexMessageProcessor { { Ok(turn_id) => { let response = TurnSteerResponse { turn_id }; - if self.config.features.enabled(Feature::GeneralAnalytics) { - self.analytics_events_client.track_response( - request_id.connection_id.0, - ClientResponse::TurnSteer { - request_id: request_id.request_id.clone(), - response: response.clone(), - }, - ); - } + self.analytics_events_client.track_response( + request_id.connection_id.0, + ClientResponse::TurnSteer { + request_id: request_id.request_id.clone(), + response: response.clone(), + }, + ); self.outgoing.send_response(request_id, response).await; } Err(err) => { @@ -8047,7 +7811,6 @@ impl CodexMessageProcessor { outgoing: Arc::clone(&self.outgoing), pending_thread_unloads: Arc::clone(&self.pending_thread_unloads), analytics_events_client: self.analytics_events_client.clone(), - general_analytics_enabled: self.config.features.enabled(Feature::GeneralAnalytics), thread_watch_manager: self.thread_watch_manager.clone(), fallback_model_provider: self.config.model_provider_id.clone(), codex_home: self.config.codex_home.to_path_buf(), @@ -8165,7 +7928,6 @@ impl CodexMessageProcessor { outgoing: Arc::clone(&self.outgoing), pending_thread_unloads: Arc::clone(&self.pending_thread_unloads), analytics_events_client: self.analytics_events_client.clone(), - general_analytics_enabled: self.config.features.enabled(Feature::GeneralAnalytics), thread_watch_manager: self.thread_watch_manager.clone(), fallback_model_provider: self.config.model_provider_id.clone(), codex_home: self.config.codex_home.to_path_buf(), @@ -8214,7 +7976,6 @@ impl CodexMessageProcessor { thread_state_manager, pending_thread_unloads, analytics_events_client: _, - general_analytics_enabled: _, thread_watch_manager, fallback_model_provider, codex_home, @@ -8290,9 +8051,7 @@ impl CodexMessageProcessor { conversation_id, conversation.clone(), thread_manager.clone(), - listener_task_context - .general_analytics_enabled - .then(|| listener_task_context.analytics_events_client.clone()), + Some(listener_task_context.analytics_events_client.clone()), thread_outgoing, thread_state.clone(), thread_watch_manager.clone(), diff --git a/codex-rs/app-server/src/codex_message_processor/plugins.rs b/codex-rs/app-server/src/codex_message_processor/plugins.rs index 8f0f4dea9a..e1d0fffad3 100644 --- a/codex-rs/app-server/src/codex_message_processor/plugins.rs +++ b/codex-rs/app-server/src/codex_message_processor/plugins.rs @@ -1,4 +1,6 @@ use super::*; +use crate::error_code::internal_error; +use crate::error_code::invalid_request; use codex_app_server_protocol::PluginInstallPolicy; impl CodexMessageProcessor { @@ -7,46 +9,33 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: PluginListParams, ) { + let result = self.plugin_list_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn plugin_list_response( + &self, + params: PluginListParams, + ) -> Result { let plugins_manager = self.thread_manager.plugins_manager(); let PluginListParams { cwds } = params; let roots = cwds.unwrap_or_default(); - let config = match self.load_latest_config(/*fallback_cwd*/ None).await { - Ok(config) => config, - Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; - } + let config = self.load_latest_config(/*fallback_cwd*/ None).await?; + let empty_response = || PluginListResponse { + marketplaces: Vec::new(), + marketplace_load_errors: Vec::new(), + featured_plugin_ids: Vec::new(), }; if !config.features.enabled(Feature::Plugins) { - self.outgoing - .send_response( - request_id, - PluginListResponse { - marketplaces: Vec::new(), - marketplace_load_errors: Vec::new(), - featured_plugin_ids: Vec::new(), - }, - ) - .await; - return; + return Ok(empty_response()); } let auth = self.auth_manager.auth().await; if !self .workspace_codex_plugins_enabled(&config, auth.as_ref()) .await { - self.outgoing - .send_response( - request_id, - PluginListResponse { - marketplaces: Vec::new(), - marketplace_load_errors: Vec::new(), - featured_plugin_ids: Vec::new(), - }, - ) - .await; - return; + return Ok(empty_response()); } plugins_manager.maybe_start_non_curated_plugin_cache_refresh(&roots); @@ -100,18 +89,11 @@ impl CodexMessageProcessor { .await { Ok(Ok(outcome)) => outcome, - Ok(Err(err)) => { - self.send_marketplace_error(request_id, err, "list marketplace plugins") - .await; - return; - } + Ok(Err(err)) => return Err(Self::marketplace_error(err, "list marketplace plugins")), Err(err) => { - self.send_internal_error( - request_id, - format!("failed to list marketplace plugins: {err}"), - ) - .await; - return; + return Err(internal_error(format!( + "failed to list marketplace plugins: {err}" + ))); } }; @@ -174,16 +156,11 @@ impl CodexMessageProcessor { Vec::new() }; - self.outgoing - .send_response( - request_id, - PluginListResponse { - marketplaces: data, - marketplace_load_errors, - featured_plugin_ids, - }, - ) - .await; + Ok(PluginListResponse { + marketplaces: data, + marketplace_load_errors, + featured_plugin_ids, + }) } pub(super) async fn plugin_read( @@ -191,6 +168,14 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: PluginReadParams, ) { + let result = self.plugin_read_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn plugin_read_response( + &self, + params: PluginReadParams, + ) -> Result { let plugins_manager = self.thread_manager.plugins_manager(); let PluginReadParams { marketplace_path, @@ -201,30 +186,16 @@ impl CodexMessageProcessor { (Some(marketplace_path), None) => Ok(marketplace_path), (None, Some(remote_marketplace_name)) => Err(remote_marketplace_name), (Some(_), Some(_)) | (None, None) => { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "plugin/read requires exactly one of marketplacePath or remoteMarketplaceName".to_string(), - data: None, - }, - ) - .await; - return; + return Err(invalid_request( + "plugin/read requires exactly one of marketplacePath or remoteMarketplaceName", + )); } }; let config_cwd = read_source.as_ref().ok().and_then(|marketplace_path| { marketplace_path.as_path().parent().map(Path::to_path_buf) }); - let config = match self.load_latest_config(config_cwd).await { - Ok(config) => config, - Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; - } - }; + let config = self.load_latest_config(config_cwd).await?; let plugin = match read_source { Ok(marketplace_path) => { @@ -232,17 +203,10 @@ impl CodexMessageProcessor { plugin_name, marketplace_path, }; - let outcome = match plugins_manager + let outcome = plugins_manager .read_plugin_for_config(&config, &request) .await - { - Ok(outcome) => outcome, - Err(err) => { - self.send_marketplace_error(request_id, err, "read plugin details") - .await; - return; - } - }; + .map_err(|err| Self::marketplace_error(err, "read plugin details"))?; let environment_manager = self.thread_manager.environment_manager(); let app_summaries = plugin_app_helpers::load_plugin_app_summaries( &config, @@ -287,19 +251,9 @@ impl CodexMessageProcessor { if !config.features.enabled(Feature::Plugins) || !config.features.enabled(Feature::RemotePlugin) { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!( - "remote plugin read is not enabled for marketplace {remote_marketplace_name}" - ), - data: None, - }, - ) - .await; - return; + return Err(invalid_request(format!( + "remote plugin read is not enabled for marketplace {remote_marketplace_name}" + ))); } let auth = self.auth_manager.auth().await; let remote_plugin_service_config = RemotePluginServiceConfig { @@ -310,36 +264,20 @@ impl CodexMessageProcessor { .chars() .all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' || ch == '~') { - self.send_invalid_request_error( - request_id, - "invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed" - .to_string(), - ) - .await; - return; + return Err(invalid_request( + "invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed", + )); } - let remote_detail = match codex_core_plugins::remote::fetch_remote_plugin_detail( + let remote_detail = codex_core_plugins::remote::fetch_remote_plugin_detail( &remote_plugin_service_config, auth.as_ref(), &remote_marketplace_name, &plugin_name, ) .await - { - Ok(remote_detail) => remote_detail, - Err(err) => { - self.outgoing - .send_error( - request_id, - remote_plugin_catalog_error_to_jsonrpc( - err, - "read remote plugin details", - ), - ) - .await; - return; - } - }; + .map_err(|err| { + remote_plugin_catalog_error_to_jsonrpc(err, "read remote plugin details") + })?; let plugin_apps = remote_detail .app_ids .iter() @@ -357,9 +295,7 @@ impl CodexMessageProcessor { } }; - self.outgoing - .send_response(request_id, PluginReadResponse { plugin }) - .await; + Ok(PluginReadResponse { plugin }) } pub(super) async fn plugin_install( @@ -367,6 +303,14 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: PluginInstallParams, ) { + let result = self.plugin_install_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn plugin_install_response( + &self, + params: PluginInstallParams, + ) -> Result { let PluginInstallParams { marketplace_path, remote_marketplace_name, @@ -375,44 +319,27 @@ impl CodexMessageProcessor { let marketplace_path = match (marketplace_path, remote_marketplace_name) { (Some(marketplace_path), None) => marketplace_path, (None, Some(remote_marketplace_name)) => { - self.remote_plugin_install(request_id, remote_marketplace_name, plugin_name) + return self + .remote_plugin_install_response(remote_marketplace_name, plugin_name) .await; - return; } (Some(_), Some(_)) | (None, None) => { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "plugin/install requires exactly one of marketplacePath or remoteMarketplaceName".to_string(), - data: None, - }, - ) - .await; - return; + return Err(invalid_request( + "plugin/install requires exactly one of marketplacePath or remoteMarketplaceName", + )); } }; let config_cwd = marketplace_path.as_path().parent().map(Path::to_path_buf); - let config = match self.load_latest_config(config_cwd.clone()).await { - Ok(config) => config, - Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; - } - }; + let config = self.load_latest_config(config_cwd.clone()).await?; let auth = self.auth_manager.auth().await; if !self .workspace_codex_plugins_enabled(&config, auth.as_ref()) .await { - self.send_invalid_request_error( - request_id, - "Codex plugins are disabled for this workspace".to_string(), - ) - .await; - return; + return Err(invalid_request( + "Codex plugins are disabled for this workspace", + )); } let plugins_manager = self.thread_manager.plugins_manager(); @@ -421,197 +348,103 @@ impl CodexMessageProcessor { marketplace_path, }; - let install_result = plugins_manager.install_plugin(request).await; - - match install_result { - Ok(result) => { - let config = match self.load_latest_config(config_cwd).await { - Ok(config) => config, - Err(err) => { - warn!( - "failed to reload config after plugin install, using current config: {err:?}" - ); - config - } - }; - - self.clear_plugin_related_caches(); - - let plugin_mcp_servers = - load_plugin_mcp_servers(result.installed_path.as_path()).await; - - if !plugin_mcp_servers.is_empty() { - if let Err(err) = self.queue_mcp_server_refresh_for_config(&config).await { - warn!( - plugin = result.plugin_id.as_key(), - "failed to queue MCP refresh after plugin install: {err:?}" - ); - } - self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) - .await; - } - - let plugin_apps = load_plugin_apps(result.installed_path.as_path()).await; - let auth = self.auth_manager.auth().await; - let apps_needing_auth = self - .plugin_apps_needing_auth_for_install( - &config, - auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth), - &result.plugin_id.as_key(), - &plugin_apps, - ) - .await; - - self.outgoing - .send_response( - request_id, - PluginInstallResponse { - auth_policy: result.auth_policy.into(), - apps_needing_auth, - }, - ) - .await; - } - Err(err) => { - if err.is_invalid_request() { - self.send_invalid_request_error(request_id, err.to_string()) - .await; - return; - } - - match err { - CorePluginInstallError::Marketplace(err) => { - self.send_marketplace_error(request_id, err, "install plugin") - .await; - } - CorePluginInstallError::Config(err) => { - self.send_internal_error( - request_id, - format!("failed to persist installed plugin config: {err}"), - ) - .await; - } - CorePluginInstallError::Remote(err) => { - self.send_internal_error( - request_id, - format!("failed to enable remote plugin: {err}"), - ) - .await; - } - CorePluginInstallError::Join(err) => { - self.send_internal_error( - request_id, - format!("failed to install plugin: {err}"), - ) - .await; - } - CorePluginInstallError::Store(err) => { - self.send_internal_error( - request_id, - format!("failed to install plugin: {err}"), - ) - .await; - } - } - } - } - } - - async fn remote_plugin_install( - &self, - request_id: ConnectionRequestId, - remote_marketplace_name: String, - plugin_name: String, - ) { - let config = match self.load_latest_config(/*fallback_cwd*/ None).await { + let result = plugins_manager + .install_plugin(request) + .await + .map_err(Self::plugin_install_error)?; + let config = match self.load_latest_config(config_cwd).await { Ok(config) => config, Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; + warn!( + "failed to reload config after plugin install, using current config: {err:?}" + ); + config } }; + + self.clear_plugin_related_caches(); + + let plugin_mcp_servers = load_plugin_mcp_servers(result.installed_path.as_path()).await; + + if !plugin_mcp_servers.is_empty() { + if let Err(err) = self.queue_mcp_server_refresh_for_config(&config).await { + warn!( + plugin = result.plugin_id.as_key(), + "failed to queue MCP refresh after plugin install: {err:?}" + ); + } + self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) + .await; + } + + let plugin_apps = load_plugin_apps(result.installed_path.as_path()).await; + let auth = self.auth_manager.auth().await; + let apps_needing_auth = self + .plugin_apps_needing_auth_for_install( + &config, + auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth), + &result.plugin_id.as_key(), + &plugin_apps, + ) + .await; + + Ok(PluginInstallResponse { + auth_policy: result.auth_policy.into(), + apps_needing_auth, + }) + } + + async fn remote_plugin_install_response( + &self, + remote_marketplace_name: String, + plugin_name: String, + ) -> Result { + let config = self.load_latest_config(/*fallback_cwd*/ None).await?; if !config.features.enabled(Feature::Plugins) || !config.features.enabled(Feature::RemotePlugin) { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!( - "remote plugin install is not enabled for marketplace {remote_marketplace_name}" - ), - data: None, - }, - ) - .await; - return; + return Err(invalid_request(format!( + "remote plugin install is not enabled for marketplace {remote_marketplace_name}" + ))); } if plugin_name.is_empty() || !plugin_name .chars() .all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' || ch == '~') { - self.send_invalid_request_error( - request_id, - "invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed" - .to_string(), - ) - .await; - return; + return Err(invalid_request( + "invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed", + )); } let auth = self.auth_manager.auth().await; let remote_plugin_service_config = RemotePluginServiceConfig { chatgpt_base_url: config.chatgpt_base_url.clone(), }; - let remote_detail = match codex_core_plugins::remote::fetch_remote_plugin_detail( + let remote_detail = codex_core_plugins::remote::fetch_remote_plugin_detail( &remote_plugin_service_config, auth.as_ref(), &remote_marketplace_name, &plugin_name, ) .await - { - Ok(remote_detail) => remote_detail, - Err(err) => { - self.outgoing - .send_error( - request_id, - remote_plugin_catalog_error_to_jsonrpc( - err, - "read remote plugin details before install", - ), - ) - .await; - return; - } - }; + .map_err(|err| { + remote_plugin_catalog_error_to_jsonrpc(err, "read remote plugin details before install") + })?; if remote_detail.summary.install_policy == PluginInstallPolicy::NotAvailable { - self.send_invalid_request_error( - request_id, - format!("remote plugin {plugin_name} is not available for install"), - ) - .await; - return; + return Err(invalid_request(format!( + "remote plugin {plugin_name} is not available for install" + ))); } - if let Err(err) = codex_core_plugins::remote::install_remote_plugin( + codex_core_plugins::remote::install_remote_plugin( &remote_plugin_service_config, auth.as_ref(), &remote_marketplace_name, &plugin_name, ) .await - { - self.outgoing - .send_error( - request_id, - remote_plugin_catalog_error_to_jsonrpc(err, "install remote plugin"), - ) - .await; - return; - } + .map_err(|err| remote_plugin_catalog_error_to_jsonrpc(err, "install remote plugin"))?; self.clear_plugin_related_caches(); @@ -629,15 +462,10 @@ impl CodexMessageProcessor { ) .await; - self.outgoing - .send_response( - request_id, - PluginInstallResponse { - auth_policy: remote_detail.summary.auth_policy, - apps_needing_auth, - }, - ) - .await; + Ok(PluginInstallResponse { + auth_policy: remote_detail.summary.auth_policy, + apps_needing_auth, + }) } async fn plugin_apps_needing_auth_for_install( @@ -709,59 +537,82 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: PluginUninstallParams, ) { + let result = self.plugin_uninstall_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn plugin_uninstall_response( + &self, + params: PluginUninstallParams, + ) -> Result { let PluginUninstallParams { plugin_id } = params; let plugins_manager = self.thread_manager.plugins_manager(); - let uninstall_result = plugins_manager.uninstall_plugin(plugin_id).await; + plugins_manager + .uninstall_plugin(plugin_id) + .await + .map_err(Self::plugin_uninstall_error)?; + self.clear_plugin_related_caches(); + Ok(PluginUninstallResponse {}) + } - match uninstall_result { - Ok(()) => { - self.clear_plugin_related_caches(); - self.outgoing - .send_response(request_id, PluginUninstallResponse {}) - .await; - } - Err(err) => { - if err.is_invalid_request() { - self.send_invalid_request_error(request_id, err.to_string()) - .await; - return; - } + fn plugin_install_error(err: CorePluginInstallError) -> JSONRPCErrorError { + if err.is_invalid_request() { + return invalid_request(err.to_string()); + } - match err { - CorePluginUninstallError::Config(err) => { - self.send_internal_error( - request_id, - format!("failed to clear plugin config: {err}"), - ) - .await; - } - CorePluginUninstallError::Remote(err) => { - self.send_internal_error( - request_id, - format!("failed to uninstall remote plugin: {err}"), - ) - .await; - } - CorePluginUninstallError::Join(err) => { - self.send_internal_error( - request_id, - format!("failed to uninstall plugin: {err}"), - ) - .await; - } - CorePluginUninstallError::Store(err) => { - self.send_internal_error( - request_id, - format!("failed to uninstall plugin: {err}"), - ) - .await; - } - CorePluginUninstallError::InvalidPluginId(_) => { - unreachable!("invalid plugin ids are handled above"); - } - } + match err { + CorePluginInstallError::Marketplace(err) => { + Self::marketplace_error(err, "install plugin") } + CorePluginInstallError::Config(err) => { + internal_error(format!("failed to persist installed plugin config: {err}")) + } + CorePluginInstallError::Remote(err) => { + internal_error(format!("failed to enable remote plugin: {err}")) + } + CorePluginInstallError::Join(err) => { + internal_error(format!("failed to install plugin: {err}")) + } + CorePluginInstallError::Store(err) => { + internal_error(format!("failed to install plugin: {err}")) + } + } + } + + fn plugin_uninstall_error(err: CorePluginUninstallError) -> JSONRPCErrorError { + if err.is_invalid_request() { + return invalid_request(err.to_string()); + } + + match err { + CorePluginUninstallError::Config(err) => { + internal_error(format!("failed to clear plugin config: {err}")) + } + CorePluginUninstallError::Remote(err) => { + internal_error(format!("failed to uninstall remote plugin: {err}")) + } + CorePluginUninstallError::Join(err) => { + internal_error(format!("failed to uninstall plugin: {err}")) + } + CorePluginUninstallError::Store(err) => { + internal_error(format!("failed to uninstall plugin: {err}")) + } + CorePluginUninstallError::InvalidPluginId(_) => { + unreachable!("invalid plugin ids are handled above"); + } + } + } + + fn marketplace_error(err: MarketplaceError, action: &str) -> JSONRPCErrorError { + match err { + MarketplaceError::MarketplaceNotFound { .. } + | MarketplaceError::InvalidMarketplaceFile { .. } + | MarketplaceError::PluginNotFound { .. } + | MarketplaceError::PluginNotAvailable { .. } + | MarketplaceError::PluginsDisabled + | MarketplaceError::InvalidPlugin(_) => invalid_request(err.to_string()), + MarketplaceError::Io { .. } => internal_error(format!("failed to {action}: {err}")), } } } diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index cc4e22e923..73332394f4 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -391,7 +391,8 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { let processor_outgoing = Arc::clone(&outgoing_message_sender); let auth_manager = - AuthManager::shared_from_config(args.config.as_ref(), args.enable_codex_api_key_env); + AuthManager::shared_from_config(args.config.as_ref(), args.enable_codex_api_key_env) + .await; let config_manager = ConfigManager::new( args.config.codex_home.to_path_buf(), args.cli_overrides, diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 52b6de0e0c..de89608101 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -469,7 +469,7 @@ pub async fn run_main_with_transport_options( config_manager .replace_thread_config_loader(Arc::clone(&discovered_thread_config_loader)); let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await; config_manager.replace_cloud_requirements_loader(auth_manager, config.chatgpt_base_url); } Err(err) => { @@ -631,7 +631,7 @@ pub async fn run_main_with_transport_options( } let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await; let remote_control_enabled = config.features.enabled(Feature::RemoteControl); if transport_accept_handles.is_empty() && !remote_control_enabled { @@ -712,7 +712,7 @@ pub async fn run_main_with_transport_options( let outgoing_message_sender = Arc::new(OutgoingMessageSender::new(outgoing_tx)); let outbound_control_tx = outbound_control_tx; let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await; let processor = Arc::new(MessageProcessor::new(MessageProcessorArgs { outgoing: outgoing_message_sender, arg0_paths, diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index ad1ce31bff..c6e4c67dae 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -653,14 +653,12 @@ impl MessageProcessor { } } } - if self.config.features.enabled(Feature::GeneralAnalytics) { - self.analytics_events_client.track_initialize( - connection_id.0, - analytics_initialize_params, - originator, - self.rpc_transport, - ); - } + self.analytics_events_client.track_initialize( + connection_id.0, + analytics_initialize_params, + originator, + self.rpc_transport, + ); set_default_client_residency_requirement(self.config.enforce_residency.value()); if let Ok(mut suffix) = USER_AGENT_SUFFIX.lock() { *suffix = Some(user_agent_suffix); @@ -716,10 +714,9 @@ impl MessageProcessor { return Err(invalid_request(experimental_required_message(reason))); } let connection_id = connection_request_id.connection_id; - if self.config.features.enabled(Feature::GeneralAnalytics) - && let ClientRequest::TurnStart { request_id, .. } - | ClientRequest::TurnSteer { request_id, .. } - | ClientRequest::TrackUsageLimitBanner { request_id, .. } = &codex_request + if let ClientRequest::TurnStart { request_id, .. } + | ClientRequest::TurnSteer { request_id, .. } + | ClientRequest::TrackUsageLimitBanner { request_id, .. } = &codex_request { self.analytics_events_client.track_request( connection_id.0, diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index 507d7865b9..850d05c04e 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -127,7 +127,7 @@ impl TracingHarness { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; let config = Arc::new(build_test_config(codex_home.path(), &server.uri()).await?); - let (processor, outgoing_rx) = build_test_processor(config); + let (processor, outgoing_rx) = build_test_processor(config).await; let tracing = init_test_tracing(); tracing.exporter.reset(); tracing::callsite::rebuild_interest_cache(); @@ -257,7 +257,7 @@ async fn build_test_config(codex_home: &Path, server_uri: &str) -> Result, ) -> ( Arc, @@ -266,7 +266,7 @@ fn build_test_processor( let (outgoing_tx, outgoing_rx) = mpsc::channel(16); let outgoing = Arc::new(OutgoingMessageSender::new(outgoing_tx)); let auth_manager = - AuthManager::shared_from_config(config.as_ref(), /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config.as_ref(), /*enable_codex_api_key_env*/ false).await; let config_manager = ConfigManager::new( config.codex_home.to_path_buf(), Vec::new(), diff --git a/codex-rs/app-server/src/transport/remote_control/tests.rs b/codex-rs/app-server/src/transport/remote_control/tests.rs index 6b0051f8db..82e02ae5de 100644 --- a/codex-rs/app-server/src/transport/remote_control/tests.rs +++ b/codex-rs/app-server/src/transport/remote_control/tests.rs @@ -497,7 +497,8 @@ async fn remote_control_start_allows_missing_auth_when_enabled() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let (transport_event_tx, _transport_event_rx) = mpsc::channel::(CHANNEL_CAPACITY); let shutdown_token = CancellationToken::new(); @@ -1085,7 +1086,8 @@ async fn remote_control_waits_for_account_id_before_enrolling() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let expected_server_name = gethostname().to_string_lossy().trim().to_string(); let expected_enrollment = RemoteControlEnrollment { account_id: "account_id".to_string(), diff --git a/codex-rs/app-server/src/transport/remote_control/websocket.rs b/codex-rs/app-server/src/transport/remote_control/websocket.rs index 464832e34a..4cd078455a 100644 --- a/codex-rs/app-server/src/transport/remote_control/websocket.rs +++ b/codex-rs/app-server/src/transport/remote_control/websocket.rs @@ -706,7 +706,7 @@ pub(crate) async fn load_remote_control_auth( "remote control requires ChatGPT authentication", )); } - auth_manager.reload(); + auth_manager.reload().await; reloaded = true; continue; }; @@ -714,7 +714,7 @@ pub(crate) async fn load_remote_control_auth( break auth; } if auth.get_account_id().is_none() && !reloaded { - auth_manager.reload(); + auth_manager.reload().await; reloaded = true; continue; } @@ -1090,7 +1090,8 @@ mod tests { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let mut auth_recovery = auth_manager.unauthorized_recovery(); let mut enrollment = Some(RemoteControlEnrollment { account_id: "account_id".to_string(), @@ -1172,7 +1173,8 @@ mod tests { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let mut auth_recovery = auth_manager.unauthorized_recovery(); let mut enrollment = None; save_auth( diff --git a/codex-rs/app-server/tests/suite/v2/analytics.rs b/codex-rs/app-server/tests/suite/v2/analytics.rs index a3ecdbc1f4..862721a154 100644 --- a/codex-rs/app-server/tests/suite/v2/analytics.rs +++ b/codex-rs/app-server/tests/suite/v2/analytics.rs @@ -79,24 +79,6 @@ async fn app_server_default_analytics_enabled_with_flag() -> Result<()> { Ok(()) } -pub(crate) async fn enable_analytics_capture(server: &MockServer, codex_home: &Path) -> Result<()> { - let config_path = codex_home.join("config.toml"); - let config_toml = std::fs::read_to_string(&config_path)?; - if !config_toml.contains("[features]") { - std::fs::write( - &config_path, - format!("{config_toml}\n[features]\ngeneral_analytics = true\n"), - )?; - } else if !config_toml.contains("general_analytics") { - std::fs::write( - &config_path, - config_toml.replace("[features]\n", "[features]\ngeneral_analytics = true\n"), - )?; - } - - mount_analytics_capture(server, codex_home).await -} - pub(crate) async fn mount_analytics_capture(server: &MockServer, codex_home: &Path) -> Result<()> { Mock::given(method("POST")) .and(path("/codex/analytics-events/events")) diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 6c43ebd626..fd773f2e30 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -42,7 +42,7 @@ use wiremock::matchers::method; use wiremock::matchers::path; use super::analytics::assert_basic_thread_initialized_event; -use super::analytics::enable_analytics_capture; +use super::analytics::mount_analytics_capture; use super::analytics::thread_initialized_event; use super::analytics::wait_for_analytics_payload; @@ -385,13 +385,8 @@ async fn thread_fork_tracks_thread_initialized_analytics() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; - create_config_toml_with_chatgpt_base_url( - codex_home.path(), - &server.uri(), - &server.uri(), - /*general_analytics_enabled*/ true, - )?; - enable_analytics_capture(&server, codex_home.path()).await?; + create_config_toml_with_chatgpt_base_url(codex_home.path(), &server.uri(), &server.uri())?; + mount_analytics_capture(&server, codex_home.path()).await?; let conversation_id = create_fake_rollout( codex_home.path(), @@ -496,7 +491,6 @@ async fn thread_fork_surfaces_cloud_requirements_load_errors() -> Result<()> { codex_home.path(), &model_server.uri(), &chatgpt_base_url, - /*general_analytics_enabled*/ false, )?; write_chatgpt_auth( codex_home.path(), @@ -793,13 +787,7 @@ fn create_config_toml_with_chatgpt_base_url( codex_home: &Path, server_uri: &str, chatgpt_base_url: &str, - general_analytics_enabled: bool, ) -> std::io::Result<()> { - let general_analytics_toml = if general_analytics_enabled { - "\ngeneral_analytics = true".to_string() - } else { - "\ngeneral_analytics = false".to_string() - }; let config_toml = codex_home.join("config.toml"); std::fs::write( config_toml, @@ -812,9 +800,6 @@ chatgpt_base_url = "{chatgpt_base_url}" model_provider = "mock_provider" -[features] -{general_analytics_toml} - [model_providers.mock_provider] name = "Mock provider for test" base_url = "{server_uri}/v1" diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 5044fcd11a..d9f5f039de 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -84,7 +84,7 @@ use wiremock::matchers::method; use wiremock::matchers::path; use super::analytics::assert_basic_thread_initialized_event; -use super::analytics::enable_analytics_capture; +use super::analytics::mount_analytics_capture; use super::analytics::thread_initialized_event; use super::analytics::wait_for_analytics_payload; @@ -185,10 +185,7 @@ async fn thread_goal_get_rejects_unmaterialized_thread() -> Result<()> { let config = std::fs::read_to_string(&config_path)?; std::fs::write( &config_path, - config.replace( - "general_analytics = true\n", - "general_analytics = true\ngoals = true\n", - ), + config.replace("personality = true\n", "personality = true\ngoals = true\n"), )?; let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; @@ -238,13 +235,8 @@ async fn thread_resume_tracks_thread_initialized_analytics() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; - create_config_toml_with_chatgpt_base_url( - codex_home.path(), - &server.uri(), - &server.uri(), - /*general_analytics_enabled*/ true, - )?; - enable_analytics_capture(&server, codex_home.path()).await?; + create_config_toml_with_chatgpt_base_url(codex_home.path(), &server.uri(), &server.uri())?; + mount_analytics_capture(&server, codex_home.path()).await?; let conversation_id = create_fake_rollout_with_text_elements( codex_home.path(), @@ -400,10 +392,7 @@ async fn thread_resume_emits_active_goal_update_before_continuation() -> Result< let config = std::fs::read_to_string(&config_path)?; std::fs::write( &config_path, - config.replace( - "general_analytics = true\n", - "general_analytics = true\ngoals = true\n", - ), + config.replace("personality = true\n", "personality = true\ngoals = true\n"), )?; let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; @@ -507,10 +496,7 @@ async fn thread_goal_set_preserves_budget_limited_same_objective() -> Result<()> let config = std::fs::read_to_string(&config_path)?; std::fs::write( &config_path, - config.replace( - "general_analytics = true\n", - "general_analytics = true\ngoals = true\n", - ), + config.replace("personality = true\n", "personality = true\ngoals = true\n"), )?; let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; @@ -608,10 +594,7 @@ async fn thread_goal_clear_deletes_goal_and_notifies() -> Result<()> { let config = std::fs::read_to_string(&config_path)?; std::fs::write( &config_path, - config.replace( - "general_analytics = true\n", - "general_analytics = true\ngoals = true\n", - ), + config.replace("personality = true\n", "personality = true\ngoals = true\n"), )?; let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; @@ -2414,7 +2397,6 @@ async fn thread_resume_surfaces_cloud_requirements_load_errors() -> Result<()> { codex_home.path(), &model_server.uri(), &chatgpt_base_url, - /*general_analytics_enabled*/ false, )?; write_chatgpt_auth( codex_home.path(), @@ -2859,7 +2841,6 @@ model_provider = "mock_provider" [features] personality = true -general_analytics = true [model_providers.mock_provider] name = "Mock provider for test" @@ -2890,7 +2871,6 @@ model_provider = "mock_provider" [features] personality = true -general_analytics = true [model_providers.mock_provider] name = "Mock provider for test" @@ -2907,13 +2887,7 @@ fn create_config_toml_with_chatgpt_base_url( codex_home: &std::path::Path, server_uri: &str, chatgpt_base_url: &str, - general_analytics_enabled: bool, ) -> std::io::Result<()> { - let general_analytics_toml = if general_analytics_enabled { - "\ngeneral_analytics = true".to_string() - } else { - "\ngeneral_analytics = false".to_string() - }; let config_toml = codex_home.join("config.toml"); std::fs::write( config_toml, @@ -2928,7 +2902,6 @@ model_provider = "mock_provider" [features] personality = true -{general_analytics_toml} [model_providers.mock_provider] name = "Mock provider for test" diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index f521d5509c..d8a50b88a4 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -34,7 +34,6 @@ use serde_json::Value; use serde_json::json; use std::path::Path; use std::path::PathBuf; -use std::time::Duration; use tempfile::TempDir; use tokio::time::timeout; use wiremock::Mock; @@ -265,12 +264,7 @@ async fn thread_start_tracks_thread_initialized_analytics() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; - create_config_toml_with_chatgpt_base_url( - codex_home.path(), - &server.uri(), - &server.uri(), - /*general_analytics_enabled*/ true, - )?; + create_config_toml_with_chatgpt_base_url(codex_home.path(), &server.uri(), &server.uri())?; mount_analytics_capture(&server, codex_home.path()).await?; let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; @@ -293,54 +287,6 @@ async fn thread_start_tracks_thread_initialized_analytics() -> Result<()> { Ok(()) } -#[tokio::test] -async fn thread_start_does_not_track_thread_initialized_analytics_without_feature() -> Result<()> { - let server = create_mock_responses_server_repeating_assistant("Done").await; - - let codex_home = TempDir::new()?; - create_config_toml_with_chatgpt_base_url( - codex_home.path(), - &server.uri(), - &server.uri(), - /*general_analytics_enabled*/ false, - )?; - mount_analytics_capture(&server, codex_home.path()).await?; - - let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let req_id = mcp - .send_thread_start_request(ThreadStartParams::default()) - .await?; - let resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(req_id)), - ) - .await??; - let _ = to_response::(resp)?; - - assert_no_thread_initialized_analytics(&server, Duration::from_millis(250)).await?; - Ok(()) -} - -async fn assert_no_thread_initialized_analytics( - server: &MockServer, - wait_duration: Duration, -) -> Result<()> { - tokio::time::sleep(wait_duration).await; - let requests = server.received_requests().await.unwrap_or_default(); - for request in requests.iter().filter(|request| { - request.method == "POST" && request.url.path() == "/codex/analytics-events/events" - }) { - let payload: Value = serde_json::from_slice(&request.body)?; - assert!( - thread_initialized_event(&payload).is_err(), - "thread analytics should be gated off when general_analytics is disabled; payload={payload}" - ); - } - Ok(()) -} - #[tokio::test] async fn thread_start_respects_project_config_from_cwd() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; @@ -643,7 +589,6 @@ async fn thread_start_surfaces_cloud_requirements_load_errors() -> Result<()> { codex_home.path(), &model_server.uri(), &chatgpt_base_url, - /*general_analytics_enabled*/ false, )?; write_chatgpt_auth( codex_home.path(), @@ -966,13 +911,7 @@ fn create_config_toml_with_chatgpt_base_url( codex_home: &Path, server_uri: &str, chatgpt_base_url: &str, - general_analytics_enabled: bool, ) -> std::io::Result<()> { - let general_analytics_toml = if general_analytics_enabled { - "\ngeneral_analytics = true".to_string() - } else { - "\ngeneral_analytics = false".to_string() - }; let config_toml = codex_home.join("config.toml"); std::fs::write( config_toml, @@ -985,9 +924,6 @@ chatgpt_base_url = "{chatgpt_base_url}" model_provider = "mock_provider" -[features] -{general_analytics_toml} - [model_providers.mock_provider] name = "Mock provider for test" base_url = "{server_uri}/v1" diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 6d66edd3d3..f81ec5f168 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -78,7 +78,6 @@ use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; -use super::analytics::enable_analytics_capture; use super::analytics::mount_analytics_capture; use super::analytics::wait_for_analytics_event; @@ -463,7 +462,7 @@ async fn turn_start_tracks_turn_event_analytics() -> Result<()> { &server.uri(), &server.uri(), )?; - enable_analytics_capture(&server, codex_home.path()).await?; + mount_analytics_capture(&server, codex_home.path()).await?; let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; @@ -538,77 +537,6 @@ async fn turn_start_tracks_turn_event_analytics() -> Result<()> { Ok(()) } -#[tokio::test] -async fn turn_start_does_not_track_turn_event_analytics_without_feature() -> Result<()> { - let responses = vec![create_final_assistant_message_sse_response("Done")?]; - let server = create_mock_responses_server_sequence_unchecked(responses).await; - - let codex_home = TempDir::new()?; - write_mock_responses_config_toml_with_chatgpt_base_url( - codex_home.path(), - &server.uri(), - &server.uri(), - )?; - let config_path = codex_home.path().join("config.toml"); - let config_toml = std::fs::read_to_string(&config_path)?; - std::fs::write( - &config_path, - format!("{config_toml}\n[features]\ngeneral_analytics = false\n"), - )?; - mount_analytics_capture(&server, codex_home.path()).await?; - - let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let thread_req = mcp - .send_thread_start_request(ThreadStartParams { - model: Some("mock-model".to_string()), - ..Default::default() - }) - .await?; - let thread_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), - ) - .await??; - let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; - - let turn_req = mcp - .send_turn_start_request(TurnStartParams { - thread_id: thread.id, - input: vec![V2UserInput::Text { - text: "hello".to_string(), - text_elements: Vec::new(), - }], - ..Default::default() - }) - .await?; - let turn_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), - ) - .await??; - let _ = to_response::(turn_resp)?; - - timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("turn/completed"), - ) - .await??; - - let turn_event = wait_for_analytics_event( - &server, - std::time::Duration::from_millis(250), - "codex_turn_event", - ) - .await; - assert!( - turn_event.is_err(), - "turn analytics should be gated off when general_analytics is disabled" - ); - Ok(()) -} - #[tokio::test] async fn turn_start_accepts_text_at_limit_with_mention_item() -> Result<()> { let responses = vec![create_final_assistant_message_sse_response("Done")?]; diff --git a/codex-rs/app-server/tests/suite/v2/turn_steer.rs b/codex-rs/app-server/tests/suite/v2/turn_steer.rs index 16e28d6cc5..a92b2db528 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_steer.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_steer.rs @@ -24,7 +24,7 @@ use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS; use tempfile::TempDir; use tokio::time::timeout; -use super::analytics::enable_analytics_capture; +use super::analytics::mount_analytics_capture; use super::analytics::wait_for_analytics_event; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); @@ -41,7 +41,7 @@ async fn turn_steer_requires_active_turn() -> Result<()> { &server.uri(), &server.uri(), )?; - enable_analytics_capture(&server, &codex_home).await?; + mount_analytics_capture(&server, &codex_home).await?; let mut mcp = McpProcess::new_without_managed_config(&codex_home).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; @@ -125,7 +125,7 @@ async fn turn_steer_rejects_oversized_text_input() -> Result<()> { &server.uri(), &server.uri(), )?; - enable_analytics_capture(&server, &codex_home).await?; + mount_analytics_capture(&server, &codex_home).await?; let mut mcp = McpProcess::new_without_managed_config(&codex_home).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; @@ -234,7 +234,7 @@ async fn turn_steer_returns_active_turn_id() -> Result<()> { &server.uri(), &server.uri(), )?; - enable_analytics_capture(&server, &codex_home).await?; + mount_analytics_capture(&server, &codex_home).await?; let mut mcp = McpProcess::new_without_managed_config(&codex_home).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; diff --git a/codex-rs/chatgpt/src/chatgpt_client.rs b/codex-rs/chatgpt/src/chatgpt_client.rs index 42aac41138..05d8186686 100644 --- a/codex-rs/chatgpt/src/chatgpt_client.rs +++ b/codex-rs/chatgpt/src/chatgpt_client.rs @@ -21,7 +21,7 @@ pub(crate) async fn chatgpt_get_request_with_timeout( ) -> anyhow::Result { let chatgpt_base_url = &config.chatgpt_base_url; let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager .auth() .await diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index 62e8040940..9dba71ce3a 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -26,7 +26,7 @@ const DIRECTORY_CONNECTORS_TIMEOUT: Duration = Duration::from_secs(60); async fn apps_enabled(config: &Config) -> bool { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager.auth().await; config .features @@ -35,7 +35,7 @@ async fn apps_enabled(config: &Config) -> bool { async fn connector_auth(config: &Config) -> anyhow::Result { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager .auth() .await diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index 4fa7272ae4..5c60c20cbf 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -362,7 +362,9 @@ pub async fn run_login_with_device_code_fallback_to_browser( pub async fn run_login_status(cli_config_overrides: CliConfigOverrides) -> ! { let config = load_config_or_exit(cli_config_overrides).await; - match CodexAuth::from_auth_storage(&config.codex_home, config.cli_auth_credentials_store_mode) { + match CodexAuth::from_auth_storage(&config.codex_home, config.cli_auth_credentials_store_mode) + .await + { Ok(Some(auth)) => match auth.auth_mode() { AuthMode::ApiKey => match auth.get_token() { Ok(api_key) => { diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 9f465521da..852dff616d 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1384,7 +1384,7 @@ async fn run_debug_models_command( .map_err(anyhow::Error::msg)?; let config = Config::load_with_cli_overrides(cli_overrides).await?; let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ true); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ true).await; let models_manager = build_models_manager(&config, auth_manager, CollaborationModesConfig::default()); models_manager diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index 86a12e7d17..d38fbb0846 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -727,7 +727,7 @@ pub fn cloud_requirements_loader( }) } -pub fn cloud_requirements_loader_for_storage( +pub async fn cloud_requirements_loader_for_storage( codex_home: PathBuf, enable_codex_api_key_env: bool, credentials_store_mode: AuthCredentialsStoreMode, @@ -738,7 +738,8 @@ pub fn cloud_requirements_loader_for_storage( enable_codex_api_key_env, credentials_store_mode, Some(chatgpt_base_url.clone()), - ); + ) + .await; cloud_requirements_loader(auth_manager, chatgpt_base_url, codex_home) } @@ -853,7 +854,7 @@ mod tests { Ok(()) } - fn auth_manager_with_api_key() -> Arc { + async fn auth_manager_with_api_key() -> Arc { let tmp = tempdir().expect("tempdir"); let auth_json = json!({ "OPENAI_API_KEY": "sk-test-key", @@ -861,15 +862,18 @@ mod tests { "last_refresh": null, }); write_auth_json(tmp.path(), auth_json).expect("write auth"); - Arc::new(AuthManager::new( - tmp.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )) + Arc::new( + AuthManager::new( + tmp.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ) } - fn auth_manager_with_plan_and_identity( + async fn auth_manager_with_plan_and_identity( plan_type: &str, chatgpt_user_id: Option<&str>, account_id: Option<&str>, @@ -886,12 +890,15 @@ mod tests { ), ) .expect("write auth"); - Arc::new(AuthManager::new( - tmp.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )) + Arc::new( + AuthManager::new( + tmp.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ) } fn chatgpt_auth_json( @@ -975,7 +982,7 @@ mod tests { manager: Arc, } - fn managed_auth_context( + async fn managed_auth_context( plan_type: &str, chatgpt_user_id: Option<&str>, account_id: Option<&str>, @@ -995,18 +1002,22 @@ mod tests { ) .expect("write auth"); ManagedAuthContext { - manager: Arc::new(AuthManager::new( - home.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )), + manager: Arc::new( + AuthManager::new( + home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ), _home: home, } } - fn auth_manager_with_plan(plan_type: &str) -> Arc { + async fn auth_manager_with_plan(plan_type: &str) -> Arc { auth_manager_with_plan_and_identity(plan_type, Some("user-12345"), Some("account-12345")) + .await } fn parse_for_fetch(contents: Option<&str>) -> Option { @@ -1118,7 +1129,7 @@ mod tests { #[tokio::test] async fn fetch_cloud_requirements_skips_non_chatgpt_auth() { - let auth_manager = auth_manager_with_api_key(); + let auth_manager = auth_manager_with_api_key().await; let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( auth_manager, @@ -1134,7 +1145,7 @@ mod tests { async fn fetch_cloud_requirements_skips_non_business_or_enterprise_plan() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("pro"), + auth_manager_with_plan("pro").await, Arc::new(StaticFetcher { contents: None }), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1147,7 +1158,7 @@ mod tests { async fn fetch_cloud_requirements_skips_team_like_usage_based_plan() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("self_serve_business_usage_based"), + auth_manager_with_plan("self_serve_business_usage_based").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1161,7 +1172,7 @@ mod tests { async fn fetch_cloud_requirements_allows_business_plan() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1193,7 +1204,7 @@ mod tests { async fn fetch_cloud_requirements_allows_business_like_usage_based_plan() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise_cbp_usage_based"), + auth_manager_with_plan("enterprise_cbp_usage_based").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1225,7 +1236,7 @@ mod tests { async fn fetch_cloud_requirements_allows_hc_plan_as_enterprise() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("hc"), + auth_manager_with_plan("hc").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1329,7 +1340,7 @@ enabled = false #[tokio::test(start_paused = true)] async fn fetch_cloud_requirements_times_out() { - let auth_manager = auth_manager_with_plan("enterprise"); + let auth_manager = auth_manager_with_plan("enterprise").await; let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( auth_manager, @@ -1356,7 +1367,7 @@ enabled = false ])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1405,12 +1416,15 @@ enabled = false ), ) .expect("write initial auth"); - let auth_manager = Arc::new(AuthManager::new( - auth_home.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )); + let auth_manager = Arc::new( + AuthManager::new( + auth_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ); write_auth_json( auth_home.path(), @@ -1479,12 +1493,15 @@ enabled = false ), ) .expect("write initial auth"); - let auth_manager = Arc::new(AuthManager::new( - auth_home.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )); + let auth_manager = Arc::new( + AuthManager::new( + auth_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ); write_auth_json( auth_home.path(), @@ -1559,7 +1576,8 @@ enabled = false Some("account-12345"), "stale-access-token", "test-refresh-token", - ); + ) + .await; write_auth_json( auth._home.path(), chatgpt_auth_json( @@ -1611,12 +1629,15 @@ enabled = false ), ) .expect("write auth"); - let auth_manager = Arc::new(AuthManager::new( - auth_home.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )); + let auth_manager = Arc::new( + AuthManager::new( + auth_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ); let fetcher = Arc::new(UnauthorizedFetcher { message: @@ -1653,7 +1674,7 @@ enabled = false ])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1678,7 +1699,7 @@ enabled = false ))])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher, codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1700,7 +1721,7 @@ enabled = false async fn fetch_cloud_requirements_uses_cache_when_valid() { let codex_home = tempdir().expect("tempdir"); let prime_service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1711,7 +1732,7 @@ enabled = false let fetcher = Arc::new(SequenceFetcher::new(vec![Err(request_error())])); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1747,7 +1768,8 @@ enabled = false "business", /*chatgpt_user_id*/ None, Some("account-12345"), - ), + ) + .await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1790,7 +1812,7 @@ enabled = false async fn fetch_cloud_requirements_does_not_use_cache_when_auth_identity_is_incomplete() { let codex_home = tempdir().expect("tempdir"); let prime_service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1807,7 +1829,8 @@ enabled = false "business", /*chatgpt_user_id*/ None, Some("account-12345"), - ), + ) + .await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1843,7 +1866,8 @@ enabled = false "business", Some("user-12345"), Some("account-12345"), - ), + ) + .await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1860,7 +1884,8 @@ enabled = false "business", Some("user-99999"), Some("account-12345"), - ), + ) + .await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1892,7 +1917,7 @@ enabled = false async fn fetch_cloud_requirements_ignores_tampered_cache() { let codex_home = tempdir().expect("tempdir"); let prime_service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1917,7 +1942,7 @@ enabled = false "allowed_approval_policies = [\"never\"]".to_string(), ))])); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise"), + auth_manager_with_plan("enterprise").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1975,7 +2000,7 @@ enabled = false "allowed_approval_policies = [\"never\"]".to_string(), ))])); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise"), + auth_manager_with_plan("enterprise").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -2007,7 +2032,7 @@ enabled = false async fn fetch_cloud_requirements_writes_signed_cache() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -2070,7 +2095,7 @@ enabled = false let fetcher = Arc::new(SequenceFetcher::new(vec![Ok(None), Err(request_error())])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise"), + auth_manager_with_plan("enterprise").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -2088,7 +2113,7 @@ enabled = false ])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise"), + auth_manager_with_plan("enterprise").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -2121,7 +2146,7 @@ enabled = false )), ])); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher, codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, diff --git a/codex-rs/cloud-tasks/src/util.rs b/codex-rs/cloud-tasks/src/util.rs index e433b892e5..9a5056aa66 100644 --- a/codex-rs/cloud-tasks/src/util.rs +++ b/codex-rs/cloud-tasks/src/util.rs @@ -44,12 +44,15 @@ pub fn normalize_base_url(input: &str) -> String { pub async fn load_auth_manager(chatgpt_base_url: Option) -> Option { // TODO: pass in cli overrides once cloud tasks properly support them. let config = Config::load_with_cli_overrides(Vec::new()).await.ok()?; - Some(AuthManager::new( - config.codex_home.to_path_buf(), - /*enable_codex_api_key_env*/ false, - config.cli_auth_credentials_store_mode, - chatgpt_base_url.or(Some(config.chatgpt_base_url)), - )) + Some( + AuthManager::new( + config.codex_home.to_path_buf(), + /*enable_codex_api_key_env*/ false, + config.cli_auth_credentials_store_mode, + chatgpt_base_url.or(Some(config.chatgpt_base_url)), + ) + .await, + ) } /// Build headers for ChatGPT-backed requests: `User-Agent`, optional `Authorization`, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 5727a4bdcf..e4d156c540 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -421,9 +421,6 @@ "fast_mode": { "type": "boolean" }, - "general_analytics": { - "type": "boolean" - }, "goals": { "type": "boolean" }, @@ -2631,9 +2628,6 @@ "fast_mode": { "type": "boolean" }, - "general_analytics": { - "type": "boolean" - }, "goals": { "type": "boolean" }, diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index d4ec6858d1..daf1acf8f3 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -262,7 +262,6 @@ impl AgentControl { parent_thread_id, .. }, )) = notification_source.as_ref() - && new_thread.thread.enabled(Feature::GeneralAnalytics) { let client_metadata = match state.get_thread(*parent_thread_id).await { Ok(parent_thread) => { diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 1fb2f42f2e..f3df3cd4c6 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -104,18 +104,16 @@ pub(crate) async fn run_codex_thread_interactive( })) .or_cancel(&cancel_token) .await??; - if parent_session.enabled(codex_features::Feature::GeneralAnalytics) { - let thread_config = codex.thread_config_snapshot().await; - let client_metadata = parent_session.app_server_client_metadata().await; - emit_subagent_session_started( - &parent_session.services.analytics_events_client, - client_metadata, - codex.session.conversation_id, - Some(parent_session.conversation_id), - thread_config, - subagent_source, - ); - } + let thread_config = codex.thread_config_snapshot().await; + let client_metadata = parent_session.app_server_client_metadata().await; + emit_subagent_session_started( + &parent_session.services.analytics_events_client, + client_metadata, + codex.session.conversation_id, + Some(parent_session.conversation_id), + thread_config, + subagent_source, + ); let codex = Arc::new(codex); // Use a child token so parent cancel cascades but we can scope it to this task diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index e9218ae7f0..ed7a95b96e 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -18,7 +18,6 @@ use codex_analytics::CompactionStatus; use codex_analytics::CompactionStrategy; use codex_analytics::CompactionTrigger; use codex_analytics::now_unix_seconds; -use codex_features::Feature; use codex_protocol::error::CodexErr; use codex_protocol::error::Result as CodexResult; use codex_protocol::items::ContextCompactionItem; @@ -295,7 +294,6 @@ async fn run_compact_task_inner_impl( } pub(crate) struct CompactionAnalyticsAttempt { - enabled: bool, thread_id: String, turn_id: String, trigger: CompactionTrigger, @@ -316,10 +314,8 @@ impl CompactionAnalyticsAttempt { implementation: CompactionImplementation, phase: CompactionPhase, ) -> Self { - let enabled = sess.enabled(Feature::GeneralAnalytics); let active_context_tokens_before = sess.get_total_token_usage().await; Self { - enabled, thread_id: sess.conversation_id.to_string(), turn_id: turn_context.sub_id.clone(), trigger, @@ -338,9 +334,6 @@ impl CompactionAnalyticsAttempt { status: CompactionStatus, error: Option, ) { - if !self.enabled { - return; - } let active_context_tokens_after = sess.get_total_token_usage().await; sess.services .analytics_events_client diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 456f3a7eac..42cd415217 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -144,7 +144,7 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools( config: &Config, ) -> Option> { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager.auth().await; if !config .features @@ -216,7 +216,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( environment_manager: &EnvironmentManager, ) -> anyhow::Result { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager.auth().await; if !config .features @@ -434,7 +434,7 @@ async fn list_directory_connectors_for_tool_suggest_with_auth( Some(auth) } else { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; loaded_auth = auth_manager.auth().await; loaded_auth.as_ref() }; diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index c4bdc916ff..7e66a5b703 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -519,6 +519,10 @@ const RESIZED_IMAGE_BYTES_ESTIMATE: i64 = 7373; // Use a direct 32px patch count only for `detail: "original"`; // all other image inputs continue to use `RESIZED_IMAGE_BYTES_ESTIMATE`. const ORIGINAL_IMAGE_PATCH_SIZE: u32 = 32; +// See https://platform.openai.com/docs/guides/images-vision#model-sizing-behavior. +// Keep this hard-coded for now; move it into model capabilities if the patch +// budget starts changing often across model releases. +const ORIGINAL_IMAGE_MAX_PATCHES: usize = 10_000; const ORIGINAL_IMAGE_ESTIMATE_CACHE_SIZE: usize = 32; static ORIGINAL_IMAGE_ESTIMATE_CACHE: LazyLock>> = @@ -621,6 +625,7 @@ fn estimate_original_image_bytes(image_url: &str) -> Option { let patches_high = height.saturating_add(patch_size.saturating_sub(1)) / patch_size; let patch_count = patches_wide.saturating_mul(patches_high); let patch_count = usize::try_from(patch_count).unwrap_or(usize::MAX); + let patch_count = patch_count.min(ORIGINAL_IMAGE_MAX_PATCHES); Some(i64::try_from(approx_bytes_for_tokens(patch_count)).unwrap_or(i64::MAX)) }) } diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index f5c30af575..ad67deb544 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -26,6 +26,7 @@ use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::truncate_text; use image::ImageBuffer; use image::ImageFormat; +use image::Luma; use image::Rgba; use pretty_assertions::assert_eq; use regex_lite::Regex; @@ -1900,6 +1901,38 @@ fn original_detail_images_scale_with_dimensions() { assert_eq!(estimated, expected); } +#[test] +fn original_detail_images_are_capped_at_max_patch_count() { + // 3201x3201 at 32px patches yields 101 * 101 = 10,201 patches, + // which exceeds the original-detail patch budget. + let width = 3201; + let height = 3201; + let image = ImageBuffer::from_pixel(width, height, Luma([12u8])); + let mut bytes = std::io::Cursor::new(Vec::new()); + image + .write_to(&mut bytes, ImageFormat::Png) + .expect("encode png"); + let payload = BASE64_STANDARD.encode(bytes.get_ref()); + let image_url = format!("data:image/png;base64,{payload}"); + let item = ResponseItem::FunctionCallOutput { + call_id: "call-original-capped".to_string(), + output: FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputImage { + image_url, + detail: Some(ImageDetail::Original), + }, + ]), + }; + + let raw_len = serde_json::to_string(&item).unwrap().len() as i64; + let estimated = estimate_response_item_model_visible_bytes(&item); + let capped_original_detail_image_bytes = + i64::try_from(approx_bytes_for_tokens(ORIGINAL_IMAGE_MAX_PATCHES)).unwrap(); + let expected = raw_len - payload.len() as i64 + capped_original_detail_image_bytes; + + assert_eq!(estimated, expected); +} + #[test] fn original_detail_webp_images_scale_with_dimensions() { // Same dimensions as the PNG case above, so the patch-based replacement cost is the same. diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 2635d641d5..850d84dd2a 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -6,7 +6,6 @@ use codex_analytics::GuardianReviewDecision; use codex_analytics::GuardianReviewFailureReason; use codex_analytics::GuardianReviewTerminalStatus; use codex_analytics::GuardianReviewTrackContext; -use codex_features::Feature; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; @@ -161,13 +160,9 @@ pub(crate) fn is_guardian_reviewer_source( fn track_guardian_review( session: &Session, - turn: &TurnContext, tracking: &GuardianReviewTrackContext, result: GuardianReviewAnalyticsResult, ) { - if !turn.config.features.enabled(Feature::GeneralAnalytics) { - return; - } session .services .analytics_events_client @@ -279,7 +274,6 @@ async fn run_guardian_review( { track_guardian_review( session.as_ref(), - turn.as_ref(), &review_tracking, GuardianReviewAnalyticsResult { decision: GuardianReviewDecision::Aborted, @@ -325,7 +319,6 @@ async fn run_guardian_review( let approved = matches!(assessment.outcome, GuardianAssessmentOutcome::Allow); track_guardian_review( session.as_ref(), - turn.as_ref(), &review_tracking, GuardianReviewAnalyticsResult { decision: if approved { @@ -356,7 +349,6 @@ async fn run_guardian_review( .to_string(); track_guardian_review( session.as_ref(), - turn.as_ref(), &review_tracking, GuardianReviewAnalyticsResult { decision: GuardianReviewDecision::Denied, @@ -395,7 +387,6 @@ async fn run_guardian_review( GuardianReviewError::Cancelled => { track_guardian_review( session.as_ref(), - turn.as_ref(), &review_tracking, GuardianReviewAnalyticsResult { decision: GuardianReviewDecision::Aborted, @@ -437,7 +428,6 @@ async fn run_guardian_review( let rationale = format!("Automatic approval review failed: {message}"); track_guardian_review( session.as_ref(), - turn.as_ref(), &review_tracking, GuardianReviewAnalyticsResult { decision: GuardianReviewDecision::Denied, diff --git a/codex-rs/core/src/memories/phase2.rs b/codex-rs/core/src/memories/phase2.rs index d156d1dee5..bcad738319 100644 --- a/codex-rs/core/src/memories/phase2.rs +++ b/codex-rs/core/src/memories/phase2.rs @@ -163,17 +163,15 @@ pub(super) async fn run(session: &Arc, config: Arc) { .get_agent_config_snapshot(thread_id) .await { - if session.enabled(Feature::GeneralAnalytics) { - let client_metadata = session.app_server_client_metadata().await; - emit_subagent_session_started( - &session.services.analytics_events_client, - client_metadata, - thread_id, - /*parent_thread_id*/ None, - thread_config, - SubAgentSource::MemoryConsolidation, - ); - } + let client_metadata = session.app_server_client_metadata().await; + emit_subagent_session_started( + &session.services.analytics_events_client, + client_metadata, + thread_id, + /*parent_thread_id*/ None, + thread_config, + SubAgentSource::MemoryConsolidation, + ); } else { warn!("failed to load memory consolidation thread config for analytics: {thread_id}"); } diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index b6c66410a1..f782c0d232 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -29,7 +29,7 @@ pub async fn build_prompt_input( config.ephemeral = true; let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await; let local_runtime_paths = ExecServerRuntimePaths::from_optional_paths( config.codex_self_exe.clone(), diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 6383b892ec..41195490b6 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -95,9 +95,7 @@ use codex_protocol::protocol::ReasoningRawContentDeltaEvent; use codex_protocol::protocol::TurnDiffEvent; use codex_protocol::protocol::WarningEvent; use codex_protocol::user_input::UserInput; -use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; -use codex_tools::ToolSpec; use codex_tools::filter_tool_suggest_discoverable_tools_for_client; use codex_utils_stream_parser::AssistantTextChunk; use codex_utils_stream_parser::AssistantTextStreamParser; @@ -664,10 +662,6 @@ async fn track_turn_resolved_config_analytics( turn_context: &TurnContext, input: &[UserInput], ) { - if !sess.enabled(Feature::GeneralAnalytics) { - return; - } - let thread_config = { let state = sess.state.lock().await; state.session_configuration.thread_config_snapshot() @@ -946,25 +940,9 @@ pub(crate) fn build_prompt( turn_context: &TurnContext, base_instructions: BaseInstructions, ) -> Prompt { - let deferred_dynamic_tools = turn_context - .dynamic_tools - .iter() - .filter(|tool| tool.defer_loading) - .map(|tool| ToolName::new(tool.namespace.clone(), tool.name.clone())) - .collect::>(); - let tools = if deferred_dynamic_tools.is_empty() { - router.model_visible_specs() - } else { - router - .model_visible_specs() - .into_iter() - .filter_map(|spec| filter_deferred_dynamic_tool_spec(spec, &deferred_dynamic_tools)) - .collect() - }; - Prompt { input, - tools, + tools: router.model_visible_specs(), parallel_tool_calls: turn_context.model_info.supports_parallel_tool_calls, base_instructions, personality: turn_context.personality, @@ -975,35 +953,6 @@ pub(crate) fn build_prompt( } } -fn filter_deferred_dynamic_tool_spec( - spec: ToolSpec, - deferred_dynamic_tools: &HashSet, -) -> Option { - match spec { - ToolSpec::Function(tool) => { - if deferred_dynamic_tools.contains(&ToolName::plain(tool.name.as_str())) { - None - } else { - Some(ToolSpec::Function(tool)) - } - } - ToolSpec::Namespace(mut namespace) => { - let namespace_name = namespace.name.clone(); - namespace.tools.retain(|tool| match tool { - ResponsesApiNamespaceTool::Function(tool) => !deferred_dynamic_tools.contains( - &ToolName::namespaced(namespace_name.as_str(), tool.name.as_str()), - ), - }); - if namespace.tools.is_empty() { - None - } else { - Some(ToolSpec::Namespace(namespace)) - } - } - spec => Some(spec), - } -} - #[allow(clippy::too_many_arguments)] #[instrument(level = "trace", skip_all, diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 1f0dec6925..aeba3b0556 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -71,23 +71,26 @@ impl ToolRouter { dynamic_tools, ); let (specs, registry) = builder.build(); - let model_visible_specs = if config.code_mode_only_enabled { - specs - .iter() - .filter_map(|configured_tool| { - if !codex_code_mode::is_code_mode_nested_tool(configured_tool.name()) { - Some(configured_tool.spec.clone()) - } else { - None - } - }) - .collect() - } else { - specs - .iter() - .map(|configured_tool| configured_tool.spec.clone()) - .collect() - }; + let deferred_dynamic_tools = dynamic_tools + .iter() + .filter(|tool| tool.defer_loading) + .map(|tool| ToolName::new(tool.namespace.clone(), tool.name.clone())) + .collect::>(); + let model_visible_specs = specs + .iter() + .filter_map(|configured_tool| { + if config.code_mode_only_enabled + && codex_code_mode::is_code_mode_nested_tool(configured_tool.name()) + { + return None; + } + + filter_deferred_dynamic_tool_spec( + configured_tool.spec.clone(), + &deferred_dynamic_tools, + ) + }) + .collect(); Self { registry, @@ -293,6 +296,39 @@ impl ToolRouter { self.registry.dispatch_any(invocation).await } } + +fn filter_deferred_dynamic_tool_spec( + spec: ToolSpec, + deferred_dynamic_tools: &HashSet, +) -> Option { + if deferred_dynamic_tools.is_empty() { + return Some(spec); + } + + match spec { + ToolSpec::Function(tool) => { + if deferred_dynamic_tools.contains(&ToolName::plain(tool.name.as_str())) { + None + } else { + Some(ToolSpec::Function(tool)) + } + } + ToolSpec::Namespace(mut namespace) => { + let namespace_name = namespace.name.clone(); + namespace.tools.retain(|tool| match tool { + ResponsesApiNamespaceTool::Function(tool) => !deferred_dynamic_tools.contains( + &ToolName::namespaced(namespace_name.as_str(), tool.name.as_str()), + ), + }); + if namespace.tools.is_empty() { + None + } else { + Some(ToolSpec::Namespace(namespace)) + } + } + spec => Some(spec), + } +} #[cfg(test)] #[path = "router_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index e8c098c41e..ac859d9aa6 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -3,8 +3,13 @@ use std::sync::Arc; use crate::session::tests::make_session_and_context; use crate::tools::context::ToolPayload; +use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::ResponseItem; +use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; +use codex_tools::ToolSpec; +use pretty_assertions::assert_eq; +use serde_json::json; use super::ToolCall; use super::ToolRouter; @@ -133,3 +138,86 @@ async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()> Ok(()) } + +#[tokio::test] +async fn model_visible_specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> { + let (_, turn) = make_session_and_context().await; + let hidden_tool = "hidden_dynamic_tool"; + let visible_tool = "visible_dynamic_tool"; + let dynamic_tools = vec![ + DynamicToolSpec { + namespace: Some("codex_app".to_string()), + name: hidden_tool.to_string(), + description: "Hidden until discovered.".to_string(), + input_schema: json!({ + "type": "object", + "properties": {}, + "additionalProperties": false, + }), + defer_loading: true, + }, + DynamicToolSpec { + namespace: Some("codex_app".to_string()), + name: visible_tool.to_string(), + description: "Visible immediately.".to_string(), + input_schema: json!({ + "type": "object", + "properties": {}, + "additionalProperties": false, + }), + defer_loading: false, + }, + ]; + + let router = ToolRouter::from_config( + &turn.tools_config, + ToolRouterParams { + deferred_mcp_tools: None, + mcp_tools: None, + unavailable_called_tools: Vec::new(), + parallel_mcp_server_names: HashSet::new(), + discoverable_tools: None, + dynamic_tools: &dynamic_tools, + }, + ); + + assert!( + router + .find_spec(&ToolName::namespaced("codex_app", hidden_tool)) + .is_some() + ); + assert_eq!( + namespace_function_names(&router.specs(), "codex_app"), + vec![hidden_tool.to_string(), visible_tool.to_string()] + ); + assert_eq!( + namespace_function_names(&router.model_visible_specs(), "codex_app"), + vec![visible_tool.to_string()] + ); + + Ok(()) +} + +fn namespace_function_names(specs: &[ToolSpec], namespace_name: &str) -> Vec { + specs + .iter() + .find_map(|spec| match spec { + ToolSpec::Namespace(namespace) if namespace.name == namespace_name => Some( + namespace + .tools + .iter() + .map(|tool| match tool { + ResponsesApiNamespaceTool::Function(tool) => tool.name.clone(), + }) + .collect(), + ), + ToolSpec::Function(_) + | ToolSpec::Freeform(_) + | ToolSpec::ToolSearch { .. } + | ToolSpec::LocalShell {} + | ToolSpec::ImageGeneration { .. } + | ToolSpec::WebSearch { .. } + | ToolSpec::Namespace(_) => None, + }) + .unwrap_or_default() +} diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 8886568435..7bd5cee686 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1091,7 +1091,8 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { config.model_provider = model_provider; let auth_manager = - match CodexAuth::from_auth_storage(codex_home.path(), AuthCredentialsStoreMode::File) { + match CodexAuth::from_auth_storage(codex_home.path(), AuthCredentialsStoreMode::File).await + { Ok(Some(auth)) => codex_core::test_support::auth_manager_from_auth(auth), Ok(None) => panic!("No CodexAuth found in codex_home"), Err(e) => panic!("Failed to load CodexAuth: {e}"), diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index b70cc6f38e..24e86881b9 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use anyhow::Result; use codex_core::compact::SUMMARY_PREFIX; use codex_login::CodexAuth; +use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::items::TurnItem; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; @@ -36,6 +37,7 @@ use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; use core_test_support::wait_for_event_with_timeout; use pretty_assertions::assert_eq; +use serde_json::Value; use serde_json::json; use tokio::time::Duration; use wiremock::ResponseTemplate; @@ -55,6 +57,53 @@ fn estimate_compact_payload_tokens(request: &responses::ResponsesRequest) -> i64 .saturating_add(approx_token_count(&request.instructions_text())) } +fn assert_tools_payload_does_not_defer(body: &Value) { + if let Some(tools) = body.get("tools") { + assert!( + !contains_defer_loading(tools), + "model-visible tools should not include deferred declarations: {tools}" + ); + } +} + +fn namespace_child_tool_names(body: &Value, namespace: &str) -> Vec { + body.get("tools") + .and_then(Value::as_array) + .and_then(|tools| { + tools.iter().find_map(|tool| { + if tool.get("type").and_then(Value::as_str) == Some("namespace") + && tool.get("name").and_then(Value::as_str) == Some(namespace) + { + tool.get("tools").and_then(Value::as_array).map(|children| { + children + .iter() + .filter_map(|child| { + child + .get("name") + .and_then(Value::as_str) + .map(str::to_string) + }) + .collect() + }) + } else { + None + } + }) + }) + .unwrap_or_default() +} + +fn contains_defer_loading(value: &Value) -> bool { + match value { + Value::Object(map) => { + map.get("defer_loading").and_then(Value::as_bool) == Some(true) + || map.values().any(contains_defer_loading) + } + Value::Array(values) => values.iter().any(contains_defer_loading), + Value::Null | Value::Bool(_) | Value::Number(_) | Value::String(_) => false, + } +} + const PRETURN_CONTEXT_DIFF_CWD: &str = "/tmp/PRETURN_CONTEXT_DIFF_CWD"; const DUMMY_FUNCTION_NAME: &str = "test_tool"; const REMOTE_COMPACT_TURN_COMPLETE_TIMEOUT: Duration = Duration::from_secs(30); @@ -358,6 +407,100 @@ async fn remote_compact_replaces_history_for_followups() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_compact_filters_deferred_dynamic_tools() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let mut test = builder.build(&server).await?; + let hidden_tool = "hidden_dynamic_tool"; + let visible_tool = "visible_dynamic_tool"; + let input_schema = json!({ + "type": "object", + "properties": {}, + "additionalProperties": false, + }); + let dynamic_tools = vec![ + DynamicToolSpec { + namespace: Some("codex_app".to_string()), + name: hidden_tool.to_string(), + description: "Hidden until discovered.".to_string(), + input_schema: input_schema.clone(), + defer_loading: true, + }, + DynamicToolSpec { + namespace: Some("codex_app".to_string()), + name: visible_tool.to_string(), + description: "Visible immediately.".to_string(), + input_schema, + defer_loading: false, + }, + ]; + let new_thread = test + .thread_manager + .start_thread_with_tools( + test.config.clone(), + dynamic_tools, + /*persist_extended_history*/ false, + ) + .await?; + test.codex = new_thread.thread; + test.session_configured = new_thread.session_configured; + let codex = test.codex.clone(); + + let responses_mock = mount_sse_once( + &server, + sse(vec![ + responses::ev_assistant_message("m1", "FIRST_REMOTE_REPLY"), + responses::ev_completed("resp-1"), + ]), + ) + .await; + let compact_mock = responses::mount_compact_json_once( + &server, + serde_json::json!({ + "output": compacted_summary_only_output("compact summary"), + }), + ) + .await; + + codex + .submit(Op::UserInput { + environments: None, + items: vec![UserInput::Text { + text: "hello remote compact".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + }) + .await?; + wait_for_turn_complete(&codex).await; + + codex.submit(Op::Compact).await?; + wait_for_turn_complete(&codex).await; + + let first_response_body = responses_mock.single_request().body_json(); + let compact_body = compact_mock.single_request().body_json(); + assert_eq!( + compact_body["tools"], first_response_body["tools"], + "compact requests should send the same model-visible tools payload as /v1/responses" + ); + assert_tools_payload_does_not_defer(&first_response_body); + assert_tools_payload_does_not_defer(&compact_body); + assert_eq!( + namespace_child_tool_names(&first_response_body, "codex_app"), + vec![visible_tool.to_string()] + ); + assert_eq!( + namespace_child_tool_names(&compact_body, "codex_app"), + vec![visible_tool.to_string()] + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn remote_compact_runs_automatically() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index fa2eca9b22..1794f2fd3f 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -605,7 +605,8 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() - "apps tools/call should include turn metadata turn_id: {apps_tool_call:?}" ); - let first_request_tools = tool_names(&requests[0].body_json()); + let first_request_body = requests[0].body_json(); + let first_request_tools = tool_names(&first_request_body); assert!( first_request_tools .iter() @@ -823,7 +824,8 @@ async fn tool_search_returns_deferred_dynamic_tool_and_routes_follow_up_call() - let requests = mock.requests(); assert_eq!(requests.len(), 3); - let first_request_tools = tool_names(&requests[0].body_json()); + let first_request_body = requests[0].body_json(); + let first_request_tools = tool_names(&first_request_body); assert!( first_request_tools .iter() @@ -853,7 +855,8 @@ async fn tool_search_returns_deferred_dynamic_tool_and_routes_follow_up_call() - })] ); - let second_request_tools = tool_names(&requests[1].body_json()); + let second_request_body = requests[1].body_json(); + let second_request_tools = tool_names(&second_request_body); assert!( !second_request_tools.iter().any(|name| name == tool_name), "follow-up request should rely on tool_search_output history, not tool injection: {second_request_tools:?}" @@ -870,7 +873,8 @@ async fn tool_search_returns_deferred_dynamic_tool_and_routes_follow_up_call() - FunctionCallOutputPayload::from_text("dynamic-search-ok".to_string()) ); - let third_request_tools = tool_names(&requests[2].body_json()); + let third_request_body = requests[2].body_json(); + let third_request_tools = tool_names(&third_request_body); assert!( !third_request_tools.iter().any(|name| name == tool_name), "post-tool follow-up should rely on tool_search_output history, not tool injection: {third_request_tools:?}" diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 334e4001d5..9e9d9bc148 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -348,7 +348,8 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result /*enable_codex_api_key_env*/ false, config_toml.cli_auth_credentials_store.unwrap_or_default(), chatgpt_base_url, - ); + ) + .await; let run_cli_overrides = cli_kv_overrides.clone(); let run_loader_overrides = loader_overrides.clone(); let run_cloud_requirements = cloud_requirements.clone(); @@ -438,7 +439,9 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result auth_credentials_store_mode: config.cli_auth_credentials_store_mode, forced_login_method: config.forced_login_method, forced_chatgpt_workspace_id: config.forced_chatgpt_workspace_id.clone(), - }) { + }) + .await + { eprintln!("{err}"); std::process::exit(1); } diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 6a2a2bc717..4a4b12be35 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -128,8 +128,6 @@ pub enum Feature { CodexGitCommit, /// Enable runtime metrics snapshots via a manual reader. RuntimeMetrics, - /// Enable thread lifecycle analytics emitted via the app-server analytics pipeline. - GeneralAnalytics, /// Persist rollout metadata to a local SQLite database. Sqlite, /// Enable startup memory extraction and file-backed memory consolidation. @@ -712,12 +710,6 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, - FeatureSpec { - id: Feature::GeneralAnalytics, - key: "general_analytics", - stage: Stage::Stable, - default_enabled: true, - }, FeatureSpec { id: Feature::Sqlite, key: "sqlite", diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index ca05d72d2d..b8ddf53506 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -156,12 +156,6 @@ fn browser_controls_are_stable_and_enabled_by_default() { assert_eq!(feature_for_key("computer_use"), Some(Feature::ComputerUse)); } -#[test] -fn general_analytics_is_stable_and_enabled_by_default() { - assert_eq!(Feature::GeneralAnalytics.stage(), Stage::Stable); - assert_eq!(Feature::GeneralAnalytics.default_enabled(), true); -} - #[test] fn use_linux_sandbox_bwrap_is_a_removed_feature_key() { assert_eq!( diff --git a/codex-rs/login/src/auth/auth_tests.rs b/codex-rs/login/src/auth/auth_tests.rs index b38a40b6f0..b6c86761ef 100644 --- a/codex-rs/login/src/auth/auth_tests.rs +++ b/codex-rs/login/src/auth/auth_tests.rs @@ -116,10 +116,11 @@ fn login_with_agent_identity_rejects_invalid_jwt() { ); } -#[test] -fn missing_auth_json_returns_none() { +#[tokio::test] +async fn missing_auth_json_returns_none() { let dir = tempdir().unwrap(); let auth = CodexAuth::from_auth_storage(dir.path(), AuthCredentialsStoreMode::File) + .await .expect("call should succeed"); assert_eq!(auth, None); } @@ -143,6 +144,7 @@ async fn pro_account_with_no_api_key_uses_chatgpt_auth() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .unwrap() .unwrap(); assert_eq!(None, auth.api_key()); @@ -196,6 +198,7 @@ async fn loads_api_key_from_auth_json() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .unwrap() .unwrap(); assert_eq!(auth.auth_mode(), AuthMode::ApiKey); @@ -222,15 +225,16 @@ fn logout_removes_auth_file() -> Result<(), std::io::Error> { Ok(()) } -#[test] -fn unauthorized_recovery_reports_mode_and_step_names() { +#[tokio::test] +async fn unauthorized_recovery_reports_mode_and_step_names() { let dir = tempdir().unwrap(); let manager = AuthManager::shared( dir.path().to_path_buf(), /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let managed = UnauthorizedRecovery { manager: Arc::clone(&manager), step: UnauthorizedRecoveryStep::Reload, @@ -250,8 +254,8 @@ fn unauthorized_recovery_reports_mode_and_step_names() { assert_eq!(external.step_name(), "external_refresh"); } -#[test] -fn refresh_failure_is_scoped_to_the_matching_auth_snapshot() { +#[tokio::test] +async fn refresh_failure_is_scoped_to_the_matching_auth_snapshot() { let codex_home = tempdir().unwrap(); write_auth_file( AuthFileParams { @@ -268,6 +272,7 @@ fn refresh_failure_is_scoped_to_the_matching_auth_snapshot() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .expect("load auth") .expect("auth available"); let mut updated_auth_dot_json = auth @@ -284,6 +289,7 @@ fn refresh_failure_is_scoped_to_the_matching_auth_snapshot() { updated_auth_dot_json, AuthCredentialsStoreMode::File, ) + .await .expect("updated auth should parse"); let manager = AuthManager::from_auth_for_testing(auth.clone()); @@ -619,9 +625,9 @@ impl Drop for EnvVarGuard { } } -#[test] +#[tokio::test] #[serial(codex_auth_env)] -fn load_auth_reads_agent_identity_from_env() { +async fn load_auth_reads_agent_identity_from_env() { let codex_home = tempdir().unwrap(); let expected_record = agent_identity_record("account-123"); let agent_identity = fake_agent_identity_jwt(&expected_record).expect("fake agent identity"); @@ -632,6 +638,7 @@ fn load_auth_reads_agent_identity_from_env() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .expect("env auth should load") .expect("env auth should be present"); @@ -645,9 +652,9 @@ fn load_auth_reads_agent_identity_from_env() { ); } -#[test] +#[tokio::test] #[serial(codex_auth_env)] -fn load_auth_keeps_codex_api_key_env_precedence() { +async fn load_auth_keeps_codex_api_key_env_precedence() { let codex_home = tempdir().unwrap(); let record = agent_identity_record("account-123"); let agent_identity = fake_agent_identity_jwt(&record).expect("fake agent identity"); @@ -659,6 +666,7 @@ fn load_auth_keeps_codex_api_key_env_precedence() { /*enable_codex_api_key_env*/ true, AuthCredentialsStoreMode::File, ) + .await .expect("env auth should load") .expect("env auth should be present"); @@ -679,8 +687,9 @@ async fn enforce_login_restrictions_logs_out_for_method_mismatch() { ) .await; - let err = - super::enforce_login_restrictions(&config).expect_err("expected method mismatch to error"); + let err = super::enforce_login_restrictions(&config) + .await + .expect_err("expected method mismatch to error"); assert!(err.to_string().contains("ChatGPT login is required")); assert!( !codex_home.path().join("auth.json").exists(), @@ -710,6 +719,7 @@ async fn enforce_login_restrictions_logs_out_for_workspace_mismatch() { .await; let err = super::enforce_login_restrictions(&config) + .await .expect_err("expected workspace mismatch to error"); assert!(err.to_string().contains("workspace org_mine")); assert!( @@ -739,7 +749,9 @@ async fn enforce_login_restrictions_allows_matching_workspace() { ) .await; - super::enforce_login_restrictions(&config).expect("matching workspace should succeed"); + super::enforce_login_restrictions(&config) + .await + .expect("matching workspace should succeed"); assert!( codex_home.path().join("auth.json").exists(), "auth.json should remain when restrictions pass" @@ -761,7 +773,9 @@ async fn enforce_login_restrictions_allows_api_key_if_login_method_not_set_but_f ) .await; - super::enforce_login_restrictions(&config).expect("matching workspace should succeed"); + super::enforce_login_restrictions(&config) + .await + .expect("matching workspace should succeed"); assert!( codex_home.path().join("auth.json").exists(), "auth.json should remain when restrictions pass" @@ -782,6 +796,7 @@ async fn enforce_login_restrictions_blocks_env_api_key_when_chatgpt_required() { .await; let err = super::enforce_login_restrictions(&config) + .await .expect_err("environment API key should not satisfy forced ChatGPT login"); assert!( err.to_string() @@ -818,8 +833,8 @@ fn fake_agent_identity_jwt(record: &AgentIdentityAuthRecord) -> std::io::Result< Ok(format!("{header_b64}.{payload_b64}.{signature_b64}")) } -#[test] -fn plan_type_maps_known_plan() { +#[tokio::test] +async fn plan_type_maps_known_plan() { let codex_home = tempdir().unwrap(); let _jwt = write_auth_file( AuthFileParams { @@ -836,14 +851,15 @@ fn plan_type_maps_known_plan() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .expect("load auth") .expect("auth available"); pretty_assertions::assert_eq!(auth.account_plan_type(), Some(AccountPlanType::Pro)); } -#[test] -fn plan_type_maps_self_serve_business_usage_based_plan() { +#[tokio::test] +async fn plan_type_maps_self_serve_business_usage_based_plan() { let codex_home = tempdir().unwrap(); let _jwt = write_auth_file( AuthFileParams { @@ -860,6 +876,7 @@ fn plan_type_maps_self_serve_business_usage_based_plan() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .expect("load auth") .expect("auth available"); @@ -869,8 +886,8 @@ fn plan_type_maps_self_serve_business_usage_based_plan() { ); } -#[test] -fn plan_type_maps_enterprise_cbp_usage_based_plan() { +#[tokio::test] +async fn plan_type_maps_enterprise_cbp_usage_based_plan() { let codex_home = tempdir().unwrap(); let _jwt = write_auth_file( AuthFileParams { @@ -887,6 +904,7 @@ fn plan_type_maps_enterprise_cbp_usage_based_plan() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .expect("load auth") .expect("auth available"); @@ -896,8 +914,8 @@ fn plan_type_maps_enterprise_cbp_usage_based_plan() { ); } -#[test] -fn plan_type_maps_unknown_to_unknown() { +#[tokio::test] +async fn plan_type_maps_unknown_to_unknown() { let codex_home = tempdir().unwrap(); let _jwt = write_auth_file( AuthFileParams { @@ -914,14 +932,15 @@ fn plan_type_maps_unknown_to_unknown() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .expect("load auth") .expect("auth available"); pretty_assertions::assert_eq!(auth.account_plan_type(), Some(AccountPlanType::Unknown)); } -#[test] -fn missing_plan_type_maps_to_unknown() { +#[tokio::test] +async fn missing_plan_type_maps_to_unknown() { let codex_home = tempdir().unwrap(); let _jwt = write_auth_file( AuthFileParams { @@ -938,6 +957,7 @@ fn missing_plan_type_maps_to_unknown() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, ) + .await .expect("load auth") .expect("auth available"); diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index 85c9b6f024..ad6d644493 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -193,7 +193,7 @@ impl From for std::io::Error { } impl CodexAuth { - fn from_auth_dot_json( + async fn from_auth_dot_json( codex_home: &Path, auth_dot_json: AuthDotJson, auth_credentials_store_mode: AuthCredentialsStoreMode, @@ -234,7 +234,7 @@ impl CodexAuth { } } - pub fn from_auth_storage( + pub async fn from_auth_storage( codex_home: &Path, auth_credentials_store_mode: AuthCredentialsStoreMode, ) -> std::io::Result> { @@ -243,6 +243,7 @@ impl CodexAuth { /*enable_codex_api_key_env*/ false, auth_credentials_store_mode, ) + .await } pub fn from_agent_identity_jwt(jwt: &str) -> std::io::Result { @@ -522,6 +523,7 @@ pub async fn logout_with_revoke( auth_credentials_store_mode, /*chatgpt_base_url*/ None, ) + .await .logout_with_revoke() .await } @@ -609,12 +611,13 @@ pub struct AuthConfig { pub forced_chatgpt_workspace_id: Option, } -pub fn enforce_login_restrictions(config: &AuthConfig) -> std::io::Result<()> { +pub async fn enforce_login_restrictions(config: &AuthConfig) -> std::io::Result<()> { let Some(auth) = load_auth( &config.codex_home, /*enable_codex_api_key_env*/ true, config.auth_credentials_store_mode, - )? + ) + .await? else { return Ok(()); }; @@ -714,15 +717,11 @@ fn logout_all_stores( Ok(removed_ephemeral || removed_managed) } -fn load_auth( +async fn load_auth( codex_home: &Path, enable_codex_api_key_env: bool, auth_credentials_store_mode: AuthCredentialsStoreMode, ) -> std::io::Result> { - let build_auth = |auth_dot_json: AuthDotJson, storage_mode| { - CodexAuth::from_auth_dot_json(codex_home, auth_dot_json, storage_mode) - }; - // API key via env var takes precedence over any other auth method. if enable_codex_api_key_env && let Some(api_key) = read_codex_api_key_from_env() { return Ok(Some(CodexAuth::from_api_key(api_key.as_str()))); @@ -735,7 +734,12 @@ fn load_auth( AuthCredentialsStoreMode::Ephemeral, ); if let Some(auth_dot_json) = ephemeral_storage.load()? { - let auth = build_auth(auth_dot_json, AuthCredentialsStoreMode::Ephemeral)?; + let auth = CodexAuth::from_auth_dot_json( + codex_home, + auth_dot_json, + AuthCredentialsStoreMode::Ephemeral, + ) + .await?; return Ok(Some(auth)); } @@ -755,7 +759,9 @@ fn load_auth( None => return Ok(None), }; - let auth = build_auth(auth_dot_json, auth_credentials_store_mode)?; + let auth = + CodexAuth::from_auth_dot_json(codex_home, auth_dot_json, auth_credentials_store_mode) + .await?; Ok(Some(auth)) } @@ -1169,6 +1175,7 @@ impl UnauthorizedRecovery { match self .manager .reload_if_account_id_matches(self.expected_account_id.as_deref()) + .await { ReloadOutcome::ReloadedChanged => { self.step = UnauthorizedRecoveryStep::RefreshToken; @@ -1279,7 +1286,7 @@ impl AuthManager { /// preferred auth method. Errors loading auth are swallowed; `auth()` will /// simply return `None` in that case so callers can treat it as an /// unauthenticated state. - pub fn new( + pub async fn new( codex_home: PathBuf, enable_codex_api_key_env: bool, auth_credentials_store_mode: AuthCredentialsStoreMode, @@ -1290,6 +1297,7 @@ impl AuthManager { enable_codex_api_key_env, auth_credentials_store_mode, ) + .await .ok() .flatten(); Self { @@ -1402,13 +1410,16 @@ impl AuthManager { /// Force a reload of the auth information from auth.json. Returns /// whether the auth value changed. - pub fn reload(&self) -> bool { + pub async fn reload(&self) -> bool { tracing::info!("Reloading auth"); - let new_auth = self.load_auth_from_storage(); + let new_auth = self.load_auth_from_storage().await; self.set_cached_auth(new_auth) } - fn reload_if_account_id_matches(&self, expected_account_id: Option<&str>) -> ReloadOutcome { + async fn reload_if_account_id_matches( + &self, + expected_account_id: Option<&str>, + ) -> ReloadOutcome { let expected_account_id = match expected_account_id { Some(account_id) => account_id, None => { @@ -1417,7 +1428,7 @@ impl AuthManager { } }; - let new_auth = self.load_auth_from_storage(); + let new_auth = self.load_auth_from_storage().await; let new_account_id = new_auth.as_ref().and_then(CodexAuth::get_account_id); if new_account_id.as_deref() != Some(expected_account_id) { @@ -1488,12 +1499,13 @@ impl AuthManager { } } - fn load_auth_from_storage(&self) -> Option { + async fn load_auth_from_storage(&self) -> Option { load_auth( &self.codex_home, self.enable_codex_api_key_env, self.auth_credentials_store_mode, ) + .await .ok() .flatten() } @@ -1557,22 +1569,25 @@ impl AuthManager { } /// Convenience constructor returning an `Arc` wrapper. - pub fn shared( + pub async fn shared( codex_home: PathBuf, enable_codex_api_key_env: bool, auth_credentials_store_mode: AuthCredentialsStoreMode, chatgpt_base_url: Option, ) -> Arc { - Arc::new(Self::new( - codex_home, - enable_codex_api_key_env, - auth_credentials_store_mode, - chatgpt_base_url, - )) + Arc::new( + Self::new( + codex_home, + enable_codex_api_key_env, + auth_credentials_store_mode, + chatgpt_base_url, + ) + .await, + ) } /// Convenience constructor returning an `Arc` wrapper from resolved config. - pub fn shared_from_config( + pub async fn shared_from_config( config: &impl AuthManagerConfig, enable_codex_api_key_env: bool, ) -> Arc { @@ -1581,7 +1596,8 @@ impl AuthManager { enable_codex_api_key_env, config.cli_auth_credentials_store_mode(), Some(config.chatgpt_base_url()), - ); + ) + .await; auth_manager.set_forced_chatgpt_workspace_id(config.forced_chatgpt_workspace_id()); auth_manager } @@ -1647,7 +1663,10 @@ impl AuthManager { .as_ref() .and_then(CodexAuth::get_account_id); - match self.reload_if_account_id_matches(expected_account_id.as_deref()) { + match self + .reload_if_account_id_matches(expected_account_id.as_deref()) + .await + { ReloadOutcome::ReloadedChanged => { tracing::info!("Skipping token refresh because auth changed after guarded reload."); Ok(()) @@ -1714,10 +1733,10 @@ impl AuthManager { /// if a file was removed, Ok(false) if no auth file existed. On success, /// reloads the in‑memory auth cache so callers immediately observe the /// unauthenticated state. - pub fn logout(&self) -> std::io::Result { + pub async fn logout(&self) -> std::io::Result { let removed = logout_all_stores(&self.codex_home, self.auth_credentials_store_mode)?; // Always reload to clear any cached auth (even if file absent). - self.reload(); + self.reload().await; Ok(removed) } @@ -1730,7 +1749,7 @@ impl AuthManager { } let result = logout_all_stores(&self.codex_home, self.auth_credentials_store_mode)?; // Always reload to clear any cached auth (even if file absent). - self.reload(); + self.reload().await; Ok(result) } @@ -1826,7 +1845,7 @@ impl AuthManager { AuthCredentialsStoreMode::Ephemeral, ) .map_err(RefreshTokenError::Transient)?; - self.reload(); + self.reload().await; Ok(()) } @@ -1846,7 +1865,7 @@ impl AuthManager { refresh_response.refresh_token, ) .map_err(RefreshTokenError::from)?; - self.reload(); + self.reload().await; Ok(()) } diff --git a/codex-rs/login/tests/suite/auth_refresh.rs b/codex-rs/login/tests/suite/auth_refresh.rs index 3ae0eb5fa1..afdf466d09 100644 --- a/codex-rs/login/tests/suite/auth_refresh.rs +++ b/codex-rs/login/tests/suite/auth_refresh.rs @@ -46,7 +46,7 @@ async fn refresh_token_succeeds_updates_storage() -> Result<()> { .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -56,7 +56,7 @@ async fn refresh_token_succeeds_updates_storage() -> Result<()> { last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; ctx.auth_manager .refresh_token_from_authority() @@ -110,7 +110,7 @@ async fn refresh_token_refreshes_when_auth_is_unchanged() -> Result<()> { .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -120,7 +120,7 @@ async fn refresh_token_refreshes_when_auth_is_unchanged() -> Result<()> { last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; ctx.auth_manager .refresh_token() @@ -164,7 +164,7 @@ async fn refresh_token_skips_refresh_when_auth_changed() -> Result<()> { skip_if_no_network!(Ok(())); let server = MockServer::start().await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); @@ -175,7 +175,7 @@ async fn refresh_token_skips_refresh_when_auth_changed() -> Result<()> { last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let disk_tokens = build_tokens("disk-access-token", "disk-refresh-token"); let disk_auth = AuthDotJson { @@ -230,7 +230,7 @@ async fn refresh_token_errors_on_account_mismatch() -> Result<()> { .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -240,7 +240,7 @@ async fn refresh_token_errors_on_account_mismatch() -> Result<()> { last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let mut disk_tokens = build_tokens("disk-access-token", "disk-refresh-token"); disk_tokens.account_id = Some("other-account".to_string()); @@ -299,7 +299,7 @@ async fn returns_fresh_tokens_as_is() -> Result<()> { .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let stale_refresh = Utc::now() - Duration::days(9); let fresh_access_token = access_token_with_expiration(Utc::now() + Duration::hours(1)); let initial_tokens = build_tokens(&fresh_access_token, INITIAL_REFRESH_TOKEN); @@ -310,7 +310,7 @@ async fn returns_fresh_tokens_as_is() -> Result<()> { last_refresh: Some(stale_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let cached_auth = ctx .auth_manager @@ -347,7 +347,7 @@ async fn refreshes_token_when_access_token_is_expired() -> Result<()> { .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let fresh_refresh = Utc::now() - Duration::days(1); let expired_access_token = access_token_with_expiration(Utc::now() - Duration::hours(1)); let initial_tokens = build_tokens(&expired_access_token, INITIAL_REFRESH_TOKEN); @@ -358,7 +358,7 @@ async fn refreshes_token_when_access_token_is_expired() -> Result<()> { last_refresh: Some(fresh_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let cached_auth = ctx .auth_manager @@ -398,7 +398,7 @@ async fn auth_reloads_disk_auth_when_cached_auth_is_stale() -> Result<()> { let server = MockServer::start().await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let stale_refresh = Utc::now() - Duration::days(9); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -408,7 +408,7 @@ async fn auth_reloads_disk_auth_when_cached_auth_is_stale() -> Result<()> { last_refresh: Some(stale_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let fresh_refresh = Utc::now() - Duration::days(1); let disk_tokens = build_tokens("disk-access-token", "disk-refresh-token"); @@ -461,7 +461,7 @@ async fn auth_reloads_disk_auth_without_calling_expired_refresh_token() -> Resul .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let stale_refresh = Utc::now() - Duration::days(9); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -471,7 +471,7 @@ async fn auth_reloads_disk_auth_without_calling_expired_refresh_token() -> Resul last_refresh: Some(stale_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let fresh_refresh = Utc::now() - Duration::days(1); let disk_tokens = build_tokens("disk-access-token", "disk-refresh-token"); @@ -522,7 +522,7 @@ async fn refresh_token_returns_permanent_error_for_expired_refresh_token() -> Re .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -532,7 +532,7 @@ async fn refresh_token_returns_permanent_error_for_expired_refresh_token() -> Re last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let err = ctx .auth_manager @@ -575,7 +575,7 @@ async fn refresh_token_does_not_retry_after_permanent_failure() -> Result<()> { .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -585,7 +585,7 @@ async fn refresh_token_does_not_retry_after_permanent_failure() -> Result<()> { last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let first_err = ctx .auth_manager @@ -642,7 +642,7 @@ async fn refresh_token_reloads_changed_auth_after_permanent_failure() -> Result< .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -652,7 +652,7 @@ async fn refresh_token_reloads_changed_auth_after_permanent_failure() -> Result< last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let first_err = ctx .auth_manager @@ -723,7 +723,7 @@ async fn refresh_token_returns_transient_error_on_server_failure() -> Result<()> .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -733,7 +733,7 @@ async fn refresh_token_returns_transient_error_on_server_failure() -> Result<()> last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let err = ctx .auth_manager @@ -776,7 +776,7 @@ async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> { .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -786,7 +786,7 @@ async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> { last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let disk_tokens = build_tokens("disk-access-token", "disk-refresh-token"); let disk_auth = AuthDotJson { @@ -870,7 +870,7 @@ async fn unauthorized_recovery_errors_on_account_mismatch() -> Result<()> { .mount(&server) .await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let initial_last_refresh = Utc::now() - Duration::days(1); let initial_tokens = build_tokens(INITIAL_ACCESS_TOKEN, INITIAL_REFRESH_TOKEN); let initial_auth = AuthDotJson { @@ -880,7 +880,7 @@ async fn unauthorized_recovery_errors_on_account_mismatch() -> Result<()> { last_refresh: Some(initial_last_refresh), agent_identity: None, }; - ctx.write_auth(&initial_auth)?; + ctx.write_auth(&initial_auth).await?; let mut disk_tokens = build_tokens("disk-access-token", "disk-refresh-token"); disk_tokens.account_id = Some("other-account".to_string()); @@ -941,7 +941,7 @@ async fn unauthorized_recovery_requires_chatgpt_auth() -> Result<()> { skip_if_no_network!(Ok(())); let server = MockServer::start().await; - let ctx = RefreshTokenTestContext::new(&server)?; + let ctx = RefreshTokenTestContext::new(&server).await?; let auth = AuthDotJson { auth_mode: Some(AuthMode::ApiKey), openai_api_key: Some("sk-test".to_string()), @@ -949,7 +949,7 @@ async fn unauthorized_recovery_requires_chatgpt_auth() -> Result<()> { last_refresh: None, agent_identity: None, }; - ctx.write_auth(&auth)?; + ctx.write_auth(&auth).await?; let mut recovery = ctx.auth_manager.unauthorized_recovery(); assert!(!recovery.has_next()); @@ -974,7 +974,7 @@ struct RefreshTokenTestContext { } impl RefreshTokenTestContext { - fn new(server: &MockServer) -> Result { + async fn new(server: &MockServer) -> Result { let codex_home = TempDir::new()?; let endpoint = format!("{}/oauth/token", server.uri()); @@ -985,7 +985,8 @@ impl RefreshTokenTestContext { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; Ok(Self { codex_home, @@ -1000,13 +1001,13 @@ impl RefreshTokenTestContext { .context("auth.json should exist") } - fn write_auth(&self, auth_dot_json: &AuthDotJson) -> Result<()> { + async fn write_auth(&self, auth_dot_json: &AuthDotJson) -> Result<()> { save_auth( self.codex_home.path(), auth_dot_json, AuthCredentialsStoreMode::File, )?; - self.auth_manager.reload(); + self.auth_manager.reload().await; Ok(()) } } diff --git a/codex-rs/login/tests/suite/logout.rs b/codex-rs/login/tests/suite/logout.rs index e703b15eb1..2364ee7f54 100644 --- a/codex-rs/login/tests/suite/logout.rs +++ b/codex-rs/login/tests/suite/logout.rs @@ -143,7 +143,8 @@ async fn auth_manager_logout_with_revoke_uses_cached_auth() -> Result<()> { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; save_auth( codex_home.path(), &chatgpt_auth_with_refresh_token("newer-disk-refresh-token"), diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index 1d904e4577..95fc4341ee 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -141,7 +141,8 @@ pub async fn run_main( arg0_paths, Arc::new(config), environment_manager, - ); + ) + .await; async move { while let Some(msg) = incoming_rx.recv().await { match msg { diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index e0a1e77f68..32d05d1f93 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -49,7 +49,7 @@ pub(crate) struct MessageProcessor { impl MessageProcessor { /// Create a new `MessageProcessor`, retaining a handle to the outgoing /// `Sender` so handlers can enqueue messages to be written to stdout. - pub(crate) fn new( + pub(crate) async fn new( outgoing: OutgoingMessageSender, arg0_paths: Arg0DispatchPaths, config: Arc, @@ -59,7 +59,8 @@ impl MessageProcessor { let auth_manager = AuthManager::shared_from_config( config.as_ref(), /*enable_codex_api_key_env*/ false, - ); + ) + .await; let thread_manager = Arc::new(ThreadManager::new( config.as_ref(), auth_manager, diff --git a/codex-rs/models-manager/src/manager_tests.rs b/codex-rs/models-manager/src/manager_tests.rs index 4046b7565f..57c42be015 100644 --- a/codex-rs/models-manager/src/manager_tests.rs +++ b/codex-rs/models-manager/src/manager_tests.rs @@ -206,7 +206,7 @@ fn static_manager_for_tests(model_catalog: ModelsResponse) -> StaticModelsManage ) } -fn chatgpt_auth_tokens_for_tests(codex_home: &Path) -> CodexAuth { +async fn chatgpt_auth_tokens_for_tests(codex_home: &Path) -> CodexAuth { let auth_dot_json = codex_login::AuthDotJson { auth_mode: Some(AuthMode::ChatgptAuthTokens), openai_api_key: None, @@ -232,6 +232,7 @@ c2ln", .expect("auth.json should be written"); CodexAuth::from_auth_storage(codex_home, AuthCredentialsStoreMode::File) + .await .expect("auth should load") .expect("auth should be present") } @@ -685,7 +686,7 @@ async fn refresh_available_models_fetches_with_chatgpt_auth_tokens() { "ChatGPT Auth Tokens", /*priority*/ 1, )]]); - let auth = chatgpt_auth_tokens_for_tests(codex_home.path()); + let auth = chatgpt_auth_tokens_for_tests(codex_home.path()).await; let manager = openai_manager_for_tests_with_auth( codex_home.path().to_path_buf(), endpoint.clone(), diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index f7d24e1e0e..d9d3b0c026 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -30,6 +30,9 @@ use codex_app_server_protocol::AdditionalPermissionProfile; use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::CommandExecutionRequestApprovalParams; use codex_app_server_protocol::ConfigWarningNotification; +use codex_app_server_protocol::FileChangeRequestApprovalParams; +use codex_app_server_protocol::FileUpdateChange; +use codex_app_server_protocol::ItemStartedNotification; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::McpServerStartupState; use codex_app_server_protocol::McpServerStatusUpdatedNotification; @@ -38,6 +41,7 @@ use codex_app_server_protocol::NetworkApprovalProtocol as AppServerNetworkApprov use codex_app_server_protocol::NetworkPolicyAmendment as AppServerNetworkPolicyAmendment; use codex_app_server_protocol::NetworkPolicyRuleAction as AppServerNetworkPolicyRuleAction; use codex_app_server_protocol::NonSteerableTurnKind as AppServerNonSteerableTurnKind; +use codex_app_server_protocol::PatchChangeKind; use codex_app_server_protocol::PermissionsRequestApprovalParams; use codex_app_server_protocol::RequestId as AppServerRequestId; use codex_app_server_protocol::ServerNotification; @@ -70,6 +74,7 @@ use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::protocol::RolloutItem; @@ -2522,6 +2527,75 @@ async fn inactive_thread_exec_approval_splits_shell_wrapped_command() { ); } +#[tokio::test] +async fn inactive_thread_file_change_approval_recovers_buffered_changes() { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let thread_id = ThreadId::new(); + app.enqueue_thread_notification( + thread_id, + ServerNotification::ItemStarted(ItemStartedNotification { + thread_id: thread_id.to_string(), + turn_id: "turn-approval".to_string(), + item: ThreadItem::FileChange { + id: "patch-approval".to_string(), + changes: vec![FileUpdateChange { + path: "README.md".to_string(), + kind: PatchChangeKind::Add, + diff: "hello\n".to_string(), + }], + status: codex_app_server_protocol::PatchApplyStatus::InProgress, + }, + }), + ) + .await + .expect("enqueue file change item"); + + let request = ServerRequest::FileChangeRequestApproval { + request_id: AppServerRequestId::Integer(9), + params: FileChangeRequestApprovalParams { + thread_id: thread_id.to_string(), + turn_id: "turn-approval".to_string(), + item_id: "patch-approval".to_string(), + reason: Some("command failed; retry without sandbox?".to_string()), + grant_root: None, + }, + }; + + let request = app + .interactive_request_for_thread_request(thread_id, &request) + .await + .expect("expected file change approval request"); + + let ThreadInteractiveRequest::Approval(ApprovalRequest::ApplyPatch { + changes, reason, .. + }) = &request + else { + panic!("expected apply-patch approval request"); + }; + assert_eq!( + changes, + &HashMap::from([( + PathBuf::from("README.md"), + FileChange::Add { + content: "hello\n".to_string(), + }, + )]) + ); + assert_eq!( + reason, + &Some("command failed; retry without sandbox?".to_string()) + ); + + app.push_thread_interactive_request(request); + let cell = match app_event_rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected patch preview history cell, saw {other:?}"), + }; + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80)); + assert!(rendered.contains("• Added README.md (+1 -0)")); + assert!(rendered.contains("1 +hello")); +} + #[tokio::test] async fn inactive_thread_permissions_approval_preserves_file_system_permissions() { let app = make_test_app().await; diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs index daf743b467..9bbb41fbd1 100644 --- a/codex-rs/tui/src/app/thread_events.rs +++ b/codex-rs/tui/src/app/thread_events.rs @@ -157,6 +157,40 @@ impl ThreadEventStore { .collect() } + pub(super) fn file_change_changes( + &self, + turn_id: &str, + item_id: &str, + ) -> Option> { + self.buffer + .iter() + .rev() + .find_map(|event| match event { + ThreadBufferedEvent::Notification(ServerNotification::ItemStarted( + notification, + )) if turn_id_matches(turn_id, ¬ification.turn_id) => { + file_change_item_changes(¬ification.item, item_id) + } + ThreadBufferedEvent::Notification(ServerNotification::ItemCompleted( + notification, + )) if turn_id_matches(turn_id, ¬ification.turn_id) => { + file_change_item_changes(¬ification.item, item_id) + } + ThreadBufferedEvent::Request(_) + | ThreadBufferedEvent::Notification(_) + | ThreadBufferedEvent::HistoryEntryResponse(_) + | ThreadBufferedEvent::FeedbackSubmission(_) => None, + }) + .or_else(|| { + self.turns + .iter() + .rev() + .filter(|turn| turn_id_matches(turn_id, &turn.id)) + .flat_map(|turn| turn.items.iter().rev()) + .find_map(|item| file_change_item_changes(item, item_id)) + }) + } + pub(super) fn apply_thread_rollback(&mut self, response: &ThreadRollbackResponse) { self.turns = response.thread.turns.clone(); self.buffer.clear(); @@ -231,6 +265,20 @@ impl ThreadEventStore { } } +fn turn_id_matches(request_turn_id: &str, candidate_turn_id: &str) -> bool { + request_turn_id.is_empty() || request_turn_id == candidate_turn_id +} + +fn file_change_item_changes( + item: &ThreadItem, + item_id: &str, +) -> Option> { + match item { + ThreadItem::FileChange { id, changes, .. } if id == item_id => Some(changes.clone()), + _ => None, + } +} + #[derive(Debug)] pub(super) struct ThreadEventChannel { pub(super) sender: mpsc::Sender, diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 5f0f52c2c7..345e560d41 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -194,6 +194,17 @@ impl App { store.session.as_ref().map(|session| session.cwd.clone()) } + async fn thread_file_change_changes( + &self, + thread_id: ThreadId, + turn_id: &str, + item_id: &str, + ) -> Option> { + let channel = self.thread_event_channels.get(&thread_id)?; + let store = channel.store.lock().await; + store.file_change_changes(turn_id, item_id) + } + pub(super) async fn interactive_request_for_thread_request( &self, thread_id: ThreadId, @@ -264,7 +275,11 @@ impl App { .thread_cwd(thread_id) .await .unwrap_or_else(|| self.config.cwd.clone()), - changes: HashMap::new(), + changes: self + .thread_file_change_changes(thread_id, ¶ms.turn_id, ¶ms.item_id) + .await + .map(crate::app_server_approval_conversions::file_update_changes_to_core) + .unwrap_or_default(), }), ), ServerRequest::McpServerElicitationRequest { request_id, params } => { @@ -311,6 +326,7 @@ impl App { pub(super) fn push_thread_interactive_request(&mut self, request: ThreadInteractiveRequest) { match request { ThreadInteractiveRequest::Approval(request) => { + self.render_inactive_patch_preview(&request); self.chat_widget.push_approval_request(request); } ThreadInteractiveRequest::McpServerElicitation(request) => { @@ -320,6 +336,23 @@ impl App { } } + fn render_inactive_patch_preview(&mut self, request: &ApprovalRequest) { + let ApprovalRequest::ApplyPatch { + thread_label, + cwd, + changes, + .. + } = request + else { + return; + }; + if thread_label.is_none() || changes.is_empty() { + return; + } + self.chat_widget + .add_to_history(history_cell::new_patch_event(changes.clone(), cwd)); + } + pub(super) async fn pending_inactive_thread_requests(&self) -> Vec<(ThreadId, ServerRequest)> { let channels: Vec<(ThreadId, Arc>)> = self .thread_event_channels diff --git a/codex-rs/tui/src/app_backtrack.rs b/codex-rs/tui/src/app_backtrack.rs index da1f82e626..2afcaa3b53 100644 --- a/codex-rs/tui/src/app_backtrack.rs +++ b/codex-rs/tui/src/app_backtrack.rs @@ -10,7 +10,8 @@ //! //! Backtrack operates as a small state machine: //! - The first `Esc` in the main view "primes" the feature and captures a base thread id. -//! - A subsequent `Esc` opens the transcript overlay (`Ctrl+T`) and highlights a user message. +//! - A subsequent `Esc` opens the transcript overlay (`Ctrl+T`) and highlights a user message when +//! there is a rewind target. //! - `Enter` requests a rollback from core and records a `pending_rollback` guard. //! - On `EventMsg::ThreadRolledBack`, we either finish an in-flight backtrack request or queue a //! rollback trim so it runs in event order with transcript inserts. @@ -44,6 +45,8 @@ use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; +const NO_PREVIOUS_MESSAGE_TO_EDIT: &str = "No previous message to edit."; + /// Aggregates all backtrack-related state used by the App. #[derive(Default)] pub(crate) struct BacktrackState { @@ -266,11 +269,21 @@ impl App { self.backtrack.primed = true; self.backtrack.nth_user_message = usize::MAX; self.backtrack.base_id = self.chat_widget.thread_id(); - self.chat_widget.show_esc_backtrack_hint(); + if has_backtrack_target(&self.transcript_cells) { + self.chat_widget.show_esc_backtrack_hint(); + } } /// Open overlay and begin backtrack preview flow (first step + highlight). fn open_backtrack_preview(&mut self, tui: &mut tui::Tui) { + if !has_backtrack_target(&self.transcript_cells) { + self.reset_backtrack_state(); + self.chat_widget + .add_info_message(NO_PREVIOUS_MESSAGE_TO_EDIT.to_string(), /*hint*/ None); + tui.frame_requester().schedule_frame(); + return; + } + self.open_transcript_overlay(tui); self.backtrack.overlay_preview_active = true; // Composer is hidden by overlay; clear its hint. @@ -280,6 +293,14 @@ impl App { /// When overlay is already open, begin preview mode and select latest user message. fn begin_overlay_backtrack_preview(&mut self, tui: &mut tui::Tui) { + if !has_backtrack_target(&self.transcript_cells) { + self.close_transcript_overlay(tui); + self.chat_widget + .add_info_message(NO_PREVIOUS_MESSAGE_TO_EDIT.to_string(), /*hint*/ None); + tui.frame_requester().schedule_frame(); + return; + } + self.backtrack.primed = true; self.backtrack.base_id = self.chat_widget.thread_id(); self.backtrack.overlay_preview_active = true; @@ -613,6 +634,10 @@ pub(crate) fn user_count(cells: &[Arc]) -> user_positions_iter(cells).count() } +fn has_backtrack_target(cells: &[Arc]) -> bool { + user_count(cells) > 0 +} + fn nth_user_position( cells: &[Arc], nth: usize, @@ -674,9 +699,22 @@ mod tests { use super::*; use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; + use pretty_assertions::assert_eq; use ratatui::prelude::Line; use std::sync::Arc; + fn render_lines(lines: &[Line<'static>]) -> Vec { + lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect() + } + #[test] fn trim_transcript_for_first_user_drops_user_and_newer_cells() { let mut cells: Vec> = vec![ @@ -885,4 +923,40 @@ mod tests { assert_eq!(agent_group_count(&cells), 2); } + + #[test] + fn backtrack_target_requires_user_message() { + let mut cells: Vec> = vec![ + Arc::new(AgentMessageCell::new( + vec![Line::from("assistant")], + /*is_first_line*/ true, + )) as Arc, + Arc::new(crate::history_cell::new_info_event( + "Context compacted".to_string(), + /*hint*/ None, + )) as Arc, + ]; + + assert!(!has_backtrack_target(&cells)); + + cells.push(Arc::new(UserHistoryCell { + message: "hello".to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + }) as Arc); + + assert!(has_backtrack_target(&cells)); + } + + #[test] + fn backtrack_unavailable_info_message_snapshot() { + let cell = crate::history_cell::new_info_event( + NO_PREVIOUS_MESSAGE_TO_EDIT.to_string(), + /*hint*/ None, + ); + let rendered = render_lines(&cell.display_lines(/*width*/ 80)).join("\n"); + + insta::assert_snapshot!(rendered); + } } diff --git a/codex-rs/tui/src/app_server_approval_conversions.rs b/codex-rs/tui/src/app_server_approval_conversions.rs index a0d86db7d6..894bd36ed7 100644 --- a/codex-rs/tui/src/app_server_approval_conversions.rs +++ b/codex-rs/tui/src/app_server_approval_conversions.rs @@ -1,9 +1,14 @@ use codex_app_server_protocol::AdditionalNetworkPermissions; +use codex_app_server_protocol::FileUpdateChange; use codex_app_server_protocol::GrantedPermissionProfile; use codex_app_server_protocol::NetworkApprovalContext as AppServerNetworkApprovalContext; +use codex_app_server_protocol::PatchChangeKind; +use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile; +use std::collections::HashMap; +use std::path::PathBuf; pub(crate) fn network_approval_context_to_core( value: AppServerNetworkApprovalContext, @@ -38,21 +43,50 @@ pub(crate) fn granted_permission_profile_from_request( } } +pub(crate) fn file_update_changes_to_core( + changes: Vec, +) -> HashMap { + changes + .into_iter() + .map(|change| { + let path = PathBuf::from(change.path); + let file_change = match change.kind { + PatchChangeKind::Add => FileChange::Add { + content: change.diff, + }, + PatchChangeKind::Delete => FileChange::Delete { + content: change.diff, + }, + PatchChangeKind::Update { move_path } => FileChange::Update { + unified_diff: change.diff, + move_path, + }, + }; + (path, file_change) + }) + .collect() +} + #[cfg(test)] mod tests { + use super::file_update_changes_to_core; use super::granted_permission_profile_from_request; use super::network_approval_context_to_core; + use codex_app_server_protocol::FileUpdateChange; + use codex_app_server_protocol::PatchChangeKind; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSpecialPath; + use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + use std::collections::HashMap; use std::path::PathBuf; fn absolute_path(path: &str) -> AbsolutePathBuf { @@ -73,6 +107,23 @@ mod tests { ); } + #[test] + fn converts_file_update_changes_to_core() { + assert_eq!( + file_update_changes_to_core(vec![FileUpdateChange { + path: "foo.txt".to_string(), + kind: PatchChangeKind::Add, + diff: "hello\n".to_string(), + }]), + HashMap::from([( + PathBuf::from("foo.txt"), + FileChange::Add { + content: "hello\n".to_string(), + }, + )]) + ); + } + #[test] fn converts_request_permissions_into_granted_permissions() { assert_eq!( diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 57d819e560..ce3b04b33b 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -9,7 +9,6 @@ use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::list_selection_view::ListSelectionView; use crate::bottom_pane::list_selection_view::SelectionItem; use crate::bottom_pane::list_selection_view::SelectionViewParams; -use crate::diff_render::DiffSummary; use crate::exec_command::strip_bash_lc_and_escape; use crate::history_cell; use crate::key_hint; @@ -633,8 +632,6 @@ fn build_header(request: &ApprovalRequest) -> Box { ApprovalRequest::ApplyPatch { thread_label, reason, - cwd, - changes, .. } => { let mut header: Vec> = Vec::new(); @@ -643,11 +640,13 @@ fn build_header(request: &ApprovalRequest) -> Box { "Thread: ".into(), thread_label.clone().bold(), ]))); - header.push(Box::new(Line::from(""))); } if let Some(reason) = reason && !reason.is_empty() { + if !header.is_empty() { + header.push(Box::new(Line::from(""))); + } header.push(Box::new( Paragraph::new(Line::from_iter([ "Reason: ".into(), @@ -655,9 +654,7 @@ fn build_header(request: &ApprovalRequest) -> Box { ])) .wrap(Wrap { trim: false }), )); - header.push(Box::new(Line::from(""))); } - header.push(DiffSummary::new(changes.clone(), cwd.clone()).into()); Box::new(ColumnRenderable::with(header)) } ApprovalRequest::McpElicitation { @@ -1556,6 +1553,32 @@ mod tests { ); } + #[test] + fn apply_patch_prompt_with_thread_label_omits_command_line() { + let (tx, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let mut changes = HashMap::new(); + changes.insert( + PathBuf::from("bug1.txt"), + FileChange::Add { + content: "one\ntwo\nthree\n".to_string(), + }, + ); + let request = ApprovalRequest::ApplyPatch { + thread_id: ThreadId::new(), + thread_label: Some("Banach [worker]".to_string()), + id: "test".to_string(), + reason: None, + cwd: absolute_path("/tmp"), + changes, + }; + let view = ApprovalOverlay::new(request, tx, Features::with_defaults()); + let rendered = render_overlay_lines(&view, /*width*/ 120); + assert!(rendered.contains("Thread: Banach [worker]")); + assert!(rendered.contains("o to open thread")); + assert!(!rendered.contains("$ apply_patch")); + } + #[test] fn network_exec_prompt_title_includes_host() { let (tx, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 9b67e29b57..1a0e39d9c3 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -1160,7 +1160,7 @@ impl ChatComposer { /// Convert canonical composer text into the textarea's internal representation. /// - /// Bash mode stores the leading `!` as prompt state instead of editable text, + /// Shell mode stores the leading `!` as prompt state instead of editable text, /// so full-buffer imports must absorb that prefix before rebuilding the textarea. fn imported_text_for_textarea( &mut self, @@ -2934,7 +2934,7 @@ impl ChatComposer { fn shell_mode_footer_line(&self) -> Option> { self.is_bang_shell_command() .then_some(()) - .map(|_| Line::from(vec![Span::from("Bash mode").light_red()])) + .map(|_| Line::from(vec![Span::from("Shell mode").light_red()])) } /// Applies any due `PasteBurst` flush at time `now`. @@ -4660,7 +4660,7 @@ mod tests { } #[test] - fn shell_command_uses_bash_accent_style() { + fn shell_command_uses_shell_accent_style() { let (tx, _rx) = unbounded_channel::(); let sender = AppEventSender::new(tx); let mut composer = ChatComposer::new( @@ -4688,11 +4688,11 @@ mod tests { let footer_text = (0..area.width) .map(|x| buf[(x, footer_y)].symbol().chars().next().unwrap_or(' ')) .collect::(); - let bash_label_x = footer_text - .find("Bash mode") - .expect("expected bash mode footer label"); + let shell_label_x = footer_text + .find("Shell mode") + .expect("expected shell mode footer label"); assert_eq!( - buf[(bash_label_x as u16, footer_y)].style().fg, + buf[(shell_label_x as u16, footer_y)].style().fg, Some(Color::LightRed) ); } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_shell_command_absorbs_bang.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_shell_command_absorbs_bang.snap index 2b44670df0..0b09860b59 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_shell_command_absorbs_bang.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_shell_command_absorbs_bang.snap @@ -10,4 +10,4 @@ expression: terminal.backend() " " " " " " -" gpt-5.4 high fast · ~/code/codex-1 · Context 0% used Bash mode " +" gpt-5.4 high fast · ~/code/codex-1 · Context 0% used Shell mode " diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 1c08c77486..8513fe8816 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -47,6 +47,7 @@ use self::realtime::PendingSteerCompareKey; use crate::app::app_server_requests::ResolvedAppServerRequest; use crate::app_command::AppCommand; use crate::app_event::RealtimeAudioDeviceKind; +use crate::app_server_approval_conversions::file_update_changes_to_core; use crate::app_server_approval_conversions::network_approval_context_to_core; use crate::app_server_session::ThreadSessionState; #[cfg(not(target_os = "linux"))] @@ -1774,36 +1775,6 @@ fn patch_approval_request_from_params( } } -fn app_server_patch_changes_to_core( - changes: Vec, -) -> HashMap { - changes - .into_iter() - .map(|change| { - let path = PathBuf::from(change.path); - let file_change = match change.kind { - codex_app_server_protocol::PatchChangeKind::Add => { - codex_protocol::protocol::FileChange::Add { - content: change.diff, - } - } - codex_app_server_protocol::PatchChangeKind::Delete => { - codex_protocol::protocol::FileChange::Delete { - content: change.diff, - } - } - codex_app_server_protocol::PatchChangeKind::Update { move_path } => { - codex_protocol::protocol::FileChange::Update { - unified_diff: change.diff, - move_path, - } - } - }; - (path, file_change) - }) - .collect() -} - fn app_server_collab_thread_id_to_core(thread_id: &str) -> Option { match ThreadId::from_string(thread_id) { Ok(thread_id) => Some(thread_id), @@ -6151,9 +6122,26 @@ impl ChatWidget { } } + fn submit_shell_command_with_history( + &mut self, + command: &str, + history_text: &str, + ) -> QueueDrain { + let drain = self.submit_shell_command(command); + if drain == QueueDrain::Stop { + self.submit_op(Op::AddToHistory { + text: history_text.to_string(), + }); + } + drain + } + fn submit_queued_shell_prompt(&mut self, user_message: UserMessage) -> QueueDrain { match user_message.text.strip_prefix('!') { - Some(command) => self.submit_shell_command(command), + Some(command) => { + let history_text = user_message.text.clone(); + self.submit_shell_command_with_history(command, &history_text) + } None => { self.submit_user_message(user_message); QueueDrain::Stop @@ -6249,7 +6237,7 @@ impl ChatWidget { if shell_escape_policy == ShellEscapePolicy::Allow && let Some(stripped) = text.strip_prefix('!') { - let app_command = match self.submit_shell_command(stripped) { + let app_command = match self.submit_shell_command_with_history(stripped, &text) { QueueDrain::Continue => None, QueueDrain::Stop => Some(AppCommand::run_user_shell_command( stripped.trim().to_string(), @@ -6780,7 +6768,7 @@ impl ChatWidget { status, codex_app_server_protocol::PatchApplyStatus::Failed ), - changes: app_server_patch_changes_to_core(changes), + changes: file_update_changes_to_core(changes), status: match status { codex_app_server_protocol::PatchApplyStatus::Completed => { codex_protocol::protocol::PatchApplyStatus::Completed @@ -7339,7 +7327,7 @@ impl ChatWidget { call_id: id, turn_id: notification.turn_id, auto_approved: false, - changes: app_server_patch_changes_to_core(changes), + changes: file_update_changes_to_core(changes), }); } ThreadItem::McpToolCall { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap index e394605dcc..8635b66682 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap @@ -1,18 +1,11 @@ --- -source: tui/src/chatwidget/tests.rs -expression: terminal.backend().vt100().screen().contents() +source: tui/src/chatwidget/tests/exec_flow.rs +expression: contents --- - - Would you like to make the following edits? Reason: The model wants to apply changes - README.md (+2 -0) - - 1 +hello - 2 +world - › 1. Yes, proceed (y) 2. Yes, and don't ask again for these files (a) 3. No, and tell Codex what to do differently (esc) diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index b9ec1d871b..d4fa2df451 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -349,25 +349,6 @@ async fn live_app_server_command_execution_strips_shell_wrapper() { ); } -#[test] -fn app_server_patch_changes_to_core_preserves_diffs() { - let changes = app_server_patch_changes_to_core(vec![FileUpdateChange { - path: "foo.txt".to_string(), - kind: PatchChangeKind::Add, - diff: "hello\n".to_string(), - }]); - - assert_eq!( - changes, - HashMap::from([( - PathBuf::from("foo.txt"), - FileChange::Add { - content: "hello\n".to_string(), - }, - )]) - ); -} - #[tokio::test] async fn live_app_server_collab_wait_items_render_history() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index 206564d06a..4d87914aa4 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -1038,6 +1038,10 @@ async fn bang_shell_enter_while_task_running_submits_run_user_shell_command() { Ok(Op::RunUserShellCommand { command }) => assert_eq!(command, "echo hi"), other => panic!("expected RunUserShellCommand op, got {other:?}"), } + assert_matches!( + op_rx.try_recv(), + Ok(Op::AddToHistory { text }) if text == "!echo hi" + ); assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); } @@ -1317,10 +1321,9 @@ async fn approval_modal_patch_snapshot() -> anyhow::Result<()> { terminal .draw(|f| chat.render(f.area(), f.buffer_mut())) .expect("draw patch approval modal"); - assert_chatwidget_snapshot!( - "approval_modal_patch", - terminal.backend().vt100().screen().contents() - ); + let contents = terminal.backend().vt100().screen().contents(); + assert!(!contents.contains("$ apply_patch")); + assert_chatwidget_snapshot!("approval_modal_patch", contents); Ok(()) } @@ -1465,28 +1468,11 @@ async fn apply_patch_events_emit_history_cells() { id: "s1".into(), msg: EventMsg::ApplyPatchApprovalRequest(ev), }); - let cells = drain_insert_history(&mut rx); assert!( - cells.is_empty(), + drain_insert_history(&mut rx).is_empty(), "expected approval request to surface via modal without emitting history cells" ); - let area = Rect::new(0, 0, 80, chat.desired_height(/*width*/ 80)); - let mut buf = ratatui::buffer::Buffer::empty(area); - chat.render(area, &mut buf); - let mut saw_summary = false; - for y in 0..area.height { - let mut row = String::new(); - for x in 0..area.width { - row.push(buf[(x, y)].symbol().chars().next().unwrap_or(' ')); - } - if row.contains("foo.txt (+1 -0)") { - saw_summary = true; - break; - } - } - assert!(saw_summary, "expected approval modal to show diff summary"); - // 2) Begin apply -> per-file apply block cell (no global header) let mut changes2 = HashMap::new(); changes2.insert( @@ -1816,7 +1802,7 @@ async fn apply_patch_untrusted_shows_approval_modal() -> anyhow::Result<()> { } #[tokio::test] -async fn apply_patch_request_shows_diff_summary() -> anyhow::Result<()> { +async fn apply_patch_request_omits_diff_summary_from_modal() -> anyhow::Result<()> { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; // Ensure we are in OnRequest so an approval is surfaced @@ -1845,43 +1831,24 @@ async fn apply_patch_request_shows_diff_summary() -> anyhow::Result<()> { }), }); - // No history entries yet; the modal should contain the diff summary - let cells = drain_insert_history(&mut rx); assert!( - cells.is_empty(), + drain_insert_history(&mut rx).is_empty(), "expected approval request to render via modal instead of history" ); let area = Rect::new(0, 0, 80, chat.desired_height(/*width*/ 80)); let mut buf = ratatui::buffer::Buffer::empty(area); chat.render(area, &mut buf); - - let mut saw_header = false; - let mut saw_line1 = false; - let mut saw_line2 = false; + let mut contents = String::new(); for y in 0..area.height { - let mut row = String::new(); for x in 0..area.width { - row.push(buf[(x, y)].symbol().chars().next().unwrap_or(' ')); - } - if row.contains("README.md (+2 -0)") { - saw_header = true; - } - if row.contains("+line one") { - saw_line1 = true; - } - if row.contains("+line two") { - saw_line2 = true; - } - if saw_header && saw_line1 && saw_line2 { - break; + contents.push(buf[(x, y)].symbol().chars().next().unwrap_or(' ')); } + contents.push('\n'); } - assert!(saw_header, "expected modal to show diff header with totals"); - assert!( - saw_line1 && saw_line2, - "expected modal to show per-line diff summary" - ); + assert!(!contents.contains("README.md (+2 -0)")); + assert!(!contents.contains("+line one")); + assert!(!contents.contains("+line two")); Ok(()) } diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index 6da41423f2..f88846be94 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -210,6 +210,10 @@ async fn queued_bang_shell_dispatches_after_active_turn() { Ok(Op::RunUserShellCommand { command }) => assert_eq!(command, "echo hi"), other => panic!("expected queued shell command op, got {other:?}"), } + assert_matches!( + op_rx.try_recv(), + Ok(Op::AddToHistory { text }) if text == "!echo hi" + ); assert!(chat.queued_user_messages.is_empty()); } @@ -287,7 +291,10 @@ async fn queued_bang_shell_waits_for_user_shell_completion_before_next_input() { Ok(Op::RunUserShellCommand { command }) => assert_eq!(command, "echo hi"), other => panic!("expected queued shell command op, got {other:?}"), } - assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); + assert_matches!( + op_rx.try_recv(), + Ok(Op::AddToHistory { text }) if text == "!echo hi" + ); assert_eq!(chat.queued_user_messages.len(), 1); let begin = begin_exec_with_source( diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 46b2fd1d21..6dd527c713 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -795,7 +795,8 @@ pub async fn run_main( /*enable_codex_api_key_env*/ false, config_toml.cli_auth_credentials_store.unwrap_or_default(), chatgpt_base_url, - ); + ) + .await; let model_provider_override = if cli.oss { let resolved = resolve_oss_provider( @@ -895,7 +896,9 @@ pub async fn run_main( auth_credentials_store_mode: config.cli_auth_credentials_store_mode, forced_login_method: config.forced_login_method, forced_chatgpt_workspace_id: config.forced_chatgpt_workspace_id.clone(), - }) { + }) + .await + { eprintln!("{err}"); std::process::exit(1); } @@ -1166,7 +1169,8 @@ async fn run_ratatui_app( /*enable_codex_api_key_env*/ false, initial_config.cli_auth_credentials_store_mode, initial_config.chatgpt_base_url.clone(), - ); + ) + .await; } // If the user made an explicit trust decision, or we showed the login flow, reload config diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 1e55b5c5d2..cbe1a1fe8b 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -986,7 +986,8 @@ mod tests { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, "https://chatgpt.com/backend-api/".to_string(), - ), + ) + .await, feedback: codex_feedback::CodexFeedback::new(), log_db: None, environment_manager: Arc::new( diff --git a/codex-rs/tui/src/snapshots/codex_tui__app_backtrack__tests__backtrack_unavailable_info_message_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__app_backtrack__tests__backtrack_unavailable_info_message_snapshot.snap new file mode 100644 index 0000000000..8e896e7d7c --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__app_backtrack__tests__backtrack_unavailable_info_message_snapshot.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/app_backtrack.rs +expression: rendered +--- +• No previous message to edit.