Commit Graph

506 Commits

  • fix: clean up styles & colors and define in styles.md (#2401)
    New style guide:
    
      # Headers, primary, and secondary text
      
    - **Headers:** Use `bold`. For markdown with various header levels,
    leave in the `#` signs.
      - **Primary text:** Default.
      - **Secondary text:** Use `dim`.
      
      # Foreground colors
      
    - **Default:** Most of the time, just use the default foreground color.
    `reset` can help get it back.
    - **Selection:** Use ANSI `blue`. (Ed & AE want to make this cyan too,
    but we'll do that in a followup since it's riskier in different themes.)
      - **User input tips and status indicators:** Use ANSI `cyan`.
      - **Success and additions:** Use ANSI `green`.
      - **Errors, failures and deletions:** Use ANSI `red`.
      - **Codex:** Use ANSI `magenta`.
      
      # Avoid
      
    - Avoid custom colors because there's no guarantee that they'll contrast
    well or look good on various terminal color themes.
    - Avoid ANSI `black`, `white`, `yellow` as foreground colors because the
    terminal theme will do a better job. (Use `reset` if you need to in
    order to get those.) The exception is if you need contrast rendering
    over a manually colored background.
      
      (There are some rules to try to catch this in `clippy.toml`.)
    
    # Testing
    
    Tested in a variety of light and dark color themes in Terminal, iTerm2, and Ghostty.
  • fix: introduce EventMsg::TurnAborted (#2365)
    Introduces `EventMsg::TurnAborted` that should be sent in response to
    `Op::Interrupt`.
    
    In the MCP server, updates the handling of a
    `ClientRequest::InterruptConversation` request such that it sends the
    `Op::Interrupt` but does not respond to the request until it sees an
    `EventMsg::TurnAborted`.
  • Added MCP server command to enable authentication using ChatGPT (#2373)
    This PR adds two new APIs for the MCP server: 1) loginChatGpt, and 2)
    cancelLoginChatGpt. The first starts a login server and returns a local
    URL that allows for browser-based authentication, and the second
    provides a way to cancel the login attempt. If the login attempt
    succeeds, a notification (in the form of an event) is sent to a
    subscriber.
    
    I also added a timeout mechanism for the existing login server. The
    loginChatGpt code path uses a 10-minute timeout by default, so if the
    user fails to complete the login flow in that timeframe, the login
    server automatically shuts down. I tested the timeout code by manually
    setting the timeout to a much lower number and confirming that it works
    as expected when used e2e.
  • Show progress indicator for /diff command (#2245)
    ## Summary
    - Show a temporary Working on diff state in the bottom pan 
    - Add `DiffResult` app event and dispatch git diff asynchronously
    
    ## Testing
    - `just fmt`
    - `just fix` *(fails: `let` expressions in this position are unstable)*
    - `cargo test --all-features` *(fails: `let` expressions in this
    position are unstable)*
    
    ------
    https://chatgpt.com/codex/tasks/task_i_689a839f32b88321840a893551d5fbef
  • Fix #2296 Add "minimal" reasoning effort for GPT 5 models (#2326)
    This pull request resolves #2296; I've confirmed if it works by:
    
    1. Add settings to ~/.codex/config.toml:
    ```toml
    model_reasoning_effort = "minimal"
    ```
    
    2. Run the CLI:
    ```
    cd codex-rs
    cargo build && RUST_LOG=trace cargo run --bin codex
    /status
    tail -f ~/.codex/log/codex-tui.log
    ```
    
    Co-authored-by: pakrym-oai <pakrym@openai.com>
  • tui: skip identical consecutive entries in local composer history (#2352)
    This PR avoids inserting duplicate consecutive messages into the Chat
    Composer's local history.
  • feat: introduce ClientRequest::SendUserTurn (#2345)
    This adds a new request type, `SendUserTurn`, that makes it possible to
    submit a `Op::UserTurn` operation (introduced in #2329) to a
    conversation. This PR also adds a new integration test that verifies
    that changing from `AskForApproval::UnlessTrusted` to
    `AskForApproval::Never` mid-conversation ensures that an elicitation is
    no longer sent for running `python3 -c print(42)`.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2345).
    * __->__ #2345
    * #2329
    * #2343
    * #2340
    * #2338
  • feat: introduce Op:UserTurn (#2329)
    This introduces `Op::UserTurn`, which makes it possible to override many
    of the fields that were set when the `Session` was originally created
    when creating a new conversation turn. This is one way we could support
    changing things like `model` or `cwd` in the middle of the conversation,
    though we may want to consider making each field optional, or
    alternatively having a separate `Op` that mutates the `TurnContext`
    associated with a `submission_loop()`.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2329).
    * #2345
    * __->__ #2329
    * #2343
    * #2340
    * #2338
  • feat: introduce TurnContext (#2343)
    This PR introduces `TurnContext`, which is designed to hold a set of
    fields that should be constant for a turn of a conversation. Note that
    the fields of `TurnContext` were previously governed by `Session`.
    
    Ultimately, we want to enable users to change these values between turns
    (changing model, approval policy, etc.), though in the current
    implementation, the `TurnContext` is constant for the entire
    conversation.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2345).
    * #2345
    * #2329
    * __->__ #2343
    * #2340
    * #2338
  • fix: try to fix flakiness in test_shell_command_approval_triggers_elicitation (#2344)
    I still see flakiness in
    `test_shell_command_approval_triggers_elicitation()` on occasion where
    `MockServer` claims it has not received all of its expected requests.
    
    I recently introduced a similar type of test in #2264,
    `test_codex_jsonrpc_conversation_flow()`, which I have not seen flake
    (yet!), so this PR pulls over two things I did in that test:
    
    - increased `worker_threads` from `2` to `4`
    - added an assertion to make sure the `task_complete` notification is
    received
    
    Honestly, I'm still not sure why `MockServer` claims it sometimes does
    not receive all its expected requests given that we assert that the
    final `JSONRPCResponse` is read on the stream, but let's give this a
    shot.
    
    Assuming this fixes things, my hypothesis is that the increase in
    `worker_threads` helps because perhaps there are async tasks in
    `MockServer` that do not reliably complete fully when there are not
    enough threads available? If that is correct, it seems like the test
    would still be flaky, though perhaps with lower frequency?
  • fix: introduce MutexExt::lock_unchecked() so we stop ignoring unwrap() throughout codex.rs (#2340)
    This way we are sure a dangerous `unwrap()` does not sneak in!
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2340).
    * #2345
    * #2329
    * #2343
    * __->__ #2340
    * #2338
  • fix: tighten up checks against writable folders for SandboxPolicy (#2338)
    I was looking at the implementation of `Session::get_writable_roots()`,
    which did not seem right, as it was a copy of writable roots, which is
    not guaranteed to be in sync with the `sandbox_policy` field.
    
    I looked at who was calling `get_writable_roots()` and its only call
    site was `apply_patch()` in `codex-rs/core/src/apply_patch.rs`, which
    took the roots and forwarded them to `assess_patch_safety()` in
    `safety.rs`. I updated `assess_patch_safety()` to take `sandbox_policy:
    &SandboxPolicy` instead of `writable_roots: &[PathBuf]` (and replaced
    `Session::get_writable_roots()` with `Session::get_sandbox_policy()`).
    
    Within `safety.rs`, it was fairly easy to update
    `is_write_patch_constrained_to_writable_paths()` to work with
    `SandboxPolicy`, and in particular, it is far more accurate because, for
    better or worse, `SandboxPolicy::get_writable_roots_with_cwd()` _returns
    an empty vec_ for `SandboxPolicy::DangerFullAccess`, suggesting that
    _nothing_ is writable when in reality _everything_ is writable. With
    this PR, `is_write_patch_constrained_to_writable_paths()` now does the
    right thing for each variant of `SandboxPolicy`.
    
    I thought this would be the end of the story, but it turned out that
    `test_writable_roots_constraint()` in `safety.rs` needed to be updated,
    as well. In particular, the test was writing to
    `std::env::current_dir()` instead of a `TempDir`, which I suspect was a
    holdover from earlier when `SandboxPolicy::WorkspaceWrite` would always
    make `TMPDIR` writable on macOS, which made it hard to write tests to
    verify `SandboxPolicy` in `TMPDIR`. Fortunately, we now have
    `exclude_tmpdir_env_var` as an option on
    `SandboxPolicy::WorkspaceWrite`, so I was able to update the test to
    preserve the existing behavior, but to no longer write to
    `std::env::current_dir()`.
    
    
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2338).
    * #2345
    * #2329
    * #2343
    * #2340
    * __->__ #2338
  • [tools] Add apply_patch tool (#2303)
    ## Summary
    We've been seeing a number of issues and reports with our synthetic
    `apply_patch` tool, e.g. #802. Let's make this a real tool - in my
    anecdotal testing, it's critical for GPT-OSS models, but I'd like to
    make it the standard across GPT-5 and codex models as well.
    
    ## Testing
    - [x] Tested locally
    - [x] Integration test
  • tui: include optional full command line in history display (#2334)
    Add env var to show the raw, unparsed command line under parsed
    commands. When we have transcript mode we should show the full command
    there, but this is useful for debugging.
  • Cleanup rust login server a bit more (#2331)
    Remove some extra abstractions.
    
    ---------
    
    Co-authored-by: easong-openai <easong@openai.com>
  • Added allow-expect-in-tests / allow-unwrap-in-tests (#2328)
    This PR:
    * Added the clippy.toml to configure allowable expect / unwrap usage in
    tests
    * Removed as many expect/allow lines as possible from tests
    * moved a bunch of allows to expects where possible
    
    Note: in integration tests, non `#[test]` helper functions are not
    covered by this so we had to leave a few lingering `expect(expect_used`
    checks around
  • Fix AF_UNIX, sockpair, recvfrom in linux sandbox (#2309)
    When using codex-tui on a linux system I was unable to run `cargo
    clippy` inside of codex due to:
    ```
    [pid 3548377] socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0,  <unfinished ...>
    [pid 3548370] close(8 <unfinished ...>
    [pid 3548377] <... socketpair resumed>0x7ffb97f4ed60) = -1 EPERM (Operation not permitted)
    ```
    And
    ```
    3611300 <... recvfrom resumed>0x708b8b5cffe0, 8, 0, NULL, NULL) = -1 EPERM (Operation not permitted)
    ```
    
    This PR:
    * Fixes a bug that disallowed AF_UNIX to allow it on `socket()`
    * Adds recvfrom() to the syscall allow list, this should be fine since
    we disable opening new sockets. But we should validate there is not a
    open socket inheritance issue.
    * Allow socketpair to be called for AF_UNIX
    * Adds tests for AF_UNIX components
    * All of which allows running `cargo clippy` within the sandbox on
    linux, and possibly other tooling using a fork server model + AF_UNIX
    comms.
  • Port login server to rust (#2294)
    Port the login server to rust.
    
    ---------
    
    Co-authored-by: pakrym-oai <pakrym@openai.com>
  • clear running commands in various places (#2325)
    we have a very unclear lifecycle for the chatwidget—this should only
    have to be added in one place! but this fixes the "hanging commands"
    issue where the active_exec_cell wasn't correctly cleared when commands
    finished.
    
    To repro w/o this PR:
    1. prompt "run sleep 10"
    2. once the command starts running, press <kbd>Esc</kbd>
    3. prompt "run echo hi"
    
    Expected: 
    
    ```
    ✓ Completed
      └ ⌨️ echo hi
    
    codex
    hi
    ```
    
    Actual:
    
    ```
    ⚙︎ Working
      └ ⌨️ echo hi
    
    ▌ Ask Codex to do anything
    ```
    
    i.e. the "Working" never changes to "Completed".
    
    The bug is fixed with this PR.
  • add a timer to running exec commands (#2321)
    sometimes i switch back to codex and i don't know how long a command has
    been running.
    
    <img width="744" height="462" alt="Screenshot 2025-08-14 at 3 30 07 PM"
    src="https://github.com/user-attachments/assets/bd80947f-5a47-43e6-ad19-69c2995a2a29"
    />
  • fix: run python_multiprocessing_lock_works integration test on Mac and Linux (#2318)
    The high-order bit on this PR is that it makes it so `sandbox.rs` tests
    both Mac and Linux, as we introduce a general
    `spawn_command_under_sandbox()` function with platform-specific
    implementations for testing.
    
    An important, and interesting, discovery in porting the test to Linux is
    that (for reasons cited in the code comments), `/dev/shm` has to be
    added to `writable_roots` on Linux in order for `multiprocessing.Lock`
    to work there. Granting write access to `/dev/shm` comes with some
    degree of risk, so we do not make this the default for Codex CLI.
    
    Piggybacking on top of #2317, this moves the
    `python_multiprocessing_lock_works` test yet again, moving
    `codex-rs/core/tests/sandbox.rs` to `codex-rs/exec/tests/sandbox.rs`
    because in `codex-rs/exec/tests` we can use `cargo_bin()` like so:
    
    ```
    let codex_linux_sandbox_exe = assert_cmd::cargo::cargo_bin("codex-exec");
    ```
    
    which is necessary so we can use `codex_linux_sandbox_exe` and therefore
    `spawn_command_under_linux_sandbox` in an integration test.
    
    This also moves `spawn_command_under_linux_sandbox()` out of `exec.rs`
    and into `landlock.rs`, which makes things more consistent with
    `seatbelt.rs` in `codex-core`.
    
    For reference, https://github.com/openai/codex/pull/1808 is the PR that
    made the change to Seatbelt to get this test to pass on Mac.
  • fix: move general sandbox tests to codex-rs/core/tests/sandbox.rs (#2317)
    Previous to this PR, `codex-rs/core/tests/sandbox.rs` contained
    integration tests that were specific to Seatbelt. This PR moves those
    tests to `codex-rs/core/src/seatbelt.rs` and designates
    `codex-rs/core/tests/sandbox.rs` to be used as the home for
    cross-platform (well, Mac and Linux...) sandbox tests.
    
    To start, this migrates
    `python_multiprocessing_lock_works_under_seatbelt()` from #1823 to the
    new `sandbox.rs` because this is the type of thing that should work on
    both Mac _and_ Linux, though I still need to do some work to clean up
    the test so it works on both platforms.
  • test(core): add seatbelt sem lock tests (#1823)
    ## Summary
    - add a unit test to ensure the macOS seatbelt policy allows POSIX
    semaphores
    - add a macOS-only test that runs a Python multiprocessing Lock under
    Seatbelt
    
    ## Testing
    - `cargo test -p codex_core seatbelt_base_policy_allows_ipc_posix_sem
    --no-fail-fast` (failed: failed to download from
    `https://static.crates.io/crates/tokio-stream/0.1.17/download`)
    - `cargo test -p codex_core seatbelt_base_policy_allows_ipc_posix_sem
    --no-fail-fast --offline` (failed: attempting to make an HTTP request,
    but --offline was specified)
    - `cargo test --all-features --no-fail-fast --offline` (failed:
    attempting to make an HTTP request, but --offline was specified)
    - `just fmt` (failed: command not found: just)
    - `just fix` (failed: command not found: just)
    
    Ran tests locally to confirm it passes on master and failed before my
    previous change
    
    ------
    https://chatgpt.com/codex/tasks/task_i_6890f221e0a4833381cfb53e11499bcc
  • fix bash commands being incorrectly quoted in display (#2313)
    The "display format" of commands was sometimes producing incorrect
    quoting like `echo foo '>' bar`, which is importantly different from the
    actual command that was being run. This refactors ParsedCommand to have
    a string in `cmd` instead of a vec, as a `vec` can't accurately capture
    a full command.
  • use a central animation loop (#2268)
    instead of each shimmer needing to have its own animation thread, have
    render_ref schedule a new frame if it wants one and coalesce to the
    earliest next frame. this also makes the animations
    frame-timing-independent, based on start time instead of frame count.
  • text elements in textarea for pasted content (#2302)
    This improves handling of pasted content in the textarea. It's no longer
    possible to partially delete a placeholder (e.g. by ^W or ^D), nor is it
    possible to place the cursor inside a placeholder. Also, we now render
    placeholders in a different color to make them more clearly
    differentiated.
    
    
    https://github.com/user-attachments/assets/2051b3c3-963d-4781-a610-3afee522ae29
  • fix: do not allow dotenv to create/modify environment variables starting with CODEX_ (#2308)
    This ensures Codex cannot drop a `.env` file with a value of
    `CODEX_HOME` that points to a folder that Codex can control.
  • fix: parallelize logic in Session::new() (#2305)
    #2291 made it so that `Session::new()` is on the critical path to
    `Codex::spawn()`, which means it is on the hot path to CLI startup. This
    refactors `Session::new()` to run a number of async tasks in parallel
    that were previously run serially to try to reduce latency.
  • remove logs from composer by default (#2307)
    Currently the composer shows `handle_codex_event:<event name>` by
    default which feels confusing. Let's make it appear in trace.
  • remove the · animation (#2271)
    the pulsing dot felt too noisy to me next to the shimmering "Working"
    text. we'll bring it back for streaming response text perhaps?
  • [context] Store context messages in rollouts (#2243)
    ## Summary
    Currently, we use request-time logic to determine the user_instructions
    and environment_context messages. This means that neither of these
    values can change over time as conversations go on. We want to add in
    additional details here, so we're migrating these to save these messages
    to the rollout file instead. This is simpler for the client, and allows
    us to append additional environment_context messages to each turn if we
    want
    
    ## Testing
    - [x] Integration test coverage
    - [x] Tested locally with a few turns, confirmed model could reference
    environment context and cached token metrics were reasonably high
  • remove "status text" in bottom line (#2279)
    this used to hold the most recent log line, but it was kinda broken and
    not that useful.
  • HistoryCell is a trait (#2283)
    refactors HistoryCell to be a trait instead of an enum. Also collapse
    the many "degenerate" HistoryCell enums which were just a store of lines
    into a single PlainHistoryCell type.
    
    The goal here is to allow more ways of rendering history cells (e.g.
    expanded/collapsed/"live"), and I expect we will return to more varied
    types of HistoryCell as we develop this area.
  • Tag InputItem (#2304)
    Instead of:
    ```
    { Text: { text: string } }
    ```
    
    It is now:
    ```
    { type: "text", data: { text: string } }
    ```
    which makes for cleaner discriminated unions
  • exploration: create Session as part of Codex::spawn() (#2291)
    Historically, `Codex::spawn()` would create the instance of `Codex` and
    enforce, by construction, that `Op::ConfigureSession` was the first `Op`
    submitted via `submit()`. Then over in `submission_loop()`, it would
    handle the case for taking the parameters of `Op::ConfigureSession` and
    turning it into a `Session`.
    
    This approach has two challenges from a state management perspective:
    
    
    https://github.com/openai/codex/blob/f968a1327ad39a7786759ea8f1d1c088fe41e91b/codex-rs/core/src/codex.rs#L718
    
    - The local `sess` variable in `submission_loop()` has to be `mut` and
    `Option<Arc<Session>>` because it is not invariant that a `Session` is
    present for the lifetime of the loop, so there is a lot of logic to deal
    with the case where `sess` is `None` (e.g., the `send_no_session_event`
    function and all of its callsites).
    - `submission_loop()` is written in such a way that
    `Op::ConfigureSession` could be observed multiple times, but in
    practice, it is only observed exactly once at the start of the loop.
    
    In this PR, we try to simplify the state management by _removing_ the
    `Op::ConfigureSession` enum variant and constructing the `Session` as
    part of `Codex::spawn()` so that it can be passed to `submission_loop()`
    as `Arc<Session>`. The original logic from the `Op::ConfigureSession`
    has largely been moved to the new `Session::new()` constructor.
    
    ---
    
    Incidentally, I also noticed that the handling of `Op::ConfigureSession`
    can result in events being dispatched in addition to
    `EventMsg::SessionConfigured`, as an `EventMsg::Error` is created for
    every MCP initialization error, so it is important to preserve that
    behavior:
    
    
    https://github.com/openai/codex/blob/f968a1327ad39a7786759ea8f1d1c088fe41e91b/codex-rs/core/src/codex.rs#L901-L916
    
    Though admittedly, I believe this does not play nice with #2264, as
    these error messages will likely be dispatched before the client has a
    chance to call `addConversationListener`, so we likely need to make it
    so `newConversation` automatically creates the subscription, but we must
    also guarantee that the "ack" from `newConversation` is returned before
    any other conversation-related notifications are sent so the client
    knows what `conversation_id` to match on.
  • feat: add support for an InterruptConversation request (#2287)
    This adds `ClientRequest::InterruptConversation`, which effectively maps
    directly to `Op::Interrupt`.
    
    ---
    
    * __->__  #2287
    * #2286
    * #2285
  • fix: add support for exec and apply_patch approvals in the new wire format (#2286)
    Now when `CodexMessageProcessor` receives either a
    `EventMsg::ApplyPatchApprovalRequest` or a
    `EventMsg::ExecApprovalRequest`, it sends the appropriate request from
    the server to the client. When it gets a response, it forwards it on to
    the `CodexConversation`.
    
    Note this takes a lot of code from:
    
    
    https://github.com/openai/codex/blob/main/codex-rs/mcp-server/src/conversation_loop.rs
    
    https://github.com/openai/codex/blob/main/codex-rs/mcp-server/src/exec_approval.rs
    
    https://github.com/openai/codex/blob/main/codex-rs/mcp-server/src/patch_approval.rs
    
    I am copy/pasting for now because I am trying to consolidate around the
    new `wire_format.rs`, so I plan to delete these other files soon.
    
    Now that we have requests going both from client-to-server and
    server-to-client, I renamed `CodexRequest` to `ClientRequest`.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2286).
    * #2287
    * __->__ #2286
    * #2285
  • fix: make all fields of Session private (#2285)
    As `Session` needs a bit of work, it will make things easier to move
    around if we can start by reducing the extent of its public API. This
    makes all the fields private, though adds three `pub(crate)` getters.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2285).
    * #2287
    * #2286
    * __->__ #2285
  • Parse reasoning text content (#2277)
    Sometimes COT is returns as text content instead of `ReasoningText`. We
    should parse it but not serialize back on requests.
    
    ---------
    
    Co-authored-by: Ahmed Ibrahim <aibrahim@openai.com>
  • fix: verify notifications are sent with the conversationId set (#2278)
    This updates `CodexMessageProcessor` so that each notification it sends
    for a `EventMsg` from a `CodexConversation` such that:
    
    - The `params` always has an appropriate `conversationId` field.
    - The `method` is now includes the name of the `EventMsg` type rather
    than using `codex/event` as the `method` type for all notifications. (We
    currently prefix the method name with `codex/event/`, but I think that
    should go away once we formalize the notification schema in
    `wire_format.rs`.)
    
    As part of this, we update `test_codex_jsonrpc_conversation_flow()` to
    verify that the `task_finished` notification has made it through the
    system instead of sleeping for 5s and "hoping" the server finished
    processing the task. Note we have seen some flakiness in some of our
    other, similar integration tests, and I expect adding a similar check
    would help in those cases, as well.