From 47bcb3a3f9e0904aed485389aaf410282b6b23fa Mon Sep 17 00:00:00 2001 From: jif Date: Sat, 10 Oct 2026 18:41:57 +0000 Subject: [PATCH] Reduce stack usage in async TUI tests (#52959) ## Why Large test futures can occupy their caller's stack frame even when passed directly to `Box::pin`. ## What changed - Add a non-inlined `box_future` helper and `boxed_future!` macro to construct and box futures outside the caller's poll frame. Use them for heavyweight TUI fixtures and test scenarios. - Return the shared `App` fixture as `Box` to keep it on the heap. - Treat `ConnectionAborted` as a closed connection in the realtime speech test server, alongside `BrokenPipe` and `ConnectionReset`. GitOrigin-RevId: 7bbf2020a5ec12bae8e0365ddf5641deef93b544 --- codex-rs/tui/src/app/recap_tests.rs | 2 +- codex-rs/tui/src/app/test_support.rs | 11 ++++++---- codex-rs/tui/src/app/tests.rs | 21 ++++++++++++++----- .../app/tests/session_lifecycle_requests.rs | 13 +++++++++--- codex-rs/tui/src/chatwidget/tests/helpers.rs | 3 ++- .../tui/src/managed_worktree_tools_tests.rs | 6 ++++++ codex-rs/tui/src/test_support.rs | 17 +++++++++++++++ 7 files changed, 59 insertions(+), 14 deletions(-) diff --git a/codex-rs/tui/src/app/recap_tests.rs b/codex-rs/tui/src/app/recap_tests.rs index 437cb66bb8..8990301727 100644 --- a/codex-rs/tui/src/app/recap_tests.rs +++ b/codex-rs/tui/src/app/recap_tests.rs @@ -952,7 +952,7 @@ fn track_in_flight_recap(app: &mut App, thread_id: ThreadId) -> (RecapRequest, T (request, temporary_thread_id) } -async fn app_with_visible_thread(thread_id: ThreadId) -> App { +async fn app_with_visible_thread(thread_id: ThreadId) -> Box { let mut app = make_test_app().await; app.active_thread_id = Some(thread_id); app diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index 8b5901c606..ae588ce001 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -3,21 +3,24 @@ //! This module keeps heavyweight `App` construction and config-inspection helpers available to //! focused sibling test modules without making `app/tests.rs` the only practical place to test //! app-owned behavior. +//! Return the large fixture on the heap so async scenarios do not embed it in their stack frames. use super::*; use crate::chatwidget::tests::make_chatwidget_manual_with_sender; +use crate::test_support::boxed_future; use codex_models_manager::test_support::construct_model_info_offline_for_tests; use codex_models_manager::test_support::get_model_offline_for_tests; -pub(crate) async fn make_test_app() -> App { - let (mut chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await; +pub(crate) async fn make_test_app() -> Box { + let (mut chat_widget, app_event_tx, _rx, _op_rx) = + boxed_future!(make_chatwidget_manual_with_sender()).await; let test_codex_home = chat_widget.test_codex_home.take(); let config = chat_widget.config_ref().clone(); let file_search = FileSearchManager::new(config.cwd.to_path_buf(), app_event_tx.clone()); let model = get_model_offline_for_tests(config.model.as_deref()); let session_telemetry = test_session_telemetry(&config, model.as_str()); - App { + Box::new(App { feature_write_lock: Arc::default(), model_catalog: chat_widget.model_catalog(), session_telemetry, @@ -121,7 +124,7 @@ pub(crate) async fn make_test_app() -> App { pending_hook_enabled_writes: HashMap::new(), recap: recap::RecapState::default(), _test_codex_home: test_codex_home, - } + }) } fn test_session_telemetry(config: &Config, model: &str) -> SessionTelemetry { diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index dae66527f7..c1560772ec 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -114,6 +114,7 @@ use crate::app_backtrack::BacktrackState; use crate::app_backtrack::nth_user_position; use crate::app_backtrack::user_count; use crate::app_event::HistoryBatchEntryResponse; +use crate::test_support::boxed_future; async fn drain_managed_worktree_start(app: &mut App, server: &mut AppServerSession) { if let Some((mode, name)) = app.pending_start_managed_worktree.take() { @@ -1380,7 +1381,8 @@ async fn replay_thread_snapshot_restores_the_matching_safety_buffer_prompt() { store.snapshot() }; - let (mut chat_widget, _app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await; + let (mut chat_widget, _app_event_tx, _rx, _op_rx) = + boxed_future!(make_chatwidget_manual_with_sender()).await; chat_widget.handle_thread_session(test_thread_session( ThreadId::new(), test_path_buf("/tmp/other-project"), @@ -1639,7 +1641,8 @@ async fn replay_thread_snapshot_in_progress_turn_restores_running_state_without_ let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await; let thread_id = ThreadId::new(); let session = test_thread_session(thread_id, test_path_buf("/tmp/project")); - let (chat_widget, _app_event_tx, _rx, _new_op_rx) = make_chatwidget_manual_with_sender().await; + let (chat_widget, _app_event_tx, _rx, _new_op_rx) = + boxed_future!(make_chatwidget_manual_with_sender()).await; app.chat_widget = chat_widget; app.chat_widget.handle_thread_session(session); @@ -1926,7 +1929,8 @@ async fn replay_thread_snapshot_restores_collaboration_mode_without_input() { .capture_thread_input_state() .expect("expected collaboration-only input state"); - let (chat_widget, _app_event_tx, _rx, _new_op_rx) = make_chatwidget_manual_with_sender().await; + let (chat_widget, _app_event_tx, _rx, _new_op_rx) = + boxed_future!(make_chatwidget_manual_with_sender()).await; app.chat_widget = chat_widget; app.chat_widget .set_plan_mode_reasoning_effort(Some(ReasoningEffortConfig::Low)); @@ -6179,7 +6183,8 @@ async fn ctrl_l_clears_owned_history_and_preserves_the_draft() -> Result<()> { } async fn make_test_app() -> Box { - let (mut chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await; + let (mut chat_widget, app_event_tx, _rx, _op_rx) = + boxed_future!(make_chatwidget_manual_with_sender()).await; let test_codex_home = chat_widget.test_codex_home.take(); let config = chat_widget.config_ref().clone(); let file_search = FileSearchManager::new(config.cwd.to_path_buf(), app_event_tx.clone()); @@ -6298,7 +6303,8 @@ pub(super) async fn make_test_app_with_channels() -> ( tokio::sync::mpsc::UnboundedReceiver, tokio::sync::mpsc::UnboundedReceiver, ) { - let (mut chat_widget, app_event_tx, rx, op_rx) = make_chatwidget_manual_with_sender().await; + let (mut chat_widget, app_event_tx, rx, op_rx) = + boxed_future!(make_chatwidget_manual_with_sender()).await; let test_codex_home = chat_widget.test_codex_home.take(); let config = chat_widget.config_ref().clone(); let file_search = FileSearchManager::new(config.cwd.to_path_buf(), app_event_tx.clone()); @@ -8404,6 +8410,11 @@ async fn remembered_current_cwd_stays_at_launch_across_in_app_resumes() -> Resul #[tokio::test] async fn prompt_edit_reverts_earlier_and_first_visible_prompts_in_place() -> Result<()> { + // Allocate the whole scenario before Tokio polls it on the test thread. + boxed_future!(prompt_edit_reverts_earlier_and_first_visible_prompts_in_place_scenario()).await +} + +async fn prompt_edit_reverts_earlier_and_first_visible_prompts_in_place_scenario() -> Result<()> { use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; use codex_protocol::models::ImageReference; diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index 52d35400ee..3e7975f219 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -1,6 +1,7 @@ use super::*; use crate::app_event::TranscriptExportDestination; use crate::bottom_pane::BottomPaneView; +use crate::test_support::boxed_future; use app_test_support::create_fake_paginated_rollout; use app_test_support::create_fake_parented_rollout_with_source; use app_test_support::create_fake_rollout; @@ -506,7 +507,7 @@ pub(super) async fn start_recording_app_server_with_realtime_speech( tokio_tungstenite::tungstenite::error::ProtocolError::ResetWithoutClosingHandshake, )) => break, Err(tokio_tungstenite::tungstenite::Error::Io(error)) - if matches!(error.kind(), std::io::ErrorKind::BrokenPipe | std::io::ErrorKind::ConnectionReset) => break, + if matches!(error.kind(), std::io::ErrorKind::BrokenPipe | std::io::ErrorKind::ConnectionReset | std::io::ErrorKind::ConnectionAborted) => break, _ => {} } let Message::Text(text) = frame? else { @@ -1338,6 +1339,12 @@ async fn archive_current_thread_returns_shared_servers_to_agents() -> Result<()> #[tokio::test] async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() -> Result<()> { + boxed_future!(local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths_scenario()) + .await +} + +async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths_scenario() +-> Result<()> { let (mut app, events, _ops) = Box::pin(make_test_app_with_channels()).await; // Invalid optional worktree settings must preserve both daemon start paths. app.config.features.enable(Feature::Worktrees)?; @@ -1822,8 +1829,8 @@ async fn embedded_server_rejects_unowned_dynamic_tool_calls() -> Result<()> { #[tokio::test] async fn dynamic_tool_requests_ignore_other_namespaces_and_dispatch_tui_namespace() -> Result<()> { - check_dynamic_tool_requests(/*rollout_enabled*/ true).await?; - check_dynamic_tool_requests(/*rollout_enabled*/ false).await + boxed_future!(check_dynamic_tool_requests(/*rollout_enabled*/ true)).await?; + boxed_future!(check_dynamic_tool_requests(/*rollout_enabled*/ false)).await } async fn check_dynamic_tool_requests(rollout_enabled: bool) -> Result<()> { diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index 1db139d731..9ce2635d3b 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -1,4 +1,5 @@ use super::*; +use crate::test_support::boxed_future; use codex_app_server_protocol::ImageGenerationItem; use codex_app_server_protocol::PluginAvailability; use codex_utils_absolute_path::test_support::PathExt; @@ -375,7 +376,7 @@ pub(crate) async fn make_chatwidget_manual_with_sender() -> ( tokio::sync::mpsc::UnboundedReceiver, tokio::sync::mpsc::UnboundedReceiver, ) { - let (widget, rx, op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let (widget, rx, op_rx) = boxed_future!(make_chatwidget_manual(/*model_override*/ None)).await; let app_event_tx = AppEventSender::new(widget.app_event_tx.app_event_tx.clone()); (widget, app_event_tx, rx, op_rx) } diff --git a/codex-rs/tui/src/managed_worktree_tools_tests.rs b/codex-rs/tui/src/managed_worktree_tools_tests.rs index e51bff0586..27904dc9f6 100644 --- a/codex-rs/tui/src/managed_worktree_tools_tests.rs +++ b/codex-rs/tui/src/managed_worktree_tools_tests.rs @@ -1,5 +1,6 @@ use super::*; use crate::legacy_core::config::ConfigOverrides; +use crate::test_support::boxed_future; use codex_config::LoaderOverrides; use pretty_assertions::assert_eq; use std::fs; @@ -88,6 +89,11 @@ async fn start_server( #[tokio::test] async fn creation_attaches_to_original_task_and_survives_service_restart() -> color_eyre::Result<()> { + boxed_future!(creation_attaches_to_original_task_and_survives_service_restart_scenario()).await +} + +async fn creation_attaches_to_original_task_and_survives_service_restart_scenario() +-> color_eyre::Result<()> { let home = tempfile::tempdir()?; let responses = app_test_support::create_mock_responses_server_repeating_assistant("Ready").await; diff --git a/codex-rs/tui/src/test_support.rs b/codex-rs/tui/src/test_support.rs index 44e9899379..a83764e1bb 100644 --- a/codex-rs/tui/src/test_support.rs +++ b/codex-rs/tui/src/test_support.rs @@ -1,5 +1,7 @@ //! Test-only helpers shared across the TUI crate. +use std::future::Future; +use std::pin::Pin; use std::sync::LazyLock; use codex_models_manager::bundled_models_response; @@ -53,3 +55,18 @@ where panic!("app-server wire value should map to legacy helper type: {err}") }) } + +/// Construct a large test future outside its caller's poll frame before boxing it. +/// `Box::pin(large_future())` can still reserve the future's full size on that stack. +#[inline(never)] +pub(crate) fn box_future(create: impl FnOnce() -> F) -> Pin> { + Box::pin(create()) +} + +// Keep boxed call sites shaped like ordinary calls, without closure indentation. +macro_rules! boxed_future { + ($future:expr) => { + $crate::test_support::box_future(|| $future) + }; +} +pub(crate) use boxed_future;