Commit Graph

10 Commits

Author SHA1 Message Date
Michael Bolin
fcd197d596 fix: use std::env::args_os instead of std::env::args (#1698)
Apparently `std::env::args()` will panic during iteration if any
argument to the process is not valid Unicode:

https://doc.rust-lang.org/std/env/fn.args.html

Let's avoid the risk and just go with `std::env::args_os()`.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1698).
* #1705
* #1703
* #1702
* __->__ #1698
* #1697
2025-07-28 08:52:18 -07:00
Michael Bolin
e78ec00e73 chore: support MCP schema 2025-06-18 (#1621)
This updates the schema in `generate_mcp_types.py` from `2025-03-26` to
`2025-06-18`, regenerates `mcp-types/src/lib.rs`, and then updates all
the code that uses `mcp-types` to honor the changes.

Ran

```
npx @modelcontextprotocol/inspector just codex mcp
```

and verified that I was able to invoke the `codex` tool, as expected.


---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1621).
* #1623
* #1622
* __->__ #1621
2025-07-19 00:09:34 -04:00
Michael Bolin
392fdd7db6 fix: honor RUST_LOG in mcp-client CLI and default to DEBUG (#1149)
We had `debug!()` logging statements already, but they weren't being
printed because `tracing_subscriber` was not set up.
2025-05-28 17:10:06 -07:00
Michael Bolin
ec5e82b77c chore: pin Rust version to 1.86 and use io::Error::other to prepare for 1.87 (#947)
Previously, our GitHub actions specified the Rust toolchain as
`dtolnay/rust-toolchain@stable`, which meant the version could change
out from under us. In this case, the move from 1.86 to 1.87 introduced
new clippy warnings, causing build failures.

Because it will take a little time to fix all the new clippy warnings,
this PR pins things to 1.86 for now to unbreak the build.

It also replaces `io::Error::new(io::ErrorKind::Other)` with
`io::Error::other()` in preparation for 1.87.
2025-05-15 14:07:16 -07:00
Michael Bolin
115fb0b95d fix: navigate initialization phase before tools/list request in MCP client (#904)
Apparently the MCP server implemented in JavaScript did not require the
`initialize` handshake before responding to tool list/call, so I missed
this.
2025-05-12 15:15:26 -07:00
Michael Bolin
fde48aaa0d feat: experimental env var: CODEX_SANDBOX_NETWORK_DISABLED (#879)
When using Codex to develop Codex itself, I noticed that sometimes it
would try to add `#[ignore]` to the following tests:

```
keeps_previous_response_id_between_tasks()
retries_on_early_close()
```

Both of these tests start a `MockServer` that launches an HTTP server on
an ephemeral port and requires network access to hit it, which the
Seatbelt policy associated with `--full-auto` correctly denies. If I
wasn't paying attention to the code that Codex was generating, one of
these `#[ignore]` annotations could have slipped into the codebase,
effectively disabling the test for everyone.

To that end, this PR enables an experimental environment variable named
`CODEX_SANDBOX_NETWORK_DISABLED` that is set to `1` if the
`SandboxPolicy` used to spawn the process does not have full network
access. I say it is "experimental" because I'm not convinced this API is
quite right, but we need to start somewhere. (It might be more
appropriate to have an env var like `CODEX_SANDBOX=full-auto`, but the
challenge is that our newer `SandboxPolicy` abstraction does not map to
a simple set of enums like in the TypeScript CLI.)

We leverage this new functionality by adding the following code to the
aforementioned tests as a way to "dynamically disable" them:

```rust
if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
    println!(
        "Skipping test because it cannot execute when network is disabled in a Codex sandbox."
    );
    return;
}
```

We can use the `debug seatbelt --full-auto` command to verify that
`cargo test` fails when run under Seatbelt prior to this change:

```
$ cargo run --bin codex -- debug seatbelt --full-auto -- cargo test
---- keeps_previous_response_id_between_tasks stdout ----

thread 'keeps_previous_response_id_between_tasks' panicked at /Users/mbolin/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/wiremock-0.6.3/src/mock_server/builder.rs:107:46:
Failed to bind an OS port for a mock server.: Os { code: 1, kind: PermissionDenied, message: "Operation not permitted" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    keeps_previous_response_id_between_tasks

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `-p codex-core --test previous_response_id`
```

Though after this change, the above command succeeds! This means that,
going forward, when Codex operates on Codex itself, when it runs `cargo
test`, only "real failures" should cause the command to fail.

As part of this change, I decided to tighten up the codepaths for
running `exec()` for shell tool calls. In particular, we do it in `core`
for the main Codex business logic itself, but we also expose this logic
via `debug` subcommands in the CLI in the `cli` crate. The logic for the
`debug` subcommands was not quite as faithful to the true business logic
as I liked, so I:

* refactored a bit of the Linux code, splitting `linux.rs` into
`linux_exec.rs` and `landlock.rs` in the `core` crate.
* gating less code behind `#[cfg(target_os = "linux")]` because such
code does not get built by default when I develop on Mac, which means I
either have to build the code in Docker or wait for CI signal
* introduced `macro_rules! configure_command` in `exec.rs` so we can
have both sync and async versions of this code. The synchronous version
seems more appropriate for straight threads or potentially fork/exec.
2025-05-09 18:29:34 -07:00
Michael Bolin
9da6ebef3f fix: add optional timeout to McpClient::send_request() (#852)
We now impose a 10s timeout on the initial `tools/list` request to an
MCP server. We do not apply a timeout for other types of requests yet,
but we should start enforcing those, as well.
2025-05-07 12:56:38 -07:00
jcoens-openai
8a89d3aeda Update cargo to 2024 edition (#842)
Some effects of this change:
- New formatting changes across many files. No functionality changes
should occur from that.
- Calls to `set_env` are considered unsafe, since this only happens in
tests we wrap them in `unsafe` blocks
2025-05-07 08:37:48 -07:00
Michael Bolin
5f1b8f707c feat: update McpClient::new_stdio_client() to accept an env (#831)
Cleans up the signature for `new_stdio_client()` to more closely mirror
how MCP servers are declared in config files (`command`, `args`, `env`).
Also takes a cue from Claude Code where the MCP server is launched with
a restricted `env` so that it only includes "safe" things like `USER`
and `PATH` (see the `create_env_for_mcp_server()` function introduced in
this PR for details) by default, as it is common for developers to have
sensitive API keys present in their environment that should only be
forwarded to the MCP server when the user has explicitly configured it
to do so.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/831).
* #829
* __->__ #831
2025-05-06 11:14:47 -07:00
Michael Bolin
2cf7aeeeb6 feat: initial McpClient for Rust (#822)
This PR introduces an initial `McpClient` that we will use to give Codex
itself programmatic access to foreign MCPs. This does not wire it up in
Codex itself yet, but the new `mcp-client` crate includes a `main.rs`
for basic testing for now.

Manually tested by sending a `tools/list` request to Codex's own MCP
server:

```
codex-rs$ cargo build
codex-rs$ cargo run --bin codex-mcp-client ./target/debug/codex-mcp-server
{
  "tools": [
    {
      "description": "Run a Codex session. Accepts configuration parameters matching the Codex Config struct.",
      "inputSchema": {
        "properties": {
          "approval-policy": {
            "description": "Execution approval policy expressed as the kebab-case variant name (`unless-allow-listed`, `auto-edit`, `on-failure`, `never`).",
            "enum": [
              "auto-edit",
              "unless-allow-listed",
              "on-failure",
              "never"
            ],
            "type": "string"
          },
          "cwd": {
            "description": "Working directory for the session. If relative, it is resolved against the server process's current working directory.",
            "type": "string"
          },
          "disable-response-storage": {
            "description": "Disable server-side response storage.",
            "type": "boolean"
          },
          "model": {
            "description": "Optional override for the model name (e.g. \"o3\", \"o4-mini\")",
            "type": "string"
          },
          "prompt": {
            "description": "The *initial user prompt* to start the Codex conversation.",
            "type": "string"
          },
          "sandbox-permissions": {
            "description": "Sandbox permissions using the same string values accepted by the CLI (e.g. \"disk-write-cwd\", \"network-full-access\").",
            "items": {
              "enum": [
                "disk-full-read-access",
                "disk-write-cwd",
                "disk-write-platform-user-temp-folder",
                "disk-write-platform-global-temp-folder",
                "disk-full-write-access",
                "network-full-access"
              ],
              "type": "string"
            },
            "type": "array"
          }
        },
        "required": [
          "prompt"
        ],
        "type": "object"
      },
      "name": "codex"
    }
  ]
}
```
2025-05-05 12:52:55 -07:00