diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 0c14f3cd56..27c4aa8f74 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -183,7 +183,7 @@ impl App { let mut config = config; let pending_model_migration_notice = - crate::model_migration::take_pending_model_migration_notice(&mut config); + crate::model_migration::maybe_show_pending_model_migration_notice(&config); let conversation_manager = Arc::new(ConversationManager::new( auth_manager.clone(), @@ -199,29 +199,52 @@ impl App { ) .await?; - let mut edits = ConfigEditsBuilder::new(&config.codex_home) - .record_model_migration_seen(notice.from_model.as_str(), notice.to_model.as_str()); - if matches!( - outcome, - crate::model_migration::ModelMigrationOutcome::Accepted - ) { - config.model = Some(notice.to_model.clone()); - edits = edits.set_model(config.model.as_deref(), config.model_reasoning_effort); - } + match outcome { + crate::model_migration::ModelMigrationOutcome::Accepted => { + config.model = Some(notice.to_model.clone()); + config + .notices + .model_migrations + .insert(notice.from_model.clone(), notice.to_model.clone()); - if let Err(err) = edits.apply().await { - tracing::error!( - error = %err, - "failed to persist model migration prompt outcome" - ); - } + let edits = ConfigEditsBuilder::new(&config.codex_home) + .record_model_migration_seen( + notice.from_model.as_str(), + notice.to_model.as_str(), + ) + .set_model(config.model.as_deref(), config.model_reasoning_effort); + if let Err(err) = edits.apply().await { + tracing::error!( + error = %err, + "failed to persist model migration prompt outcome" + ); + } + } + crate::model_migration::ModelMigrationOutcome::Rejected => { + config + .notices + .model_migrations + .insert(notice.from_model.clone(), notice.to_model.clone()); - if matches!(outcome, crate::model_migration::ModelMigrationOutcome::Exit) { - return Ok(AppExitInfo { - token_usage: TokenUsage::default(), - conversation_id: None, - update_action: None, - }); + let edits = ConfigEditsBuilder::new(&config.codex_home) + .record_model_migration_seen( + notice.from_model.as_str(), + notice.to_model.as_str(), + ); + if let Err(err) = edits.apply().await { + tracing::error!( + error = %err, + "failed to persist model migration prompt outcome" + ); + } + } + crate::model_migration::ModelMigrationOutcome::Exit => { + return Ok(AppExitInfo { + token_usage: TokenUsage::default(), + conversation_id: None, + update_action: None, + }); + } } } diff --git a/codex-rs/tui/src/model_migration.rs b/codex-rs/tui/src/model_migration.rs index 9c57727226..f89fbd1adb 100644 --- a/codex-rs/tui/src/model_migration.rs +++ b/codex-rs/tui/src/model_migration.rs @@ -26,12 +26,9 @@ pub(crate) use prompt_ui::ModelMigrationOutcome; pub(crate) use prompt_ui::ModelMigrationScreen; pub(crate) use prompt_ui::migration_copy_for_models; -/// Read and clear the one-shot migration notice file, returning the notice if it should be shown. -/// -/// If the notice is returned, this also updates `config.notices.model_migrations` to prevent -/// re-scheduling within the current process. -pub(crate) fn take_pending_model_migration_notice( - config: &mut Config, +/// Read the pending migration notice file, returning the notice if it should be shown. +pub(crate) fn maybe_show_pending_model_migration_notice( + config: &Config, ) -> Option { let notice_path = pending_model_migration_notice_path(config); let contents = match std::fs::read_to_string(¬ice_path) { @@ -55,6 +52,7 @@ pub(crate) fn take_pending_model_migration_notice( notice_path = %notice_path.display(), "failed to parse pending model migration notice" ); + let _ = std::fs::remove_file(¬ice_path); return None; } }; @@ -78,14 +76,6 @@ pub(crate) fn take_pending_model_migration_notice( return None; } - // Best-effort: clear the one-shot file so it doesn't appear again. - let _ = std::fs::remove_file(¬ice_path); - - config - .notices - .model_migrations - .insert(notice.from_model.clone(), notice.to_model.clone()); - Some(notice) } diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index d6dbd60dc2..c24deb0d6a 100644 --- a/codex-rs/tui2/src/app.rs +++ b/codex-rs/tui2/src/app.rs @@ -224,7 +224,7 @@ impl App { let mut config = config; let pending_model_migration_notice = - crate::model_migration::take_pending_model_migration_notice(&mut config); + crate::model_migration::maybe_show_pending_model_migration_notice(&config); let conversation_manager = Arc::new(ConversationManager::new( auth_manager.clone(), @@ -240,30 +240,53 @@ impl App { ) .await?; - let mut edits = ConfigEditsBuilder::new(&config.codex_home) - .record_model_migration_seen(notice.from_model.as_str(), notice.to_model.as_str()); - if matches!( - outcome, - crate::model_migration::ModelMigrationOutcome::Accepted - ) { - config.model = Some(notice.to_model.clone()); - edits = edits.set_model(config.model.as_deref(), config.model_reasoning_effort); - } + match outcome { + crate::model_migration::ModelMigrationOutcome::Accepted => { + config.model = Some(notice.to_model.clone()); + config + .notices + .model_migrations + .insert(notice.from_model.clone(), notice.to_model.clone()); - if let Err(err) = edits.apply().await { - tracing::error!( - error = %err, - "failed to persist model migration prompt outcome" - ); - } + let edits = ConfigEditsBuilder::new(&config.codex_home) + .record_model_migration_seen( + notice.from_model.as_str(), + notice.to_model.as_str(), + ) + .set_model(config.model.as_deref(), config.model_reasoning_effort); + if let Err(err) = edits.apply().await { + tracing::error!( + error = %err, + "failed to persist model migration prompt outcome" + ); + } + } + crate::model_migration::ModelMigrationOutcome::Rejected => { + config + .notices + .model_migrations + .insert(notice.from_model.clone(), notice.to_model.clone()); - if matches!(outcome, crate::model_migration::ModelMigrationOutcome::Exit) { - return Ok(AppExitInfo { - token_usage: TokenUsage::default(), - conversation_id: None, - update_action: None, - session_lines: Vec::new(), - }); + let edits = ConfigEditsBuilder::new(&config.codex_home) + .record_model_migration_seen( + notice.from_model.as_str(), + notice.to_model.as_str(), + ); + if let Err(err) = edits.apply().await { + tracing::error!( + error = %err, + "failed to persist model migration prompt outcome" + ); + } + } + crate::model_migration::ModelMigrationOutcome::Exit => { + return Ok(AppExitInfo { + token_usage: TokenUsage::default(), + conversation_id: None, + update_action: None, + session_lines: Vec::new(), + }); + } } } diff --git a/codex-rs/tui2/src/model_migration.rs b/codex-rs/tui2/src/model_migration.rs index d6565c2c54..2812e8534d 100644 --- a/codex-rs/tui2/src/model_migration.rs +++ b/codex-rs/tui2/src/model_migration.rs @@ -26,12 +26,9 @@ pub(crate) use prompt_ui::ModelMigrationOutcome; pub(crate) use prompt_ui::ModelMigrationScreen; pub(crate) use prompt_ui::migration_copy_for_models; -/// Read and clear the one-shot migration notice file, returning the notice if it should be shown. -/// -/// If the notice is returned, this also updates `config.notices.model_migrations` to prevent -/// re-scheduling within the current process. -pub(crate) fn take_pending_model_migration_notice( - config: &mut Config, +/// Read the pending migration notice file, returning the notice if it should be shown. +pub(crate) fn maybe_show_pending_model_migration_notice( + config: &Config, ) -> Option { let notice_path = pending_model_migration_notice_path(config); let contents = match std::fs::read_to_string(¬ice_path) { @@ -55,6 +52,7 @@ pub(crate) fn take_pending_model_migration_notice( notice_path = %notice_path.display(), "failed to parse pending model migration notice" ); + let _ = std::fs::remove_file(¬ice_path); return None; } }; @@ -78,14 +76,6 @@ pub(crate) fn take_pending_model_migration_notice( return None; } - // Best-effort: clear the one-shot file so it doesn't appear again. - let _ = std::fs::remove_file(¬ice_path); - - config - .notices - .model_migrations - .insert(notice.from_model.clone(), notice.to_model.clone()); - Some(notice) }