Rust style guide

How we write Rust in this repository. The goal is a codebase that reads as if one careful person wrote it: predictable structure, errors that carry context, comments that explain why, and a formatter/linter setup that makes style a non-topic in review.

This guide is grounded in the Rust API Guidelines, the official Rust Style Guide (what rustfmt implements), the Clippy lint groups, and the panic policy articulated in "Using unwrap() in Rust is Okay" — adapted to this workspace's specifics. Where existing code diverges from a rule, the rule wins for new code; see Adopting this guide for how to handle the gap.

Scope

Applies to all Rust in the workspace:

Crate	Role	Special rules
`crates/chidori`	Agent framework + CLI binary	`anyhow` errors, `tokio` async, `tracing`
`crates/chidori-js`	Pure-Rust JS engine (lib)	Synchronous, `Result<_, Value>` errors, zero `unsafe`
`crates/test262-runner`	Conformance harness (`publish = false`)	Test-tool leniency: `unwrap`/`expect` fine

Toolchain and formatting

Toolchain is pinned by rust-toolchain.toml (stable channel, rustfmt + clippy components). The workspace MSRV is rust-version = "1.95" in each crate's Cargo.toml; bump it deliberately and note why (the current floor comes from oxc).
Formatting is default cargo fmt, no configuration. There is intentionally no rustfmt.toml: the upstream default style is the house style, and every toolchain update applies it identically for everyone. Never hand-format against the formatter. The pre-commit hook (hk.pkl, installed via mise) auto-formats staged files, and CI rejects unformatted code with cargo fmt --check.
#[rustfmt::skip] is reserved for genuinely tabular code — opcode tables, test matrices, aligned bytecode listings — where alignment carries meaning. Add a one-line comment saying what the alignment communicates.
Edition 2021 across the workspace. Edition bumps are a workspace-wide, single-PR affair.

Clippy

Clippy is enforced: CI and the pre-commit hook both run

cargo clippy --workspace --all-targets -- -D warnings

so run it locally before pushing. Guidance on lint levels:

The default groups (correctness, suspicious, complexity, perf, style) are the baseline. correctness findings are bugs — fix them, never suppress them.
The few workspace-wide exceptions live in [workspace.lints.clippy] in the root Cargo.toml (each crate inherits via [lints] workspace = true), each with a comment saying why. That table is the only place for blanket allows; think hard before growing it.
When a lint is wrong for a specific site, suppress it narrowly (item level, not module level) and prefer #[expect(clippy::..., reason = "...")] over #[allow]: expect warns when the suppression goes stale, and the mandatory reason documents the judgment call. Existing module-wide allows (#![allow(clippy::all)] on generated unicode_tables.rs, #![allow(dead_code)] on not-yet-wired runtime modules) are the pattern for the rare legitimate blanket case: generated code and scaffolding, with the module header saying so.
Some discipline goes beyond what the default groups machine-check; treat these as always-on reviewer expectations: undocumented_unsafe_blocks (every unsafe block gets a // SAFETY: comment), await_holding_lock (no sync lock guards across .await), dbg_macro and stray todo!/unimplemented! (never merged), let_underscore_future (a dropped future silently does nothing).

Naming

Follow RFC 430 casing and the API-guidelines conventions; the ones that come up most here:

Types, traits, enum variants: UpperCamelCase. Functions, methods, modules, fields: snake_case. Constants and statics: SCREAMING_SNAKE_CASE. Crate features: concrete words, no use- / with- placeholders.
Conversions follow as_ / to_ / into_: as_ is a free borrowed→borrowed view, to_ is an expensive copy, into_ consumes self.
Getters are the bare field name (fn seq(&self) -> Seq), not get_seq; get is reserved for lookups that take a key and may miss.
Iterator-producing methods are iter(), iter_mut(), into_iter() (C-ITER); a custom iterator type is named after the method that produces it.
Name things for the domain, and keep word order consistent with the neighbors: an error variant family that starts as ParseError, CompileError should not grow an ErrorLimit.

Modules and visibility

Directory modules use mod.rs (runtime/mod.rs, providers/mod.rs, ...). That is the established layout; don't mix in the foo.rs + foo/ sibling style — pick up the convention that is already there.
chidori-js stays flat: one file per engine concern (vm.rs, gc.rs, exec.rs). Split a file when it grows a second responsibility, not at a line count.
Every module starts with a //! header: what the module is, the one or two invariants a reader must know, and pointers to the design doc under docs/ when one exists. Most of the tree already does this; new modules must.
Default to pub(crate). Reach for plain pub only when the item is deliberately part of the crate's public API — for chidori, that means it's re-exported through the curated framework facade or otherwise documented as stable. A pub item is a compatibility promise; a pub(crate) item is refactorable on a whim. (The current tree is pub-heavy; tighten opportunistically when touching a module.)
Keep lib.rs a table of contents: module declarations, curated re-exports, crate docs. Logic lives in modules.

Error handling

The workspace deliberately runs two error regimes. Know which side of the boundary you're on, and don't let them leak into each other.

`chidori` (framework, CLI, server): `anyhow`

Application-style code uses anyhow::Result<T> end to end:

Propagate with ?. Attach context at the point where a failure would otherwise be ambiguous:
```
let manifest = fs::read_to_string(&path)
    .with_context(|| format!("reading package manifest {}", path.display()))?;
```
Use .context(...) for cheap static messages and .with_context(|| ...) when the message allocates. A good context line names the operation and the subject ("reading package manifest X"), not the error class ("IO error").
bail! and ensure! for early exits and precondition checks that are error conditions (bad user input, malformed config) rather than bugs.
Don't introduce thiserror enums or custom error types unless a caller genuinely needs to match on the failure (e.g. a provider distinguishing retryable rate limits from fatal auth errors). One unified error chain that prints well is the point of the anyhow regime. If you do add a typed error, it implements std::error::Error + Debug + Display and converts into anyhow::Error at the boundary for free.

`chidori-js` (engine): errors are JS values

Inside the engine, a failure is a thrown JavaScript exception, so the idiom is Result<T, Value> where the Err payload is the JS value routed through Completion::Throw. Constructor selection goes through the engine's ErrorKind mapping.

Never import anyhow (or any other error crate) into chidori-js. If an engine-internal failure can't be expressed as a throwable JS value, it is a bug — see panics below.
Host-boundary code in chidori that calls into the engine converts at the boundary: JS exception → structured error/context on the anyhow chain, once, in one place.

Panics: bugs, not errors

We follow the bug vs. error condition distinction: a Result models something that is expected to fail sometimes (I/O, user input, network, subprocesses); a panic asserts an invariant — something that cannot happen unless the program has a bug.

In Result-returning code, reach for ?, not .unwrap(). An unwrap() on an operation that can legitimately fail converts a recoverable error into a process abort — in a runtime that promises durable, resumable agents, that's the worst possible failure mode.
unwrap()/expect() are acceptable when the failure would be a bug: a mutex poisoned only if another thread already panicked, a regex literal known valid at compile time, an index established by the loop above, a key inserted two lines earlier. Prefer expect("...") when the message adds diagnostic value a stack trace wouldn't; write it as the invariant that was supposed to hold ("bytecode register allocated by compile pass"), not a restatement of the failure ("failed to get register").
panic!, assert!, debug_assert! follow the same rule: invariants only. Prefer debug_assert! for hot-path checks the release interpreter can't afford.
Tests, benches, examples, and test262-runner may unwrap freely. A panic is exactly the right test-failure mechanism, and ?-plumbing in tests obscures the assertion under test.
Never unwrap() on a value derived from user input, agent code, the network, the filesystem, or an LLM response. Agents run arbitrary TypeScript; anything they can influence is an error condition by definition.

Logging and output

Library and runtime code speaks tracing, never println!. Everything under runtime/, providers/, mcp/, pkg/ (except interactive install progress), server.rs, scheduler.rs, storage.rs, and all of chidori-js uses tracing::{trace,debug,info,warn,error} with structured fields:
```
tracing::debug!(session_id = %id, seq, "replaying journal entry");
```
Structured fields (field = value), not format-string interpolation — the OTLP exporter and any downstream collector can only filter on fields.
println!/eprintln! are for CLI user-facing output only: main.rs command handlers, init.rs, interactive prompts, install progress. Rule of thumb: if the text is the product of the command, print it (results to stdout, user-facing errors to stderr); if it describes what the program is doing internally, it's a tracing event.
Pick levels for an operator reading production logs: error = something was lost or a request failed; warn = degraded but recovered; info = lifecycle milestones (server started, session resumed); debug/trace = developers only. Don't log at info inside per-opcode or per-journal-entry loops.
No dbg! in committed code.

Async and concurrency

chidori is async on tokio; chidori-js is deliberately synchronous (determinism and replay depend on it) and must stay free of async runtimes.

Never block a runtime worker thread. CPU-heavy or blocking work (synchronous file I/O bursts, subprocess waits, running the JS engine) goes through tokio::task::spawn_blocking or a dedicated thread. JS execution threads must be spawned with the JS_THREAD_STACK_BYTES stack size — the interpreter is deeply recursive and the default stack will overflow.
No lock guards across .await. Use std::sync::Mutex for short critical sections that never span an await; if you genuinely must hold a lock across an await point, that's the one case for tokio::sync::Mutex — but first ask whether a channel or an owning task removes the shared state entirely. Message passing over shared locks where either fits.
tokio::spawn is for actual concurrency, not a cheap function call. A spawned task's JoinHandle is either awaited or its abandonment is explicit and commented; let _ = some_future; silently does nothing.
Be deliberate about cancellation safety in select! loops: a branch losing the race drops its future mid-flight. Anything with side effects that must complete (journal writes, checkpoint flushes) belongs in a task or a cancel-safe pattern, not directly in a select! arm. Use RAII/drop guards for cleanup that must run even when a task is cancelled.
Bound everything: channels get explicit capacities, retries get caps and backoff, per-run resource use goes through the existing limit machinery.

Unsafe code

chidori-js contains zero unsafe and must stay that way — "pure-Rust, no unsafe" is a documented property of the engine (see docs/architecture.md) and part of the sandbox story.
In chidori, unsafe is confined to the OS-isolation layer (runtime/isolate/* — rlimits, seccomp, landlock FFI) and mem_guard.rs. New unsafe outside that neighborhood needs a strong justification in the PR description.

Every unsafe block is minimal (one operation per block where practical) and carries a // SAFETY: comment stating the invariant that makes it sound — what must be true, and why it is true here:

// SAFETY: setrlimit is async-signal-safe and `lim` is a valid, initialized
// rlimit struct; we are single-threaded post-fork at this point.
unsafe { libc::setrlimit(libc::RLIMIT_AS, &lim) };

Documentation and comments

The house comment style is prose-heavy and rationale-driven — see the workspace Cargo.toml or any runtime/ module header for the register. Keep it that way:

//! module docs on every module; /// docs on every pub item. For fallible or panicking public functions, document the failure modes (# Errors, # Panics) — C-FAILURE.
Comments explain why, not what: the constraint, the invariant, the rejected alternative, the workaround's upstream issue link. If a comment paraphrases the next line of code, delete it. If a decision took you ten minutes of thought, spend one more writing down the reasoning — the comment is for the person (possibly you) who would otherwise have to re-derive it.
Design-level reasoning goes in docs/*.md and gets linked from the module header, not duplicated across code comments. New subsystems of any size get a design doc; that's the established pattern (~30 docs and counting).
Doc examples compile (they're tests). Use ? in examples rather than unwrap() (C-QUESTION-MARK).

Testing and benchmarks

Unit tests live inline in #[cfg(test)] mod tests next to the code they exercise. Integration tests live in the crate's tests/ directory and exercise the public surface. Async tests use #[tokio::test].
Engine language behavior is validated by Test262 via test262-runner (see docs/conformance.md) — don't hand-write unit tests for spec behavior the suite already covers; do add a focused regression test when fixing a bug the suite missed.
Name tests for the behavior and the condition (resume_skips_replayed_llm_calls), not test_1. A failing test's name should tell you what broke before you open the file.
Tests must be deterministic and parallel-safe: tempfile for filesystem state, no fixed ports, no sleeps as synchronization, no reliance on wall clock. A test that needs the network is an integration test behind an explicit opt-in, never part of cargo test --workspace.
Performance work goes through the criterion benches (benches/) and the JS workload suite; claims about speedups in PRs come with bench numbers. cargo test --workspace plus cargo fmt --check is the CI gate — keep both green locally before pushing.

Dependencies and Cargo

Adding a dependency is an architectural decision: prefer std and existing deps first; check maintenance, transitive weight, and license (workspace is Apache-2.0; deps must be permissively licensed). Say why in the PR.
No unused dependencies. A declared-but-unused crate is worse than a missing one — it implies a convention nobody follows.
TLS is rustls everywhere (reqwest is configured without OpenSSL); don't introduce anything that links OpenSSL or other C libraries lightly — the single-static-binary install story depends on it.
Platform-specific code is cfg-gated the way runtime/isolate already is (cfg(unix), cfg(target_os = "linux")), with a portable fallback or a clear compile error, and gets covered by the os-isolation CI workflow when it touches sandboxing.
Shared package metadata is inherited from [workspace.package] (field.workspace = true); keep new common fields there rather than repeating them per crate.

Public API design

For the publishable crates (chidori, chidori-js), the Rust API Guidelines checklist applies; the highlights we care most about:

Derive the common traits eagerly where semantically valid: Debug on every public type (non-negotiable — C-DEBUG), plus Clone, PartialEq/Eq, Hash, Default, and serde derives where they make sense.
Prefer newtypes over primitives for identifiers and quantities that can be confused (Seq, session ids, byte budgets) — a u64 parameter named seq next to one named limit is an accident waiting to happen (C-NEWTYPE).
Avoid boolean and Option parameters that read as mystery values at the call site; use a two-variant enum or a builder (C-CUSTOM-TYPE, C-BUILDER). Constructors are static inherent methods (new, with_config), conversions use From/TryFrom rather than ad-hoc methods.
Keep struct fields private and take impl AsRef<Path> / impl Into<String> style generics at ergonomic boundaries — but only where the flexibility is actually used; don't generalize speculatively.

Adopting this guide

Parts of the tree predate these rules — most visibly the density of .unwrap() in crates/chidori/src and stray println! in runtime modules. The policy for closing the gap:

New and modified code follows the guide. Review against it.
Clean up opportunistically, in scope. When a change touches a function, bring that function up to the guide (convert unwraps on fallible operations to ? + context, replace runtime println! with tracing).
No drive-by churn. Don't send mass mechanical rewrites mixed into feature work; a focused, reviewable cleanup PR for one module at a time is fine.
Clippy is already gated in CI and the pre-commit hook, so the tree stays warning-free by construction; the remaining gap is the unwrap() / println! backlog, which items 1–3 burn down over time.

References

Rust API Guidelines and the checklist
Rust Style Guide (the spec rustfmt implements)
Clippy lint list and usage
"Using unwrap() in Rust is Okay" — the panic policy this guide adopts
Tokio tutorial — spawning, shared state, channels
tokio::sync::Mutex docs — on when not to use it
Repo-specific design docs: docs/architecture.md, docs/sandbox-model.md, docs/replay.md, docs/conformance.md

On this page