rust-cli-agent-style

어떤 스킬인가요?

Coding patterns extracted from OpenAI Codex Rust codebase - a production CLI/agent system with strict error handling, async patterns, and workspace organization

설치 방법

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

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

스킬 원문 파일

# Rust CLI Agent

**Version 1.0.0**  
OpenAI Codex  
2026-01-28

> **Note:**  
> This document is mainly for agents and LLMs to follow when maintaining,  
> generating, or refactoring codebases. Humans may also find it useful,  
> but guidance here is optimized for automation and consistency by AI-assisted workflows.

---

## Abstract

Coding patterns extracted from OpenAI Codex Rust codebase - a production CLI/agent system with 50 crates, 787 Rust files, strict error handling (deny unwrap/expect), tokio async runtime, and thiserror/anyhow error patterns.

---

## Table of Contents

1. [Error Handling](references/_sections.md#1-error-handling) — **CRITICAL**
   - 1.1 [Add context to errors with .context()](references/err-context-chain.md) — HIGH (Error context chains make debugging production issues much easier)
   - 1.2 [Avoid expect() in library code](references/err-no-expect.md) — CRITICAL (Library code should never panic - callers cannot recover from panics)
   - 1.3 [Document error conditions with # Errors](references/err-doc-errors.md) — MEDIUM (Clear documentation helps users handle errors correctly)
   - 1.4 [Never use unwrap() in non-test code](references/err-no-unwrap.md) — CRITICAL (Panics in production code cause crashes and poor user experience)
   - 1.5 [Use #[error(transparent)] for wrapped errors](references/err-transparent.md) — MEDIUM (Preserves original error messages when wrapping without additional context)
   - 1.6 [Use #[from] for automatic error conversion](references/err-from-derive.md) — MEDIUM (Reduces boilerplate for common error conversions)
   - 1.7 [Use anyhow::Result for application entry points](references/err-anyhow-application.md) — HIGH (Simplifies error handling in application code while preserving error context)
   - 1.8 [Use map_err for error type conversion](references/err-map-err-conversion.md) — MEDIUM (Explicit error conversion improves code clarity and control)
   - 1.9 [Use std::io::Result for I/O functions](references/err-io-result.md) — MEDIUM (Standard I/O result type for consistency with ecosystem)
   - 1.10 [Use structured error variants with fields](references/err-structured-variants.md) — HIGH (Structured errors enable better debugging and programmatic error handling)
   - 1.11 [Use thiserror for domain error types](references/err-thiserror-domain.md) — CRITICAL (Consistent error types enable proper error propagation and debugging across the codebase)
2. [Organization](references/_sections.md#2-organization) — **HIGH**
   - 2.1 [Create separate crate for shared test utilities](references/org-test-common-crate.md) — MEDIUM (Shared test utilities reduce duplication and enable consistent testing patterns)
   - 2.2 [Define error types in dedicated errors.rs file](references/org-errors-file.md) — MEDIUM (Centralized error definitions improve discoverability and consistency)
   - 2.3 [Organize by feature with mod.rs files](references/org-feature-modules.md) — MEDIUM (Feature-based organization improves code navigation and maintainability)
   - 2.4 [Organize integration tests in suite directory](references/org-integration-tests-suite.md) — MEDIUM (Structured test organization improves maintainability and test discovery)
   - 2.5 [Place handlers in dedicated subdirectory](references/org-handlers-subdir.md) — MEDIUM (Separating handlers from core logic improves code organization)
   - 2.6 [Use flat workspace structure with utils subdirectory](references/org-workspace-flat.md) — HIGH (Consistent workspace layout enables easy navigation and discovery)
   - 2.7 [Use kebab-case for crate directories with project prefix](references/org-crate-naming.md) — HIGH (Consistent naming enables easy identification of internal crates)
   - 2.8 [Use pub(crate) for internal APIs](references/org-module-visibility.md) — HIGH (Proper visibility prevents accidental external dependencies on internal APIs)
3. [Component Patterns](references/_sections.md#3-component-patterns) — **HIGH**
   - 3.1 [Create type aliases for complex generic types](references/mod-type-alias-complex.md) — MEDIUM (Type aliases improve readability of complex nested generics)
   - 3.2 [Default to private fields with public constructor](references/mod-struct-visibility.md) — MEDIUM (Encapsulation enables future changes without breaking API)
   - 3.3 [Derive JsonSchema for API types](references/mod-jsonschema-derive.md) — MEDIUM (Schema generation enables automatic API documentation and validation)
   - 3.4 [Include Send + Sync + 'static for concurrent traits](references/mod-trait-bounds.md) — HIGH (Proper bounds enable traits to work in async and multithreaded contexts)
   - 3.5 [Order derive macros consistently](references/mod-derive-order.md) — MEDIUM (Consistent derive ordering improves code readability and diff quality)
   - 3.6 [Order impl blocks consistently](references/mod-impl-block-order.md) — LOW (Consistent ordering makes code navigation predictable)
   - 3.7 [Use #[async_trait] for async trait methods](references/mod-async-trait-macro.md) — HIGH (Enables async methods in traits which Rust doesn't natively support yet)
   - 3.8 [Use builder pattern for complex configuration](references/mod-builder-pattern.md) — MEDIUM (Builders make construction of complex types readable and flexible)
   - 3.9 [Use Ext suffix for extension traits](references/mod-extension-trait-suffix.md) — MEDIUM (Clear naming distinguishes extension traits from core traits)
   - 3.10 [Use newtype pattern for type safety](references/mod-newtype-pattern.md) — HIGH (Newtypes prevent mixing up values with the same underlying type)
   - 3.11 [Use serde rename for wire format compatibility](references/mod-serde-rename.md) — HIGH (Proper serde configuration ensures API compatibility)
   - 3.12 [Use where clauses for complex bounds](references/mod-generic-constraints.md) — MEDIUM (Where clauses improve readability of complex generic constraints)
4. [Cross-Crate](references/_sections.md#4-cross-crate) — **HIGH**
   - 4.1 [Centralize dependency versions in workspace](references/cross-workspace-deps.md) — HIGH (Consistent dependency versions prevent version conflicts)
   - 4.2 [Define common lints in workspace Cargo.toml](references/cross-workspace-lints.md) — HIGH (Consistent lint configuration across all workspace crates)
5. [Naming Conventions](references/_sections.md#5-naming-conventions) — **MEDIUM**
   - 5.1 [Avoid _async suffix for async functions](references/name-async-no-suffix.md) — MEDIUM (Cleaner API - async is evident from the function signature)
   - 5.2 [Define crate-specific Result type alias](references/name-result-type-alias.md) — MEDIUM (Reduces boilerplate and makes error types explicit)
   - 5.3 [Name environment variable constants with _ENV_VAR suffix](references/name-const-env-var.md) — LOW (Clear distinction between config keys and their environment variable sources)
   - 5.4 [Pair Request/Response types](references/name-request-response.md) — MEDIUM (Consistent API type pairing improves code navigation and understanding)
   - 5.5 [Use Client suffix for API clients](references/name-client-suffix.md) — HIGH (Clear identification of types that make external API calls)
   - 5.6 [Use Error suffix for error types](references/name-error-suffix.md) — HIGH (Consistent error naming enables easy identification and handling)
   - 5.7 [Use Handler suffix for trait implementations](references/name-handler-suffix.md) — MEDIUM (Clear naming indicates the purpose of handler types)
   - 5.8 [Use Info suffix for read-only data structures](references/name-info-suffix.md) — LOW (Clear indication that a type is for information retrieval, not mutation)
   - 5.9 [Use is_/has_/should_ prefix for boolean functions](references/name-bool-is-prefix.md) — MEDIUM (Boolean function naming follows English grammar for readability)
   - 5.10 [Use Manager suffix for lifecycle management](references/name-manager-suffix.md) — MEDIUM (Clear identification of types that manage resource lifecycles)
   - 5.11 [Use Options suffix for configuration bundles](references/name-options-suffix.md) — MEDIUM (Clear naming for optional configuration structures)
   - 5.12 [Use plural names for collections](references/name-plural-collections.md) — LOW (Natural English naming for collection types)
   - 5.13 [Use Provider suffix for service implementations](references/name-provider-suffix.md) — MEDIUM (Clear identification of service provider types)
   - 5.14 [Use try_ prefix for fallible constructors](references/name-try-prefix-fallible.md) — HIGH (Clear indication that construction can fail)
   - 5.15 [Use with_ prefix for builder methods](references/name-with-prefix-builder.md) — MEDIUM (Consistent builder API across the codebase)
6. [Style](references/_sections.md#6-style) — **MEDIUM**
   - 6.1 [Add module-level documentation](references/style-module-docs.md) — MEDIUM (Module docs provide context and improve API discoverability)
   - 6.2 [Deny direct stdout/stderr in library code](references/style-deny-stdout.md) — HIGH (Library code should use structured logging, not direct output)
   - 6.3 [Place unit tests in #[cfg(test)] mod tests](references/style-cfg-test-module.md) — HIGH (Standard Rust test organization pattern)
   - 6.4 [Use #[expect] with reason for lint suppression](references/style-expect-reason.md) — MEDIUM (Documents why lints are suppressed and ensures suppressions are still needed)
   - 6.5 [Use inline format arguments](references/style-inline-format-args.md) — MEDIUM (Cleaner, more readable format strings)
   - 6.6 [Use one item per use statement](references/style-import-granularity.md) — MEDIUM (Cleaner diffs and easier import management)

---

## References

1. [https://github.com/openai/codex](https://github.com/openai/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 impact ordering |
| [assets/templates/_template.md](assets/templates/_template.md) | Template for creating new rules |
| [SKILL.md](SKILL.md) | Quick reference entry point |
| [metadata.json](metadata.json) | Version and reference URLs |

# rust-cli-agent-style

Coding style patterns extracted from the OpenAI Codex Rust codebase.

## Overview

This skill captures the coding patterns, conventions, and best practices from [OpenAI Codex](https://github.com/openai/codex) (`codex-rs/` subdirectory) - a production Rust CLI/agent system with:

- 50 workspace crates
- 787 Rust source files
- Rust Edition 2024
- Strict error handling (deny unwrap/expect)
- Tokio async runtime

## Getting Started

```bash
# Install dependencies
pnpm install

# Build the skill
pnpm build

# Validate the skill
pnpm validate
```

## Key Patterns

### Error Handling (CRITICAL)

The codebase enforces `deny(clippy::unwrap_used, clippy::expect_used)` at the workspace level:

- Use `thiserror` for domain-specific error types
- Use `anyhow::Result` for application entry points
- Always add context with `.context()` for debugging
- Never use `unwrap()` or `expect()` in non-test code

### Workspace Organization

- Flat workspace structure with crates at root level
- Small utilities grouped under `utils/` subdirectory
- Shared test utilities in `tests/common/` crate
- kebab-case directories with `codex-` package prefix

### Async Patterns

- `#[async_trait]` for async trait methods
- `Send + Sync + 'static` bounds for concurrent traits
- `pub(crate)` for internal APIs

### Naming Conventions

- `try_` prefix for fallible constructors
- `with_` prefix for builder methods
- Consistent suffixes: `Error`, `Client`, `Provider`, `Manager`, `Handler`
- `is_`/`has_`/`should_` prefix for boolean functions

## Creating a New Rule

1. Create a new file in `references/` with the pattern `{prefix}-{slug}.md`
2. Use the template from `assets/templates/_template.md`
3. Include YAML frontmatter with title, impact, impactDescription, and tags
4. Add Incorrect and Correct code examples
5. Run validation to ensure the rule is properly formatted

## Rule File Structure

Each rule file follows this structure:

```markdown
---
title: Rule title here
impact: HIGH|MEDIUM|LOW|CRITICAL
impactDescription: Brief explanation of why this rule matters
tags: prefix, relevant, tags
---

# Rule Title

Brief description of the rule.

## Why This Matters

- Point 1
- Point 2

**Incorrect (what not to do):**

\`\`\`rust
// Bad example
\`\`\`

**Correct (recommended pattern):**

\`\`\`rust
// Good example
\`\`\`
```

## File Naming Convention

Rule files follow the pattern: `{category-prefix}-{descriptive-slug}.md`

| Category | Prefix | Example |
|----------|--------|---------|
| Error Handling | `err-` | `err-no-unwrap.md` |
| Organization | `org-` | `org-workspace-flat.md` |
| Component Patterns | `mod-` | `mod-async-trait-macro.md` |
| Cross-Crate | `cross-` | `cross-workspace-lints.md` |
| Naming Conventions | `name-` | `name-try-prefix-fallible.md` |
| Style | `style-` | `style-import-granularity.md` |

## Impact Levels

| Level | Description | When to Use |
|-------|-------------|-------------|
| CRITICAL | Must follow - violations cause runtime errors or security issues | Error handling, panic prevention |
| HIGH | Strongly recommended - significant code quality impact | Architecture, API design |
| MEDIUM | Recommended - improves maintainability | Naming, organization |
| LOW | Nice to have - minor improvements | Formatting preferences |

## Scripts

```bash
# Validate skill structure and rules
pnpm validate

# Build AGENTS.md from individual rules
pnpm build

# Validate with strict mode (fail on warnings)
pnpm validate --strict
```

## Rules by Category

| Category | Count | Impact |
|----------|-------|--------|
| Error Handling | 11 | CRITICAL/HIGH |
| Organization | 8 | HIGH/MEDIUM |
| Component Patterns | 12 | HIGH/MEDIUM |
| Cross-Crate | 2 | HIGH |
| Naming Conventions | 15 | MEDIUM |
| Style | 6 | MEDIUM |
| **Total** | **54** | |

## Contributing

1. Fork the repository
2. Create a feature branch
3. Add or modify rules following the file structure above
4. Run `pnpm validate` to ensure rules are properly formatted
5. Submit a pull request

## Source

Analyzed from [openai/codex](https://github.com/openai/codex) on 2026-01-28.

---
name: rust-cli-agent-style
description: Coding patterns extracted from OpenAI Codex Rust codebase - a production CLI/agent system with strict error handling, async patterns, and workspace organization
---

# OpenAI Codex Rust CLI Agent Best Practices

This skill teaches you to write Rust code in the style of the OpenAI Codex codebase - a production CLI/agent system with 50 crates and 787 Rust files.

## Key Characteristics

- **Edition 2024** with strict Clippy configuration
- **Zero unwrap/expect** in non-test code (enforced at workspace level)
- **Tokio async runtime** with proper Send + Sync bounds
- **thiserror** for library errors, **anyhow** for application code
- **Flat workspace** structure with centralized dependencies

## When to Apply

Apply this skill when:
- Building CLI tools or agent systems in Rust
- Writing async Rust with Tokio
- Designing Rust workspace organization
- Implementing error handling patterns
- Working on production Rust codebases

## Quick Reference

### Critical Rules (Must Follow)

| Rule | Description |
|------|-------------|
| [err-no-unwrap](references/err-no-unwrap.md) | Never use `unwrap()` in non-test code |
| [err-no-expect](references/err-no-expect.md) | Avoid `expect()` in library code |
| [err-thiserror-domain](references/err-thiserror-domain.md) | Use thiserror for domain errors |
| [err-context-chain](references/err-context-chain.md) | Add context to errors with `.context()` |

### Error Handling

| Rule | Description |
|------|-------------|
| [err-anyhow-application](references/err-anyhow-application.md) | Use anyhow::Result for entry points |
| [err-from-derive](references/err-from-derive.md) | Use #[from] for error conversion |
| [err-transparent](references/err-transparent.md) | Use #[error(transparent)] for wrapped errors |
| [err-structured-variants](references/err-structured-variants.md) | Include relevant data in error variants |
| [err-io-result](references/err-io-result.md) | Use std::io::Result for I/O functions |
| [err-map-err-conversion](references/err-map-err-conversion.md) | Use map_err for error conversion |
| [err-doc-errors](references/err-doc-errors.md) | Document error conditions |

### Organization

| Rule | Description |
|------|-------------|
| [org-workspace-flat](references/org-workspace-flat.md) | Flat workspace with utils subdirectory |
| [org-crate-naming](references/org-crate-naming.md) | kebab-case directories, project prefix |
| [org-module-visibility](references/org-module-visibility.md) | Use pub(crate) for internal APIs |
| [org-test-common-crate](references/org-test-common-crate.md) | Shared test utilities crate |
| [org-integration-tests-suite](references/org-integration-tests-suite.md) | Tests in suite directory |
| [org-feature-modules](references/org-feature-modules.md) | Feature-based module organization |
| [org-handlers-subdir](references/org-handlers-subdir.md) | Handlers in dedicated subdirectory |
| [org-errors-file](references/org-errors-file.md) | Errors in dedicated file |

### Component Patterns

| Rule | Description |
|------|-------------|
| [mod-derive-order](references/mod-derive-order.md) | Consistent derive macro ordering |
| [mod-async-trait-macro](references/mod-async-trait-macro.md) | Use #[async_trait] for async traits |
| [mod-trait-bounds](references/mod-trait-bounds.md) | Send + Sync + 'static for concurrent traits |
| [mod-extension-trait-suffix](references/mod-extension-trait-suffix.md) | Ext suffix for extension traits |
| [mod-builder-pattern](references/mod-builder-pattern.md) | Builder pattern for complex config |
| [mod-type-alias-complex](references/mod-type-alias-complex.md) | Type aliases for complex generics |
| [mod-impl-block-order](references/mod-impl-block-order.md) | Consistent impl block ordering |
| [mod-generic-constraints](references/mod-generic-constraints.md) | Where clauses for complex bounds |
| [mod-newtype-pattern](references/mod-newtype-pattern.md) | Newtypes for type safety |
| [mod-struct-visibility](references/mod-struct-visibility.md) | Private fields with public constructor |
| [mod-serde-rename](references/mod-serde-rename.md) | Serde rename for wire format |
| [mod-jsonschema-derive](references/mod-jsonschema-derive.md) | JsonSchema for API types |

### Naming Conventions

| Rule | Description |
|------|-------------|
| [name-async-no-suffix](references/name-async-no-suffix.md) | No _async suffix for async functions |
| [name-try-prefix-fallible](references/name-try-prefix-fallible.md) | try_ prefix for fallible constructors |
| [name-with-prefix-builder](references/name-with-prefix-builder.md) | with_ prefix for builder methods |
| [name-handler-suffix](references/name-handler-suffix.md) | Handler suffix for handlers |
| [name-error-suffix](references/name-error-suffix.md) | Error suffix for error types |
| [name-result-type-alias](references/name-result-type-alias.md) | Crate-specific Result alias |
| [name-const-env-var](references/name-const-env-var.md) | _ENV_VAR suffix for env constants |
| [name-request-response](references/name-request-response.md) | Request/Response type pairing |
| [name-options-suffix](references/name-options-suffix.md) | Options suffix for config bundles |
| [name-info-suffix](references/name-info-suffix.md) | Info suffix for read-only data |
| [name-provider-suffix](references/name-provider-suffix.md) | Provider suffix for services |
| [name-client-suffix](references/name-client-suffix.md) | Client suffix for API clients |
| [name-manager-suffix](references/name-manager-suffix.md) | Manager suffix for lifecycle mgmt |
| [name-bool-is-prefix](references/name-bool-is-prefix.md) | is_/has_/should_ for booleans |
| [name-plural-collections](references/name-plural-collections.md) | Plural names for collections |

### Style

| Rule | Description |
|------|-------------|
| [style-import-granularity](references/style-import-granularity.md) | One item per use statement |
| [style-deny-stdout](references/style-deny-stdout.md) | Deny stdout/stderr in libraries |
| [style-inline-format-args](references/style-inline-format-args.md) | Inline format arguments |
| [style-module-docs](references/style-module-docs.md) | Module-level documentation |
| [style-expect-reason](references/style-expect-reason.md) | #[expect] with reason for lints |
| [style-cfg-test-module](references/style-cfg-test-module.md) | Unit tests in mod tests |

### Cross-Crate

| Rule | Description |
|------|-------------|
| [cross-workspace-lints](references/cross-workspace-lints.md) | Workspace-level lint config |
| [cross-workspace-deps](references/cross-workspace-deps.md) | Centralized dependency versions |

## Example: Proper Error Handling

```rust
use thiserror::Error;
use anyhow::Context;

// Domain error with thiserror
#[derive(Debug, Error)]
pub enum ConfigError {
    #[error("failed to read config file: {path}")]
    ReadFailed {
        path: PathBuf,
        #[source]
        source: std::io::Error,
    },

    #[error(transparent)]
    Parse(#[from] toml::de::Error),
}

// Library function returns domain error
pub fn load_config(path: &Path) -> Result<Config, ConfigError> {
    let content = fs::read_to_string(path)
        .map_err(|source| ConfigError::ReadFailed {
            path: path.to_owned(),
            source,
        })?;
    toml::from_str(&content).map_err(Into::into)
}

// Application code uses anyhow with context
fn main() -> anyhow::Result<()> {
    let config = load_config(Path::new("config.toml"))
        .context("failed to load configuration")?;
    run(config).await
}
```

## Source

Patterns extracted from [OpenAI Codex](https://github.com/openai/codex) (`codex-rs/` subdirectory) - a production Rust codebase with 50 crates and 787 Rust files.

---
title: Rule title here
impact: HIGH|MEDIUM|LOW|CRITICAL
impactDescription: Brief explanation of why this rule matters
tags: [category, relevant, tags]
---

# Rule Title

Brief description of the rule and what it enforces.

## Why This Matters

- Point 1
- Point 2
- Point 3

## Incorrect

```rust
// Code example showing what NOT to do
```

## Correct

```rust
// Code example showing the correct pattern
```

## When NOT to Use

Optional section describing exceptions or cases where the rule doesn't apply.

## Related Rules

- [related-rule-1](related-rule-1.md)
- [related-rule-2](related-rule-2.md)

{
  "name": "rust-cli-agent-style",
  "version": "1.0.0",
  "technology": "Rust CLI Agent",
  "organization": "OpenAI Codex",
  "date": "2026-01-28",
  "abstract": "Coding patterns extracted from OpenAI Codex Rust codebase - a production CLI/agent system with 50 crates, 787 Rust files, strict error handling (deny unwrap/expect), tokio async runtime, and thiserror/anyhow error patterns.",
  "description": "Coding style patterns extracted from OpenAI Codex Rust codebase - a production CLI/agent system with 50 crates and 787 Rust files",
  "source": {
    "repository": "https://github.com/openai/codex",
    "subdirectory": "codex-rs",
    "commit": "main",
    "analyzedAt": "2026-01-28",
    "fileCount": 787,
    "crateCount": 50
  },
  "language": "rust",
  "rustEdition": "2024",
  "categories": [
    "error-handling",
    "organization",
    "component",
    "naming",
    "style",
    "cross-crate"
  ],
  "ruleCount": 54,
  "keyDependencies": [
    "tokio",
    "serde",
    "thiserror",
    "anyhow",
    "clap",
    "tracing",
    "ratatui",
    "reqwest",
    "axum"
  ],
  "references": [
    "https://github.com/openai/codex"
  ],
  "tags": [
    "rust",
    "cli",
    "async",
    "tokio",
    "production",
    "agent",
    "workspace"
  ]
}

# Rule Categories

This document defines the categories used to organize rules in the rust-cli-agent-style skill.

## 1. Error Handling (err)

**Impact:** CRITICAL
**Description:** Rules for handling errors safely and consistently. The codebase enforces deny(unwrap_used, expect_used) at the workspace level.

## 2. Organization (org)

**Impact:** HIGH
**Description:** Rules for organizing workspaces, crates, modules, and files.

## 3. Component Patterns (mod)

**Impact:** HIGH
**Description:** Rules for designing structs, traits, impls, and type patterns.

## 4. Cross-Crate (cross)

**Impact:** HIGH
**Description:** Rules for workspace-level configuration and cross-crate patterns.

## 5. Naming Conventions (name)

**Impact:** MEDIUM
**Description:** Rules for naming functions, types, constants, and variables.

## 6. Style (style)

**Impact:** MEDIUM
**Description:** Rules for code formatting, imports, and documentation.

---
title: Centralize dependency versions in workspace
impact: HIGH
impactDescription: Consistent dependency versions prevent version conflicts
tags: cross, dependencies, workspace
---

# Centralize dependency versions in workspace

Define all dependency versions in `[workspace.dependencies]` section.

## Why This Matters

- Single version per dependency
- No version conflicts between crates
- Easy to update dependencies
- Clear dependency audit

**Incorrect (scattered versions):**

```toml
# In core/Cargo.toml
[dependencies]
tokio = "1.35"
serde = "1.0.193"

# In cli/Cargo.toml - different versions!
[dependencies]
tokio = "1.34"
serde = "1.0.190"
```

**Correct (centralized versions):**

```toml
# In workspace Cargo.toml
[workspace.dependencies]
tokio = { version = "1", features = ["full"] }
serde = { version = "1", features = ["derive"] }

# In core/Cargo.toml
[dependencies]
tokio.workspace = true
serde.workspace = true
```

## Full Example

Workspace Cargo.toml:

```toml
[workspace]
resolver = "2"
members = ["core", "cli", "tui", "protocol"]

[workspace.package]
version = "0.1.0"
edition = "2024"
license = "MIT"

[workspace.dependencies]
# Async runtime
tokio = { version = "1", features = ["full"] }
async-trait = "0.1"

# Serialization
serde = { version = "1", features = ["derive"] }
serde_json = "1"

# Error handling
thiserror = "2"
anyhow = "1"

# Internal crates
codex-core = { path = "core" }
codex-protocol = { path = "protocol" }
```

Crate Cargo.toml:

```toml
[package]
name = "codex-core"
version.workspace = true
edition.workspace = true

[dependencies]
tokio.workspace = true
serde.workspace = true
thiserror.workspace = true

# With additional features
reqwest = { workspace = true, features = ["cookies"] }

[dev-dependencies]
tempfile = "3"  # Only used in this crate
```

## Benefits

1. **Version consistency**: All crates use the same version
2. **Single update point**: Update once, applies everywhere
3. **Feature consistency**: Default features defined centrally
4. **Clear overview**: All dependencies visible in one place

---
title: Define common lints in workspace Cargo.toml
impact: HIGH
impactDescription: Consistent lint configuration across all workspace crates
tags: cross, lints, workspace
---

# Define common lints in workspace Cargo.toml

Use `[workspace.lints.clippy]` to enforce consistent lint rules across all crates.

## Why This Matters

- Consistent code quality across workspace
- Single source of truth for lint config
- Easy to update rules
- Prevents lint drift between crates

**Incorrect (per-crate lints):**

```toml
# In core/Cargo.toml
[lints.clippy]
unwrap_used = "deny"

# In cli/Cargo.toml - inconsistent!
[lints.clippy]
unwrap_used = "warn"  # Different level
```

**Correct (workspace lints):**

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

# In core/Cargo.toml
[lints]
workspace = true
```

## Full Example

Workspace Cargo.toml:

```toml
[workspace.lints.rust]
unsafe_code = "deny"

[workspace.lints.clippy]
# Deny panicking operations
unwrap_used = "deny"
expect_used = "deny"
panic = "deny"

# Deny common mistakes
print_stdout = "deny"
print_stderr = "deny"
dbg_macro = "deny"

# Code quality
needless_pass_by_value = "warn"
redundant_clone = "warn"
uninlined_format_args = "warn"

# Style preferences
module_name_repetitions = "allow"
```

## Crate-Level Inheritance

In each crate's Cargo.toml:

```toml
[package]
name = "codex-core"
version.workspace = true

[lints]
workspace = true
```

## Crate-Specific Overrides

When a crate needs different rules:

```toml
[package]
name = "codex-cli"

[lints]
workspace = true

[lints.clippy]
# CLI binary can print to stdout
print_stdout = "allow"
print_stderr = "allow"
```

## Test Code Exceptions

In clippy.toml:

```toml
allow-unwrap-in-tests = true
allow-expect-in-tests = true
```

---
title: Use anyhow::Result for application entry points
impact: HIGH
impactDescription: Simplifies error handling in application code while preserving error context
tags: err, anyhow, application
---

# Use anyhow::Result for application entry points

Use `anyhow::Result` in `main()` and top-level application code. Reserve `thiserror` for library/domain errors.

## Why This Matters

- `anyhow` provides easy error context chaining
- Automatic error source tracking
- Good for application code where you just want to report errors
- Separates "what went wrong" from "how to handle it"

## Pattern

```
Library code (thiserror) → Application code (anyhow) → User
```

**Incorrect (avoid this pattern):**

```rust
// Using thiserror for main() - overly complex
#[derive(Debug, Error)
enum MainError {
    #[error(transparent)
    Config(#[from] ConfigError),
    #[error(transparent)
    Server(#[from] ServerError),
    // ... many more variants
}

fn main() -> Result<(), MainError> { ... }
```

**Correct (recommended):**

```rust
use anyhow::Result;
use anyhow::Context;

fn main() -> Result<()> {
    let config = load_config()
        .context("failed to load configuration")?;

    let server = Server::new(&config)
        .context("failed to initialize server")?;

    server.run()
        .context("server error")?;

    Ok(())
}

// Or with tokio
#[tokio::main
async fn main() -> Result<()> {
    run_app().await
}

async fn run_app() -> Result<()> {
    // Application logic with anyhow for easy error chaining
}
```

## Library vs Application

| Context | Error Type | Example |
|---------|-----------|---------|
| Library crate | `thiserror` | `ConfigError`, `ParseError` |
| Application entry | `anyhow::Result` | `main()`, `run_app()` |
| CLI handlers | `anyhow::Result` | Command implementations |

---
title: Add context to errors with .context()
impact: HIGH
impactDescription: Error context chains make debugging production issues much easier
tags: err, context, anyhow, debugging
---

# Add context to errors with .context()

Use `.context()` or `.with_context()` from anyhow to add contextual information to errors. This creates a chain of context that aids debugging.

## Why This Matters

- Raw errors like "file not found" don't tell you WHICH file
- Context chains show the full path of what went wrong
- Invaluable for debugging production issues
- Creates human-readable error messages

**Incorrect (avoid this pattern):**

```rust
// No context - hard to debug
let data = fs::read_to_string(path)?;
let config: Config = toml::from_str(&data)?;
let response = client.get(url).send().await?;
```

**Correct (recommended):**

```rust
use anyhow::Context;

// Add context describing what operation failed
let data = fs::read_to_string(path)
    .context("failed to read config file")?;

let config: Config = toml::from_str(&data)
    .context("failed to parse config as TOML")?;

// Use with_context for dynamic messages (avoids allocation when no error)
let response = client.get(&url).send().await
    .with_context(|| format!("failed to fetch {url}"))?;

// Include relevant variables in context
let user = find_user(user_id)
    .with_context(|| format!("failed to find user {user_id}"))?;
```

## Error Output Example

With proper context:
```
Error: failed to initialize server

Caused by:
    0: failed to load configuration
    1: failed to read config file
    2: No such file or directory (os error 2)
```

Without context:
```
Error: No such file or directory (os error 2)
```

---
title: Document error conditions with # Errors
impact: MEDIUM
impactDescription: Clear documentation helps users handle errors correctly
tags: err, documentation, api
---

# Document error conditions with # Errors

Public functions returning `Result` should document possible errors in a `# Errors` section.

## Why This Matters

- Users know what errors to expect
- Enables proper error handling by callers
- Part of the public API contract
- Helps during code review

**Incorrect (avoid this pattern):**

```rust
/// Loads configuration from the given path.
pub fn load_config(path: &Path) -> Result<Config, ConfigError> {
    // ...
}
// Missing error documentation!
```

**Correct (recommended):**

```rust
/// Loads configuration from the given path.
///
/// # Errors
///
/// Returns an error if:
/// - The file does not exist or cannot be read
/// - The file contents are not valid TOML
/// - Required configuration fields are missing
pub fn load_config(path: &Path) -> Result<Config, ConfigError> {
    // ...
}

/// Connects to the server at the given address.
///
/// # Errors
///
/// Returns [`ConnectionError::Timeout`] if the connection
/// cannot be established within 30 seconds.
///
/// Returns [`ConnectionError::AuthFailed`] if the provided
/// credentials are invalid.
pub async fn connect(addr: &str, creds: &Credentials) -> Result<Connection, ConnectionError> {
    // ...
}

/// Parses the command string into structured arguments.
///
/// # Errors
///
/// Returns an error if the command contains invalid syntax,
/// such as unmatched quotes or invalid escape sequences.
pub fn parse_command(cmd: &str) -> Result<ParsedCommand, ParseError> {
    // ...
}
```

## Additional Sections

Also consider documenting:
- `# Panics` - if the function can panic (should be rare with proper error handling)
- `# Safety` - for unsafe functions

---
title: Use #[from] for automatic error conversion
impact: MEDIUM
impactDescription: Reduces boilerplate for common error conversions
tags: err, thiserror, from
---

# Use #[from] for automatic error conversion

Apply the `#[from]` attribute to error variants for automatic `From` trait implementation. This enables using `?` operator seamlessly.

## Why This Matters

- Eliminates manual `From` impl boilerplate
- Makes `?` operator work automatically
- Cleaner error propagation
- Compile-time guaranteed conversions

**Incorrect (avoid this pattern):**

```rust
#[derive(Debug, Error)
pub enum MyError {
    #[error("IO error: {0}")
    Io(std::io::Error),
}

// Manual From impl - verbose
impl From<std::io::Error> for MyError {
    fn from(e: std::io::Error) -> Self {
        MyError::Io(e)
    }
}
```

**Correct (recommended):**

```rust
use thiserror::Error;

#[derive(Debug, Error)
pub enum MyError {
    #[error(transparent)
    Io(#[from] std::io::Error),

    #[error(transparent)
    Json(#[from] serde_json::Error),

    #[error("request failed")
    Request(#[from] reqwest::Error),
}

// Now ? operator works automatically
fn load_data(path: &Path) -> Result<Data, MyError> {
    let content = fs::read_to_string(path)?; // auto-converts io::Error
    let data = serde_json::from_str(&content)?; // auto-converts serde_json::Error
    Ok(data)
}
```

## With #[error(transparent)

Use `#[error(transparent)]` when the wrapped error's message is sufficient:

```rust
#[error(transparent)
Io(#[from] std::io::Error),
// Display shows: "No such file or directory (os error 2)"

#[error("failed to parse config")
Parse(#[from] toml::de::Error),
// Display shows: "failed to parse config"
```

---
title: Use std::io::Result for I/O functions
impact: MEDIUM
impactDescription: Standard I/O result type for consistency with ecosystem
tags: err, io, result
---

# Use std::io::Result for I/O functions

Functions primarily doing I/O operations should return `std::io::Result` for consistency with the Rust ecosystem.

## Why This Matters

- Standard type that all Rust I/O code expects
- Easy composition with other I/O functions
- Well-understood error semantics
- Interoperates with `?` operator

**Incorrect (avoid this pattern):**

```rust
// Custom error for simple I/O
fn read_file(path: &Path) -> Result<String, MyError> {
    fs::read_to_string(path).map_err(MyError::Io)
}

// Stringly-typed errors
fn write_data(path: &Path, data: &[u8]) -> Result<(), String> {
    fs::write(path, data).map_err(|e| e.to_string())
}
```

**Correct (recommended):**

```rust
use std::io::{self, Read, Write};

// Pure I/O functions use std::io::Result
fn read_file(path: &Path) -> io::Result<String> {
    fs::read_to_string(path)
}

fn write_data(path: &Path, data: &[u8]) -> io::Result<()> {
    fs::write(path, data)
}

// I/O with additional processing can still use io::Result
fn copy_with_transform<R: Read, W: Write>(
    reader: &mut R,
    writer: &mut W,
    transform: impl Fn(&[u8]) -> Vec<u8>,
) -> io::Result<u64> {
    let mut buf = [0u8; 8192];
    let mut total = 0u64;
    loop {
        let n = reader.read(&mut buf)?;
        if n == 0 { break; }
        let transformed = transform(&buf[..n]);
        writer.write_all(&transformed)?;
        total += n as u64;
    }
    Ok(total)
}
```

## When to Use Domain Error Instead

When the function does more than I/O:

```rust
// This does I/O AND parsing, so domain error is appropriate
fn load_config(path: &Path) -> Result<Config, ConfigError> {
    let content = fs::read_to_string(path)
        .map_err(|e| ConfigError::ReadFailed { source: e })?;
    toml::from_str(&content)
        .map_err(ConfigError::ParseFailed)
}
```

---
title: Use map_err for error type conversion
impact: MEDIUM
impactDescription: Explicit error conversion improves code clarity and control
tags: err, map_err, conversion
---

# Use map_err for error type conversion

Convert between error types using `map_err` with a closure when `#[from]` automatic conversion isn't appropriate.

## Why This Matters

- Explicit conversion is clearer than implicit
- Allows adding context during conversion
- Works when automatic `From` isn't available
- Enables error type narrowing

**Incorrect (letting errors propagate without conversion):**

```rust
// Error type mismatch - won't compile without conversion
fn parse_config(s: &str) -> Result<Config, ConfigError> {
    let value: i32 = s.parse()?;  // ParseIntError != ConfigError
    Ok(Config { value })
}
```

**Correct (explicit conversion with map_err):**

```rust
fn parse_config(s: &str) -> Result<Config, ConfigError> {
    let value: i32 = s.parse()
        .map_err(|_| ConfigError::InvalidValue(s.to_string()))?;
    Ok(Config { value })
}

// With context
fn parse_port(s: &str) -> Result<u16, ConfigError> {
    s.parse::<u16>()
        .map_err(|_| ConfigError::InvalidPort(s.to_string()))
}

// Creating io::Error from other errors
fn custom_io_operation() -> std::io::Result<()> {
    some_fallible_op()
        .map_err(|e| std::io::Error::other(format!("operation failed: {e}")))?;
    Ok(())
}
```

## Common Patterns

```rust
// String to error
.map_err(|e| Error::new(e.to_string()))

// Wrap with context
.map_err(|e| Error::Context { message: "loading config", source: e })

// Discard error details
.map_err(|_| Error::Failed)

// Convert to io::Error
.map_err(|e| io::Error::other(e))
```

## When to Use #[from] Instead

Use `#[from]` when the conversion is always appropriate and no additional context is needed.

---
title: Avoid expect() in library code
impact: CRITICAL
impactDescription: Library code should never panic - callers cannot recover from panics
tags: err, expect, panic, library
---

# Avoid expect() in library code

The codebase enforces `#![deny(clippy::expect_used)]` at the workspace level. Library code should return `Result` instead of panicking with `expect()`.

## Why This Matters

- Library consumers cannot catch panics easily
- Panics bypass error handling code paths
- `Result` types allow callers to decide how to handle errors
- Better composability with other error-returning functions

**Incorrect (avoid this pattern):**

```rust
// In library code
pub fn load_config(path: &Path) -> Config {
    let content = fs::read_to_string(path)
        .expect("config file must exist");
    toml::from_str(&content)
        .expect("config must be valid TOML")
}
```

**Correct (recommended):**

```rust
// Return Result instead
pub fn load_config(path: &Path) -> Result<Config, ConfigError> {
    let content = fs::read_to_string(path)
        .map_err(|e| ConfigError::ReadFailed { path: path.to_owned(), source: e })?;
    toml::from_str(&content)
        .map_err(ConfigError::ParseFailed)
}

// If expect is truly necessary (static data, etc.), use #[expect] with reason
#[expect(clippy::expect_used, reason = "static regex is always valid")
static PATTERN: Lazy<Regex> = Lazy::new(|| {
    Regex::new(r"^\d+$").expect("static regex")
});
```

## When expect() Might Be Acceptable

Only in truly infallible situations:
- Static/const data initialization
- After explicit validation
- In binary entry points (main)

Even then, prefer returning `Result` when possible.

---
title: Never use unwrap() in non-test code
impact: CRITICAL
impactDescription: Panics in production code cause crashes and poor user experience
tags: err, unwrap, panic
---

# Never use unwrap() in non-test code

The codebase enforces `#![deny(clippy::unwrap_used)]` at the workspace level. Use proper error handling instead of `unwrap()`.

## Why This Matters

- `unwrap()` panics on `None`/`Err`, crashing the program
- No recovery possible from panics
- Production code must handle all error cases gracefully
- Stack traces from panics are not user-friendly

**Incorrect (avoid this pattern):**

```rust
let value = option.unwrap();
let data = result.unwrap();
let first = vec.first().unwrap();
```

**Correct (recommended):**

```rust
// Option: return error or use default
let value = option.ok_or_else(|| Error::MissingValue)?;
let value = option.unwrap_or_default();
let value = option.unwrap_or(fallback);

// Result: propagate with ?
let data = result?;

// Use if-let for conditional handling
if let Some(first) = vec.first() {
    process(first);
}

// Use expect() only when the invariant is truly guaranteed
// (but prefer returning Result even then)
let value = option.expect("value is always set after init");
```

## Test Code Exception

In test code, `unwrap()` is allowed via `allow_unwrap_in_tests = true` in clippy.toml:

```rust
#[cfg(test)
mod tests {
    #[test
    fn test_something() {
        let result = do_thing().unwrap(); // OK in tests
        assert_eq!(result, expected);
    }
}
```

---
title: Use structured error variants with fields
impact: HIGH
impactDescription: Structured errors enable better debugging and programmatic error handling
tags: err, thiserror, debugging
---

# Use structured error variants with fields

Include relevant data in error variants for debugging. Named fields are preferred over positional.

## Why This Matters

- Provides context for debugging production issues
- Enables programmatic error handling
- Error messages include relevant data
- Pattern matching can extract error details

**Incorrect (avoid this pattern):**

```rust
#[derive(Debug, Error)
pub enum GitError {
    #[error("git command failed")
    CommandFailed,  // No details!

    #[error("branch not found")
    BranchNotFound,  // Which branch?

    #[error("merge conflict")
    MergeConflict,  // Which files?
}
```

**Correct (recommended):**

```rust
#[derive(Debug, Error)
pub enum GitError {
    #[error("git command `{command}` failed: {stderr}")
    CommandFailed {
        command: String,
        stderr: String,
        exit_code: i32,
    },

    #[error("branch `{branch}` not found in repository")
    BranchNotFound { branch: String },

    #[error("merge conflict in {file_count} files")
    MergeConflict {
        file_count: usize,
        files: Vec<PathBuf>,
    },

    #[error("failed to clone {url}: {reason}")
    CloneFailed { url: String, reason: String },
}

// Usage
fn checkout_branch(branch: &str) -> Result<(), GitError> {
    // ...
    Err(GitError::BranchNotFound {
        branch: branch.to_string(),
    })
}

// Pattern matching
match error {
    GitError::CommandFailed { exit_code, .. } if exit_code == 128 => {
        // Handle specific exit code
    }
    _ => {}
}
```

---
title: Use thiserror for domain error types
impact: CRITICAL
impactDescription: Consistent error types enable proper error propagation and debugging across the codebase
tags: err, thiserror, domain
---

# Use thiserror for domain error types

Define domain-specific error types using the `thiserror` derive macro. This provides automatic `Display` and `Error` trait implementations with structured error variants.

## Why This Matters

- Compile-time checked error messages
- Automatic `From` implementations with `#[from]`
- Structured error variants enable pattern matching
- Clear error messages for debugging

**Incorrect (avoid this pattern):**

```rust
// String-based errors lose type information
struct MyError(String);

impl std::fmt::Display for MyError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "{}", self.0)
    }
}

// Or using anyhow for domain errors
fn parse_config() -> anyhow::Result<Config> {
    // anyhow is for application code, not library code
}
```

**Correct (recommended):**

```rust
use thiserror::Error;

#[derive(Debug, Error)
pub enum ConfigError {
    #[error("failed to read config file: {path}")
    ReadFailed {
        path: PathBuf,
        #[source
        source: std::io::Error,
    },

    #[error("invalid config format: {0}")
    InvalidFormat(String),

    #[error(transparent)
    Io(#[from] std::io::Error),
}

fn parse_config(path: &Path) -> Result<Config, ConfigError> {
    let content = fs::read_to_string(path)
        .map_err(|source| ConfigError::ReadFailed {
            path: path.to_owned(),
            source,
        })?;
    // ...
}
```

---
title: Use #[error(transparent)] for wrapped errors
impact: MEDIUM
impactDescription: Preserves original error messages when wrapping without additional context
tags: err, thiserror, transparent
---

# Use #[error(transparent)] for wrapped errors

Use the `transparent` attribute when wrapping errors without adding additional context. The wrapped error's Display impl is used directly.

## Why This Matters

- Avoids redundant error messages
- Original error details preserved
- Cleaner error output
- Proper source chain maintained

**Incorrect (avoid this pattern):**

```rust
#[derive(Debug, Error)
pub enum MyError {
    #[error("IO error: {0}")]  // Redundant - io::Error already says "IO error"
    Io(#[from] std::io::Error),

    #[error("JSON error: {0}")]  // Adds noise
    Json(#[from] serde_json::Error),
}
// Output: "IO error: No such file or directory (os error 2)"
```

**Correct (recommended):**

```rust
#[derive(Debug, Error)
pub enum MyError {
    #[error(transparent)
    Io(#[from] std::io::Error),

    #[error(transparent)
    Json(#[from] serde_json::Error),
}
// Output: "No such file or directory (os error 2)"
```

## When NOT to Use Transparent

Add context when the error needs clarification:

```rust
#[derive(Debug, Error)
pub enum ConfigError {
    // transparent - the io::Error is self-explanatory
    #[error(transparent)
    Io(#[from] std::io::Error),

    // NOT transparent - add context about what failed
    #[error("failed to parse config file")
    Parse(#[source] toml::de::Error),

    // Structured variant with context
    #[error("invalid value for {field}: {message}")
    InvalidField { field: String, message: String },
}
```

---
title: Use #[async_trait] for async trait methods
impact: HIGH
impactDescription: Enables async methods in traits which Rust doesn't natively support yet
tags: mod, async, traits
---

# Use #[async_trait] for async trait methods

Apply the `#[async_trait]` attribute when defining traits with async methods.

## Why This Matters

- Rust doesn't have native async trait support yet
- `async_trait` macro provides the workaround
- Required for async trait methods to compile
- Widely used in async Rust ecosystem

**Incorrect (avoid this pattern):**

```rust
// Won't compile - async fn not allowed in traits (without RPITIT)
pub trait Handler {
    async fn handle(&self, request: Request) -> Response;
}
```

**Correct (recommended):**

```rust
use async_trait::async_trait;

#[async_trait
pub trait Handler {
    async fn handle(&self, request: Request) -> Response;
}

#[async_trait
impl Handler for MyHandler {
    async fn handle(&self, request: Request) -> Response {
        // Implementation
    }
}
```

## With Send Bound (Default)

The `#[async_trait]` macro adds `Send` bound by default:

```rust
#[async_trait
pub trait Transport: Send + Sync {
    async fn send(&self, data: &[u8]) -> io::Result<()>;
    async fn recv(&self) -> io::Result<Vec<u8>>;
}
```

## Without Send Bound

For single-threaded contexts:

```rust
#[async_trait(?Send)
pub trait LocalHandler {
    async fn handle(&self) -> Result<()>;
}
```

## Note on Rust 1.75+

Rust 1.75+ supports async fn in traits via RPITIT for some cases. However, `#[async_trait]` is still commonly used for:
- Object safety (`dyn Trait`)
- Complex trait bounds
- Compatibility with older code

---
title: Use builder pattern for complex configuration
impact: MEDIUM
impactDescription: Builders make construction of complex types readable and flexible
tags: mod, builder, construction
---

# Use builder pattern for complex configuration

Implement the builder pattern with `Default` for types with many optional fields.

## Why This Matters

- Clear, readable construction
- Optional fields without Option<> overload
- Compile-time checked required fields
- Chainable API

**Incorrect (avoid this pattern):**

```rust
// Too many constructor parameters
let config = Config::new(
    "localhost",
    8080,
    true,
    Some(Duration::from_secs(30)),
    None,
    true,
    false,
);
```

**Correct (recommended):**

```rust
#[derive(Debug, Clone)
pub struct Config {
    host: String,
    port: u16,
    tls_enabled: bool,
    timeout: Duration,
    max_retries: u32,
}

#[derive(Debug, Clone, Default)
pub struct ConfigBuilder {
    host: Option<String>,
    port: Option<u16>,
    tls_enabled: bool,
    timeout: Duration,
    max_retries: u32,
}

impl ConfigBuilder {
    pub fn new() -> Self {
        Self::default()
    }

    pub fn with_host(mut self, host: impl Into<String>) -> Self {
        self.host = Some(host.into());
        self
    }

    pub fn with_port(mut self, port: u16) -> Self {
        self.port = Some(port);
        self
    }

    pub fn with_tls(mut self, enabled: bool) -> Self {
        self.tls_enabled = enabled;
        self
    }

    pub fn with_timeout(mut self, timeout: Duration) -> Self {
        self.timeout = timeout;
        self
    }

    pub fn build(self) -> Result<Config, ConfigError> {
        Ok(Config {
            host: self.host.ok_or(ConfigError::MissingField("host"))?,
            port: self.port.unwrap_or(8080),
            tls_enabled: self.tls_enabled,
            timeout: self.timeout,
            max_retries: self.max_retries,
        })
    }
}

// Usage
let config = ConfigBuilder::new()
    .with_host("localhost")
    .with_port(3000)
    .with_tls(true)
    .with_timeout(Duration::from_secs(60))
    .build()?;
```

## Default Trait Integration

```rust
impl Default for ConfigBuilder {
    fn default() -> Self {
        Self {
            host: None,
            port: None,
            tls_enabled: false,
            timeout: Duration::from_secs(30),
            max_retries: 3,
        }
    }
}
```

---
title: Order derive macros consistently
impact: MEDIUM
impactDescription: Consistent derive ordering improves code readability and diff quality
tags: mod, derive, formatting
---

# Order derive macros consistently

Order derive macros in a consistent sequence: Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, then others alphabetically.

## Why This Matters

- Predictable code structure
- Easier to scan for specific traits
- Cleaner diffs when adding derives
- Team consistency

## Standard Order

1. `Debug` - always first (essential for development)
2. `Clone` / `Copy` - value semantics
3. `PartialEq` / `Eq` - equality
4. `PartialOrd` / `Ord` - ordering
5. `Hash` - hashing
6. `Default` - default construction
7. `Serialize` / `Deserialize` - serde
8. Others alphabetically

**Incorrect (avoid this pattern):**

```rust
#[derive(Serialize, Debug, Clone)
pub struct Config { }

#[derive(PartialEq, Debug, Deserialize, Clone, Serialize)
pub struct Message { }
```

**Correct (recommended):**

```rust
#[derive(Debug, Clone, Serialize)
pub struct Config { }

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)
pub struct Message { }

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)
pub struct Id(u64);

#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)
pub struct Options {
    pub timeout: Option<Duration>,
    pub retry: bool,
}
```

## With Additional Macros

```rust
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)
#[serde(rename_all = "camelCase")
pub struct ApiRequest {
    pub request_id: String,
    pub payload: Value,
}
```

---
title: Use Ext suffix for extension traits
impact: MEDIUM
impactDescription: Clear naming distinguishes extension traits from core traits
tags: mod, traits, naming
---

# Use Ext suffix for extension traits

Name extension traits with the `Ext` suffix to distinguish them from core traits.

## Why This Matters

- Clear indication of extension trait
- Conventional in Rust ecosystem
- Easy to find extension methods
- Distinguishes from main trait

**Incorrect (avoid this pattern):**

```rust
// Ambiguous - is this the main trait or extension?
pub trait OrCancel {
    fn or_cancel(&self) -> Result<()>;
}

pub trait RectHelpers {
    fn center(&self) -> Point;
}
```

**Correct (recommended):**

```rust
// Clear extension trait naming
pub trait OrCancelExt {
    fn or_cancel(self, token: CancellationToken) -> OrCancel<Self>
    where
        Self: Sized;
}

pub trait RectExt {
    fn center(&self) -> Point;
    fn contains_point(&self, point: Point) -> bool;
}

pub trait StreamExt {
    fn aggregate(self) -> AggregateStream<Self>
    where
        Self: Sized;
}
```

## Implementation Pattern

```rust
// Extension trait for existing type
pub trait RectExt {
    fn center(&self) -> Point;
}

impl RectExt for Rect {
    fn center(&self) -> Point {
        Point {
            x: self.x + self.width / 2,
            y: self.y + self.height / 2,
        }
    }
}

// Usage requires importing the extension trait
use crate::RectExt;

let rect = Rect::new(0, 0, 100, 100);
let center = rect.center();  // Extension method
```

## Common Patterns

- `FutureExt` - extensions for `Future`
- `StreamExt` - extensions for `Stream`
- `IteratorExt` - extensions for `Iterator`
- `ResultExt` - extensions for `Result`
- `OptionExt` - extensions for `Option`

---
title: Use where clauses for complex bounds
impact: MEDIUM
impactDescription: Where clauses improve readability of complex generic constraints
tags: mod, generics, constraints
---

# Use where clauses for complex bounds

Move complex generic bounds to where clauses for readability.

## Why This Matters

- Function signatures stay clean
- Complex bounds are readable
- Easier to modify constraints
- Better formatted by rustfmt

**Incorrect (avoid this pattern):**

```rust
// Hard to read - bounds inline
fn process<T: Debug + Clone + Send + Sync + 'static, E: Error + Send>(
    data: T,
    handler: impl Fn(T) -> Result<(), E>,
) -> Result<(), E> {
    // ...
}

// Long struct definition
pub struct Handler<T: Clone + Send + Sync + 'static, S: AsyncRead + AsyncWrite + Unpin> {
    data: T,
    stream: S,
}
```

**Correct (recommended):**

```rust
// Clean signature with where clause
fn process<T, E>(
    data: T,
    handler: impl Fn(T) -> Result<(), E>,
) -> Result<(), E>
where
    T: Debug + Clone + Send + Sync + 'static,
    E: Error + Send,
{
    // ...
}

// Struct with where clause
pub struct Handler<T, S>
where
    T: Clone + Send + Sync + 'static,
    S: AsyncRead + AsyncWrite + Unpin,
{
    data: T,
    stream: S,
}

impl<T, S> Handler<T, S>
where
    T: Clone + Send + Sync + 'static,
    S: AsyncRead + AsyncWrite + Unpin,
{
    pub fn new(data: T, stream: S) -> Self {
        Self { data, stream }
    }
}
```

## When to Use Inline vs Where

**Inline bounds** (short):
```rust
fn clone_it<T: Clone>(x: T) -> T { x.clone() }
```

**Where clause** (complex):
```rust
fn complex<T, U>(x: T, y: U)
where
    T: Clone + Debug + Send,
    U: From<T> + Default,
{
    // ...
}
```

---
title: Order impl blocks consistently
impact: LOW
impactDescription: Consistent ordering makes code navigation predictable
tags: mod, impl, ordering
---

# Order impl blocks consistently

Order impl blocks: inherent impl first, then trait impls (std traits first), then From/Into impls.

## Why This Matters

- Predictable code structure
- Easy to find specific implementations
- Consistent across codebase
- Follows community conventions

**Incorrect (random ordering):**

```rust
impl From<String> for Config {
    fn from(host: String) -> Self { Self::new(host) }
}

impl Config {
    pub fn new(host: impl Into<String>) -> Self { /* ... */ }
}

impl Default for Config {
    fn default() -> Self { Self::new("localhost") }
}
```

**Correct (consistent ordering):**

```rust
pub struct Config {
    pub timeout: Duration,
    pub host: String,
}

// 1. Inherent impl first
impl Config {
    pub fn new(host: impl Into<String>) -> Self {
        Self {
            timeout: Duration::from_secs(30),
            host: host.into(),
        }
    }

    pub fn with_timeout(mut self, timeout: Duration) -> Self {
        self.timeout = timeout;
        self
    }
}

// 2. Standard library traits
impl Default for Config {
    fn default() -> Self {
        Self::new("localhost")
    }
}

impl fmt::Display for Config {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "{}:{}", self.host, self.timeout.as_secs())
    }
}

// 3. From/Into conversions
impl From<String> for Config {
    fn from(host: String) -> Self {
        Self::new(host)
    }
}

// 4. Crate traits
impl crate::Validate for Config {
    fn validate(&self) -> Result<(), ValidationError> {
        // ...
    }
}
```

---
title: Derive JsonSchema for API types
impact: MEDIUM
impactDescription: Schema generation enables automatic API documentation and validation
tags: mod, jsonschema, api, documentation
---

# Derive JsonSchema for API types

Types used in API definitions should derive `JsonSchema` for documentation.

## Why This Matters

- Automatic OpenAPI/Swagger generation
- Client SDK generation
- Request/response validation
- Self-documenting APIs

**Incorrect (no schema derivation):**

```rust
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct CreateUserRequest {
    pub email: String,
    pub role: UserRole,
}
// No schema available for documentation or validation
```

**Correct (with JsonSchema):**

```rust
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct CreateUserRequest {
    /// The user's email address
    pub email: String,

    /// Display name (optional)
    #[serde(skip_serializing_if = "Option::is_none")]
    pub display_name: Option<String>,

    /// User role
    pub role: UserRole,
}

#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "SCREAMING_SNAKE_CASE")]
pub enum UserRole {
    Admin,
    Member,
    Guest,
}
```

## Generating Schema

```rust
use schemars::schema_for;

fn main() {
    let schema = schema_for!(CreateUserRequest);
    println!("{}", serde_json::to_string_pretty(&schema).unwrap());
}
```

## With Validation Hints

```rust
#[derive(JsonSchema)]
pub struct Config {
    /// Port number (1-65535)
    #[schemars(range(min = 1, max = 65535))]
    pub port: u16,

    /// Timeout in seconds
    #[schemars(range(min = 1))]
    pub timeout_secs: u32,
}
```

---
title: Use newtype pattern for type safety
impact: HIGH
impactDescription: Newtypes prevent mixing up values with the same underlying type
tags: mod, newtype, types, safety
---

# Use newtype pattern for type safety

Wrap primitive types in newtypes for domain-specific meaning.

## Why This Matters

- Compiler catches mixing up IDs
- Self-documenting code
- Type-safe function signatures
- Can add domain-specific methods

**Incorrect (avoid this pattern):**

```rust
// Easy to mix up - both are u64!
fn process_user(user_id: u64, org_id: u64) { }

// Oops - swapped arguments
process_user(org_id, user_id);  // Compiles but wrong!

// String IDs are even worse
fn find_user(user_id: &str, session_id: &str) { }
```

**Correct (recommended):**

```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)
pub struct UserId(pub u64);

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)
pub struct OrgId(pub u64);

#[derive(Debug, Clone, PartialEq, Eq, Hash)
pub struct SessionId(pub String);

// Type-safe signature
fn process_user(user_id: UserId, org_id: OrgId) { }

// Compiler error if swapped!
process_user(org_id, user_id);  // Error: expected UserId, found OrgId
```

## Implementing Newtypes

```rust
use serde::{Deserialize, Serialize};

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)
#[serde(transparent)
pub struct RequestId(pub u64);

impl RequestId {
    pub fn new() -> Self {
        static COUNTER: AtomicU64 = AtomicU64::new(0);
        Self(COUNTER.fetch_add(1, Ordering::Relaxed))
    }
}

impl fmt::Display for RequestId {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "req-{}", self.0)
    }
}
```

## Common Newtypes

- `UserId`, `OrgId`, `ProjectId` - entity identifiers
- `RequestId`, `TraceId` - tracing identifiers
- `Port`, `Timeout` - configuration values
- `Bytes`, `Count` - units of measure

---
title: Use serde rename for wire format compatibility
impact: HIGH
impactDescription: Proper serde configuration ensures API compatibility
tags: mod, serde, api, serialization
---

# Use serde rename for wire format compatibility

Apply `#[serde(rename = "...")]` or `#[serde(rename_all = "camelCase")]` for JSON APIs.

## Why This Matters

- Rust uses snake_case, JSON typically uses camelCase
- API contracts must be stable
- Self-documenting serialization format
- Avoids runtime surprises

**Incorrect (avoid this pattern):**

```rust
#[derive(Serialize, Deserialize)
pub struct ApiResponse {
    pub user_id: String,      // Serializes as "user_id"
    pub created_at: DateTime, // Serializes as "created_at"
}
// JSON: {"user_id": "...", "created_at": "..."}
// But API expects: {"userId": "...", "createdAt": "..."}
```

**Correct (recommended):**

```rust
#[derive(Debug, Clone, Serialize, Deserialize)
#[serde(rename_all = "camelCase")
pub struct ApiResponse {
    pub user_id: String,      // Serializes as "userId"
    pub created_at: DateTime, // Serializes as "createdAt"
    pub is_active: bool,      // Serializes as "isActive"
}
// JSON: {"userId": "...", "createdAt": "...", "isActive": true}
```

## Individual Field Rename

```rust
#[derive(Debug, Clone, Serialize, Deserialize)
#[serde(rename_all = "camelCase")
pub struct Message {
    pub message_id: String,

    #[serde(rename = "type")]  // "type" is a Rust keyword
    pub message_type: MessageType,

    #[serde(rename = "ID")]    // Custom casing
    pub external_id: String,
}
```

## Common Patterns

```rust
// Skip serializing None values
#[serde(skip_serializing_if = "Option::is_none")
pub optional_field: Option<String>,

// Default value on deserialize
#[serde(default)
pub count: u32,

// Flatten nested struct
#[serde(flatten)
pub metadata: Metadata,

// Enum variants as strings
#[derive(Serialize, Deserialize)
#[serde(rename_all = "SCREAMING_SNAKE_CASE")
pub enum Status {
    InProgress,  // "IN_PROGRESS"
    Completed,   // "COMPLETED"
}
```

---
title: Default to private fields with public constructor
impact: MEDIUM
impactDescription: Encapsulation enables future changes without breaking API
tags: mod, visibility, encapsulation
---

# Default to private fields with public constructor

Keep struct fields private by default; provide `new()` or builder for construction.

## Why This Matters

- Internal representation can change
- Invariants can be enforced
- Validation happens at construction
- Clear API boundary

**Incorrect (avoid this pattern):**

```rust
// Public fields expose implementation
pub struct Config {
    pub host: String,
    pub port: u16,
    pub timeout_ms: u64,  // Implementation detail exposed
}

// Consumers can create invalid state
let config = Config {
    host: String::new(),  // Empty host - invalid!
    port: 0,              // Port 0 - invalid!
    timeout_ms: 0,
};
```

**Correct (recommended):**

```rust
pub struct Config {
    host: String,
    port: u16,
    timeout: Duration,
}

impl Config {
    /// Creates a new configuration.
    ///
    /// # Errors
    /// Returns error if host is empty or port is 0.
    pub fn new(host: impl Into<String>, port: u16) -> Result<Self, ConfigError> {
        let host = host.into();
        if host.is_empty() {
            return Err(ConfigError::EmptyHost);
        }
        if port == 0 {
            return Err(ConfigError::InvalidPort);
        }
        Ok(Self {
            host,
            port,
            timeout: Duration::from_secs(30),
        })
    }

    // Getters for read access
    pub fn host(&self) -> &str {
        &self.host
    }

    pub fn port(&self) -> u16 {
        self.port
    }

    pub fn timeout(&self) -> Duration {
        self.timeout
    }

    // Builder-style setters that validate
    pub fn with_timeout(mut self, timeout: Duration) -> Self {
        self.timeout = timeout;
        self
    }
}
```

## When Public Fields Are OK

- Simple data transfer objects (DTOs)
- Builder intermediate state
- Test-only types
- Tuple structs: `pub struct Id(pub u64);`

---
title: Include Send + Sync + 'static for concurrent traits
impact: HIGH
impactDescription: Proper bounds enable traits to work in async and multithreaded contexts
tags: mod, traits, concurrency
---

# Include Send + Sync + 'static for concurrent traits

Traits used in async/concurrent contexts should require `Send + Sync + 'static` bounds.

## Why This Matters

- Tokio requires `Send` for spawned tasks
- `Sync` enables sharing references across threads
- `'static` required for spawned futures
- Without these, trait objects won't work in async code

**Incorrect (avoid this pattern):**

```rust
// Missing bounds - won't work with tokio::spawn
pub trait Handler {
    fn handle(&self, req: Request) -> Response;
}

async fn process(handler: Arc<dyn Handler>) {
    tokio::spawn(async move {
        handler.handle(req)  // Error: Handler is not Send + Sync
    });
}
```

**Correct (recommended):**

```rust
pub trait Handler: Send + Sync + 'static {
    fn handle(&self, req: Request) -> Response;
}

async fn process(handler: Arc<dyn Handler>) {
    tokio::spawn(async move {
        handler.handle(req)  // Works!
    });
}
```

## Common Patterns

```rust
// Async handler with proper bounds
#[async_trait
pub trait AsyncHandler: Send + Sync + 'static {
    async fn handle(&self, req: Request) -> Response;
}

// Generic with bounds
pub fn spawn_handler<H>(handler: H)
where
    H: Handler + Send + Sync + 'static,
{
    tokio::spawn(async move {
        handler.handle(req).await
    });
}

// Trait object type alias
pub type DynHandler = Arc<dyn Handler + Send + Sync>;
```

## When NOT to Add These Bounds

- Single-threaded contexts (`?Send`)
- Stack-only usage (no spawning)
- Deliberately !Send types (containing `Rc`, etc.)

---
title: Create type aliases for complex generic types
impact: MEDIUM
impactDescription: Type aliases improve readability of complex nested generics
tags: mod, types, aliases
---

# Create type aliases for complex generic types

Define type aliases for nested generics to improve readability.

## Why This Matters

- Complex types become readable
- Single point of change
- Self-documenting code
- Reduces cognitive load

**Incorrect (avoid this pattern):**

```rust
// Hard to read and error-prone
fn get_pending() -> Arc<Mutex<HashMap<ThreadId, VecDeque<Event>>>> {
    // ...
}

fn process(
    data: Arc<RwLock<HashMap<String, Vec<Box<dyn Handler + Send + Sync>>>>>,
) {
    // ...
}
```

**Correct (recommended):**

```rust
// Clear type aliases
pub(crate) type PendingEvents = Arc<Mutex<HashMap<ThreadId, VecDeque<Event>>>>;
pub(crate) type HandlerRegistry = Arc<RwLock<HashMap<String, Vec<BoxedHandler>>>>;
pub type BoxedHandler = Box<dyn Handler + Send + Sync>;

fn get_pending() -> PendingEvents {
    // ...
}

fn process(handlers: HandlerRegistry) {
    // ...
}
```

## Common Patterns

```rust
// Result aliases
pub type Result<T> = std::result::Result<T, Error>;
pub type IoResult<T> = std::io::Result<T>;

// Callback types
pub type Callback = Box<dyn Fn(Event) + Send + Sync>;
pub type AsyncCallback = Box<dyn Fn(Event) -> BoxFuture<'static, ()> + Send + Sync>;

// Collection types
pub type EventQueue = VecDeque<Event>;
pub type HandlerMap = HashMap<String, BoxedHandler>;

// Pinned futures
pub type BoxFuture<'a, T> = Pin<Box<dyn Future<Output = T> + Send + 'a>>;
```

## When to Create Aliases

Create aliases when:
- Type has 3+ levels of nesting
- Type is used in multiple places
- Type represents a domain concept
- Type signature exceeds ~60 characters

---
title: Avoid _async suffix for async functions
impact: MEDIUM
impactDescription: Cleaner API - async is evident from the function signature
tags: name, async, functions
---

# Avoid _async suffix for async functions

Do not add `_async` suffix to async functions unless a sync version exists.

## Why This Matters

- The `async` keyword already indicates async
- Cleaner API surface
- Consistent with Rust ecosystem
- Suffix only needed when both versions exist

**Incorrect (avoid this pattern):**

```rust
// Redundant suffix - no sync version exists
pub async fn load_config_async() -> Result<Config> { }
pub async fn fetch_user_async(id: &str) -> Result<User> { }
pub async fn process_request_async(req: Request) -> Response { }
```

**Correct (recommended):**

```rust
// No suffix needed - async is in the signature
pub async fn load_config() -> Result<Config> { }
pub async fn fetch_user(id: &str) -> Result<User> { }
pub async fn process_request(req: Request) -> Response { }
```

## When Suffix IS Appropriate

Use suffix only when providing both sync and async versions:

```rust
// Sync version
pub fn read_file(path: &Path) -> io::Result<Vec<u8>> {
    std::fs::read(path)
}

// Async version - suffix differentiates
pub async fn read_file_async(path: &Path) -> io::Result<Vec<u8>> {
    tokio::fs::read(path).await
}

// Or use a separate async module
mod sync {
    pub fn read_file(path: &Path) -> io::Result<Vec<u8>> { }
}

mod async_api {
    pub async fn read_file(path: &Path) -> io::Result<Vec<u8>> { }
}
```

---
title: Use is_/has_/should_ prefix for boolean functions
impact: MEDIUM
impactDescription: Boolean function naming follows English grammar for readability
tags: name, functions, boolean
---

# Use is_/has_/should_ prefix for boolean functions

Boolean-returning functions should use `is_`, `has_`, `should_`, or `can_` prefix.

## Why This Matters

- Reads like English
- Clear return type expectation
- Self-documenting code
- Consistent with Rust ecosystem

**Incorrect (avoid this pattern):**

```rust
fn dangerous_command(cmd: &str) -> bool { }
fn empty(list: &[T]) -> bool { }
fn valid(input: &str) -> bool { }
fn admin(user: &User) -> bool { }
fn retry(attempts: u32) -> bool { }
```

**Correct (recommended):**

```rust
fn is_dangerous_command(cmd: &str) -> bool { }
fn is_empty(list: &[T]) -> bool { }
fn is_valid(input: &str) -> bool { }
fn is_admin(user: &User) -> bool { }
fn should_retry(attempts: u32) -> bool { }
fn can_execute(user: &User, action: &Action) -> bool { }
fn has_permission(user: &User, resource: &Resource) -> bool { }
```

## Prefix Guidelines

| Prefix | Use Case | Example |
|--------|----------|---------|
| `is_` | State/property check | `is_empty()`, `is_valid()`, `is_running()` |
| `has_` | Possession/presence | `has_permission()`, `has_children()` |
| `can_` | Capability/ability | `can_execute()`, `can_read()` |
| `should_` | Decision/recommendation | `should_retry()`, `should_cache()` |
| `needs_` | Requirement | `needs_update()`, `needs_refresh()` |

## Struct Methods

```rust
impl User {
    pub fn is_admin(&self) -> bool {
        self.role == Role::Admin
    }

    pub fn has_permission(&self, perm: Permission) -> bool {
        self.permissions.contains(&perm)
    }

    pub fn can_access(&self, resource: &Resource) -> bool {
        self.has_permission(resource.required_permission())
    }
}

// Usage reads naturally
if user.is_admin() && user.can_access(&resource) {
    // ...
}
```

---
title: Use Client suffix for API clients
impact: HIGH
impactDescription: Clear identification of types that make external API calls
tags: name, api, client
---

# Use Client suffix for API clients

Types that make external API calls should use the `Client` suffix.

## Why This Matters

- Immediately identifies network I/O types
- Clear dependency on external services
- Easy to mock in tests
- Consistent pattern across codebase

**Incorrect (avoid this pattern):**

```rust
// Unclear these make network calls
pub struct Api { }
pub struct Backend { }
pub struct Service { }
pub struct Gateway { }
```

**Correct (recommended):**

```rust
/// HTTP client for the Codex backend API.
pub struct BackendClient {
    http: reqwest::Client,
    base_url: String,
    auth_token: String,
}

impl BackendClient {
    pub fn new(base_url: &str, auth_token: &str) -> Self { }

    pub async fn get_user(&self, id: &str) -> Result<User, ApiError> { }
    pub async fn create_task(&self, task: &Task) -> Result<TaskId, ApiError> { }
}

/// Client for OpenAI API.
pub struct OpenAIClient {
    api_key: String,
    http: reqwest::Client,
}

/// Client for GitHub API.
pub struct GitHubClient {
    token: String,
    http: reqwest::Client,
}

/// MCP (Model Context Protocol) client.
pub struct McpClient {
    transport: Transport,
}
```

## Client Trait Pattern

```rust
#[async_trait
pub trait ApiClient: Send + Sync {
    async fn request<T: DeserializeOwned>(
        &self,
        endpoint: &str,
        method: Method,
        body: Option<impl Serialize>,
    ) -> Result<T, ApiError>;
}

impl ApiClient for BackendClient {
    // ...
}
```

## Testing with Clients

```rust
#[cfg(test)
mod tests {
    struct MockClient { }

    impl ApiClient for MockClient {
        async fn request<T>(&self, ...) -> Result<T, ApiError> {
            // Return mock data
        }
    }
}
```

---
title: Name environment variable constants with _ENV_VAR suffix
impact: LOW
impactDescription: Clear distinction between config keys and their environment variable sources
tags: name, constants, environment
---

# Name environment variable constants with _ENV_VAR suffix

Constants holding environment variable names should end with `_ENV_VAR`.

## Why This Matters

- Distinguishes env var names from values
- Easy to find all env vars in codebase
- Self-documenting code
- Prevents confusion with config keys

**Incorrect (avoid this pattern):**

```rust
// Unclear if this is the env var name or something else
const CODEX_HOME: &str = "CODEX_HOME";
const API_KEY: &str = "API_KEY";
const LOG_LEVEL: &str = "LOG_LEVEL";
```

**Correct (recommended):**

```rust
/// Environment variable for the Codex home directory.
const CODEX_HOME_ENV_VAR: &str = "CODEX_HOME";

/// Environment variable for the API key.
const API_KEY_ENV_VAR: &str = "CODEX_API_KEY";

/// Environment variable for log level.
const LOG_LEVEL_ENV_VAR: &str = "CODEX_LOG_LEVEL";

// Usage
fn get_home_dir() -> Option<PathBuf> {
    std::env::var(CODEX_HOME_ENV_VAR)
        .ok()
        .map(PathBuf::from)
}
```

## Related Constants

```rust
// Default values for configs
const DEFAULT_PORT: u16 = 8080;
const DEFAULT_TIMEOUT_SECS: u64 = 30;

// Environment variable names
const PORT_ENV_VAR: &str = "CODEX_PORT";
const TIMEOUT_ENV_VAR: &str = "CODEX_TIMEOUT";

// Config file paths
const CONFIG_FILE_NAME: &str = "config.toml";
const CONFIG_DIR_NAME: &str = ".codex";
```

---
title: Use Error suffix for error types
impact: HIGH
impactDescription: Consistent error naming enables easy identification and handling
tags: name, errors, types
---

# Use Error suffix for error types

Error types should end with `Error` to clearly identify them as error types.

## Why This Matters

- Immediately identifies error types
- Consistent with Rust ecosystem
- Easy to grep for error types
- Clear API contracts

**Incorrect (avoid this pattern):**

```rust
// Unclear these are errors
pub enum ConfigProblem { }
pub enum GitFailure { }
pub struct ParseIssue { }
pub enum NetworkException { }  // Java-ism
```

**Correct (recommended):**

```rust
pub enum ConfigError {
    #[error("failed to read config")
    ReadFailed,
    #[error("invalid format")
    InvalidFormat,
}

pub enum GitError {
    #[error("command failed")
    CommandFailed { command: String },
    #[error("not a repository")
    NotARepository,
}

pub struct ParseError {
    pub line: usize,
    pub message: String,
}

pub enum NetworkError {
    #[error("connection refused")
    ConnectionRefused,
    #[error("timeout")
    Timeout,
}
```

## Naming Patterns

| Domain | Error Type |
|--------|-----------|
| Configuration | `ConfigError` |
| Git operations | `GitError` |
| HTTP client | `HttpError`, `RequestError` |
| File system | `FsError`, `IoError` |
| Parsing | `ParseError` |
| Validation | `ValidationError` |
| Authentication | `AuthError` |
| Database | `DbError`, `QueryError` |

## Compound Errors

```rust
// Crate-level error that wraps module errors
pub enum Error {
    #[error(transparent)
    Config(#[from] ConfigError),

    #[error(transparent)
    Git(#[from] GitError),

    #[error(transparent)
    Network(#[from] NetworkError),
}
```

---
title: Use Handler suffix for trait implementations
impact: MEDIUM
impactDescription: Clear naming indicates the purpose of handler types
tags: name, handlers, traits
---

# Use Handler suffix for trait implementations

Types that implement handler traits should use the `Handler` suffix.

## Why This Matters

- Immediately identifies purpose
- Consistent pattern across codebase
- Easy to find all handlers
- Clear relationship with trait

**Incorrect (avoid this pattern):**

```rust
// Unclear what these types do
pub struct Git { }
pub struct Shell { }
pub struct FileOps { }

impl Tool for Git { }
impl Tool for Shell { }
impl EventProcessor for FileOps { }
```

**Correct (recommended):**

```rust
// Clear handler naming
pub struct GitToolHandler { }
pub struct ShellToolHandler { }
pub struct FileEventHandler { }

impl Tool for GitToolHandler { }
impl Tool for ShellToolHandler { }
impl EventProcessor for FileEventHandler { }
```

## Common Handler Patterns

```rust
// Request handlers
pub struct AuthHandler { }
pub struct ApiRequestHandler { }

// Event handlers
pub struct KeyboardEventHandler { }
pub struct MouseEventHandler { }
pub struct SystemEventHandler { }

// Message handlers
pub struct MessageHandler { }
pub struct CommandHandler { }

// Callback handlers
pub struct ResponseHandler { }
pub struct ErrorHandler { }
```

## With Generics

```rust
pub struct TypedHandler<T> {
    _marker: PhantomData<T>,
}

// Specific handler for a type
pub type UserRequestHandler = TypedHandler<UserRequest>;
pub type OrderRequestHandler = TypedHandler<OrderRequest>;
```

---
title: Use Info suffix for read-only data structures
impact: LOW
impactDescription: Clear indication that a type is for information retrieval, not mutation
tags: name, types, data
---

# Use Info suffix for read-only data structures

Read-only data transfer types should use the `Info` suffix.

## Why This Matters

- Indicates immutable/read-only semantics
- Distinguishes from mutable state
- Clear data direction (outgoing)
- Common in status/inspection APIs

**Incorrect (avoid this pattern):**

```rust
// Unclear if these are mutable or read-only
pub struct GitStatus { }    // Can I modify this?
pub struct UserData { }     // Is this for input or output?
pub struct ServerState { }  // Am I supposed to change this?
```

**Correct (recommended):**

```rust
/// Read-only information about the git repository.
#[derive(Debug, Clone)
pub struct GitInfo {
    pub branch: String,
    pub commit_hash: String,
    pub is_dirty: bool,
    pub remotes: Vec<String>,
}

/// Read-only user profile information.
#[derive(Debug, Clone, Serialize)
pub struct UserInfo {
    pub id: String,
    pub email: String,
    pub created_at: DateTime<Utc>,
}

/// Read-only model provider information.
#[derive(Debug, Clone, Serialize)
pub struct ModelProviderInfo {
    pub name: String,
    pub models: Vec<String>,
    pub is_available: bool,
}

// Usage
fn get_git_info(repo: &Repository) -> GitInfo { }
fn get_user_info(user_id: &str) -> Result<UserInfo> { }
fn list_providers() -> Vec<ModelProviderInfo> { }
```

## Related Patterns

| Suffix | Use Case | Mutability |
|--------|----------|------------|
| `Info` | Status/inspection data | Read-only |
| `State` | Runtime state | Mutable |
| `Data` | Generic data container | Varies |
| `Details` | Expanded information | Read-only |

---
title: Use Manager suffix for lifecycle management
impact: MEDIUM
impactDescription: Clear identification of types that manage resource lifecycles
tags: name, lifecycle, types
---

# Use Manager suffix for lifecycle management

Types that manage resource lifecycles should use the `Manager` suffix.

## Why This Matters

- Identifies ownership/lifecycle responsibility
- Clear resource management pattern
- Distinguishes from the resources themselves
- Common pattern in async Rust

**Incorrect (avoid this pattern):**

```rust
// Unclear lifecycle responsibility
pub struct Threads { }      // Collection or manager?
pub struct Connections { }  // Pool or active set?
pub struct Tasks { }        // List or scheduler?
```

**Correct (recommended):**

```rust
/// Manages background thread lifecycles.
pub struct ThreadManager {
    threads: HashMap<ThreadId, JoinHandle<()>>,
    shutdown_tx: broadcast::Sender<()>,
}

impl ThreadManager {
    pub fn spawn(&mut self, name: &str, task: impl FnOnce() + Send + 'static) -> ThreadId;
    pub fn stop(&mut self, id: ThreadId) -> Result<(), ThreadError>;
    pub async fn shutdown_all(&mut self);
}

/// Manages database connection pool.
pub struct ConnectionManager {
    pool: Pool<Postgres>,
    max_connections: usize,
}

/// Manages background task execution.
pub struct TaskManager {
    tasks: HashMap<TaskId, JoinHandle<()>>,
    runtime: Handle,
}

/// Manages subscription lifecycles.
pub struct SubscriptionManager {
    subscriptions: HashMap<SubscriptionId, Subscription>,
}
```

## Manager Pattern

```rust
impl ThreadManager {
    pub fn new() -> Self { }

    /// Spawns a new managed thread.
    pub fn spawn<F>(&mut self, f: F) -> ThreadId
    where
        F: FnOnce() + Send + 'static,
    {
        let id = ThreadId::new();
        let handle = std::thread::spawn(f);
        self.threads.insert(id, handle);
        id
    }

    /// Joins and removes a thread.
    pub fn join(&mut self, id: ThreadId) -> Result<(), ThreadError> {
        let handle = self.threads.remove(&id)
            .ok_or(ThreadError::NotFound)?;
        handle.join().map_err(|_| ThreadError::Panicked)
    }
}
```

---
title: Use Options suffix for configuration bundles
impact: MEDIUM
impactDescription: Clear naming for optional configuration structures
tags: name, configuration, types
---

# Use Options suffix for configuration bundles

Types that bundle optional configuration should use the `Options` suffix.

## Why This Matters

- Distinguishes from required config
- Clear that fields are optional
- Consistent pattern for overrides
- Self-documenting API

**Incorrect (avoid this pattern):**

```rust
// Unclear these are optional configurations
pub struct ServerConfig { }      // Required or optional?
pub struct ClientSettings { }    // All required?
pub struct RequestParams { }     // Configuration or data?
```

**Correct (recommended):**

```rust
/// Optional server configuration. All fields have defaults.
#[derive(Debug, Clone, Default)
pub struct ServerOptions {
    pub port: Option<u16>,
    pub host: Option<String>,
    pub timeout: Option<Duration>,
    pub max_connections: Option<usize>,
}

/// Optional client configuration.
#[derive(Debug, Clone, Default)
pub struct ClientOptions {
    pub retry_count: Option<u32>,
    pub user_agent: Option<String>,
    pub proxy: Option<String>,
}

// Usage - all optional
fn create_server(options: ServerOptions) -> Server {
    let port = options.port.unwrap_or(8080);
    let host = options.host.unwrap_or_else(|| "localhost".into());
    // ...
}

// Or with builder
let server = Server::builder()
    .options(ServerOptions {
        port: Some(3000),
        ..Default::default()
    })
    .build();
```

## Related Patterns

| Suffix | Use Case |
|--------|----------|
| `Options` | Optional configuration bundle |
| `Config` | Required configuration |
| `Settings` | User preferences |
| `Params` | Function parameters bundle |

---
title: Use plural names for collections
impact: LOW
impactDescription: Natural English naming for collection types
tags: name, variables, collections
---

# Use plural names for collections

Variables holding collections should use plural names.

## Why This Matters

- Reads naturally in loops
- Clear multiplicity indication
- Distinguishes from single items
- Consistent convention

**Incorrect (avoid this pattern):**

```rust
let event: Vec<Event> = vec![];
let user: HashSet<User> = HashSet::new();
let message: VecDeque<Message> = VecDeque::new();

for e in event {  // Confusing - "event" sounds singular
    process(e);
}
```

**Correct (recommended):**

```rust
let events: Vec<Event> = vec![];
let users: HashSet<User> = HashSet::new();
let messages: VecDeque<Message> = VecDeque::new();

for event in events {  // Clear - iterating "events", each is "event"
    process(event);
}

for user in &users {
    notify(user);
}
```

## Iterator Naming

```rust
// Collection is plural
let items: Vec<Item> = get_items();

// Iterator variable is singular
for item in items {
    println!("{}", item);
}

// With enumerate
for (index, item) in items.iter().enumerate() {
    println!("{}: {}", index, item);
}

// Filtered collections
let active_users: Vec<&User> = users
    .iter()
    .filter(|u| u.is_active())
    .collect();
```

## Maps and Associative Collections

```rust
// Plural for the collection concept
let user_by_id: HashMap<UserId, User> = HashMap::new();
let permissions_by_role: HashMap<Role, Vec<Permission>> = HashMap::new();

// Or describe the mapping
let id_to_user: HashMap<UserId, User> = HashMap::new();
```

---
title: Use Provider suffix for service implementations
impact: MEDIUM
impactDescription: Clear identification of service provider types
tags: name, services, types
---

# Use Provider suffix for service implementations

Types that provide services should use the `Provider` suffix.

## Why This Matters

- Identifies service abstraction
- Clear dependency injection pattern
- Easy to find all providers
- Distinguishes from consumers

**Incorrect (avoid this pattern):**

```rust
// Unclear these are service providers
pub struct Auth { }
pub struct Telemetry { }
pub struct ModelBackend { }
```

**Correct (recommended):**

```rust
/// Provides authentication services.
pub trait AuthProvider: Send + Sync {
    fn authenticate(&self, token: &str) -> Result<User, AuthError>;
    fn refresh_token(&self, token: &str) -> Result<String, AuthError>;
}

pub struct OAuthProvider {
    client: OAuthClient,
}

impl AuthProvider for OAuthProvider {
    // ...
}

/// Provides telemetry/observability services.
pub struct OtelProvider {
    tracer: Tracer,
    meter: Meter,
}

/// Provides model inference services.
pub trait ModelProvider: Send + Sync {
    async fn complete(&self, prompt: &str) -> Result<String, ModelError>;
}

pub struct OpenAIProvider {
    client: OpenAIClient,
}

pub struct OllamaProvider {
    client: OllamaClient,
}
```

## Provider Pattern

```rust
// Application setup
pub struct App {
    auth_provider: Arc<dyn AuthProvider>,
    model_provider: Arc<dyn ModelProvider>,
    otel_provider: OtelProvider,
}

impl App {
    pub fn new(
        auth: impl AuthProvider + 'static,
        model: impl ModelProvider + 'static,
    ) -> Self {
        Self {
            auth_provider: Arc::new(auth),
            model_provider: Arc::new(model),
            otel_provider: OtelProvider::new(),
        }
    }
}
```

---
title: Pair Request/Response types
impact: MEDIUM
impactDescription: Consistent API type pairing improves code navigation and understanding
tags: name, api, types
---

# Pair Request/Response types

API types should come in Request/Response pairs with matching prefixes.

## Why This Matters

- Easy to find related types
- Clear API contract
- Consistent naming across APIs
- Self-documenting code

**Incorrect (avoid this pattern):**

```rust
// Inconsistent naming
pub struct UserData { }           // Request?
pub struct UserResult { }         // Response?
pub struct CreateUserInput { }    // Input?
pub struct UserOutput { }         // Output?
```

**Correct (recommended):**

```rust
// Consistent Request/Response pairing
#[derive(Debug, Serialize, Deserialize)
pub struct CreateUserRequest {
    pub email: String,
    pub name: String,
}

#[derive(Debug, Serialize, Deserialize)
pub struct CreateUserResponse {
    pub user_id: String,
    pub created_at: DateTime<Utc>,
}

#[derive(Debug, Serialize, Deserialize)
pub struct GetUserRequest {
    pub user_id: String,
}

#[derive(Debug, Serialize, Deserialize)
pub struct GetUserResponse {
    pub user: User,
}

#[derive(Debug, Serialize, Deserialize)
pub struct ListUsersRequest {
    pub page: u32,
    pub limit: u32,
}

#[derive(Debug, Serialize, Deserialize)
pub struct ListUsersResponse {
    pub users: Vec<User>,
    pub total: u64,
}
```

## Handler Pattern

```rust
pub async fn create_user(
    req: CreateUserRequest,
) -> Result<CreateUserResponse, ApiError> {
    // ...
}

pub async fn get_user(
    req: GetUserRequest,
) -> Result<GetUserResponse, ApiError> {
    // ...
}
```

## Alternative: Input/Output for Internal APIs

```rust
// For internal, non-HTTP APIs
pub struct ProcessInput { }
pub struct ProcessOutput { }
```

---
title: Define crate-specific Result type alias
impact: MEDIUM
impactDescription: Reduces boilerplate and makes error types explicit
tags: name, result, types, alias
---

# Define crate-specific Result type alias

Define `type Result<T> = std::result::Result<T, CrateError>` for convenience.

## Why This Matters

- Reduces repetitive typing
- Clear what error type the crate uses
- Consistent signatures
- Easy to change error type later

**Incorrect (avoid this pattern):**

```rust
// Repetitive and verbose
pub fn load_config() -> std::result::Result<Config, ConfigError> { }
pub fn parse_command() -> std::result::Result<Command, ConfigError> { }
pub fn validate() -> std::result::Result<(), ConfigError> { }
```

**Correct (recommended):**

```rust
// In lib.rs or errors.rs
pub type Result<T> = std::result::Result<T, Error>;

// Or for module-specific errors
pub mod config {
    pub type Result<T> = std::result::Result<T, ConfigError>;
}
```

Usage:

```rust
use crate::Result;

pub fn load_config() -> Result<Config> { }
pub fn parse_command() -> Result<Command> { }
pub fn validate() -> Result<()> { }
```

## Multiple Error Types

When a crate has multiple error types:

```rust
// Crate-level alias
pub type Result<T> = std::result::Result<T, Error>;

// Module-specific aliases
pub mod io {
    pub type Result<T> = std::result::Result<T, IoError>;
}

pub mod parse {
    pub type Result<T> = std::result::Result<T, ParseError>;
}
```

## Naming the Alias

```rust
// Standard pattern
pub type Result<T> = std::result::Result<T, Error>;

// Or if "Error" name is taken
pub type CrateResult<T> = std::result::Result<T, CrateError>;
```

---
title: Use try_ prefix for fallible constructors
impact: HIGH
impactDescription: Clear indication that construction can fail
tags: name, constructors, error-handling
---

# Use try_ prefix for fallible constructors

Constructors that can fail should use the `try_` prefix and return `Result`.

## Why This Matters

- Clear API contract
- Follows Rust convention (`try_from`, `try_into`)
- Distinguishes from infallible `new()`
- Compile-time enforced error handling

**Incorrect (avoid this pattern):**

```rust
impl Config {
    // Unclear - does this panic or return Result?
    pub fn from_file(path: &Path) -> Result<Self, ConfigError> { }
    pub fn from_env() -> Result<Self, ConfigError> { }
    pub fn from_str(s: &str) -> Result<Self, ConfigError> { }
}
```

**Correct (recommended):**

```rust
impl Config {
    /// Creates a config with defaults. Cannot fail.
    pub fn new() -> Self {
        Self::default()
    }

    /// Tries to load config from a file.
    pub fn try_from_file(path: &Path) -> Result<Self, ConfigError> {
        let content = fs::read_to_string(path)?;
        Self::try_from_str(&content)
    }

    /// Tries to load config from environment variables.
    pub fn try_from_env() -> Result<Self, ConfigError> {
        // ...
    }

    /// Tries to parse config from a string.
    pub fn try_from_str(s: &str) -> Result<Self, ConfigError> {
        toml::from_str(s).map_err(ConfigError::Parse)
    }
}
```

## Standard Library Pattern

Follows the standard library convention:

```rust
// std::convert
trait TryFrom<T> {
    type Error;
    fn try_from(value: T) -> Result<Self, Self::Error>;
}

// Usage
let num: u8 = u8::try_from(256i32)?;  // Returns Err
```

## Related Naming

| Infallible | Fallible |
|-----------|----------|
| `new()` | `try_new()` |
| `from_*()` | `try_from_*()` |
| `with_*()` | `try_with_*()` |
| `parse()` | Returns Result (already fallible) |

---
title: Use with_ prefix for builder methods
impact: MEDIUM
impactDescription: Consistent builder API across the codebase
tags: name, builder, methods
---

# Use with_ prefix for builder methods

Builder methods that set a value should use the `with_` prefix.

## Why This Matters

- Consistent API pattern
- Clear distinction from getters
- Chainable method indication
- Self-documenting code

**Incorrect (avoid this pattern):**

```rust
impl ConfigBuilder {
    pub fn set_timeout(mut self, t: Duration) -> Self {
        self.timeout = t;
        self
    }

    pub fn timeout(mut self, t: Duration) -> Self {  // Confusing - is this getter or setter?
        self.timeout = t;
        self
    }

    pub fn add_host(mut self, h: String) -> Self {  // "add" implies collection append
        self.host = h;
        self
    }
}
```

**Correct (recommended):**

```rust
impl ConfigBuilder {
    pub fn with_timeout(mut self, timeout: Duration) -> Self {
        self.timeout = timeout;
        self
    }

    pub fn with_host(mut self, host: impl Into<String>) -> Self {
        self.host = host.into();
        self
    }

    pub fn with_tls(mut self, enabled: bool) -> Self {
        self.tls_enabled = enabled;
        self
    }

    pub fn with_retries(mut self, count: u32) -> Self {
        self.max_retries = count;
        self
    }
}

// Usage
let config = ConfigBuilder::new()
    .with_host("localhost")
    .with_timeout(Duration::from_secs(30))
    .with_tls(true)
    .build()?;
```

## Setter vs Builder

| Pattern | Use Case | Convention |
|---------|----------|------------|
| `with_*` | Builder (consumes self) | `fn with_x(mut self, x: T) -> Self` |
| `set_*` | Mutation (borrows self) | `fn set_x(&mut self, x: T)` |
| Getter | Read access | `fn x(&self) -> &T` |

---
title: Use kebab-case for crate directories with project prefix
impact: HIGH
impactDescription: Consistent naming enables easy identification of internal crates
tags: org, naming, crates
---

# Use kebab-case for crate directories with project prefix

Directory names use kebab-case. Package names in Cargo.toml use a project prefix for internal crates.

## Why This Matters

- Distinguishes internal crates from external dependencies
- Prevents name collisions with crates.io packages
- Easy to identify project crates in Cargo.lock
- Consistent with Rust conventions

**Incorrect (avoid this pattern):**

```
my_crate/           # Wrong: underscore
myClient/           # Wrong: camelCase
```

```toml
[package
name = "backend-client"  # Wrong: no project prefix
```

**Correct (recommended):**

Directory structure:
```
backend-client/     # kebab-case directory
app-server/
mcp-types/
```

Cargo.toml:
```toml
# In backend-client/Cargo.toml
[package
name = "codex-backend-client"  # Project prefix: codex-

# In app-server/Cargo.toml
[package
name = "codex-app-server"
```

## Dependency References

```toml
# In another crate's Cargo.toml
[dependencies
codex-backend-client = { path = "../backend-client" }
codex-protocol = { path = "../protocol" }
```

## Internal Crate Imports

```rust
use codex_backend_client::Client;
use codex_protocol::Message;
```

Note: Package names with hyphens become underscores in `use` statements.

---
title: Define error types in dedicated errors.rs file
impact: MEDIUM
impactDescription: Centralized error definitions improve discoverability and consistency
tags: org, errors, modules
---

# Define error types in dedicated errors.rs file

Place error type definitions in `errors.rs` or `error.rs` file within the module.

## Why This Matters

- All errors in one discoverable location
- Easy to audit error handling
- Consistent error definitions
- Clear module structure

**Incorrect (errors scattered across files):**

```rust
// src/config/loader.rs
#[derive(Debug, Error)]
pub enum LoadError { /* ... */ }

// src/config/validation.rs
#[derive(Debug, Error)]
pub enum ValidationError { /* ... */ }

// Hard to find all error types!
```

**Correct (errors in dedicated file):**

```text
src/
├── lib.rs
├── errors.rs           # Crate-level errors
└── config/
    ├── mod.rs
    ├── errors.rs       # Config-specific errors
    └── loader.rs
```

```rust
// src/errors.rs
//! Error types for the crate.

use thiserror::Error;

/// The main error type for this crate.
#[derive(Debug, Error)]
pub enum Error {
    #[error("configuration error")]
    Config(#[from] ConfigError),

    #[error("network error")]
    Network(#[from] NetworkError),

    #[error("IO error")]
    Io(#[from] std::io::Error),
}

/// Convenience type alias for Results.
pub type Result<T> = std::result::Result<T, Error>;
```

## Module-Level errors.rs

```rust
// src/config/errors.rs
use thiserror::Error;

#[derive(Debug, Error)]
pub enum ConfigError {
    #[error("failed to read config file: {path}")]
    ReadFailed { path: String, source: std::io::Error },

    #[error("invalid configuration: {0}")]
    Invalid(String),
}
```

## Re-exporting Errors

```rust
// src/lib.rs
mod errors;
pub use errors::{Error, Result};

// src/config/mod.rs
mod errors;
pub use errors::ConfigError;
```

---
title: Organize by feature with mod.rs files
impact: MEDIUM
impactDescription: Feature-based organization improves code navigation and maintainability
tags: org, modules, features
---

# Organize by feature with mod.rs files

Group related functionality in directories with `mod.rs` files.

## Why This Matters

- Related code is co-located
- Easy to understand feature scope
- Clear module boundaries
- Supports encapsulation

**Incorrect (flat structure with many files):**

```text
src/
├── lib.rs
├── git_clone.rs
├── git_status.rs
├── git_commit.rs
├── shell_exec.rs
├── shell_parse.rs
└── file_read.rs
```

**Correct (feature-based organization):**

```text
src/
├── lib.rs
├── config/
│   ├── mod.rs
│   ├── loader.rs
│   └── validation.rs
├── tools/
│   ├── mod.rs
│   ├── git.rs
│   ├── shell.rs
│   └── file_ops.rs
└── sandboxing/
    ├── mod.rs
    ├── linux.rs
    └── macos.rs
```

## mod.rs Pattern

```rust
// src/tools/mod.rs
//! Tool implementations for the agent.

mod git;
mod shell;
mod file_ops;

// Re-export public API
pub use git::GitTool;
pub use shell::ShellTool;
pub use file_ops::FileOps;

// Internal shared utilities
pub(crate) mod utils;
```

## lib.rs References

```rust
// src/lib.rs
pub mod config;
pub mod tools;
pub mod sandboxing;
```

## When to Use Directories vs Single Files

**Use directories when:**
- Feature has 3+ related components
- Clear sub-features exist
- Shared internal utilities needed

**Use single files when:**
- Feature is cohesive and small
- No logical sub-divisions
- Under ~300 lines

---
title: Place handlers in dedicated subdirectory
impact: MEDIUM
impactDescription: Separating handlers from core logic improves code organization
tags: org, handlers, modules
---

# Place handlers in dedicated subdirectory

When a module has multiple handlers, place them in a `handlers/` subdirectory with `mod.rs`.

## Why This Matters

- Separates dispatch logic from implementation
- Easy to find all handlers for a feature
- Clear naming convention
- Scales as handlers grow

**Incorrect (handlers mixed with other code):**

```text
src/api/
├── mod.rs
├── router.rs
├── auth_handler.rs      # Mixed with other files
├── user_handler.rs
├── task_handler.rs
├── context.rs
└── middleware.rs
```

**Correct (handlers in subdirectory):**

```text
src/api/
├── mod.rs           # API module root
├── router.rs        # Request routing
├── context.rs
├── middleware.rs
└── handlers/
    ├── mod.rs       # Handler exports
    ├── auth.rs      # Auth handlers
    ├── users.rs     # User handlers
    └── tasks.rs     # Task handlers
```

## handlers/mod.rs

```rust
//! API request handlers.

mod auth;
mod users;
mod tasks;

pub use auth::*;
pub use users::*;
pub use tasks::*;
```

## Handler Pattern

```rust
// handlers/users.rs
use crate::api::context::ApiContext;
use crate::api::error::ApiError;

pub async fn get_user(
    ctx: &ApiContext,
    user_id: &str,
) -> Result<User, ApiError> {
    ctx.db.find_user(user_id).await
        .ok_or(ApiError::NotFound)
}

pub async fn create_user(
    ctx: &ApiContext,
    input: CreateUserInput,
) -> Result<User, ApiError> {
    // Validation and creation logic
}
```

## Router Integration

```rust
// router.rs
use crate::api::handlers;

pub fn setup_routes(router: Router) -> Router {
    router
        .route("/users/:id", get(handlers::get_user))
        .route("/users", post(handlers::create_user))
}
```

---
title: Organize integration tests in suite directory
impact: MEDIUM
impactDescription: Structured test organization improves maintainability and test discovery
tags: org, testing, integration
---

# Organize integration tests in suite directory

Place integration tests in `tests/suite/` directory with `mod.rs` for organization.

## Why This Matters

- Separates integration tests from unit tests
- Groups related tests together
- Clearer test structure for large projects
- Easy to run specific test suites

**Incorrect (scattered test files):**

```text
crate/
├── src/
│   └── lib.rs
└── tests/
    ├── test_api.rs          # Flat structure
    ├── test_config.rs       # Hard to manage
    ├── test_integration.rs  # No organization
    └── helpers.rs           # Mixed with tests
```

**Correct (suite directory structure):**

```text
crate/
├── src/
│   └── lib.rs
└── tests/
    ├── common/        # Shared test utilities (if crate-specific)
    │   └── mod.rs
    └── suite/
        ├── mod.rs     # Re-exports test modules
        ├── api_tests.rs
        ├── config_tests.rs
        └── integration_tests.rs
```

## tests/suite/mod.rs

```rust
//! Integration test suite for the crate.

mod api_tests;
mod config_tests;
mod integration_tests;
```

## Test File Example

```rust
// tests/suite/api_tests.rs
use crate::common::*;  // If you have shared utilities

#[tokio::test]
async fn test_api_endpoint() {
    // Integration test
}

#[tokio::test]
async fn test_api_error_handling() {
    // Another integration test
}
```

## Running Tests

```bash
# Run all tests
cargo test

# Run only integration tests
cargo test --test suite

# Run specific test file
cargo test --test suite::api_tests
```

## Alternative: Flat tests/ Structure

For smaller crates, a flat structure is acceptable:

```text
crate/
└── tests/
    ├── api_tests.rs
    └── config_tests.rs
```

---
title: Use pub(crate) for internal APIs
impact: HIGH
impactDescription: Proper visibility prevents accidental external dependencies on internal APIs
tags: org, visibility, api
---

# Use pub(crate) for internal APIs

Prefer `pub(crate)` for crate-internal functions and types over private or fully public.

## Why This Matters

- Prevents external code from depending on internals
- Makes the public API surface explicit
- Allows refactoring internal code freely
- Clearer intent than `pub` for internal use

## Visibility Levels

| Visibility | Scope | Use Case |
|-----------|-------|----------|
| `pub` | Everywhere | Public API |
| `pub(crate)` | Current crate | Internal shared code |
| `pub(super)` | Parent module | Module-internal helpers |
| (private) | Current module | Implementation details |

**Incorrect (avoid this pattern):**

```rust
// Too broad - exposes internal implementation
pub fn internal_helper() { }

// Too narrow - can't use from sibling modules
fn shared_utility() { }
```

**Correct (recommended):**

```rust
// Public API - explicitly intended for external use
pub fn process_request(req: Request) -> Response {
    let validated = validate_internal(req);
    transform_internal(validated)
}

// Internal shared code - accessible within crate only
pub(crate) fn validate_internal(req: Request) -> ValidatedRequest {
    // ...
}

pub(crate) fn transform_internal(req: ValidatedRequest) -> Response {
    // ...
}

// Module-only helper
fn helper_function() {
    // ...
}
```

## Struct Fields

```rust
pub struct Config {
    /// Public configuration option
    pub timeout: Duration,

    /// Internal field, not part of public API
    pub(crate) internal_state: State,

    /// Private implementation detail
    cache: HashMap<String, Value>,
}
```

## Re-exports

Use `pub use` to expose internal items at a higher level:

```rust
// In lib.rs
mod internal;

// Re-export specific items as public API
pub use internal::PublicType;
pub use internal::public_function;
```

---
title: Create separate crate for shared test utilities
impact: MEDIUM
impactDescription: Shared test utilities reduce duplication and enable consistent testing patterns
tags: org, testing, utilities
---

# Create separate crate for shared test utilities

Place shared test utilities in `tests/common/` as a separate workspace member crate.

## Why This Matters

- Avoids duplicating test helpers across crates
- Consistent test patterns across workspace
- Proper dependency management for test code
- Cleaner test module organization

**Incorrect (duplicated test helpers):**

```rust
// core/tests/integration_tests.rs
fn setup_test_dir() -> tempfile::TempDir {
    tempfile::tempdir().expect("test setup")
}

// cli/tests/cli_tests.rs
fn setup_test_dir() -> tempfile::TempDir {  // Duplicated!
    tempfile::tempdir().expect("test setup")
}
```

**Correct (shared test crate):**

```text
workspace/
├── core/
│   └── tests/
│       └── integration_tests.rs
├── cli/
│   └── tests/
│       └── cli_tests.rs
└── tests/
    └── common/
        ├── Cargo.toml
        └── src/
            └── lib.rs
```

## tests/common/Cargo.toml

```toml
[package]
name = "codex-test-common"
version = "0.1.0"
edition = "2024"
publish = false  # Never publish test utilities

[dependencies]
tokio = { workspace = true }
tempfile = "3"
assert_matches = "1"

# Crates being tested
codex-core = { path = "../../core" }
codex-protocol = { path = "../../protocol" }
```

## tests/common/src/lib.rs

```rust
//! Shared test utilities for the codex workspace.

pub mod fixtures;
pub mod mocks;
pub mod assertions;

/// Creates a temporary directory with test fixtures.
pub fn setup_test_dir() -> tempfile::TempDir {
    tempfile::tempdir().expect("test setup")
}

/// Mock server for testing client code.
pub struct MockServer {
    // ...
}
```

## Using in Tests

```rust
// In core/tests/integration_tests.rs
use codex_test_common::{setup_test_dir, MockServer};

#[tokio::test]
async fn test_client_connection() {
    let server = MockServer::start().await;
    let dir = setup_test_dir();
    // ...
}
```

---
title: Use flat workspace structure with utils subdirectory
impact: HIGH
impactDescription: Consistent workspace layout enables easy navigation and discovery
tags: org, workspace, structure
---

# Use flat workspace structure with utils subdirectory

Place all crates at workspace root level. Group small utility crates under a `utils/` subdirectory.

## Why This Matters

- Easy to find and navigate crates
- Clear separation between main and utility crates
- Consistent with Rust ecosystem conventions
- Scales well as workspace grows

**Incorrect (deeply nested or disorganized):**

```text
workspace/
├── Cargo.toml
├── crates/
│   ├── main/
│   │   └── core/      # Too deeply nested
│   └── helpers/
│       └── utils/
│           └── git/   # Hard to navigate
└── lib/
    └── common/        # Inconsistent location
```

**Correct (flat with utils subdirectory):**

```text
workspace/
├── Cargo.toml          # Workspace manifest
├── Cargo.lock
├── core/               # Main crates at root
├── cli/
├── tui/
├── protocol/
├── app-server/
├── backend-client/
├── utils/              # Small utilities grouped
│   ├── git/
│   ├── cache/
│   ├── pty/
│   └── config/
└── tests/
    └── common/         # Shared test utilities
```

## Workspace Cargo.toml

```toml
[workspace]
resolver = "2"
members = [
    "core",
    "cli",
    "tui",
    "protocol",
    "app-server",
    "backend-client",
    "utils/*",
    "tests/common",
]

[workspace.package]
version = "0.1.0"
edition = "2024"
license = "MIT"

[workspace.dependencies]
# Centralized dependencies
tokio = { version = "1", features = ["full"] }
serde = { version = "1", features = ["derive"] }
```

## When to Use utils/

Move crates to `utils/` when:
- Single-purpose utility functionality
- Unlikely to be used externally
- Small codebase (few files)
- No complex dependencies

Keep at root when:
- Core application component
- Significant size/complexity
- External API surface

---
title: Place unit tests in #[cfg(test)] mod tests
impact: HIGH
impactDescription: Standard Rust test organization pattern
tags: style, testing, modules
---

# Place unit tests in #[cfg(test)] mod tests

Unit tests go in a `tests` submodule at the bottom of the file.

## Why This Matters

- Standard Rust convention
- Tests compiled only for test builds
- Tests have access to private items
- Clear separation of concerns

**Incorrect (tests in separate file or mixed with code):**

```rust
// src/config.rs
pub struct Config {
    timeout: Duration,
}

// Tests scattered or in wrong location
#[test]  // Wrong: not in cfg(test) module
fn test_config() {
    let config = Config::new();
}
```

**Correct (tests in cfg(test) module):**

```rust
// src/config.rs
use std::path::Path;

pub struct Config {
    timeout: Duration,
}

impl Config {
    pub fn new() -> Self {
        Self {
            timeout: Duration::from_secs(30),
        }
    }

    fn validate(&self) -> bool {
        self.timeout.as_secs() > 0
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_new_config_has_default_timeout() {
        let config = Config::new();
        assert_eq!(config.timeout, Duration::from_secs(30));
    }

    #[test]
    fn test_validate_accepts_valid_config() {
        let config = Config::new();
        assert!(config.validate());  // Can access private method
    }
}
```

## Async Tests

```rust
#[cfg(test)]
mod tests {
    use super::*;

    #[tokio::test]
    async fn test_async_operation() {
        let result = async_function().await;
        assert!(result.is_ok());
    }
}
```

## Test Helpers

```rust
#[cfg(test)]
mod tests {
    use super::*;

    // Test-only helper
    fn create_test_config() -> Config {
        Config {
            timeout: Duration::from_millis(100),
        }
    }

    #[test]
    fn test_something() {
        let config = create_test_config();
        // ...
    }
}
```

## Integration vs Unit Tests

| Type | Location | Access |
|------|----------|--------|
| Unit tests | `src/*.rs` in `mod tests` | Private items |
| Integration tests | `tests/*.rs` | Public API only |

---
title: Deny direct stdout/stderr in library code
impact: HIGH
impactDescription: Library code should use structured logging, not direct output
tags: style, logging, library
---

# Deny direct stdout/stderr in library code

Use `#![deny(clippy::print_stdout, clippy::print_stderr)]` in library crates.

## Why This Matters

- Libraries shouldn't produce output directly
- Callers control logging/output
- Enables structured logging
- Clean separation of concerns

## Configuration

In lib.rs:
```rust
#![deny(clippy::print_stdout)
#![deny(clippy::print_stderr)
```

Or in Cargo.toml (workspace-level):
```toml
[workspace.lints.clippy
print_stdout = "deny"
print_stderr = "deny"
```

**Incorrect (avoid this pattern):**

```rust
// In library code
pub fn process(data: &str) -> Result<Output> {
    println!("Processing: {}", data);  // Deny!
    eprintln!("Warning: slow path");   // Deny!
    // ...
}
```

**Correct (recommended):**

```rust
use tracing::debug;
use tracing::warn;

pub fn process(data: &str) -> Result<Output> {
    debug!(data = %data, "processing input");
    warn!("taking slow path");
    // ...
}
```

## Binary Crates

Binary crates can use print statements:

```rust
// In main.rs or bin/*.rs - OK to use print
fn main() -> Result<()> {
    let result = mylib::process("data")?;
    println!("{}", result);  // OK in binary
    Ok(())
}
```

## Exception Pattern

When print is truly needed (CLI output after TUI restore):

```rust
#[expect(clippy::print_stderr, reason = "TUI output after terminal restore")
fn print_error(msg: &str) {
    eprintln!("Error: {}", msg);
}
```

---
title: Use #[expect] with reason for lint suppression
impact: MEDIUM
impactDescription: Documents why lints are suppressed and ensures suppressions are still needed
tags: style, lints, documentation
---

# Use #[expect] with reason for lint suppression

When suppressing lints, use `#[expect(lint, reason = "...")]` instead of `#[allow]`.

## Why This Matters

- Documents WHY the lint is suppressed
- Warns when suppression is no longer needed
- Better than silent `#[allow]`
- Rust 1.81+ feature

**Incorrect (avoid this pattern):**

```rust
#[allow(clippy::print_stderr)
fn output_error(msg: &str) {
    eprintln!("Error: {}", msg);
}

#[allow(dead_code)
fn unused_helper() { }

#[allow(clippy::unwrap_used)
fn infallible_parse() {
    let _: u32 = "42".parse().unwrap();
}
```

**Correct (recommended):**

```rust
#[expect(clippy::print_stderr, reason = "TUI output after terminal restore")
fn output_error(msg: &str) {
    eprintln!("Error: {}", msg);
}

#[expect(dead_code, reason = "used in integration tests only")
fn test_helper() { }

#[expect(clippy::unwrap_used, reason = "static string always parses as u32")
fn infallible_parse() {
    let _: u32 = "42".parse().unwrap();
}
```

## #[expect] vs #[allow

| Attribute | Behavior |
|-----------|----------|
| `#[allow]` | Silently suppresses lint, no warning if unused |
| `#[expect]` | Suppresses lint, warns if lint no longer triggers |

## When Suppression Is Removed

If you fix the code and the lint no longer applies:

```rust
// This will warn: "this lint expectation is unfulfilled"
#[expect(clippy::unwrap_used, reason = "...")
fn now_uses_proper_error_handling() {
    let _ = "42".parse::<u32>().ok();  // No unwrap!
}
```

## Module-Level Expects

```rust
// At module level for broader scope
#![expect(clippy::module_inception, reason = "deliberate module structure")
```

---
title: Use one item per use statement
impact: MEDIUM
impactDescription: Cleaner diffs and easier import management
tags: style, imports, formatting
---

# Use one item per use statement

Each `use` statement should import exactly one item (imports_granularity = Item).

## Why This Matters

- Cleaner git diffs
- Easy to add/remove imports
- Clear dependency visibility
- Consistent formatting

## rustfmt.toml Configuration

```toml
imports_granularity = "Item"
```

**Incorrect (avoid this pattern):**

```rust
use std::{fs, io, path::Path};
use tokio::{sync::mpsc, task::JoinHandle};
use serde::{Deserialize, Serialize};
```

**Correct (recommended):**

```rust
use std::fs;
use std::io;
use std::path::Path;

use tokio::sync::mpsc;
use tokio::task::JoinHandle;

use serde::Deserialize;
use serde::Serialize;
```

## Import Ordering

Group imports in this order (rustfmt handles this):

1. Standard library (`std::`)
2. External crates
3. Crate-internal (`crate::`, `super::`, `self::`)

```rust
// Standard library
use std::collections::HashMap;
use std::path::PathBuf;

// External crates
use serde::Deserialize;
use serde::Serialize;
use tokio::sync::mpsc;

// Crate-internal
use crate::config::Config;
use crate::error::Error;
```

## Git Diff Comparison

Adding `std::path::PathBuf`:

**Grouped (bad diff):**
```diff
-use std::{fs, io};
+use std::{fs, io, path::PathBuf};
```

**Granular (clean diff):**
```diff
 use std::fs;
 use std::io;
+use std::path::PathBuf;
```

---
title: Use inline format arguments
impact: MEDIUM
impactDescription: Cleaner, more readable format strings
tags: style, formatting, strings
---

# Use inline format arguments

Inline variable names in format strings instead of positional arguments.

## Why This Matters

- More readable format strings
- Fewer places to make mistakes
- Enforced by clippy::uninlined_format_args
- Rust 1.58+ feature

## Configuration

```toml
[workspace.lints.clippy
uninlined_format_args = "warn"
```

**Incorrect (avoid this pattern):**

```rust
let name = "Alice";
let count = 42;

println!("Hello, {}!", name);
println!("{} items remaining", count);
format!("User {} has {} points", name, count);
tracing::info!("Processing {} items for {}", count, name);
```

**Correct (recommended):**

```rust
let name = "Alice";
let count = 42;

println!("Hello, {name}!");
println!("{count} items remaining");
format!("User {name} has {count} points");
tracing::info!("Processing {count} items for {name}");
```

## With Expressions

For expressions, you still need positional or named arguments:

```rust
// Expression in format - needs positional
println!("Result: {}", compute_value());

// Or use a binding
let result = compute_value();
println!("Result: {result}");

// Named argument for clarity
println!("Sum: {sum}", sum = a + b);
```

## Debug and Display

```rust
let config = Config::new();

// Debug format
println!("Config: {config:?}");
println!("Config: {config:#?}");  // Pretty print

// With width/precision
println!("Value: {value:>10}");
println!("Float: {f:.2}");
```

---
title: Add module-level documentation
impact: MEDIUM
impactDescription: Module docs provide context and improve API discoverability
tags: style, documentation, modules
---

# Add module-level documentation

Start lib.rs and mod.rs files with `//!` documentation comments.

## Why This Matters

- Provides crate/module overview
- Appears in generated docs
- Explains purpose and usage
- Improves API discoverability

**Incorrect (avoid this pattern):**

```rust
// lib.rs with no module docs
use std::path::PathBuf;

pub mod config;
pub mod error;
```

**Correct (recommended):**

```rust
// lib.rs
//! Codex Core Library
//!
//! This crate provides the core functionality for the Codex agent,
//! including configuration management, tool execution, and
//! sandboxing capabilities.
//!
//! # Quick Start
//!
//! ```rust
//! use codex_core::Config;
//!
//! let config = Config::load()?;
//! let agent = Agent::new(config);
//! agent.run().await?;
//! ```
//!
//! # Features
//!
//! - `sandbox` - Enable sandboxing support (enabled by default)
//! - `telemetry` - Enable OpenTelemetry integration

use std::path::PathBuf;

pub mod config;
pub mod error;
```

## Submodule Documentation

```rust
// src/config/mod.rs
//! Configuration management for Codex.
//!
//! This module handles loading, validating, and providing access
//! to configuration from various sources (files, environment, CLI).

mod loader;
mod validation;

pub use loader::ConfigLoader;
pub use validation::validate;
```

## Key Sections

- **Overview** - What the crate/module does
- **Quick Start** - Basic usage example
- **Features** - Cargo feature flags
- **Modules** - Brief description of submodules