Conversation
116a7f8 to
8843496
Compare
zbarsky-openai
approved these changes
Apr 17, 2026
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.
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
Unused imports in
core/tests/suite/unified_exec.rsin the Windows build were not caught by Bazel CI on #18096. I spot-checked https://github.com/openai/codex/actions/workflows/rust-ci-full.yml?query=branch%3Amain and noticed that builds were consistently red. This revealed that our Cargo builds were properly catching these issues, identifying a Windows-specific coverage hole in the Bazel clippy job.The Windows Bazel clippy job uses
--skip_incompatible_explicit_targetsso it can lint a broad target set without failing immediately on targets that are genuinely incompatible with Windows. However, with the default Windows host platform,rust_testtargets such as//codex-rs/core:core-all-testcould be skipped before the clippy aspect reached their integration-test modules. As a result, the imports incore/tests/suite/unified_exec.rswere not being linted by the Windows Bazel clippy job at all.The clippy diagnostic that Windows Bazel should have surfaced was:
What changed
--windows-msvc-host-platform, matching the Windows Bazel test job. This keeps--skip_incompatible_explicit_targetswhile ensuring Windowsrust_testtargets such as//codex-rs/core:core-all-testare still linted.core/tests/suite/unified_exec.rs.--print-failed-action-summaryto.github/scripts/run-bazel-ci.shso Bazel action failures can be summarized after the build exits.Failure reporting
Once the coverage issue was fixed, an intentionally reintroduced unused import made the Windows Bazel clippy job fail as expected. That exposed a separate usability problem: because the job keeps
--keep_going, the top-level Bazel output could still end with:without the underlying rustc/clippy diagnostic being visible in the obvious part of the GitHub Actions log.
To keep
--keep_goingwhile making failures actionable, the wrapper now scans the captured Bazel console output for failed actions and prints the matching rustc/clippy diagnostic block. When a diagnostic block is found, it is emitted both as a GitHub::errorannotation and as plain expanded log output, rather than being hidden in a collapsed group.Verification
To validate the CI path, I intentionally introduced an unused import in
core/tests/suite/unified_exec.rs. The Windows Bazel clippy job failed as expected, confirming that the integration-test module is now covered by Bazel clippy. The same failure also verified that the wrapper surfaces the matching clippy diagnostics directly in the Actions output.