Commit Graph

75 Commits

  • 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.
  • feat: support traditional JSON-RPC request/response in MCP server (#2264)
    This introduces a new set of request types that our `codex mcp`
    supports. Note that these do not conform to MCP tool calls so that
    instead of having to send something like this:
    
    ```json
    {
      "jsonrpc": "2.0",
      "method": "tools/call",
      "id": 42,
      "params": {
        "name": "newConversation",
        "arguments": {
          "model": "gpt-5",
          "approvalPolicy": "on-request"
        }
      }
    }
    ```
    
    we can send something like this:
    
    
    ```json
    {
      "jsonrpc": "2.0",
      "method": "newConversation",
      "id": 42,
      "params": {
        "model": "gpt-5",
        "approvalPolicy": "on-request"
      }
    }
    ```
    
    Admittedly, this new format is not a valid MCP tool call, but we are OK
    with that right now. (That is, not everything we might want to request
    of `codex mcp` is something that is appropriate for an autonomous agent
    to do.)
    
    To start, this introduces four request types:
    
    - `newConversation`
    - `sendUserMessage`
    - `addConversationListener`
    - `removeConversationListener`
    
    The new `mcp-server/tests/codex_message_processor_flow.rs` shows how
    these can be used.
    
    The types are defined on the `CodexRequest` enum, so we introduce a new
    `CodexMessageProcessor` that is responsible for dealing with requests
    from this enum. The top-level `MessageProcessor` has been updated so
    that when `process_request()` is called, it first checks whether the
    request conforms to `CodexRequest` and dispatches it to
    `CodexMessageProcessor` if so.
    
    Note that I also decided to use `camelCase` for the on-the-wire format,
    as that seems to be the convention for MCP.
    
    For the moment, the new protocol is defined in `wire_format.rs` within
    the `mcp-server` crate, but in a subsequent PR, I will probably move it
    to its own crate to ensure the protocol has minimal dependencies and
    that we can codegen a schema from it.
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2264).
    * #2278
    * __->__ #2264
  • fix: update OutgoingMessageSender::send_response() to take Serialize (#2263)
    This makes `send_response()` easier to work with.
  • chore: introduce ConversationManager as a clearinghouse for all conversations (#2240)
    This PR does two things because after I got deep into the first one I
    started pulling on the thread to the second:
    
    - Makes `ConversationManager` the place where all in-memory
    conversations are created and stored. Previously, `MessageProcessor` in
    the `codex-mcp-server` crate was doing this via its `session_map`, but
    this is something that should be done in `codex-core`.
    - It unwinds the `ctrl_c: tokio::sync::Notify` that was threaded
    throughout our code. I think this made sense at one time, but now that
    we handle Ctrl-C within the TUI and have a proper `Op::Interrupt` event,
    I don't think this was quite right, so I removed it. For `codex exec`
    and `codex proto`, we now use `tokio::signal::ctrl_c()` directly, but we
    no longer make `Notify` a field of `Codex` or `CodexConversation`.
    
    Changes of note:
    
    - Adds the files `conversation_manager.rs` and `codex_conversation.rs`
    to `codex-core`.
    - `Codex` and `CodexSpawnOk` are no longer exported from `codex-core`:
    other crates must use `CodexConversation` instead (which is created via
    `ConversationManager`).
    - `core/src/codex_wrapper.rs` has been deleted in favor of
    `ConversationManager`.
    - `ConversationManager::new_conversation()` returns `NewConversation`,
    which is in line with the `new_conversation` tool we want to add to the
    MCP server. Note `NewConversation` includes `SessionConfiguredEvent`, so
    we eliminate checks in cases like `codex-rs/core/tests/client.rs` to
    verify `SessionConfiguredEvent` is the first event because that is now
    internal to `ConversationManager`.
    - Quite a bit of code was deleted from
    `codex-rs/mcp-server/src/message_processor.rs` since it no longer has to
    manage multiple conversations itself: it goes through
    `ConversationManager` instead.
    - `core/tests/live_agent.rs` has been deleted because I had to update a
    bunch of tests and all the tests in here were ignored, and I don't think
    anyone ever ran them, so this was just technical debt, at this point.
    - Removed `notify_on_sigint()` from `util.rs` (and in a follow-up, I
    hope to refactor the blandly-named `util.rs` into more descriptive
    files).
    - In general, I started replacing local variables named `codex` as
    `conversation`, where appropriate, though admittedly I didn't do it
    through all the integration tests because that would have added a lot of
    noise to this PR.
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2240).
    * #2264
    * #2263
    * __->__ #2240
  • Re-add markdown streaming (#2029)
    Wait for newlines, then render markdown on a line by line basis. Word wrap it for the current terminal size and then spit it out line by line into the UI. Also adds tests and fixes some UI regressions.
  • [mcp-server] Support CodexToolCallApprovalPolicy::OnRequest (#2187)
    ## Summary
    #1865 added `AskForApproval::OnRequest`, but missed adding it to our
    custom struct in `mcp-server`. This adds the missing configuration
    
    ## Testing
    - [x] confirmed locally
  • [1/3] Parse exec commands and format them more nicely in the UI (#2095)
    # Note for reviewers
    The bulk of this PR is in in the new file, `parse_command.rs`. This file
    is designed to be written TDD and implemented with Codex. Do not worry
    about reviewing the code, just review the unit tests (if you want). If
    any cases are missing, we'll add more tests and have Codex fix them.
    
    I think the best approach will be to land and iterate. I have some
    follow-ups I want to do after this lands. The next PR after this will
    let us merge (and dedupe) multiple sequential cells of the same such as
    multiple read commands. The deduping will also be important because the
    model often reads the same file multiple times in a row in chunks
    
    ===
    
    This PR formats common commands like reading, formatting, testing, etc
    more nicely:
    
    It tries to extract things like file names, tests and falls back to the
    cmd if it doesn't. It also only shows stdout/err if the command failed.
    
    <img width="770" height="238" alt="CleanShot 2025-08-09 at 16 05 15"
    src="https://github.com/user-attachments/assets/0ead179a-8910-486b-aa3d-7d26264d751e"
    />
    <img width="348" height="158" alt="CleanShot 2025-08-09 at 16 05 32"
    src="https://github.com/user-attachments/assets/4302681b-5e87-4ff3-85b4-0252c6c485a9"
    />
    <img width="834" height="324" alt="CleanShot 2025-08-09 at 16 05 56 2"
    src="https://github.com/user-attachments/assets/09fb3517-7bd6-40f6-a126-4172106b700f"
    />
    
    Part 2: https://github.com/openai/codex/pull/2097
    Part 3: https://github.com/openai/codex/pull/2110
  • [env] Remove git config for now (#1884)
    ## Summary
    Forgot to remove this in #1869 last night! Too much of a performance hit
    on the main thread. We can bring it back via an async thread on startup.
  • [prompts] Add <environment_context> (#1869)
    ## Summary
    Includes a new user message in the api payload which provides useful
    environment context for the model, so it knows about things like the
    current working directory and the sandbox.
    
    ## Testing
    Updated unit tests
  • [tests] Investigate flakey mcp-server test (#1877)
    ## Summary
    Have seen these tests flaking over the course of today on different
    boxes. `wiremock` seems to be generally written with tokio/threads in
    mind but based on the weird panics from the tests, let's see if this
    helps.
  • chore: remove unnecessary default_ prefix (#1854)
    This prefix is not inline with the other fields on the `ConfigOverrides`
    struct.
  • 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>
  • 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
  • Fix flaky test_shell_command_approval_triggers_elicitation test (#1802)
    This doesn't flake very often but this should fix it.
  • MCP: add conversation.create tool [Stack 2/2] (#1783)
    Introduce conversation.create handler (handle_create_conversation) and
    wire it in MessageProcessor.
    
    Stack:
    Top: #1783 
    Bottom: #1784
    
    ---------
    
    Co-authored-by: Gabriel Peal <gpeal@users.noreply.github.com>
  • Add Error variant to ConversationCreateResult [Stack 1/2] (#1784)
    Switch ConversationCreateResult from a struct to a tagged enum (Ok |
    Error)
    
    Stack:
    Top: #1783 
    Bottom: #1784
  • 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
  • Introduce a new function to just send user message [Stack 3/3] (#1686)
    - MCP server: add send-user-message tool to send user input to a running
    Codex session
    - Added an integration tests for the happy and sad paths
    
    Changes:
    •	Add tool definition and schema.
    •	Expose tool in capabilities.
    •	Route and handle tool requests with validation.
    •	Tests for success, bad UUID, and missing session.
    
    
    follow‑ups
    • Listen path not implemented yet; the tool is present but marked “don’t
    use yet” in code comments.
    • Session run flag reset: clear running_session_id_set appropriately
    after turn completion/errors.
    
    This is the third PR in a stack.
    Stack:
    Final: #1686
    Intermediate: #1751
    First: #1750
  • MCP server: route structured tool-call requests and expose mcp_protocol [Stack 2/3] (#1751)
    - Expose mcp_protocol from mcp-server for reuse in tests and callers.
    - In MessageProcessor, detect structured ToolCallRequestParams in
    tools/call and forward to a new handler.
    - Add handle_new_tool_calls scaffold (returns error for now).
    - Test helper: add send_send_user_message_tool_call to McpProcess to
    send ConversationSendMessage requests;
    
    This is the second PR in a stack.
    Stack:
    Final: #1686
    Intermediate: #1751
    First: #1750
  • MCP Protocol: Align tool-call response with CallToolResult [Stack 1/3] (#1750)
    # Summary
    - Align MCP server responses with mcp_types by emitting [CallToolResult,
    RequestId] instead of an object.
    Update send-message result to a tagged enum: Ok or Error { message }.
    
    # Why
    Protocol compliance with current MCP schema.
    
    # Tests
    - Updated assertions in mcp_protocol.rs for create/stream/send/list and
    error cases.
    
    This is the first PR in a stack.
    Stack:
    Final: #1686
    Intermediate: #1751
    First: #1750
  • 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
  • Auto format toml (#1745)
    Add recommended extension and configure it to auto format prompt.
  • moving input item from MCP Protocol back to core Protocol (#1740)
    - Currently we have duplicate input item. Let's have one source of truth
    in the core.
    - Used Requestid type
  • fix ci (#1739)
    I think this commit broke the CI because it changed the
    `McpToolCallBeginEvent` type:
    https://github.com/openai/codex/commit/347c81ad0049103c84e0aa2c0d7e2988db18218a
  • 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`
  • 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.
  • [mcp-server] Populate notifications._meta with requestId (#1704)
    ## Summary
    Per the [latest MCP
    spec](https://modelcontextprotocol.io/specification/2025-06-18/basic#meta),
    the `_meta` field is reserved for metadata. In the [Typescript
    Schema](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/0695a497eb50a804fc0e88c18a93a21a675d6b3e/schema/2025-06-18/schema.ts#L37-L40),
    `progressToken` is defined as a value to be attached to subsequent
    notifications for that request.
    
    The
    [CallToolRequestParams](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/0695a497eb50a804fc0e88c18a93a21a675d6b3e/schema/2025-06-18/schema.ts#L806-L817)
    extends this definition but overwrites the params field. This ambiguity
    makes our generated type definitions tricky, so I'm going to skip
    `progressToken` field for now and just send back the `requestId`
    instead.
     
    In a future PR, we can clarify, update our `generate_mcp_types.py`
    script, and update our progressToken logic accordingly.
    
    ## Testing
    - [x] Added unit tests
    - [x] Manually tested with mcp client
  • Serializing the eventmsg type to snake_case (#1709)
    This was an abrupt change on our clients. We need to serialize as
    snake_case.
  • 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
  • fix: create separate test_support crates to eliminate #[allow(dead_code)] (#1667)
    Because of a quirk of how implementation tests work in Rust, we had a
    number of `#[allow(dead_code)]` annotations that were misleading because
    the functions _were_ being used, just not by all integration tests in a
    `tests/` folder, so when compiling the test that did not use the
    function, clippy would complain that it was unused.
    
    This fixes things by create a "test_support" crate under the `tests/`
    folder that is imported as a dev dependency for the respective crate.
  • Flaky CI fix (#1647)
    Flushing before sending `TaskCompleteEvent` and ending the submission
    loop to avoid race conditions.
  • Add call_id to patch approvals and elicitations (#1660)
    Builds on https://github.com/openai/codex/pull/1659 and adds call_id to
    a few more places for the same reason.
  • Improve messages emitted for exec failures (#1659)
    1. Emit call_id to exec approval elicitations for mcp client convenience
    2. Remove the `-retry` from the call id for the same reason as above but
    upstream the reset behavior to the mcp client
  • Add support for custom base instructions (#1645)
    Allows providing custom instructions file as a config parameter and
    custom instruction text via MCP tool call.
  • Add an elicitation for approve patch and refactor tool calls (#1642)
    1. Added an elicitation for `approve-patch` which is very similar to
    `approve-exec`.
    2. Extracted both elicitations to their own files to prevent
    `codex_tool_runner` from blowing up in size.
  • [mcp-server] Add reply tool call (#1643)
    ## Summary
    Adds a new mcp tool call, `codex-reply`, so we can continue existing
    sessions. This is a first draft and does not yet support sessions from
    previous processes.
    
    ## Testing
    - [x] tested with mcp client
  • test: add integration test for MCP server (#1633)
    This PR introduces a single integration test for `cargo mcp`, though it
    also introduces a number of reusable components so that it should be
    easier to introduce more integration tests going forward.
    
    The new test is introduced in `codex-rs/mcp-server/tests/elicitation.rs`
    and the reusable pieces are in `codex-rs/mcp-server/tests/common`.
    
    The test itself verifies new functionality around elicitations
    introduced in https://github.com/openai/codex/pull/1623 (and the fix
    introduced in https://github.com/openai/codex/pull/1629) by doing the
    following:
    
    - starts a mock model provider with canned responses for
    `/v1/chat/completions`
    - starts the MCP server with a `config.toml` to use that model provider
    (and `approval_policy = "untrusted"`)
    - sends the `codex` tool call which causes the mock model provider to
    request a shell call for `git init`
    - the MCP server sends an elicitation to the client to approve the
    request
    - the client replies to the elicitation with `"approved"`
    - the MCP server runs the command and re-samples the model, getting a
    `"finish_reason": "stop"`
    - in turn, the MCP server sends the final response to the original
    `codex` tool call
    - verifies that `git init` ran as expected
    
    To test:
    
    ```
    cargo test shell_command_approval_triggers_elicitation
    ```
    
    In writing this test, I discovered that `ExecApprovalResponse` does not
    conform to `ElicitResult`, so I added a TODO to fix that, since I think
    that should be updated in a separate PR. As it stands, this PR does not
    update any business logic, though it does make a number of members of
    the `mcp-server` crate `pub` so they can be used in the test.
    
    One additional learning from this PR is that
    `std::process::Command::cargo_bin()` from the `assert_cmd` trait is only
    available for `std::process::Command`, but we really want to use
    `tokio::process::Command` so that everything is async and we can
    leverage utilities like `tokio::time::timeout()`. The trick I came up
    with was to use `cargo_bin()` to locate the program, and then to use
    `std::process::Command::get_program()` when constructing the
    `tokio::process::Command`.
  • fix: address review feedback on #1621 and #1623 (#1631)
    - formalizes `ExecApprovalElicitRequestParams`
    - adds some defensive logic when messages fail to parse
    - fixes a typo in a comment
  • feat: leverage elicitations in the MCP server (#1623)
    This updates the MCP server so that if it receives an
    `ExecApprovalRequest` from the `Codex` session, it in turn sends an [MCP
    elicitation](https://modelcontextprotocol.io/specification/draft/client/elicitation)
    to the client to ask for the approval decision. Upon getting a response,
    it forwards the client's decision via `Op::ExecApproval`.
    
    Admittedly, we should be doing the same thing for
    `ApplyPatchApprovalRequest`, but this is our first time experimenting
    with elicitations, so I'm inclined to defer wiring that code path up
    until we feel good about how this one works.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1623).
    * __->__ #1623
    * #1622
    * #1621
    * #1620
  • chore: introduce OutgoingMessageSender (#1622)
    Previous to this change, `MessageProcessor` had a
    `tokio::sync::mpsc::Sender<JSONRPCMessage>` as an abstraction for server
    code to send a message down to the MCP client. Because `Sender` is cheap
    to `clone()`, it was straightforward to make it available to tasks
    scheduled with `tokio::task::spawn()`.
    
    This worked well when we were only sending notifications or responses
    back down to the client, but we want to add support for sending
    elicitations in #1623, which means that we need to be able to send
    _requests_ to the client, and now we need a bit of centralization to
    ensure all request ids are unique.
    
    To that end, this PR introduces `OutgoingMessageSender`, which houses
    the existing `Sender<OutgoingMessage>` as well as an `AtomicI64` to mint
    out new, unique request ids. It has methods like `send_request()` and
    `send_response()` so that callers do not have to deal with
    `JSONRPCMessage` directly, as having to set the `jsonrpc` for each
    message was a bit tedious (this cleans up `codex_tool_runner.rs` quite a
    bit).
    
    We do not have `OutgoingMessageSender` implement `Clone` because it is
    important that the `AtomicI64` is shared across all users of
    `OutgoingMessageSender`. As such, `Arc<OutgoingMessageSender>` must be
    used instead, as it is frequently shared with new tokio tasks.
    
    As part of this change, we update `message_processor.rs` to embrace
    `await`, though we must be careful that no individual handler blocks the
    main loop and prevents other messages from being handled.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1622).
    * #1623
    * __->__ #1622
    * #1621
    * #1620
  • 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