Conversation
This was referenced Apr 22, 2026
alexsong-oai
approved these changes
Apr 22, 2026
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 960d5145fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| exit /b 0 | ||
| ) | ||
|
|
||
| "%test_bin%" !SHARD_TESTS! --exact |
Contributor
There was a problem hiding this comment.
This launcher concatenates every selected test into SHARD_TESTS and invokes one command ("%test_bin%" !SHARD_TESTS! --exact), which can exceed cmd.exe’s ~8191-character command-line limit on large shards (for example, crates like tui with many long test paths). When that happens, the shard fails before running tests, so Windows sharded unit-test runs become unreliable as suites grow.
Useful? React with 👍 / 👎.
bolinfest
added a commit
that referenced
this pull request
Apr 22, 2026
## Why `build_prompt_input` now initializes `ExecServerRuntimePaths`, which requires a configured Codex executable path. The previous inline unit test in `core/src/prompt_debug.rs` built a bare `test_config()` and then failed before it could assert anything useful: ```text Codex executable path is not configured ``` This coverage is also integration-shaped: it drives the public `build_prompt_input` entry point through config, thread, and session setup rather than testing a small internal helper in isolation. Bazel CI did not catch this earlier because the affected test was behind the same wrapped Rust unit-test path fixed by #18913. Before that launcher/sharding fix, the outer `workspace_root_test` changed the working directory for Insta compatibility while the inner `rules_rust` sharding wrapper still expected its runfiles working directory. In practice, Bazel could report success without executing the Rust test cases in that shard. Once #18913 makes the wrapper run the Rust test binary directly and shard with libtest arguments, this stale unit test actually runs and exposes the missing `codex_self_exe` setup. ## What Changed - Moved `build_prompt_input_includes_context_and_user_message` out of `core/src/prompt_debug.rs`. - Added `core/tests/suite/prompt_debug_tests.rs` and registered it from `core/tests/suite/mod.rs`. - Builds the test config with `ConfigBuilder` and provides `codex_self_exe` using the current test executable, matching the runtime-path invariant required by prompt debug setup. - Preserves the existing assertions that the generated prompt input includes both the debug user message and project-specific user instructions. ## Verification - `cargo test -p codex-core --test all prompt_debug_tests::build_prompt_input_includes_context_and_user_message` - `bazel test //codex-rs/core:core-all-test --test_arg=prompt_debug_tests::build_prompt_input_includes_context_and_user_message --test_output=errors` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18916). * #18913 * __->__ #18916
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The
codex-tuiCargo test suite was catching stale snapshot expectations, but the matching Bazel unit-test target was still green. The TUI unit target is wrapped byworkspace_root_testso tests run from the repository root and Insta can resolve Cargo-like snapshot paths. After native Bazel sharding was enabled for that wrapped target, rules_rust also inserted its own sharding wrapper around the Rust test binary.Those two wrappers did not compose: rules_rust's sharding wrapper expects to run from its own runfiles cwd, while
workspace_root_testdeliberately changes cwd to the repo root before invoking the test. In that configuration, the inner wrapper could fail to enumerate the Rust tests and exit successfully with empty shards, so snapshot regressions were not being exercised by Bazel.What Changed
experimental_enable_shardingfor unit-test binaries created bycodex_rust_crate.shard_counton the outerworkspace_root_testtarget.workspace_root_test_launcher.sh.tplandworkspace_root_test_launcher.bat.tplafter the launcher has resolved the actual test binary and established the intended repository-root cwd.cmd.execommand line.This PR is stacked on top of #18912, which contains only the snapshot expectation updates exposed once the Bazel target actually runs the TUI unit tests. It is also the reason #18916 becomes visible: once this wrapper fix makes Bazel execute the affected
codex-coretest, that test needs its own executable-path setup fixed.Verification
cargo test -p codex-tuibazel test //codex-rs/tui:tui-unit-tests --test_output=errorsbazel test //codex-rs/tui:all --test_output=errorsbash -n workspace_root_test_launcher.sh.tpl