25 Commits

• [codex] Use expect in integration tests (#28441)

  The workspace denies `clippy::expect_used` in production. Although
  `clippy.toml` allows `expect` in tests, Bazel Clippy compiles
  integration-test helper code in a way that does not receive that
  exemption, which encouraged verbose `unwrap_or_else(... panic!(...))`
  and equivalent `match`/`let else` forms.
  
  This allows `clippy::expect_used` once at each integration-test crate
  root (including aggregated suites and test-support libraries), then
  replaces manual panic-based Result and Option unwraps with
  `expect`/`expect_err`. Standalone `tests/*.rs` files remain their own
  crate roots. Intentional assertion and unexpected-variant panics remain
  unchanged, and the production `expect_used = "deny"` lint remains in
  place.
  
  The cleanup is mechanical and net-negative in line count.

  pakrym-oai · 2026-06-15 21:53:47 -07:00
  e752f7b4ae
• Support multi-environment apply_patch selection (#21617)

  ## Summary
  - add multi-environment apply_patch routing for both freeform and
  function-call tool flows
  - parse and reconcile the optional environment selector in the main
  apply_patch parser, then verify against the selected environment in the
  handler
  - carry environment_id through runtime and approval surfaces so
  remote-targeted patches stay explicit end to end
  
  ## Testing
  - just fmt
  - remote exec-server e2e: `cargo test -p codex-core --test all
  apply_patch_multi_environment_uses_remote_executor -- --nocapture` on
  dev via `scripts/test-remote-env.sh`
  
  ---------
  
  Co-authored-by: Codex <noreply@openai.com>

  starr-openai · 2026-05-11 16:33:44 -07:00
  22e84c49d0
• Stabilize Bazel tests (timeout tweaks and flake fixes) (#17791)
  David de Regt · 2026-04-16 07:57:51 -07:00
  6adba99f4d
• fix: increase timeout to account for slow PowerShell startup (#16608)

  Similar to https://github.com/openai/codex/pull/16604, I am seeing
  failures on Windows Bazel that could be due to PowerShell startup
  timeouts, so try increasing.

  Michael Bolin · 2026-04-02 12:40:19 -07:00
  5d64e58a38
• chore: clean up argument-comment lint and roll out all-target CI on macOS (#16054)

  ## Why
  
  `argument-comment-lint` was green in CI even though the repo still had
  many uncommented literal arguments. The main gap was target coverage:
  the repo wrapper did not force Cargo to inspect test-only call sites, so
  examples like the `latest_session_lookup_params(true, ...)` tests in
  `codex-rs/tui_app_server/src/lib.rs` never entered the blocking CI path.
  
  This change cleans up the existing backlog, makes the default repo lint
  path cover all Cargo targets, and starts rolling that stricter CI
  enforcement out on the platform where it is currently validated.
  
  ## What changed
  
  - mechanically fixed existing `argument-comment-lint` violations across
  the `codex-rs` workspace, including tests, examples, and benches
  - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` and
  `tools/argument-comment-lint/run.sh` so non-`--fix` runs default to
  `--all-targets` unless the caller explicitly narrows the target set
  - fixed both wrappers so forwarded cargo arguments after `--` are
  preserved with a single separator
  - documented the new default behavior in
  `tools/argument-comment-lint/README.md`
  - updated `rust-ci` so the macOS lint lane keeps the plain wrapper
  invocation and therefore enforces `--all-targets`, while Linux and
  Windows temporarily pass `-- --lib --bins`
  
  That temporary CI split keeps the stricter all-targets check where it is
  already cleaned up, while leaving room to finish the remaining Linux-
  and Windows-specific target-gated cleanup before enabling
  `--all-targets` on those runners. The Linux and Windows failures on the
  intermediate revision were caused by the wrapper forwarding bug, not by
  additional lint findings in those lanes.
  
  ## Validation
  
  - `bash -n tools/argument-comment-lint/run.sh`
  - `bash -n tools/argument-comment-lint/run-prebuilt-linter.sh`
  - shell-level wrapper forwarding check for `-- --lib --bins`
  - shell-level wrapper forwarding check for `-- --tests`
  - `just argument-comment-lint`
  - `cargo test` in `tools/argument-comment-lint`
  - `cargo test -p codex-terminal-detection`
  
  ## Follow-up
  
  - Clean up remaining Linux-only target-gated callsites, then switch the
  Linux lint lane back to the plain wrapper invocation.
  - Clean up remaining Windows-only target-gated callsites, then switch
  the Windows lint lane back to the plain wrapper invocation.

  Michael Bolin · 2026-03-27 19:00:44 -07:00
  61dfe0b86c
• Stabilize shell approval MCP test (#14101)

  ## Summary
  - replace the Python-based file creation command in the MCP shell
  approval test with native platform commands
  - build the expected command string from the exact argv that the test
  sends
  
  ## Why this fixes the flake
  The old test depended on Python startup and shell quoting details that
  varied across runners. The new version still verifies the same approval
  flow, but it uses `touch` on Unix and `New-Item` on Windows so the
  assertion only depends on the MCP shell command that Codex actually
  forwards.

  Ahmed Ibrahim · 2026-03-09 11:18:26 -07:00
  203a70a191
• chore: remove codex-core public protocol/shell re-exports (#12432)

  ## Why
  
  `codex-rs/core/src/lib.rs` re-exported a broad set of types and modules
  from `codex-protocol` and `codex-shell-command`. That made it easy for
  workspace crates to import those APIs through `codex-core`, which in
  turn hides dependency edges and makes it harder to reduce compile-time
  coupling over time.
  
  This change removes those public re-exports so call sites must import
  from the source crates directly. Even when a crate still depends on
  `codex-core` today, this makes dependency boundaries explicit and
  unblocks future work to drop `codex-core` dependencies where possible.
  
  ## What Changed
  
  - Removed public re-exports from `codex-rs/core/src/lib.rs` for:
  - `codex_protocol::protocol` and related protocol/model types (including
  `InitialHistory`)
    - `codex_protocol::config_types` (`protocol_config_types`)
  - `codex_shell_command::{bash, is_dangerous_command, is_safe_command,
  parse_command, powershell}`
  - Migrated workspace Rust call sites to import directly from:
    - `codex_protocol::protocol`
    - `codex_protocol::config_types`
    - `codex_protocol::models`
    - `codex_shell_command`
  - Added explicit `Cargo.toml` dependencies (`codex-protocol` /
  `codex-shell-command`) in crates that now import those crates directly.
  - Kept `codex-core` internal modules compiling by using `pub(crate)`
  aliases in `core/src/lib.rs` (internal-only, not part of the public
  API).
  - Updated the two utility crates that can already drop a `codex-core`
  dependency edge entirely:
    - `codex-utils-approval-presets`
    - `codex-utils-cli`
  
  ## Verification
  
  - `cargo test -p codex-utils-approval-presets`
  - `cargo test -p codex-utils-cli`
  - `cargo check --workspace --all-targets`
  - `just clippy`

  Michael Bolin · 2026-02-20 23:45:35 -08:00
  1af2a37ada
• chore: rm remote models fflag (#11699)

  rm `remote_models` feature flag.
  
  We see issues like #11527 when a user has `remote_models` disabled, as
  we always use the default fallback `ModelInfo`. This causes issues with
  model performance.
  
  Builds on #11690, which helps by warning the user when they are using
  the default fallback. This PR will make that happen much less frequently
  as an accidental consequence of disabling `remote_models`.

  sayan-oai · 2026-02-17 11:43:16 -08:00
  41800fc876
• Fix flaky windows CI test (#10993)

  Hardens PTY Python REPL test and make MCP test startup deterministic
  
  **Summary**
  - `utils/pty/src/tests.rs`
  - Added a REPL readiness handshake (`wait_for_python_repl_ready`) that
  repeatedly sends a marker and waits for it in PTY output before sending
  test commands.
    - Updated `pty_python_repl_emits_output_and_exits` to:
      - wait for readiness first,
      - preserve startup output,
      - append output collected through process exit.
  - Reduces Windows/ConPTY flakiness from early stdin writes racing REPL
  startup.
  
  - `mcp-server/tests/suite/codex_tool.rs`
  - Avoid remote model refresh during MCP test startup, reducing
  timeout-prone nondeterminism.

  Eric Traut · 2026-02-07 08:55:42 -08:00
  10336068db
• chore: nuke chat/completions API (#10157)
  jif-oai · 2026-02-03 11:31:57 +00:00
  d2394a2494
• feat: replace custom mcp-types crate with equivalents from rmcp (#10349)

  We started working with MCP in Codex before
  https://crates.io/crates/rmcp was mature, so we had our own crate for
  MCP types that was generated from the MCP schema:
  
  
  https://github.com/openai/codex/blob/8b95d3e082376f4cb23e92641705a22afb28a9da/codex-rs/mcp-types/README.md
  
  Now that `rmcp` is more mature, it makes more sense to use their MCP
  types in Rust, as they handle details (like the `_meta` field) that our
  custom version ignored. Though one advantage that our custom types had
  is that our generated types implemented `JsonSchema` and `ts_rs::TS`,
  whereas the types in `rmcp` do not. As such, part of the work of this PR
  is leveraging the adapters between `rmcp` types and the serializable
  types that are API for us (app server and MCP) introduced in #10356.
  
  Note this PR results in a number of changes to
  `codex-rs/app-server-protocol/schema`, which merit special attention
  during review. We must ensure that these changes are still
  backwards-compatible, which is possible because we have:
  
  ```diff
  - export type CallToolResult = { content: Array<ContentBlock>, isError?: boolean, structuredContent?: JsonValue, };
  + export type CallToolResult = { content: Array<JsonValue>, structuredContent?: JsonValue, isError?: boolean, _meta?: JsonValue, };
  ```
  
  so `ContentBlock` has been replaced with the more general `JsonValue`.
  Note that `ContentBlock` was defined as:
  
  ```typescript
  export type ContentBlock = TextContent | ImageContent | AudioContent | ResourceLink | EmbeddedResource;
  ```
  
  so the deletion of those individual variants should not be a cause of
  great concern.
  
  Similarly, we have the following change in
  `codex-rs/app-server-protocol/schema/typescript/Tool.ts`:
  
  ```
  - export type Tool = { annotations?: ToolAnnotations, description?: string, inputSchema: ToolInputSchema, name: string, outputSchema?: ToolOutputSchema, title?: string, };
  + export type Tool = { name: string, title?: string, description?: string, inputSchema: JsonValue, outputSchema?: JsonValue, annotations?: JsonValue, icons?: Array<JsonValue>, _meta?: JsonValue, };
  ```
  
  so:
  
  - `annotations?: ToolAnnotations` ➡️ `JsonValue`
  - `inputSchema: ToolInputSchema` ➡️ `JsonValue`
  - `outputSchema?: ToolOutputSchema` ➡️ `JsonValue`
  
  and two new fields: `icons?: Array<JsonValue>, _meta?: JsonValue`
  
  ---
  [//]: # (BEGIN SAPLING FOOTER)
  Stack created with [Sapling](https://sapling-scm.com). Best reviewed
  with [ReviewStack](https://reviewstack.dev/openai/codex/pull/10349).
  * #10357
  * __->__ #10349
  * #10356

  Michael Bolin · 2026-02-02 17:41:55 -08:00
  66447d5d2c
• fix(mcp): include threadId in both content and structuredContent in CallToolResult (#9338)
  Michael Bolin · 2026-01-15 18:33:11 -08:00
  99f47d6e9a
• feat: add threadId to MCP server messages (#9192)

  This favors `threadId` instead of `conversationId` so we use the same
  terms as https://developers.openai.com/codex/sdk/.
  
  To test the local build:
  
  ```
  cd codex-rs
  cargo build --bin codex
  npx -y @modelcontextprotocol/inspector ./target/debug/codex mcp-server
  ```
  
  I sent:
  
  ```json
  {
    "method": "tools/call",
    "params": {
      "name": "codex",
      "arguments": {
        "prompt": "favorite ls option?"
      },
      "_meta": {
        "progressToken": 0
      }
    }
  }
  ```
  
  and got:
  
  ```json
  {
    "content": [
      {
        "type": "text",
        "text": "`ls -lah` (or `ls -alh`) — long listing, includes dotfiles, human-readable sizes."
      }
    ],
    "structuredContent": {
      "threadId": "019bbb20-bff6-7130-83aa-bf45ab33250e"
    }
  }
  ```
  
  and successfully used the `threadId` in the follow-up with the
  `codex-reply` tool call:
  
  ```json
  {
    "method": "tools/call",
    "params": {
      "name": "codex-reply",
      "arguments": {
        "prompt": "what is the long versoin",
        "threadId": "019bbb20-bff6-7130-83aa-bf45ab33250e"
      },
      "_meta": {
        "progressToken": 1
      }
    }
  }
  ```
  
  whose response also has the `threadId`:
  
  ```json
  {
    "content": [
      {
        "type": "text",
        "text": "Long listing is `ls -l` (adds permissions, owner/group, size, timestamp)."
      }
    ],
    "structuredContent": {
      "threadId": "019bbb20-bff6-7130-83aa-bf45ab33250e"
    }
  }
  ```
  
  Fixes https://github.com/openai/codex/issues/3712.

  Michael Bolin · 2026-01-13 22:14:41 -08:00
  0c09dc3c03
• Assemble sandbox/approval/network prompts dynamically (#8961)

  - Add a single builder for developer permissions messaging that accepts
  SandboxPolicy and approval policy. This builder now drives the developer
  “permissions” message that’s injected at session start and any time
  sandbox/approval settings change.
  - Trim EnvironmentContext to only include cwd, writable roots, and
  shell; removed sandbox/approval/network duplication and adjusted XML
  serialization and tests accordingly.
  
  Follow-up: adding a config value to replace the developer permissions
  message for custom sandboxes.

  Ahmed Ibrahim · 2026-01-12 23:12:59 +00:00
  87f7226cca
• Removed experimental "command risk assessment" feature (#7799)

  This experimental feature received lukewarm reception during internal
  testing. Removing from the code base.

  Eric Traut · 2025-12-10 09:48:11 -08:00
  c4af707e09
• Migrate coverage to shell_command (#7042)
  pakrym-oai · 2025-11-21 03:44:00 +00:00
  767b66f407
• [codex] add developer instructions (#5897)

  we are using developer instructions for code reviews, we need to pass
  them in cli as well.

  Anton Panasenko · 2025-10-30 11:18:31 -07:00
  9572cfc782
• Added model summary and risk assessment for commands that violate sandbox policy (#5536)

  This PR adds support for a model-based summary and risk assessment for
  commands that violate the sandbox policy and require user approval. This
  aids the user in evaluating whether the command should be approved.
  
  The feature works by taking a failed command and passing it back to the
  model and asking it to summarize the command, give it a risk level (low,
  medium, high) and a risk category (e.g. "data deletion" or "data
  exfiltration"). It uses a new conversation thread so the context in the
  existing thread doesn't influence the answer. If the call to the model
  fails or takes longer than 5 seconds, it falls back to the current
  behavior.
  
  For now, this is an experimental feature and is gated by a config key
  `experimental_sandbox_command_assessment`.
  
  Here is a screen shot of the approval prompt showing the risk assessment
  and summary.
  
  <img width="723" height="282" alt="image"
  src="https://github.com/user-attachments/assets/4597dd7c-d5a0-4e9f-9d13-414bd082fd6b"
  />

  Eric Traut · 2025-10-24 15:23:44 -07:00
  f8af4f5c8d
• chore: rework tools execution workflow (#5278)

  Re-work the tool execution flow. Read `orchestrator.rs` to understand
  the structure

  jif-oai · 2025-10-20 20:57:37 +01:00
  5e4f3bbb0b
• feat: add Vec<ParsedCommand> to ExecApprovalRequestEvent (#5222)

  This adds `parsed_cmd: Vec<ParsedCommand>` to `ExecApprovalRequestEvent`
  in the core protocol (`protocol/src/protocol.rs`), which is also what
  this field is named on `ExecCommandBeginEvent`. Honestly, I don't love
  the name (it sounds like a single command, but it is actually a list of
  them), but I don't want to get distracted by a naming discussion right
  now.
  
  This also adds `parsed_cmd` to `ExecCommandApprovalParams` in
  `codex-rs/app-server-protocol/src/protocol.rs`, so it will be available
  via `codex app-server`, as well.
  
  For consistency, I also updated `ExecApprovalElicitRequestParams` in
  `codex-rs/mcp-server/src/exec_approval.rs` to include this field under
  the name `codex_parsed_cmd`, as that struct already has a number of
  special `codex_*` fields. Note this is the code for when Codex is used
  as an MCP _server_ and therefore has to conform to the official spec for
  an MCP elicitation type.

  Michael Bolin · 2025-10-15 13:58:40 -07:00
  995f5c3614
• make tests pass cleanly in sandbox (#4067)

  This changes the reqwest client used in tests to be sandbox-friendly,
  and skips a bunch of other tests that don't work inside the
  sandbox/without network.

  Jeremy Rose · 2025-09-25 13:11:14 -07:00
  4a5f05c136
• chore: clippy on redundant closure (#4058)

  Add redundant closure clippy rules and let Codex fix it by minimising
  FQP

  jif-oai · 2025-09-22 19:30:16 +00:00
  be366a31ab
• Add non_sandbox_test helper (#3880)

  Makes tests shorter

  pakrym-oai · 2025-09-22 14:50:41 +00:00
  14a115d488
• chore: try to make it easier to debug the flakiness of test_shell_command_approval_triggers_elicitation (#2848)

  `test_shell_command_approval_triggers_elicitation()` is one of a number
  of integration tests that we have observed to be flaky on GitHub CI, so
  this PR tries to reduce the flakiness _and_ to provide us with more
  information when it flakes. Specifically:
  
  - Changed the command that we use to trigger the elicitation from `git
  init` to `python3 -c 'import pathlib; pathlib.Path(r"{}").touch()'`
  because running `git` seems more likely to invite variance.
  - Increased the timeout to wait for the task response from 10s to 20s.
  - Added more logging.

  Michael Bolin · 2025-08-28 12:33:33 -07:00
  1e9e703b96
• test: faster test execution in codex-core (#2633)

  this dramatically improves time to run `cargo test -p codex-core` (~25x
  speedup).
  
  before:
  ```
  cargo test -p codex-core  35.96s user 68.63s system 19% cpu 8:49.80 total
  ```
  
  after:
  ```
  cargo test -p codex-core  5.51s user 8.16s system 63% cpu 21.407 total
  ```
  
  both tests measured "hot", i.e. on a 2nd run with no filesystem changes,
  to exclude compile times.
  
  approach inspired by [Delete Cargo Integration
  Tests](https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html),
  we move all test cases in tests/ into a single suite in order to have a
  single binary, as there is significant overhead for each test binary
  executed, and because test execution is only parallelized with a single
  binary.

  Jeremy Rose · 2025-08-24 11:10:53 -07:00
  32bbbbad61