diff --git a/AGENTS.md b/AGENTS.md index a0cb30ab4..47c54a595 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -142,17 +142,16 @@ Internal APIs are borrow-centric: lookups and get-or-create accessors return references, clone keys only on insertion, and build error context lazily. - **Never** rewrite a site tagged `POLONIUS(...)` into a double lookup - (`contains_key` + `get_mut`), an `entry(key.clone())` call, or an - id/index round-trip. Those forms are the NLL-era workarounds this - codebase deliberately retired; the direct form is compiler-verified. + (`contains_key` + `get_mut`), an `entry(key.clone())` call, or an id/index + round-trip. Those forms are the NLL-era workarounds this codebase + deliberately retired; the direct form is compiler-verified. - **Never** silence a borrow error by adding a defensive `.clone()`, an eager error-context computation, or a snapshot `collect()` without first - compiling the natural borrow-returning form under the project toolchain — - it usually compiles. + compiling the natural borrow-returning form under the project toolchain — it + usually compiles. - Respect `POLONIUS-REFUSED(...)` tags: the named constraint (persistent - identity, lock boundaries, aliasing, suspension points, thread - boundaries) is permanent. Do not convert those sites to - reference-returning forms. + identity, lock boundaries, aliasing, suspension points, thread boundaries) is + permanent. Do not convert those sites to reference-returning forms. - When adding a new borrow-centric API, verify it with and without `-Zpolonius=next` and record the classification in `docs/polonius.md`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 34642f62f..1aa810e85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,17 +6,15 @@ - Route graph-view node registration through a borrow-returning `NodePathRegistry` accessor that looks paths up once on hits and clones a - path only on insertion - ([#465](https://github.com/leynos/netsuke/issues/465)) + path only on insertion ([#465](https://github.com/leynos/netsuke/issues/465)) - Build with the Polonius borrow checker (`-Zpolonius=next`) on the pinned - `nightly-2026-06-25` toolchain; checkout builds pick this up - automatically via `rustup`, while registry installs must pass the - toolchain and flag explicitly + `nightly-2026-06-25` toolchain; checkout builds pick this up automatically via + `rustup`, while registry installs must pass the toolchain and flag explicitly (`RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke`) ([#465](https://github.com/leynos/netsuke/issues/465)) - Remove the `rust-version = "1.89.0"` minimum-supported-Rust-version - declaration from `Cargo.toml`; `rust-toolchain.toml` is now the single - source of truth for the compiler contract + declaration from `Cargo.toml`; `rust-toolchain.toml` is now the single source + of truth for the compiler contract ([#465](https://github.com/leynos/netsuke/issues/465)) ## [0.1.0] - 2026-07-28 diff --git a/README.md b/README.md index 18350b36e..0f93ce656 100644 --- a/README.md +++ b/README.md @@ -35,16 +35,16 @@ Netsuke currently requires: - [Ninja](https://ninja-build.org/) on `PATH`; - when installing from source, the dated Rust nightly toolchain pinned in [`rust-toolchain.toml`](rust-toolchain.toml) (`rustup` installs it - automatically in a checkout). Netsuke builds with the Polonius borrow - checker (`-Zpolonius=next`), which is nightly-only until it stabilizes; see + automatically in a checkout). Netsuke builds with the Polonius borrow checker + (`-Zpolonius=next`), which is nightly-only until it stabilizes; see [ADR-006](docs/adr-006-adopt-polonius-nightly-toolchain.md). ### Installation Netsuke v0.1.0 is available from crates.io. Where -[`cargo binstall`](https://github.com/cargo-bins/cargo-binstall) is -available, prefer it: it fetches a prebuilt release binary and avoids the -toolchain requirement below. +[`cargo binstall`](https://github.com/cargo-bins/cargo-binstall) is available, +prefer it: it fetches a prebuilt release binary and avoids the toolchain +requirement below. @@ -53,8 +53,8 @@ cargo binstall netsuke ``` Building from the registry instead runs outside a repository checkout, so -neither the pinned toolchain nor the Polonius flag is picked up -automatically; supply both explicitly: +neither the pinned toolchain nor the Polonius flag is picked up automatically; +supply both explicitly: diff --git a/docs/adr-006-adopt-polonius-nightly-toolchain.md b/docs/adr-006-adopt-polonius-nightly-toolchain.md index 1618dd215..b8040c8a5 100644 --- a/docs/adr-006-adopt-polonius-nightly-toolchain.md +++ b/docs/adr-006-adopt-polonius-nightly-toolchain.md @@ -19,8 +19,8 @@ several of these shapes, so the natural borrow-returning form of an accessor can compile where NLL rejected it. Adopting those borrow-centric designs binds the source tree to a -Polonius-enabled compiler, which is nightly-only until the analysis -stabilizes. That conflicts with three standing policies: +Polonius-enabled compiler, which is nightly-only until the analysis stabilizes. +That conflicts with three standing policies: - `rust-toolchain.toml` pinned stable `1.89.0`; - `Cargo.toml` declared `rust-version = "1.89.0"` as a minimum supported Rust @@ -52,20 +52,20 @@ Adopt Polonius now, as a nightly-only source tree: nightly requirement there, and advertising `1.89.0` would misstate the contract; `rust-toolchain.toml` is now the single source of truth. -Every borrow-centric rewrite that depends on the flag is verified both with -and without `-Zpolonius=next` and recorded in +Every borrow-centric rewrite that depends on the flag is verified both with and +without `-Zpolonius=next` and recorded in [polonius migration notes](polonius.md), including refusals where owned style remains correct. ## Rationale - **Design over deployment breadth.** Netsuke ships binaries, not a library - API. Consumers install packaged artefacts or build from source; the - toolchain pin costs contributors one `rustup` fetch, whereas NLL-era - double lookups and key clones cost every call site, forever. + API. Consumers install packaged artefacts or build from source; the toolchain + pin costs contributors one `rustup` fetch, whereas NLL-era double lookups and + key clones cost every call site, forever. - **Reproducibility.** A dated nightly behaves like a release: the same - compiler bits build the tree everywhere. `rustup` provisions it - automatically from `rust-toolchain.toml`. + compiler bits build the tree everywhere. `rustup` provisions it automatically + from `rust-toolchain.toml`. - **Coherent tooling.** Putting the flag in `.cargo/config.toml` keeps rust-analyzer, Clippy, Whitaker (whose Dylint driver is nightly-based), and Kani borrow-checking the same dialect, avoiding phantom editor errors on @@ -81,19 +81,18 @@ remains correct. `rust-toolchain.toml` and `.cargo/config.toml` (and Cargo would not apply them to a registry build anyway), so a bare `cargo install netsuke` of a Polonius-dependent release fails borrow checking on the user's default - toolchain. Registry installs must select the pinned nightly and pass the - flag explicitly - (`RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke`); - the README and users' guide document this command and a contract test pins - it. Source installs from a checkout are unaffected because the pinned - toolchain and workspace configuration apply there. + toolchain. Registry installs must select the pinned nightly and pass the flag + explicitly + (`RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke`); the + README and users' guide document this command and a contract test pins it. + Source installs from a checkout are unaffected because the pinned toolchain + and workspace configuration apply there. - Release packaging builds from the pinned nightly. Binary artefacts are - unaffected: the borrow checker changes what compiles, not what is - generated. + unaffected: the borrow checker changes what compiles, not what is generated. - Dependabot-style toolchain drift is impossible; moving the pin is a deliberate act. Move it forward periodically (and especially once Polonius - stabilizes), re-running the full gate suite, and update this ADR's - references when doing so. + stabilizes), re-running the full gate suite, and update this ADR's references + when doing so. - Sites that genuinely require Polonius are tagged `POLONIUS(...)` in source and must not be rewritten into NLL-era defensive forms; `AGENTS.md` and [polonius migration notes](polonius.md) carry the anti-regression guidance. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index f033aff83..3bcf7b435 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -109,8 +109,8 @@ Run these commands before finalizing any change: - `make lint` - `make test` -When the change touches any Markdown file — documentation, ADRs, execplans, -or the README — also run: +When the change touches any Markdown file — documentation, ADRs, execplans, or +the README — also run: - `make fmt` - `make markdownlint` @@ -151,12 +151,11 @@ cross-directory symlink canonicalization, which `cap_std` cannot express) and `netsuke::runner::process::file_io` (temporary-file synchronization), and `netsuke::cli::discovery::paths` (canonicalizing an ambient `--directory` to match OrthoConfig's layer paths) are exempt; the rest of `netsuke` stays under -the capability policy. The -behavioural step definitions, CLI integration tests, and shared -workflow-reading helper that stage fixtures ambiently are scoped the same way. -A crate-level entry is justified only when the ambient access lives in the -crate root itself, where a path entry would be no narrower — that covers the -Cargo build script, the `test_support` fixture crate, and the enumerated +the capability policy. The behavioural step definitions, CLI integration tests, +and shared workflow-reading helper that stage fixtures ambiently are scoped the +same way. A crate-level entry is justified only when the ambient access lives +in the crate root itself, where a path entry would be no narrower — that covers +the Cargo build script, the `test_support` fixture crate, and the enumerated integration-test crates. Permanent exceptions belong in `dylint.toml`, scoped as narrowly as the lint @@ -434,10 +433,9 @@ than through `make` means arranging that `PATH` order yourself. `make dev-fast-check` prints the resolved `mold` path alongside its version, so an unexpected pick is visible. A version that differs from the pin fails the check, as does a missing `mold` or one that cannot report its version; run -`make install-dev-fast` to install the pinned release ahead of any -distribution `mold` on `PATH`. An advisory pin is not a pin: tolerating drift -would let the linker actually in use stop matching what the repository -claims. +`make install-dev-fast` to install the pinned release ahead of any distribution +`mold` on `PATH`. An advisory pin is not a pin: tolerating drift would let the +linker actually in use stop matching what the repository claims. For screen readers: the following flowchart traces `make install-dev-fast` from start to exit. It reads the pinned linker version, then branches on the host @@ -485,23 +483,22 @@ themselves. The accelerated configuration lives in `tools/dev-fast/config.toml`, which is deliberately *not* `.cargo/config.toml`. Cargo auto-discovers the latter, so placing Cranelift and the Linux-only `mold` linker there would silently apply -them to every build in the repository, including release, packaging, -coverage, and formal-verification builds. The fragment is instead passed -explicitly with `cargo --config tools/dev-fast/config.toml` from the -`make dev-*` targets, and must not be sourced from any target that CI -invokes. +them to every build in the repository, including release, packaging, coverage, +and formal-verification builds. The fragment is instead passed explicitly with +`cargo --config tools/dev-fast/config.toml` from the `make dev-*` targets, and +must not be sourced from any target that CI invokes. A repository-root `.cargo/config.toml` does exist, and legitimately so: it carries the Polonius flag (`[build] rustflags = ["-Zpolonius=next"]`, see -Polonius under Composition rules) needed by every build in the repository. -The rule is about what belongs in that file, not about whether it may exist: +Polonius under Composition rules) needed by every build in the repository. The +rule is about what belongs in that file, not about whether it may exist: settings needed everywhere may go there; settings that are only safe for the accelerated dev loop must not. -The fragment sets the `codegen-backend` unstable flag, `codegen-backend = -"cranelift"` on the `dev` profile, and a `cfg(target_os = "linux")`-gated -rustflags list carrying both `-Zpolonius=next` and -`-Clink-arg=-fuse-ld=mold`. +The fragment sets the `codegen-backend` unstable flag, +`codegen-backend = "cranelift"` on the `dev` profile, and a +`cfg(target_os = "linux")`-gated rustflags list carrying both `-Zpolonius=next` +and `-Clink-arg=-fuse-ld=mold`. ### Composition rules @@ -577,16 +574,16 @@ network, and no real `mold`, `rustup`, or Cargo — so they run as part of `make test` on any Linux host. - `tests/dev_fast_check_tests.rs`: the capability gate. Which diagnostic each - failure mode emits, exit status, pin resolution, and refusal of a - malformed pin. + failure mode emits, exit status, pin resolution, and refusal of a malformed + pin. - `tests/dev_fast_install_tests.rs`: the installer's happy path and its refusals, plus the benchmark script's Markdown output. - `tests/dev_fast_checksum_tests.rs`: property coverage for checksum verification against a model. - `tests/dev_fast_make_target_tests.rs`: the Make recipes. Toolchain and fragment selection; that a failed gate reaches zero Cargo invocations - (`dev-build` and `dev-test` stop before Cargo runs); the fragment's - contents; and `install-dev-fast` forwarding. + (`dev-build` and `dev-test` stop before Cargo runs); the fragment's contents; + and `install-dev-fast` forwarding. - `tests/dev_fast_bench_tests.rs`: `make bench-build`. Per-variant target directories, the clean/incremental cycle, and both variant rows. - `tests/dev_fast_bench_lock_tests.rs`: the benchmark's exclusion lock. That a @@ -634,10 +631,10 @@ The fixtures live in `test_support::dev_fast`: Cranelift component, and a `RecordingCargo` installed — and is shared by the Make-target and benchmark suites. `InstallerScenario` is a sandbox with a published `FakeRelease` and a usable `rustup`, letting a test concentrate on - the linker half of the installer; the installer and checksum suites share - it. The module also exports `TEST_MOLD_VERSION`, deliberately not a real - `mold` version so a test that accidentally reaches the network fails rather - than silently succeeding against an upstream artefact, and `WRONG_SHA256`. + the linker half of the installer; the installer and checksum suites share it. + The module also exports `TEST_MOLD_VERSION`, deliberately not a real `mold` + version so a test that accidentally reaches the network fails rather than + silently succeeding against an upstream artefact, and `WRONG_SHA256`. `InstallerFixture` groups the installer's pin path, checksum path, and release URL, and renders them via `script_env()`. @@ -645,8 +642,8 @@ A scenario earns its place here once a second suite needs it, and not before; suite-specific conveniences stay with their suite — the installer tests keep their own `ChecksumFailure` enum and `with_failure` helper, because a fixture encoding one suite's failure taxonomy is not shared ground. Scenario -constructors stay free of assertions, so a scenario cannot decide on a -caller's behalf what counts as correct. +constructors stay free of assertions, so a scenario cannot decide on a caller's +behalf what counts as correct. Assert on the shape of a timing cell, never on a duration. Reuse the sandbox for any future target with the same shape, and do not reach for `PathGuard`: @@ -675,11 +672,10 @@ Prefer a model that predicts an outcome over a table that restates one. Where an invariant lives in a shell script, the cost is a process per case, so keep the corpus small and the strategy structural. -A `#[cfg(test)]` unit test added inside `test_support` will not run as part -of `make test`, because `Cargo.toml` excludes `test_support` from the -workspace. Put assertions about the fixtures themselves in the -`tests/dev_fast_*.rs` integration crates instead, where the gate will -actually exercise them. +A `#[cfg(test)]` unit test added inside `test_support` will not run as part of +`make test`, because `Cargo.toml` excludes `test_support` from the workspace. +Put assertions about the fixtures themselves in the `tests/dev_fast_*.rs` +integration crates instead, where the gate will actually exercise them. ### Benchmark evidence @@ -698,9 +694,9 @@ permanently newer. Rather than leave that to convention, the benchmark takes immediately, naming the lock and the remedy, and does so before touching anything, so the holder's state is unaffected. The lock is released however the run ends, including on interrupt. To benchmark two things at once, override -`BENCH_ROOT` and `BENCH_TOUCH_FILE` per run; the lock path follows `BENCH_ROOT`, -so distinct roots do not contend. If a killed run ever leaves the directory -behind, remove it. +`BENCH_ROOT` and `BENCH_TOUCH_FILE` per run; the lock path follows +`BENCH_ROOT`, so distinct roots do not contend. If a killed run ever leaves the +directory behind, remove it. Results below were recorded on a 24-core x86_64 Linux host, with both variants on the repository's own `nightly-2026-06-25` supplying Cranelift 0.132.0, and @@ -856,9 +852,8 @@ and then runs the bounded harness suite through `make kani-ir` under a 20-minute job timeout; it does not run `make verus`, coverage, CodeScene upload, or the normal build matrix. Its cache is intentionally separate from ordinary Cargo build artefacts: the job uses a Kani-specific cache key derived -from -`tools/kani/VERSION` and the Makefile, then caches the job-local Kani Cargo -home plus Kani support-file home. +from `tools/kani/VERSION` and the Makefile, then caches the job-local Kani +Cargo home plus Kani support-file home. ## Test execution @@ -869,8 +864,8 @@ home plus Kani support-file home. `RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)"` (the Makefile re-states the Polonius flag because a set `RUSTFLAGS` overrides `.cargo/config.toml`, and the `$${RUSTFLAGS:+$$RUSTFLAGS }` prefix preserves - any `RUSTFLAGS` inherited from the caller). This runs every unit, - integration, `rstest`, and `rstest-bdd` test. + any `RUSTFLAGS` inherited from the caller). This runs every unit, integration, + `rstest`, and `rstest-bdd` test. - `make doctest` — `cargo test --doc --all-features`, with the same `RUSTFLAGS`. nextest cannot execute doctests, so they need their own pass. Note that the previous `cargo test --all-targets` invocation never ran @@ -1320,27 +1315,26 @@ Do **not** call `std::env::set_var` directly in BDD steps — use ### `tracing_capture` -Production tracing has one process-wide subscriber, installed by -`init_tracing` in `src/main.rs` with a reloadable filter initially set to -`OFF`. Early configuration resolution therefore cannot write selector events -before the effective JSON mode is known. On success, -`resolve_json_mode_or_exit` calls `set_tracing_filter` with the resolved mode: -JSON stays `OFF`, while human mode enables `TRACE` for `--verbose` or `ERROR` -otherwise. Full human-mode merging repeats discovery after the filter is -enabled, so its selector events remain available. If early resolution fails, -human mode enables its fallback filter and replays resolution to retain bounded -failure diagnostics; JSON mode leaves the filter off and discards them. No -library module installs a global subscriber. +Production tracing has one process-wide subscriber, installed by `init_tracing` +in `src/main.rs` with a reloadable filter initially set to `OFF`. Early +configuration resolution therefore cannot write selector events before the +effective JSON mode is known. On success, `resolve_json_mode_or_exit` calls +`set_tracing_filter` with the resolved mode: JSON stays `OFF`, while human mode +enables `TRACE` for `--verbose` or `ERROR` otherwise. Full human-mode merging +repeats discovery after the filter is enabled, so its selector events remain +available. If early resolution fails, human mode enables its fallback filter +and replays resolution to retain bounded failure diagnostics; JSON mode leaves +the filter off and discards them. No library module installs a global +subscriber. Tests use a separate capture boundary: `src/test_tracing_capture.rs` (`crate::test_tracing_capture`) is the -workspace's single implementation for capturing structured tracing events -in tests. `with_test_subscriber` installs a capturing `Layer` as the -default subscriber for the duration of a closure, then returns the -closure's result. Each event's fields are rendered as a space-separated -list of `name=value` pairs — strings and `Debug` values are quoted — and -appended to a shared buffer: +workspace's single implementation for capturing structured tracing events in +tests. `with_test_subscriber` installs a capturing `Layer` as the default +subscriber for the duration of a closure, then returns the closure's result. +Each event's fields are rendered as a space-separated list of `name=value` +pairs — strings and `Debug` values are quoted — and appended to a shared buffer: ```rust use crate::test_tracing_capture::with_test_subscriber; @@ -1356,24 +1350,23 @@ with_test_subscriber(LevelFilter::TRACE, |captured| { `with_test_subscriber` installs the subscriber through [`tracing::subscriber::with_default`], which registers a *thread-local* -default. Only events emitted on the calling thread are captured; events -emitted from threads spawned inside the closure are silently dropped. +default. Only events emitted on the calling thread are captured; events emitted +from threads spawned inside the closure are silently dropped. -The module is `#[cfg(test)]` in the root crate, so it is available to -unit tests only; integration tests under `tests/` compile as separate -crates and cannot reach it. Coverage that needs the real binary's tracing -output instead asserts on the process's stderr — see -`tests/logging_stderr/config_tracing.rs`. +The module is `#[cfg(test)]` in the root crate, so it is available to unit +tests only; integration tests under `tests/` compile as separate crates and +cannot reach it. Coverage that needs the real binary's tracing output instead +asserts on the process's stderr — see `tests/logging_stderr/config_tracing.rs`. `CapturedEvents` has no `Default` implementation — obtain it only from the -handle passed into the `with_test_subscriber` closure. `snapshot()` -recovers a poisoned lock rather than panicking, so a panic on another test -thread cannot cascade into a snapshot assertion. +handle passed into the `with_test_subscriber` closure. `snapshot()` recovers a +poisoned lock rather than panicking, so a panic on another test thread cannot +cascade into a snapshot assertion. Tests that snapshot tracing output with `insta` should normalize runtime-dependent fields, such as the bounded `path_hash` correlation -identifier, to a stable placeholder before asserting the snapshot, and -assert the real value separately with its own check. See +identifier, to a stable placeholder before asserting the snapshot, and assert +the real value separately with its own check. See `src/cli/discovery_tracing_tests.rs` for this pattern. ## `TestWorld` field groups @@ -1544,17 +1537,16 @@ Unit tests that only need to verify explicit config path precedence should test the process environment. Config selector resolution remains a pure query: `resolve_config_selector` -records the winning selector, its optional path, and every environment -lookup evaluated, and emits no tracing itself. Structured diagnostics are -emitted only at the file-layer boundary, where -`collect_file_layers_with_env` calls `trace_config_path_resolution` after -resolution completes. +records the winning selector, its optional path, and every environment lookup +evaluated, and emits no tracing itself. Structured diagnostics are emitted only +at the file-layer boundary, where `collect_file_layers_with_env` calls +`trace_config_path_resolution` after resolution completes. Tracing never logs full paths or formatted parser errors. Path values are -bounded to a `path_hash` correlation identifier plus `path_file_name`, and -load failures are classified with the `ConfigLoadFailureKind` enum instead -of the formatted error text. `path_hash` is a bounded identifier for -correlating events, not a cryptographic guarantee. +bounded to a `path_hash` correlation identifier plus `path_file_name`, and load +failures are classified with the `ConfigLoadFailureKind` enum instead of the +formatted error text. `path_hash` is a bounded identifier for correlating +events, not a cryptographic guarantee. #### `json` contract @@ -1572,8 +1564,8 @@ split diagnostics, path comparison, and tests out of the main discovery flow: - `discovery_diagnostics.rs` — bounded tracing helpers (`path_hash`, `short_hash`, `debug_config_path`, `debug_optional_config_path`, - `warn_explicit_config_load_failed`) and the `ConfigLoadFailureKind` enum - used to classify a load failure without retaining error text. + `warn_explicit_config_load_failed`) and the `ConfigLoadFailureKind` enum used + to classify a load failure without retaining error text. - `discovery_paths.rs` — `normalized_path_key` resolves a path to a comparable, canonicalized form and returns canonicalization errors to its caller. The discovery-side `comparison_key` fallback uses the original path @@ -1585,24 +1577,22 @@ split diagnostics, path comparison, and tests out of the main discovery flow: path, and tracing remains at the orchestration boundary. - `discovery_event_assertions.rs` — shared test-only helpers: `capture_events` runs a closure under a TRACE capturing subscriber, - `find_event` locates one emitted event by substring, and - `EventAssertion` bundles an event with its path to assert bounded - `path_hash`/`path_file_name` fields, the absence of the raw path or - formatted error text, and to normalize the hash before an `insta` - snapshot. + `find_event` locates one emitted event by substring, and `EventAssertion` + bundles an event with its path to assert bounded `path_hash`/`path_file_name` + fields, the absence of the raw path or formatted error text, and to normalize + the hash before an `insta` snapshot. - `discovery_tracing_tests.rs` — tests selector precedence (`--config` versus `NETSUKE_CONFIG`), the removed legacy - `NETSUKE_CONFIG_PATH` alias, and event-schema snapshots for both - selection and explicit load failures. + `NETSUKE_CONFIG_PATH` alias, and event-schema snapshots for both selection + and explicit load failures. - `discovery_layer_tests.rs` — tests which branch `collect_diag_file_layers_with_env` takes (explicit path versus automatic discovery) and the project-scope second pass in `collect_file_layers`. -Both test modules import `capture_events`, `find_event`, and -`EventAssertion` from `discovery_event_assertions` rather than duplicating -them. The `insta` snapshot calls themselves stay in the test modules -because snapshot names bind to the test module's path, not to a shared -helper module. +Both test modules import `capture_events`, `find_event`, and `EventAssertion` +from `discovery_event_assertions` rather than duplicating them. The `insta` +snapshot calls themselves stay in the test modules because snapshot names bind +to the test module's path, not to a shared helper module. ## BDD command helpers and environment handling @@ -1800,9 +1790,9 @@ of a trait one, but only when the inherent impl's bound is satisfied, so otherwise. Each assertion is paired with a positive control (`u8: LowerHex`, `Vec: io::Write`) so the probe cannot pass by reporting `false` for everything. Runtime tests confirm the replacements produce correct digests, but -they cannot notice the pre-0.11 patterns becoming available again — for -example if `sha2` were downgraded. A silent downgrade to 0.10 would not fail -the ordinary build, because 0.10's `GenericArray` also derefs to `[u8]`, so +they cannot notice the pre-0.11 patterns becoming available again — for example +if `sha2` were downgraded. A silent downgrade to 0.10 would not fail the +ordinary build, because 0.10's `GenericArray` also derefs to `[u8]`, so `to_lower_hex` and `DigestWriter` keep compiling; the absence of the two impls is what distinguishes the versions, and it is what these guards check. @@ -1812,9 +1802,9 @@ builds the host crate as a fixture dependency while discarding workspace rebuilt `netsuke` without `-Zpolonius=next`; see the "Harness consequences" section of `docs/polonius.md`, which asks that trybuild cases depending on the `netsuke` crate not be reintroduced while the tree is Polonius-only. The -compile-time probe is also strictly better on its own merits: no subprocess, -no scratch project, and no toolchain-sensitive `.stderr` snapshot to re-bless -on every compiler bump. +compile-time probe is also strictly better on its own merits: no subprocess, no +scratch project, and no toolchain-sensitive `.stderr` snapshot to re-bless on +every compiler bump. `stdlib::path::hash_utils` unit-tests the chunked streaming loop against a one-shot digest for inputs that span more than one 8192-byte read, plus a diff --git a/docs/execplans/4-1-2-kani-smoke-ci-job.md b/docs/execplans/4-1-2-kani-smoke-ci-job.md index 62db234c9..9e8db17ea 100644 --- a/docs/execplans/4-1-2-kani-smoke-ci-job.md +++ b/docs/execplans/4-1-2-kani-smoke-ci-job.md @@ -572,16 +572,15 @@ The references section should include: ## Addendum: 2026-08-02 — bounded harness wiring completed -The bounded Kani harness work deferred at the time of this ExecPlan's -execution has since been completed, in follow-up work tracked as +The bounded Kani harness work deferred at the time of this ExecPlan's execution +has since been completed, in follow-up work tracked as [issue #445](https://github.com/leynos/netsuke/issues/445). The pull-request `kani-smoke` job now runs `make kani-check` (the pinned-version drift guard) -and then `make kani-ir` (the bounded harness suite), after -`make install-kani`. The `kani-smoke` job declares `timeout-minutes: 20`. The -bounded suite contains 13 `#[kani::proof]` harnesses: 4 in -`src/ir/from_manifest_verification.rs` and 9 in -`src/ir/cycle_verification.rs`. +and then `make kani-ir` (the bounded harness suite), after `make install-kani`. +The `kani-smoke` job declares `timeout-minutes: 20`. The bounded suite contains +13 `#[kani::proof]` harnesses: 4 in `src/ir/from_manifest_verification.rs` and +9 in `src/ir/cycle_verification.rs`. [Pull request #470](https://github.com/leynos/netsuke/pull/470) carries the -implementation and declares `Closes #445`; the issue itself remains open at -the time of writing and closes on merge. This addendum records later work and -does not amend the plan as executed. +implementation and declares `Closes #445`; the issue itself remains open at the +time of writing and closes on merge. This addendum records later work and does +not amend the plan as executed. diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index bd009c313..d2d80526a 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2584,8 +2584,8 @@ flowchart LR ``` Netsuke configuration discovery is implemented in `src/cli/discovery.rs`. -Explicit file selection is handled by `explicit_config_path_with_env(...)`, which -applies the precedence `--config` > `NETSUKE_CONFIG`. Layer loading and +Explicit file selection is handled by `explicit_config_path_with_env(...)`, +which applies the precedence `--config` > `NETSUKE_CONFIG`. Layer loading and automatic discovery are handled by `push_file_layers(...)`, which also applies the `-C/--directory` flag as the project-discovery root. @@ -2702,11 +2702,10 @@ manual flag repetition. override, relying on OrthoConfig's platform-specific defaults for standard directory resolution. - Netsuke-owned environment reads for explicit config selection and early JSON - resolution go through the `EnvProvider` port in - `src/cli/discovery.rs`. Production code uses `StdEnvProvider`; tests can - inject a map-backed provider instead of mutating the process environment. - OrthoConfig discovery remains an external boundary and may still read - platform environment variables directly. + resolution go through the `EnvProvider` port in `src/cli/discovery.rs`. + Production code uses `StdEnvProvider`; tests can inject a map-backed provider + instead of mutating the process environment. OrthoConfig discovery remains an + external boundary and may still read platform environment variables directly. - Configuration files use TOML format by default. JSON5 (`.json`, `.json5`) and YAML (`.yaml`, `.yml`) formats are supported when the corresponding Cargo features are enabled. @@ -2943,13 +2942,12 @@ selected for this project and the rationale for their inclusion. Netsuke compiles with the Polonius alpha borrow-checking analysis (`-Zpolonius=next`) on the dated nightly toolchain pinned in -`rust-toolchain.toml` -([ADR-006](adr-006-adopt-polonius-nightly-toolchain.md)). Internal APIs -follow a borrow-centric design contract: lookups and registries return -references (`&mut V` accessors with clone-on-miss keys), mutation happens in -place, and error context is built lazily on the failure path. Owned-value -style is reserved for genuine constraints — aliasing, suspension points, -thread and process boundaries, and persistent identity — and each such +`rust-toolchain.toml` ([ADR-006](adr-006-adopt-polonius-nightly-toolchain.md)). +Internal APIs follow a borrow-centric design contract: lookups and registries +return references (`&mut V` accessors with clone-on-miss keys), mutation +happens in place, and error context is built lazily on the failure path. +Owned-value style is reserved for genuine constraints — aliasing, suspension +points, thread and process boundaries, and persistent identity — and each such refusal is recorded in the [polonius migration notes](polonius.md) alongside the sites that depend on the analysis. diff --git a/docs/polonius.md b/docs/polonius.md index f845ff17c..93578d36f 100644 --- a/docs/polonius.md +++ b/docs/polonius.md @@ -3,41 +3,41 @@ Netsuke compiles with the Polonius alpha borrow-checking analysis (`-Zpolonius=next`) on the dated nightly pinned in `rust-toolchain.toml`. [ADR-006](adr-006-adopt-polonius-nightly-toolchain.md) records the toolchain -policy; this document records the audit that motivated it, the API -evolutions it enabled, and the refusals that bound it. Issue +policy; this document records the audit that motivated it, the API evolutions +it enabled, and the refusals that bound it. Issue [#465](https://github.com/leynos/netsuke/issues/465) tracked the migration. ## Method -The migration ran the `nll-to-polonius` two-pass audit with the compiler as -the oracle: +The migration ran the `nll-to-polonius` two-pass audit with the compiler as the +oracle: 1. **Workaround scan** — mechanical sweep for local non-lexical-lifetimes (NLL) workaround shapes: double lookups, `entry()` with unconditionally - cloned keys, re-lookup after insert, index-returning finders, - borrow-killing `drop()` calls, and eager error context. + cloned keys, re-lookup after insert, index-returning finders, borrow-killing + `drop()` calls, and eager error context. 2. **Design-pressure scan** — structural sweep for owned lookup results, id/index indirection, clone-modify-writeback, snapshot-collect loops, and per-module clone hotspots. Every change was compiled twice on `nightly-2026-06-25`: once with -`-Zpolonius=next` (must pass) and once without. The no-flag compile exists -only to classify the individual change: a failure proves the design -genuinely depends on Polonius and the site is tagged `POLONIUS(...)`; -success means the old form was habit rather than necessity and the -improvement carries no toolchain caveat. The complete behavioural test -suite runs under `-Zpolonius=next` — the tree's only supported -configuration — and was required to pass unchanged after every change. +`-Zpolonius=next` (must pass) and once without. The no-flag compile exists only +to classify the individual change: a failure proves the design genuinely +depends on Polonius and the site is tagged `POLONIUS(...)`; success means the +old form was habit rather than necessity and the improvement carries no +toolchain caveat. The complete behavioural test suite runs under +`-Zpolonius=next` — the tree's only supported configuration — and was required +to pass unchanged after every change. ## Polonius-dependent sites -| Site | Tag | Verification | -| --- | --- | --- | +| Site | Tag | Verification | +| ------------------------------------------------------------- | ------------------ | ------------------------------------------------------------------------------- | | `src/graph_view/mod.rs` — `NodePathRegistry::ensure_node_mut` | `POLONIUS(case-3)` | Passes with `-Zpolonius=next`; rejected by NLL with E0499 on nightly-2026-06-25 | `ensure_node_mut` is the get-or-insert accessor behind graph projection: it -returns `&mut NodeKind`, performs a single lookup on the hit path, and -clones the path only on insertion. It replaced three +returns `&mut NodeKind`, performs a single lookup on the hit path, and clones +the path only on insertion. It replaced three `entry(path.clone()).or_insert(NodeKind::Source)` sites that cloned every input, implicit-dependency, and order-only path on every registration. The `get_mut` loan escapes only via the early return, which is the canonical @@ -51,24 +51,24 @@ well — the owned style was habit, so they carry no toolchain caveat: - `src/stdlib/collections.rs` — `group_by_filter` consumed its resolved key in `entry(key_value)` instead of cloning it first. - `src/ir/cycle.rs` — `detect_targets` snapshots borrowed - `&'targets Utf8Path` keys for its deterministic sort instead of cloning - every target path per analysis. The snapshot exists for sorting, not to - end a borrow, so it stays. + `&'targets Utf8Path` keys for its deterministic sort instead of cloning every + target path per analysis. The snapshot exists for sorting, not to end a + borrow, so it stays. - `src/stdlib/which/env.rs` — `EnvSnapshot::resolved_dirs` returns - `Vec<&Utf8Path>` borrowed from the snapshot; the search loop reads - borrowed directories and the paths are copied into the owned - `ResolveError::NotFound` only at the error boundary. + `Vec<&Utf8Path>` borrowed from the snapshot; the search loop reads borrowed + directories and the paths are copied into the owned `ResolveError::NotFound` + only at the error boundary. ## Refusals -Owned style retained deliberately. The constraint, not the borrow checker, -is load-bearing; each site carries the matching source tag: +Owned style retained deliberately. The constraint, not the borrow checker, is +load-bearing; each site carries the matching source tag: -| Site | Tag | Constraint | -| --- | --- | --- | -| `src/ir/from_manifest_support.rs` — `register_action` | `POLONIUS-REFUSED(id-is-data)` | The action hash is persistent IR identity: stored on every `BuildEdge` and named in the generated Ninja file. Remains owned unless callers demonstrate a need for the canonical interned value. | -| `src/stdlib/which/cache.rs` — `WhichResolver::try_cache` | `POLONIUS-REFUSED(lock-boundary)` | Cache hits are cloned out of the LRU because references cannot outlive the `MutexGuard`; the resolver is shared across evaluation sites. | -| `src/stdlib/collections.rs` — `GroupedValues::new` | `POLONIUS-REFUSED(miss-dominant)` | First-wins string-key registration almost always inserts, so the owned-key `entry` form pays nothing on the rare hit. | +| Site | Tag | Constraint | +| -------------------------------------------------------- | --------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/ir/from_manifest_support.rs` — `register_action` | `POLONIUS-REFUSED(id-is-data)` | The action hash is persistent IR identity: stored on every `BuildEdge` and named in the generated Ninja file. Remains owned unless callers demonstrate a need for the canonical interned value. | +| `src/stdlib/which/cache.rs` — `WhichResolver::try_cache` | `POLONIUS-REFUSED(lock-boundary)` | Cache hits are cloned out of the LRU because references cannot outlive the `MutexGuard`; the resolver is shared across evaluation sites. | +| `src/stdlib/collections.rs` — `GroupedValues::new` | `POLONIUS-REFUSED(miss-dominant)` | First-wins string-key registration almost always inserts, so the owned-key `entry` form pays nothing on the rare hit. | ## Non-candidates reviewed and cleared @@ -88,60 +88,58 @@ Scanner suspects that turned out not to be NLL residue: - Test-suite `drop()` calls (environment guards, HTTP fixture teardown) are semantic Drop effects, not borrow appeasement. -The plumbing itself is contract-tested: -`tests/polonius_toolchain_contract.rs` pins the dated-nightly channel, the -`.cargo/config.toml` `build.rustflags` entry, the `POLONIUS_FLAGS` default -and every RUSTFLAGS-setting Makefile recipe, and the `RUSTFLAGS` and -toolchain presets in the CI, Netsukefile, coverage, and packaging -workflows. +The plumbing itself is contract-tested: `tests/polonius_toolchain_contract.rs` +pins the dated-nightly channel, the `.cargo/config.toml` `build.rustflags` +entry, the `POLONIUS_FLAGS` default and every RUSTFLAGS-setting Makefile +recipe, and the `RUSTFLAGS` and toolchain presets in the CI, Netsukefile, +coverage, and packaging workflows. ## Harness consequences -Tooling that rebuilds the crate with its own flags must propagate the -Polonius flag or avoid compiling the crate: +Tooling that rebuilds the crate with its own flags must propagate the Polonius +flag or avoid compiling the crate: - **trybuild** discards ambient `RUSTFLAGS` and workspace `build.rustflags`, replacing them via `--config` on its scratch project, and it always builds the host crate as a fixture dependency. The Kani cfg policy fixture is therefore compiled and run directly with the workspace `rustc` - (`tests/kani_cfg_ui_tests.rs`); do not reintroduce trybuild cases that - depend on the `netsuke` crate while the tree is Polonius-only. + (`tests/kani_cfg_ui_tests.rs`); do not reintroduce trybuild cases that depend + on the `netsuke` crate while the tree is Polonius-only. - **Kani** and **Whitaker** run under their own toolchains but read the workspace `.cargo/config.toml` or the Makefile `RUSTFLAGS`, so they borrow-check with `-Zpolonius=next` and need no special handling. - **CI setup actions**: `actions-rust-lang/setup-rust-toolchain` exports `RUSTFLAGS="-D warnings"` into the job environment when the variable is - unset, which shadows `.cargo/config.toml` for every later step. The - workflows therefore pre-set `RUSTFLAGS` (including `-Zpolonius=next`) at - job level — the action defers to an existing value — and the Makefile - recipes append `POLONIUS_FLAGS` to any ambient `RUSTFLAGS` as a second - line of defence. `cargo-llvm-cov` appends its instrumentation flags to - the ambient value, so coverage inherits the flag from the job - environment. + unset, which shadows `.cargo/config.toml` for every later step. The workflows + therefore pre-set `RUSTFLAGS` (including `-Zpolonius=next`) at job level — + the action defers to an existing value — and the Makefile recipes append + `POLONIUS_FLAGS` to any ambient `RUSTFLAGS` as a second line of defence. + `cargo-llvm-cov` appends its instrumentation flags to the ambient value, so + coverage inherits the flag from the job environment. - **Registry installs**: the crates.io package excludes `rust-toolchain.toml` and `.cargo/config.toml`, and registry builds run outside the checkout, so `cargo install netsuke` must select the pinned nightly and pass the flag explicitly - (`RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke`). - The README and users' guide document the command and + (`RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke`). The + README and users' guide document the command and `tests/documentation_examples_tests.rs` pins it. - **cargo-mutants** (scheduled, informational) runs through the shared - `mutation-cargo.yml` workflow, which controls its own environment; if - those runs regress with E0499 at tagged sites, the shared workflow needs - the same `RUSTFLAGS` treatment. + `mutation-cargo.yml` workflow, which controls its own environment; if those + runs regress with E0499 at tagged sites, the shared workflow needs the same + `RUSTFLAGS` treatment. ## Clone counts -Measured with `rg --count '\.clone\(\)'` over `src/` (tests included where -they live in `src/`): +Measured with `rg --count '\.clone\(\)'` over `src/` (tests included where they +live in `src/`): -| Scope | Before | After | -| --- | --- | --- | -| `src/` total | 158 | 151 | -| `src/graph_view/mod.rs` | 17 | 14 | -| `src/ir/cycle.rs` (non-test) | 1 | 0 | -| `src/stdlib/which/env.rs` | 4 | 1 | -| `src/stdlib/collections.rs` | 4 | 3 | +| Scope | Before | After | +| ---------------------------- | ------ | ----- | +| `src/` total | 158 | 151 | +| `src/graph_view/mod.rs` | 17 | 14 | +| `src/ir/cycle.rs` (non-test) | 1 | 0 | +| `src/stdlib/which/env.rs` | 4 | 1 | +| `src/stdlib/collections.rs` | 4 | 3 | The scanner's clone-modify-writeback section was empty before and after the migration. The remaining graph_view clones construct owned keys for the two @@ -160,8 +158,8 @@ When `-Zpolonius=next` (or its successor) reaches stable Rust: ## Anti-regression guidance -The contract for new code and reviews (also summarized in `AGENTS.md` and -the [developers' guide](developers-guide.md)): +The contract for new code and reviews (also summarized in `AGENTS.md` and the +[developers' guide](developers-guide.md)): - Do not rewrite `POLONIUS(...)` sites into double lookups, `entry(key.clone())`, or `contains_key` guards — the direct form is @@ -171,7 +169,7 @@ the [developers' guide](developers-guide.md)): borrow-returning form compiles under the project toolchain. - Respect `POLONIUS-REFUSED(...)` tags: the named constraint (identity, locks, aliasing, suspension points, thread boundaries) is permanent, and - "simplifying" those sites into reference-returning forms will not compile - or will break the design. + "simplifying" those sites into reference-returning forms will not compile or + will break the design. - Classify any new borrow-centric API by compiling with and without the flag, then record it here. diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md index 775a78637..17bda652b 100644 --- a/docs/snapshot-testing-in-netsuke-using-insta.md +++ b/docs/snapshot-testing-in-netsuke-using-insta.md @@ -281,9 +281,9 @@ function, serializing locale state across the test suite. > In this repository the canonical runner is cargo-nextest: `make test`, or > `cargo nextest run --test ninja_snapshot_tests` for a focused run. See > [Test execution](developers-guide.md#test-execution). `cargo insta` needs to -> be told which runner to drive, so use `cargo insta test --test-runner -> nextest` and `cargo insta review` when accepting changes. The `cargo test` -> invocations below are the generic form. +> be told which runner to drive, so use +> `cargo insta test --test-runner nextest` and `cargo insta review` when +> accepting changes. The `cargo test` invocations below are the generic form. To execute the snapshot tests, run `cargo test`. All tests (including our new snapshot tests) will run. On the first run (or whenever a snapshot differs from diff --git a/docs/users-guide.md b/docs/users-guide.md index 9d039600c..324907b3a 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -11,15 +11,14 @@ change before 1.0. Pin the Netsuke version in automated workflows. ## Install Netsuke Netsuke requires [Ninja](https://ninja-build.org/) on `PATH`. A source build -also requires the dated Rust nightly toolchain pinned in -`rust-toolchain.toml`, because Netsuke builds with the Polonius borrow -checker (`-Zpolonius=next`); `rustup` installs it automatically inside a -checkout. +also requires the dated Rust nightly toolchain pinned in `rust-toolchain.toml`, +because Netsuke builds with the Polonius borrow checker (`-Zpolonius=next`); +`rustup` installs it automatically inside a checkout. Netsuke v0.1.0 is available from crates.io. Where -[`cargo binstall`](https://github.com/cargo-bins/cargo-binstall) is -available, prefer it: it fetches a prebuilt release binary and avoids the -toolchain requirement below. +[`cargo binstall`](https://github.com/cargo-bins/cargo-binstall) is available, +prefer it: it fetches a prebuilt release binary and avoids the toolchain +requirement below. @@ -28,8 +27,8 @@ cargo binstall netsuke ``` Building from the registry instead runs outside a repository checkout, so -neither the pinned toolchain nor the Polonius flag is picked up -automatically; supply both explicitly: +neither the pinned toolchain nor the Polonius flag is picked up automatically; +supply both explicitly: @@ -559,8 +558,7 @@ fall back to discovery. Pass `--verbose` to see how Netsuke selected its configuration. Structured events report whether `--config`, `NETSUKE_CONFIG`, or automatic discovery won, whether a path was present, and which environment lookups were attempted. -Events then identify whether Netsuke uses an explicit file or discovered -layers. +Events then identify whether Netsuke uses an explicit file or discovered layers. If an explicit file cannot be loaded, the warning records `failure_kind` as `Missing` or `LoadError`. Path fields are bounded to `path_hash` and