Sync fork with upstream/main#2
Merged
Merged
Conversation
…rsations (openai#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). * openai#2264 * openai#2263 * __->__ openai#2240
…2237) ## Summary Ripgrep is our preferred tool for file search. When users install via `brew install codex`, it's automatically installed as a dependency. We want to ensure that users running via an npm install also have this tool! Microsoft has already solved this problem for VS Code - let's not reinvent the wheel. This approach of appending to the PATH directly might be a bit heavy-handed, but feels reasonably robust to a variety of environment concerns. Open to thoughts on better approaches here! ## Testing - [x] confirmed this import approach works with `node -e "const { rgPath } = require('@vscode/ripgrep'); require('child_process').spawn(rgPath, ['--version'], { stdio: 'inherit' })"` - [x] Ran codex.js locally with `rg` uninstalled, asked it to run `which rg`. Output below: ``` ⚡ Ran command which rg; echo $? ⎿ /Users/dylan.hurd/code/dh--npm-rg/node_modules/@vscode/ripgrep/bin/rg 0 codex Re-running to confirm the path and exit code. - Path: `/Users/dylan.hurd/code/dh--npm-rg/node_modules/@vscode/ripgrep/bin/rg` - Exit code: `0` ```
<img width="328" height="95" alt="image" src="https://github.com/user-attachments/assets/70e1e6c2-a88f-4058-8763-85c3a02eedb4" />
…ze` (openai#2263) This makes `send_response()` easier to work with.
Fixes: openai#2131 Response doesn't have the delay in a separate field (yet) so parse the message.
gray color doesn't work very well with white terminals. `.dim` doesn't have an effect for some reason. after: <img width="1080" height="149" alt="image" src="https://github.com/user-attachments/assets/26c0f8bb-550d-4d71-bd06-11b3189bc1d7" /> Before <img width="1077" height="186" alt="image" src="https://github.com/user-attachments/assets/b1fba0c7-bc4d-4da1-9754-6c0a105e8cd1" />
Replace mixed `⎿` and `L` prefixes with `└` in TUI rendering. <img width="454" height="659" alt="Screenshot 2025-08-13 at 4 02 03 PM" src="https://github.com/user-attachments/assets/61c9c7da-830b-4040-bb79-a91be90870ca" />
…s slow, particularly with --all-features set (openai#2276) I put this PR together because I noticed I have to wait quite a bit longer on my PRs since we added openai#2242 to catch more build issues. I think we should think about reigning in our use of create features, but this should be good enough to speed things up for now.
## Summary - enable reasoning for any model slug starting with `codex-` - provide default model info for `codex-*` slugs - test that codex models are detected and support reasoning ## Testing - `just fmt` - `just fix` *(fails: E0658 `let` expressions in this position are unstable)* - `cargo test --all-features` *(fails: E0658 `let` expressions in this position are unstable)* ------ https://chatgpt.com/codex/tasks/task_i_689d13f8705483208a6ed21c076868e1
…enai#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). * openai#2278 * __->__ openai#2264
…i#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.
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>
Co-authored-by: Dylan <dylan.hurd@openai.com>
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). * openai#2287 * openai#2286 * __->__ openai#2285
…ormat (openai#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). * openai#2287 * __->__ openai#2286 * openai#2285
This adds `ClientRequest::InterruptConversation`, which effectively maps directly to `Op::Interrupt`. --- * __->__ openai#2287 * openai#2286 * openai#2285
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 openai#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.
Instead of:
```
{ Text: { text: string } }
```
It is now:
```
{ type: "text", data: { text: string } }
```
which makes for cleaner discriminated unions
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.
this used to hold the most recent log line, but it was kinda broken and not that useful.
## 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
the pulsing dot felt too noisy to me next to the shimmering "Working" text. we'll bring it back for streaming response text perhaps?
Currently the composer shows `handle_codex_event:<event name>` by default which feels confusing. Let's make it appear in trace.
openai#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.
…ing with CODEX_ (openai#2308) This ensures Codex cannot drop a `.env` file with a value of `CODEX_HOME` that points to a folder that Codex can control.
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
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.
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.
## 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
- For selectable options, use sentences starting in lowercase and not ending with periods. To be honest I don't love this style, but better to be consistent for now. - Tweak some other strings. - Put in more compelling suggestions on launch. Excited to put `/mcp` in there next.
This adds the terminal version to the UA header.
also tweak agents.md for faster `just fix`
…nts (openai#2489) bringing the tui more into tokio-land to make it easier to factorize. fyi @bolinfest
## Summary Follow up to openai#2186 for openai#2072 - we added handling for `applypatch` in default commands, but forgot to add detection to the heredocs logic. ## Testing - [x] Added unit tests
this is in preparation for adding more separate "modes" to the tui, in particular, a "transcript mode" to view a full history once openai#2316 lands. 1. split apart "tui events" from "app events". 2. remove onboarding-related events from AppEvent. 3. move several general drawing tools out of App and into a new Tui class
This PR: - fixes for internal employee because we currently want to prefer SIWC for them. - fixes retrying forever on unauthorized access. we need to break eventually on max retries.
Change to match `.github/workflows/rust-ci.yml`, which was updated in openai#2242: https://github.com/openai/codex/blob/250ae37c8492f475b5f2af3f7a9f3e96973b2657/.github/workflows/rust-ci.yml#L120-L128
## What? Why? How? - When running on Windows, codex often tries to invoke bash commands, which commonly fail (unless WSL is installed) - Fix: Detect if powershell is available and, if so, route commands to it - Also add a shell_name property to environmental context for codex to default to powershell commands when running in that environment ## Testing - Tested within WSL and powershell (e.g. get top 5 largest files within a folder and validated that commands generated were powershell commands) - Tested within Zsh - Updated unit tests --------- Co-authored-by: Eddy Escardo <eddy@openai.com>
this adds a new 'transcript mode' that shows the full event history in a "pager"-style interface. https://github.com/user-attachments/assets/52df7a14-adb2-4ea7-a0f9-7f5eb8235182
Plan is for full CoT summaries to be visible in a "transcript view" when we implement that, but for now they're hidden. https://github.com/user-attachments/assets/e8a1b0ef-8f2a-48ff-9625-9c3c67d92cdb
record the full reasoning trace and show it in transcript mode
previously the upgrade banner was disappearing into scrollback when we cleared the screen to start the tui.
This PR adds the following: * A getAuthStatus method on the mcp server. This returns the auth method currently in use (chatgpt or apikey) or none if the user is not authenticated. It also returns the "preferred auth method" which reflects the `preferred_auth_method` value in the config. * A logout method on the mcp server. If called, it logs out the user and deletes the `auth.json` file — the same behavior in the cli's `/logout` command. * An `authStatusChange` event notification that is sent when the auth status changes due to successful login or logout operations. * Logic to pass command-line config overrides to the mcp server at startup time. This allows use cases like `codex mcp -c preferred_auth_method=apikey`.
## Summary Before we land openai#2243, let's start printing environment_context in our preferred format. This struct will evolve over time with new information, xml gives us a balance of human readable without too much parsing, llm readable, and extensible. Also moves us over to an Option-based struct, so we can easily provide diffs to the model. ## Testing - [x] Updated tests to reflect new format
this actually works fine already in iterm without this change, but Terminal.app adds a bunch of excess whitespace when we clear all. https://github.com/user-attachments/assets/c5bd1809-c2ed-4daa-a148-944d2df52876
This updates our logic for AGENTS.md to match documented behavior, which is to read all AGENTS.md files from cwd up to git root.
<img width="906" height="350" alt="Screenshot 2025-08-20 at 2 38 29 PM" src="https://github.com/user-attachments/assets/272c43c2-dfa8-497f-afa0-cea31e26ca1f" />
no functional change, just moving the code that handles /foo into chatwidget, since most commands just do things with chatwidget.
moves TranscriptApp to be an "overlay", and continue to pump AppEvents while the transcript is active, but forward all tui handling to the transcript screen.
Adding some notes about MCP tool calls are not running within the sandbox
…821-151806 # Conflicts: # codex-rs/core/src/codex.rs # codex-rs/tui/src/chatwidget.rs
|
I have read the CLA Document and I hereby sign the CLA 0 out of 16 committers have signed the CLA. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge latest changes from openai/codex into fork's main.