openai-codex-rust-patterns

어떤 스킬인가요?

OpenAI Codex Rust coding patterns distilled from the codex-rs workspace. Use this skill whenever writing, reviewing, or refactoring Rust code — especially for async agents, CLI tools, sandboxing, secret handling, Ratatui TUIs, JSON-RPC protocols, tokio-based services, or any codebase that needs defensive panic discipline. Trigger even when the user does not explicitly mention Codex, because the patterns generalize to any production Rust workspace. Covers async cancellation, error enum design, process sandboxing, DNS-rebinding defense, credential hardening (zeroize/mlock/ctor), Cargo workspace architecture, wiremock-based fakes, insta snapshot testing, OpenTelemetry tracing, and Ratatui rendering.

설치 방법

프로젝트 폴더에서 아래 명령어를 실행하고, 설치할 에이전트를 선택하세요.

설치 명령을 직접 실행해야 적용됩니다. 지원 에이전트와 필요한 권한·라이선스는 제작자의 안내를 확인하세요.

스킬 원문 파일

# Rust

**Version 1.1.0**  
OpenAI  
May 2026

---

## Abstract

Distilled Rust coding patterns extracted from openai/codex (codex-rs, 2,008 Rust files across 119 workspace crates), refreshed against main at commit 8a94430 (2026-05-25). Captures the end-to-end craft of its top contributors — Michael Bolin, jif-oai, Ahmed Ibrahim, Eric Traut, and others — across defensive coding, error discipline, async cancellation, sandboxing, secret handling and process hardening, type invariants, testing, protocol design, workspace organization, observability, and Ratatui TUI architecture. Each rule cites the exact codex-rs file and shows a minimal incorrect/correct pair so the reader can internalize the judgment, not just the syntax.

---

## Table of Contents

1. [Defensive Coding & Panic Discipline](references/_sections.md#1-defensive-coding-&-panic-discipline) — **CRITICAL**
   - 1.1 [Avoid learning allowlist rules for general-purpose interpreters](references/defensive-banned-interpreter-prefixes.md) — CRITICAL (prevents approval amendments from green-lighting arbitrary interpreter flags)
   - 1.2 [Canonicalize shell wrappers before hashing approval keys](references/defensive-canonicalize-approval-cache-key.md) — HIGH (avoids re-prompting users for logically identical commands and blocks cache collisions between wrapped scripts)
   - 1.3 [Cap subprocess output with a head-and-tail ring buffer](references/defensive-head-tail-output-buffer.md) — CRITICAL (prevents OOM on runaway output and preserves the most informative tail lines)
   - 1.4 [Deny unwrap and expect at the workspace level](references/defensive-deny-unwrap-workspace-wide.md) — CRITICAL (prevents unreviewed panic sites across a 75-crate workspace)
   - 1.5 [Load untrusted plugins fault-isolated and sanitize their model-facing text](references/defensive-fault-isolate-plugin-load.md) — HIGH (one broken or hostile plugin can't fail startup or inject the model's prompt)
   - 1.6 [Recover a poisoned lock with into_inner instead of unwrapping it](references/defensive-recover-poisoned-lock.md) — CRITICAL (stops one thread's panic from cascading a poisoned lock into every other holder)
   - 1.7 [Refuse to run when the sandbox cannot enforce the policy](references/defensive-refuse-to-run-unsandboxed.md) — CRITICAL (prevents silent privilege erosion when the sandbox backend lacks the required primitive)
   - 1.8 [Register a drain timeout to escape grandchild pipe leaks](references/defensive-io-drain-timeout-grandchildren.md) — CRITICAL (prevents the whole agent from hanging when a killed child leaves grandchildren holding stdout)
   - 1.9 [Use debug_assert with safe fallback on unreachable branches](references/defensive-debug-assert-with-early-return.md) — CRITICAL (prevents release-mode panics while keeping bugs loud in tests)
2. [Error Handling & Result Discipline](references/_sections.md#2-error-handling-&-result-discipline) — **CRITICAL**
   - 2.1 [Carry the server-requested retry delay inside the error variant](references/errors-carry-retry-delay-in-variant.md) — HIGH (eliminates out-of-band retry-after plumbing through the error flow)
   - 2.2 [Classify retryable errors via an exhaustive match](references/errors-exhaustive-retryable-match.md) — CRITICAL (prevents silent retry drift when a new error variant is added)
   - 2.3 [Encode transient vs permanent failures as two enum variants](references/errors-transient-permanent-type-split.md) — CRITICAL (prevents boolean retry-policy checks that drift out of sync with the error source)
   - 2.4 [Split tool errors into respond-to-model and fatal variants](references/errors-tool-call-respond-vs-fatal.md) — HIGH (eliminates downcasting every failure to decide whether the LLM can recover)
   - 2.5 [Store display-relevant error state in a struct, not a string](references/errors-struct-display-payload.md) — HIGH (enables plan-specific tests to assert against error state instead of fragile English sentences)
   - 2.6 [Translate errors at the layer boundary in one function](references/errors-boundary-error-translator.md) — CRITICAL (eliminates reqwest error status inspection scattered across business logic)
   - 2.7 [Wrap io::Error in a struct with a context field](references/errors-io-error-with-context-struct.md) — MEDIUM-HIGH (enables PartialEq tests against IoError without dragging anyhow into a library crate)
3. [Async, Concurrency & Cancellation](references/_sections.md#3-async,-concurrency-&-cancellation) — **HIGH**
   - 3.1 [Bound submissions but leave events unbounded](references/async-bounded-vs-unbounded-channel-split.md) — HIGH (avoids session loop stalls on slow consumers while rate-limiting misbehaving producers)
   - 3.2 [Cancel cooperatively first, then abort after a grace deadline](references/async-graceful-then-forceful-cancel.md) — HIGH (prevents unbounded shutdown latency while still letting well-behaved tasks clean up)
   - 3.3 [Give spawned sub-tasks child tokens, not parent clones](references/async-child-cancellation-tokens.md) — HIGH (prevents cancelling one child from cascading into all siblings)
   - 3.4 [Store JoinHandles as AbortOnDropHandle so Drop cancels them](references/async-abort-on-drop-handle.md) — HIGH (prevents leaked background tasks when a session or turn is cleared)
   - 3.5 [Use biased select when cancellation must win ties](references/async-biased-select-for-cancellation.md) — HIGH (prevents rare approval races where cancel and response fire in the same poll)
   - 3.6 [Wrap background JoinHandle in Shared BoxFuture for multi-waiter joins](references/async-shared-boxfuture-joinhandle.md) — MEDIUM-HIGH (enables multiple independent callers to await the same background task completion)
4. [Sandboxing & Process Isolation](references/_sections.md#4-sandboxing-&-process-isolation) — **HIGH**
   - 4.1 [Clear the env and tether children via pre_exec before every spawn](references/sandbox-env-clear-pre-exec.md) — HIGH (prevents LD_PRELOAD inheritance and orphaned grandchildren after a parent kill)
   - 4.2 [Keep sandbox policy as shared data, not per-platform code](references/sandbox-shared-policy-data-model.md) — HIGH (prevents three independently-drifting notions of "workspace-write")
   - 4.3 [Mount /dev/null over the first missing path component](references/sandbox-dev-null-first-missing-mount.md) — HIGH (prevents mkdir-and-write escapes through non-existent protected paths)
   - 4.4 [Multiplex helper binaries via argv[0] and symlinks](references/sandbox-argv0-multiplex-binary.md) — MEDIUM-HIGH (eliminates TOCTOU risk and packaging overhead of shipping multiple binaries)
   - 4.5 [Resolve hostnames and reject private IPs before allowing egress](references/sandbox-resolve-before-allow-dns-rebinding.md) — HIGH (defeats DNS-rebinding bypass of a string-based egress allowlist)
   - 4.6 [Stack env, syscalls, and namespace for network isolation](references/sandbox-three-layer-network-isolation.md) — HIGH (prevents network escape through any single uncooperative tool)
   - 4.7 [Stage incompatible restrictions via re-executing the same binary](references/sandbox-staged-restrictions-re-exec.md) — HIGH (eliminates the "seccomp breaks bwrap" conflict via two-stage application)
5. [Secrets & Process Hardening](references/_sections.md#5-secrets-&-process-hardening) — **HIGH**
   - 5.1 [Harden a secret-handling process before main() runs, and fail closed](references/secrets-ctor-pre-main-hardening.md) — HIGH (closes the core-dump / ptrace / LD_PRELOAD window before any arg parsing or allocation)
   - 5.2 [Read a secret into a zeroized stack buffer, then mlock it — never through stdin()](references/secrets-read-into-locked-buffer.md) — HIGH (guarantees exactly one in-memory copy of an API key, locked out of swap and core dumps)
   - 5.3 [Write a manual Debug impl that elides credentials instead of deriving it](references/secrets-manual-debug-elide.md) — HIGH (stops tokens and credential providers leaking into {:?} and tracing output)
6. [Type Design & Invariants](references/_sections.md#6-type-design-&-invariants) — **HIGH**
   - 6.1 [Mark public wire-level enums non_exhaustive from the start](references/types-non-exhaustive-public-enums.md) — HIGH (prevents breaking external match statements when a variant is added)
   - 6.2 [Pass deserializer context via a thread-local RAII guard](references/types-thread-local-raii-serde.md) — HIGH (enables serde to run path resolution without DeserializeSeed plumbing)
   - 6.3 [Preserve unrecognized wire values in an Unknown variant](references/types-unknown-variant-forward-compat.md) — HIGH (prevents older readers from crashing on configs written by newer versions)
   - 6.4 [Use serde try_from on newtypes to run validation on every parse](references/types-try-from-newtype-validation.md) — HIGH (eliminates forgotten validation calls at construction sites via parse-don't-validate)
7. [Testing Architecture](references/_sections.md#7-testing-architecture) — **MEDIUM-HIGH**
   - 7.1 [Attach tests as sibling files via a path attribute](references/testing-path-attribute-sibling-tests.md) — MEDIUM-HIGH (prevents 5000-line modules where implementation hides inside a mile-long test body)
   - 7.2 [Enable test-only behavior via AtomicBool, not a cargo feature](references/testing-atomic-bool-test-opt-in.md) — MEDIUM-HIGH (avoids doubling the build matrix while keeping deterministic IDs for tests)
   - 7.3 [Snapshot terminal rendering with insta for stable TUI diffs](references/testing-insta-snapshot-tui-rendering.md) — MEDIUM-HIGH (enables 1400 reviewable terminal snapshots that diff cleanly in PRs)
   - 7.4 [Use start_paused and advance for deterministic timing tests](references/testing-paused-runtime-advance.md) — MEDIUM-HIGH (eliminates wall-clock flakes from timing-dependent tests)
   - 7.5 [Use wiremock and small SSE constructors instead of mocking HTTP traits](references/testing-wiremock-sse-fakes.md) — MEDIUM-HIGH (enables serialization, retry, and streaming coverage on every test)
8. [Protocol & Serde Design](references/_sections.md#8-protocol-&-serde-design) — **MEDIUM-HIGH**
   - 8.1 [Dispatch JSON-RPC via an internally tagged enum with a macro](references/proto-internally-tagged-rpc-dispatch.md) — MEDIUM-HIGH (eliminates hand-rolled method dispatch that drifts from typed param validation)
   - 8.2 [Gate experimental fields by runtime presence, not capability flags](references/proto-experimental-runtime-gate.md) — MEDIUM-HIGH (enables adding unstable fields to stable methods without duplicating the request type)
   - 8.3 [Keep removed feature flags as parseable no-op tombstones](references/proto-removed-feature-tombstone.md) — MEDIUM-HIGH (lets old and new configs round-trip across versions without parse failures)
   - 8.4 [Pair rename and alias to migrate wire names without breaking clients](references/proto-rename-alias-wire-migration.md) — MEDIUM-HIGH (prevents flag-day migrations by keeping old wire names as read-only aliases)
   - 8.5 [Split internal error enums from wire error enums](references/proto-internal-vs-wire-error-split.md) — MEDIUM-HIGH (enables internal error refactors without breaking the stable wire contract)
   - 8.6 [Treat SSE streams as idle-timeout with a required terminator](references/proto-sse-idle-timeout-terminator.md) — MEDIUM-HIGH (prevents long turns from being killed by wall-clock deadlines and silent half-closes)
   - 8.7 [Use double-nested Options to distinguish absent, null, and set](references/proto-double-option-tri-state.md) — MEDIUM-HIGH (eliminates invented FieldAction enums for PATCH-like update APIs)
9. [Workspace & Crate Organization](references/_sections.md#9-workspace-&-crate-organization) — **MEDIUM**
   - 9.1 [Avoid per-crate features; use target-cfg or split crates](references/workspace-ban-per-crate-features.md) — MEDIUM (prevents combinatorial build matrix explosion across a ~100-crate workspace)
   - 9.2 [Encode design policy in workspace.lints and clippy.toml](references/workspace-lint-config-package.md) — MEDIUM (prevents policy drift from review-only conventions)
   - 9.3 [Place shared utilities in single-purpose microcrates under utils/](references/workspace-utils-microcrate-fanout.md) — MEDIUM (enables parallel compilation and minimal dependency graphs per concern)
   - 9.4 [Register shared test helpers as workspace member crates](references/workspace-test-support-as-member-crates.md) — MEDIUM (enables cross-crate test helper reuse without path-attribute hacks)
   - 9.5 [Stack HTTP layers as transport, api, and core crates](references/workspace-layered-transport-api-core.md) — MEDIUM (enables client crate reuse and prevents business logic from pulling in retries)
10. [Observability & Tracing](references/_sections.md#10-observability-&-tracing) — **MEDIUM**
   - 10.1 [Build per-layer EnvFilter instances with boxed fmt layers](references/otel-layered-subscribers-env-filter.md) — MEDIUM (enables independently-filtered sinks without per-layer generic divergence)
   - 10.2 [Declare span fields as field Empty then record when known](references/otel-field-empty-then-record.md) — MEDIUM (reduces duplicate child spans by keeping one parent span renamed at the apm)
   - 10.3 [Default instrument spans to trace level, reserve info for network calls](references/otel-instrument-at-trace-level.md) — MEDIUM (enables free internal instrumentation that costs zero in normal operation)
   - 10.4 [Propagate W3C traceparent via env, RPC, and HTTP headers](references/otel-w3c-traceparent-propagation.md) — MEDIUM (enables distributed tracing from CI runner through codex to backend APIs)
   - 10.5 [Route PII to log-only targets and keep traces cardinality-safe](references/otel-log-only-vs-trace-safe-targets.md) — MEDIUM (prevents PII from leaking into wider-access trace backends)
11. [TUI (Ratatui) Rendering](references/_sections.md#11-tui-(ratatui)-rendering) — **MEDIUM**
   - 11.1 [Coalesce redraws through a FrameRequester actor](references/tui-schedule-frame-coalescer.md) — MEDIUM (reduces redraw count when multiple producers request frames in the same tick)
   - 11.2 [Detect unbracketed paste bursts via a character timing state machine](references/tui-paste-burst-state-machine.md) — MEDIUM (prevents mid-paste shortcut key interpretation on terminals without bracketed paste)
   - 11.3 [Pause the event stream by dropping it before subprocess handoff](references/tui-event-broker-pause-resume.md) — MEDIUM (prevents stdin race with child processes after handing off the terminal)
   - 11.4 [Replace fixed throttles with hysteresis-gated smooth and catch-up modes](references/tui-two-gear-hysteresis-chunking.md) — MEDIUM (prevents visible lag on bursts without sacrificing the typewriter cadence feel)
   - 11.5 [Restore terminal state via a Drop guard and chained panic hook](references/tui-drop-guard-panic-hook-chain.md) — MEDIUM (prevents wedged terminals that require manual `reset` after a panic)

---

## References

1. [https://github.com/openai/codex](https://github.com/openai/codex)
2. [https://github.com/openai/codex/tree/main/codex-rs](https://github.com/openai/codex/tree/main/codex-rs)
3. [https://github.com/openai/codex/blob/main/AGENTS.md](https://github.com/openai/codex/blob/main/AGENTS.md)
4. [https://developers.openai.com/codex](https://developers.openai.com/codex)

---

## Source Files

This document was compiled from individual reference files. For detailed editing or extension:

| File | Description |
|------|-------------|
| [references/_sections.md](references/_sections.md) | Category definitions and ordering |
| [SKILL.md](SKILL.md) | Quick reference entry point |
| [metadata.json](metadata.json) | Version and reference URLs |

# Gotchas

Append failure points discovered while applying the rules in this skill. Each entry should be specific enough that a future reader can avoid it, not just "be careful with X".

Patterns in this skill were last refreshed against `openai/codex` `main` at commit `8a94430` on 2026-05-25. codex-rs refactors aggressively, so line references — and sometimes whole file paths — drift fast. Cross-check with the live repo before quoting an exact location.

## codex-rs moves fast: verify file *paths*, not just line numbers

In the ~6 weeks between the first extraction (2026-04-12) and the first refresh (2026-05-25) the repo grew from 1,418 to 2,008 Rust files (72 → 119 crates), and only 5 of 60 citations were still byte-for-byte correct. Two structural moves caused most of the churn:

- **`core/src/codex.rs` was deleted and split** into `core/src/session/mod.rs`, `core/src/session/turn.rs`, and `core/src/codex_delegate.rs`. Any rule that cited `core/src/codex.rs` now points at one of those. When re-validating, grep for the *symbol* (a struct/fn name from the Correct block), not the old path.
- **`FunctionCallError` was extracted into a new `codex-tools` crate** (`tools/src/function_call_error.rs`); `core/src/function_tool.rs` is now just a `pub use` re-export. Watch for similar "type lifted into its own crate" moves.

## The repo migrated to Bazel — the `justfile` is gone

Build/policy that used to live in the `justfile` now lives in `BUILD.bazel` + `docs/bazel.md`. Don't cite the `justfile`. Relatedly, the absolute claim "there is not a single `[features]` section in the workspace" is **no longer true**: `code-mode` and `v8-poc` each declare `[features] sandbox = ["v8/v8_enable_sandbox"]` to forward a native-dependency build flag. State conventions as "codex avoids X except where Y," not as absolutes — absolutes rot.

## A few docs were pruned

`docs/tui-chat-composer.md` and `docs/tui-stream-chunking-tuning.md` (and the older `-review.md`) were removed; `docs/` now holds only `bazel.md`, `codex_mcp_interface.md`, and `protocol_v1.md`. Prefer citing source `.rs` files over `docs/*.md`, which are deleted more readily.

## Don't trust "never uses X" claims without grepping

A mining pass proposed a rule that codex "never uses `#[async_trait]`, always spells out `impl Future + Send`." A `git grep` found **78** live `#[async_trait]` uses — both styles coexist. Before encoding an absolute behavioral claim, count occurrences in the live tree.

{
  "version": "1.1.0",
  "organization": "OpenAI",
  "technology": "Rust",
  "discipline": "distillation",
  "type": "library-reference",
  "date": "May 2026",
  "abstract": "Distilled Rust coding patterns extracted from openai/codex (codex-rs, 2,008 Rust files across 119 workspace crates), refreshed against main at commit 8a94430 (2026-05-25). Captures the end-to-end craft of its top contributors — Michael Bolin, jif-oai, Ahmed Ibrahim, Eric Traut, and others — across defensive coding, error discipline, async cancellation, sandboxing, secret handling and process hardening, type invariants, testing, protocol design, workspace organization, observability, and Ratatui TUI architecture. Each rule cites the exact codex-rs file and shows a minimal incorrect/correct pair so the reader can internalize the judgment, not just the syntax.",
  "references": [
    "https://github.com/openai/codex",
    "https://github.com/openai/codex/tree/main/codex-rs",
    "https://github.com/openai/codex/blob/main/AGENTS.md",
    "https://developers.openai.com/codex"
  ]
}

# OpenAI Codex Rust Patterns — Skill Repository

## Overview

This skill distills non-obvious Rust coding patterns from [`openai/codex`](https://github.com/openai/codex) — specifically the `codex-rs/` workspace, a 72-crate, 1,418-file Rust codebase that implements the Codex CLI coding agent. Every rule here is extracted from actual production code written by the codex team (Michael Bolin, jif-oai, Ahmed Ibrahim, Eric Traut, Pavel Krymets, and others) and cites the exact file where the pattern lives.

Unlike most Rust "best practices" skills, the rules here are not copied from the Rust book or tutorial sites. They come from reading the code of people who ship a production coding agent that must survive LLM-generated input, cross-platform sandboxing, bursty streaming, and a 75-crate workspace — and they encode judgment that isn't obvious until you've been burned.

## Getting Started

```bash
pnpm install
pnpm build
pnpm validate
```

The skill is read by Claude Code automatically when it triggers. Agents can also browse `references/` directly to read individual rules.

To regenerate `AGENTS.md` after adding rules:

```bash
node /Users/pedroproenca/.claude/plugins/marketplaces/dot-claude/plugins/dev-skill/scripts/build-agents-md.js ~/.claude/skills/.experimental/openai-codex-rust-patterns
```

To validate the skill after changes:

```bash
node /Users/pedroproenca/.claude/plugins/marketplaces/dot-claude/plugins/dev-skill/scripts/validate-skill.js ~/.claude/skills/.experimental/openai-codex-rust-patterns
```

## Creating a New Rule

1. Pick a category prefix from `references/_sections.md` (e.g. `async`, `defensive`, `errors`).
2. Create `references/{prefix}-{slug}.md` — filename is all lowercase kebab-case.
3. Fill in the frontmatter: `title`, `impact`, `impactDescription`, `tags` (first tag must equal the category prefix).
4. Write the body: H2 heading matching the title, a 2–4 sentence explanation of the WHY, then `**Incorrect (annotation):**` and `**Correct (annotation):**` code blocks with language specifiers.
5. Re-run `build-agents-md.js` and `validate-skill.js`.

## Rule File Structure

Every rule file has this shape:

```markdown
---
title: Imperative Verb + Noun Phrase
impact: CRITICAL | HIGH | MEDIUM-HIGH | MEDIUM | LOW-MEDIUM | LOW
impactDescription: Quantified outcome (e.g., "prevents silent data loss on cancel")
tags: {prefix}, {technique}, {tool}, {concept}
---

## Imperative Verb + Noun Phrase

Explanation of the pattern and why it works — the reasoning the reader should
internalize so they can apply it to novel situations. 2–4 sentences.

**Incorrect (describes the failure mode):**

```rust
// naive / broken version
```

**Correct (describes the benefit):**

```rust
// the codex-rs pattern — cited with file:line in the explanation
```

Reference: `codex-rs/{crate}/src/{path}.rs`
```

## File Naming Convention

- Directory: `references/`
- Pattern: `{prefix}-{slug}.md`
- Example: `async-abort-on-drop-handle.md`
- The prefix must match one of the category prefixes defined in `references/_sections.md`.
- Slugs are lowercase kebab-case, ideally short enough to read in a TOC.

## Impact Levels

The skill uses six levels, ordered highest to lowest:

| Level | When to use |
|-------|-------------|
| CRITICAL | Pattern prevents a class of production outages or silent corruption. The reader should never ship without it. |
| HIGH | Pattern saves meaningful debugging time, prevents common correctness bugs, or unblocks a whole architecture. |
| MEDIUM-HIGH | Pattern is load-bearing for a specific concern (tests, protocols) and is non-obvious. |
| MEDIUM | Pattern cleans up a real friction point. The codebase suffers without it but does not crash. |
| LOW-MEDIUM | Pattern is specific to a UI layer or tooling surface; broadly applicable but narrower in scope. |
| LOW | Minor stylistic or convention-level guidance. |

Impact inflation is a red flag — the distillation rubric expects at most 1–2 CRITICAL categories.

## Scripts

| Script | Purpose |
|--------|---------|
| `dev-skill/scripts/validate-skill.js` | Runs structural + substance validation. |
| `dev-skill/scripts/build-agents-md.js` | Regenerates `AGENTS.md` from rule files. |

Both scripts live in the dev-skill plugin, not inside the skill itself.

## Contributing

Additions should:

1. Come from real production code, not invented examples.
2. Cite the exact `codex-rs/` file path for traceability.
3. Explain the WHY the pattern matters — what goes wrong without it.
4. Include both an incorrect and a correct example that differ minimally.
5. Pass `validate-skill.js` with zero errors before submission.

Rules that merely restate Rust book material (use Result, prefer enums, avoid unwrap) are rejected — the quality bar is "surprising to a mid-level Rust engineer".

# Rule Categories

This document defines the category structure, impact levels, and file-name prefixes used by every rule in `references/`. Categories are ordered CRITICAL → LOW so the reader sees highest-impact patterns first.

## 1. Defensive Coding & Panic Discipline (defensive)

**Impact:** CRITICAL
**Description:** Patterns that prevent panics in production and turn "should never happen" into grepable tombstones. Codex cannot crash when a tool handler sees malformed input, a subprocess spawns a grandchild, a lock is poisoned by another task, or a sandbox backend cannot enforce the requested policy — so the defensive rules are load-bearing for service availability. Also covers treating loaded plugins as untrusted input.

## 2. Error Handling & Result Discipline (errors)

**Impact:** CRITICAL
**Description:** Patterns that shape how `Result` and error enums flow through the system. Retry classification, transient-vs-permanent splits, layer-boundary translators, and struct-typed display payloads — the things that decide whether a user sees a clean message or a cryptic trace.

## 3. Async, Concurrency & Cancellation (async)

**Impact:** HIGH
**Description:** Tokio patterns for long-lived agents where tasks must clean up reliably, cancellation must win race ties, and channels must balance throughput against responsiveness. Covers AbortOnDropHandle, CancellationToken discipline, and the bounded-vs-unbounded channel split.

## 4. Sandboxing & Process Isolation (sandbox)

**Impact:** HIGH
**Description:** Cross-platform sandbox patterns from running LLM-generated commands under Seatbelt, Landlock, seccomp, and Windows restricted tokens. Policy-as-data, argv[0] multiplexing, staged restrictions, refusing to run when enforcement is impossible, and resolving hostnames to defeat DNS-rebinding bypass of an egress allowlist.

## 5. Secrets & Process Hardening (secrets)

**Impact:** HIGH
**Description:** Patterns for protecting credentials and the process itself, drawn from the responses-api proxy, `process-hardening`, and the auth crates. Reading a secret into a single zeroized, mlock'd copy; `#[ctor]` pre-main hardening that fails closed; and hand-written `Debug` impls that elide credentials so they never reach a log.

## 6. Type Design & Invariants (types)

**Impact:** HIGH
**Description:** Newtype, enum, and trait patterns that encode invariants at compile time — thread-local RAII for serde context, try_from-driven validation, and forward-compatible enum variants.

## 7. Testing Architecture (testing)

**Impact:** MEDIUM-HIGH
**Description:** Test organization patterns from a codebase with multi-thousand-line test files. Sibling `foo_tests.rs` files via `#[path]`, wiremock-based fakes for SSE streams, AtomicBool test opt-ins, insta snapshot tests for Ratatui, and start_paused deterministic timing.

## 8. Protocol & Serde Design (proto)

**Impact:** MEDIUM-HIGH
**Description:** Serde-based protocol patterns for a JSON-RPC-like wire format with streaming, experimental fields, and forward compatibility — macro-generated dispatchers, Option<Option<T>>, rename+alias migration, runtime experimental gating, and removed-feature tombstones for config round-tripping.

## 9. Workspace & Crate Organization (workspace)

**Impact:** MEDIUM
**Description:** Cargo workspace patterns from a ~100-crate monorepo with near-zero per-crate features, layered transport/api/core crates, test-support as member crates, utils microcrate fan-out, and workspace-level lint enforcement via clippy.toml.

## 10. Observability & Tracing (otel)

**Impact:** MEDIUM
**Description:** tracing and OpenTelemetry patterns for services with privacy constraints — log-only vs trace-safe targets, field::Empty placeholders, W3C traceparent propagation across env vars and RPC envelopes, and trace-level `#[instrument]` as the default.

## 11. TUI (Ratatui) Rendering (tui)

**Impact:** MEDIUM
**Description:** Ratatui patterns from a streaming LLM TUI — two-gear hysteresis chunking, frame-request coalescing, panic-hook terminal restoration, unbracketed-paste burst detection, and pausing the event stream before a subprocess handoff.

---
title: Store JoinHandles as AbortOnDropHandle so Drop cancels them
impact: HIGH
impactDescription: prevents leaked background tasks when a session or turn is cleared
tags: async, cancellation, structured-concurrency, tokio-util
---

## Store JoinHandles as AbortOnDropHandle so Drop cancels them

A raw `JoinHandle` forces every cleanup path to remember `handle.abort()`; one missed error branch and the task leaks, running on against state that's being torn down. Codex stores tasks as `tokio_util::task::AbortOnDropHandle` inside its state structs, so dropping the owner aborts the task automatically — clearing the turn's task map is enough, with no abort call to forget. When a task is *meant* to outlive its owner, that intent is made explicit with `.detach()` rather than left implicit.

**Incorrect (easy to leak tasks on error paths):**

```rust
pub(crate) struct RunningTask {
    pub(crate) handle: JoinHandle<()>,
}
fn clear_turn(turn: &mut ActiveTurn) {
    for task in turn.drain_tasks() {
        task.handle.abort(); // every error branch must remember this
    }
}
```

**Correct (Drop does the work; detach is the deliberate opt-out):**

```rust
// core/src/state/turn.rs
pub(crate) struct RunningTask {
    pub(crate) cancellation_token: CancellationToken,
    pub(crate) handle: AbortOnDropHandle<()>,
    /* ... */
}

// core/src/tasks/mod.rs — construction site
let handle = tokio::spawn(async move { /* task body */ }.instrument(task_span));
turn.add_task(RunningTask { handle: AbortOnDropHandle::new(handle), /* ... */ });

// core/src/state/turn.rs — when removal should NOT cancel, say so explicitly
let task = self.tasks.swap_remove(sub_id)?;
task.handle.detach(); // intentionally let it finish after removal
```

Clearing the `IndexMap<String, RunningTask>` drops every `AbortOnDropHandle`, which aborts each underlying task — you never grep for "where is the abort". The one place a task should survive removal calls `detach()`, so the exception is visible in the code rather than being an accidental leak. Pair with [[async-child-cancellation-tokens]] for cooperative shutdown before the hard abort.

Reference: `codex-rs/core/src/state/turn.rs:77`, `codex-rs/core/src/tasks/mod.rs:445`.

---
title: Use biased select when cancellation must win ties
impact: HIGH
impactDescription: prevents rare approval races where cancel and response fire in the same poll
tags: async, cancellation, select, tokio
---

## Use biased select when cancellation must win ties

`tokio::select!` picks a ready branch at random by default — deliberate to avoid starvation, but it means a cancelled token and a just-arrived response can coin-flip. One run in fifty, cancellation loses the race and an approval appears granted. Adding `biased;` evaluates branches top-down instead, so the cancellation arm always wins when both are ready. Critically, the cancel arm does not just break — it actively notifies the parent waiter with an empty response so any pending consumer unwinds instead of hanging on an orphaned approval.

**Incorrect (plain select, occasional lost cancellation):**

```rust
tokio::select! {
    _ = cancel_token.cancelled() => { /* tear down */ }
    response = fut => { return response; }
}
```

**Correct (biased + active unwind):**

```rust
// core/src/codex_delegate.rs
tokio::select! {
    biased;
    _ = cancel_token.cancelled() => {
        let empty = RequestUserInputResponse {
            answers: HashMap::new(),
        };
        parent_session
            .notify_user_input_response(sub_id, empty.clone())
            .await;
        empty
    }
    response = fut => response.unwrap_or_else(|| {
        RequestUserInputResponse { answers: HashMap::new() }
    }),
}
```

`biased;` plus an active unwind of the parent's wait queue. Cancellation does not just drop the future; it converts to a synthetic decline that every waiter can observe — which is what prevents the "ghost approval" bug.

Reference: `codex-rs/core/src/codex_delegate.rs:779`.

---
title: Bound submissions but leave events unbounded
impact: HIGH
impactDescription: avoids session loop stalls on slow consumers while rate-limiting misbehaving producers
tags: async, channels, backpressure, async-channel
---

## Bound submissions but leave events unbounded

Defaulting every channel to `mpsc::channel(1024)` looks safe, but it creates two opposite bugs: an internal session loop stalls when a UI pauses (because its event channel is full), or an input queue OOMs when a client floods. Codex deliberately splits the two halves of its submission-event pair — user-facing submissions are *bounded* (clients backpressure when they submit too fast, which rate-limits them — a feature), and outbound events are *unbounded* (the event producer is the session loop itself, which must never block on a slow UI consumer or the whole agent stalls).

**Incorrect (uniform bounded channels for both directions):**

```rust
let (submission_tx, submission_rx) = mpsc::channel(1024);
let (event_tx, event_rx) = mpsc::channel(1024);
// Session loop blocks when event_tx fills, even during critical lock sections.
```

**Correct (split: bounded submissions, unbounded events):**

```rust
// core/src/session/mod.rs
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
let (tx_event, rx_event) = async_channel::unbounded();
```

The decision rule is "who is the producer, and can they afford to wait?". External caller? Bounded, backpressure is a feature. Internal task holding a critical lock? Unbounded, blocking corrupts the session. For latency-sensitive data (audio frames) codex goes further with `try_send` plus drop-on-full: `TrySendError::Full` logs a warning and drops the frame rather than blocking.

Reference: `codex-rs/core/src/session/mod.rs:480`, `codex-rs/core/src/realtime_conversation.rs:418`.

---
title: Give spawned sub-tasks child tokens, not parent clones
impact: HIGH
impactDescription: prevents cancelling one child from cascading into all siblings
tags: async, cancellation, structured-concurrency, tokio-util
---

## Give spawned sub-tasks child tokens, not parent clones

`CancellationToken::clone()` and `CancellationToken::child_token()` look similar but have opposite semantics. Cloning gives you the *same* token — cancelling any clone cancels every other clone, including the parent. `child_token()` creates a derived token that inherits cancellation from its parent but can be cancelled independently. Codex uses `child_token()` religiously so a failed leg can be torn down without nuking its siblings, and a top-level cancel still cascades through the whole tree.

**Incorrect (clone cascades an unrelated failure):**

```rust
let events_cancel = cancel_token.clone();
let ops_cancel = cancel_token.clone();
tokio::spawn(async move { forward_events(rx, events_cancel).await });
tokio::spawn(async move { forward_ops(tx, ops_cancel).await });
// Cancelling events_cancel ALSO cancels ops — not what you want.
```

**Correct (child tokens scope independently):**

```rust
// core/src/codex_delegate.rs
let cancel_token_events = cancel_token.child_token();
let cancel_token_ops = cancel_token.child_token();
tokio::spawn(async move {
    forward_events(rx, cancel_token_events).await;
});
tokio::spawn(async move {
    forward_ops(tx, cancel_token_ops).await;
});

// core/src/tasks/mod.rs — body derives ITS OWN child, so outer
// code can't accidentally observe the local token
let task_cancellation_token = cancellation_token.child_token();
let handle = tokio::spawn(async move {
    task_for_run
        .run(ctx, input, task_cancellation_token.child_token())
        .await;
    if !task_cancellation_token.is_cancelled() {
        sess.on_task_finished(/* ... */).await;
    }
});
```

The local `is_cancelled()` check after `run(...)` is how Codex decides whether to emit the completion event: "finished normally" and "finished via cancellation" are both "the future returned" — the token is how you distinguish them.

Reference: `codex-rs/core/src/codex_delegate.rs:119`, `codex-rs/core/src/tasks/mod.rs:388`.

---
title: Cancel cooperatively first, then abort after a grace deadline
impact: HIGH
impactDescription: prevents unbounded shutdown latency while still letting well-behaved tasks clean up
tags: async, cancellation, shutdown, tokio
---

## Cancel cooperatively first, then abort after a grace deadline

`CancellationToken::cancel()` is a request, not a kill — the task must reach an `.await` point that observes it. Plain `handle.abort()` skips cleanup entirely. Codex races the task's self-reported "done" `Notify` against a grace timeout in `select!`: signal cancellation, wait for up to `GRACEFULL_INTERRUPTION_TIMEOUT_MS`, then fall back to `handle.abort()`. Well-behaved tasks get time to flush rollouts and emit finalization events; stuck tasks are bounded.

**Incorrect (immediate abort skips cleanup):**

```rust
async fn shutdown_turn(turn: ActiveTurn) {
    for task in turn.drain_tasks() {
        task.handle.abort(); // rollouts, events, locks: all lost
    }
}
```

**Correct (cancel, wait, then abort as fallback):**

```rust
// core/src/tasks/mod.rs
async fn handle_task_abort(
    self: &Arc<Self>,
    task: RunningTask,
    reason: TurnAbortReason,
) {
    if task.cancellation_token.is_cancelled() {
        return;
    }
    task.cancellation_token.cancel();
    // Cancel sub-trackers owned by the task ...
    select! {
        _ = task.done.notified() => {}
        _ = tokio::time::sleep(
            Duration::from_millis(GRACEFULL_INTERRUPTION_TIMEOUT_MS),
        ) => {
            warn!(
                "task {sub_id} didn't complete gracefully after {}ms",
                GRACEFULL_INTERRUPTION_TIMEOUT_MS,
            );
        }
    }
    task.handle.abort(); // hard kill, no-op if task already exited
}
```

`task.done.notified()` is the task's way of volunteering that it reached its cleanup tail; `handle.abort()` is the hard kill. Both always run — `abort()` is a no-op if the task already returned.

Reference: `codex-rs/core/src/tasks/mod.rs:846`.

---
title: Wrap background JoinHandle in Shared BoxFuture for multi-waiter joins
impact: MEDIUM-HIGH
impactDescription: enables multiple independent callers to await the same background task completion
tags: async, shutdown, futures, join-handle
---

## Wrap background JoinHandle in Shared BoxFuture for multi-waiter joins

A `JoinHandle` can only be awaited once — it takes `self`. When several call sites need to know "has this background task finished?" (shutdown, parent supervisor, tests), you either hand out the handle and pray, or wrap it in `Arc<Mutex<Option<JoinHandle>>>` and the "first waiter" semantics break the second. Codex uses `futures::future::Shared<BoxFuture<'static, ()>>`: the combinator turns any future into one that can be cloned and polled from multiple places, with every clone resolving to the same result when the inner future completes.

**Incorrect (single handle, second waiter panics):**

```rust
pub struct SessionHandle {
    pub join: JoinHandle<()>, // owned; only one caller can await
}
// Caller 1: await session.join — consumed.
// Caller 2: cannot even observe completion.
```

**Correct (Shared lets every caller await the same future):**

```rust
// core/src/session/mod.rs
pub(crate) type SessionLoopTermination = Shared<BoxFuture<'static, ()>>;

pub(crate) fn session_loop_termination_from_handle(
    handle: JoinHandle<()>,
) -> SessionLoopTermination {
    async move {
        let _ = handle.await;
    }
    .boxed()
    .shared()
}

// Codex struct holds one of these; any number of callers can clone + await.
```

The closure swallows `handle.await`'s `Result` (panic detail is dropped on purpose — callers only care *when* it ends). `Shared` requires the output to be `Clone`, which `()` trivially is — that is why the function returns `()` instead of propagating the result.

Reference: `codex-rs/core/src/session/mod.rs:380`, `codex-rs/core/src/session/mod.rs:819`.

---
title: Avoid learning allowlist rules for general-purpose interpreters
impact: CRITICAL
impactDescription: prevents approval amendments from green-lighting arbitrary interpreter flags
tags: defensive, allowlist, security, command-approval
---

## Avoid learning allowlist rules for general-purpose interpreters

When a system learns approvals and offers "always allow commands with this prefix", a single careless click on `python3 -c "import os; os.system(...)"` would green-light arbitrary code execution forever. Codex keeps a `BANNED_PREFIX_SUGGESTIONS` list of interpreter prefixes that the amendment suggester refuses to propose, using exact-length-and-sequence matching rather than `starts_with`, so legitimate rules like `python3 myscript.py` remain allowable while escape hatches like `python3 -c` are blocked.

**Incorrect (learns an unbounded allowlist rule):**

```rust
// User approved "python3 -c 'print(1)'", offer to remember the prefix
fn derive_amendment_from_approval(argv: &[String]) -> Option<Rule> {
    let prefix = argv.iter().take(2).cloned().collect();
    Some(Rule::allow_prefix(prefix))  // allows EVERY `python3 -c ...` forever
}
```

**Correct (explicit interpreter denylist, exact-sequence match):**

```rust
// core/src/exec_policy.rs
static BANNED_PREFIX_SUGGESTIONS: &[&[&str]] = &[
    &["python3", "-c"], &["python", "-c"],
    &["bash", "-lc"], &["sh", "-c"], &["sh", "-lc"],
    &["pwsh", "-Command"], &["node", "-e"],
    &["perl", "-e"], &["ruby", "-e"], &["osascript"],
];

if BANNED_PREFIX_SUGGESTIONS.iter().any(|banned| {
    prefix_rule.len() == banned.len()
        && prefix_rule.iter().map(String::as_str).eq(banned.iter().copied())
}) {
    return None; // refuse to suggest a permanent allow rule
}
```

The match is exact-length and exact-sequence — `python3 script.py` is still policy-able (different length), but `python3 -c` is not. Adding a new interpreter takes one line in a central list, not a code audit across every call site.

Reference: `codex-rs/core/src/exec_policy.rs:52`, `codex-rs/core/src/exec_policy.rs:876`.

---
title: Canonicalize shell wrappers before hashing approval keys
impact: HIGH
impactDescription: avoids re-prompting users for logically identical commands and blocks cache collisions between wrapped scripts
tags: defensive, canonicalization, approval, security
---

## Canonicalize shell wrappers before hashing approval keys

A naive approval cache keyed on `argv` re-prompts every time the same command arrives with a different shell wrapper — `bash -lc` vs `/bin/bash -lc` vs a heredoc. Codex canonicalizes first: unwrap simple `sh -lc "cargo test"` wrappers into their inner argv, and for unparseable scripts replace the shell path with a sentinel (`__codex_shell_script__`) while keeping the exact script text. The sentinel never collides with a real executable, so a match means "identical script", not "close enough".

**Incorrect (caches on raw argv — re-prompts on every wrapper variation):**

```rust
fn approval_key(command: &[String]) -> String {
    command.join(" ")
}
// "bash -lc 'cargo test'" and "/bin/bash -lc 'cargo test'"
// hash to different keys even though they run the same script.
```

**Correct (unwrap simple wrappers, replace interpreter with sentinel otherwise):**

```rust
// core/src/command_canonicalization.rs
pub(crate) fn canonicalize_command_for_approval(
    command: &[String],
) -> Vec<String> {
    if let Some(parsed) = parse_shell_lc_plain_commands(command)
        && let [single_command] = parsed.as_slice()
    {
        return single_command.clone();
    }
    if let Some((_shell, script)) = extract_bash_command(command) {
        let shell_mode = command.get(1).cloned().unwrap_or_default();
        return vec![
            CANONICAL_BASH_SCRIPT_PREFIX.to_string(),
            shell_mode,
            script.to_string(),
        ];
    }
    command.to_vec()
}
```

The `&& let [single_command] = ...` guard refuses to collapse to an inner argv unless the parse produced exactly one command — a compound script cannot be mistakenly matched against an approval for one of its sub-commands. The sentinel constant `CANONICAL_BASH_SCRIPT_PREFIX` is chosen so it cannot appear as a legitimate binary name.

Reference: `codex-rs/core/src/command_canonicalization.rs:14`.

---
title: Use debug_assert with safe fallback on unreachable branches
impact: CRITICAL
impactDescription: prevents release-mode panics while keeping bugs loud in tests
tags: defensive, debug-assert, panic-discipline, graceful-degradation
---

## Use debug_assert with safe fallback on unreachable branches

`unreachable!()` and `panic!()` fire in both debug and release, so a single wrong assumption crashes production. Codex reaches for `debug_assert!(false, "…")` followed by an early `return` with a conservative fallback: loud failure in tests, graceful degradation in release. This is strictly distinct from `unreachable!()`, which is reserved for cases the type system already ruled out.

**Incorrect (panics in production when a new git subcommand is added):**

```rust
match subcommand {
    "status" | "diff" | "log" => true,
    other => panic!("unexpected git subcommand: {other}"),
}
```

**Correct (loud in debug, safe in release):**

```rust
// shell-command/src/command_safety/is_safe_command.rs
match subcommand {
    "status" | "diff" | "log" => true,
    other => {
        debug_assert!(false, "unexpected git subcommand from matcher: {other}");
        false
    }
}
```

The fallback chooses the *safer* answer — `false` for "is this command safe?" — so a missed invariant never weakens security when it matters most. Tests observe the assertion and catch the regression during development; production users see a command fall through to the approval path instead of a crash.

Reference: `codex-rs/shell-command/src/command_safety/is_safe_command.rs:192`, `codex-rs/core/src/codex_thread.rs:359`.

---
title: Deny unwrap and expect at the workspace level
impact: CRITICAL
impactDescription: prevents unreviewed panic sites across a 75-crate workspace
tags: defensive, lint, clippy, panic-discipline
---

## Deny unwrap and expect at the workspace level

Panics in production are almost always `.unwrap()` or `.expect()` calls that slipped through review. Codex inverts the default: `[workspace.lints.clippy]` sets `unwrap_used = "deny"` and `expect_used = "deny"` so panicking becomes a compile error, and `clippy.toml` relaxes the ban inside tests only. Every intentional panic site must be annotated locally, turning each exception into a grepable tombstone whose reason is spelled out next to the code.

**Incorrect (panic site slips through review):**

```rust
// Buried in a helper function — reviewers can't scan for this
fn absolute_tmp_root() -> AbsolutePathBuf {
    AbsolutePathBuf::from_absolute_path("/tmp")
        .expect("/tmp is absolute")
}
```

**Correct (workspace-wide deny, local annotated escape hatch):**

```toml
# Cargo.toml — workspace root lints
[workspace.lints.clippy]
expect_used = "deny"
unwrap_used = "deny"
```

```rust
// protocol/src/permissions.rs — escape hatch is visible and justified
FileSystemSpecialPath::SlashTmp => {
    #[allow(clippy::expect_used)]
    let slash_tmp = AbsolutePathBuf::from_absolute_path("/tmp")
        .expect("/tmp is absolute");
    /* ... */
}
```

The `#[allow]` attribute is the declaration that this `expect` is intentional, and the adjacent comment documents the invariant that makes it safe. Reviewers can `git grep expect_used` across the repo to audit every panic site in minutes.

Reference: `codex-rs/Cargo.toml:438`, `codex-rs/protocol/src/permissions.rs:1476`.

---
title: Load untrusted plugins fault-isolated and sanitize their model-facing text
impact: HIGH
impactDescription: one broken or hostile plugin can't fail startup or inject the model's prompt
tags: defensive, plugins, untrusted-input, prompt-injection
---

## Load untrusted plugins fault-isolated and sanitize their model-facing text

A plugin manifest is untrusted input, so the two reflexive choices are both wrong: `?`-propagating a load error lets one malformed plugin abort startup for everyone, and forwarding the manifest's `description` straight into the model's capability summary hands an attacker a prompt-injection channel. Codex treats each plugin as a fault domain — a failed load becomes an inert record, not a hard error — and runs every manifest string through a sanitizer before it can reach the model.

**Incorrect (one bad plugin kills startup; manifest text reaches the model raw):**

```rust
for cfg in configs {
    let plugin = load_plugin(cfg)?; // a single failure aborts the whole load
    summary.push(plugin.manifest_description.unwrap_or_default()); // unbounded, injectable
}
```

**Correct (error captured per plugin; description sanitized and capped):**

```rust
// plugin/src/load_outcome.rs
pub struct LoadedPlugin<M> {
    pub error: Option<String>, // a load failure is recorded, not propagated
    /* ... */
}
impl<M> LoadedPlugin<M> {
    pub fn is_active(&self) -> bool {
        self.enabled && self.error.is_none() // errored plugins are silently excluded
    }
}

pub fn prompt_safe_plugin_description(description: Option<&str>) -> Option<String> {
    let description = description?.split_whitespace().collect::<Vec<_>>().join(" ");
    (!description.is_empty())
        .then(|| description.chars().take(MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN).collect())
}
```

Whitespace is collapsed (defeating layout-based injection) and the result is hard-capped at 1024 chars before it ever enters a model-facing summary. Plugin id segments are separately validated to `[A-Za-z0-9_-]` so a manifest can't smuggle `../` into the on-disk cache path. The shape generalizes: untrusted extension input is isolated per-unit and sanitized at the boundary.

Reference: `codex-rs/plugin/src/load_outcome.rs:32`, `codex-rs/plugin/src/plugin_id.rs:51`.

---
title: Cap subprocess output with a head-and-tail ring buffer
impact: CRITICAL
impactDescription: prevents OOM on runaway output and preserves the most informative tail lines
tags: defensive, resource-limits, ring-buffer, subprocess
---

## Cap subprocess output with a head-and-tail ring buffer

Trailing truncation (`output[..MAX]`) is wrong twice: it OOMs before the cap because you buffer everything first, and it discards the last lines — usually the most informative, containing errors and stack traces. Codex streams output into a fixed head budget plus a ring-buffer tail, tracks `omitted_bytes` between them, and uses `saturating_*` arithmetic everywhere so oversized chunks cannot panic. The reader also keeps consuming bytes past the cap so the child process does not deadlock on a full pipe.

**Incorrect (trailing truncate loses the tail and still OOMs):**

```rust
let mut collected = Vec::new();
while let Ok(read_count) = reader.read(&mut chunk_buf).await {
    if read_count == 0 { break; }
    collected.extend_from_slice(&chunk_buf[..read_count]);
}
collected.truncate(MAX_OUTPUT_BYTES); // last lines silently dropped
```

**Correct (bounded head, ring-buffer tail, keeps draining after cap):**

```rust
// core/src/unified_exec/head_tail_buffer.rs
pub(crate) fn push_chunk(&mut self, chunk: Vec<u8>) {
    if self.max_bytes == 0 {
        self.omitted_bytes = self.omitted_bytes.saturating_add(chunk.len());
        return;
    }
    if self.head_bytes < self.head_budget {
        let remaining_head = self.head_budget.saturating_sub(self.head_bytes);
        if chunk.len() <= remaining_head {
            self.head_bytes = self.head_bytes.saturating_add(chunk.len());
            self.head.push_back(chunk);
            return;
        }
        /* split head / tail */
    }
    /* push into ring buffer tail, updating omitted_bytes */
}

// core/src/exec.rs — keep draining after cap to avoid back-pressure
fn append_capped(dst: &mut Vec<u8>, src: &[u8], max_bytes: usize) {
    if dst.len() >= max_bytes { return; }
    let remaining = max_bytes.saturating_sub(dst.len());
    let take = remaining.min(src.len());
    dst.extend_from_slice(&src[..take]);
}
```

The "keep draining after cap" rule is load-bearing: stop reading and a long-running child process deadlocks on its own full pipe, hanging the agent forever.

Reference: `codex-rs/core/src/unified_exec/head_tail_buffer.rs:65`, `codex-rs/core/src/exec.rs:856`.

---
title: Register a drain timeout to escape grandchild pipe leaks
impact: CRITICAL
impactDescription: prevents the whole agent from hanging when a killed child leaves grandchildren holding stdout
tags: defensive, timeout, subprocess, resource-leak
---

## Register a drain timeout to escape grandchild pipe leaks

Killing a timed-out child is not enough. If the child already forked grandchildren, they inherit the stdout and stderr pipes and hold them open, so the `read()` on the pipe never returns — hanging the whole agent. Codex runs the stdout collector in its own `tokio::spawn` and applies a separate `IO_DRAIN_TIMEOUT_MS` to joining that task, aborting the drain if it exceeds the deadline and returning an empty `StreamOutput`. The in-file comment explicitly pins the reasoning in place so no one removes the timeout as "redundant".

**Incorrect (single timeout on the child — hangs on inherited pipes):**

```rust
let output = tokio::time::timeout(
    child_deadline,
    child.wait_with_output(),
).await?;
// If the child is killed mid-run, grandchildren still hold stdout.
// wait_with_output() never returns — agent hangs forever.
```

**Correct (separate drain timeout with grepable justification):**

```rust
// core/src/exec.rs:73 — comment pins the invariant
// If the child process spawned grandchildren that inherited its
// stdout/stderr file descriptors those pipes may stay open after we
// `kill` the direct child on timeout. That would cause the `read_capped`
// tasks to block on `read()` indefinitely, effectively hanging the whole
// agent.
pub const IO_DRAIN_TIMEOUT_MS: u64 = 2_000;

async fn await_output(
    handle: &mut JoinHandle<io::Result<StreamOutput<Vec<u8>>>>,
    drain_timeout: Duration,
) -> io::Result<StreamOutput<Vec<u8>>> {
    match tokio::time::timeout(drain_timeout, &mut *handle).await {
        Ok(join_res) => join_res?,
        Err(_elapsed) => {
            handle.abort();
            Ok(StreamOutput { text: Vec::new(), truncated_after_lines: None })
        }
    }
}
```

The killing order matters too: `kill_child_process_group(&mut child)` runs before `child.start_kill()` so the grandchildren get a SIGKILL through the process group, and the drain timeout is the belt-and-braces safety net if that fails.

Reference: `codex-rs/core/src/exec.rs:81`, `codex-rs/core/src/exec.rs:1391`.

---
title: Recover a poisoned lock with into_inner instead of unwrapping it
impact: CRITICAL
impactDescription: stops one thread's panic from cascading a poisoned lock into every other holder
tags: defensive, mutex, poison, panic-discipline
---

## Recover a poisoned lock with into_inner instead of unwrapping it

`mutex.lock().unwrap()` is the idiomatic-looking default, and in a long-lived multi-task agent it is a latent outage: if any thread panics while holding the lock, the `Mutex` becomes *poisoned*, and from then on every other `.lock().unwrap()` panics too. A single recoverable failure in one task thereby cascades into a process-wide crash. On its long-lived paths codex recovers the guarded data from the poison error instead of unwrapping — consistent with the workspace setting `unwrap_used = "deny"` (see [[defensive-deny-unwrap-workspace-wide]]).

**Incorrect (poison turns one panic into a chain reaction):**

```rust
let mut emitted = self.app_used_emitted_keys.lock().unwrap(); // panics forever once poisoned
```

**Correct (recover the data and keep serving):**

```rust
// analytics/src/client.rs — recover through the PoisonError
let mut emitted = self
    .app_used_emitted_keys
    .lock()
    .unwrap_or_else(std::sync::PoisonError::into_inner);

// the match form is equivalent and used where `?`-style reads better:
let guard = match self.cache.lock() {
    Ok(g) => g,
    Err(poisoned) => poisoned.into_inner(),
};
```

`PoisonError::into_inner` hands back the same `MutexGuard`, so the surviving threads keep working instead of inheriting an unrelated task's panic. Recover like this when the panicking section didn't leave the guarded value half-updated; if a broken invariant is possible, reset the state explicitly rather than blindly trusting it. The idiom recurs at ~40 call sites across ~17 crates precisely because process longevity depends on it.

Reference: `codex-rs/analytics/src/client.rs:95`, `codex-rs/keyring-store/src/lib.rs:128`.

---
title: Refuse to run when the sandbox cannot enforce the policy
impact: CRITICAL
impactDescription: prevents silent privilege erosion when the sandbox backend lacks the required primitive
tags: defensive, security, sandbox, fail-closed
---

## Refuse to run when the sandbox cannot enforce the policy

Most software follows graceful degradation: if a feature is unsupported, do the best you can. For security boundaries, Codex does the opposite — it returns an error rather than running the command with weaker confinement. Every refusal message repeats the phrase `"refusing to run unsandboxed"` so the string is grepable across the codebase and obviously load-bearing to anyone tempted to soften it into an `Ok(None)` to make a test pass.

**Incorrect (silent privilege erosion):**

```rust
fn prepare_windows_sandbox(roots: &[WritableRoot]) -> Result<SandboxArgs> {
    if windows_cannot_enforce_split_roots(roots) {
        // Couldn't enforce — best effort, run without this restriction
        tracing::warn!("split roots not enforceable, running anyway");
        return Ok(SandboxArgs::default());
    }
    /* ... */
}
```

**Correct (fail closed with a grepable refusal string):**

```rust
// core/src/exec.rs
let Some(legacy_root) = legacy_writable_roots.iter().find(|candidate| {
    normalize_windows_override_path(candidate.root.as_path())
        .is_ok_and(|candidate_path| candidate_path == split_root_path)
}) else {
    return Err(
        "windows unelevated restricted-token sandbox cannot enforce split \
         writable root sets directly; refusing to run unsandboxed"
            .to_string(),
    );
};
```

`refusing to run unsandboxed` appears verbatim in every refusal message across the sandbox backends, turning it into an audit grep. When ops debugs a blocked command, the error says exactly which primitive the backend lacks — not "permission denied" or "sandbox failed" which look like transient errors worth retrying.

Reference: `codex-rs/core/src/exec.rs:1053`, `codex-rs/core/src/exec.rs:1094`.

---
title: Translate errors at the layer boundary in one function
impact: CRITICAL
impactDescription: eliminates reqwest error status inspection scattered across business logic
tags: errors, boundary, thiserror, layering
---

## Translate errors at the layer boundary in one function

When layers are transport → api → core → protocol, letting `?` bubble a `reqwest::Error` straight up to the retry loop forces every caller to re-parse HTTP status codes, headers, and JSON bodies — inconsistently. Codex centralizes the translation in one `map_api_error` function per boundary. This function is the only place that parses error-body JSON, pulls `cf-ray` and `x-request-id` headers, and invents protocol-level semantic variants like `ServerOverloaded` and `UsageLimitReached`.

**Incorrect (HTTP inspection scattered across business logic):**

```rust
// In three different files:
let resp = client.post(url).send().await?;
if resp.status() == StatusCode::SERVICE_UNAVAILABLE {
    return Err(CodexErr::ServerOverloaded);
}
// Meanwhile in another caller: forgets the overloaded check entirely.
```

**Correct (one function owns the translation):**

```rust
// codex-api/src/api_bridge.rs
pub fn map_api_error(err: ApiError) -> CodexErr {
    match err {
        ApiError::ContextWindowExceeded => CodexErr::ContextWindowExceeded,
        ApiError::QuotaExceeded => CodexErr::QuotaExceeded,
        ApiError::Retryable { message, delay } => CodexErr::Stream(message, delay),
        ApiError::Transport(transport) => match transport {
            TransportError::Http { status, body, .. } => {
                let body_text = body.unwrap_or_default();
                if status == http::StatusCode::SERVICE_UNAVAILABLE
                    && let Ok(value) = serde_json::from_str::<serde_json::Value>(&body_text)
                    && matches!(
                        value.get("error").and_then(|e| e.get("code"))
                            .and_then(serde_json::Value::as_str),
                        Some("server_is_overloaded" | "slow_down")
                    )
                {
                    return CodexErr::ServerOverloaded;
                }
                /* other status-specific conversions */
                CodexErr::UnexpectedStatus(status)
            }
            /* transport-level errors */
        },
    }
}
```

The layer below returns a flat `TransportError::Http { status, headers, body }` and knows nothing about product semantics. The layer above never talks HTTP. Refactoring the reqwest client is now local — nothing above the boundary cares.

Reference: `codex-rs/codex-api/src/api_bridge.rs:18`.

---
title: Carry the server-requested retry delay inside the error variant
impact: HIGH
impactDescription: eliminates out-of-band retry-after plumbing through the error flow
tags: errors, retry, backoff, thiserror
---

## Carry the server-requested retry delay inside the error variant

When the server sends `Retry-After` headers or encodes per-error delays, the naive approach is to thread `retry_after: Option<Duration>` alongside the error as a second return value, or stash it on the session struct. Codex puts the delay *inside* the error variant itself, so the retry loop pattern-matches to pick between "server said wait 2s" and "default exponential backoff". No extra argument plumbing, no side-channel state.

**Incorrect (side-channel delay, prone to drift):**

```rust
fn send_turn(&self) -> Result<Turn, (CodexErr, Option<Duration>)> { /* ... */ }

match self.send_turn() {
    Err((err, Some(d))) => tokio::time::sleep(d).await,
    Err((err, None)) => tokio::time::sleep(default_backoff()).await,
    Ok(turn) => return Ok(turn),
}
```

**Correct (delay lives inside the variant):**

```rust
// protocol/src/error.rs
#[derive(Debug, thiserror::Error)]
pub enum CodexErr {
    /// Optionally includes the requested delay before retrying the turn.
    #[error("stream disconnected before completion: {0}")]
    Stream(String, Option<Duration>),
    /* other variants */
}

// core/src/session/turn.rs — retry loop reads the hint directly
let delay = match &err {
    CodexErr::Stream(_, requested_delay) => {
        requested_delay.unwrap_or_else(|| backoff(retries))
    }
    _ => backoff(retries),
};
tokio::time::sleep(delay).await;
```

The two-tuple variant `Stream(String, Option<Duration>)` is unusual — most thiserror users would define a struct variant. The positional form makes the "this carries a delay hint" fact visible at every construction site.

Reference: `codex-rs/protocol/src/error.rs:79`, `codex-rs/core/src/session/turn.rs:993`.

---
title: Classify retryable errors via an exhaustive match
impact: CRITICAL
impactDescription: prevents silent retry drift when a new error variant is added
tags: errors, retry, thiserror, exhaustive-match
---

## Classify retryable errors via an exhaustive match

A retry loop that uses `matches!(err, ErrorA | ErrorB)` or string-matching on error messages silently breaks every time a new variant is added — retryability is decided by whoever last touched the match site, not by the author of the new error. Codex defines `is_retryable(&self) -> bool` as a single `match self` listing every variant in both arms. The enum is deliberately NOT `#[non_exhaustive]`, so adding a variant forces a compile error in this function until the author classifies it.

**Incorrect (positive list with wildcard — new variants silently fall through):**

```rust
impl CodexErr {
    pub fn is_retryable(&self) -> bool {
        matches!(
            self,
            CodexErr::Stream(..) | CodexErr::Timeout | CodexErr::Io(_)
        )
        // A new CodexErr::BrokenPipe returns false by default.
    }
}
```

**Correct (exhaustive match, no wildcard arm):**

```rust
// protocol/src/error.rs
pub fn is_retryable(&self) -> bool {
    match self {
        CodexErr::TurnAborted
        | CodexErr::Interrupted
        | CodexErr::EnvVar(_)
        | CodexErr::Fatal(_) => false,
        CodexErr::Stream(..)
        | CodexErr::Timeout
        | CodexErr::UnexpectedStatus(_)
        | CodexErr::ResponseStreamFailed(_)
        | CodexErr::ConnectionFailed(_)
        | CodexErr::Io(_)
        | CodexErr::Json(_)
        | CodexErr::TokioJoin(_) => true,
        #[cfg(target_os = "linux")]
        CodexErr::LandlockRuleset(_)
        | CodexErr::LandlockPathFd(_) => false,
    }
}
```

Both arms list every variant. The companion retry loop becomes a one-liner: `if !err.is_retryable() { return Err(err); }`. Impact is compile-time — a PR that adds `BrokenPipe` cannot merge until the author picks which bucket it belongs to.

Reference: `codex-rs/protocol/src/error.rs:173`, `codex-rs/core/src/session/turn.rs:968`.

---
title: Wrap io::Error in a struct with a context field
impact: MEDIUM-HIGH
impactDescription: enables PartialEq tests against IoError without dragging anyhow into a library crate
tags: errors, io, thiserror, library
---

## Wrap io::Error in a struct with a context field

`anyhow::Context` gives you the "operation context plus underlying cause" shape, but it forces `anyhow` on every downstream consumer and makes `PartialEq` test assertions impossible — which matters when you are testing the shape of an error enum, not just the message. Codex defines a named struct with a `context: String` field and a `#[source] source: std::io::Error` field, derives `thiserror::Error` with `#[error("{context}: {source}")]`, and adds a blanket `From<io::Error>` that supplies a default context for bare `?` propagation.

**Incorrect (anyhow leaks into a library crate):**

```rust
// apply-patch/src/lib.rs
pub fn parse_patch(path: &Path) -> anyhow::Result<Patch> {
    let data = std::fs::read_to_string(path)
        .with_context(|| format!("Failed to read {}", path.display()))?;
    /* ... */
}
// Consumers must take an anyhow dependency; no PartialEq on anyhow::Error.
```

**Correct (named struct with context, no anyhow):**

```rust
// apply-patch/src/lib.rs
#[derive(Debug, thiserror::Error)]
#[error("{context}: {source}")]
pub struct IoError {
    context: String,
    #[source]
    source: std::io::Error,
}

impl PartialEq for IoError {
    fn eq(&self, other: &Self) -> bool {
        self.context == other.context
            && self.source.to_string() == other.source.to_string()
    }
}

impl From<std::io::Error> for ApplyPatchError {
    fn from(err: std::io::Error) -> Self {
        ApplyPatchError::IoError(IoError {
            context: "I/O error".to_string(),
            source: err,
        })
    }
}

// Callers upgrade the context where they know better:
return Err(ApplyPatchError::IoError(IoError {
    context: format!("Failed to read {}", path.display()),
    source: e,
}));
```

`#[source]` (not `#[from]`) on the field is deliberate — the derivation intentionally does not auto-wrap raw `io::Error` into `IoError`; that is what the hand-written `From` is for, so the default context is visible at the boundary. The custom `PartialEq` uses `source.to_string()` because `io::Error` does not implement `PartialEq`.

Reference: `codex-rs/apply-patch/src/lib.rs:82`, `codex-rs/apply-patch/src/invocation.rs:192`.

---
title: Store display-relevant error state in a struct, not a string
impact: HIGH
impactDescription: enables plan-specific tests to assert against error state instead of fragile English sentences
tags: errors, display, ui, testing
---

## Store display-relevant error state in a struct, not a string

When an error needs rich user-facing rendering (plan-specific wording, retry timestamps, request ids), the temptation is to format the final message at construction time: `CodexErr::UsageLimit(format!("You've hit..."))`. Every test that wants to check a plan-specific code path then does substring matching on a fragile English sentence. Codex keeps the raw inputs in a struct, hand-writes `impl Display`, and embeds the struct in the error enum — so tests assert against structured state and the UI still gets a clean error string.

**Incorrect (format at construction, lose the state):**

```rust
fn usage_limit_error(plan: PlanType, reset: DateTime<Utc>) -> CodexErr {
    let msg = format!(
        "You've reached your {} plan limit. Resets at {}.",
        plan.name(),
        reset.format("%H:%M"),
    );
    CodexErr::UsageLimitReached(msg)
}
// Test: assert!(err.to_string().contains("Pro plan")); — breaks on wording change
```

**Correct (structured payload, Display renders lazily):**

```rust
// protocol/src/error.rs
#[derive(Debug)]
pub struct UsageLimitReachedError {
    pub plan_type: Option<PlanType>,
    pub resets_at: Option<DateTime<Utc>>,
    pub rate_limits: Option<Box<RateLimitSnapshot>>,
    pub promo_message: Option<String>,
}

impl std::fmt::Display for UsageLimitReachedError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        let message = match self.plan_type.as_ref() {
            Some(PlanType::Known(KnownPlan::Plus)) => format!(/* ... */),
            /* other plans */
            _ => "Usage limit reached".to_string(),
        };
        write!(f, "{message}")
    }
}

#[derive(Debug, thiserror::Error)]
pub enum CodexErr {
    #[error("{0}")]
    UsageLimitReached(UsageLimitReachedError),
    /* ... */
}
```

`rate_limits` is boxed to keep the enum variant small enough to pass the workspace's `large-error-threshold = 256` clippy lint. Tests assert against `err.plan_type` and `err.resets_at` directly; the UI still gets `err.to_string()`.

Reference: `codex-rs/protocol/src/error.rs:450`.

---
title: Split tool errors into respond-to-model and fatal variants
impact: HIGH
impactDescription: eliminates downcasting every failure to decide whether the LLM can recover
tags: errors, agent, tool-dispatch, thiserror
---

## Split tool errors into respond-to-model and fatal variants

In an agent loop, some tool failures should be surfaced back to the LLM as function-call output so the model can react ("file not found — try another path"), while others should abort the whole turn (auth expired, disk full). Codex defines `FunctionCallError` with variant names that encode *where the error flows*, not what went wrong. Every tool handler in the crate returns `Result<T, FunctionCallError>`. The upper layer has one match that converts `RespondToModel` into a conversation item and `Fatal` into a terminal `CodexErr::Fatal`.

**Incorrect (anyhow::Error forces downstream downcasting):**

```rust
fn handle_apply_patch(argv: &[String]) -> anyhow::Result<String> {
    if /* missing file */ {
        anyhow::bail!("patch rejected: file not found");
    }
    /* ... */
}
// Upper layer: try to downcast or string-match to decide what to do.
```

**Correct (two variants encoded at the construction site):**

```rust
// tools/src/function_call_error.rs
#[derive(Debug, thiserror::Error, PartialEq)]
pub enum FunctionCallError {
    #[error("{0}")]
    RespondToModel(String),
    #[error("LocalShellCall without call_id or id")]
    MissingLocalShellCallId,
    #[error("Fatal error: {0}")]
    Fatal(String),
}

// core/src/stream_events_utils.rs — upper layer matches once
Err(FunctionCallError::RespondToModel(message)) => {
    let response = ResponseInputItem::FunctionCallOutput {
        call_id: String::new(),
        output: FunctionCallOutputPayload {
            body: FunctionCallOutputBody::Text(message),
            ..Default::default()
        },
    };
    output.needs_follow_up = true;
}
Err(FunctionCallError::Fatal(message)) => {
    return Err(CodexErr::Fatal(message));
}
```

Handlers convert every downstream error at the construction site — `apply_patch` turns a patch rejection into `RespondToModel(...)` while authentication failures become `Fatal`. There is no `#[from]` conversion: the crate author wants callers to consciously choose.

Reference: `codex-rs/tools/src/function_call_error.rs:5`, `codex-rs/core/src/stream_events_utils.rs:425`.

---
title: Encode transient vs permanent failures as two enum variants
impact: CRITICAL
impactDescription: prevents boolean retry-policy checks that drift out of sync with the error source
tags: errors, thiserror, retry, auth
---

## Encode transient vs permanent failures as two enum variants

When a failure can be either "log in again" (fatal) or "try in 2s" (retryable), a single error type plus a `fn is_retryable(&self) -> bool` is a refactor hazard — the decision is recomputed at every call site. Codex defines two variants whose names encode the retry policy, not the cause, and implements `From` conversions that explicitly map each variant to the right downstream error kind. The decision is made once, at the lowest level that has the information, and never recomputed.

**Incorrect (one variant, boolean policy decided by callers):**

```rust
pub struct RefreshError {
    pub kind: RefreshErrorKind,
    pub message: String,
}
impl RefreshError {
    pub fn is_retryable(&self) -> bool {
        matches!(self.kind, RefreshErrorKind::Transient)
    }
}
```

**Correct (two variants, conversions decide policy once):**

```rust
// login/src/auth/manager.rs
#[derive(Debug, Error)]
pub enum RefreshTokenError {
    #[error("{0}")]
    Permanent(#[from] RefreshTokenFailedError),
    #[error(transparent)]
    Transient(#[from] std::io::Error),
}

// core/src/client.rs — caller branches once, never recomputes
Err(RefreshTokenError::Permanent(failed)) => {
    Err(CodexErr::RefreshTokenFailed(failed))
}
Err(RefreshTokenError::Transient(other)) => {
    Err(CodexErr::Io(other))
}
```

`Permanent` maps to a dedicated user-facing variant; `Transient` routes through `CodexErr::Io`, which `is_retryable()` reports as `true`. The retry loop handles it automatically — the two paths never need a shared conditional.

Reference: `codex-rs/login/src/auth/manager.rs:102`, `codex-rs/core/src/client.rs:2011`.

---
title: Declare span fields as field Empty then record when known
impact: MEDIUM
impactDescription: reduces duplicate child spans by keeping one parent span renamed at the apm
tags: otel, tracing, spans, dynamic-fields
---

## Declare span fields as field Empty then record when known

`#[instrument]` captures field values at entry, but the span's "true name" is often only known several statements later — after peeking at the next SSE event, or after dispatching a tool call by name. Spawning a new child span duplicates the parent's fields and loses start-to-first-byte timing on the original. Codex declares identifying fields as `field::Empty` up front, then calls `span.record("field_name", value)` once the fact is known. OpenTelemetry has a special field, `otel.name`, which `tracing-opentelemetry` uses to override the span name at export.

**Incorrect (spawn a new child span once the identity is known):**

```rust
let parent = trace_span!("receiving_stream");
let event = stream.next().await?;
// Lose parent's timing; duplicate fields on child
let child = trace_span!(parent: &parent, "tool_call", tool_name = ?event.tool);
```

**Correct (Empty placeholder, record when facts arrive):**

```rust
// core/src/session/turn.rs
let receiving_span = trace_span!("receiving_stream");
let handle_responses = trace_span!(
    parent: &receiving_span,
    "handle_responses",
    otel.name = field::Empty,
    tool_name = field::Empty,
    from = field::Empty,
);

// otel/src/events/session_telemetry.rs
pub fn record_responses(
    &self,
    handle_responses_span: &Span,
    event: &ResponseEvent,
) {
    handle_responses_span.record(
        "otel.name",
        SessionTelemetry::responses_type(event),
    );
    match event {
        ResponseEvent::OutputItemDone(item) => {
            handle_responses_span.record("from", "output_item_done");
            if let ResponseItem::FunctionCall { name, .. } = item {
                handle_responses_span.record("tool_name", name.as_str());
            }
        }
        /* ... */
    }
}

// core/src/tools/parallel.rs — default field + record-on-change
let dispatch_span = trace_span!(
    "dispatch_tool_call",
    otel.name = display_name.as_str(),
    tool_name = display_name.as_str(),
    call_id = call.call_id.as_str(),
    aborted = false,
);
// ... later, inside tokio::select! on cancel:
dispatch_span.record("aborted", true);
```

`field::Empty` is load-bearing — tracing-subscriber will not emit the field if `record` is never called, so empty placeholders reserve schema slots without producing null-like noise. The `aborted = false` default plus a single `record("aborted", true)` on cancel is how Codex tracks abort rates without a counter.

Reference: `codex-rs/core/src/session/turn.rs:1750`, `codex-rs/otel/src/events/session_telemetry.rs:401`.

---
title: Default instrument spans to trace level, reserve info for network calls
impact: MEDIUM
impactDescription: enables free internal instrumentation that costs zero in normal operation
tags: otel, tracing, spans, performance
---

## Default instrument spans to trace level, reserve info for network calls

Sprinkling `info_span!` or `#[instrument]` at default level on every helper drowns stderr at INFO and pays the formatting cost for every call. Codex uses `level = "trace"` for almost all internal `#[instrument]` attributes (tool dispatch, turn sampling, parallel execution). Only functions that *actually issue a network request* are tagged `level = "info"`. Since the default subscriber filter is `codex_core=info`, internal spans cost zero at runtime — the subscriber evaluates the static metadata and returns before formatting any arguments.

**Incorrect (default-level instrument drowns stderr):**

```rust
#[instrument] // defaults to INFO — fires on every tool dispatch
async fn dispatch_tool_call(
    call: ToolCall,
    turn: &TurnContext,
) -> ToolResult { /* ... */ }
// Stderr floods with "dispatch_tool_call" records in normal operation.
```

**Correct (trace default, info for network boundary, skip_all):**

```rust
// core/src/session/turn.rs — internal code path
#[instrument(
    level = "trace",
    skip_all,
    fields(
        turn_id = %turn_context.sub_id,
        model = %turn_context.model_info.slug,
        cwd = %turn_context.cwd.display(),
    ),
)]
async fn run_sampling_request(
    /* ... */
) -> CodexResult<SamplingRequestResult> { /* ... */ }

// core/src/client.rs — network boundary gets INFO
#[instrument(
    name = "model_client.websocket_connection",
    level = "info",
    skip_all,
    fields(
        provider = %self.client.state.provider.name,
        wire_api = %self.client.state.provider.wire_api,
        transport = "responses_websocket",
        api.path = "responses",
        turn.has_metadata_header = params.turn_metadata_header.is_some(),
    ),
)]
async fn websocket_connection(
    &mut self,
    params: WebsocketConnectParams<'_>,
) -> WebsocketResult { /* ... */ }

// core/src/tools/router.rs
#[instrument(level = "trace", skip_all, err)]
pub async fn build_tool_call(/* ... */) -> ToolResult { /* ... */ }
```

The `err` argument is the idiomatic shortcut for "if this function returns `Err`, record it on the span automatically" — no manual error logging. Fields use `%` (Display) not `?` (Debug) for paths and ids, because Display is bounded where Debug can explode. `turn.has_metadata_header = ... .is_some()` is a booleanization pattern — the field is always present with cardinality 2, never the raw header value.

Reference: `codex-rs/core/src/client.rs:1121`, `codex-rs/core/src/session/turn.rs:892`.

---
title: Build per-layer EnvFilter instances with boxed fmt layers
impact: MEDIUM
impactDescription: enables independently-filtered sinks without per-layer generic divergence
tags: otel, tracing-subscriber, logging, layers
---

## Build per-layer EnvFilter instances with boxed fmt layers

A single global `EnvFilter` shared across sinks forces every layer to accept the same threshold — so you cannot have INFO stderr logs while JSON-file logs capture TRACE. And swapping format conditionally between pretty and JSON forces you to duplicate the entire registry build because the two fmt layer types diverge. Codex builds one `registry()` with every layer chained via `.with(...)`, gives each layer its own `EnvFilter` via a closure, and uses `.boxed()` inside a `match` on the format enum so both arms produce the same `Layer` trait object.

**Incorrect (shared filter, duplicated registry):**

```rust
let filter = EnvFilter::from_default_env();
if json_logs {
    tracing_subscriber::registry()
        .with(fmt::layer().json().with_filter(filter))
        .init();
} else {
    // Have to rebuild the entire registry — every layer duplicated.
    tracing_subscriber::registry()
        .with(fmt::layer().with_filter(filter))
        .init();
}
```

**Correct (per-layer filters, boxed fmt to unify types):**

```rust
// tui/src/lib.rs
let env_filter = || {
    EnvFilter::try_from_default_env().unwrap_or_else(|_| {
        EnvFilter::new("codex_core=info,codex_tui=info,codex_rmcp_client=info")
    })
};

let file_layer = tracing_subscriber::fmt::layer()
    .with_writer(non_blocking)
    .with_target(true)
    .with_ansi(false)
    .with_span_events(
        tracing_subscriber::fmt::format::FmtSpan::NEW
            | tracing_subscriber::fmt::format::FmtSpan::CLOSE,
    )
    .with_filter(env_filter());

// app-server/src/lib.rs — .boxed() unifies divergent generic types
let stderr_fmt: StderrLogLayer = match log_format_from_env() {
    LogFormat::Json => tracing_subscriber::fmt::layer()
        .json()
        .with_writer(std::io::stderr)
        .with_span_events(FmtSpan::FULL)
        .with_filter(EnvFilter::from_default_env())
        .boxed(),
    LogFormat::Default => tracing_subscriber::fmt::layer()
        .with_writer(std::io::stderr)
        .with_span_events(FmtSpan::FULL)
        .with_filter(EnvFilter::from_default_env())
        .boxed(),
};

let _ = tracing_subscriber::registry()
    .with(stderr_fmt)
    .with(feedback_layer)
    .with(log_db_layer)
    .with(otel_logger_layer)
    .with(otel_tracing_layer)
    .try_init();
```

`FmtSpan::NEW | FmtSpan::CLOSE` emits one record at span entry and one at close — giving timing for every instrumented function without writing any `info!("started")` / `info!("done")` pairs. `try_init` (vs `init`) is used because tests may have already set a subscriber.

Reference: `codex-rs/tui/src/lib.rs:1202`, `codex-rs/app-server/src/lib.rs:619`.

---
title: Route PII to log-only targets and keep traces cardinality-safe
impact: MEDIUM
impactDescription: prevents PII from leaking into wider-access trace backends
tags: otel, privacy, tracing, targets
---

## Route PII to log-only targets and keep traces cardinality-safe

Traces and logs typically go to different backends with different privacy tiers — traces to a wider-access APM, logs to a restricted pipeline. Per-field redaction is fragile; Codex gates at the *target* level. Two sentinel tracing targets — `codex_otel.log_only` and `codex_otel.trace_safe` — are installed on the logger layer and trace layer via filter functions that route on `meta.target()`. Events under `log_only` (carrying `user.email`, `user.account_id`) silently vanish from the trace exporter.

**Incorrect (one target, per-field redaction after the fact):**

```rust
tracing::info!(
    user.email = metadata.account_email,
    user.account_id = metadata.account_id,
    conversation.id = %metadata.conversation_id,
    "conversation started"
);
// Downstream processor has to remember to drop account_email per span.
```

**Correct (target routing via two macros):**

```rust
// otel/src/events/shared.rs
macro_rules! log_event {
    ($self:expr, $($fields:tt)*) => {{
        tracing::event!(
            target: $crate::targets::OTEL_LOG_ONLY_TARGET,
            tracing::Level::INFO,
            $($fields)*
            event.timestamp = %$crate::events::shared::timestamp(),
            conversation.id = %$self.metadata.conversation_id,
            user.account_id = $self.metadata.account_id,
            user.email = $self.metadata.account_email,
            model = %$self.metadata.model,
        );
    }};
}
// trace_event! — same expansion, but drops account_id / email.

// otel/src/provider.rs — filter functions on each layer
pub fn log_export_filter(meta: &tracing::Metadata<'_>) -> bool {
    is_log_export_target(meta.target())
}
pub fn trace_export_filter(meta: &tracing::Metadata<'_>) -> bool {
    meta.is_span() || is_trace_safe_target(meta.target())
}
```

The `log_and_trace_event!` composite macro forces callers to explicitly classify extra fields as `log:`-only, `trace:`-only, or `common:`. Sensitive shapes go to `log:`; their cardinality-bounded counterparts (counts, booleans) go to `trace:`. Even the auth env fingerprint is a boolean, never the key itself.

Reference: `codex-rs/otel/src/events/shared.rs:4`, `codex-rs/otel/src/provider.rs:184`.

---
title: Propagate W3C traceparent via env, RPC, and HTTP headers
impact: MEDIUM
impactDescription: enables distributed tracing from CI runner through codex to backend APIs
tags: otel, distributed-tracing, propagation, correlation
---

## Propagate W3C traceparent via env, RPC, and HTTP headers

When Codex is one hop in a distributed trace — downstream of a CI system, upstream of an API server — each entry point needs to accept an incoming trace context and every outbound request needs to emit one. Codex funnels four entry points through the same `TraceContextPropagator`: `TRACEPARENT` env vars read once via `OnceLock`, typed `W3cTraceContext` fields inside JSON-RPC envelopes, `traceparent` headers on outbound HTTP, and `warn!` on invalid inbound contexts so nothing panics or fabricates a new root.

**Incorrect (home-grown request_id UUID in log lines):**

```rust
let request_id = Uuid::new_v4();
tracing::info!("request_id={request_id} starting turn");
// Correlating across services requires scraping logs.
```

**Correct (W3C traceparent in, W3C traceparent out, OnceLock env cache):**

```rust
// otel/src/trace_context.rs
pub fn traceparent_context_from_env() -> Option<Context> {
    TRACEPARENT_CONTEXT
        .get_or_init(load_traceparent_context)
        .clone()
}

fn load_traceparent_context() -> Option<Context> {
    let traceparent = env::var(TRACEPARENT_ENV_VAR).ok()?;
    let tracestate = env::var(TRACESTATE_ENV_VAR).ok();
    match context_from_trace_headers(
        Some(&traceparent),
        tracestate.as_deref(),
    ) {
        Some(context) => {
            debug!("continuing parent trace context");
            Some(context)
        }
        None => {
            warn!("TRACEPARENT is set but invalid; ignoring");
            None
        }
    }
}

pub fn span_w3c_trace_context(span: &Span) -> Option<W3cTraceContext> {
    let context = span.context();
    if !context.span().span_context().is_valid() {
        return None;
    }
    let mut headers = HashMap::new();
    TraceContextPropagator::new()
        .inject_context(&context, &mut headers);
    Some(W3cTraceContext {
        traceparent: headers.remove("traceparent"),
        tracestate: headers.remove("tracestate"),
    })
}
```

```rust
// app-server/src/app_server_tracing.rs — request > env > new root
fn attach_parent_context(
    span: &Span,
    method: &str,
    request_id: &impl std::fmt::Display,
    parent_trace: Option<&W3cTraceContext>,
) {
    if let Some(trace) = parent_trace {
        if !set_parent_from_w3c_trace_context(span, trace) {
            tracing::warn!(
                rpc_method = method,
                rpc_request_id = %request_id,
                "ignoring invalid inbound request trace carrier"
            );
        }
    } else if let Some(context) = traceparent_context_from_env() {
        set_parent_from_context(span, context);
    }
}
```

The env-var load is gated behind `OnceLock` because `TRACEPARENT` is set at process start — reading it every span creation would be wasteful. The fallback priority (request-provided > env > new root) is the inverse of what most codebases get wrong.

Reference: `codex-rs/otel/src/trace_context.rs:91`, `codex-rs/app-server/src/app_server_tracing.rs:132`.

---
title: Use double-nested Options to distinguish absent, null, and set
impact: MEDIUM-HIGH
impactDescription: eliminates invented FieldAction enums for PATCH-like update APIs
tags: proto, serde, api-design, tri-state
---

## Use double-nested Options to distinguish absent, null, and set

In a PATCH-like update API a plain `Option<T>` collapses "leave this field alone" and "explicitly clear this field" into one state. `Option<Option<T>>` recovers the third state — but only if you wire up the deserializer, because serde's default maps a JSON `null` straight to the *outer* `None`, making it indistinguishable from an omitted field. Codex routes these fields through `serde_with::rust::double_option` so `None` = omitted (leave unchanged), `Some(None)` = JSON `null` (clear), and `Some(Some(v))` = set.

**Incorrect (plain Option, or a bare `Option<Option<T>>` without the helper):**

```rust
// Both of these collapse "clear" into "unchanged":
service_tier: Option<String>,           // {"service_tier": null} == field omitted
service_tier: Option<Option<String>>,   // null still deserializes to the OUTER None
```

**Correct (double-option helper wired via deserialize_with):**

```rust
// app-server-protocol/src/protocol/v2/thread.rs
#[serde(
    default,
    deserialize_with = "crate::protocol::serde_helpers::deserialize_double_option",
    serialize_with = "crate::protocol::serde_helpers::serialize_double_option",
    skip_serializing_if = "Option::is_none"
)]
pub service_tier: Option<Option<String>>,

// app-server-protocol/src/protocol/serde_helpers.rs — one shared implementation
pub fn deserialize_double_option<'de, T, D>(d: D) -> Result<Option<Option<T>>, D::Error>
where T: Deserialize<'de>, D: Deserializer<'de> {
    serde_with::rust::double_option::deserialize(d)
}
```

`#[serde(default)]` supplies the omitted → `None` case; `deserialize_double_option` forces a present-but-`null` value to `Some(None)` instead of letting it fold back to `None`. Centralizing the helper in `serde_helpers.rs` means every mutation-shaped field (`ThreadStartParams`, `TurnOptions`, process/realtime params) shares one implementation — no per-field `FieldAction<T> { Unchanged, Clear, Set(T) }` enum with three mappings each.

Reference: `codex-rs/app-server-protocol/src/protocol/v2/thread.rs:100`, `codex-rs/app-server-protocol/src/protocol/serde_helpers.rs:16`.

---
title: Gate experimental fields by runtime presence, not capability flags
impact: MEDIUM-HIGH
impactDescription: enables adding unstable fields to stable methods without duplicating the request type
tags: proto, experimental, macros, versioning
---

## Gate experimental fields by runtime presence, not capability flags

Adding an unstable field to a stable method normally forces you to either duplicate the whole request type (`StableThreadStartParams` vs `ExperimentalThreadStartParams`) or make every caller opt into an "experimental" capability just to call the stable part. Codex has a `#[derive(ExperimentalApi)]` proc-macro plus `#[experimental("method.fieldName")]` attributes. The generated impl walks the struct at runtime and returns a reason string *only if that field is actually present with a non-default value* — empty Vec, false, or None all count as "not using the experimental feature".

**Incorrect (duplicate request types for every experimental field):**

```rust
// Two parallel types, each adds bloat for every stable field:
pub struct StableThreadStartParams { /* 20 fields */ }
pub struct ExperimentalThreadStartParams {
    /* 20 fields + experimental_dynamic_tools: Vec<Tool> */
}
```

**Correct (runtime presence check via derive macro):**

```rust
// codex-experimental-api-macros/src/lib.rs
fn presence_expr_for_access(
    access: proc_macro2::TokenStream,
    ty: &Type,
) -> proc_macro2::TokenStream {
    if let Some(inner) = option_inner(ty) {
        let inner_expr = presence_expr_for_ref(quote!(value), inner);
        return quote! {
            #access.as_ref().is_some_and(|value| #inner_expr)
        };
    }
    if is_vec_like(ty) || is_map_like(ty) {
        return quote! { !#access.is_empty() };
    }
    if is_bool(ty) {
        return quote! { #access };
    }
    quote! { true }
}

// app-server-protocol/src/experimental_api.rs
impl<T: ExperimentalApi> ExperimentalApi for Option<T> {
    fn experimental_reason(&self) -> Option<&'static str> {
        self.as_ref()
            .and_then(ExperimentalApi::experimental_reason)
    }
}
```

Reason strings follow a reverse-DNS-ish scheme (`thread/start.dynamicTools`, `askForApproval.granular`) that maps 1:1 to the wire method and field name. The dispatcher calls `experimental_reason()` after parsing; if the client did not negotiate `experimentalApi: true` during `initialize` and a reason is returned, the method is rejected.

Reference: `codex-rs/codex-experimental-api-macros/src/lib.rs:260`.

---
title: Split internal error enums from wire error enums
impact: MEDIUM-HIGH
impactDescription: enables internal error refactors without breaking the stable wire contract
tags: proto, errors, api-design, versioning
---

## Split internal error enums from wire error enums

A single `pub enum ProtocolError` that doubles as both internal type and wire type freezes refactoring — every internal change risks breaking year-old clients. Codex keeps them separate: `CodexErr` is the internal `thiserror` enum with 30+ variants, `From` conversions from `io::Error` and `serde_json::Error`, and `.downcast_ref()` helpers. `CodexErrorInfo` is the wire type — ~15 variants, every "connection failed" shape carries `http_status_code: Option<u16>`. A translator `to_codex_protocol_error()` maps one to the other and picks up the HTTP status from whichever variant carries it.

**Incorrect (single enum doubles as internal + wire):**

```rust
#[derive(Serialize, Deserialize, thiserror::Error)]
pub enum ProtocolError {
    // Refactoring internal shape breaks wire clients.
    Io(#[from] std::io::Error), // serde panics on this boundary anyway
    Stream(String),
}
```

**Correct (internal enum + wire enum + translator):**

```rust
// protocol/src/error.rs — internal, rich
impl CodexErr {
    pub fn to_codex_protocol_error(&self) -> CodexErrorInfo {
        match self {
            CodexErr::ContextWindowExceeded => {
                CodexErrorInfo::ContextWindowExceeded
            }
            CodexErr::UsageLimitReached(_)
            | CodexErr::QuotaExceeded
            | CodexErr::UsageNotIncluded => {
                CodexErrorInfo::UsageLimitExceeded
            }
            CodexErr::ServerOverloaded => CodexErrorInfo::ServerOverloaded,
            CodexErr::RetryLimit(_) => {
                CodexErrorInfo::ResponseTooManyFailedAttempts {
                    http_status_code: self.http_status_code_value(),
                }
            }
            CodexErr::ConnectionFailed(_) => {
                CodexErrorInfo::HttpConnectionFailed {
                    http_status_code: self.http_status_code_value(),
                }
            }
            /* ... */
        }
    }
}

// app-server-protocol/src/protocol/v2/shared.rs — wire, frozen shape
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
pub enum CodexErrorInfo {
    ContextWindowExceeded,
    UsageLimitExceeded,
    HttpConnectionFailed {
        #[serde(rename = "httpStatusCode")]
        http_status_code: Option<u16>,
    },
    /* ~15 variants, every one additive */
}
```

You can refactor `CodexErr` freely (add fields, reshape tuples, swap underlying libraries) and only the translator cares — the wire protocol stays frozen. The two types never share a derive chain; there is no `From` conversion between them, only the explicit `to_codex_protocol_error()` method.

Reference: `codex-rs/protocol/src/error.rs:220`, `codex-rs/app-server-protocol/src/protocol/v2/shared.rs:71`.

---
title: Dispatch JSON-RPC via an internally tagged enum with a macro
impact: MEDIUM-HIGH
impactDescription: eliminates hand-rolled method dispatch that drifts from typed param validation
tags: proto, serde, jsonrpc, macros
---

## Dispatch JSON-RPC via an internally tagged enum with a macro

Hand-writing a dispatcher that matches `request["method"]` and then runs `serde_json::from_value::<FooParams>(request["params"])` drifts silently every time a method is added — the match, the param type, and the response type each live in a different file. Codex defines a macro (`client_request_definitions!`) that generates a `#[serde(tag = "method", rename_all = "camelCase")]` enum where each variant carries `request_id` and `params` as struct fields. Internally tagging lines up with JSON-RPC's wire format — one `serde_json::from_value` call validates method and parses typed params in a single pass.

**Incorrect (hand-rolled dispatch drifts from typed params):**

```rust
let method = request["method"].as_str().unwrap();
match method {
    "initialize" => {
        let params: InitializeParams =
            serde_json::from_value(request["params"].clone())?;
        handler.initialize(params).await
    }
    "threadStart" => {
        let params: ThreadStartParams =
            serde_json::from_value(request["params"].clone())?;
        handler.thread_start(params).await
    }
    // Missed any? Silent ignore. Added a method? Edit three places.
}
```

**Correct (internally tagged enum, one from_value call):**

```rust
// app-server-protocol/src/protocol/common.rs — macro expansion
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(tag = "method", rename_all = "camelCase")]
pub enum ClientRequest {
    Initialize {
        #[serde(rename = "id")]
        request_id: RequestId,
        params: InitializeParams,
    },
    ThreadStart {
        #[serde(rename = "id")]
        request_id: RequestId,
        params: ThreadStartParams,
    },
    /* 30+ more — each generated by the macro */
}

impl TryFrom<JSONRPCRequest> for ServerRequest {
    type Error = serde_json::Error;
    fn try_from(value: JSONRPCRequest) -> Result<Self, Self::Error> {
        serde_json::from_value(serde_json::to_value(value)?)
    }
}
```

The macro invocation pairs each variant with both a params *and* response type, so you cannot add a method without declaring both sides of the conversation. The `TryFrom<JSONRPCRequest>` becomes a one-liner because all the work is in the serde tag attribute.

Reference: `codex-rs/app-server-protocol/src/protocol/common.rs:161`.

---
title: Keep removed feature flags as parseable no-op tombstones
impact: MEDIUM-HIGH
impactDescription: lets old and new configs round-trip across versions without parse failures
tags: proto, features, forward-compat, config
---

## Keep removed feature flags as parseable no-op tombstones

The instinct when retiring a feature is to delete its enum variant and config key. But now an older config file that still sets the key fails to parse on the new binary, and a config written by the new binary may surprise an older one — the flag becomes a hard compatibility break in both directions. Codex models a flag's whole lifecycle in a `Stage` enum and keeps removed flags as inert, still-parseable entries; the value is ignored but the key never errors.

**Incorrect (deleting the variant breaks existing configs):**

```rust
pub enum Feature {
    ShellTool,
    // WebSearch removed — now {"web_search": true} in any saved config fails to parse
}
```

**Correct (Stage::Removed tombstone, ignored not rejected):**

```rust
// features/src/lib.rs
pub enum Stage {
    UnderDevelopment,
    Experimental { name: &'static str, menu_description: &'static str, announcement: &'static str },
    Stable,
    Deprecated,
    /// The feature flag is useless but kept for backward compatibility.
    Removed,
}

// apply_map: a Removed key is consumed and skipped, never an error;
// a genuinely unknown key is warn!-logged, not fatal.
```

A `Removed` flag is excluded from the experimental menu and from metrics, but it still parses, so configs survive across versions in both directions. The same registry distinguishes `Removed` (kept for compat) from `Deprecated` (still works, discouraged) — two different promises to existing users. This is the config-evolution dual of [[types-unknown-variant-forward-compat]].

Reference: `codex-rs/features/src/lib.rs:44`, `codex-rs/features/src/lib.rs:413`.

---
title: Pair rename and alias to migrate wire names without breaking clients
impact: MEDIUM-HIGH
impactDescription: prevents flag-day migrations by keeping old wire names as read-only aliases
tags: proto, serde, versioning, backcompat
---

## Pair rename and alias to migrate wire names without breaking clients

Renaming a variant or field on the wire normally means a flag day — ship the new name and every old client breaks. Codex renames wire strings in place but keeps the old string alive as a read-only alias. `#[serde(rename)]` controls what goes *out*; `#[serde(alias)]` controls what can come *in*. New code writes `task_started`; an old client that still sends `turn_started` parses fine. Combined with `#[non_exhaustive]` on the enum, external crates also cannot write exhaustive matches that would block the migration.

**Incorrect (rename only — every old client breaks):**

```rust
#[serde(rename = "task_started")]
TurnStarted(TurnStartedEvent),
// v1 client sending "turn_started" -> serde error, ignored or crashes.
```

**Correct (rename + alias, documented with a v1/v2 note):**

```rust
// protocol/src/protocol.rs
/// Agent has started a turn.
/// v1 wire format uses `task_started`; accept `turn_started` for v2 interop.
#[serde(rename = "task_started", alias = "turn_started")]
TurnStarted(TurnStartedEvent),

/// Agent has completed all actions.
/// v1 wire format uses `task_complete`; accept `turn_complete` for v2 interop.
#[serde(rename = "task_complete", alias = "turn_complete")]
TurnComplete(TurnCompleteEvent),
```

The Rust identifier (`TurnStarted`) is decoupled from both wire names — renaming internally is free. A doc comment records which name is v1 and which is v2, so future grep-and-refactor passes can find the migration sites. Other files use `#[serde(default, alias = "agent_type")]` when field names (not variants) migrate the same way.

Reference: `codex-rs/protocol/src/protocol.rs:1174`.

---
title: Treat SSE streams as idle-timeout with a required terminator
impact: MEDIUM-HIGH
impactDescription: prevents long turns from being killed by wall-clock deadlines and silent half-closes
tags: proto, streaming, sse, timeouts
---

## Treat SSE streams as idle-timeout with a required terminator

A `while let Some(event) = stream.next().await` loop with a wall-clock deadline either kills legitimate long turns or never fires at all. Codex's `process_sse` loop re-arms the timeout on every `stream.next()` call — activity resets it, so long-running turns never hit a total deadline. And a clean `Ok(None)` return (stream closed) is treated as an *error* unless a `response.completed` event was observed: `"stream closed before response.completed"`.

**Incorrect (wall-clock deadline kills legit turns):**

```rust
let deadline = Instant::now() + Duration::from_secs(60);
while Instant::now() < deadline {
    match stream.next().await {
        Some(Ok(event)) => process(event),
        Some(Err(_)) | None => break,
    }
}
// A 90-second turn dies at 60s; a half-closed stream silently succeeds.
```

**Correct (per-poll idle timeout, terminator required):**

```rust
// codex-api/src/sse/responses.rs
loop {
    let response = timeout(idle_timeout, stream.next()).await;
    let sse = match response {
        Ok(Some(Ok(sse))) => sse,
        Ok(Some(Err(transport_err))) => {
            let _ = tx_event.send(Err(transport_err.into())).await;
            return;
        }
        Ok(None) => {
            let error = response_error.unwrap_or(ApiError::Stream(
                "stream closed before response.completed".into(),
            ));
            let _ = tx_event.send(Err(error)).await;
            return;
        }
        Err(_) => {
            let _ = tx_event
                .send(Err(ApiError::Stream(
                    "idle timeout waiting for SSE".into(),
                )))
                .await;
            return;
        }
    };
    /* dispatch sse event */
}
```

The missing-terminator error maps to `CodexErr::Stream`, which `is_retryable()` reports as `true` — so the session loop auto-retries half-closes instead of surfacing a mystery. The stream is bridged to the consumer via a bounded `mpsc::channel(1600)` rather than exposed as a raw `futures::Stream`, giving proper backpressure and an explicit close signal.

Reference: `codex-rs/codex-api/src/sse/responses.rs:399`, `codex-rs/protocol/src/error.rs:78`.

---
title: Multiplex helper binaries via argv[0] and symlinks
impact: MEDIUM-HIGH
impactDescription: eliminates TOCTOU risk and packaging overhead of shipping multiple binaries
tags: sandbox, deployment, argv, linux
---

## Multiplex helper binaries via argv[0] and symlinks

Shipping multiple binaries (`codex`, `codex-linux-sandbox`, `apply_patch`) is a packaging headache — and finding `codex-linux-sandbox` via `which` opens a TOCTOU between lookup and exec. Codex ships one binary. On startup it inspects `argv[0]`'s basename and dispatches into the relevant sub-entry-point, otherwise falls through to `main`. At startup the CLI creates a locked per-session temp dir under `~/.codex/tmp/arg0/`, drops symlinks for each alias pointing at `current_exe()`, and prepends that dir to `PATH`.

**Incorrect (multiple binaries, TOCTOU on lookup):**

```rust
let helper = which::which("codex-linux-sandbox")?; // race window
Command::new(helper).args(...).spawn()?;
```

**Correct (single binary, argv[0] dispatch, locked temp symlinks):**

```rust
// arg0/src/lib.rs
pub fn arg0_dispatch() -> Option<Arg0PathEntryGuard> {
    let mut args = std::env::args_os();
    let argv0 = args.next().unwrap_or_default();
    let exe_name = Path::new(&argv0)
        .file_name()
        .and_then(|s| s.to_str())
        .unwrap_or("");
    if exe_name == CODEX_LINUX_SANDBOX_ARG0 {
        codex_linux_sandbox::run_main();
    } else if exe_name == APPLY_PATCH_ARG0 {
        codex_apply_patch::main();
    }
    /* return guard for symlink temp dir */
}

// linux-sandbox/src/linux_run_main.rs — bwrap preserves argv0
if supports_argv0 {
    argv.splice(
        command_separator_index..command_separator_index,
        ["--argv0".to_string(), CODEX_LINUX_SANDBOX_ARG0.to_string()],
    );
}
```

The temp dir is `chmod 0700` and locked via `fs2::try_lock` so a janitor thread can clean stale siblings without racing live sessions. Windows, which lacks good symlinks, falls back to generated `.bat` stubs that exec the main binary.

Reference: `codex-rs/arg0/src/lib.rs:54`, `codex-rs/linux-sandbox/src/linux_run_main.rs:422`.

---
title: Mount /dev/null over the first missing path component
impact: HIGH
impactDescription: prevents mkdir-and-write escapes through non-existent protected paths
tags: sandbox, linux, bwrap, toctou
---

## Mount /dev/null over the first missing path component

A naive read-only allowlist that says "`.codex/` is read-only inside the writable workspace" has a gap: if `.codex/` does not exist at sandbox setup time, there is nothing to bind-mount over, and a child process can `mkdir .codex` and write whatever it wants. Codex walks the protected path, finds the first non-existent component, and bind-mounts `/dev/null` onto it. That turns the would-be directory into an unwritable character device, so `mkdir` fails with `ENOTDIR`.

**Incorrect (only mounts existing paths — gap on non-existent ones):**

```rust
if subpath.exists() {
    args.push("--ro-bind".to_string());
    args.push(path_to_string(subpath));
    args.push(path_to_string(subpath));
}
// Else: child can mkdir the protected name and write freely.
```

**Correct (mount /dev/null over the first missing component):**

```rust
// linux-sandbox/src/bwrap.rs
if !subpath.exists() {
    if let Some(first_missing_component) =
        find_first_non_existent_component(subpath)
        && is_within_allowed_write_paths(
            &first_missing_component,
            allowed_write_paths,
        )
    {
        args.push("--ro-bind".to_string());
        args.push("/dev/null".to_string());
        args.push(path_to_string(&first_missing_component));
    }
    return;
}

// The file-fd-mount variant for unreadable carveouts:
if preserved_files.is_empty() {
    preserved_files.push(File::open("/dev/null")?);
}
let null_fd = preserved_files[0].as_raw_fd().to_string();
args.push("--perms".to_string());
args.push("000".to_string());
args.push("--ro-bind-data".to_string());
args.push(null_fd);
args.push(path_to_string(unreadable_root));
```

The file-fd side uses `preserved_files: Vec<File>` to keep the `/dev/null` handle alive across the spawn. The equivalent Seatbelt policy blocks the same hole via `(require-not (literal ...))` alongside `(require-not (subpath ...))` because Seatbelt's `(subpath)` predicate does not cover first-time creation of the directory itself.

Reference: `codex-rs/linux-sandbox/src/bwrap.rs:1058`, `codex-rs/linux-sandbox/src/bwrap.rs:1076`.

---
title: Clear the env and tether children via pre_exec before every spawn
impact: HIGH
impactDescription: prevents LD_PRELOAD inheritance and orphaned grandchildren after a parent kill
tags: sandbox, unix, lifecycle, env-scrubbing
---

## Clear the env and tether children via pre_exec before every spawn

Inherited environments leak `LD_LIBRARY_PATH`, `DYLD_INSERT_LIBRARIES`, and ambient shell secrets into every child — and if the agent is `kill -9`'d, its children keep running compute forever. Codex's spawn path calls `cmd.env_clear()` before re-adding a whitelisted env map, and in the `pre_exec` closure does three orthogonal things: `detach_from_tty`, `PR_SET_PDEATHSIG(SIGTERM)`, and outside the closure `kill_on_drop(true)`.

**Incorrect (inherits env and leaks grandchildren):**

```rust
let mut cmd = Command::new(program);
cmd.args(args); // inherits LD_PRELOAD, LD_LIBRARY_PATH, secrets
let handle = cmd.spawn()?; // no pdeathsig — kill -9 orphans compute
```

**Correct (clear env + tether via pre_exec):**

```rust
// core/src/spawn.rs
let mut cmd = Command::new(&program);
#[cfg(unix)]
cmd.arg0(
    arg0.map_or_else(|| program.to_string_lossy().to_string(), String::from),
);
cmd.args(args);
cmd.current_dir(cwd);
cmd.env_clear();
cmd.envs(allowed_env);

#[cfg(unix)]
unsafe {
    let detach_from_tty = matches!(
        stdio_policy,
        StdioPolicy::RedirectForShellTool,
    );
    #[cfg(target_os = "linux")]
    let parent_pid = libc::getpid(); // captured BEFORE the closure
    cmd.pre_exec(move || {
        if detach_from_tty {
            codex_utils_pty::process_group::detach_from_tty()?;
        }
        #[cfg(target_os = "linux")]
        codex_utils_pty::process_group::set_parent_death_signal(parent_pid)?;
        Ok(())
    });
}
cmd.stdin(Stdio::null()); // ripgrep hangs on open empty pipe otherwise
cmd.kill_on_drop(true);
```

`parent_pid` is captured *before* the closure because inside `pre_exec` the child is already a new process — `getpid()` there would return the child's own pid. `stdin = Stdio::null()` is specifically because ripgrep has a heuristic that reads stdin when it's an open pipe, causing it to hang on an empty one.

Reference: `codex-rs/core/src/spawn.rs:75`, `codex-rs/core/src/spawn.rs:91`.

---
title: Resolve hostnames and reject private IPs before allowing egress
impact: HIGH
impactDescription: defeats DNS-rebinding bypass of a string-based egress allowlist
tags: sandbox, network, dns-rebinding, egress
---

## Resolve hostnames and reject private IPs before allowing egress

A network allowlist enforced by string-matching the hostname is trivially bypassed: an attacker registers `evil.example.com`, points its A record at `127.0.0.1` (or a metadata endpoint like `169.254.169.254`), and the literal-string check happily allows it. Codex's egress proxy treats string checks as insufficient — when local binding is disabled it does a best-effort DNS lookup with a timeout and blocks the request if **any** resolved IP is non-public, *even when the host is on the allowlist*.

**Incorrect (string allowlist, rebinding walks right through):**

```rust
// "localhost"/"127.0.0.1" literals blocked, but evil.example.com -> 127.0.0.1 is allowed
if is_allowlisted(host_str) && host_str != "localhost" {
    return Decision::Allowed;
}
```

**Correct (classify the literal, then resolve and classify the IPs):**

```rust
// network-proxy/src/runtime.rs — when local binding is off
let local_literal = if is_loopback_host(&host) {
    true
} else if let Ok(ip) = host_no_scope.parse::<IpAddr>() {
    is_non_public_ip(ip) // 127/8, 10/8, 169.254/16, ::1, link-local, ...
} else {
    false
};

if local_literal {
    if !is_explicit_local_allowlisted(&allowed_domains, &host) {
        return Ok(Blocked(NotAllowedLocal));
    }
} else if host_resolves_to_non_public_ip(host_str, port, DNS_LOOKUP_TIMEOUT, resolve).await {
    return Ok(Blocked(NotAllowedLocal)); // rebinding caught here, allowlist or not
}
```

The two-step check matters: an IP *literal* is classified directly, but a *hostname* must be resolved first, because the danger lives in what it resolves to, not how it is spelled. `is_non_public_ip` leans on stdlib classifiers (`is_loopback`, `is_private`, `is_link_local`) plus CIDR fallbacks for ranges stdlib doesn't cover yet (CGNAT, TEST-NET).

Reference: `codex-rs/network-proxy/src/runtime.rs:385`, `codex-rs/network-proxy/src/policy.rs:51`.

---
title: Keep sandbox policy as shared data, not per-platform code
impact: HIGH
impactDescription: prevents three independently-drifting notions of "workspace-write"
tags: sandbox, cross-platform, policy-as-data, architecture
---

## Keep sandbox policy as shared data, not per-platform code

Most cross-platform sandbox implementations grow three parallel `#[cfg(unix)]` / `#[cfg(windows)]` engines — each with its own struct fields, its own validation, and inevitably its own bugs. Codex keeps one platform-neutral `SandboxPolicy` (plus `FileSystemSandboxPolicy`, `NetworkSandboxPolicy`), and each OS backend compiles the shared data into its native vocabulary (Seatbelt s-expressions, bubblewrap argv, Windows restricted token). The core never mentions `landlock`, `sandbox-exec`, or `CreateRestrictedToken`.

**Incorrect (three drifting structs, no shared schema):**

```rust
#[cfg(target_os = "macos")]
struct SeatbeltPolicy { /* custom fields */ }

#[cfg(target_os = "linux")]
struct LinuxPolicy { /* different fields, same concept */ }

// Adding a new "forbid /etc" restriction requires editing BOTH.
```

**Correct (one shared model, backends render):**

```rust
// sandboxing/src/manager.rs
pub enum SandboxType {
    None,
    MacosSeatbelt,
    LinuxSeccomp,
    WindowsRestrictedToken,
}

let (argv, arg0_override) = match sandbox {
    SandboxType::None => (os_argv_to_strings(raw_argv), None),
    #[cfg(target_os = "macos")]
    SandboxType::MacosSeatbelt => {
        let args = create_seatbelt_command_args(
            os_argv_to_strings(raw_argv),
            &effective_file_system_policy,
            effective_network_policy,
            sandbox_policy_cwd,
            enforce_managed_network,
            network,
        );
        (args, Some(SEATBELT_ARG0.to_string()))
    }
    SandboxType::LinuxSeccomp => {
        let exe = codex_linux_sandbox_exe
            .ok_or(SandboxTransformError::MissingLinuxSandboxExecutable)?;
        /* render to bwrap argv */
    }
};
```

The shared `SandboxPolicy` is serializable and gets passed through argv to the Linux sandbox helper as JSON — so even sub-processes share the schema. Adding a new constraint is one edit in the shared type plus three backend render patches, not three independent rewrites.

Reference: `codex-rs/sandboxing/src/manager.rs:23`, `codex-rs/sandboxing/src/manager.rs:196`.

---
title: Stage incompatible restrictions via re-executing the same binary
impact: HIGH
impactDescription: eliminates the "seccomp breaks bwrap" conflict via two-stage application
tags: sandbox, linux, seccomp, bubblewrap
---

## Stage incompatible restrictions via re-executing the same binary

On Linux, some restrictions are mutually exclusive at setup time. Bubblewrap may need `CAP_SYS_ADMIN` to build the filesystem view, but turning on seccomp plus `PR_SET_NO_NEW_PRIVS` first would strip that capability and break bwrap. The solution: run bwrap as the outer stage, then have bwrap re-exec the same Codex binary back with a hidden `--apply-seccomp-then-exec` flag. The inner stage is already inside the namespace, applies seccomp to its own thread, then `execvp`'s the real user command.

**Incorrect (single-stage, restrictions fight each other):**

```rust
fn apply_all_restrictions() -> io::Result<()> {
    apply_seccomp()?; // sets NO_NEW_PRIVS
    bwrap_build_namespace()?; // fails — needs CAP_SYS_ADMIN
    Ok(())
}
```

**Correct (outer bwrap re-execs self to apply inner seccomp):**

```rust
// linux-sandbox/src/linux_run_main.rs
// Inner stage: apply seccomp after bubblewrap has already built
// the filesystem view.
if apply_seccomp_then_exec {
    if let Err(e) = apply_sandbox_policy_to_current_thread(
        &sandbox_policy,
        network_sandbox_policy,
        &sandbox_policy_cwd,
        /*apply_landlock_fs*/ false,
        allow_network_for_proxy,
        proxy_routing_active,
    ) {
        panic!("error applying Linux sandbox restrictions: {e:?}");
    }
    exec_or_panic(command);
}

// linux-sandbox/src/landlock.rs — conditional gate
if network_seccomp_mode.is_some()
    || (apply_landlock_fs && !sandbox_policy.has_full_disk_write_access())
{
    set_no_new_privs()?;
}
```

`apply_sandbox_policy_to_current_thread` is applied to the current *thread*, not the process — so it only affects the about-to-exec path and cannot leak into the outer process. The sandbox helper is literally the same ELF as the Codex CLI — work is selected by argv flags.

Reference: `codex-rs/linux-sandbox/src/linux_run_main.rs:178`, `codex-rs/linux-sandbox/src/landlock.rs:61`.

---
title: Stack env, syscalls, and namespace for network isolation
impact: HIGH
impactDescription: prevents network escape through any single uncooperative tool
tags: sandbox, network, seccomp, defense-in-depth
---

## Stack env, syscalls, and namespace for network isolation

Any single layer of network isolation has blind spots — env vars only work for cooperating tools, syscall filters can be bypassed via `io_uring`, and namespaces can be unshared. Codex stacks three coordinated layers: (1) env vars tell cooperating tools to refuse network (`PIP_NO_INDEX`, `NPM_CONFIG_OFFLINE`, `CARGO_NET_OFFLINE`, plus `HTTP(S)_PROXY=http://127.0.0.1:9` to break uncooperative ones); (2) a seccomp filter denies `connect`, `bind`, `listen`, `sendto`, `recvmmsg` and restricts `socket()` to `AF_UNIX`; (3) bubblewrap enters `--unshare-net`.

**Incorrect (single layer, python slips through):**

```rust
env_map.insert("HTTP_PROXY".into(), "http://127.0.0.1:9".into());
// python -c "import socket; socket.socket().connect(...)" still works.
```

**Correct (three layers stacked):**

```rust
// linux-sandbox/src/landlock.rs — syscall layer
NetworkSeccompMode::Restricted => {
    deny_syscall(&mut rules, libc::SYS_connect);
    deny_syscall(&mut rules, libc::SYS_accept);
    deny_syscall(&mut rules, libc::SYS_listen);
    deny_syscall(&mut rules, libc::SYS_sendto);
    deny_syscall(&mut rules, libc::SYS_recvmmsg);
    // recvfrom is allowed on purpose — `cargo clippy` uses socketpair
    // for child IPC and needs it.
    let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new(
        0,
        SeccompCmpArgLen::Dword,
        SeccompCmpOp::Ne,
        libc::AF_UNIX as u64,
    )?])?;
    rules.insert(libc::SYS_socket, vec![unix_only_rule]);
    // io_uring syscalls are unconditionally denied — historic seccomp bypass.
    deny_syscall(&mut rules, libc::SYS_io_uring_setup);
    deny_syscall(&mut rules, libc::SYS_io_uring_enter);
    deny_syscall(&mut rules, libc::SYS_io_uring_register);
}

// windows-sandbox-rs/src/env.rs — env layer + deny-bin PATH prefix
env_map.entry("HTTP_PROXY".into())
    .or_insert_with(|| "http://127.0.0.1:9".into());
env_map.entry("GIT_SSH_COMMAND".into())
    .or_insert_with(|| "cmd /c exit 1".into());
let base = ensure_denybin(&["ssh", "scp"], None)?;
```

The `io_uring` denial is the most surprising one — io_uring has historically been a seccomp bypass path because submissions are queued asynchronously rather than through direct syscalls. Proxy-routed mode inverts the `AF_UNIX` rule and *denies* `AF_UNIX` so a process cannot smuggle traffic through a Unix-socket bridge.

Reference: `codex-rs/linux-sandbox/src/landlock.rs:187`, `codex-rs/windows-sandbox-rs/src/env.rs:129`.

---
title: Harden a secret-handling process before main() runs, and fail closed
impact: HIGH
impactDescription: closes the core-dump / ptrace / LD_PRELOAD window before any arg parsing or allocation
tags: secrets, hardening, ctor, ptrace
---

## Harden a secret-handling process before main() runs, and fail closed

Hardening done at the top of `main()` is already too late: the runtime's constructors and the allocator have run, and an attacker who set `LD_PRELOAD` has already had their code loaded. Codex runs hardening from a `#[ctor::ctor]` function that executes *before* `main`, disabling core dumps, blocking debugger attach, and stripping dangerous environment variables. Each step that fails calls `std::process::exit` with a distinct code rather than continuing in a weakened state — hardening is fail-closed, not best-effort.

**Incorrect (hardening after the process is already exposed):**

```rust
fn main() -> anyhow::Result<()> {
    disable_core_dumps(); // constructors + allocator already ran; LD_PRELOAD already loaded
    let args = Args::parse();
    run(args)
}
```

**Correct (pre-main ctor, fail-closed, byte-level env filtering):**

```rust
// responses-api-proxy/src/main.rs
#[ctor::ctor]
fn pre_main() {
    codex_process_hardening::pre_main_hardening();
}

// process-hardening/src/lib.rs — Linux path
let ret = unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) };
if ret != 0 {
    eprintln!("ERROR: prctl(PR_SET_DUMPABLE, 0) failed: {}", std::io::Error::last_os_error());
    std::process::exit(PRCTL_FAILED_EXIT_CODE); // refuse to run un-hardened
}
set_core_file_size_limit_to_zero();             // RLIMIT_CORE = 0
remove_env_vars_with_prefix(b"LD_");            // macOS strips b"DYLD_"
```

Env keys are filtered on raw bytes (`key.as_os_str().as_bytes().starts_with(prefix)`), not UTF-8 strings, so a non-UTF-8 `LD_…` key can't slip past a lossy conversion. macOS additionally calls `ptrace(PT_DENY_ATTACH)`; each failure exits with its own code so the cause is greppable.

Reference: `codex-rs/responses-api-proxy/src/main.rs:4`, `codex-rs/process-hardening/src/lib.rs:44`, `codex-rs/process-hardening/src/lib.rs:133`.

---
title: Write a manual Debug impl that elides credentials instead of deriving it
impact: HIGH
impactDescription: stops tokens and credential providers leaking into {:?} and tracing output
tags: secrets, debug, logging, redaction
---

## Write a manual Debug impl that elides credentials instead of deriving it

`#[derive(Debug)]` is the reflexive choice, but on any struct that holds a token, credential provider, or auth header it is a latent leak: a single `tracing::debug!(?ctx)` or `{:?}` interpolation dumps the secret into logs that get shipped to a telemetry backend. Codex implements `Debug` by hand on credential-bearing types, printing only the safe fields and ending with `finish_non_exhaustive()` so the omission is visible and new fields don't silently start leaking.

**Incorrect (derive leaks the provider into every log line):**

```rust
#[derive(Clone, Debug)] // {:?} now prints the credentials provider
pub struct AwsAuthContext {
    credentials_provider: SharedCredentialsProvider,
    region: String,
    service: String,
}
```

**Correct (manual Debug, secret field omitted):**

```rust
// aws-auth/src/lib.rs
impl std::fmt::Debug for AwsAuthContext {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.debug_struct("AwsAuthContext")
            .field("region", &self.region)
            .field("service", &self.service)
            .finish_non_exhaustive() // trailing `..` marks the elided credential
    }
}
```

`finish_non_exhaustive()` is the key choice over `finish()`: it documents that fields were deliberately dropped, and because the secret field is never named, adding another secret field later can't accidentally re-expose it through this impl. The same convention recurs wherever a credential is stored (`backend-client`, `login`, `realtime-webrtc`).

Reference: `codex-rs/aws-auth/src/lib.rs:70`.

---
title: Read a secret into a zeroized stack buffer, then mlock it — never through stdin()
impact: HIGH
impactDescription: guarantees exactly one in-memory copy of an API key, locked out of swap and core dumps
tags: secrets, zeroize, mlock, api-key
---

## Read a secret into a zeroized stack buffer, then mlock it — never through stdin()

The obvious way to read an API key leaves copies everywhere: `io::stdin().read_to_string()` keeps the bytes in `Stdin`'s internal `BufReader` with no way to zero them, and every intermediate `String`/`format!` is another heap copy the allocator may later hand to unrelated code. Codex's responses-api proxy reads the key with a single `read(2)` into a fixed stack buffer, zeroizes that buffer on **every** exit path, builds exactly one heap `String`, then `leak()`s it to `&'static str` and `mlock(2)`s the page so it can never be swapped to disk or captured in a core dump.

**Incorrect (multiple un-scrubbable copies):**

```rust
// stdin()'s BufReader keeps a copy; `key` and `header` are un-zeroed heap allocations
let mut key = String::new();
std::io::stdin().read_to_string(&mut key)?;
let header = format!("Bearer {key}");
```

**Correct (one copy, zeroized buffer, mlock'd result):**

```rust
// responses-api-proxy/src/read_api_key.rs — read(2) into a stack buffer
let mut buf = [0u8; BUFFER_SIZE];
buf[..AUTH_HEADER_PREFIX.len()].copy_from_slice(AUTH_HEADER_PREFIX); // "Bearer "
// ... fill buf via read_fn, breaking on newline/EOF; on any error: buf.zeroize() then return ...

let header_value = String::from(header_str); // the only heap copy
buf.zeroize();                               // scrub the stack buffer immediately

let leaked: &'static mut str = header_value.leak();
mlock_str(leaked); // pin the page: no swap, no core-dump capture
Ok(leaked)
```

Every early return zeroizes first (`buf.zeroize(); return Err(...)`), so a parse failure can't leave the key on the stack. `read(2)` is chosen explicitly over `stdin()` because `Stdin`'s `BufReader` would retain an unreachable copy. See [[secrets-ctor-pre-main-hardening]] for the process-level hardening that protects this buffer.

Reference: `codex-rs/responses-api-proxy/src/read_api_key.rs:72`, `codex-rs/responses-api-proxy/src/read_api_key.rs:159`.

---
title: Enable test-only behavior via AtomicBool, not a cargo feature
impact: MEDIUM-HIGH
impactDescription: avoids doubling the build matrix while keeping deterministic IDs for tests
tags: testing, test-apis, atomic, determinism
---

## Enable test-only behavior via AtomicBool, not a cargo feature

A `#[cfg(feature = "test")]` path doubles the build matrix and still breaks downstream integration test crates that compile against the production build. Codex exposes `pub(crate) fn with_..._for_tests(...)` constructors with explicit "not for production" doc comments, and for determinism reads a `static AtomicBool` that only tests set to `true`. A single cargo build produces a binary that behaves deterministically when asked and identically to production otherwise — no `#[cfg]` branches in the hot path.

**Incorrect (cargo feature fragments the build graph):**

```rust
#[cfg(feature = "test-helpers")]
pub fn deterministic_process_id() -> String { "pid-0".into() }

#[cfg(not(feature = "test-helpers"))]
pub fn deterministic_process_id() -> String {
    format!("pid-{}", Uuid::new_v4())
}
// Integration tests in core/tests/ compile against non-test feature set.
```

**Correct (AtomicBool opt-in, single build):**

```rust
// core/src/unified_exec/process_manager.rs
/// Test-only override for deterministic unified exec process IDs.
///
/// In production builds this value should remain at its default (`false`)
/// and must not be toggled.
static FORCE_DETERMINISTIC_PROCESS_IDS: AtomicBool = AtomicBool::new(false);

pub(super) fn set_deterministic_process_ids_for_tests(enabled: bool) {
    FORCE_DETERMINISTIC_PROCESS_IDS.store(enabled, Ordering::Relaxed);
}

fn should_use_deterministic_process_ids() -> bool {
    cfg!(test) || deterministic_process_ids_forced_for_tests()
}

// core/src/test_support.rs — public gate for integration tests
//! Test-only helpers exposed for cross-crate integration tests.
//! Production code should not depend on this module. We prefer this
//! to a crate feature to avoid building multiple permutations.
```

`cfg!(test)` alone isn't enough because integration tests in `core/tests/` compile against the non-test build — the AtomicBool bridges that gap. The `_for_tests` suffix is how reviewers audit the test-only surface via grep.

Reference: `codex-rs/core/src/unified_exec/process_manager.rs:80`, `codex-rs/core/src/test_support.rs:1`.

---
title: Snapshot terminal rendering with insta for stable TUI diffs
impact: MEDIUM-HIGH
impactDescription: enables 1400 reviewable terminal snapshots that diff cleanly in PRs
tags: testing, snapshots, tui, insta
---

## Snapshot terminal rendering with insta for stable TUI diffs

Asserting individual strings with `assert!(popup.contains("Read Only"))` misses layout regressions and accepts arbitrary whitespace changes. Codex instantiates a real `ratatui::Terminal` backed by a VT100 emulator, draws the widget once, and `insta::assert_snapshot!(terminal.backend())` — the snapshot is a full ANSI-colored text dump. For stable file names when tests move between modules, it wraps the assertion in `insta::Settings::clone_current()`, `set_prepend_module_to_snapshot(false)`, `set_snapshot_path("snapshots")`, and binds a macro around it.

**Incorrect (substring assertions miss layout regressions):**

```rust
let rendered = render_popup(&state);
assert!(rendered.contains("Read Only"));
assert!(rendered.contains("Workspace Write"));
// A typo in alignment code? Still passes.
```

**Correct (full-terminal snapshot with stable paths):**

```rust
// tui/src/onboarding/trust_directory.rs
let mut terminal =
    Terminal::new(VT100Backend::new(70, 14)).expect("terminal");
terminal
    .draw(|frame| (&widget).render_ref(frame.area(), frame.buffer_mut()))
    .expect("draw");
insta::assert_snapshot!(terminal.backend());

// tui/src/chatwidget/tests.rs — macro with stable paths
macro_rules! assert_chatwidget_snapshot {
    ($name:expr, $value:expr $(,)?) => {{
        let mut settings = insta::Settings::clone_current();
        settings.set_prepend_module_to_snapshot(false);
        settings.set_snapshot_path(
            crate::chatwidget::tests::chatwidget_snapshot_dir(),
        );
        settings.bind(|| {
            insta::assert_snapshot!(
                format!("codex_tui__chatwidget__tests__{}", $name),
                $value,
            );
        });
    }};
}

// Platform-specific variant
#[cfg(target_os = "windows")]
insta::with_settings!({ snapshot_suffix => "windows" }, {
    assert_chatwidget_snapshot!("approvals_selection_popup", popup);
});
```

The macro hard-codes the `codex_tui__chatwidget__tests__` prefix so moving tests between submodules does not rename the snapshot files — a workaround for insta's default behavior. Look for `@windows` suffixes in `tui/src/chatwidget/snapshots/` to see how cross-platform variants coexist.

Reference: `codex-rs/tui/src/onboarding/trust_directory.rs:231`, `codex-rs/tui/src/chatwidget/tests.rs:197`.

---
title: Attach tests as sibling files via a path attribute
impact: MEDIUM-HIGH
impactDescription: prevents 5000-line modules where implementation hides inside a mile-long test body
tags: testing, organization, modules, path-attribute
---

## Attach tests as sibling files via a path attribute

The default Rust convention — `#[cfg(test)] mod tests { ... }` at the bottom of every module — stops scaling around 400 lines. Scrolling past a 2000-line test body to find the implementation is painful, and `git blame` attributes test changes to whoever last touched the module. Codex ends every module file with a three-line stub and keeps the tests in a sibling `foo_tests.rs` at the same path depth. `use super::*;` still gives access to `pub(crate)` items without a public test API.

**Incorrect (inline mod tests — implementation drowns in tests):**

```rust
// core/src/exec_policy.rs — 500 lines of implementation ...

#[cfg(test)]
mod tests {
    use super::*;
    // ... 2000 lines of tests inside this file
    #[test]
    fn exercises_exec_policy() { /* ... */ }
}
```

**Correct (sibling tests file via #[path]):**

```rust
// core/src/exec_policy.rs — ends with this 3-line stub
#[cfg(test)]
#[path = "exec_policy_tests.rs"]
mod tests;

// core/src/exec_policy_tests.rs — lives next to exec_policy.rs
use super::*;

#[test]
fn exercises_exec_policy() {
    /* ... */
}

// core/src/session/tests.rs — nested when even the sibling is huge
mod guardian_tests; // core/src/session/tests/guardian_tests.rs
```

`ls core/src` shows over 60 `foo.rs` / `foo_tests.rs` pairs — this is house style, not a one-off. The nested `#[path]` inside `codex_tests.rs` is the escape hatch when even the sibling file hits 5000 lines. Tests still see `pub(crate)` items because they compile as a child module of the parent, same as inline tests.

Reference: `codex-rs/core/src/exec_policy.rs:1046`, `codex-rs/core/src/session/tests.rs:181`.

---
title: Use start_paused and advance for deterministic timing tests
impact: MEDIUM-HIGH
impactDescription: eliminates wall-clock flakes from timing-dependent tests
tags: testing, async, determinism, tokio
---

## Use start_paused and advance for deterministic timing tests

Testing timeouts, retries, or debounces with real `tokio::time::sleep` calls blows wall-clock budgets and flakes under CI load. Codex marks timing tests with `#[tokio::test(start_paused = true)]` — the runtime starts with virtual time frozen. The test then spawns the unit under test, yields control once with `tokio::task::yield_now().await` to let the spawned task subscribe to the timer, and then calls `tokio::time::advance(duration).await` to jump forward deterministically. No wall-clock wait, no flakes.

**Incorrect (real sleep, flaky under load):**

```rust
#[tokio::test]
async fn times_out_after_five_seconds() {
    let handle = tokio::spawn(operation_with_timeout());
    tokio::time::sleep(Duration::from_secs(6)).await; // actual wait
    assert!(handle.await.unwrap().is_err());
}
```

**Correct (start_paused + yield + advance):**

```rust
// cloud-requirements/src/lib.rs
#[tokio::test(start_paused = true)]
async fn fetch_cloud_requirements_times_out() {
    let auth_manager = auth_manager_with_plan("enterprise");
    let codex_home = tempdir().expect("tempdir");
    let service = CloudRequirementsService::new(
        auth_manager,
        Arc::new(PendingFetcher),
        codex_home.path().to_path_buf(),
        CLOUD_REQUIREMENTS_TIMEOUT,
    );
    let handle = tokio::spawn(async move {
        service.fetch_with_timeout().await
    });
    tokio::time::advance(
        CLOUD_REQUIREMENTS_TIMEOUT + Duration::from_millis(1),
    ).await;

    let result = handle.await.expect("cloud requirements task");
    let err = result.expect_err("timeout should fail closed");
}

// The pattern when advance is called during setup:
let handle = tokio::spawn(async move { service.fetch().await });
tokio::task::yield_now().await;
tokio::time::advance(Duration::from_secs(1)).await;
```

The `yield_now()` between `spawn` and `advance` is load-bearing — without it, the spawned task has not yet registered its timer and `advance` is a no-op. This is the specific tokio idiom for paused-runtime tests and is not well documented.

Reference: `codex-rs/cloud-requirements/src/lib.rs:1534`, `codex-rs/cloud-requirements/src/lib.rs:1545`.

---
title: Use wiremock and small SSE constructors instead of mocking HTTP traits
impact: MEDIUM-HIGH
impactDescription: enables serialization, retry, and streaming coverage on every test
tags: testing, fakes, async, wiremock
---

## Use wiremock and small SSE constructors instead of mocking HTTP traits

Defining `trait ModelClient` and mocking it with `mockall` bypasses serialization, retry logic, and the streaming parser entirely — which is exactly where the real bugs live. Codex spins up a real `wiremock::MockServer` on a random port, rewrites the config's `base_url` to point at it, and serves a canned Server-Sent-Events body assembled from small event constructor functions (`ev_response_created`, `ev_assistant_message`, `ev_function_call`, `ev_completed`) piped through a single `sse(Vec<Value>)` formatter. Tests then assert against the actual request captured by `ResponseMock::single_request()`.

**Incorrect (trait mock drifts from real wire format):**

```rust
#[mockall::automock]
trait ModelClient {
    async fn send_turn(&self, params: TurnParams) -> Result<Turn>;
}
// Passes local tests, fails in production when SSE parser bug lands.
```

**Correct (real HTTP server with SSE event constructors):**

```rust
// core/tests/common/responses.rs
pub fn sse(events: Vec<Value>) -> String {
    use std::fmt::Write as _;
    let mut out = String::new();
    for event in events {
        let kind = event.get("type")
            .and_then(|value| value.as_str())
            .unwrap();
        writeln!(&mut out, "event: {kind}").unwrap();
        write!(&mut out, "data: {event}\n\n").unwrap();
    }
    out
}

// core/src/session/tests/guardian_tests.rs
let _request_log = mount_sse_once(
    &server,
    sse(vec![
        ev_response_created("resp-guardian"),
        ev_assistant_message("msg-guardian", &json!({}).to_string()),
        ev_completed("resp-guardian"),
    ]),
).await;
```

`mount_sse_once` returns a `ResponseMock` handle; the same handle then lets the test assert what Codex actually *sent* — bidirectional coverage. Every test exercises the HTTP layer, serde, retry, and the SSE parser for free. No test in the codebase mocks a trait for the model client.

Reference: `codex-rs/core/tests/common/responses.rs:600`, `codex-rs/core/src/session/tests/guardian_tests.rs:74`.

---
title: Restore terminal state via a Drop guard and chained panic hook
impact: MEDIUM
impactDescription: prevents wedged terminals that require manual `reset` after a panic
tags: tui, terminal, panic, raii
---

## Restore terminal state via a Drop guard and chained panic hook

Calling `disable_raw_mode()` at the end of `main` leaves the user's terminal wedged on any panic halfway through — raw mode stays on, alternate screen stays active, and a `reset` is the only way out. Codex wraps the main body in a `TerminalRestoreGuard { active: bool }` whose `Drop` calls `restore_silently()`, and installs a `panic::set_hook` that *chains the previous hook* rather than replacing it — so color-eyre's rich backtrace still fires *after* the terminal is restored.

**Incorrect (explicit restore at end of main — panic wedges terminal):**

```rust
fn main() -> io::Result<()> {
    enable_raw_mode()?;
    execute!(stdout(), EnterAlternateScreen)?;
    run_app()?; // panic here leaves terminal in alt-screen raw mode
    execute!(stdout(), LeaveAlternateScreen)?;
    disable_raw_mode()?;
    Ok(())
}
```

**Correct (Drop guard + chained panic hook):**

```rust
// tui/src/lib.rs — chain, don't replace
let prev_hook = std::panic::take_hook();
std::panic::set_hook(Box::new(move |info| {
    tracing::error!("panic: {info}");
    prev_hook(info);
}));
let mut terminal = tui::init()?;
let mut terminal_restore_guard = TerminalRestoreGuard::new();

// tui/src/tui.rs — init also installs its own restore hook
fn set_panic_hook() {
    let hook = panic::take_hook();
    panic::set_hook(Box::new(move |panic_info| {
        let _ = restore(); // ignore errors, we're already failing
        hook(panic_info);
    }));
}

// TerminalRestoreGuard
impl Drop for TerminalRestoreGuard {
    fn drop(&mut self) {
        if self.active {
            let _ = restore();
            self.active = false;
        }
    }
}
```

There are *two* layered panic hooks — `tui::init` installs one to restore the terminal, and `run` adds a tracing hook on top. Both chain the previous one. The `Drop for TerminalRestoreGuard` means you can `return Err(...)` from anywhere without leaving raw mode on. The `active: bool` gate also lets explicit early restore (before exec-ing `git commit` into the same terminal) not double-fire.

Reference: `codex-rs/tui/src/tui.rs:462`, `codex-rs/tui/src/lib.rs:1304`.

---
title: Pause the event stream by dropping it before subprocess handoff
impact: MEDIUM
impactDescription: prevents stdin race with child processes after handing off the terminal
tags: tui, events, subprocess, crossterm
---

## Pause the event stream by dropping it before subprocess handoff

When a TUI launches `$EDITOR`, `git commit`, or a pager, just stopping the poll loop is not enough. Crossterm's `EventStream` spawns an internal reader thread that keeps reading from stdin even if you never call `poll_next`, stealing input the subprocess meant to see. Codex wraps the stream in an `EventBroker` with three states (`Paused`, `Start`, `Running(S)`). Pause drops the stream entirely; resume recreates it; `flush_terminal_input_buffer()` uses `libc::tcflush` to discard any bytes the user typed during the handoff.

**Incorrect (stop polling but keep the stream):**

```rust
async fn run_git_commit() -> io::Result<()> {
    *polling_enabled.lock().await = false;
    let status = Command::new("git").arg("commit").status().await?;
    *polling_enabled.lock().await = true;
    // Crossterm's internal reader already stole half of git's keystrokes.
    Ok(())
}
```

**Correct (drop the stream, tcflush, recreate):**

```rust
// tui/src/tui/event_stream.rs
//! The motivation for dropping/recreating the crossterm event stream is
//! to enable the TUI to fully relinquish stdin. If the stream is not
//! dropped, it will continue to read from stdin even if it is not
//! actively being polled (due to how crossterm's EventStream is
//! implemented), stealing input from other processes
//! reading stdin, like terminal text editors.

pub fn pause_events(&self) {
    *self.state.lock().unwrap() = EventBrokerState::Paused;
}

// tui/src/tui.rs — flush stale stdin bytes on resume
#[cfg(unix)]
fn flush_terminal_input_buffer() {
    let result = unsafe {
        libc::tcflush(libc::STDIN_FILENO, libc::TCIFLUSH)
    };
    if result != 0 {
        tracing::warn!(
            "failed to tcflush stdin: {}",
            std::io::Error::last_os_error(),
        );
    }
}
```

Stopping polling is not enough — crossterm spawns an internal reader thread that races the child for stdin bytes. You must drop the stream. And even after recreating it, stale bytes can remain in the kernel's tty input buffer from the handoff window; `tcflush(TCIFLUSH)` drops them. Windows has a sibling using `FlushConsoleInputBuffer`.

Reference: `codex-rs/tui/src/tui/event_stream.rs:90`, `codex-rs/tui/src/tui.rs:322`.

---
title: Detect unbracketed paste bursts via a character timing state machine
impact: MEDIUM
impactDescription: prevents mid-paste shortcut key interpretation on terminals without bracketed paste
tags: tui, input, paste, state-machine
---

## Detect unbracketed paste bursts via a character timing state machine

Windows consoles, VS Code integrated terminals, and a surprising number of environments cannot deliver a single `Event::Paste` — they send one `KeyCode::Char` per pasted character, and if one of those is a shortcut key (like `?`) it gets interpreted mid-paste. Codex builds a pure state machine that consumes plain char events and returns a decision: `RetainFirstChar` (hold the first fast char so you can unwind), `BeginBufferFromPending`, `BeginBuffer { retro_chars }` (retroactively yank N already-inserted chars out of the textarea), or `BufferAppend`.

**Incorrect (insert each char immediately, mid-paste shortcuts fire):**

```rust
fn on_key(event: KeyEvent, textarea: &mut TextArea) {
    if let KeyCode::Char(ch) = event.code {
        textarea.insert(ch); // pasted "?" triggers help dialog
    }
}
```

**Correct (character-timing state machine returns decisions):**

```rust
// tui/src/bottom_pane/paste_burst.rs
#[cfg(not(windows))]
const PASTE_BURST_CHAR_INTERVAL: Duration = Duration::from_millis(8);
#[cfg(windows)]
const PASTE_BURST_CHAR_INTERVAL: Duration = Duration::from_millis(30);

pub fn on_plain_char(
    &mut self,
    character: char,
    now: Instant,
) -> CharDecision {
    self.note_plain_char(now);
    if self.active {
        return CharDecision::BufferAppend;
    }
    // Two fast chars -> upgrade held char into buffer
    if let Some((held, held_at)) = self.pending_first_char
        && now.duration_since(held_at) <= PASTE_BURST_CHAR_INTERVAL
    {
        self.active = true;
        let _ = self.pending_first_char.take();
        self.buffer.push(held);
        return CharDecision::BeginBufferFromPending;
    }
    if self.consecutive_plain_char_burst >= PASTE_BURST_MIN_CHARS {
        return CharDecision::BeginBuffer {
            retro_chars: self
                .consecutive_plain_char_burst
                .saturating_sub(1),
        };
    }
    self.pending_first_char = Some((character, now));
    CharDecision::RetainFirstChar
}
```

The `PasteBurst` never touches the textarea itself — it only returns decisions, and `ChatComposer` interprets them. That is why it is unit-testable. A specific pitfall: `clear_window_after_non_char` clears the last timestamp, so if you call it while `buffer` is non-empty without flushing first, the buffered text never flushes. The rule: flush before clearing, always.

Reference: `codex-rs/tui/src/bottom_pane/paste_burst.rs:157`.

---
title: Coalesce redraws through a FrameRequester actor
impact: MEDIUM
impactDescription: reduces redraw count when multiple producers request frames in the same tick
tags: tui, ratatui, rendering, performance
---

## Coalesce redraws through a FrameRequester actor

Drawing synchronously on every event produces wasted frames when three back-to-back updates all land in the same tick. Codex exposes a cheap, cloneable `FrameRequester` that sends an `Instant` over an unbounded channel. A dedicated tokio task coalesces every request received before the next deadline into a single `draw_tx.send(())` broadcast, clamped by a 120 FPS `FrameRateLimiter`. The event loop never draws spontaneously; widgets never call `draw` — they call `schedule_frame()` and go back to work.

**Incorrect (draw per event — wasted frames on bursts):**

```rust
async fn event_loop(mut terminal: Terminal<B>) -> io::Result<()> {
    while let Some(event) = events.next().await {
        handle_event(event);
        terminal.draw(|frame| render(frame))?; // draws on every event
    }
    Ok(())
}
```

**Correct (actor coalesces draw requests, draws on deadline):**

```rust
// tui/src/tui/frame_requester.rs
async fn run(mut self) {
    const ONE_YEAR: Duration = Duration::from_secs(60 * 60 * 24 * 365);
    let mut next_deadline: Option<Instant> = None;
    loop {
        let target = next_deadline.unwrap_or_else(|| {
            Instant::now() + ONE_YEAR
        });
        let deadline = tokio::time::sleep_until(target.into());
        tokio::pin!(deadline);
        tokio::select! {
            draw_at = self.receiver.recv() => {
                let Some(draw_at) = draw_at else { break };
                let draw_at = self.rate_limiter.clamp_deadline(draw_at);
                next_deadline = Some(
                    next_deadline
                        .map_or(draw_at, |cur| cur.min(draw_at)),
                );
                continue; // do NOT draw yet — recompute sleep
            }
            _ = &mut deadline => {
                if next_deadline.is_some() {
                    next_deadline = None;
                    self.rate_limiter.mark_emitted(target);
                    let _ = self.draw_tx.send(());
                }
            }
        }
    }
}
```

The `continue` after receiving a request is the crux — it does not draw, just tightens the sleep target. Three back-to-back `schedule_frame()` calls produce exactly one draw notification. The `ONE_YEAR` sentinel replaces the "how do I block forever on select" dance with a simple future-time constant.

Reference: `codex-rs/tui/src/tui/frame_requester.rs:96`.

---
title: Replace fixed throttles with hysteresis-gated smooth and catch-up modes
impact: MEDIUM
impactDescription: prevents visible lag on bursts without sacrificing the typewriter cadence feel
tags: tui, streaming, chunking, hysteresis
---

## Replace fixed throttles with hysteresis-gated smooth and catch-up modes

A fixed inter-line delay either looks choppy under bursts or abandons the typewriter feel entirely. Codex runs a single baseline cadence (one line per animation tick) in `Smooth` mode and flips to `CatchUp` when queue pressure builds, draining the backlog in one tick. Hysteresis on both entry and exit prevents gear-flapping: enter uses OR (depth OR age), exit uses AND (depth AND age), with an `EXIT_HOLD` window before coming back and a cooldown after exit unless backlog is severe.

**Incorrect (fixed per-line delay, bursts visibly lag):**

```rust
for line in new_lines {
    render_line(line).await;
    tokio::time::sleep(Duration::from_millis(16)).await; // choppy
}
```

**Correct (hysteresis thresholds, asymmetric enter/exit):**

```rust
// tui/src/streaming/chunking.rs
const ENTER_QUEUE_DEPTH_LINES: usize = 8;
const ENTER_OLDEST_AGE: Duration = Duration::from_millis(120);
const EXIT_QUEUE_DEPTH_LINES: usize = 2;
const EXIT_OLDEST_AGE: Duration = Duration::from_millis(40);
const EXIT_HOLD: Duration = Duration::from_millis(250);
const REENTER_CATCH_UP_HOLD: Duration = Duration::from_millis(250);
const SEVERE_QUEUE_DEPTH_LINES: usize = 64;
const SEVERE_OLDEST_AGE: Duration = Duration::from_millis(300);

pub fn decide(&self, snapshot: QueueSnapshot, now: Instant) -> Decision {
    let enter = snapshot.queued_lines >= ENTER_QUEUE_DEPTH_LINES
        || snapshot
            .oldest_age
            .map(|age| age >= ENTER_OLDEST_AGE)
            .unwrap_or(false);
    let exit = snapshot.queued_lines <= EXIT_QUEUE_DEPTH_LINES
        && snapshot
            .oldest_age
            .map(|age| age < EXIT_OLDEST_AGE)
            .unwrap_or(true);
    /* transition logic with hold windows */
}
```

Enter OR, exit AND — that asymmetry is what kills oscillation when only one signal is noisy. Tune in this order: thresholds → holds → severe gates → baseline cadence.

Reference: `codex-rs/tui/src/streaming/chunking.rs:85`.

---
title: Mark public wire-level enums non_exhaustive from the start
impact: HIGH
impactDescription: prevents breaking external match statements when a variant is added
tags: types, enums, api-design, versioning
---

## Mark public wire-level enums non_exhaustive from the start

Adding a variant to a public enum is normally a breaking change — downstream crates write exhaustive matches. `#[non_exhaustive]` tells the compiler to require a `_` arm in external crates, so new variants become additive. Codex puts this on `protocol::Op`, `user_input::UserInput`, and every top-level wire enum in the protocol crate. Because the protocol is used by a CLI, a TUI, multiple external clients, and a TypeScript SDK, committing to exhaustive matches would force a major version bump for every new feature.

**Incorrect (missing attribute — every new variant is a breaking change):**

```rust
#[derive(Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum Op {
    Interrupt,
    RealtimeStart(RealtimeParams),
    /* ... */
}
// Downstream `match op { Op::Interrupt => ..., Op::RealtimeStart(_) => ... }`
// won't compile when a new variant is added.
```

**Correct (non_exhaustive plus internally-tagged serde):**

```rust
// protocol/src/protocol.rs
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema)]
#[serde(tag = "type", rename_all = "snake_case")]
#[allow(clippy::large_enum_variant)]
#[non_exhaustive]
pub enum Op {
    Interrupt,
    CleanBackgroundTerminals,
    RealtimeConversationStart(ConversationStartParams),
    /* 30+ more variants, each additive */
}

// protocol/src/user_input.rs
#[non_exhaustive]
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, TS, JsonSchema)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum UserInput {
    Text { /* ... */ },
    Image { /* ... */ },
    /* ... */
}
```

The attribute sits above the derives, separate from serde and strum attributes. It is paired with `#[serde(tag = "type")]` so the wire encoding is also version-tolerant — unknown tags fail loudly rather than silently dropping data. Apply it from day one; retrofitting later is as breaking as adding a variant without it.

Reference: `codex-rs/protocol/src/protocol.rs:478`, `codex-rs/protocol/src/user_input.rs:12`.

---
title: Pass deserializer context via a thread-local RAII guard
impact: HIGH
impactDescription: enables serde to run path resolution without DeserializeSeed plumbing
tags: types, serde, raii, smart-constructor
---

## Pass deserializer context via a thread-local RAII guard

Serde's `Deserialize::deserialize` takes only a `Deserializer` — there is no room for extra parameters like "the base path for resolving relative values". The canonical workaround is `DeserializeSeed`, which means writing a parallel type for every context-aware struct. Codex instead stashes the context in a `thread_local! RefCell<Option<T>>` managed by an RAII guard scoped around the `from_str` call. Drop clears the slot; the `Deserialize` impl reads it and fails loudly if no guard is active and the wire value is not already self-sufficient.

**Incorrect (DeserializeSeed everywhere or post-pass mutation):**

```rust
#[derive(Deserialize)]
struct Config {
    log_path: PathBuf, // relative to what?
}
let mut cfg: Config = serde_json::from_str(src)?;
cfg.log_path = base.join(cfg.log_path); // forgot this one site → bug
```

**Correct (thread-local RAII guard, validated on deserialize):**

```rust
// utils/absolute-path/src/lib.rs
thread_local! {
    static ABSOLUTE_PATH_BASE: RefCell<Option<PathBuf>> =
        const { RefCell::new(None) };
}

pub struct AbsolutePathBufGuard;

impl AbsolutePathBufGuard {
    pub fn new(base_path: &Path) -> Self {
        ABSOLUTE_PATH_BASE.with(|cell| {
            *cell.borrow_mut() = Some(base_path.to_path_buf());
        });
        Self
    }
}

impl Drop for AbsolutePathBufGuard {
    fn drop(&mut self) {
        ABSOLUTE_PATH_BASE.with(|cell| *cell.borrow_mut() = None);
    }
}

impl<'de> Deserialize<'de> for AbsolutePathBuf {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where D: Deserializer<'de>
    {
        let path = PathBuf::deserialize(deserializer)?;
        ABSOLUTE_PATH_BASE.with(|cell| match cell.borrow().as_deref() {
            Some(base) => Ok(Self::resolve_path_against_base(path, base)),
            None if path.is_absolute() => {
                Self::from_absolute_path(path).map_err(SerdeError::custom)
            }
            None => Err(SerdeError::custom(
                "AbsolutePathBuf deserialized without a base path",
            )),
        })
    }
}
```

The guard is a zero-sized struct — it carries no data, only manages the TLS slot lifetime. Failing when no guard is active is deliberate: you cannot silently produce an invalid `AbsolutePathBuf`.

Reach for this only when the deserialization is single-threaded and runs on the thread that created the guard — the context lives in thread-local storage, so it is invisible to a multi-threaded or work-stealing deserializer and is not inherited by spawned tasks. When several callers need *different* bases concurrently, fall back to `DeserializeSeed`; the TLS guard trades that flexibility for not having to thread a seed type through every nested struct.

Reference: `codex-rs/utils/absolute-path/src/lib.rs:334`.

---
title: Use serde try_from on newtypes to run validation on every parse
impact: HIGH
impactDescription: eliminates forgotten validation calls at construction sites via parse-don't-validate
tags: types, newtype, serde, smart-constructor
---

## Use serde try_from on newtypes to run validation on every parse

A plain `struct AgentPath { path: String }` with a `validate()` method fails any time a caller forgets to run it. `#[serde(try_from = "String", into = "String")]` plumbs validation through serde: every deserialization invokes your `TryFrom<String>` impl, so an invalid wire value cannot silently produce a valid type. Codex uses three newtype strategies depending on strictness — `#[serde(transparent)]` for opaque strings, `try_from/into` for validated ones, hand-rolled `Serialize`/`Deserialize` only when the in-memory representation diverges from the wire format.

**Incorrect (validation method callers must remember):**

```rust
#[derive(Deserialize)]
pub struct AgentPath { path: String }

impl AgentPath {
    pub fn validate(&self) -> Result<(), String> { /* ... */ }
}
// Bug waiting to happen: someone forgets to call validate() after deserialize.
```

**Correct (try_from runs validation on every parse):**

```rust
// protocol/src/agent_path.rs
#[derive(
    Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash,
    Serialize, Deserialize, JsonSchema, TS,
)]
#[serde(try_from = "String", into = "String")]
#[schemars(with = "String")]
#[ts(type = "string")]
pub struct AgentPath(String);

impl AgentPath {
    pub fn from_string(path: String) -> Result<Self, String> {
        validate_absolute_path(path.as_str())?;
        Ok(Self(path))
    }
}

impl TryFrom<String> for AgentPath {
    type Error = String;
    fn try_from(value: String) -> Result<Self, Self::Error> {
        Self::from_string(value)
    }
}
```

The derived `Deserialize` automatically invokes `TryFrom<String>`, which calls `validate_absolute_path`. There is no constructor that bypasses validation — not even a private one. `#[ts(type = "string")]` and `#[schemars(with = "String")]` keep the type-export story consistent across TypeScript and JSON Schema, so consumers see a plain string, not an opaque newtype wrapper.

Reference: `codex-rs/protocol/src/agent_path.rs:12`.

---
title: Preserve unrecognized wire values in an Unknown variant
impact: HIGH
impactDescription: prevents older readers from crashing on configs written by newer versions
tags: types, enums, serde, forward-compat
---

## Preserve unrecognized wire values in an Unknown variant

Forward compatibility is the dual of `#[non_exhaustive]`. `non_exhaustive` says "you can add variants in the next version"; forward compatibility says "a reader on an older version must not reject values it does not recognize". Codex's `FileSystemSpecialPath` has an explicit `Unknown { path: String, subpath: Option<PathBuf> }` variant that captures any tag not matched by the known ones. An older runtime loads the new config, passes the Unknown through unchanged on round-trips, and writes it back out — the config file is never corrupted by a downgrade.

**Incorrect (strict enum — old reader crashes on new config):**

```rust
#[derive(Deserialize)]
#[serde(tag = "kind", rename_all = "snake_case")]
pub enum FileSystemSpecialPath {
    Root,
    Tmpdir,
    SlashTmp,
}
// New version adds a Minimal variant. Old version:
// `serde: unknown variant `minimal`` — config file fails to load.
```

**Correct (Unknown variant captures the tail):**

```rust
// protocol/src/permissions.rs
#[derive(
    Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS,
)]
#[serde(tag = "kind", rename_all = "snake_case")]
#[ts(tag = "kind")]
pub enum FileSystemSpecialPath {
    Root,
    Minimal,
    CurrentWorkingDirectory,
    ProjectRoots { subpath: Option<PathBuf> },
    Tmpdir,
    SlashTmp,
    /// WARNING: `:special_path` tokens are part of config compatibility.
    /// New parser support should be additive, while unknown values must stay
    /// representable so config from a newer Codex degrades to warn-and-ignore
    /// instead of failing to load. Codex 0.112.0 rejected unknown values
    /// here, which broke forward compatibility for newer config.
    Unknown {
        path: String,
        subpath: Option<PathBuf>,
    },
}
```

The `Unknown` variant carries the raw string, not a parsed form — you cannot lose the original value by round-tripping through this type. The warning doc comment is load-bearing; it names a real regression (`Codex 0.112.0`) that future editors must not reintroduce.

Reference: `codex-rs/protocol/src/permissions.rs:138`.

---
title: Avoid per-crate features; use target-cfg or split crates
impact: MEDIUM
impactDescription: prevents combinatorial build matrix explosion across a ~100-crate workspace
tags: workspace, cargo, features, build-matrix
---

## Avoid per-crate features; use target-cfg or split crates

In a workspace this size, Cargo `[features]` create a combinatorial build matrix — each subset is its own compilation unit, CI multiplies, and "it built locally, why did CI fail?" becomes routine. Codex handles optionality two other ways instead: platform variants go in `[target.'cfg(...)'.dependencies]`, and semantic variants become their own crates that compile to nothing on the wrong target. Across ~100 crates there are only two `[features]` sections, both for the same narrow reason a feature is genuinely the right tool: a native C dependency (`v8`) that exposes a compile-time toggle which cannot be expressed as a separate crate or a target-cfg.

**Incorrect (features accrete across crates, the cross product explodes):**

```toml
# core/Cargo.toml
[features]
default = ["linux-sandbox"]
linux-sandbox = ["dep:landlock"]
macos-sandbox = ["dep:sandbox-exec"]
windows-sandbox = ["dep:windows-sys"]
```

**Correct (target-cfg for platforms; separate crates for semantics):**

```toml
# core/Cargo.toml — platform differences live in target tables, not features
[target.x86_64-unknown-linux-musl.dependencies]
openssl-sys = { workspace = true, features = ["vendored"] }

[target.'cfg(unix)'.dependencies]
codex-shell-escalation = { workspace = true }
```

```toml
# code-mode/Cargo.toml — the rare justified feature: a native lib's build-time flag
[features]
sandbox = ["v8/v8_enable_sandbox"]
```

Sandbox backends are *crates* (`codex-linux-sandbox`, `codex-windows-sandbox`, `codex-macos-seatbelt`), each pulled in only under its `[target.'cfg(target_os = "...")']` table, so every build produces the same binary shape regardless of host. Reserve `[features]` for the case codex does — forwarding a flag a third-party native dependency requires — not for first-party optionality.

Reference: `codex-rs/core/Cargo.toml:122`, `codex-rs/code-mode/Cargo.toml:12`.

---
title: Stack HTTP layers as transport, api, and core crates
impact: MEDIUM
impactDescription: enables client crate reuse and prevents business logic from pulling in retries
tags: workspace, layering, architecture, crates
---

## Stack HTTP layers as transport, api, and core crates

A single `api_client` crate that pulls in config, auth, and business retry policies is un-reusable — anyone touching retries must recompile your prompt templates. Codex splits "the API client" into three crates with a strict downward dependency: `codex-client` knows only HTTP/SSE/retry primitives (zero Codex awareness), `codex-api` adds request shapes and SSE parsing on top, `codex-core` consumes `codex-api` and owns business logic. `codex-api` depends on `codex-client` and `codex-protocol` — the pure wire crates — but *not* on `codex-core`.

**Incorrect (monolithic client drags business logic everywhere):**

```toml
# codex-api/Cargo.toml
[dependencies]
codex-core = { workspace = true }    # business logic
codex-config = { workspace = true }  # config loading
codex-auth = { workspace = true }    # auth state
reqwest = { workspace = true }
```

**Correct (three layers, strict downward dependencies):**

```text
# codex-client/README.md
Generic transport layer that wraps HTTP requests, retries, and streaming
primitives without any Codex/OpenAI awareness.
- Defines `HttpTransport` and a default `ReqwestTransport`
- Provides retry utilities (`RetryPolicy`, `RetryOn`, `run_with_retry`)
- Consumed by higher-level crates like `codex-api`; it stays neutral on
  endpoints, headers, or API-specific error shapes.
```

```toml
# codex-api/Cargo.toml
[dependencies]
codex-client = { workspace = true }
codex-protocol = { workspace = true }
reqwest = { workspace = true, features = ["json", "stream"] }
eventsource-stream = { workspace = true }
# Notably absent: codex-core, codex-config, codex-auth
```

`codex-client` can be swapped into unrelated projects; `codex-api` can be reused by a different frontend without dragging in `core`'s ~50-dep transitive closure. The layer boundary is enforced by grep — any PR adding `codex-core` to `codex-api/Cargo.toml` is a review objection.

Reference: `codex-rs/codex-client/README.md:1`, `codex-rs/codex-api/Cargo.toml:7`.

---
title: Encode design policy in workspace.lints and clippy.toml
impact: MEDIUM
impactDescription: prevents policy drift from review-only conventions
tags: workspace, lints, clippy, policy
---

## Encode design policy in workspace.lints and clippy.toml

Treating design rules as review-only conventions fails across 75 crates — one missed review lets the bad pattern proliferate. Codex writes ~30 clippy lints as `deny` in `[workspace.lints.clippy]` and then opts each leaf crate in with `[lints] workspace = true`. `clippy.toml` at the workspace root relaxes the ban inside tests (`allow-expect-in-tests = true`) and encodes *design rules* like "don't hard-code Rgb colors — use ANSI themes" via `disallowed-methods`. The `core/src/lib.rs` header layers crate-local `#![deny(clippy::print_stdout, clippy::print_stderr)]` to force library output through tracing.

**Incorrect (each crate opts in ad-hoc, policy drifts):**

```rust
// core/src/lib.rs
#![deny(clippy::unwrap_used)]
// tui/src/lib.rs -- forgot this header, lints don't apply
```

**Correct (workspace-wide denies, leaves opt in):**

```toml
# Cargo.toml (root)
[workspace.lints]
rust = {}

[workspace.lints.clippy]
expect_used = "deny"
unwrap_used = "deny"
manual_clamp = "deny"
needless_collect = "deny"
redundant_clone = "deny"
```

```toml
# clippy.toml (root)
allow-expect-in-tests = true
allow-unwrap-in-tests = true
disallowed-methods = [
    {
        path = "ratatui::style::Color::Rgb",
        reason = "Use ANSI colors, which work better in various terminal themes.",
    },
    {
        path = "ratatui::style::Stylize::yellow",
        reason = "Avoid yellow; prefer other colors in `tui/styles.md`.",
    },
]
large-error-threshold = 256
```

```rust
// core/src/lib.rs — crate-local augmentation
//! Prevent accidental direct writes to stdout/stderr in library code.
//! All user-visible output must go through the TUI or tracing stack.
#![deny(clippy::print_stdout, clippy::print_stderr)]
```

`large-error-threshold = 256` is a subtle choice that gates `result_large_err` up from the default to allow rich `thiserror` enums. `disallowed-methods` is the architectural enforcement mechanism — banning direct `Color::Rgb` frees code review to focus on logic.

Reference: `codex-rs/Cargo.toml:432`, `codex-rs/clippy.toml:1`, `codex-rs/core/src/lib.rs:6`.

---
title: Register shared test helpers as workspace member crates
impact: MEDIUM
impactDescription: enables cross-crate test helper reuse without path-attribute hacks
tags: workspace, testing, cargo, dev-dependencies
---

## Register shared test helpers as workspace member crates

When integration tests across multiple crates share fixture code, the usual workaround is `#[path = "../../other_crate/tests/common/mod.rs"] mod common;` — fragile, confusing, and it recompiles the helpers for every test binary. Codex promotes each crate's test helpers to a first-class workspace member *without* moving them out of the crate that owns them. `core_test_support`, `app_test_support`, and `mcp_test_support` live at paths like `core/tests/common/Cargo.toml` — physically inside `core/tests/`, but registered in the root `[workspace.dependencies]` by path.

**Incorrect (path-attribute shims per test file):**

```rust
// In every test file across five crates
#[path = "../../other_crate/tests/common/mod.rs"]
mod common;
use common::setup_test_codex;
```

**Correct (test-support as a normal workspace crate):**

```toml
# Cargo.toml (root)
[workspace.dependencies]
# Internal — test-only crates live inside the tests/ directory
# of the crate they support, but registered here as workspace deps.
app_test_support = { path = "app-server/tests/common" }
core_test_support = { path = "core/tests/common" }
mcp_test_support = { path = "mcp-server/tests/common" }
```

```toml
# core/tests/common/Cargo.toml
[package]
name = "core_test_support"
version.workspace = true
edition.workspace = true
license.workspace = true

[lib]
path = "lib.rs"

[lints]
workspace = true
```

```toml
# core/Cargo.toml — consumer
[dev-dependencies]
core_test_support = { workspace = true }
```

The test-support crate uses snake_case (`core_test_support`) to signal it's internal; the `[lib] path = "lib.rs"` pulls the library root out of a `src/` subdirectory; it is NOT a member of `[workspace] members` but *is* registered in `[workspace.dependencies]`. `app_test_support` itself depends on `core_test_support`, proving these test crates compose into a helper pyramid just like production crates.

Reference: `codex-rs/Cargo.toml:239`, `codex-rs/core/tests/common/Cargo.toml:2`.

---
title: Place shared utilities in single-purpose microcrates under utils/
impact: MEDIUM
impactDescription: enables parallel compilation and minimal dependency graphs per concern
tags: workspace, crate-granularity, compilation, reuse
---

## Place shared utilities in single-purpose microcrates under utils/

"We'll put this in a shared `utils` module" creates a monolithic crate that forces every caller to compile the union of every dependency anyone ever wanted. Codex has a dedicated `utils/` directory that holds 20+ microcrates, each a single concern: `utils/absolute-path`, `utils/elapsed`, `utils/fuzzy-match`, `utils/home-dir`, `utils/readiness`, `utils/stream-parser`. Each is a separate crate so its dependency graph is minimal — e.g. `codex-utils-elapsed` is a duration formatter that does not drag in tokio.

**Incorrect (one monolithic utils crate pulls in everything):**

```toml
# codex-utils/Cargo.toml — the "shared module" crate
[dependencies]
tokio = { workspace = true }       # only needed by pty helper
regex = { workspace = true }       # only needed by fuzzy-match
chrono = { workspace = true }      # only needed by elapsed
ratatui = { workspace = true }     # only needed by sandbox-summary
# Every consumer pays for all of these.
```

**Correct (a microcrate per concern under utils/):**

```toml
# Cargo.toml (workspace root)
[workspace]
members = [
    "utils/absolute-path",
    "utils/cargo-bin",
    "utils/cache",
    "utils/image",
    "utils/json-to-toml",
    "utils/home-dir",
    "utils/pty",
    "utils/readiness",
    "utils/rustls-provider",
    "utils/string",
    "utils/elapsed",
    "utils/sandbox-summary",
    "utils/sleep-inhibitor",
    "utils/fuzzy-match",
    "utils/stream-parser",
    "utils/template",
]

[workspace.dependencies]
codex-utils-absolute-path = { path = "utils/absolute-path" }
codex-utils-elapsed       = { path = "utils/elapsed" }
codex-utils-fuzzy-match   = { path = "utils/fuzzy-match" }
```

The directory structure (`utils/foo`) is distinct from the crate name (`codex-utils-foo`) — the directory namespace keeps the 75-crate `ls` output legible, while the crate-name prefix makes `cargo add codex-utils-*` greppable. `profile.release` uses `lto = "fat"` and `codegen-units = 1` so the crate explosion has zero runtime cost after link-time optimization.

Reference: `codex-rs/Cargo.toml:82`, `codex-rs/Cargo.toml:196`.

---
name: openai-codex-rust-patterns
description: OpenAI Codex Rust coding patterns distilled from the codex-rs workspace. Use this skill whenever writing, reviewing, or refactoring Rust code — especially for async agents, CLI tools, sandboxing, secret handling, Ratatui TUIs, JSON-RPC protocols, tokio-based services, or any codebase that needs defensive panic discipline. Trigger even when the user does not explicitly mention Codex, because the patterns generalize to any production Rust workspace. Covers async cancellation, error enum design, process sandboxing, DNS-rebinding defense, credential hardening (zeroize/mlock/ctor), Cargo workspace architecture, wiremock-based fakes, insta snapshot testing, OpenTelemetry tracing, and Ratatui rendering.
---

# OpenAI Codex Rust Best Practices

Distilled from [`openai/codex`](https://github.com/openai/codex) `codex-rs/` — a 119-crate, 2,008-file Rust workspace that ships the Codex CLI coding agent. Contains 63 rules across 11 categories, each citing the exact file in codex-rs where the pattern lives, so you can write Rust the way its top contributors (Michael Bolin, jif-oai, Ahmed Ibrahim, Eric Traut, Pavel Krymets) actually ship it. Citations were refreshed against `main` at commit `8a94430` (2026-05-25).

## When to Apply

Reference these guidelines when:

- Writing or reviewing async Rust code that spawns tokio tasks, owns cancellation tokens, or manages long-lived background workers.
- Designing error enums, `Result` flows, retry loops, or layer boundaries in a library or service.
- Building a CLI tool that spawns subprocesses, enforces sandboxing, or runs LLM-generated code safely.
- Architecting a Cargo workspace with more than ~5 crates, deciding what to split out, and how to manage shared dependencies.
- Adding tests to a Rust codebase where existing tests are inline `mod tests { ... }` blocks and scaling is becoming painful.
- Implementing a JSON-RPC or custom wire protocol with serde — especially one that must evolve without breaking clients.
- Reading API keys or other secrets into memory, or hardening a binary that handles credentials against core dumps, debugger attach, and `LD_PRELOAD`.
- Enforcing a network egress allowlist that must survive DNS rebinding, or loading untrusted plugins/extensions.
- Wiring OpenTelemetry traces, logs, or metrics into a service that has privacy constraints around PII.
- Building a Ratatui-based TUI that streams LLM output, handles paste bursts, or manages raw-mode terminal state.
- Any time you find yourself reaching for `.unwrap()`, `.lock().unwrap()`, `anyhow::Result<()>`, or `#[cfg(feature = "test")]` — this skill explains what codex does instead.

## Rule Categories by Priority

| Priority | Category | Impact | Prefix |
|----------|----------|--------|--------|
| 1 | Defensive Coding & Panic Discipline | CRITICAL | `defensive-` |
| 2 | Error Handling & Result Discipline | CRITICAL | `errors-` |
| 3 | Async, Concurrency & Cancellation | HIGH | `async-` |
| 4 | Sandboxing & Process Isolation | HIGH | `sandbox-` |
| 5 | Secrets & Process Hardening | HIGH | `secrets-` |
| 6 | Type Design & Invariants | HIGH | `types-` |
| 7 | Testing Architecture | MEDIUM-HIGH | `testing-` |
| 8 | Protocol & Serde Design | MEDIUM-HIGH | `proto-` |
| 9 | Workspace & Crate Organization | MEDIUM | `workspace-` |
| 10 | Observability & Tracing | MEDIUM | `otel-` |
| 11 | TUI (Ratatui) Rendering | MEDIUM | `tui-` |

## Quick Reference

### 1. Defensive Coding & Panic Discipline (CRITICAL)

- [`defensive-deny-unwrap-workspace-wide`](references/defensive-deny-unwrap-workspace-wide.md) — Deny unwrap and expect at the workspace level, opt in locally.
- [`defensive-debug-assert-with-early-return`](references/defensive-debug-assert-with-early-return.md) — Use debug_assert(false) with a safe fallback on unreachable branches.
- [`defensive-recover-poisoned-lock`](references/defensive-recover-poisoned-lock.md) — Recover a poisoned lock with into_inner instead of unwrapping it.
- [`defensive-banned-interpreter-prefixes`](references/defensive-banned-interpreter-prefixes.md) — Avoid learning allowlist rules for general-purpose interpreters.
- [`defensive-head-tail-output-buffer`](references/defensive-head-tail-output-buffer.md) — Cap subprocess output with a head-and-tail ring buffer.
- [`defensive-io-drain-timeout-grandchildren`](references/defensive-io-drain-timeout-grandchildren.md) — Time out the I/O drain task separately from the child process.
- [`defensive-refuse-to-run-unsandboxed`](references/defensive-refuse-to-run-unsandboxed.md) — Refuse to run when the sandbox cannot enforce the requested policy.
- [`defensive-canonicalize-approval-cache-key`](references/defensive-canonicalize-approval-cache-key.md) — Canonicalize shell wrappers before hashing approval keys.
- [`defensive-fault-isolate-plugin-load`](references/defensive-fault-isolate-plugin-load.md) — Isolate plugin load failures and sanitize manifest text before the model sees it.

### 2. Error Handling & Result Discipline (CRITICAL)

- [`errors-exhaustive-retryable-match`](references/errors-exhaustive-retryable-match.md) — Classify retryable errors with an exhaustive match on every variant.
- [`errors-transient-permanent-type-split`](references/errors-transient-permanent-type-split.md) — Encode transient vs permanent outcomes as two enum variants.
- [`errors-boundary-error-translator`](references/errors-boundary-error-translator.md) — Translate errors at the layer boundary in a single function.
- [`errors-carry-retry-delay-in-variant`](references/errors-carry-retry-delay-in-variant.md) — Carry the server-requested retry delay inside the error variant.
- [`errors-struct-display-payload`](references/errors-struct-display-payload.md) — Put display-relevant error state in a struct, not a preformatted string.
- [`errors-tool-call-respond-vs-fatal`](references/errors-tool-call-respond-vs-fatal.md) — Split tool errors into respond-to-model and fatal variants.
- [`errors-io-error-with-context-struct`](references/errors-io-error-with-context-struct.md) — Wrap io::Error in a struct with a context field instead of anyhow.

### 3. Async, Concurrency & Cancellation (HIGH)

- [`async-abort-on-drop-handle`](references/async-abort-on-drop-handle.md) — Store JoinHandles as AbortOnDropHandle so Drop cancels them.
- [`async-graceful-then-forceful-cancel`](references/async-graceful-then-forceful-cancel.md) — Cancel cooperatively first, then abort after a grace deadline.
- [`async-biased-select-for-cancellation`](references/async-biased-select-for-cancellation.md) — Use biased select to make cancellation always win race ties.
- [`async-bounded-vs-unbounded-channel-split`](references/async-bounded-vs-unbounded-channel-split.md) — Bound the submission channel but leave the event channel unbounded.
- [`async-child-cancellation-tokens`](references/async-child-cancellation-tokens.md) — Give spawned sub-tasks child tokens, not clones of the parent.
- [`async-shared-boxfuture-joinhandle`](references/async-shared-boxfuture-joinhandle.md) — Wrap a background JoinHandle in Shared<BoxFuture> for multi-waiter joins.

### 4. Sandboxing & Process Isolation (HIGH)

- [`sandbox-shared-policy-data-model`](references/sandbox-shared-policy-data-model.md) — Keep sandbox policy as shared data, not per-platform code.
- [`sandbox-staged-restrictions-re-exec`](references/sandbox-staged-restrictions-re-exec.md) — Stage incompatible restrictions by re-executing the same binary.
- [`sandbox-resolve-before-allow-dns-rebinding`](references/sandbox-resolve-before-allow-dns-rebinding.md) — Resolve hostnames and reject private IPs to defeat DNS rebinding.
- [`sandbox-dev-null-first-missing-mount`](references/sandbox-dev-null-first-missing-mount.md) — Mount /dev/null over the first missing path to block mkdir escapes.
- [`sandbox-three-layer-network-isolation`](references/sandbox-three-layer-network-isolation.md) — Stack env vars, seccomp, and namespaces for network isolation.
- [`sandbox-env-clear-pre-exec`](references/sandbox-env-clear-pre-exec.md) — Clear the env and tether children via pre_exec before every spawn.
- [`sandbox-argv0-multiplex-binary`](references/sandbox-argv0-multiplex-binary.md) — Multiplex helper binaries via argv[0] and symlinks.

### 5. Secrets & Process Hardening (HIGH)

- [`secrets-read-into-locked-buffer`](references/secrets-read-into-locked-buffer.md) — Read a secret into a zeroized stack buffer, then mlock it — never through stdin().
- [`secrets-ctor-pre-main-hardening`](references/secrets-ctor-pre-main-hardening.md) — Harden a secret-handling process before main() runs, and fail closed.
- [`secrets-manual-debug-elide`](references/secrets-manual-debug-elide.md) — Write a manual Debug impl that elides credentials instead of deriving it.

### 6. Type Design & Invariants (HIGH)

- [`types-thread-local-raii-serde`](references/types-thread-local-raii-serde.md) — Pass deserializer context via a thread-local RAII guard.
- [`types-try-from-newtype-validation`](references/types-try-from-newtype-validation.md) — Use serde try_from on a newtype to run validation on every parse.
- [`types-non-exhaustive-public-enums`](references/types-non-exhaustive-public-enums.md) — Mark every public wire-level enum non_exhaustive from the start.
- [`types-unknown-variant-forward-compat`](references/types-unknown-variant-forward-compat.md) — Preserve unrecognized values in an Unknown variant.

### 7. Testing Architecture (MEDIUM-HIGH)

- [`testing-path-attribute-sibling-tests`](references/testing-path-attribute-sibling-tests.md) — Attach tests as sibling files via #[path] instead of inline mod tests.
- [`testing-wiremock-sse-fakes`](references/testing-wiremock-sse-fakes.md) — Fake the network with wiremock and small SSE event constructors.
- [`testing-atomic-bool-test-opt-in`](references/testing-atomic-bool-test-opt-in.md) — Gate test-only behavior with an AtomicBool, not a cargo feature.
- [`testing-insta-snapshot-tui-rendering`](references/testing-insta-snapshot-tui-rendering.md) — Snapshot terminal rendering with insta for stable UI diffs.
- [`testing-paused-runtime-advance`](references/testing-paused-runtime-advance.md) — Use start_paused and advance to make timing-dependent tests deterministic.

### 8. Protocol & Serde Design (MEDIUM-HIGH)

- [`proto-internally-tagged-rpc-dispatch`](references/proto-internally-tagged-rpc-dispatch.md) — Dispatch JSON-RPC by an internally tagged enum with a macro.
- [`proto-double-option-tri-state`](references/proto-double-option-tri-state.md) — Use Option<Option<T>> to distinguish absent, null, and set.
- [`proto-rename-alias-wire-migration`](references/proto-rename-alias-wire-migration.md) — Pair rename and alias to migrate wire names without breaking clients.
- [`proto-experimental-runtime-gate`](references/proto-experimental-runtime-gate.md) — Gate experimental fields by runtime presence, not capability flags.
- [`proto-sse-idle-timeout-terminator`](references/proto-sse-idle-timeout-terminator.md) — Treat SSE streams as idle-timeout with required terminator.
- [`proto-internal-vs-wire-error-split`](references/proto-internal-vs-wire-error-split.md) — Split internal error enums from wire error enums.
- [`proto-removed-feature-tombstone`](references/proto-removed-feature-tombstone.md) — Keep removed feature flags as parseable no-op tombstones.

### 9. Workspace & Crate Organization (MEDIUM)

- [`workspace-layered-transport-api-core`](references/workspace-layered-transport-api-core.md) — Stack HTTP layers as transport, api, and core crates.
- [`workspace-lint-config-package`](references/workspace-lint-config-package.md) — Encode policy in workspace.lints and clippy.toml.
- [`workspace-utils-microcrate-fanout`](references/workspace-utils-microcrate-fanout.md) — Place shared utilities in single-purpose microcrates under utils/.
- [`workspace-test-support-as-member-crates`](references/workspace-test-support-as-member-crates.md) — Register shared test helpers as workspace member crates.
- [`workspace-ban-per-crate-features`](references/workspace-ban-per-crate-features.md) — Avoid per-crate features; use target-cfg or separate crates instead.

### 10. Observability & Tracing (MEDIUM)

- [`otel-log-only-vs-trace-safe-targets`](references/otel-log-only-vs-trace-safe-targets.md) — Route PII to log-only targets and keep traces cardinality-safe.
- [`otel-field-empty-then-record`](references/otel-field-empty-then-record.md) — Declare span fields as field::Empty, then record them when known.
- [`otel-layered-subscribers-env-filter`](references/otel-layered-subscribers-env-filter.md) — Build per-layer EnvFilter instances with boxed fmt layers.
- [`otel-w3c-traceparent-propagation`](references/otel-w3c-traceparent-propagation.md) — Propagate W3C traceparent via env vars, JSON-RPC, and HTTP headers.
- [`otel-instrument-at-trace-level`](references/otel-instrument-at-trace-level.md) — Default #[instrument] to trace level, reserve info for network calls.

### 11. TUI (Ratatui) Rendering (MEDIUM)

- [`tui-two-gear-hysteresis-chunking`](references/tui-two-gear-hysteresis-chunking.md) — Replace fixed throttles with hysteresis-gated smooth and catch-up modes.
- [`tui-schedule-frame-coalescer`](references/tui-schedule-frame-coalescer.md) — Coalesce redraws through a FrameRequester actor and rate limiter.
- [`tui-drop-guard-panic-hook-chain`](references/tui-drop-guard-panic-hook-chain.md) — Restore terminal state via a Drop guard and a chained panic hook.
- [`tui-paste-burst-state-machine`](references/tui-paste-burst-state-machine.md) — Detect unbracketed paste bursts via a character timing state machine.
- [`tui-event-broker-pause-resume`](references/tui-event-broker-pause-resume.md) — Pause the event stream by dropping it before a subprocess handoff.

## How to Use

Read individual reference files for detailed explanations and code examples cited from `codex-rs/`:

- [Section definitions](references/_sections.md) — Category structure, impact levels, and prefixes
- [AGENTS.md](AGENTS.md) — Auto-generated navigation document compiling every rule

Each rule file contains:

- Imperative title matching its frontmatter
- 2–4 sentence explanation of the WHY
- **Incorrect** example showing the naive approach
- **Correct** example from codex-rs with the file path cited

## Reference Files

| File | Description |
|------|-------------|
| [AGENTS.md](AGENTS.md) | Auto-built TOC document compiling every rule |
| [README.md](README.md) | Skill repository docs — contribution, structure, commands |
| [references/_sections.md](references/_sections.md) | Category definitions and ordering |
| [gotchas.md](gotchas.md) | Failure points discovered while applying these rules |
| [metadata.json](metadata.json) | Version, discipline, references to codex-rs |