Skip to content

AGENTS.md

Latest commit

 

History

History
194 lines (168 loc) · 45 KB

AGENTS.md

File metadata and controls

194 lines (168 loc) · 45 KB
Raw

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

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.

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

env RUST_LOG=warn,liboxen=debug,integration_test=debug bin/test --no-capture test_name

Code Quality

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:

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

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

CLI Usage

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 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 — 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. 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.
  • 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 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. Tests that open the same remote helper and then each clone it their own way (a subtree, a depth, --all, a branch) are one test as well, since a clone only reads the remote: clone into a separate directory per case under the one fixture, and do any step that changes the remote, such as pushing a new branch, after the clones that expect it unchanged.
  • A change that merges or deletes tests keeps executing every line of non-test code the old tests executed. Much of that never shows in an assertion (a server branch a fixture's setup happens to reach, an error path a misnamed test hits by accident), and reading the code cannot settle which lines a test reaches, so measure it. In a fresh worktree of the base commit and one of the change, run eval "$(cargo llvm-cov show-env --sh)", cargo llvm-cov clean --workspace, bin/test, and cargo llvm-cov report --lcov --output-path <arm>.lcov. That environment instruments the oxen-server that bin/test starts as well, which writes its counts when bin/test stops it. Compare the non-test lines the two runs executed, matching lines by file and source text, since edits shift line numbers. Each line only the base run reached either runs again, by choosing the merged test's paths, data, or loop entries so that it does, or by adding a call and an assertion to a test whose fixture already reaches it, or belongs to code the change deletes on purpose. A line reached only by tests the change left alone is run-to-run variation, so confirm each candidate by running just the touched modules under coverage on the base worktree.
  • 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.
  • Tests that build the same fixture and then each edit it their own way are one test when each case can work in its own corner of it: give each case its own workspace under the one fixture, and its own branch when it commits, as the repositories::workspaces::data_frames tests do. Only a step that leaves the fixture unusable for the cases after it, such as deleting a commit's node, has to come last.
  • Decide whether merging tests pays from each test's own duration, taken with --test-threads=1 as the Testing section above describes, never from a module total taken in a full run (a module reported at 29s there sums to 1.6s alone). Dozens of tests at a few hundredths of a second that each build the same fixture spend most of that time in the fixture, and a case moved into a merged test costs only its own operations, so the merge recovers most of their summed time: 42 such repositories::workspaces::data_frames tests summing to 1.4s became 6 summing to 0.66s.
  • Never sleep in a test. A commit records its time to below a second, so two commits made back to back already sort by date, and work that runs in the background gets polled at a 2ms interval against a bounded deadline rather than waited out.
  • 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 LazyLocks 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)].