Commit Graph

13 Commits

  • feat: use ProcessId in exec-server (#15866)
    Use a full struct for the ProcessId to increase readability and make it
    easier in the future to make it evolve if needed
  • feat: exec-server prep for unified exec (#15691)
    This PR partially rebase `unified_exec` on the `exec-server` and adapt
    the `exec-server` accordingly.
    
    ## What changed in `exec-server`
    
    1. Replaced the old "broadcast-driven; process-global" event model with
    process-scoped session events. The goal is to be able to have dedicated
    handler for each process.
    2. Add to protocol contract to support explicit lifecycle status and
    stream ordering:
    - `WriteResponse` now returns `WriteStatus` (Accepted, UnknownProcess,
    StdinClosed, Starting) instead of a bool.
      - Added seq fields to output/exited notifications.
      - Added terminal process/closed notification.
    3. Demultiplexed remote notifications into per-process channels. Same as
    for the event sys
    4. Local and remote backends now both implement ExecBackend.
    5. Local backend wraps internal process ID/operations into per-process
    ExecProcess objects.
    6. Remote backend registers a session channel before launch and
    unregisters on failed launch.
    
    ## What changed in `unified_exec`
    
    1. Added unified process-state model and backend-neutral process
    wrapper. This will probably disappear in the future, but it makes it
    easier to keep the work flowing on both side.
    - `UnifiedExecProcess` now handles both local PTY sessions and remote
    exec-server processes through a shared `ProcessHandle`.
    - Added `ProcessState` to track has_exited, exit_code, and terminal
    failure message consistently across backends.
    2. Routed write and lifecycle handling through process-level methods.
    
    ## Some rationals
    
    1. The change centralizes execution transport in exec-server while
    preserving policy and orchestration ownership in core, avoiding
    duplicated launch approval logic. This comes from internal discussion.
    2. Session-scoped events remove coupling/cross-talk between processes
    and make stream ordering and terminal state explicit (seq, closed,
    failed).
    3. The failure-path surfacing (remote launch failures, write failures,
    transport disconnects) makes command tool output and cleanup behavior
    deterministic
    
    ## Follow-ups:
    * Unify the concept of thread ID behind an obfuscated struct
    * FD handling
    * Full zsh-fork compatibility
    * Full network sandboxing compatibility
    * Handle ws disconnection
  • Add cached environment manager for exec server URL (#15785)
    Add environment manager that is a singleton and is created early in
    app-server (before skill manager, before config loading).
    
    Use an environment variable to point to a running exec server.
  • Split exec process into local and remote implementations (#15233)
    ## Summary
    - match the exec-process structure to filesystem PR #15232
    - expose `ExecProcess` on `Environment`
    - make `LocalProcess` the real implementation and `RemoteProcess` a thin
    network proxy over `ExecServerClient`
    - make `ProcessHandler` a thin RPC adapter delegating to `LocalProcess`
    - add a shared local/remote process test
    
    ## Validation
    - `just fmt`
    - `CARGO_TARGET_DIR=~/.cache/cargo-target/codex cargo test -p
    codex-exec-server`
    - `just fix -p codex-exec-server`
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • Refactor ExecServer filesystem split between local and remote (#15232)
    For each feature we have:
    1. Trait exposed on environment
    2. **Local Implementation** of the trait
    3. Remote implementation that uses the client to proxy via network
    4. Handler implementation that handles PRC requests and calls into
    **Local Implementation**
  • Add exec-server exec RPC implementation (#15090)
    Stacked PR 2/3, based on the stub PR.
    
    Adds the exec RPC implementation and process/event flow in exec-server
    only.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • Move environment abstraction into exec server (#15125)
    The idea is that codex-exec exposes an Environment struct with services
    on it. Each of those is a trait.
    
    Depending on construction parameters passed to Environment they are
    either backed by local or remote server but core doesn't see these
    differences.
  • Remove stdio transport from exec server (#15119)
    Summary
    - delete the deprecated stdio transport plumbing from the exec server
    stack
    - add a basic `exec_server()` harness plus test utilities to start a
    server, send requests, and await events
    - refresh exec-server dependencies, configs, and documentation to
    reflect the new flow
    
    Testing
    - Not run (not requested)
    
    ---------
    
    Co-authored-by: starr-openai <starr@openai.com>
    Co-authored-by: Codex <noreply@openai.com>
  • Add exec-server stub server and protocol docs (#15089)
    Stacked PR 1/3.
    
    This is the initialize-only exec-server stub slice: binary/client
    scaffolding and protocol docs, without exec/filesystem implementation.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • refactor: delete exec-server and move execve wrapper into shell-escalation (#12632)
    ## Why
    
    We already plan to remove the shell-tool MCP path, and doing that
    cleanup first makes the follow-on `shell-escalation` work much simpler.
    
    This change removes the last remaining reason to keep
    `codex-rs/exec-server` around by moving the `codex-execve-wrapper`
    binary and shared shell test fixtures to the crates/tests that now own
    that functionality.
    
    ## What Changed
    
    ### Delete `codex-rs/exec-server`
    
    - Remove the `exec-server` crate, including the MCP server binary,
    MCP-specific modules, and its test support/test suite
    - Remove `exec-server` from the `codex-rs` workspace and update
    `Cargo.lock`
    
    ### Move `codex-execve-wrapper` into `codex-rs/shell-escalation`
    
    - Move the wrapper implementation into `shell-escalation`
    (`src/unix/execve_wrapper.rs`)
    - Add the `codex-execve-wrapper` binary entrypoint under
    `shell-escalation/src/bin/`
    - Update `shell-escalation` exports/module layout so the wrapper
    entrypoint is hosted there
    - Move the wrapper README content from `exec-server` to
    `shell-escalation/README.md`
    
    ### Move shared shell test fixtures to `app-server`
    
    - Move the DotSlash `bash`/`zsh` test fixtures from
    `exec-server/tests/suite/` to `app-server/tests/suite/`
    - Update `app-server` zsh-fork tests to reference the new fixture paths
    
    ### Keep `shell-tool-mcp` as a shell-assets package
    
    - Update `.github/workflows/shell-tool-mcp.yml` packaging so the npm
    artifact contains only patched Bash/Zsh payloads (no Rust binaries)
    - Update `shell-tool-mcp/package.json`, `shell-tool-mcp/src/index.ts`,
    and docs to reflect the shell-assets-only package shape
    - `shell-tool-mcp-ci.yml` does not need changes because it is already
    JS-only
    
    ## Verification
    
    - `cargo shear`
    - `cargo clippy -p codex-shell-escalation --tests`
    - `just clippy`
  • refactor: normalize unix module layout for exec-server and shell-escalation (#12556)
    ## Why
    Shell execution refactoring in `exec-server` had become split between
    duplicated code paths, which blocked a clean introduction of the new
    reusable shell escalation flow. This commit creates a dedicated
    foundation crate so later shell tooling changes can share one
    implementation.
    
    ## What changed
    - Added the `codex-shell-escalation` crate and moved the core escalation
    pieces (`mcp` protocol/socket/session flow, policy glue) that were
    previously in `exec-server` into it.
    - Normalized `exec-server` Unix structure under a dedicated `unix`
    module layout and kept non-Unix builds narrow.
    - Wired crate/build metadata so `shell-escalation` is a first-class
    workspace dependency for follow-on integration work.
    
    ## Verification
    - Built and linted the stack at this commit point with `just clippy`.
    
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/12556).
    * #12584
    * #12583
    * __->__ #12556
  • fix: add integration tests for codex-exec-mcp-server with execpolicy (#7617)
    This PR introduces integration tests that run
    [codex-shell-tool-mcp](https://www.npmjs.com/package/@openai/codex-shell-tool-mcp)
    as a user would. Note that this requires running our fork of Bash, so we
    introduce a [DotSlash](https://dotslash-cli.com/) file for `bash` so
    that we can run the integration tests on multiple platforms without
    having to check the binaries into the repository. (As noted in the
    DotSlash file, it is slightly more heavyweight than necessary, which may
    be worth addressing as disk space in CI is limited:
    https://github.com/openai/codex/pull/7678.)
    
    To start, this PR adds two tests:
    
    - `list_tools()` makes the `list_tools` request to the MCP server and
    verifies we get the expected response
    - `accept_elicitation_for_prompt_rule()` defines a `prefix_rule()` with
    `decision="prompt"` and verifies the elicitation flow works as expected
    
    Though the `accept_elicitation_for_prompt_rule()` test **only works on
    Linux**, as this PR reveals that there are currently issues when running
    the Bash fork in a read-only sandbox on Linux. This will have to be
    fixed in a follow-up PR.
    
    Incidentally, getting this test run to correctly on macOS also requires
    a recent fix we made to `brew` that hasn't hit a mainline release yet,
    so getting CI green in this PR required
    https://github.com/openai/codex/pull/7680.
  • chore: refactor exec-server to prepare it for standalone MCP use (#6944)
    This PR reorganizes things slightly so that:
    
    - Instead of a single multitool executable, `codex-exec-server`, we now
    have two executables:
      - `codex-exec-mcp-server` to launch the MCP server
    - `codex-execve-wrapper` is the `execve(2)` wrapper to use with the
    `BASH_EXEC_WRAPPER` environment variable
    - `BASH_EXEC_WRAPPER` must be a single executable: it cannot be a
    command string composed of an executable with args (i.e., it no longer
    adds the `escalate` subcommand, as before)
    - `codex-exec-mcp-server` takes `--bash` and `--execve` as options.
    Though if `--execve` is not specified, the MCP server will check the
    directory containing `std::env::current_exe()` and attempt to use the
    file named `codex-execve-wrapper` within it. In development, this works
    out since these executables are side-by-side in the `target/debug`
    folder.
    
    With respect to testing, this also fixes an important bug in
    `dummy_exec_policy()`, as I was using `ends_with()` as if it applied to
    a `String`, but in this case, it is used with a `&Path`, so the
    semantics are slightly different.
    
    Putting this all together, I was able to test this by running the
    following:
    
    ```
    ~/code/codex/codex-rs$ npx @modelcontextprotocol/inspector \
        ./target/debug/codex-exec-mcp-server --bash ~/code/bash/bash
    ```
    
    If I try to run `git status` in `/Users/mbolin/code/codex` via the
    `shell` tool from the MCP server:
    
    <img width="1589" height="1335" alt="image"
    src="https://github.com/user-attachments/assets/9db6aea8-7fbc-4675-8b1f-ec446685d6c4"
    />
    
    then I get prompted with the following elicitation, as expected:
    
    <img width="1589" height="1335" alt="image"
    src="https://github.com/user-attachments/assets/21b68fe0-494d-4562-9bad-0ddc55fc846d"
    />
    
    Though a current limitation is that the `shell` tool defaults to a
    timeout of 10s, which means I only have 10s to respond to the
    elicitation. Ideally, the time spent waiting for a response from a human
    should not count against the timeout for the command execution. I will
    address this in a subsequent PR.
    
    ---
    
    Note `~/code/bash/bash` was created by doing:
    
    ```
    cd ~/code
    git clone https://github.com/bminor/bash
    cd bash
    git checkout a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b
    <apply the patch below>
    ./configure
    make
    ```
    
    The patch:
    
    ```
    diff --git a/execute_cmd.c b/execute_cmd.c
    index 070f5119..d20ad2b9 100644
    --- a/execute_cmd.c
    +++ b/execute_cmd.c
    @@ -6129,6 +6129,19 @@ shell_execve (char *command, char **args, char **env)
       char sample[HASH_BANG_BUFSIZ];
       size_t larray;
    
    +  char* exec_wrapper = getenv("BASH_EXEC_WRAPPER");
    +  if (exec_wrapper && *exec_wrapper && !whitespace (*exec_wrapper))
    +    {
    +      char *orig_command = command;
    +
    +      larray = strvec_len (args);
    +
    +      memmove (args + 2, args, (++larray) * sizeof (char *));
    +      args[0] = exec_wrapper;
    +      args[1] = orig_command;
    +      command = exec_wrapper;
    +    }
    +
    ```