AGENTS.md

Bencher AGENTS.md

The goal of this file is to describe the common mistakes and confusion points an agent might face as they work in this codebase. If you ever encounter something in the project that surprises you, please alert the developer working with you and indicate that this is the case by editing the AGENTS.md file to help prevent future agents from having the same issue.

Project Overview

Bencher is a continuous benchmarking platform that detects and prevents performance regressions.

• Bencher API Server (services/api) - see services/api/AGENTS.md
• bencher CLI (services/cli) - see services/cli/AGENTS.md
• Bencher Console (services/console) - see services/console/AGENTS.md
• Bare Metal runner (services/runner) - Bare Metal benchmark runner

Terminology: use the definitions in docs/glossary.md.

Version Control

Version control uses Jujutsu (jj) with Git.

• Do NOT add Co-Authored-By lines to commit messages
• Do NOT add 🤖 Generated with [Claude Code](https://claude.com/claude-code) to PR descriptions

Public Disclosure

This is a public repository. NEVER disclose sensitive information about the production setup, data, or operations in any public-facing artifact: PR descriptions, PR/issue comments, commit messages, code comments, or docs. This includes production database sizes, row counts, table statistics, customer or project identifiers, incident details and timelines, and measured production performance characteristics. Describe changes in relative terms (e.g., "a large report" instead of actual production numbers) and keep concrete production figures in private channels.

Writing Style

• NEVER use emdashes in any writing output: code comments, commit messages, PR descriptions, docs, issue/PR replies, and chat responses. Use a colon, semicolon, comma, parentheses, or two sentences instead.

Development Methodology

• Practice test-driven development (TDD)
• All new code should be designed for testability and maintainability
• All changes should include appropriate unit and integration tests

Building

cargo build

Testing

cargo nextest run

Test a single package:

cargo nextest run -p my_package

bencher_schema tests require the plus feature (cargo nextest run -p bencher_schema --features plus); some #[cfg(test)] modules (e.g., model/project/metric.rs) use plus-only items without a feature gate, so the default-feature test build fails to compile.

nextest does not support doctests, so also run:

cargo test --doc

Crates that depend on bencher_valid will need to specify either:

1. server feature for server-side usage

cargo nextest run -p my_package --features server

2. client feature for client-side usage

cargo nextest run -p my_package --features client

Otherwise, you will see:

use of undeclared type `Regex`

bencher_otel has no tests, so nextest needs --no-tests pass (cargo nextest run -p bencher_otel --no-tests pass). With --all-features it also needs --features bencher_json/server, since its plus feature reaches bencher_valid through bencher_json without picking a side.

Formatting

cargo fmt

Linting

cargo clippy --no-deps --all-targets --all-features -- -Dwarnings

Checking non-Plus

cargo check --no-default-features

Linux Cross-Compilation Checks

When modifying target_os = "linux" crates (bencher_init, bencher_noise, bencher_rootfs, bencher_runner, bencher_runner_cli), also run the cross-compilation checks locally:

./scripts/clippy.sh            # Runs clippy natively + cross-compiles to x86_64-unknown-linux-gnu
./scripts/test.sh --linux-only # Cross-compiles tests for the Linux-only crates

These scripts require a cross-compiler (zig, x86_64-linux-gnu-gcc, or x86_64-unknown-linux-gnu-gcc) and the x86_64-unknown-linux-gnu Rust target. The clippy script will install the target automatically and warn if no cross-compiler is found.

Code Quality

Core Axiom: Less is more. Err on the side of simplicity. KISS.

• Always run cargo fmt and cargo clippy when testing or before committing
• Run cargo fmt one final time after all changes are complete (including any generated code or lint fixes), since clippy fixes and other automated changes can introduce formatting drift
• Use #[expect(...)] instead of #[allow(...)] for lint suppression
• Do NOT suppress a lint outside of a test module without explicit approval
• All dependency versions go in the workspace Cargo.toml
• When reviewing code, also check:
  • cargo check --no-default-features
  • cargo gen-types (if the API changed at all)
  • cargo deny check (if dependencies were added or updated)
• Use idiomatic, strong types instead of String and serde_json::Value where possible
• Database model fields should use strong validated types (e.g., ProjectId, ProjectUuid, ProjectName, DateTime, VersionNumber) with Diesel ToSql/FromSql impls rather than raw primitives (i32, i64, String). All conversion happens inside the Diesel impls, not in the model layer.
• Avoid select! macros - use futures_concurrency::stream::Merge::merge
• Prefer stream combinators (try_fold, try_for_each, try_collect, etc.) over manual while let Some(chunk) = stream.next().await loops when processing streams
• Prefer structured concurrency: every spawned task has an owner that holds its JoinHandle and awaits it at shutdown. Where cancel-on-drop semantics make sense, use a JoinSet, which aborts its tasks when the owner drops. Do NOT detach a task by dropping its handle without explicit justification. A detached task is unsupervised, its panics are silent, and shutdown cannot order itself against it
• All time-based tests should be deterministic and use time manipulation not real wall-clock time
• Use bencher_json::Clock::Custom (behind the test-clock feature) to inject a fake clock in tests instead of calling DateTime::now() directly. Clock is available on ApiContext.
• With tokio::time::pause(), call tokio::task::yield_now().await before tokio::time::advance() when the code under test spawns a task that sleeps: a task is not polled until the test yields, so an advance issued first moves the clock before the sleep is registered, and the timer then fires that much later in virtual time, after the test has already asserted
• For unit tests without access to ApiContext/Clock, use bencher_json::DateTime::TEST (a fixed deterministic const). Enable test-clock in bencher_json dev-dependencies to access it.
• No tautological tests. Do not write tests for anything the type system already guarantees.
• Most wire type definitions are in the bencher_valid or bencher_json crate
• Always pass strong types (MyTypeId, MyTypeUuid, etc) into a function instead of its stringly typed equivalent, even in tests
• Do NOT use shared, global mutable state
• Always use thiserror for error types in libraries and production binaries (services/). Do not use anyhow in those crates. anyhow is acceptable in tasks/ crates (build tasks, test harnesses) where convenience outweighs structured errors.
• Error enum variants must wrap the original error type, not String. Use #[error("context: {0}")] Variant(OriginalError), not Variant(String) with .to_string() at the call site.
• Do not use Box<dyn Error> (or Box<dyn std::error::Error + Send + Sync>) as a return type. Use HttpError for API endpoint errors or define specific thiserror error enums. The only acceptable uses of Box<dyn Error> are when wrapping third-party APIs that return boxed errors (e.g., diesel migrations, dropshot server creation).
• Track adverse events (auth failures, write errors, rate limit hits) with OpenTelemetry metrics via bencher_otel::ApiMeter::increment. These events may not warrant a Sentry error but give SRE/security visibility. Use dimensional attributes (reason, resource type) for drill-down.
• Do NOT use dyn std::any::Any without explicit justification and approval
• When adding workspace dependencies without extra options (no optional, no features), use the shorthand dep.workspace = true form instead of dep = { workspace = true }
• Use camino (Utf8Path/Utf8PathBuf) for file paths whenever practical instead of std::path::Path/PathBuf. Exception: tempfile::tempdir() in tests may use std::path since it returns TempDir with &Path; convert via Utf8Path::from_path() at the boundary when needed.
• Use clap for CLI argument parsing
  • The clap struct definitions should live in a separate parser module
  • The subcommand handler logic should live in a separate module named after the binary for production code (ie bencher) or a module named task for tasks/* crates
  • Do NOT use num_args on flags in bencher run — it uses trailing_var_arg = true to match docker run semantics, and num_args conflicts with trailing vararg parsing. Validate collection sizes at the type/deserialization layer instead (e.g., TryFrom impls in bencher_json).
• Within a module, order definitions top-down by scope: primary struct first, then its error type, then impl blocks, then supporting/child types (enums, helpers) used by the primary type. Readers should encounter the "what" before the "how".
• Prefer destructuring a struct (let Self { field1, field2, .. } = self; or let Foo { .. } = foo;) over individual field access (.field1, .field2) when consuming or converting all fields. This ensures the compiler flags a build error when a field is added, preventing silent omissions.
• Use macros for database connection access. All of these macros have a single-use and expanded closure-like form for use multiple times in the same scope.
  • public_conn!() - For read-only public access
    • This optionally takes in a PublicUser
  • auth_conn!() - For read-only authenticated access
  • write_conn!() - For single writer access
• Use diesel::QueryResult<T> instead of Result<T, diesel::result::Error> — it is a type alias and more idiomatic
• Database write methods that may be called both standalone and from within an outer transaction should NOT wrap in conn.transaction() internally. Instead, the standalone callers should wrap the call in a transaction. This avoids unnecessary SQLite savepoints when the method is called from batch operations.
• Use the write_transaction!() macro (or conn.immediate_transaction(...) on an existing writer &mut DbConnection) for any transaction that writes — never conn.transaction(...).

Scripts Policy

Shell scripts are used very sparingly. Prefer creating tasks in tasks/ (invoked via cargo aliases). Administrative-only tasks go in xtask/. Shell scripts are only acceptable as ultra-lightweight wrappers around commands like git or docker.

Cargo Aliases

Defined in .cargo/config.toml:

• cargo xtask - Administrative tasks
• cargo gen-types / cargo gen-spec / cargo gen-ts - Type generation
• cargo test-api - API testing and DB seeding
  • cargo test-api seed needs the API server already running (cargo run in services/api) with a fresh database (services/api/data holds only a tracked .gitignore; delete services/api/data/bencher.db, not the whole directory)
  • cargo test-api seed also needs the bencher CLI binary already built (cargo build --bin bencher); it shells out via assert_cmd, which panics with `CARGO_BIN_EXE_bencher` is unset if the binary is missing
  • Pass --no-git when running the seed test in this repo: there is no colocated .git, so bencher run cannot derive a git context and the on-the-fly project naming assertions (bencher vs Project) will fail without it
• cargo test-runner - Runner integration tests (requires Linux + KVM + root)
  • cargo test-runner scenarios always fails unelevated: the sandbox is built by dropping privilege, so the scenarios refuse to start without it. Build unprivileged, then run elevated, which also keeps cargo from leaving root-owned artifacts in target:
    • cargo test-runner scenarios --build-only
    • sudo BENCHER_RUNNER_BIN=./target/debug/runner ./target/debug/test_runner scenarios

Git Flow

• PRs are opened against the devel branch
• Deploy to Bencher Cloud: once devel CI has uploaded the commit's builds, reset cloud to devel and push
• After successful deploy: CI resets main to cloud
• Release tags (e.g., v0.5.10) are created off devel
• Do NOT use auto-closing keywords (Closes #N, Fixes #N) in PR descriptions if there have been changes to the CLI

Type Sharing Flow

Rust is the single source of truth for types:

1. Rust structs annotated with #[typeshare] in bencher_json and other crates
2. cargo gen-types generates OpenAPI spec (services/api/openapi.json) and TypeScript types (services/console/src/types/bencher.ts)
3. bencher_valid is compiled to WASM for browser-side validation
4. bencher_client is auto-generated from the OpenAPI spec via progenitor

If cargo gen-types fails with Failed to read input: .ts and No such file or directory, look for a dangling symlink in the repo (for example lib/bencher_client/codegen.rs, a gitignored convenience symlink into target/ that dangles after the build dir is cleaned). Typeshare walks the whole repo and errors on dangling symlinks; delete the symlink.

Docker

When adding a new crate, update all four Dockerfiles:

• services/api/Dockerfile copies the whole workspace (COPY . .), so it usually needs no change
• services/cli/Dockerfile, services/console/Dockerfile, and docker/bench.Dockerfile stub out unused workspace members with cargo init --lib, so each needs a stub line for the new crate

CI Path Filters

When adding a new crate or changing crate dependencies, update the path filters in .github/workflows/ci.yml (changes job) for any affected filter (rust, cli, runner, console, action, docker, nix).