Skip to content

fork-ci: build workspace bins before cargo test - #17

Merged
thedancingdeveloper merged 1 commit into
fork/mainfrom
fork/ci-helpers
Oct 9, 2026
Merged

thedancingdeveloper merged 1 commit into
fork/mainfrom
fork/ci-helpers

Conversation

@thedancingdeveloper

Copy link
Copy Markdown

Summary

  • Every PR touching `codex-app-server` fails `thread_start_unloads_after_requesting_connection_closes_during_mcp_startup` with `could not locate binary "test_stdio_server"`. `cargo test -p` does not build other packages' bins, so `CARGO_BIN_EXE_*` is unset and the fallback lookup in `target/debug` misses.
  • Tests resolve about twenty helper bins this way through `cargo_bin()` (`codex`, `codex-code-mode-host`, the rmcp-client test servers, `codex-app-server`, `codex-exec`, `apply_patch`, `codex-linux-sandbox`, `exec-server` and others). Upstream's Bazel build declares them as data deps. The test step now runs `cargo build --workspace --bins --exclude codex-voice-host` once before the per-crate loop, so `assert_cmd` finds them in `target/debug`.
  • `codex-voice-host` stays excluded: it needs GStreamer >= 1.28 and Ubuntu 24.04 ships 1.24.

Test plan

Tests find helper binaries in other packages through cargo_bin(), and
cargo test -p never builds another package's bins. CARGO_BIN_EXE_* is unset
and the fallback lookup in target/debug misses, so every PR touching
codex-app-server fails thread_start_unloads_after_requesting_connection
with "could not locate binary test_stdio_server". There are about twenty
such bins, so the test step builds every workspace bin once first.
codex-voice-host is excluded: it needs GStreamer >= 1.28, which Ubuntu
24.04 does not ship. Upstream's Bazel build declares the same bins as data
dependencies instead.

@thedancingdeveloper thedancingdeveloper left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: no blockers (head 18a8048)

One commit on fork/main feaf20e, +7 lines in fork-ci's test step. It adds a single cargo build --workspace --bins --exclude codex-voice-host before the per-crate test loop, so every helper bin the tests resolve through codex_utils_cargo_bin::cargo_bin() (I counted 20 distinct names, many from other packages, e.g. codex-rmcp-client's test_stdio_server) is in target/debug for the assert_cmd fallback. That covers the whole class, not just the one test. The codex-voice-host exclusion matches the existing workspace check. Tests that need it would fail only for a PR touching those crates, which we don't plan.

Merge condition: this PR's own fork-ci run is green. The real proof needs a PR that touches codex-app-server, which this one doesn't, so right after merge re-run #6 or #12 and confirm thread_start_unloads_after_requesting_connection_closes_during_mcp_startup passes. If it doesn't, this gets reverted or fixed before anything else merges.

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.

1 participant