Commit Graph

169 Commits

  • Add OSS model info (#1860)
    Add somewhat arbitrarily chosen context window/output limit.
  • [approval_policy] Add OnRequest approval_policy (#1865)
    ## Summary
    A split-up PR of #1763 , stacked on top of a tools refactor #1858 to
    make the change clearer. From the previous summary:
    
    > Let's try something new: tell the model about the sandbox, and let it
    decide when it will need to break the sandbox. Some local testing
    suggests that it works pretty well with zero iteration on the prompt!
    
    ## Testing
    - [x] Added unit tests
    - [x] Tested locally and it appears to work smoothly!
  • [core] Separate tools config from openai client (#1858)
    ## Summary
    In an effort to make tools easier to work with and more configurable,
    I'm introducing `ToolConfig` and updating `Prompt` to take in a general
    list of Tools. I think this is simpler and better for a few reasons:
    - We can easily assemble tools from various sources (our own harness,
    mcp servers, etc.) and we can consolidate the logic for constructing the
    logic in one place that is separate from serialization.
    - client.rs no longer needs arbitrary config values, it just takes in a
    list of tools to serialize
    
    A hefty portion of the PR is now updating our conversion of
    `mcp_types::Tool` to `OpenAITool`, but considering that @bolinfest
    accurately called this out as a TODO long ago, I think it's time we
    tackled it.
    
    ## Testing
    - [x] Experimented locally, no changes, as expected
    - [x] Added additional unit tests
    - [x] Responded to rust-review
  • [core] Stop escalating timeouts (#1853)
    ## Summary
    Escalating out of sandbox is (almost always) not going to fix
    long-running commands timing out - therefore we should just pass the
    failure back to the model instead of asking the user to re-run a command
    that took a long time anyway.
    
    ## Testing
    - [x] Ran locally with a timeout and confirmed this worked as expected
  • chore: remove unnecessary default_ prefix (#1854)
    This prefix is not inline with the other fields on the `ConfigOverrides`
    struct.
  • fix: when using --oss, ensure correct configuration is threaded through correctly (#1859)
    This PR started as an investigation with the goal of eliminating the use
    of `unsafe { std::env::set_var() }` in `ollama/src/client.rs`, as
    setting environment variables in a multithreaded context is indeed
    unsafe and these tests were observed to be flaky, as a result.
    
    Though as I dug deeper into the issue, I discovered that the logic for
    instantiating `OllamaClient` under test scenarios was not quite right.
    In this PR, I aimed to:
    
    - share more code between the two creation codepaths,
    `try_from_oss_provider()` and `try_from_provider_with_base_url()`
    - use the values from `Config` when setting up Ollama, as we have
    various mechanisms for overriding config values, so we should be sure
    that we are always using the ultimate `Config` for things such as the
    `ModelProviderInfo` associated with the `oss` id
    
    Once this was in place,
    `OllamaClient::try_from_provider_with_base_url()` could be used in unit
    tests for `OllamaClient` so it was possible to create a properly
    configured client without having to set environment variables.
  • Introduce --oss flag to use gpt-oss models (#1848)
    This adds support for easily running Codex backed by a local Ollama
    instance running our new open source models. See
    https://github.com/openai/gpt-oss for details.
    
    If you pass in `--oss` you'll be prompted to install/launch ollama, and
    it will automatically download the 20b model and attempt to use it.
    
    We'll likely want to expand this with some options later to make the
    experience smoother for users who can't run the 20b or want to run the
    120b.
    
    Co-authored-by: Michael Bolin <mbolin@openai.com>
  • Rescue chat completion changes (#1846)
    https://github.com/openai/codex/pull/1835 has some messed up history.
    
    This adds support for streaming chat completions, which is useful for ollama. We should probably take a very skeptical eye to the code introduced in this PR.
    
    ---------
    
    Co-authored-by: Ahmed Ibrahim <aibrahim@openai.com>
  • [prompt] Update prompt.md (#1839)
    ## Summary
    Additional clarifications to our prompt. Still very concise, but we'll
    continue to add more here.
  • chore: introduce ModelFamily abstraction (#1838)
    To date, we have a number of hardcoded OpenAI model slug checks spread
    throughout the codebase, which makes it hard to audit the various
    special cases for each model. To mitigate this issue, this PR introduces
    the idea of a `ModelFamily` that has fields to represent the existing
    special cases, such as `supports_reasoning_summaries` and
    `uses_local_shell_tool`.
    
    There is a `find_family_for_model()` function that maps the raw model
    slug to a `ModelFamily`. This function hardcodes all the knowledge about
    the special attributes for each model. This PR then replaces the
    hardcoded model name checks with checks against a `ModelFamily`.
    
    Note `ModelFamily` is now available as `Config::model_family`. We should
    ultimately remove `Config::model` in favor of
    `Config::model_family::slug`.
  • Stream model responses (#1810)
    Stream models thoughts and responses instead of waiting for the whole
    thing to come through. Very rough right now, but I'm making the risk call to push through.
  • [prompts] Better user_instructions handling (#1836)
    ## Summary
    Our recent change in #1737 can sometimes lead to the model confusing
    AGENTS.md context as part of the message. But a little prompting and
    formatting can help fix this!
    
    ## Testing
    - Ran locally with a few different prompts to verify the model
    behaves well.
    - Updated unit tests
  • feat: accept custom instructions in profiles (#1803)
    Allows users to set their experimental_instructions_file in configs.
    
    For example the below enables experimental instructions when running
    `codex -p foo`.
    ```
    [profiles.foo]
    experimental_instructions_file = "/Users/foo/.codex/prompt.md"
    ```
    
    # Testing
    -  Running against a profile with experimental_instructions_file works.
    -  Running against a profile without experimental_instructions_file
    works.
    -  Running against no profile with experimental_instructions_file
    works.
    -  Running against no profile without experimental_instructions_file
    works.
  • Add a TurnDiffTracker to create a unified diff for an entire turn (#1770)
    This lets us show an accumulating diff across all patches in a turn.
    Refer to the docs for TurnDiffTracker for implementation details.
    
    There are multiple ways this could have been done and this felt like the
    right tradeoff between reliability and completeness:
    *Pros*
    * It will pick up all changes to files that the model touched including
    if they prettier or another command that updates them.
    * It will not pick up changes made by the user or other agents to files
    it didn't modify.
    
    *Cons*
    * It will pick up changes that the user made to a file that the model
    also touched
    * It will not pick up changes to codegen or files that were not modified
    with apply_patch
  • [sandbox] Filter out certain non-sandbox errors (#1804)
    ## Summary
    Users frequently complain about re-approving commands that have failed
    for non-sandbox reasons. We can't diagnose with complete accuracy which
    errors happened because of a sandbox failure, but we can start to
    eliminate some common simple cases.
    
    This PR captures the most common case I've seen, which is a `command not
    found` error.
    
    ## Testing
    - [x] Added unit tests
    - [x] Ran a few cases locally
  • fix command duration display (#1806)
    we were always displaying "0ms" before.
    
    <img width="731" height="101" alt="Screenshot 2025-08-02 at 10 51 22 PM"
    src="https://github.com/user-attachments/assets/f56814ed-b9a4-4164-9e78-181c60ce19b7"
    />
  • Fix MacOS multiprocessing by relaxing sandbox (#1808)
    The following test script fails in the codex sandbox:
    ```
    import multiprocessing
    from multiprocessing import Lock, Process
    
    def f(lock):
        with lock:
            print("Lock acquired in child process")
    
    if __name__ == '__main__':
        lock = Lock()
        p = Process(target=f, args=(lock,))
        p.start()
        p.join()
    ```
    
    with 
    ```
    Traceback (most recent call last):
      File "/Users/david.hao/code/codex/codex-rs/cli/test.py", line 9, in <module>
        lock = Lock()
               ^^^^^^
      File "/Users/david.hao/.local/share/uv/python/cpython-3.12.9-macos-aarch64-none/lib/python3.12/multiprocessing/context.py", line 68, in Lock
        return Lock(ctx=self.get_context())
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/david.hao/.local/share/uv/python/cpython-3.12.9-macos-aarch64-none/lib/python3.12/multiprocessing/synchronize.py", line 169, in __init__
        SemLock.__init__(self, SEMAPHORE, 1, 1, ctx=ctx)
      File "/Users/david.hao/.local/share/uv/python/cpython-3.12.9-macos-aarch64-none/lib/python3.12/multiprocessing/synchronize.py", line 57, in __init__
        sl = self._semlock = _multiprocessing.SemLock(
                             ^^^^^^^^^^^^^^^^^^^^^^^^^
    PermissionError: [Errno 1] Operation not permitted
    ```
    
    After reading, adding this line to the sandbox configs fixes things -
    MacOS multiprocessing appears to use sem_lock(), which opens an IPC
    which is considered a disk write even though no file is created. I
    interrogated ChatGPT about whether it's okay to loosen, and my
    impression after reading is that it is, although would appreciate a
    close look
    
    
    Breadcrumb: You can run `cargo run -- debug seatbelt --full-auto <cmd>`
    to test the sandbox
  • Fix compact (#1798)
    We are not recording the summary in the history.
  • feat: make .git read-only within a writable root when using Seatbelt (#1765)
    To make `--full-auto` safer, this PR updates the Seatbelt policy so that
    a `SandboxPolicy` with a `writable_root` that contains a `.git/`
    _directory_ will make `.git/` _read-only_ (though as a follow-up, we
    should also consider the case where `.git` is a _file_ with a `gitdir:
    /path/to/actual/repo/.git` entry that should also be protected).
    
    The two major changes in this PR:
    
    - Updating `SandboxPolicy::get_writable_roots_with_cwd()` to return a
    `Vec<WritableRoot>` instead of a `Vec<PathBuf>` where a `WritableRoot`
    can specify a list of read-only subpaths.
    - Updating `create_seatbelt_command_args()` to honor the read-only
    subpaths in `WritableRoot`.
    
    The logic to update the policy is a fairly straightforward update to
    `create_seatbelt_command_args()`, but perhaps the more interesting part
    of this PR is the introduction of an integration test in
    `tests/sandbox.rs`. Leveraging the new API in #1785, we test
    `SandboxPolicy` under various conditions, including ones where `$TMPDIR`
    is not readable, which is critical for verifying the new behavior.
    
    To ensure that Codex can run its own tests, e.g.:
    
    ```
    just codex debug seatbelt --full-auto -- cargo test if_git_repo_is_writable_root_then_dot_git_folder_is_read_only
    ```
    
    I had to introduce the use of `CODEX_SANDBOX=sandbox`, which is
    comparable to how `CODEX_SANDBOX_NETWORK_DISABLED=1` was already being
    used.
    
    Adding a comparable change for Landlock will be done in a subsequent PR.
  • chore: introduce SandboxPolicy::WorkspaceWrite::include_default_writable_roots (#1785)
    Without this change, it is challenging to create integration tests to
    verify that the folders not included in `writable_roots` in
    `SandboxPolicy::WorkspaceWrite` are read-only because, by default,
    `get_writable_roots_with_cwd()` includes `TMPDIR`, which is where most
    integrationt
    tests do their work.
    
    This introduces a `use_exact_writable_roots` option to disable the
    default
    includes returned by `get_writable_roots_with_cwd()`.
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1785).
    * #1765
    * __->__ #1785
  • feat: stream exec stdout events (#1786)
    ## Summary
    - stream command stdout as `ExecCommandStdout` events
    - forward streamed stdout to clients and ignore in human output
    processor
    - adjust call sites for new streaming API
  • Add /compact (#1527)
    - Add operation to summarize the context so far.
    - The operation runs a compact task that summarizes the context.
    - The operation clear the previous context to free the context window
    - The operation didn't use `run_task` to avoid corrupting the session
    - Add /compact in the tui
    
    
    
    https://github.com/user-attachments/assets/e06c24e5-dcfb-4806-934a-564d425a919c
  • Send account id when available (#1767)
    For users with multiple accounts we need to specify the account to use.
  • chore: refactor exec.rs: create separate seatbelt.rs and spawn.rs files (#1762)
    At 550 lines, `exec.rs` was a bit large. In particular, I found it hard
    to locate the Seatbelt-related code quickly without a file with
    `seatbelt` in the name, so this refactors things so:
    
    - `spawn_command_under_seatbelt()` and dependent code moves to a new
    `seatbelt.rs` file
    - `spawn_child_async()` and dependent code moves to a new `spawn.rs`
    file
  • fix: ensure PatchApplyBeginEvent and PatchApplyEndEvent are dispatched reliably (#1760)
    This is a follow-up to https://github.com/openai/codex/pull/1705, as
    that PR inadvertently lost the logic where `PatchApplyBeginEvent` and
    `PatchApplyEndEvent` events were sent when patches were auto-approved.
    
    Though as part of this fix, I believe this also makes an important
    safety fix to `assess_patch_safety()`, as there was a case that returned
    `SandboxType::None`, which arguably is the thing we were trying to avoid
    in #1705.
    
    On a high level, we want there to be only one codepath where
    `apply_patch` happens, which should be unified with the patch to run
    `exec`, in general, so that sandboxing is applied consistently for both
    cases.
    
    Prior to this change, `apply_patch()` in `core` would either:
    
    * exit early, delegating to `exec()` to shell out to `apply_patch` using
    the appropriate sandbox
    * proceed to run the logic for `apply_patch` in memory
    
    
    https://github.com/openai/codex/blob/549846b29ad52f6cb4f8560365a731966054a9b3/codex-rs/core/src/apply_patch.rs#L61-L63
    
    In this implementation, only the latter would dispatch
    `PatchApplyBeginEvent` and `PatchApplyEndEvent`, though the former would
    dispatch `ExecCommandBeginEvent` and `ExecCommandEndEvent` for the
    `apply_patch` call (or, more specifically, the `codex
    --codex-run-as-apply-patch PATCH` call).
    
    To unify things in this PR, we:
    
    * Eliminate the back half of the `apply_patch()` function, and instead
    have it also return with `DelegateToExec`, though we add an extra field
    to the return value, `user_explicitly_approved_this_action`.
    * In `codex.rs` where we process `DelegateToExec`, we use
    `SandboxType::None` when `user_explicitly_approved_this_action` is
    `true`. This means **we no longer run the apply_patch logic in memory**,
    as we always `exec()`. (Note this is what allowed us to delete so much
    code in `apply_patch.rs`.)
    * In `codex.rs`, we further update `notify_exec_command_begin()` and
    `notify_exec_command_end()` to take additional fields to determine what
    type of notification to send: `ExecCommand` or `PatchApply`.
    
    Admittedly, this PR also drops some of the functionality about giving
    the user the opportunity to expand the set of writable roots as part of
    approving the `apply_patch` command. I'm not sure how much that was
    used, and we should probably rethink how that works as we are currently
    tidying up the protocol to the TUI, in general.
  • Add codex login --api-key (#1759)
    Allow setting the API key via `codex login --api-key`
  • fix git tests (#1747)
    the git tests were failing on my local machine due to gpg signing config
    in my ~/.gitconfig. tests should not be affected by ~/.gitconfig, so
    configure them to ignore it.
  • fix: run apply_patch calls through the sandbox (#1705)
    Building on the work of https://github.com/openai/codex/pull/1702, this
    changes how a shell call to `apply_patch` is handled.
    
    Previously, a shell call to `apply_patch` was always handled in-process,
    never leveraging a sandbox. To determine whether the `apply_patch`
    operation could be auto-approved, the
    `is_write_patch_constrained_to_writable_paths()` function would check if
    all the paths listed in the paths were writable. If so, the agent would
    apply the changes listed in the patch.
    
    Unfortunately, this approach afforded a loophole: symlinks!
    
    * For a soft link, we could fix this issue by tracing the link and
    checking whether the target is in the set of writable paths, however...
    * ...For a hard link, things are not as simple. We can run `stat FILE`
    to see if the number of links is greater than 1, but then we would have
    to do something potentially expensive like `find . -inum <inode_number>`
    to find the other paths for `FILE`. Further, even if this worked, this
    approach runs the risk of a
    [TOCTOU](https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use)
    race condition, so it is not robust.
    
    The solution, implemented in this PR, is to take the virtual execution
    of the `apply_patch` CLI into an _actual_ execution using `codex
    --codex-run-as-apply-patch PATCH`, which we can run under the sandbox
    the user specified, just like any other `shell` call.
    
    This, of course, assumes that the sandbox prevents writing through
    symlinks as a mechanism to write to folders that are not in the writable
    set configured by the sandbox. I verified this by testing the following
    on both Mac and Linux:
    
    ```shell
    #!/usr/bin/env bash
    set -euo pipefail
    
    # Can running a command in SANDBOX_DIR write a file in EXPLOIT_DIR?
    
    # Codex is run in SANDBOX_DIR, so writes should be constrianed to this directory.
    SANDBOX_DIR=$(mktemp -d -p "$HOME" sandboxtesttemp.XXXXXX)
    # EXPLOIT_DIR is outside of SANDBOX_DIR, so let's see if we can write to it.
    EXPLOIT_DIR=$(mktemp -d -p "$HOME" sandboxtesttemp.XXXXXX)
    
    echo "SANDBOX_DIR: $SANDBOX_DIR"
    echo "EXPLOIT_DIR: $EXPLOIT_DIR"
    
    cleanup() {
      # Only remove if it looks sane and still exists
      [[ -n "${SANDBOX_DIR:-}" && -d "$SANDBOX_DIR" ]] && rm -rf -- "$SANDBOX_DIR"
      [[ -n "${EXPLOIT_DIR:-}" && -d "$EXPLOIT_DIR" ]] && rm -rf -- "$EXPLOIT_DIR"
    }
    
    trap cleanup EXIT
    
    echo "I am the original content" > "${EXPLOIT_DIR}/original.txt"
    
    # Drop the -s to test hard links.
    ln -s "${EXPLOIT_DIR}/original.txt" "${SANDBOX_DIR}/link-to-original.txt"
    
    cat "${SANDBOX_DIR}/link-to-original.txt"
    
    if [[ "$(uname)" == "Linux" ]]; then
        SANDBOX_SUBCOMMAND=landlock
    else
        SANDBOX_SUBCOMMAND=seatbelt
    fi
    
    # Attempt the exploit
    cd "${SANDBOX_DIR}"
    
    codex debug "${SANDBOX_SUBCOMMAND}" bash -lc "echo pwned > ./link-to-original.txt" || true
    
    cat "${EXPLOIT_DIR}/original.txt"
    ```
    
    Admittedly, this change merits a proper integration test, but I think I
    will have to do that in a follow-up PR.
  • Add support for a separate chatgpt auth endpoint (#1712)
    Adds a `CodexAuth` type that encapsulates information about available
    auth modes and logic for refreshing the token.
    Changes `Responses` API to send requests to different endpoints based on
    the auth type.
    Updates login_with_chatgpt to support API-less mode and skip the key
    exchange.
  • remove conversation history widget (#1727)
    this widget is no longer used.
  • Mcp protocol (#1715)
    - Add typed MCP protocol surface in
    `codex-rs/mcp-server/src/mcp_protocol.rs` for `requests`, `responses`,
    and `notifications`
    - Requests: `NewConversation`, `Connect`, `SendUserMessage`,
    `GetConversations`
    - Message content parts: `Text`, `Image` (`ImageUrl`/`FileId`, optional
    `ImageDetail`), File (`Url`/`Id`/`inline Data`)
    - Responses: `ToolCallResponseEnvelope` with optional `isError` and
    `structuredContent` variants (`NewConversation`, `Connect`,
    `SendUserMessageAccepted`, `GetConversations`)
    - Notifications: `InitialState`, `ConnectionRevoked`, `CodexEvent`,
    `Cancelled`
    - Uniform `_meta` on `notifications` via `NotificationMeta`
    (`conversationId`, `requestId`)
    - Unit tests validate JSON wire shapes for key
    `requests`/`responses`/`notifications`
  • Trim bash lc and run with login shell (#1725)
    include .zshenv, .zprofile by running with the `-l` flag and don't start
    a shell inside a shell when we see the typical `bash -lc` invocation.
  • Add an experimental plan tool (#1726)
    This adds a tool the model can call to update a plan. The tool doesn't
    actually _do_ anything but it gives clients a chance to read and render
    the structured plan. We will likely iterate on the prompt and tools
    exposed for planning over time.
  • Relative instruction file (#1722)
    Passing in an instruction file with a bad path led to silent failures,
    also instruction relative paths were handled in an unintuitive fashion.
  • Serializing the eventmsg type to snake_case (#1709)
    This was an abrupt change on our clients. We need to serialize as
    snake_case.
  • chore: split apply_patch logic out of codex.rs and into apply_patch.rs (#1703)
    This is a straight refactor, moving apply-patch-related code from
    `codex.rs` and into the new `apply_patch.rs` file. The only "logical"
    change is inlining `#[allow(clippy::unwrap_used)]` instead of declaring
    `#![allow(clippy::unwrap_used)]` at the top of the file (which is
    currently the case in `codex.rs`).
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1703).
    * #1705
    * __->__ #1703
    * #1702
    * #1698
    * #1697
  • 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
  • chore: update Codex::spawn() to return a struct instead of a tuple (#1677)
    Also update `init_codex()` to return a `struct` instead of a tuple, as well.
  • Changing method in MCP notifications (#1684)
    - Changing the codex/event type
  • chore: use one write call per item in rollout_writer() (#1679)
    Most of the time, we expect the `String` returned by
    `serde_json::to_string()` to have extra capacity, so `push('\n')` is
    unlikely to allocate, which seems cheaper than an extra `write(2)` call,
    on average?
  • Easily Selectable History (#1672)
    This update replaces the previous ratatui history widget with an
    append-only log so that the terminal can handle text selection and
    scrolling. It also disables streaming responses, which we'll do our best
    to bring back in a later PR. It also adds a small summary of token use
    after the TUI exits.
  • Update render name in tui for approval_policy to match with config values (#1675)
    Currently, codex on start shows the value for the approval policy as
    name of
    [AskForApproval](https://github.com/openai/codex/blob/2437a8d17a0cf972d1a6e7f303d469b6e2f57eae/codex-rs/core/src/protocol.rs#L128)
    enum, which differs from
    [approval_policy](https://github.com/openai/codex/blob/2437a8d17a0cf972d1a6e7f303d469b6e2f57eae/codex-rs/config.md#approval_policy)
    config values.
    E.g. "untrusted" becomes "UnlessTrusted", "on-failure" -> "OnFailure",
    "never" -> "Never".
    This PR changes render names of the approval policy to match with
    configuration values.