Commit Graph

3 Commits

  • ci: run Windows argument-comment-lint via native Bazel (#16120)
    ## Why
    
    Follow-up to #16106.
    
    `argument-comment-lint` already runs as a native Bazel aspect on Linux
    and macOS, but Windows is still the long pole in `rust-ci`. To move
    Windows onto the same native Bazel lane, the toolchain split has to let
    exec-side helper binaries build in an MSVC environment while still
    linting repo crates as `windows-gnullvm`.
    
    Pushing the Windows lane onto the native Bazel path exposed a second
    round of Windows-only issues in the mixed exec-toolchain plumbing after
    the initial wrapper/target fixes landed.
    
    ## What Changed
    
    - keep the Windows lint lanes on the native Bazel/aspect path in
    `rust-ci.yml` and `rust-ci-full.yml`
    - add a dedicated `local_windows_msvc` platform for exec-side helper
    binaries while keeping `local_windows` as the `windows-gnullvm` target
    platform
    - patch `rules_rust` so `repository_set(...)` preserves explicit
    exec-platform constraints for the generated toolchains, keep the
    Windows-specific bootstrap/direct-link fixes needed for the nightly lint
    driver, and expose exec-side `rustc-dev` `.rlib`s to the MSVC sysroot
    - register the custom Windows nightly toolchain set with MSVC exec
    constraints while still exposing both `x86_64-pc-windows-msvc` and
    `x86_64-pc-windows-gnullvm` targets
    - enable `dev_components` on the custom Windows nightly repository set
    so the MSVC exec helper toolchain actually downloads the
    compiler-internal crates that `clippy_utils` needs
    - teach `run-argument-comment-lint-bazel.sh` to enumerate concrete
    Windows Rust rules, normalize the resulting labels, and skip explicitly
    requested incompatible targets instead of failing before the lint run
    starts
    - patch `rules_rust` build-script env propagation so exec-side
    `windows-msvc` helper crates drop forwarded MinGW include and linker
    search paths as whole flag/path pairs instead of emitting malformed
    `CFLAGS`, `CXXFLAGS`, and `LDFLAGS`
    - export the Windows VS/MSVC SDK environment in `setup-bazel-ci` and
    pass the relevant variables through `run-bazel-ci.sh` via `--action_env`
    / `--host_action_env` so Bazel build scripts can see the MSVC and UCRT
    headers on native Windows runs
    - add inline comments to the Windows `setup-bazel-ci` MSVC environment
    export step so it is easier to audit how `vswhere`, `VsDevCmd.bat`, and
    the filtered `GITHUB_ENV` export fit together
    - patch `aws-lc-sys` to skip its standalone `memcmp` probe under Bazel
    `windows-msvc` build-script environments, which avoids a Windows-native
    toolchain mismatch that blocked the lint lane before it reached the
    aspect execution
    - patch `aws-lc-sys` to prefer its bundled `prebuilt-nasm` objects for
    Bazel `windows-msvc` build-script runs, which avoids missing
    `generated-src/win-x86_64/*.asm` runfiles in the exec-side helper
    toolchain
    - annotate the Linux test-only callsites in `codex-rs/linux-sandbox` and
    `codex-rs/core` that the wider native lint coverage surfaced
    
    ## Patches
    
    This PR introduces a large patch stack because the Windows Bazel lint
    lane currently depends on behavior that upstream dependencies do not
    provide out of the box in the mixed `windows-gnullvm` target /
    `windows-msvc` exec-toolchain setup.
    
    - Most of the `rules_rust` patches look like upstream candidates rather
    than OpenAI-only policy. Preserving explicit exec-platform constraints,
    forwarding the right MSVC/UCRT environment into exec-side build scripts,
    exposing exec-side `rustc-dev` artifacts, and keeping the Windows
    bootstrap/linker behavior coherent all look like fixes to the Bazel/Rust
    integration layer itself.
    - The two `aws-lc-sys` patches are more tactical. They special-case
    Bazel `windows-msvc` build-script environments to avoid a `memcmp` probe
    mismatch and missing NASM runfiles. Those may be harder to upstream
    as-is because they rely on Bazel-specific detection instead of a general
    Cargo/build-script contract.
    - Short term, carrying these patches in-tree is reasonable because they
    unblock a real CI lane and are still narrow enough to audit. Long term,
    the goal should not be to keep growing a permanent local fork of either
    dependency.
    - My current expectation is that the `rules_rust` patches are less
    controversial and should be broken out into focused upstream proposals,
    while the `aws-lc-sys` patches are more likely to be temporary escape
    hatches unless that crate wants a more general hook for hermetic build
    systems.
    
    Suggested follow-up plan:
    
    1. Split the `rules_rust` deltas into upstream-sized PRs or issues with
    minimized repros.
    2. Revisit the `aws-lc-sys` patches during the next dependency bump and
    see whether they can be replaced by an upstream fix, a crate upgrade, or
    a cleaner opt-in mechanism.
    3. Treat each dependency update as a chance to delete patches one by one
    so the local patch set only contains still-needed deltas.
    
    ## Verification
    
    - `./.github/scripts/run-argument-comment-lint-bazel.sh
    --config=argument-comment-lint --keep_going`
    - `RUNNER_OS=Windows
    ./.github/scripts/run-argument-comment-lint-bazel.sh --nobuild
    --config=argument-comment-lint --platforms=//:local_windows
    --keep_going`
    - `cargo test -p codex-linux-sandbox`
    - `cargo test -p codex-core shell_snapshot_tests`
    - `just argument-comment-lint`
    
    ## References
    
    - #16106
  • ci: run SDK tests with a Bazel-built codex (#16046)
    ## Why
    
    Before this change, the SDK CI job built `codex` with Cargo before
    running the TypeScript package tests. That step has been getting more
    expensive as the Rust workspace grows, while the repo already has a
    Bazel-backed build path for the CLI.
    
    The SDK tests also need a normal executable path they can spawn
    repeatedly. Moving the job to Bazel exposed an extra CI detail: a plain
    `bazel-bin/...` lookup is not reliable under the Linux config because
    top-level outputs may stay remote and the wrapper emits status lines
    around `cquery` output.
    
    ## What Changed
    
    - taught `sdk/typescript/tests/testCodex.ts` to honor `CODEX_EXEC_PATH`
    before falling back to the local Cargo-style `target/debug/codex` path
    - added `--remote-download-toplevel` to
    `.github/scripts/run-bazel-ci.sh` so workflows can force Bazel to
    materialize top-level outputs on disk after a build
    - switched `.github/workflows/sdk.yml` from `cargo build --bin codex` to
    the shared Bazel CI setup and `//codex-rs/cli:codex` build target
    - changed the SDK workflow to resolve the built CLI with wrapper-backed
    `cquery --output=files`, stage the binary into
    `${GITHUB_WORKSPACE}/.tmp/sdk-ci/codex`, and point the SDK tests at that
    path via `CODEX_EXEC_PATH`
    - kept the warm-up step before Jest and the Bazel repository-cache save
    step
    
    ## Verification
    
    - `bash -n .github/scripts/run-bazel-ci.sh`
    - `./.github/scripts/run-bazel-ci.sh -- cquery --output=files --
    //codex-rs/cli:codex | grep -E '^(/|bazel-out/)' | tail -n 1`
    - `./.github/scripts/run-bazel-ci.sh --remote-download-toplevel -- build
    --build_metadata=TAG_job=sdk -- //codex-rs/cli:codex`
    - `CODEX_EXEC_PATH="$PWD/.tmp/sdk-ci/codex" pnpm --dir sdk/typescript
    test --runInBand`
    - `pnpm --dir sdk/typescript lint`
  • ci: add Bazel clippy workflow for codex-rs (#15955)
    ## Why
    `bazel.yml` already builds and tests the Bazel graph, but `rust-ci.yml`
    still runs `cargo clippy` separately. This PR starts the transition to a
    Bazel-backed lint lane for `codex-rs` so we can eventually replace the
    duplicate Rust build, test, and lint work with Bazel while explicitly
    keeping the V8 Bazel path out of scope for now.
    
    To make that lane practical, the workflow also needs to look like the
    Bazel job we already trust. That means sharing the common Bazel setup
    and invocation logic instead of hand-copying it, and covering the arm64
    macOS path in addition to Linux.
    
    Landing the workflow green also required fixing the first lint findings
    that Bazel surfaced and adding the matching local entrypoint.
    
    ## What changed
    - add a reusable `build:clippy` config to `.bazelrc` and export
    `codex-rs/clippy.toml` from `codex-rs/BUILD.bazel` so Bazel can run the
    repository's existing Clippy policy
    - add `just bazel-clippy` so the local developer entrypoint matches the
    new CI lane
    - extend `.github/workflows/bazel.yml` with a dedicated Bazel clippy job
    for `codex-rs`, scoped to `//codex-rs/... -//codex-rs/v8-poc:all`
    - run that clippy job on Linux x64 and arm64 macOS
    - factor the shared Bazel workflow setup into
    `.github/actions/setup-bazel-ci/action.yml` and the shared Bazel
    invocation logic into `.github/scripts/run-bazel-ci.sh` so the clippy
    and build/test jobs stay aligned
    - fix the first Bazel-clippy findings needed to keep the lane green,
    including the cross-target `cmsghdr::cmsg_len` normalization in
    `codex-rs/shell-escalation/src/unix/socket.rs` and the no-`voice-input`
    dead-code warnings in `codex-rs/tui` and `codex-rs/tui_app_server`
    
    ## Verification
    - `just bazel-clippy`
    - `RUNNER_OS=macOS ./.github/scripts/run-bazel-ci.sh -- build
    --config=clippy --build_metadata=COMMIT_SHA=local-check
    --build_metadata=TAG_job=clippy -- //codex-rs/...
    -//codex-rs/v8-poc:all`
    - `bazel build --config=clippy
    //codex-rs/shell-escalation:shell-escalation`
    - `CARGO_TARGET_DIR=/tmp/codex4-shell-escalation-test cargo test -p
    codex-shell-escalation`
    - `ruby -e 'require "yaml";
    YAML.load_file(".github/workflows/bazel.yml");
    YAML.load_file(".github/actions/setup-bazel-ci/action.yml")'`
    
    ## Notes
    - `CARGO_TARGET_DIR=/tmp/codex4-tui-app-server-test cargo test -p
    codex-tui-app-server` still hits existing guardian-approvals test and
    snapshot failures unrelated to this PR's Bazel-clippy changes.
    
    Related: #15954