Skip to content

Fix TUI attach fallback test stack overflow - #19597

Closed
etraut-openai wants to merge 1 commit into
mainfrom
etraut/fix-tui-stack-overflow-test
Closed

Fix TUI attach fallback test stack overflow#19597
etraut-openai wants to merge 1 commit into
mainfrom
etraut/fix-tui-stack-overflow-test

Conversation

@etraut-openai

Copy link
Copy Markdown
Contributor

Why

Fixes #19596.

The codex-tui fallback tests for attach_live_thread_for_selection exercise the embedded app-server path, which can exceed the default test-thread stack in debug builds. When that happens, the process aborts with a stack overflow before the tests can report a normal assertion result. The issue was reported on Windows, and the same failure reproduced locally without increasing RUST_MIN_STACK.

What changed

The two affected fallback tests now run through a small test helper that creates an explicit 4 MiB thread stack and drives the async test body with a current-thread Tokio runtime. The test behavior is unchanged: both cases still assert that empty or unmaterialized fallback threads are rejected without creating a local thread event channel.

Verification

  • cargo test -p codex-tui attach_live_thread_for_selection_rejects_ -- --test-threads=1
  • cargo test -p codex-tui no longer aborts with the reported stack overflow; it progressed past these tests and later failed on unrelated sandbox-sensitive status snapshot/config expectations.

@BaseInfinity

BaseInfinity commented Apr 25, 2026

Copy link
Copy Markdown

Thanks for jumping on this! I tested the same class of fix locally and wanted to share one extra data point from Windows.

The per-test large-stack wrapper fixes the originally reported attach fallback tests, but on my machine cargo test -p codex-tui then continued far enough to hit the same STATUS_STACK_OVERFLOW class in other debug-heavy app tests, including:

  • app::tests::discard_side_thread_removes_agent_navigation_entry
  • app::tests::interrupt_without_active_turn_is_treated_as_handled
  • app::tests::open_agent_picker_marks_loaded_threads_open

A broader approach that worked locally was setting RUST_MIN_STACK=16777216 for Windows codex-tui tests from tui/build.rs:

fn main() {
    if std::env::var_os("CARGO_CFG_WINDOWS").is_some() {
        // Match the CLI runtime's worker stack so debug-heavy app tests do
        // not overflow Windows libtest threads before assertions run.
        println!("cargo:rustc-env=RUST_MIN_STACK=16777216");
    }
}

Local verification:

  • RED on clean HEAD: cargo test -p codex-tui app::tests::attach_live_thread_for_selection_rejects_unmaterialized_fallback_threads -- --test-threads=1 aborted with STATUS_STACK_OVERFLOW.
  • With the RUST_MIN_STACK build-script env, the originally failing exact test passed.
  • An unwrapped test that later overflowed in full-suite runs, app::tests::open_agent_picker_marks_loaded_threads_open, also passed with the build-script env.
  • cargo test -p codex-tui no longer aborted with stack overflow locally; it progressed to normal assertion failures in unrelated status::helpers::*compose_agents_summary* path-format expectations on my Windows environment.

I don't want to derail this PR if the narrower wrapper is preferred, but if the intent is to unblock local Windows codex-tui runs generally, the crate-level test stack setting may avoid needing to wrap more app tests as they surface.

Thanks again for responding and getting back so quickly, I've been ramping up my codex cli usage on Windows and love to give back to such a great tool! Thanks for making it open source!

@etraut-openai

Copy link
Copy Markdown
Contributor Author

OK, then this problem is more involved. I'm going to abandon this approach. This will need to take a back seat since it's not affecting our CI or other users.

@BaseInfinity

Copy link
Copy Markdown

Thanks for looking into this! I'll let you know from my end if my local fix was enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: codex-tui app test stack overflows on clean HEAD

2 participants