Commit Graph

16 Commits

  • Add AuthManager and enhance GetAuthStatus command (#2577)
    This PR adds a central `AuthManager` struct that manages the auth
    information used across conversations and the MCP server. Prior to this,
    each conversation and the MCP server got their own private snapshots of
    the auth information, and changes to one (such as a logout or token
    refresh) were not seen by others.
    
    This is especially problematic when multiple instances of the CLI are
    run. For example, consider the case where you start CLI 1 and log in to
    ChatGPT account X and then start CLI 2 and log out and then log in to
    ChatGPT account Y. The conversation in CLI 1 is still using account X,
    but if you create a new conversation, it will suddenly (and
    unexpectedly) switch to account Y.
    
    With the `AuthManager`, auth information is read from disk at the time
    the `ConversationManager` is constructed, and it is cached in memory.
    All new conversations use this same auth information, as do any token
    refreshes.
    
    The `AuthManager` is also used by the MCP server's GetAuthStatus
    command, which now returns the auth method currently used by the MCP
    server.
    
    This PR also includes an enhancement to the GetAuthStatus command. It
    now accepts two new (optional) input parameters: `include_token` and
    `refresh_token`. Callers can use this to request the in-use auth token
    and can optionally request to refresh the token.
    
    The PR also adds tests for the login and auth APIs that I recently added
    to the MCP server.
  • chore: move mcp-server/src/wire_format.rs to protocol/src/mcp_protocol.rs (#2423)
    The existing `wire_format.rs` should share more types with the
    `codex-protocol` crate (like `AskForApproval` instead of maintaining a
    parallel `CodexToolCallApprovalPolicy` enum), so this PR moves
    `wire_format.rs` into `codex-protocol`, renaming it as
    `mcp-protocol.rs`. We also de-dupe types, where appropriate.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2423).
    * #2424
    * __->__ #2423
  • 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
  • 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: 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
  • 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>
  • 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
  • Serializing the eventmsg type to snake_case (#1709)
    This was an abrupt change on our clients. We need to serialize as
    snake_case.
  • 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.
  • 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.
  • 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`.