Oxen

**AGENTS.md**

This file provides guidance to AI agents when working with code in this repository.

# Project Overview

Oxen is a fast, unstructured data version control system written in Rust. It's designed to version large machine learning datasets efficiently and provides both a CLI tool and server implementation.

# Project Organization

The Cargo workspace lives at the repository root, with crates under `crates/`:
- `crates/liboxen/` - Core shared library (`liboxen`)
- `crates/oxen-cli/` - Command-line interface binary (`oxen`)
- `crates/oxen-server/` - HTTP server binary (`oxen-server`)
- `crates/oxen-py/` - Python bindings (Rust source for `oxen-python`)
- `oxen-python/` - Python package source, tests, and `pyproject.toml`

## Architecture

The project follows a workspace structure with crates in the `crates/` directory:

- **`liboxen/`** - Core shared library (`liboxen`) containing all the business logic
- **`oxen-cli/`** - Command-line interface binary (`oxen`)
- **`oxen-server/`** - HTTP server binary (`oxen-server`)

The CLI and server both depend on the shared library to avoid code duplication. All core functionality should be implemented in the lib first, then exposed through appropriate interfaces in the CLI and server.

## Key Components

### Core Architecture (`crates/liboxen/src/`)
- **`core/`** - Core data structures and database operations (RocksDB for metadata, DuckDB for tabular data)
- **`model/`** - Data structures representing commits, branches, entries, diffs, etc.
- **`repositories/`** - Repository operations (init, clone, add, commit, push, pull, etc.) - Most high-level operations start here
- **`view/`** - Response/view models for API endpoints
- **`storage/`** - Storage backends (local filesystem, S3)

### Data Storage
- Uses RocksDB for metadata and version control information
- Uses DuckDB for tabular data processing and querying
- Implements Merkle trees for efficient change detection

## Common Development Commands

*IMPORTANT*: Our codebase assumes cargo commands are run on the whole workspace, from the workspace root, _NOT_ on specific packages.
GOOD: `cargo check --workspace`
BAD: `cargo check --package liboxen`

### Building
```bash
cargo build --workspace                           # Debug build
cargo build --workspace --bins --tests            # Debug build that a later `cargo test` reuses
```
A `cargo build` that a `cargo test` follows, in a script or a CI job, builds `--bins --tests` with the same packages and features the test command selects, as `bin/test` and the Rust Tests jobs do. Dev-dependencies add features to crates such as `hyper` and `getrandom`, so a plain `cargo build` resolves a different feature set, and the `cargo test` after it compiles a second copy of every dependency above them, DuckDB, RocksDB, polars and the AWS SDK included.

### Testing
Use the `bin/test` script to run the tests — it is the standard, supported path. The script builds the workspace, raises the file-handle limit, sets up a ramdisk for test data, starts `oxen-server` on a free port, exports the environment the tests expect, runs the suite with `cargo test --workspace --no-fail-fast`, and tears everything down on exit. Its full usage is documented in a comment at the top of the script. Running `cargo test` directly is supported only if you first reproduce that setup by hand (a running `oxen-server` on the default host/port, user config, a raised file-handle limit, and the env vars the script exports — see the "Manual Test Setup" section of `crates/liboxen/README.md`); without that setup the tests fail, so prefer `bin/test`.
```bash
bin/test                         # Build and run all Rust tests
bin/test test_function_name      # Run only Rust tests matching test_function_name
bin/test --timings test_name     # Report each matching test's duration
bin/test -p                      # Run the Python test suite (via pytest + maturin)
bin/test -p -k test_init         # Run Python tests matching test_init
```
- The script starts `oxen-server` itself, so you do not need to start it separately.
- It does not install prerequisites by default. If a dependency is missing, run `bin/install-prereqs` (or re-run with `bin/test --install-deps`).
- If the ramdisk cannot be mounted, pass `--no-ramdisk` to run against the regular filesystem.
- `--timings` prints each test's duration next to its result; plain `cargo test` reports only a per-binary total. Doctests are skipped. When comparing numbers, discard the first run after a build (a cold run reads two to three times high) and keep both sides on the same base commit (main moves fast enough that an older base is a different suite).
- **Run both sides of a timing comparison from fresh `git worktree` checkouts, and alternate them.** A long-lived working tree reports the same unchanged tests markedly slower than a worktree of the same commit does (a 23-test set summed 14.8s in one against 11.5s in a worktree, with identical code), so a working-tree-against-worktree comparison charges that gap to whichever side the working tree is on. Alternating the arms covers the other half: run them in a fixed order and a drifting machine shows up as a change in whichever side always runs second. Sanity-check every result by summing the tests the change did not touch, which should match across arms to a percent or two, and distrust the comparison when they do not.
- **Measure what a test costs with `--test-threads=1`.** A duration from a parallel run includes the time a test spent waiting on its neighbors, even when the filter selects a single module: the 49 `repositories::commits::` tests summed 12.7s run together and 2.7s run one at a time. A change that removes tests therefore makes the tests it left alone look faster in a parallel run, which fails the untouched-tests check above for a reason that has nothing to do with the change.
- Arguments after the script's own flags are forwarded after `--` to the libtest binaries (or `pytest` under `-p`).
- Output the terminal doesn't show goes to two log files, both overwritten per run: `./data/test/oxen-server.log` (server subprocess stdout+stderr) and `./data/test/cargo-test.log` (cargo test's stderr — indicatif progress bars, and any `tracing` output emitted from tokio worker or `spawn_blocking` threads that don't inherit libtest's per-thread capture). Check these first when the terminal report doesn't explain a failure.

### Testing with Debug Output
```bash
env RUST_LOG=warn,liboxen=debug,integration_test=debug bin/test --no-capture test_name
```

### Code Quality
```bash
cargo fmt --all                                    # Format code
cargo clippy --workspace --no-deps -- -D warnings  # Lint code
pre-commit run --all-files                         # Run pre-commit hooks (runs format and lint)
```

### Benchmarks
The `liboxen::test` helper module is gated behind the `test-utils` feature so it isn't compiled
into release builds. Benches that consume it (`download`, `fetch`, `push`, `workspace_add`)
declare `required-features = ["test-utils"]` and are skipped unless the feature is enabled:
```bash
cargo bench --features liboxen/test-utils           # Run all benches
cargo bench --features liboxen/test-utils --bench push
```
Downstream crates (`oxen-cli`, `oxen-server`) enable `test-utils` via dev-dependencies, so their
own tests can call `liboxen::test::*` as before.

### Server Development
```bash
ulimit -n 10240                     # Increase file handles before running the server
bacon server                        # Start server with live reload
```

### CLI Usage
```bash
export PATH="$PATH:/path/to/Oxen/target/debug"
oxen init                           # Initialize repository
oxen status                         # Check status
oxen add images/                    # Add files
oxen commit -m "message"            # Commit changes
oxen push origin main               # Push to remote
```

## Code Organization
- We define module exports in a `<module_name>.rs` file at the same level as the corresponding `module_name/` directory and *NOT* the older `mod.rs` pattern.
- Prefer importing bare items (structs, enums, traits, functions, constants, macros) and referring to them unqualified, rather than importing a parent module and qualifying at each use site — e.g. `use std::time::Duration;` then `Duration::from_secs(5)`, not `std::time::Duration::from_secs(5)`. Exception: keep enough of the path to disambiguate when a bare import would be ambiguous or misleading, such as two same-named items from different modules, or where a module qualifier is the established idiom (the classic case is `use std::fmt;` then `fmt::Result` to avoid clashing with the prelude `Result`); reach for an `as` alias when that reads better than a module qualifier.
- **Take the concrete type, and make a generic earn its place.** When you write or change a function, give each parameter the type its callers pass: `&str`, `&Path`, `&OsStr`, `&[T]`, `String`, `PathBuf`. Deref coercion already turns `&String`, `&PathBuf`, `&OsString`, and `&Vec<T>` into the borrowed form, so a conversion bound (`impl AsRef<_>`, `impl Into<_>`, `impl Borrow<_>`) or a type parameter buys those callers nothing. It only adds monomorphization cost and `.as_ref()`/`.into()` noise in the body.
  + Add a generic, or keep an existing one, only when the function must accept whatever type its caller has, such as one taking any iterator (`impl Iterator<Item = T>`) or any reader (`impl Read`).
  + Callers passing different types is not a reason, even when coercion can't unify them: if some pass a `&str` and others a `&PathBuf`, take `&Path` and let the `&str` callers write `Path::new(s)`. Nor is a lone `""`/`String::new()` caller: let it write `.to_string()`, or take `&str` if no ownership is needed. Nor is matching the surrounding functions, even though much of this codebase uses `impl AsRef<str>` and `impl AsRef<Path>`.
  + Only reconsider a function's generics if you're already changing that function for the task at hand. Leave generics in unrelated functions alone, even if you'd write them differently.
- **Give every new item the narrowest visibility that compiles, and widen only when a real caller at that distance appears.** Order of preference: private → `pub(super)` / `pub(crate)` → `pub`. In `liboxen`, `pub` carries a specific meaning — *this is intended for `oxen-server`, `oxen-cli`, or the Python bindings to call.* An item that is `pub` merely because nobody chose otherwise is unearned public API that future refactors have to preserve. Applies to every new `fn`, `struct`, `enum`, `trait`, `const`, type alias, field, and module, and to each name added to a `pub use` re-export (a re-export is public surface too). The asymmetry is the reason: widening later (private → `pub`) is backward-compatible and cheap, narrowing later (`pub` → private) is a breaking change. Two things that catch people out: a **child module can already read its ancestors' private items through `super`**, so a helper shared between a parent and its children needs no `pub` at all; and a `pub fn` returning a third-party type leaks that dependency into the wider API even when the module otherwise contains it. When an item genuinely *is* the intended surface, `pub` is correct — the rule is against breadth by default, not breadth by intent.

## Error Handling
- **Read [docs/error_handling.md](docs/error_handling.md) before writing or changing any error handling, and follow it.** Not a reference to consult when stuck — read it up front whenever a change adds an `OxenError` or `OxenHttpError` variant, changes how one is raised or classified, alters an HTTP status or a log level, or touches `hint`, `is_not_found`, `is_fatal_for_retry`, or `error_response`. It carries a checklist for the parts that are routinely missed, because **every one of them fails silently**: classification lives in `match` arms that fall through to a default, so a variant missing from a list still compiles, still passes the tests, and is simply classified wrong at runtime. The compiler will not catch these for you, and neither will the existing suite.
- After adding a variant, walk that checklist rather than assuming the default is right: the wrong HTTP status, a missing hint, an absent not-found or fatal-for-retry classification, and a variable interpolated into a log message each have a specific and non-obvious cost spelled out there.
- Use the result type (`Result<T, Error>`) when an operation could fail.
- Never use `.unwrap()` or `.expect()` on a `Result` or on an `Option`.
  + Exception: In test-only code, it is ok to use use `.expect(<descriptive explanation of invariant that was violated>)` since we want to fail fast and have good stack traces for failing test cases.
  + This rule still applies when the panic feels "guaranteed unreachable" because of an upstream invariant. If the enclosing function returns a `Result`, propagate it with `?` — a panic in production code is never preferable to a clean error path, no matter how confident you are the case can't happen.
- If an error type is acted upon in code, then use as specific of an error type as possible. Don't use a wider type unless it's necessary. When making modules and related pieces of code, try to use a locally-defined error enum for them if they all have similar errors.
- liboxen uses `OxenError` as the top-level type for errors. Unify different error types under `OxenError`. Fallible functions should return `Result<T, OxenError>`. Do not create or use additional error types outside of `OxenError` if it can be avoided. If an error is never inspected internally and cannot be returned to a caller of the liboxen library through a public API, then use `OxenError::InternalError` with a formatted string. If an error is inspected or can be returned to a caller of the liboxen library through a public API, make a structured error variant on the `OxenError` `enum`.
- oxen-server uses `OxenHttpError` as the top-level type for errors. All `OxenError` variants that we want to differentiate to the caller of the oxen-server API should be mapped to specific `OxenHttpError` variants. Otherwise they should be mapped to `OxenHttpError::InternalServerError`. Do not create or use additional error types outside of `OxenHttpError` if it can be avoided.
- Implement proper error propagation through the `?` operator.

# Making Changes

- This repository is **public**. Do not mention Oxen's private/internal repositories — by name or description — in code comments, doc-comments, error messages, commit messages, PR titles or descriptions, or any other code or documentation committed here. Keep references to private repos out of public artifacts entirely; if internal context is genuinely needed, point to the relevant Linear issue rather than inlining private-repo details. This also bars describing a private/internal system's behavior as the justification for a change — even when the system is not named (e.g. "a downstream client loops forever without this"); describe what the code in this repo does and guarantees instead.
- When changing something that is documented in nearby code, or appears in any markdown files in the repository, update the affected documentation.
- The `oxen` client (the CLI and other user-facing client behavior) is documented on the public docs site at docs.oxen.ai, maintained in the separate `docs` repo (Mintlify/MDX — the CLI guides live under `getting-started/command-line/`). After any change to the client — a new or changed command, flag, default, or user-visible output — check whether a docs page needs updating, and update it (likely as a separate `docs`-repo change) or call out the gap.
- The Python API pages on docs.oxen.ai are **generated** from the `oxen-python` docstrings by `generate-python-docs.sh` in the `docs` repo, so a docstring change is not published until those pages are regenerated. After editing a docstring under `oxen-python/python/oxen/`, or adding a method or property to one of those classes, regenerate them as a separate `docs`-repo change by running `<docs-checkout>/generate-python-docs.sh <docs-checkout>/` from `oxen-python/`. The script rewrites all of its pages and has no per-page flag, so that change carries every page whose docstrings moved. Write each docstring for what pydoc-markdown does with it: an indented `word: value` line inside an argument's description starts a *new* argument on the generated page, so a `Default: False` sitting on its own line publishes a `Default` argument that does not exist. Keep such a sentence on the same line as the description it belongs to.
- When prompted to always do something a certain way in general, add an entry to this section of the AGENTS.md file.
- **Server-global state lives directly under `$SYNC_DIR`, never under a `$SYNC_DIR/.oxen/` directory.** A file goes at `$SYNC_DIR/<name>` and a set of files in `$SYNC_DIR/<dir>/`, alongside `last_migration.txt` and the `name_table/` index. A `.oxen` directory means "this is a repository" everywhere else in the product, so one at the top of the sync dir makes the whole data directory answer to `util::fs::get_repo_root`, which the CLI walks up from the working directory to find. Two things a new top-level entry has to do: add its name to `SERVER_OWNED_DIRS` in `crates/liboxen/src/sync_dir.rs`, since `namespaces::list` and `namespaces::get` otherwise report the directory as a namespace on `GET /api/namespaces` and `GET /api/namespaces/<name>`, and `sync_dir::namespace_dirs`, which every walk over the server's repositories starts from, treats it as one, and leave it out of `SelfHosting.md`, which covers what an operator configures and sees rather than the server's internals (`last_migration.txt` is likewise absent) — document there only the operator-visible behavior the state creates, if any.
- **Until 1.0, `SelfHosting.md` covers the current state of hosting, not how it got there.** Leave out past behavior, legacy on-disk forms, upgrade notes, and migrations (a step a CLI user or self-hosted operator runs on upgrading goes in `docs/migrations.md`) — an operator reading it wants to know what to configure and what the server does now. When a current behavior carries a caveat that applies only to repositories an older release created, drop the caveat rather than explaining the older release in order to justify it.
- **A RocksDB database a repository opens by path goes through a `WeakDbCache` registry** (`crates/liboxen/src/core/db/weak_cache.rs`), the way refs, staged, the workspace name index, and the commit count caches do: a `LazyLock<WeakDbCache<_>>` static with its own warm capacity, opening through `get_or_open`. Never hold a process-wide lock across `DB::open`, which makes every repository's open wait on every other's, and never reopen the database on every call. Kept-warm handles hold the database's files open after the last caller drops them, so code that moves or removes a directory holding one evicts through the store's `remove_*_from_cache_with_children` both before and after the filesystem call, capturing the call's result and propagating it after the second eviction, since a handle opened while the move or removal runs keeps the old files open. A new registry also gets an eviction call in the test cleanup helpers that remove repository directories (`maybe_cleanup_repo` in liboxen, `cleanup_sync_dir` in oxen-server), which evict every registry first.
- When calling `get_staged_db_manager`, follow the doc comment on that function: drop the returned `StagedDBManager` as soon as possible (via a block scope or explicit `drop()`) to avoid holding the shared database handle longer than necessary.
- When altering the `OxenError` enum, consider whether a hint needs to be added or updated in the `hint` method.
- Instead of using `cargo test` to test Rust code, use the `bin/test` script. The script usage is documented in a comment at the top of its file.
- If the ram disk is not able to be mounted in `bin/test`, then use the `--no-ramdisk` option.
- The `bin/test` script does not install prerequisites by default. If any dependencies turn out to be missing, prompt the user to run `bin/install-prereqs` (or re-run `bin/test --install-deps`).
- Prefer using inline code over creating a new function when the function would only be called once and the function body would be less than 15 lines.
- Do not use "out parameters" (functions that take an `&mut Vec` / `&mut HashMap` / etc. for the callee to fill). Return the value directly instead. Exceptions: the user explicitly asks for an out parameter, or the caller genuinely needs to reuse a pre-allocated buffer across many calls to avoid allocation churn in a measured hot path.
- Don't add type annotations the compiler doesn't need. Let inference do its job and annotate only where the code won't compile (or won't resolve to the type you intend) without it — e.g. a `collect()`/`parse()`/`sum()` whose target type is otherwise ambiguous, or an integer literal that needs pinning. Redundant annotations on bindings whose type is already obvious from the right-hand side just add noise and rot when the expression changes.
- Version-gated deprecation checks must inline the triggering version as a literal right at the comparison site, with a brief comment naming what it gates and pointing to `docs/deprecations.md`. Do not hoist it into a named constant defined in another module/crate (e.g. `crate::constants::SOME_VERSION`): the point of the check is to see *which version* trips it while reading the check, and the indirection defeats that. (This is distinct from broad, widely-reused version floors like `MIN_OXEN_VERSION`, which legitimately stay named constants.)
- When adding server endpoints that supersede existing ones, introduce them additively alongside the old endpoints rather than replacing them. Switch the client to the new path, register the old endpoints in `docs/deprecations.md` with a removal target ~5 minor versions out, and remove them in a later tech-debt PR. Avoids flag-day breakage for older clients still in the field.
- Preserve code comments whenever possible. Comments that were written by someone other than Claude should always be preserved or updated if possible.
- The set of merkle nodes + version blobs reachable from a commit is the **reachable objects** / **reachable set** — do not call it the "closure" or "object closure." Applies to code identifiers, comments, error messages, commit messages, and issue text. (The word "closure" is fine in its unrelated senses: Rust closures.)
- Function doc-comments describe **what** the function does, not **how** it does it — unless the "how" is caller-relevant (ordering constraints like "call before `commit()`", cross-filesystem rules like "src and target must live on the same filesystem", thread-safety invariants like "never hold a guard across `.await`"). The body shows the implementation; the doc-comment's job is to convey the contract callers rely on, which rots if it duplicates internal mechanism. Trim "stamps the temp file with mtime before the rename" to "the published file carries mtime"; trim "uses the Channel hand-off helper" to whatever visible behavior the caller observes. Applies to public and private doc-comments alike, and to inline comments that risk narrating implementation details rather than describing current behavior.
- Before manually serializing a value (hand-built `serde_json::json!({...})`, a one-off request/response struct, format-string assembly), check whether an existing type or helper already serializes it. Most domain types already derive `Serialize`/`Deserialize` (and often `utoipa::ToSchema`), and there are usually conversion helpers between them (e.g. `UserConfig::to_user()` yields a `User` that serializes to the exact `{name, email}` the merge endpoint expects). Reuse the existing type with `.json(&value)` / `serde_json::to_*` and deserialize back into that same type rather than re-describing its shape by hand — the manual version silently drifts from the canonical type when fields change. Introduce a new serialization struct only when no existing type fits.
- The Python project calls into the Rust project. Whenever changing the Rust code, check to see if the Python code needs to be updated.
- **Always update the corresponding Python interface when a Rust model or view gains a field.** `PyWorkspace`, `PyCommit`, `PyRemoteRepo`, and the other `crates/oxen-py/src/py_*.rs` wrappers each mirror a Rust type, and so do their `oxen-python/python/oxen/*.py` counterparts. A new field on the Rust side is not done until both layers expose it — do not conclude "Python doesn't need this" merely because the crate still compiles and no test fails. Follow the existing precedent for the field's type (`PyCommit::timestamp` returns a `String`, so a timestamp crosses as an RFC 3339 `String`, not a native object), add the getter to the `#[pymethods]` block, and add the matching `@property` with a docstring to the Python class.
- After changing any Rust or Python code, verify that Rust tests pass with `bin/test` and Python tests pass with `bin/test -p`
- When updating a dependency, prefer updating to the latest stable version.
- When adding or updating GitHub Actions in `.github/workflows/`, pin them per the org-wide policy in [docs/github_actions_pinning.md](docs/github_actions_pinning.md) — version tag for first-party/high-trust orgs, full commit SHA (with a `# vX.Y.Z` comment) for third-party.
- **`Re-run failed jobs` never works for the release/build jobs that run on self-hosted EC2 runners** (Windows, Linux, and Docker jobs in `release_*.yml` and `build_wheels_*.yml`). Those runners are provisioned per-attempt by a sibling `start-self-hosted-runner` job and terminated by a `stop-self-hosted-runner` job at the end of the attempt. A re-run of only the failed job reuses the *succeeded* start job's cached output — a `runs-on` label whose instance no longer exists — so the job sits queued until GitHub times it out (~24h) with no error. Use `Re-run all jobs`, which re-executes the start job and provisions a fresh runner, or push a new tag. Diagnose it by comparing the start job's `started_at` against the failed job's: a start job timestamped from the earlier attempt is the tell.
- Code that touches IO follows the **sync-core / async-edge** policy in [docs/async_policy.md](docs/async_policy.md). Summary: public APIs are `async fn`; network IO (AWS SDK, reqwest, etc.) uses native async APIs; filesystem and sync-DB IO (`std::fs`, RocksDB, LMDB, DuckDB) runs inside `tokio::task::spawn_blocking` at *operation* granularity, not per syscall; DB transactions live entirely inside one `spawn_blocking` closure (never spanning `.await`); CPU-parallel batch work uses `rayon` inside one `spawn_blocking`, not `FuturesUnordered<spawn_blocking>`; streaming sync↔network IO uses a long-lived `spawn_blocking` task paired with the async side via `tokio::sync::mpsc`. Do **not** reach for `tokio::fs` in hot loops — it dispatches each syscall through the blocking pool and pays per-call overhead. See the doc for the full patterns (Bracket, Sandwich, Channel hand-off) and anti-patterns.
- In `oxen-server`, move work off the request thread with `crate::tasks::spawn_blocking` (blocking closures) or `crate::tasks::inherit_hub` (a future, before `tokio::spawn` / `JoinSet::spawn`) instead of `tokio::task::spawn_blocking`, `tokio::spawn`, or `actix_web::web::block`. The per-request Sentry hub *and* the current tracing span are thread-locals bound only around polls of the request future, so a task spawned the bare way reports a panic with no route and no request, and its spans are exported detached from the request's trace. Use `crate::tasks::spawn_blocking_per_item` for blocking work dispatched once per item in a loop — same context, but no span of its own, because a bulk endpoint handling thousands of files per request would otherwise emit a span per file. Recognize it by asking whether the response is still in flight: if the handler awaits the task, or the task produces the response body, it needs the request's context — and a response-body future must be bound at construction time inside the handler, since the middleware's binding is gone by the time a body is polled. Tasks that deliberately outlive their response (a background delete, a cleanup at stream EOF, a process-lifetime loop) keep using tokio's own spawn. This is `oxen-server` only: liboxen calls tokio directly because it must not depend on `sentry`. When auditing, remember `actix_web::web::block` is a `spawn_blocking` that no `spawn_blocking` search will find. Full rationale and the checklist: [docs/async_policy.md](docs/async_policy.md).
- Streamed IO (anything reading or writing through an `AsyncRead`/`AsyncWrite` whose total length isn't bounded ahead of time) must use a large buffer rather than rely on `tokio::io::copy`'s 8 KB default. Wrap the write side in `tokio::io::BufWriter::with_capacity(10 * 1024 * 1024, ...)` (and the read side in `BufReader` if the source isn't already buffered), and remember to explicitly `flush().await?` the `BufWriter` before any downstream `sync_all`/rename/checksum step — `BufWriter`'s `Drop` does **not** auto-flush, so unflushed bytes are silently dropped. The canonical example is the S3 store's local-cache path in `crates/liboxen/src/storage/s3.rs` (`copy_version_to_path`). See [docs/async_policy.md](docs/async_policy.md) for the broader async/sync context this fits into.
- All production filesystem writes go through `AtomicFile` (`crates/liboxen/src/util/fs.rs`) — never `std::fs::write` / `tokio::fs::write` / `File::create` to a canonical path directly. `AtomicFile` writes to a sibling temp file and atomically renames over the target, so a crash mid-write can never leave a torn or partially-written canonical file. Chain `.with_hash(h)` for content-addressed writes and `.with_mtime(t)` when publishing into the working tree. The non-atomic writers `util::fs::write` and `util::fs::write_to_path` are gated to test / `test-utils` builds for fixture setup only. Detailed contract on the `AtomicFile` struct rustdoc.
- Prefer `bytes::Bytes` over `&[u8]` and `Vec<u8>` for byte payloads that cross a module, trait, `spawn_blocking`, or external-SDK boundary. `Bytes` is `'static + Clone + Send` and refcounted: `Bytes::from(Vec<u8>)` reuses the allocation (zero copy), `Bytes::from_static(b"...")` is compile-time (zero cost), and cloning a `Bytes` is a refcount bump rather than a memcpy. Most async IO ecosystems we touch (`axum` request bodies, `reqwest::Response::bytes_stream`, `aws_sdk_s3::primitives::ByteStream`) speak `Bytes` natively — handing them a `Vec<u8>` costs an allocation and handing them a `&[u8]` costs an allocation **and** a memcpy. The canonical example is `VersionStore::store_version` in `crates/liboxen/src/storage/version_store.rs`: the trait signature went from `data: &[u8]` to `data: Bytes` so the in-memory writers can move bytes across the `spawn_blocking` boundary as a refcount transfer and the S3 impl can pass through to `ByteStream::from` without a `.to_vec()` copy. `&[u8]` is still the right type for transient borrows that don't cross a boundary (hashing once and discarding, inline inspection); use `BytesMut` for growable buffers and `freeze()` into `Bytes` when handing them off (e.g. the `read_buf` + `split().freeze()` pattern in `AtomicFile::stream_async`).
- Stream a file's *content* rather than buffering the whole thing into memory: a function that reads or writes file data should pass it incrementally, using whatever streaming primitive fits the layer — an `AsyncRead`/`AsyncWrite`, a byte-chunk `Stream` (e.g. the `BoxedByteStream` alias the version store and client return), a chunked reader, an HTTP body — instead of materializing a whole-file `Vec<u8>` / `Bytes`. Every path that handles file data needs to eventually handle files larger than memory, so whole-file reads don't scale. Where a caller genuinely needs the entire payload in memory, collect it inline at the call site (a visible `while let Some(chunk) = stream.next().await { buf.extend_from_slice(&chunk?); }` drain, or `read_to_end` for an `AsyncRead`) rather than hiding it behind a shared collect helper — keeping each buffering site apparent and greppable for future migration to full streaming. (This rule is about whether to hold the whole payload at all; the `Bytes`-over-`&[u8]` rule just above is about which type to use for payloads you *do* hold.)
- When computing the number of fixed-size chunks needed to cover a total byte count, use `total_size.div_ceil(chunk_size)`, not `(total_size / chunk_size) + 1`. The `+1` form overshoots by one when `total_size` is an exact multiple of `chunk_size`, producing a spurious zero-byte chunk request that the server rejects with HTTP 500 "beyond end of file" — see the bug fixed in `download_large_entry` for what this looks like in production.
- oxen-server operations should never touch a local checkout on disk when doing operations initiated by its API.
- When traversing a repository's working tree, use `metadata.is_dir()` instead of `path.is_dir()`. `path.is_dir()` follows symlinks, which Oxen does not track — using it risks descending into directories outside the working tree (or into cycles via cyclic links). This applies to the working tree only: walks over oxen-server's sync dir and over a repository's `.oxen` internals follow symlinks, as the server does when it serves a repository.
- Oxen does not track symlinks. New code that traverses the working tree should check `metadata.is_symlink()` and skip rather than resolve, follow, or record symlinks.
- Never use `unsafe` code when a safe alternative would meet our needs. Reach for `unsafe` only when there is a concrete reason no safe construct will do (e.g. FFI, a measured performance requirement that safe code cannot satisfy, no clear alternative); in that case, justify the choice in a comment at the `unsafe` site.
- Any `unsafe` block or `unsafe fn` must be preceded by a `// SAFETY:` comment that explains why the operation is sound — which invariants the caller relies on, why they hold here, and what would break if they didn't. Follow the Rust style guide: <https://std-dev-guide.rust-lang.org/policy/safety-comments.html>. For an `unsafe fn`, document the caller's obligations with a `# Safety` section in the doc comment, and pair each call site with its own `// SAFETY:` comment justifying that those obligations are met.

# Testing Rules
- Use the test helpers in `crates/liboxen/src/test.rs` (e.g., `run_empty_local_repo_test`) for unit tests in the lib code.
- When picking a helper from `crates/liboxen/src/test.rs`, choose the lightest one that meets your test's actual needs. Rough cost order, cheapest first: `run_empty_dir_test_async` → `run_empty_local_repo_test_async` → `run_one_commit_local_repo_test_async` → `run_readme_remote_repo_test` (remote with a single README pushed) → `run_one_commit_sync_repo_test` (local + remote, one inline commit) → `run_training_data_repo_test_no_commits_async` (training files written, not committed) → `run_training_data_repo_test_fully_committed_async` (training files + 6 commits, no remote) → `run_training_data_fully_sync_remote` (training files + 6 commits + remote + push; ~1–2s per test). Reach for the training-data helpers **only** when your assertions depend on the specific tree structure (paths under `nlp/classification/`, `annotations/test/`, etc.). A test that just modifies, renames, or asserts about `README.md` belongs on `run_readme_remote_repo_test`, not `run_training_data_fully_sync_remote`. A test whose only training-tree file is `annotations/train/bounding_box.csv` belongs on `run_bounding_box_csv_repo_test_fully_committed_async` (local, one commit) or `run_remote_repo_test_bounding_box_csv_pushed` (pushed to a remote), which write that file with the same rows the training tree does.
- A helper that hands a test back a `(LocalRepository, RemoteRepository)` pair has already attached that repo to that remote, so a test needing two actors takes that repo as one of them and clones only the other. Opening with `|_, remote_repo|` and then cloning twice builds a repo the helper already built. Which role it plays depends on the scenario: it is the user who advances the remote when the test is about a clone that has fallen behind, and it is the user who pushes first when the test is about the second clone hitting a conflict.
- Add assertions to a test that already builds the right fixture rather than writing a new test, and go looking for that host before writing the test rather than after review finds it. A new test pays for its whole fixture again, priced by the list above. A test's name only has to name something important the test checks, not everything it checks, so a name that fits less well after the addition is not a reason to build a second fixture — put the rest in the assertion message, as in `assert_eq!(mergeability.conflicts.len(), 1, "both branches rewrote README.md")`. Write a separate test only when carrying the assertion would need a tear-down and re-setup partway through the host, when the host's fixture data decides the expected value (asserting against a value computed from the fixture usually dissolves that), or when the host becomes hard to follow. Two tests in one file that open the same helper and reach the same state before their first assertion are one test, and so are two whose scenarios are prefixes of one another: assert at the earlier state, then keep building to the later one and assert again. The prefix case holds across files too: a test in one module that stops partway through a scenario another module's test runs to completion (a subtree clone and commit that a push test also clones, commits, and then pushes) is already covered, so search the other modules' tests for the scenario before writing it.
- Bound a verification loop to the cases that separate a pass from a failure, because every iteration is a full execution of the code under test. A sweep over every ordered pair drawn from N commits, or one call repeated 200 times to check that the answer never varies, reads as thoroughness while costing more than the rest of its module. Ask what the loop recomputes before writing it: a reference value derived from N distinct inputs belongs outside the loop, walked once and shared across the cases, which keeps every case and every assertion. A repeated call that samples for nondeterminism the code can no longer produce covers nothing that one call does not.
- Files under `data/test/` exist to serve specific tests. When you delete or rewrite a test, audit whether any fixture file under `data/test/` is now unreferenced and delete the orphan in the same PR. Never add a fixture file speculatively or "for a future test" — add it only when the test that uses it already exists.
- When a test needs a small directory of placeholder text files, call `test::populate_dir_with_txt_files(dir, prefix, count)` instead of open-coding `util::fs::create_dir_all` + a loop over `write_txt_file_to_path`.
- Never add a ```` ```ignore ```` doctest. An `ignore` fence is neither compiled nor run, so the example rots silently while still looking maintained. Write it so it runs **without `oxen-server`** — `cargo test --doc` has to pass on its own, so an example may use the filesystem but must not need a running server or a reachable remote. `liboxen::test`'s helpers are available to doctests: `test::run_empty_dir_test` for a sync example, `test::run_empty_dir_test_async` with a hidden `# #[tokio::main]` line for an async one; both make a unique temp directory and clean it up. When an example genuinely needs a server or a remote, use `no_run` with a line above it saying so, or don't write the example. A doctest on a private item can never run, since it compiles as an external crate.
- Only copy real fixture images into a test repo (`test::test_img_file_with_name` + `util::fs::copy`) when an assertion depends on image content or metadata — dimensions, mime type, resize behavior. When the test just needs files that exist, get added, get modified, or get counted, write small inline text files instead: `util::fs::write_to_path(dir.join(format!("cat_{i}.jpg")), format!("cat {i}"))`, or `test::populate_dir_with_txt_files`. Change a file's content with `test::modify_txt_file` rather than re-encoding an image through `util::image::resize_and_save`. `add`/`commit`/`status`/`rm` never decode file content, so a `.jpg` holding text exercises the same code paths without the fixture I/O or the image encode.
- Test runs override `OXEN_STREAM_SEGMENT_SIZE` to 128 KiB via `bin/test`, so any file larger than ~128 KiB exercises the streamed-transfer / chunked-download code paths. When writing a test that needs to exercise that path, size the file via `stream_segment_size() + N` (where N is small — anywhere from 1 byte to ~1 MiB), **not** a hardcoded multi-MB value. A 1.1 MiB test file exercises the same chunked-transfer code as a 100 MB production file at a fraction of the cost.
- When possible, put tests in the higher-level `repositories` module rather than the lower-level, version-specific implementation.
    - e.g., Tests should go in `repositories/commits.rs` rather than `core/v_latest/commits.rs`.
- Tests create unique temporary directories and clean up automatically
- Under `cargo test` each test target compiles to its own test binary and runs as a separate process — the lib's unit tests are one binary, each file under `tests/` is another, and doctests are another — so process-global state (env vars, `OnceLock` / `LazyLock` / `OnceCell` singletons, cached HTTP clients / connection pools, static caches keyed by name) is shared only among the tests within a single binary, where it can leak from one test into the next. New tests must not depend on per-process state that isn't explicitly re-initialized: don't `std::env::set_var` in a test body without a scoped restore (or a `Once`-guarded write of a constant), don't cache anything at process scope that's keyed on values a sibling test might reuse (e.g. bucket names, workspace names, repo names — use UUID-derived values), and don't build `LazyLock`s whose init reads state a sibling test might have already read at a different value. When a test genuinely needs exclusive access to a process-wide singleton (a DB-cache flush, a global counter), gate it with `#[serial_test::serial(named_key)]` — pair every test that touches the same singleton on the same key. `serial_test` only serializes tests within the same binary (its lock is process-local); it cannot coordinate tests in different binaries — and process-global statics don't cross a binary boundary anyway (each binary is its own process with its own copy), so the hazard it guards is always intra-binary. Canonical examples: the s3 cloud_reads tests mint UUID-tagged buckets in `setup()` so `polars-io`'s cloud-store cache can't collide; the `df_db` flush tests share `#[serial_test::serial(df_db_cache)]`.

More agent context in Oxen-AI/Oxen

One other file this repository gives its agents.