Skip to content

Latest commit

 

History

History
172 lines (123 loc) · 10.7 KB

File metadata and controls

172 lines (123 loc) · 10.7 KB

Rust/codex-rs

Temporary Reapply Guardrails (rust-v0.125.0)

  • Current work on this branch is an upstream reapply / re-implementation for rust-v0.125.0.
  • Only implementation code and necessary docs may change for this task. Do not add or modify tests or snapshot files.
  • Do not run lint / format / auto-fix commands for this reapply, including cargo fmt, just fmt, cargo clippy, cargo clippy --fix, and just fix.
  • Acceptance for this reapply is limited to the codex-upstream-reapply skill criteria, including cd codex-rs && cargo build -p codex-cli and cd codex-rs && cargo build -p codex-cli --release.

The codex-core crate

Over 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.

To that end: resist adding code to codex-core!

Particularly when introducing a new concept/feature/API, before adding to codex-core, consider whether:

  • There is an existing crate other than codex-core that is an appropriate place for your new code to live.
  • 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.

Likewise, when reviewing code, do not hesitate to push back on PRs that would unnecessarily add code to codex-core.

TUI style conventions

See codex-rs/tui/styles.md.

TUI code conventions

  • Use concise styling helpers from ratatui’s Stylize trait.
    • Basic spans: use "text".into()
    • Styled spans: use "text".red(), "text".green(), "text".magenta(), "text".dim(), etc.
    • Prefer these over constructing styles with Span::styled and Style directly.
    • Example: patch summary file lines
      • Desired: vec![" └ ".into(), "M".red(), " ".dim(), "tui/src/app.rs".dim()]

TUI Styling (ratatui)

  • Prefer Stylize helpers: use "text".dim(), .bold(), .cyan(), .italic(), .underlined() instead of manual Style where possible.
  • 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).
  • Computed styles: if the Style is computed at runtime, using Span::styled is OK (Span::from(text).set_style(style) is also acceptable).
  • Avoid hardcoded white: do not use .white(); prefer the default foreground (no color).
  • Chaining: combine helpers by chaining for readability (e.g., url.cyan().underlined()).
  • 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.
  • 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![…]).
  • 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().
  • 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.

Text wrapping

  • Always use textwrap::wrap to wrap plain strings.
  • 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.
  • If you need to indent wrapped lines, use the initial_indent / subsequent_indent options from RtOptions if you can, rather than writing custom logic.
  • 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.

Tests

Snapshot tests

This repo uses snapshot tests (via insta), especially in codex-rs/tui, to validate rendered output.

Requirement: any change that affects user-visible UI (including adding new UI) must include corresponding insta snapshot coverage (add a new snapshot test if one doesn't exist yet, or update the existing snapshot). Review and accept snapshot updates as part of the PR so UI impact is easy to review and future diffs stay visual.

When UI or text output changes intentionally, update the snapshots as follows:

  • Run tests to generate any updated snapshots:
    • cargo test -p codex-tui
  • Check what’s pending:
    • cargo insta pending-snapshots -p codex-tui
  • Review changes by reading the generated *.snap.new files directly in the repo, or preview a specific file:
    • cargo insta show -p codex-tui path/to/file.snap.new
  • Only if you intend to accept all new snapshots in this crate, run:
    • cargo insta accept -p codex-tui

If you don’t have the tool:

  • cargo install cargo-insta

Test assertions

  • Tests should use pretty_assertions::assert_eq for clearer diffs. Import this at the top of the test module if it isn't already.
  • Prefer deep equals comparisons whenever possible. Perform assert_eq!() on entire objects, rather than individual fields.
  • Avoid mutating process environment in tests; prefer passing environment-derived flags or dependencies from above.

Spawning workspace binaries in tests (Cargo vs Bazel)

  • Prefer codex_utils_cargo_bin::cargo_bin("...") over assert_cmd::Command::cargo_bin(...) or escargot when tests need to spawn first-party binaries.
    • 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.
  • 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.

Integration tests (core)

  • Prefer the utilities in core_test_support::responses when writing end-to-end Codex tests.

  • All mount_sse* helpers return a ResponseMock; hold onto it so you can assert against outbound /responses POST bodies.

  • Use ResponseMock::single_request() when a test should only issue one POST, or ResponseMock::requests() to inspect every captured ResponsesRequest.

  • 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.

  • Build SSE payloads with the provided ev_* constructors and the sse(...).

  • Prefer wait_for_event over wait_for_event_with_timeout.

  • Prefer mount_sse_once over mount_sse_once_match or mount_sse_sequence

  • Typical pattern:

    let mock = responses::mount_sse_once(&server, responses::sse(vec![
        responses::ev_response_created("resp-1"),
        responses::ev_function_call(call_id, "shell", &serde_json::to_string(&args)?),
        responses::ev_completed("resp-1"),
    ])).await;
    
    codex.submit(Op::UserTurn { ... }).await?;
    
    // Assert request body if needed.
    let request = mock.single_request();
    // assert using request.function_call_output(call_id) or request.json_body() or other helpers.

App-server API Development Best Practices

These guidelines apply to app-server protocol work in codex-rs, especially:

  • app-server-protocol/src/protocol/common.rs
  • app-server-protocol/src/protocol/v2.rs
  • app-server/README.md

Core Rules

  • All active API development should happen in app-server v2. Do not add new API surface area to v1.
  • Follow payload naming consistently: *Params for request payloads, *Response for responses, and *Notification for notifications.
  • Expose RPC methods as <resource>/<method> and keep <resource> singular (for example, thread/read, app/list).
  • 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.
  • 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).
  • Always set #[ts(export_to = "v2/")] on v2 request/response/notification types so generated TypeScript lands in the correct namespace.
  • Never use #[serde(skip_serializing_if = "Option::is_none")] for v2 API payload fields. Exception: client->server requests that intentionally have no params may use: params: #[ts(type = "undefined")] #[serde(skip_serializing_if = "Option::is_none")] Option<()>.
  • Keep Rust and TS wire renames aligned. If a field or variant uses #[serde(rename = "...")], add matching #[ts(rename = "...")].
  • For discriminated unions, use explicit tagging in both serializers: #[serde(tag = "type", ...)] and #[ts(tag = "type", ...)].
  • Prefer plain String IDs at the API boundary (do UUID parsing/conversion internally if needed).
  • Timestamps should be integer Unix seconds (i64) and named *_at (for example, created_at, updated_at, resets_at).
  • For experimental API surface area: 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.

Client->server request payloads (*Params)

  • Every optional field must be annotated with #[ts(optional = nullable)]. Do not use #[ts(optional = nullable)] outside client->server request payloads (*Params).
  • 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.
  • 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>.
  • For new list methods, implement cursor pagination by default: request fields pub cursor: Option<String> and pub limit: Option<u32>, response fields pub data: Vec<...> and pub next_cursor: Option<String>.

Development Workflow

  • Update docs/examples when API behavior changes (at minimum app-server/README.md).
  • Regenerate schema fixtures when API shapes change: just write-app-server-schema (and just write-app-server-schema --experimental when experimental API fixtures are affected).
  • Validate with cargo test -p codex-app-server-protocol.
  • Avoid boilerplate tests that only assert experimental field markers for individual request fields in common.rs; rely on schema generation/tests and behavioral coverage instead.