Commit Graph

8 Commits

  • codex-rs: make tool calls prettier (#1211)
    This PR overhauls how active tool calls and completed tool calls are
    displayed:
    
    1. More use of colour to indicate success/failure and distinguish
    between components like tool name+arguments
    2. Previously, the entire `CallToolResult` was serialized to JSON and
    pretty-printed. Now, we extract each individual `CallToolResultContent`
    and print those
    1. The previous solution was wasting space by unnecessarily showing
    details of the `CallToolResult` struct to users, without formatting the
    actual tool call results nicely
    2. We're now able to show users more information from tool results in
    less space, with nicer formatting when tools return JSON results
    
    ### Before:
    
    <img width="1251" alt="Screenshot 2025-06-03 at 11 24 26"
    src="https://github.com/user-attachments/assets/5a58f222-219c-4c53-ace7-d887194e30cf"
    />
    
    ### After:
    
    <img width="1265" alt="image"
    src="https://github.com/user-attachments/assets/99fe54d0-9ebe-406a-855b-7aa529b91274"
    />
    
    ## Future Work
    
    1. Integrate image tool result handling better. We should be able to
    display images even if they're not the first `CallToolResultContent`
    2. Users should have some way to view the full version of truncated tool
    results
    3. It would be nice to add some left padding for tool results, make it
    more clear that they are results. This is doable, just a little fiddly
    due to the way `first_visible_line` scrolling works
    4. There's almost certainly a better way to format JSON than "all on 1
    line with spaces to make Ratatui wrapping work". But I think that works
    OK for now.
  • Disallow expect via lints (#865)
    Adds `expect()` as a denied lint. Same deal applies with `unwrap()`
    where we now need to put `#[expect(...` on ones that we legit want. Took
    care to enable `expect()` in test contexts.
    
    # Tests
    
    ```
    cargo fmt
    cargo clippy --all-features --all-targets --no-deps -- -D warnings
    cargo test
    ```
  • Workspace lints and disallow unwrap (#855)
    Sets submodules to use workspace lints. Added denying unwrap as a
    workspace level lint, which found a couple of cases where we could have
    propagated errors. Also manually labeled ones that were fine by my eye.
  • Update submodules version to come from the workspace (#850)
    Tie the version of submodules to the workspace version.
  • Update cargo to 2024 edition (#842)
    Some effects of this change:
    - New formatting changes across many files. No functionality changes
    should occur from that.
    - Calls to `set_env` are considered unsafe, since this only happens in
    tests we wrap them in `unsafe` blocks
  • feat: introduce mcp-server crate (#792)
    This introduces the `mcp-server` crate, which contains a barebones MCP
    server that provides an `echo` tool that echoes the user's request back
    to them.
    
    To test it out, I launched
    [modelcontextprotocol/inspector](https://github.com/modelcontextprotocol/inspector)
    like so:
    
    ```
    mcp-server$ npx @modelcontextprotocol/inspector cargo run --
    ```
    
    and opened up `http://127.0.0.1:6274` in my browser:
    
    
    ![image](https://github.com/user-attachments/assets/83fc55d4-25c2-4497-80cd-e9702283ff93)
    
    I also had to make a small fix to `mcp-types`, adding
    `#[serde(untagged)]` to a number of `enum`s.
  • fix: mcp-types serialization wasn't quite working (#791)
    While creating a basic MCP server in
    https://github.com/openai/codex/pull/792, I discovered a number of bugs
    with the initial `mcp-types` crate that I needed to fix in order to
    implement the server.
    
    For example, I discovered that when serializing a message, `"jsonrpc":
    "2.0"` was not being included.
    
    I changed the codegen so that the field is added as:
    
    ```rust
        #[serde(rename = "jsonrpc", default = "default_jsonrpc")]
        pub jsonrpc: String,
    ```
    
    This ensures that the field is serialized as `"2.0"`, though the field
    still has to be assigned, which is tedious. I may experiment with
    `Default` or something else in the future. (I also considered creating a
    custom serializer, but I'm not sure it's worth the trouble.)
    
    While here, I also added `MCP_SCHEMA_VERSION` and `JSONRPC_VERSION` as
    `pub const`s for the crate.
    
    I also discovered that MCP rejects sending `null` for optional fields,
    so I had to add `#[serde(skip_serializing_if = "Option::is_none")]` on
    `Option` fields.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/791).
    * #792
    * __->__ #791
  • feat: introduce mcp-types crate (#787)
    This adds our own `mcp-types` crate to our Cargo workspace. We vendor in
    the
    [`2025-03-26/schema.json`](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/05f204513641c05bd78d056791af99c6c84520fa/schema/2025-03-26/schema.json)
    from the MCP repo and introduce a `generate_mcp_types.py` script to
    codegen the `lib.rs` from the JSON schema.
    
    Test coverage is currently light, but I plan to refine things as we
    start making use of this crate.
    
    And yes, I am aware that
    https://github.com/modelcontextprotocol/rust-sdk exists, though the
    published https://crates.io/crates/rmcp appears to be a competing
    effort. While things are up in the air, it seems better for us to
    control our own version of this code.
    
    Incidentally, Codex did a lot of the work for this PR. I told it to
    never edit `lib.rs` directly and instead to update
    `generate_mcp_types.py` and then re-run it to update `lib.rs`. It
    followed these instructions and once things were working end-to-end, I
    iteratively asked for changes to the tests until the API looked
    reasonable (and the code worked). Codex was responsible for figuring out
    what to do to `generate_mcp_types.py` to achieve the requested test/API
    changes.