Commit Graph

6 Commits

  • chore: move pty and windows sandbox to Rust 2024 (#15954)
    ## Why
    
    `codex-utils-pty` and `codex-windows-sandbox` were the remaining crates
    in `codex-rs` that still overrode the workspace's Rust 2024 edition.
    Moving them forward in a separate PR keeps the baseline edition update
    isolated from the follow-on Bazel clippy workflow in #15955, while
    making linting and formatting behavior consistent with the rest of the
    workspace.
    
    This PR also needs Cargo and Bazel to agree on the edition for
    `codex-windows-sandbox`. Without the Bazel-side sync, the experimental
    Bazel app-server builds fail once they compile `windows-sandbox-rs`.
    
    ## What changed
    
    - switch `codex-rs/utils/pty` and `codex-rs/windows-sandbox-rs` to
    `edition = "2024"`
    - update `codex-utils-pty` callsites and tests to use the collapsed `if
    let` form that Clippy expects under the new edition
    - fix the Rust 2024 fallout in `windows-sandbox-rs`, including the
    reserved `gen` identifier, `unsafe extern` requirements, and new Clippy
    findings that surfaced under the edition bump
    - keep the edition bump separate from a larger unsafe cleanup by
    temporarily allowing `unsafe_op_in_unsafe_fn` in the Windows entrypoint
    modules that now report it under Rust 2024
    - update `codex-rs/windows-sandbox-rs/BUILD.bazel` to `crate_edition =
    "2024"` so Bazel compiles the crate with the same edition as Cargo
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/15954).
    * #15976
    * #15955
    * __->__ #15954
  • Use released DotSlash package for argument-comment lint (#15199)
    ## Why
    The argument-comment lint now has a packaged DotSlash artifact from
    [#15198](https://github.com/openai/codex/pull/15198), so the normal repo
    lint path should use that released payload instead of rebuilding the
    lint from source every time.
    
    That keeps `just clippy` and CI aligned with the shipped artifact while
    preserving a separate source-build path for people actively hacking on
    the lint crate.
    
    The current alpha package also exposed two integration wrinkles that the
    repo-side prebuilt wrapper needs to smooth over:
    - the bundled Dylint library filename includes the host triple, for
    example `@nightly-2025-09-18-aarch64-apple-darwin`, and Dylint derives
    `RUSTUP_TOOLCHAIN` from that filename
    - on Windows, Dylint's driver path also expects `RUSTUP_HOME` to be
    present in the environment
    
    Without those adjustments, the prebuilt CI jobs fail during `cargo
    metadata` or driver setup. This change makes the checked-in prebuilt
    wrapper normalize the packaged library name to the plain
    `nightly-2025-09-18` channel before invoking `cargo-dylint`, and it
    teaches both the wrapper and the packaged runner source to infer
    `RUSTUP_HOME` from `rustup show home` when the environment does not
    already provide it.
    
    After the prebuilt Windows lint job started running successfully, it
    also surfaced a handful of existing anonymous literal callsites in
    `windows-sandbox-rs`. This PR now annotates those callsites so the new
    cross-platform lint job is green on the current tree.
    
    ## What Changed
    - checked in the current
    `tools/argument-comment-lint/argument-comment-lint` DotSlash manifest
    - kept `tools/argument-comment-lint/run.sh` as the source-build wrapper
    for lint development
    - added `tools/argument-comment-lint/run-prebuilt-linter.sh` as the
    normal enforcement path, using the checked-in DotSlash package and
    bundled `cargo-dylint`
    - updated `just clippy` and `just argument-comment-lint` to use the
    prebuilt wrapper
    - split `.github/workflows/rust-ci.yml` so source-package checks live in
    a dedicated `argument_comment_lint_package` job, while the released lint
    runs in an `argument_comment_lint_prebuilt` matrix on Linux, macOS, and
    Windows
    - kept the pinned `nightly-2025-09-18` toolchain install in the prebuilt
    CI matrix, since the prebuilt package still relies on rustup-provided
    toolchain components
    - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` to
    normalize host-qualified nightly library filenames, keep the `rustup`
    shim directory ahead of direct toolchain `cargo` binaries, and export
    `RUSTUP_HOME` when needed for Windows Dylint driver setup
    - updated `tools/argument-comment-lint/src/bin/argument-comment-lint.rs`
    so future published DotSlash artifacts apply the same nightly-filename
    normalization and `RUSTUP_HOME` inference internally
    - fixed the remaining Windows lint violations in
    `codex-rs/windows-sandbox-rs` by adding the required `/*param*/`
    comments at the reported callsites
    - documented the checked-in DotSlash file, wrapper split, archive
    layout, nightly prerequisite, and Windows `RUSTUP_HOME` requirement in
    `tools/argument-comment-lint/README.md`
  • windows-sandbox: add runner IPC foundation for future unified_exec (#14139)
    # Summary
    
    This PR introduces the Windows sandbox runner IPC foundation that later
    unified_exec work will build on.
    
    The key point is that this is intentionally infrastructure-only. The new
    IPC transport, runner plumbing, and ConPTY helpers are added here, but
    the active elevated Windows sandbox path still uses the existing
    request-file bootstrap. In other words, this change prepares the
    transport and module layout we need for unified_exec without switching
    production behavior over yet.
    
    Part of this PR is also a source-layout cleanup: some Windows sandbox
    files are moved into more explicit `elevated/`, `conpty/`, and shared
    locations so it is clearer which code is for the elevated sandbox flow,
    which code is legacy/direct-spawn behavior, and which helpers are shared
    between them. That reorganization is intentional in this first PR so
    later behavioral changes do not also have to carry a large amount of
    file-move churn.
    
    # Why This Is Needed For unified_exec
    
    Windows elevated sandboxed unified_exec needs a long-lived,
    bidirectional control channel between the CLI and a helper process
    running under the sandbox user. That channel has to support:
    
    - starting a process and reporting structured spawn success/failure
    - streaming stdout/stderr back incrementally
    - forwarding stdin over time
    - terminating or polling a long-lived process
    - supporting both pipe-backed and PTY-backed sessions
    
    The existing elevated one-shot path is built around a request-file
    bootstrap and does not provide those primitives cleanly. Before we can
    turn on Windows sandbox unified_exec, we need the underlying runner
    protocol and transport layer that can carry those lifecycle events and
    streams.
    
    # Why Windows Needs More Machinery Than Linux Or macOS
    
    Linux and macOS can generally build unified_exec on top of the existing
    sandbox/process model: the parent can spawn the child directly, retain
    normal ownership of stdio or PTY handles, and manage the lifetime of the
    sandboxed process without introducing a second control process.
    
    Windows elevated sandboxing is different. To run inside the sandbox
    boundary, we cross into a different user/security context and then need
    to manage a long-lived process from outside that boundary. That means we
    need an explicit helper process plus an IPC transport to carry spawn,
    stdin, output, and exit events back and forth. The extra code here is
    mostly that missing Windows sandbox infrastructure, not a conceptual
    difference in unified_exec itself.
    
    # What This PR Adds
    
    - the framed IPC message types and transport helpers for parent <->
    runner communication
    - the renamed Windows command runner with both the existing request-file
    bootstrap and the dormant IPC bootstrap
    - named-pipe helpers for the elevated runner path
    - ConPTY helpers and process-thread attribute plumbing needed for
    PTY-backed sessions
    - shared sandbox/process helpers that later PRs will reuse when
    switching live execution paths over
    - early file/module moves so later PRs can focus on behavior rather than
    layout churn
    
    # What This PR Does Not Yet Do
    
    - it does not switch the active elevated one-shot path over to IPC yet
    - it does not enable Windows sandbox unified_exec yet
    - it does not remove the existing request-file bootstrap yet
    
    So while this code compiles and the new path has basic validation, it is
    not yet the exercised production path. That is intentional for this
    first PR: the goal here is to land the transport and runner foundation
    cleanly before later PRs start routing real command execution through
    it.
    
    # Follow-Ups
    
    Planned follow-up PRs will:
    
    1. switch elevated one-shot Windows sandbox execution to the new runner
    IPC path
    2. layer Windows sandbox unified_exec sessions on top of the same
    transport
    3. remove the legacy request-file path once the IPC-based path is live
    
    # Validation
    
    - `cargo build -p codex-windows-sandbox`
  • Fix inverted Windows PTY TerminateProcess handling (#13989)
    Addresses #13945
    
    The vendored WezTerm ConPTY backend in
    `codex-rs/utils/pty/src/win/mod.rs` treated `TerminateProcess` return
    values backwards: nonzero success was handled as failure, and `0`
    failure was handled as success.
    
    This is likely causing a number of bugs reported against Codex running
    on Windows native where processes are not cleaned up.
  • Vendor ConPtySystem (#7656)
    The repo we were depending on is very large and we need very small part
    of it.
    
    ---------
    
    Co-authored-by: Pavel <pavel@krymets.com>