# Contributing to synckit

What you need to know before changing code here: how the two crates divide the
work, which feature flags exist and why, how to run each test configuration, and
the conventions a new test is expected to follow.

Design and strategy are not in this file. `synckit-client/docs/architecture.md`
covers the `SyncStore` engine and the wire format,
`synckit-client/docs/integration_patterns.md` covers consuming the SDK from an
app, and the maintainer wiki holds the rest (`synckit-overview`).

## Two crates, no workspace

Each crate builds on its own. There is no root `Cargo.toml`, matching the
ecosystem convention, so `cargo` commands run from inside a crate directory or
with an explicit `--manifest-path`.

| Crate | Role | Depends on |
|-------|------|------------|
| `synckit-client` | The SDK: transport, crypto, the `SyncStore` engine | `synckit-config` (optional, behind `store`) |
| `synckit-config` | Local key/value settings with per-key sync postures | Nothing internal |

The dependency runs one way and must stay that way. `synckit-config` is usable
with no network and no SDK, which is the point: a TUI that only wants to
remember a theme links it alone. The adapter that turns a `ConfigSpec` into a
`SyncTable` lives in `synckit-client/src/store/config.rs`, on the client side of
the boundary.

Inside `synckit-client`, `src/client/` is the HTTP surface split by domain
(auth, sync, blob, groups, rotation, subscription), `src/store/` is the local
SQLite engine, and `crypto.rs`, `conflict.rs`, `identity.rs` are the pure layers
underneath both. Pure logic belongs in the pure layer, where it can be tested
without a mock server.

## Feature flags

Three, and the interactions matter more than the flags do.

- **`keychain`** (default on) stores the master key in the OS secret service.
  With it off, `keystore::store_key` is a no-op stub. That is what makes
  rotation testable on a headless host, so the flag is a real test axis rather
  than a build convenience.
- **`store`** (default on) brings in the `SyncStore` engine and its bundled
  rusqlite. A consumer that only wants transport and crypto sets
  `default-features = false` and skips compiling SQLite; `mnw-cli` does exactly
  that, so it is a supported configuration and not a hypothetical one.
- **`testing`** exposes constructors that bypass key derivation
  (`set_master_key_raw`, `with_http_client`). Never in `default`: a consumer
  build must have no chosen-key injection point.

`testing` reaches this crate's own tests through a self dev-dependency:

```toml
synckit-client = { path = ".", default-features = false, features = ["testing"] }
```

`default-features = false` there is load-bearing. It adds `testing` to whatever
the run already selected instead of forcing the defaults back on, which is what
lets `--no-default-features --features store,testing` genuinely turn `keychain`
off.

The Apple targets are checked, not built, by `scripts/check-mobile-targets.sh`.
Run it after touching `keystore.rs`, the `keychain` feature, or any
keyring-family dependency. A target can be broken there while every host we
build on compiles cleanly.

## Running the suites

From `synckit-client/`:

```
cargo test                                                  # defaults: keychain + store
cargo test --no-default-features --features store,testing   # keychain off
```

Both configurations have to pass. They are not the same set of tests: the
rotation orchestration module is gated `#[cfg(not(feature = "keychain"))]`
because `rotate_key` finishes by caching the new key through the OS secret
service, which a headless host does not have.

Integration tests live in one binary, `tests/integration/main.rs`, with one
module per domain and shared fixtures in `tests/integration/common.rs`. The
`[[test]]` block in `Cargo.toml` names that path explicitly; without it,
`mod common;` would resolve to `tests/common.rs` and miss the directory.

`synckit-config` is a plain `cargo test`.

Before committing, run `cargo fmt` and
`cargo clippy --all-targets` (both feature configurations). Clippy is
`pedantic`-with-an-allow-list, and the allow-list block is kept identical across
repos, so extend it in all of them or in none.

Activate the hooks once per clone:

```
git config core.hooksPath scripts/githooks
```

That gates commits on gitleaks and rustfmt.

## Test conventions

The full standard is `_private/docs/meta/test_style.md` and it applies here
unchanged. The parts this repo leans on hardest:

- **Names are prose, no `test_` prefix.** `push_retries_on_503`, not
  `test_push_retry`. Name the outcome, not the function.
- **Unit tests go at the bottom of the file they test**, in one
  `#[cfg(test)] mod tests`. A unit test that reaches for a database or a mock
  server is an integration test in the wrong file.
- **No file over ~800 lines.** Split a suite that outgrows it into a directory
  of per-domain modules.
- **A fixture earns a place in `common.rs` when a second module wants it.** One
  caller, one module: leave it where it is.
- **Every test owns its world.** A fresh `MockServer` per test, a fresh
  in-memory database. Tests must pass in any order under any `--test-threads`,
  so anything process-global (the rustls provider) installs idempotently behind
  a `Once`, which is what `ensure_crypto_provider` is for.
- **Match the error variant**, not `is_err()`. `assert!(matches!(err,
  SyncKitError::TokenExpired))` fails for the right reason.
- **No network.** `wiremock` stands in for the server. Fixtures are validated
  against the server's OpenAPI schemas in `tests/openapi.json`, so a wire shape
  that has drifted from the server fails in `contract.rs` rather than in
  production.

Two kinds of test carry more weight here than a plain example does, because the
contracts they cover are universally quantified:

- **Properties** (`proptest`). An order is an order for every pair; a round-trip
  round-trips for every input. `proptest-regressions/` is committed, so a shrunk
  counterexample becomes a permanent case.
- **Metamorphic relations.** Relate two runs instead of judging one, which needs
  no table of expected values. Pagination must not change what a pull returns; a
  rotation must not change the plaintext a pull yields. Both live in the
  integration suite. Assert on content, not on request counts.

## Writing

House rules apply to comments, docs and commit messages: no emoji, no em
dashes, no AI tells. `_private/docs/meta/brand.md` is the reference.

A test comment explains why the assertion is what it is. It never narrates what
the next line does.