AGENTS.md

1# Rust/codex-rs
2 
3In the codex-rs folder where the rust code lives:
4 
5- Crate names are prefixed with `codex-`. For example, the `core` folder's crate is named `codex-core`
6- When using format! and you can inline variables into {}, always do that.
7- Install any commands the repo relies on (for example `just`, `rg`, or `cargo-insta`) if they aren't already available before running instructions here.
8- Never add or modify any code related to `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` or `CODEX_SANDBOX_ENV_VAR`.
9  - You operate in a sandbox where `CODEX_SANDBOX_NETWORK_DISABLED=1` will be set whenever you use the `shell` tool. Any existing code that uses `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` was authored with this fact in mind. It is often used to early exit out of tests that the author knew you would not be able to run given your sandbox limitations.
10  - Similarly, when you spawn a process using Seatbelt (`/usr/bin/sandbox-exec`), `CODEX_SANDBOX=seatbelt` will be set on the child process. Integration tests that want to run Seatbelt themselves cannot be run under Seatbelt, so checks for `CODEX_SANDBOX=seatbelt` are also often used to early exit out of tests, as appropriate.
11- Always collapse if statements per https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
12- Always inline format! args when possible per https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
13- Use method references over closures when possible per https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls
14- Avoid bool or ambiguous `Option` parameters that force callers to write hard-to-read code such as `foo(false)` or `bar(None)`. Prefer enums, named methods, newtypes, or other idiomatic Rust API shapes when they keep the callsite self-documenting.
15- When you cannot make that API change and still need a small positional-literal callsite in Rust, follow the `argument_comment_lint` convention:
16  - Use an exact `/*param_name*/` comment before opaque literal arguments such as `None`, booleans, and numeric literals when passing them by position.
17  - A method's sole non-self argument is exempt when the method and parameter names match, such as `.enabled(false)` for `fn enabled(&self, enabled: bool)`.
18  - Do not add these comments for string or char literals unless the comment adds real clarity; those literals are intentionally exempt from the lint.
19  - The parameter name in the comment must exactly match the callee signature.
20  - You can run `just argument-comment-lint` to run the lint check locally. This is powered by Bazel, so running it the first time can be slow if Bazel is not warmed up, though incremental invocations should take <15s. Most of the time, it is best to update the PR and let CI take responsibility for checking this (or run it asynchronously in the background after submitting the PR). Note CI checks all three platforms, which the local run does not.
21- When possible, make `match` statements exhaustive and avoid wildcard arms.
22- Newly added traits should include doc comments that explain their role and how implementations are expected to use them.
23- Discourage both `#[async_trait]` and `#[allow(async_fn_in_trait)]` in Rust traits.
24  - Prefer native RPITIT trait methods with explicit `Send` bounds on the returned future, as in `3c7f013f9735` / `#16630`.
25  - Preferred trait shape:
26    `fn foo(&self, ...) -> impl std::future::Future<Output = T> + Send;`
27  - Implementations may still use `async fn foo(&self, ...) -> T` when they satisfy that contract.
28  - Do not use `#[allow(async_fn_in_trait)]` as a shortcut around spelling the future contract explicitly.
29- When writing tests, prefer comparing the equality of entire objects over fields one by one.
30- Do not add tests for values that are statically defined.
31- Do not add negative tests for logic that was removed.
32- Do not add general product or user-facing documentation to the `docs/` folder. The official Codex documentation lives elsewhere. The exception is app-server API documentation, which is covered by the app-server guidance below.
33- Prefer private modules and explicitly exported public crate API.
34- If you change `ConfigToml` or nested config types, run `just write-config-schema` to update `codex-rs/core/config.schema.json`.
35- When working with MCP tool calls, prefer using `codex-rs/codex-mcp/src/mcp_connection_manager.rs` to handle mutation of tools and tool calls. Aim to minimize the footprint of changes and leverage existing abstractions rather than plumbing code through multiple levels of function calls.
36- Do not call `reset_client_session` unnecessarily; let the incremental check logic decide whether to reuse the previous request.
37- If you change Rust dependencies (`Cargo.toml` or `Cargo.lock`), run `just bazel-lock-update` from the
38  repo root to refresh `MODULE.bazel.lock`, and include that lockfile update in the same change. CI
39  verifies lockfile drift.
40- Bazel does not automatically make source-tree files available to compile-time Rust file access. If
41  you add `include_str!`, `include_bytes!`, `sqlx::migrate!`, or similar build-time file or
42  directory reads, update the crate's `BUILD.bazel` (`compile_data`, `build_script_data`, or test
43  data) or Bazel may fail even when Cargo passes.
44- Do not create small helper methods that are referenced only once.
45- For tracing async work, instrument the function or method definition with
46  `#[tracing::instrument(...)]` instead of attaching spans to futures with
47  `.instrument(...)` at call sites. Before adding instrumentation, check whether the callee—or
48  the implementation method it immediately delegates to—is already instrumented.
49- Avoid large modules:
50  - Prefer adding new modules instead of growing existing ones.
51  - Target Rust modules under 500 LoC, excluding tests.
52  - If a file exceeds roughly 800 LoC, add new functionality in a new module instead of extending
53    the existing file unless there is a strong documented reason not to.
54  - This rule applies especially to high-touch files that already attract unrelated changes, such
55    as `codex-rs/tui/src/app.rs`, `codex-rs/tui/src/bottom_pane/chat_composer.rs`,
56    `codex-rs/tui/src/bottom_pane/footer.rs`, `codex-rs/tui/src/chatwidget.rs`,
57    `codex-rs/tui/src/bottom_pane/mod.rs`, and similarly central orchestration modules.
58  - When extracting code from a large module, move the related tests and module/type docs toward
59    the new implementation so the invariants stay close to the code that owns them.
60  - Avoid adding new standalone methods to `codex-rs/tui/src/chatwidget.rs` unless the change is
61    trivial; prefer new modules/files and keep `chatwidget.rs` focused on orchestration.
62- When running Rust commands (e.g. `just fix` or `just test`) be patient with the command and never try to kill them using the PID. Rust lock can make the execution slow, this is expected.
63 
64Run `just fmt` (in the `codex-rs` directory) automatically after you have finished making code changes anywhere in this repository; do not ask for approval to run it. Additionally, run the tests:
65 
661. Do not run `cargo test` directly. Use `just test` so test execution follows the repo defaults.
672. Run the test for the specific project that was changed. For example, if changes were made in `codex-rs/tui`, run `just test -p codex-tui`.
683. Once those pass, if any changes were made in common, core, or protocol, run the complete test suite with `just test`. Avoid `--all-features` for routine local runs because it expands the build matrix and can significantly increase `target/` disk usage; use it only when you specifically need full feature coverage. project-specific or individual tests can be run without asking the user, but do ask the user before running the complete test suite.
69 
70Before finalizing a large change to `codex-rs`, run `just fix -p <project>` (in `codex-rs` directory) to fix any linter issues in the code. Prefer scoping with `-p` to avoid slow workspace‑wide Clippy builds; only run `just fix` without `-p` if you changed shared crates. Do not re-run tests after running `fix` or `fmt`.
71 
72## The `codex-core` crate
73 
74Over time, the `codex-core` crate (defined in `codex-rs/core/`) has become bloated because it is the largest crate, so it is often easier to add something new to `codex-core` rather than refactor out the library code you need so your new code neither takes a dependency on, nor contributes to the size of, `codex-core`.
75 
76To that end: **resist adding code to codex-core**!
77 
78Particularly when introducing a new concept/feature/API, before adding to `codex-core`, consider whether:
79 
80- There is an existing crate other than `codex-core` that is an appropriate place for your new code to live.
81- It is time to introduce a new crate to the Cargo workspace for your new functionality. Refactor existing code as necessary to make this happen.
82 
83Likewise, when reviewing code, do not hesitate to push back on PRs that would unnecessarily add code to `codex-core`.
84 
85## Code Review Rules
86 
87### Crate API surface
88 
89Keep crate API surfaces as small as possible. Avoid proliferating test-only helpers.
90 
91### Model visible context
92 
93Codex maintains a context (history of messages) that is sent to the model in inference requests.
94 
951. No history rewrite - the context must be built up incrementally.
962. Avoid frequent changes to context that cause cache misses.
973. No unbounded items - everything injected in the model context must have a bounded size and a hard cap.
984. No items larger than 10K tokens.
995. Highlight new individual items that can cross >1k tokens as P0. These need an additional manual review.
1006. All injected fragments must be defined as structs in `core/context` and implement ContextualUserFragment trait
101 
102### Breaking changes
103 
104Search for breaking changes in external integration surfaces:
105 
106- app-server APIs
107- raw response item events (`rawResponseItem/*`), even while experimental
108- CLI parameters
109- configuration loading
110- resuming sessions from existing rollouts
111 
112### Test authoring guidance
113 
114For agent changes prefer integration tests over unit tests. Integration tests are under `core/suite` and use `test_codex` to set up a test instance of codex.
115 
116Features that change the agent logic MUST add an integration test:
117 
118- Provide a list of major logic changes and user-facing behaviors that need to be tested.
119 
120If unit tests are needed, put them in a dedicated test file (\*\_tests.rs).
121Avoid test-only functions in the main implementation.
122 
123Check whether there are existing helpers to make tests more streamlined and readable.
124 
125### Change size guidance (800 lines)
126 
127Unless the change is mechanical the total number of changed lines should not exceed 800 lines.
128For complex logic changes the size should be under 500 lines.
129 
130If the change is larger, explore whether it can be split into reviewable stages and identify the smallest coherent stage to land first.
131Base the staging suggestion on the actual diff, dependencies, and affected call sites.
132 
133## TUI style conventions
134 
135See `codex-rs/tui/styles.md`.
136 
137## TUI code conventions
138 
139- Use concise styling helpers from ratatui’s Stylize trait.
140  - Basic spans: use "text".into()
141  - Styled spans: use "text".red(), "text".green(), "text".magenta(), "text".dim(), etc.
142  - Prefer these over constructing styles with `Span::styled` and `Style` directly.
143  - Example: patch summary file lines
144    - Desired: vec!["  └ ".into(), "M".red(), " ".dim(), "tui/src/app.rs".dim()]
145 
146### TUI Styling (ratatui)
147 
148- Prefer Stylize helpers: use "text".dim(), .bold(), .cyan(), .italic(), .underlined() instead of manual Style where possible.
149- Prefer simple conversions: use "text".into() for spans and vec![…].into() for lines; when inference is ambiguous (e.g., Paragraph::new/Cell::from), use Line::from(spans) or Span::from(text).
150- Computed styles: if the Style is computed at runtime, using `Span::styled` is OK (`Span::from(text).set_style(style)` is also acceptable).
151- Avoid hardcoded white: do not use `.white()`; prefer the default foreground (no color).
152- Chaining: combine helpers by chaining for readability (e.g., url.cyan().underlined()).
153- Single items: prefer "text".into(); use Line::from(text) or Span::from(text) only when the target type isn’t obvious from context, or when using .into() would require extra type annotations.
154- Building lines: use vec![…].into() to construct a Line when the target type is obvious and no extra type annotations are needed; otherwise use Line::from(vec![…]).
155- Avoid churn: don’t refactor between equivalent forms (Span::styled ↔ set_style, Line::from ↔ .into()) without a clear readability or functional gain; follow file‑local conventions and do not introduce type annotations solely to satisfy .into().
156- Compactness: prefer the form that stays on one line after rustfmt; if only one of Line::from(vec![…]) or vec![…].into() avoids wrapping, choose that. If both wrap, pick the one with fewer wrapped lines.
157 
158### Text wrapping
159 
160- Always use textwrap::wrap to wrap plain strings.
161- If you have a ratatui Line and you want to wrap it, use the helpers in tui/src/wrapping.rs, e.g. word_wrap_lines / word_wrap_line.
162- If you need to indent wrapped lines, use the initial_indent / subsequent_indent options from RtOptions if you can, rather than writing custom logic.
163- If you have a list of lines and you need to prefix them all with some prefix (optionally different on the first vs subsequent lines), use the `prefix_lines` helper from line_utils.
164 
165## Tests
166 
167### Test module organization
168 
169- When adding a new test module, define its contents in a separate sibling file rather than inline in the implementation file.
170- Use an explicit `#[path = "..._tests.rs"]` attribute so the test filename is descriptive and easy to locate:
171 
172```rust
173  #[cfg(test)]
174  #[path = "parser_tests.rs"]
175  mod tests;
176```
177 
178- This applies only when introducing a new test module. Do not move or rewrite existing inline `#[cfg(test)] mod tests { ... }` modules solely to follow this convention.
179 
180### Snapshot tests
181 
182This repo uses snapshot tests (via `insta`), especially in `codex-rs/tui`, to validate rendered output.
183 
184**Requirement:** any change that affects user-visible UI (including adding new UI) must include
185corresponding `insta` snapshot coverage (add a new snapshot test if one doesn't exist yet, or
186update the existing snapshot). Review and accept snapshot updates as part of the PR so UI impact
187is easy to review and future diffs stay visual.
188 
189When UI or text output changes intentionally, update the snapshots as follows:
190 
191- Run tests to generate any updated snapshots:
192  - `just test -p codex-tui`
193- Check what’s pending:
194  - `cargo insta pending-snapshots -p codex-tui`
195- Review changes by reading the generated `*.snap.new` files directly in the repo, or preview a specific file:
196  - `cargo insta show -p codex-tui path/to/file.snap.new`
197- Only if you intend to accept all new snapshots in this crate, run:
198  - `cargo insta accept -p codex-tui`
199 
200If you don’t have the tool:
201 
202- `cargo install --locked cargo-insta`
203 
204### Benchmarks
205 
206cargo benchmarks can be run with `just bench`, use the divan crate to write new ones.
207 
208Use `just bench-smoke` to dry-run the benchmark for a single iteration to ensure it works.
209 
210### Test assertions
211 
212- Tests should use pretty_assertions::assert_eq for clearer diffs. Import this at the top of the test module if it isn't already.
213- Prefer deep equals comparisons whenever possible. Perform `assert_eq!()` on entire objects, rather than individual fields.
214- Avoid mutating process environment in tests; prefer passing environment-derived flags or dependencies from above.
215 
216### Spawning workspace binaries in tests (Cargo vs Bazel)
217 
218- Prefer `codex_utils_cargo_bin::cargo_bin("...")` over `assert_cmd::Command::cargo_bin(...)` or `escargot` when tests need to spawn first-party binaries.
219  - Under Bazel, binaries and resources may live under runfiles; use `codex_utils_cargo_bin::cargo_bin` to resolve absolute paths that remain stable after `chdir`.
220- When locating fixture files or test resources under Bazel, avoid `env!("CARGO_MANIFEST_DIR")`. Prefer `codex_utils_cargo_bin::find_resource!` so paths resolve correctly under both Cargo and Bazel runfiles.
221 
222### Integration tests
223 
224#### codex_core integration testing
225 
226- Prefer the utilities in `core_test_support::responses` when writing end-to-end Codex tests.
227- Use `TestCodexBuilder::build_with_auto_env()` by default to ensure that new tests work with
228  foreign app/exec OSes. See $remote-tests for details.
229- All `mount_sse*` helpers return a `ResponseMock`; hold onto it so you can assert against outbound `/responses` POST bodies.
230- Use `ResponseMock::single_request()` when a test should only issue one POST, or `ResponseMock::requests()` to inspect every captured `ResponsesRequest`.
231- `ResponsesRequest` exposes helpers (`body_json`, `input`, `function_call_output`, `custom_tool_call_output`, `call_output`, `header`, `path`, `query_param`) so assertions can target structured payloads instead of manual JSON digging.
232- Build SSE payloads with the provided `ev_*` constructors and the `sse(...)`.
233- Prefer `wait_for_event` over `wait_for_event_with_timeout`.
234- Prefer `mount_sse_once` over `mount_sse_once_match` or `mount_sse_sequence`
235 
236- Typical pattern:
237 
238```rust
239  let mock = responses::mount_sse_once(&server, responses::sse(vec![
240      responses::ev_response_created("resp-1"),
241      responses::ev_function_call(call_id, "shell", &serde_json::to_string(&args)?),
242      responses::ev_completed("resp-1"),
243  ])).await;
244 
245  codex.submit(Op::UserTurn { ... }).await?;
246 
247  // Assert request body if needed.
248  let request = mock.single_request();
249  // assert using request.function_call_output(call_id) or request.json_body() or other helpers.
250```
251 
252#### app-server integration testing
253 
254- Tests should exercise app-server's public JSON-RPC API.
255- Use similar server mocking as for core integration tests.
256- Use `TestAppServer::builder().build()` and `TestAppServer::send_thread_start_request_with_auto_env()`
257  by default to ensure that new tests work with foreign app/exec OSes. See `$remote-tests` for
258  details.
259 
260## App-server API Development Best Practices
261 
262These guidelines apply to app-server protocol work in `codex-rs`, especially:
263 
264- `app-server-protocol/src/protocol/common.rs`
265- `app-server-protocol/src/protocol/v2.rs`
266- `app-server/README.md`
267 
268### Core Rules
269 
270- All active API development should happen in app-server v2. Do not add new API surface area to v1.
271- Follow payload naming consistently:
272  `*Params` for request payloads, `*Response` for responses, and `*Notification` for notifications.
273- Expose RPC methods as `<resource>/<method>` and keep `<resource>` singular (for example, `thread/read`, `app/list`).
274- Always expose fields as camelCase on the wire with `#[serde(rename_all = "camelCase")]` unless a tagged union or explicit compatibility requirement needs a targeted rename.
275- Always expose string enum values as camelCase on the wire with matching serde and TS `rename_all = "camelCase"` annotations unless an explicit compatibility requirement needs targeted renames.
276- Exception: config RPC payloads are expected to use snake_case to mirror config.toml keys (see the config read/write/list APIs in `app-server-protocol/src/protocol/v2.rs`).
277- Always set `#[ts(export_to = "v2/")]` on v2 request/response/notification types so generated TypeScript lands in the correct namespace.
278- Never use `#[serde(skip_serializing_if = "Option::is_none")]` for v2 API payload fields.
279  Exception: client->server requests that intentionally have no params may use:
280  `params: #[ts(type = "undefined")] #[serde(skip_serializing_if = "Option::is_none")] Option<()>`.
281- Keep Rust and TS wire renames aligned. If a field or variant uses `#[serde(rename = "...")]`, add matching `#[ts(rename = "...")]`.
282- For discriminated unions, use explicit tagging in both serializers:
283  `#[serde(tag = "type", ...)]` and `#[ts(tag = "type", ...)]`.
284- Prefer plain `String` IDs at the API boundary (do UUID parsing/conversion internally if needed).
285- Timestamps should be integer Unix seconds (`i64`) and named `*_at` (for example, `created_at`, `updated_at`, `resets_at`).
286- For experimental API surface area:
287  use `#[experimental("method/or/field")]`, derive `ExperimentalApi` when field-level gating is needed, and use `inspect_params: true` in `common.rs` when only some fields of a method are experimental.
288 
289### Client->server request payloads (`*Params`)
290 
291- Every optional field must be annotated with `#[ts(optional = nullable)]`. Do not use `#[ts(optional = nullable)]` outside client->server request payloads (`*Params`).
292- Optional collection fields (for example `Vec`, `HashMap`) must use `Option<...>` + `#[ts(optional = nullable)]`. Do not use `#[serde(default)]` to model optional collections, and do not use `skip_serializing_if` on v2 payload fields.
293- When you want omission to mean `false` for boolean fields, use `#[serde(default, skip_serializing_if = "std::ops::Not::not")] pub field: bool` over `Option<bool>`.
294- For new list methods, implement cursor pagination by default:
295  request fields `pub cursor: Option<String>` and `pub limit: Option<u32>`,
296  response fields `pub data: Vec<...>` and `pub next_cursor: Option<String>`.
297 
298### Development Workflow
299 
300- Update app-server docs/examples when API behavior changes (at minimum `app-server/README.md`).
301- Regenerate schema fixtures when API shapes change:
302  `just write-app-server-schema`
303  (and `just write-app-server-schema --experimental` when experimental API fixtures are affected).
304- Validate with `just test -p codex-app-server-protocol`.
305- Avoid boilerplate tests that only assert experimental field markers for individual
306  request fields in `common.rs`; rely on schema generation/tests and behavioral coverage instead.
307 
308## Python Development Best Practices
309 
310### Ignore Python 2 compatibility
311 
312This project uses Python 3+. You should not use the `__future__` module.
313 
314If you need to worry about feature compatibility between different 3.xx point releases, check the
315closest `pyproject.toml`'s `requires-python` field to see what minimum runtime version is supported.
316 
317## Platform Support
318 
319Tests and features must support Linux, macOS and Windows unless feature is explicitly OS-specific.
320 
321Codex supports running connected app-server and exec-server on different operating systems. See the
322`$remote-tests` skill for details about integration testing these configurations.
323