diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 02ba966c7..2d531f7db 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -43,6 +43,7 @@ updates: directories: - "/" - "/test_support" + - "/tests/ui/cli_configuration_pass" open-pull-requests-limit: 5 labels: - "dependencies" diff --git a/benches/config_load_cached_merge.rs b/benches/config_load_cached_merge.rs index f04a553fe..e5c218b8b 100644 --- a/benches/config_load_cached_merge.rs +++ b/benches/config_load_cached_merge.rs @@ -24,6 +24,10 @@ impl ConfigEnvProvider for BenchmarkEnv { fn get(&self, _key: &str) -> Option { None } + + fn entries(&self) -> Vec<(OsString, OsString)> { + Vec::new() + } } /// Create a large nested configuration payload with valid build targets. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 7cc01469e..5a3034a6c 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -285,25 +285,24 @@ The lowering stages have deliberately separate responsibilities: binding set for the recipe, then interpolates every scalar or list entry with that set. `{{ ins }}` and `{{ outs }}` markers and standalone `$in` and `$out` tokens are resolved per entry; tokens inside backticks are preserved. - The resulting action contains ordinary command text and no Ninja - placeholders. -- `src/ninja_gen/mod.rs` emits a scalar command unchanged. For a list, it puts + The resulting action contains ordinary command text and no Ninja placeholders. +- `src/ninja_gen.rs` emits a scalar command unchanged. For a list, it puts each entry in a brace group and joins the groups with `&&`. Each group uses `eval` with a shell-quoted entry payload. This keeps an inline comment or a trailing control operator such as `&` inside the entry from consuming the - generated group terminator. Braces run in the current shell, not a - subshell, so directory changes, environment assignments, and shell - variables can carry from one entry to the next. The `&&` chain remains - fail-fast. Each entry may start at most one background job; the generated - wrapper waits for that job before it evaluates a later entry. Ninja - generation rejects entries that start more than one background job. It also - rejects entries whose nested `eval` payload makes the background-job count - dynamic, because the wrapper cannot safely determine which jobs to wait for. - A direct simple `exec`, optionally prefixed by shell assignments, is - evaluated in a retaining subshell so its success or failure remains visible - to the wrapper; a successful `exec` ends the remaining chain. Structured or - nested `exec` forms are rejected during Ninja generation because the wrapper - cannot supervise them without changing their shell semantics. + generated group terminator. Braces run in the current shell, not a subshell, + so directory changes, environment assignments, and shell variables can carry + from one entry to the next. The `&&` chain remains fail-fast. Each entry may + start at most one background job; the generated wrapper waits for that job + before it evaluates a later entry. Ninja generation rejects entries that + start more than one background job. It also rejects entries whose nested + `eval` payload makes the background-job count dynamic because the wrapper + cannot safely determine which jobs to wait for. A direct simple `exec`, + optionally prefixed by shell assignments, is evaluated in a retaining + subshell so its success or failure remains visible to the wrapper; a + successful `exec` ends the remaining chain. Structured or nested `exec` forms + are rejected during Ninja generation because the wrapper cannot supervise + them without changing their shell semantics. - `src/runner/process` forwards the command's output and recognizes the bounded `netsuke command-list failure: action HASH, entry M` marker. A failed list therefore retains the original exit status while adding the fixed-width @@ -322,26 +321,25 @@ fragmented shell quoting, which is appropriate for a literal shell word but not for the command-list `eval` payload. That renderer requires a canonical single-quoted payload so existing generated Ninja list text remains byte-for-byte stable, and the delimiter/boundary tests continue to hold. Keep -that quoting in the deliberately local `shell_single_quote` function; it is -not a general-purpose helper. Neither quoting path is the platform-specific +that quoting in the deliberately local `shell_single_quote` function; it is not +a general-purpose helper. Neither quoting path is the platform-specific `src/stdlib/command/quote.rs` implementation behind the `command.quote` template wrapper, which must retain its `cmd.exe` quoting behaviour on Windows. -Attributed list failures emit the bounded tracing fields -`command_list_action` (a fixed-width action fingerprint) and -`command_list_entry` (the one-based entry index), plus the matching -`command_list_failure` marker. The process boundary records -`netsuke_ninja_command_list_failures_total` and -`netsuke_ninja_command_list_failure_duration_seconds`, with an `outcome` -label of `failure`. Elapsed failure duration is measured through the injected -`monotony::MonotonicClock`; production uses `StdMonotonicClock`, while tests use -deterministic test clocks. These diagnostics and metrics contain no command +Attributed list failures emit the bounded tracing fields `command_list_action` +(a fixed-width action fingerprint) and `command_list_entry` (the one-based +entry index), plus the matching `command_list_failure` marker. The process +boundary records `netsuke_ninja_command_list_failures_total` and +`netsuke_ninja_command_list_failure_duration_seconds`, with an `outcome` label +of `failure`. Elapsed failure duration is measured through the injected +`monotony::MonotonicClock`; production uses `StdMonotonicClock`, while tests +use deterministic test clocks. These diagnostics and metrics contain no command text. Changes to this pipeline must preserve the scalar/list distinction, per-entry rendering, current-shell state sharing, and failure attribution. The focused -rendering, lowering, Ninja-generation, and real-Ninja integration tests are -the behavioural contract for these boundaries. +rendering, lowering, Ninja-generation, and real-Ninja integration tests are the +behavioural contract for these boundaries. ## Package and target naming @@ -377,13 +375,12 @@ from the `bin-name` field that sidecar, per `[common.binstall]` in `.github/release-staging.toml`, and the "Hoist cargo-binstall archives" step in `.github/workflows/release.yml` runs `scripts/hoist_binstall_archives.py` under a pinned Python 3.13 installed by - `setup-uv`. The script validates that every target's archive and checksum - are present, are regular files rather than symlinks, and have a free - destination before moving them to the release root for upload; the read-only - discovery and validation half lives in `scripts/hoist_binstall_discovery.py`. + `setup-uv`. The script validates that every target's archive and checksum are + present, are regular files rather than symlinks, and have a free destination + before moving them to the release root for upload; the read-only discovery + and validation half lives in `scripts/hoist_binstall_discovery.py`. `tests/binstall_metadata_tests.rs` and - `tests/workflow_contracts/hoist_binstall_archives_test.py` hold this - contract. + `tests/workflow_contracts/hoist_binstall_archives_test.py` hold this contract. Only the two registry installation commands name `netsuke-build`, and `tests/documentation_installation_tests.rs` pins both. When adding a release @@ -434,12 +431,12 @@ confusing `E0499` rather than an obvious configuration error. Four workflows carry the contract: -| Workflow | Job | Shared action | `with.rustflags` | -| --- | --- | --- | --- | -| [`ci.yml`](../.github/workflows/ci.yml) | `build-test` | `setup-rust` | `-D warnings -Zpolonius=next` | -| [`coverage-main.yml`](../.github/workflows/coverage-main.yml) | `coverage-upload` | `setup-rust` | `-D warnings -Zpolonius=next` | -| [`netsukefile-test.yml`](../.github/workflows/netsukefile-test.yml) | `netsukefile` | `setup-rust` | `-Zpolonius=next` | -| [`build-and-package.yml`](../.github/workflows/build-and-package.yml) | `build` | `rust-build-release` | `-Zpolonius=next` | +| Workflow | Job | Shared action | `with.rustflags` | +| --------------------------------------------------------------------- | ----------------- | -------------------- | ----------------------------- | +| [`ci.yml`](../.github/workflows/ci.yml) | `build-test` | `setup-rust` | `-D warnings -Zpolonius=next` | +| [`coverage-main.yml`](../.github/workflows/coverage-main.yml) | `coverage-upload` | `setup-rust` | `-D warnings -Zpolonius=next` | +| [`netsukefile-test.yml`](../.github/workflows/netsukefile-test.yml) | `netsukefile` | `setup-rust` | `-Zpolonius=next` | +| [`build-and-package.yml`](../.github/workflows/build-and-package.yml) | `build` | `rust-build-release` | `-Zpolonius=next` | CI and coverage add `-D warnings` because those jobs gate on a warning-free build; the Netsukefile and packaging jobs carry the Polonius flag alone, so a @@ -675,30 +672,30 @@ so pinning the SHA in a test buys nothing and costs a manual edit per bump. #### Exception: the Polonius shared-action contract -The four workflows described under [Polonius CI shared-action -contract](#polonius-ci-shared-action-contract) do depend on a specific -revision. The `rustflags` input they rely on was introduced at a known commit -in `leynos/shared-actions`. A revision that predates it does not fail the run — -an unrecognized `with:` key on a composite action is a warning, not an error — -it simply never exports the flag, so the build fails later as a borrow-check -error rather than as a configuration error. +The four workflows described under +[Polonius CI shared-action contract](#polonius-ci-shared-action-contract) do +depend on a specific revision. The `rustflags` input they rely on was +introduced at a known commit in `leynos/shared-actions`. A revision that +predates it does not fail the run — an unrecognized `with:` key on a composite +action is a warning, not an error — it simply never exports the flag, so the +build fails later as a borrow-check error rather than as a configuration error. `tests/polonius_toolchain_contract.rs` therefore requires the four workflows' shared-action references to agree, rather than restating the expected pin as a constant. It extracts every `leynos/shared-actions` reference from the checked workflows with the shared YAML-parsing helper in -`tests/support/shared_actions.rs`, validates that each is a full -40-character lowercase-hex commit SHA, and derives the pin the workflows must -share from that set. A complete bump — Dependabot's or a manual one — moves -every reference together and passes with no test edit. A partial bump, where -some workflows move and others are left behind, fails on the disagreement -between references: the same failure that previously broke `main` when a bump -missed the hand-maintained constants this contract used to hold. The -revision-level dependency on the `rustflags` input is now protected by that -agreement requirement together with `shared-actions`' own contract tests -upstream, rather than by a constant edited by hand here. Restrict this -exception to callers with a genuine revision-level dependency; everywhere -else, the shape-only policy applies. +`tests/support/shared_actions.rs`, validates that each is a full 40-character +lowercase-hex commit SHA, and derives the pin the workflows must share from +that set. A complete bump — Dependabot's or a manual one — moves every +reference together and passes with no test edit. A partial bump, where some +workflows move and others are left behind, fails on the disagreement between +references: the same failure that previously broke `main` when a bump missed +the hand-maintained constants this contract used to hold. The revision-level +dependency on the `rustflags` input is now protected by that agreement +requirement together with `shared-actions`' own contract tests upstream, rather +than by a constant edited by hand here. Restrict this exception to callers with +a genuine revision-level dependency; everywhere else, the shape-only policy +applies. If a workflow's behaviour does not depend on a feature from a particular commit onwards, do not assert its SHA — express any advisory note as a comment or a @@ -869,9 +866,9 @@ PowerShell external help under date from `SOURCE_DATE_EPOCH`, falling back to `1970-01-01` when unset or invalid. -Shell completions are generated separately by `build.rs` from -`Cli::command()` for Bash, Elvish, Fish, PowerShell, and Zsh. Release staging -copies these portable completion sidecars into each standalone archive under +Shell completions are generated separately by `build.rs` from `Cli::command()` +for Bash, Elvish, Fish, PowerShell, and Zsh. Release staging copies these +portable completion sidecars into each standalone archive under `completions//`. They remain separate files for users to copy into the completion location documented by their shell; package installation does not claim to install them. @@ -1495,16 +1492,15 @@ the package-versus-target naming split described in [package and target naming](#package-and-target-naming). The first asserts the manual page `build.rs` generates; the second pins the single `[package.metadata.binstall]` `pkg-url` template against the release staging -configuration and the workflow target matrix, and fails if per-target -overrides reappear. +configuration and the workflow target matrix, and fails if per-target overrides +reappear. The hoist step that makes that template resolvable is covered by `tests/workflow_contracts/hoist_binstall_archives_test.py`, which combines example-based cases with Hypothesis property tests over generated target sets and staging states. Run it with `make test-workflow-contracts`; the target provisions `pytest`, `pyyaml`, and `hypothesis` through `uv run --with`, so -`uv` is the only prerequisite and no virtual environment needs creating by -hand. +`uv` is the only prerequisite and no virtual environment needs creating by hand. ### Temporary executable test helpers @@ -1578,9 +1574,9 @@ all valid inputs. `explicit_config_path` selector-precedence invariant for generated optional paths. - Layer-precedence and replay transitions are also property-tested: - `tests/cli_tests/merge_precedence_proptests.rs` asserts scalar precedence - and list appending for arbitrary file, environment, and CLI layer - combinations, and `src/cli/discovery_replay_proptests.rs` proves repeated + `tests/cli_tests/merge_precedence_proptests.rs` asserts scalar precedence and + list appending for arbitrary file, environment, and CLI layer combinations, + and `src/cli/discovery_replay_proptests.rs` proves repeated discovery-diagnostic replays stay identical without re-reading the environment. @@ -1636,10 +1632,10 @@ Each gate reveals one real dependency through a pre-materialized Ninja dyndep file. The gate edge associated with the next sidecar depends on the preceding gate, which keeps later direct dependencies unavailable to the scheduler until earlier work succeeds. The runner materializes every sidecar file before Ninja -starts; no Ninja edge produces sidecar content. This is not an order-only -chain or a Ninja pool: both leave the real dependencies visible to Ninja too -early. Preserve one top-level Ninja invocation so shared nodes keep Ninja's -normal execute-once memoization. +starts; no Ninja edge produces sidecar content. This is not an order-only chain +or a Ninja pool: both leave the real dependencies visible to Ninja too early. +Preserve one top-level Ninja invocation so shared nodes keep Ninja's normal +execute-once memoization. `GeneratedNinja` is the query-command boundary: generation may construct and return it, but it must not publish any filesystem state. @@ -1663,10 +1659,10 @@ or successful clean while retaining the lease through bundle consumption. `.netsuke/dyndep` directory lease through Ninja or generated-output consumption. While the lease is held, stale `.tmp` files are removed and obsolete `.dd` files are retained in deterministic path order up to 32 files -and 1 MiB. The current bundle is always retained. `build` and `generate` -prune after materialization; `clean` prunes only after successful -`ninja -t clean`, never after a failed clean. Do not introduce age-based -cleanup or mutate an existing content-addressed sidecar. See +and 1 MiB. The current bundle is always retained. `build` and `generate` prune +after materialization; `clean` prunes only after successful `ninja -t clean`, +never after a failed clean. Do not introduce age-based cleanup or mutate an +existing content-addressed sidecar. See [ADR-012](adr-012-bound-dyndep-sidecar-retention.md) for the durable policy. `src/runner/dyndep_generation_telemetry.rs` owns runner-boundary generation @@ -1679,8 +1675,8 @@ explicit. The intended serial guarantee is path-scoped. A later dependency that is independently reachable elsewhere in the requested graph may start via that -other path. Do not broaden the implementation with a global lock, pool, or -new scheduler without an approved design change. See +other path. Do not broaden the implementation with a global lock, pool, or new +scheduler without an approved design change. See [ADR-011](adr-011-use-ninja-dyndep-for-serial-dependency-ordering.md) for the durable decision and its alternatives. @@ -1814,9 +1810,9 @@ Glob expansion lives in `src/manifest/glob/`, and `glob_paths` is its only boundary. `src/manifest/mod.rs` declares `mod glob;` privately and re-exports just that function, so nothing else in the module — `GlobPattern`, the error helpers in `glob/errors.rs`, the `walk` submodule, or the `GlobEntryResult` -alias — is reachable from the crate root. `GlobEntryResult` in particular -stays private to `manifest::glob`: only `glob_paths` and `walk` consume it, and -it names a `glob` crate type that callers should never have to depend on. +alias — is reachable from the crate root. `GlobEntryResult` in particular stays +private to `manifest::glob`: only `glob_paths` and `walk` consume it, and it +names a `glob` crate type that callers should never have to depend on. Two compile-time guards hold that boundary: @@ -1826,14 +1822,15 @@ Two compile-time guards hold that boundary: because `src/manifest/mod.rs` deliberately re-exports it, making it genuinely reachable; every item here that is not re-exported stays guarded. - A pair of doctests attached to the public `glob_paths` documentation: a - `compile_fail,E0603` block importing `netsuke::manifest::glob::GlobEntryResult` - and a passing block importing `netsuke::manifest::glob_paths`. Together they - validate the downstream view — the alias has no public path, while the entry - point does. The passing block is the control: if the rustdoc harness wiring - breaks, it fails rather than letting the rejection pass vacuously. Both are - attached to `glob_paths` rather than to the private items they describe - because rustdoc renders and runs the examples of public items, which also - makes the boundary discoverable from the published API documentation. + `compile_fail,E0603` block importing + `netsuke::manifest::glob::GlobEntryResult` and a passing block importing + `netsuke::manifest::glob_paths`. Together they validate the downstream view — + the alias has no public path, while the entry point does. The passing block + is the control: if the rustdoc harness wiring breaks, it fails rather than + letting the rejection pass vacuously. Both are attached to `glob_paths` + rather than to the private items they describe because rustdoc renders and + runs the examples of public items, which also makes the boundary discoverable + from the published API documentation. When adding to this module, keep new items private, or `pub(super)` when a sibling submodule needs them; widen the boundary only by adding a deliberate @@ -1846,8 +1843,8 @@ The metadata check that filters directories out of a glob's results goes through a `cap_std::fs::Dir` handle rather than a raw filesystem call. `walk::open_root_dir` opens that handle at the pattern's longest literal directory prefix, computed by `walk::literal_dir_prefix`: the pattern text up -to the first `*`, `?`, `[`, or `{`, trimmed back to the last path separator. -For `src/**/*.c` that prefix is `src/`. +to the first `*`, `?`, `[`, or `{`, trimmed back to the last path separator. For +`src/**/*.c` that prefix is `src/`. `walk::open_literal_prefix` owns the opening policy: it opens the lexical root or current directory ambiently once, then opens each normal literal component @@ -1856,38 +1853,36 @@ metadata lookups remain the responsibility of that root. - **Bracketed literal escapes do not stop the scan.** The `[*]`, `[?]`, `[[]`, `[]]`, `[{]`, and `[}]` forms that `normalize::force_literal_escapes` - produces from `\*`, `\?`, and the like name a literal character rather than - a wildcard, so `src/[*]x/generated/*.c` reaches `src/[*]x/generated/`, not + produces from `\*`, `\?`, and the like name a literal character rather than a + wildcard, so `src/[*]x/generated/*.c` reaches `src/[*]x/generated/`, not `src/`. A genuine character class such as `[ab]` is still a wildcard and still stops the scan. The resulting prefix is still pattern text, so `walk::unescape_literal_escapes` resolves it to the path it names - (`src/[*]x/` becomes the directory `src/*x/`) before the capability is - opened and before any match is stripped of it. + (`src/[*]x/` becomes the directory `src/*x/`) before the capability is opened + and before any match is stripped of it. - **`GlobRoot` couples the handle with the prefix.** Matches keep the pattern's own rooting as they arrive from the `glob` crate's walker — an absolute pattern yields absolute matches, while a parent-relative pattern such as `../*.txt` yields matches like `../out.txt` — so - `GlobRoot::relativise` rebases each one onto the prefix before the - metadata lookup. A path that does not start with the prefix is rejected - outright rather than resolved through a wider capability. + `GlobRoot::relativise` rebases each one onto the prefix before the metadata + lookup. A path that does not start with the prefix is rejected outright + rather than resolved through a wider capability. - **No literal directory component falls back to the working directory.** A pattern such as `*.c` yields a prefix of `.`. `walk::prefix_is_unopenable` treats a missing prefix, and a prefix that names something other than a - directory, as no capability at all; `glob_paths` then returns an empty - match set rather than an error. Any other failure to open the prefix - propagates. + directory, as no capability at all; `glob_paths` then returns an empty match + set rather than an error. Any other failure to open the prefix propagates. - **`walk::is_unresolvable_link` governs which failed lookups are skipped rather than fatal.** Only `io::ErrorKind::PermissionDenied` (an escape from - the capability's tree, or a genuine permission failure the capability - cannot distinguish from one) and `io::ErrorKind::NotFound` (a dangling - link) count, and only when some component of the matched path is actually - a symbolic link. A `FilesystemLoop` is a broken tree rather than an absent - file, so it propagates instead of being skipped. + the capability's tree, or a genuine permission failure the capability cannot + distinguish from one) and `io::ErrorKind::NotFound` (a dangling link) count, + and only when some component of the matched path is actually a symbolic link. + A `FilesystemLoop` is a broken tree rather than an absent file, so it + propagates instead of being skipped. - **The boundary that remains.** The match walk itself is the `glob` crate's, - and that crate traverses the filesystem ambiently. Only the metadata check - is capability-scoped, so narrowing the capability's opening point narrows - what the metadata check can resolve, not what the walk itself can see on - disk. + and that crate traverses the filesystem ambiently. Only the metadata check is + capability-scoped, so narrowing the capability's opening point narrows what + the metadata check can resolve, not what the walk itself can see on disk. [ADR-010](adr-010-scope-glob-capability-to-literal-prefix.md) records the decision to scope the capability this way and the alternatives it rejected. @@ -1900,12 +1895,11 @@ neither reaches the top-level diagnostics: a literal prefix that names no directory, and matches dropped because a symbolic link cannot be resolved through the capability, including an unreadable link within the prefix. It aggregates every skipped entry while retaining at most the first four -unreachable-symlink paths as a trace sample. The -`src/manifest/mod.rs` adapter records those observations after the query at the -Jinja `glob` helper's orchestration boundary, via `glob::record_expansion`. -Keeping recording there leaves the expansion query free of metrics and tracing -side effects while keeping a degraded expansion visible without having to -reproduce it. +unreachable-symlink paths as a trace sample. The `src/manifest/mod.rs` adapter +records those observations after the query at the Jinja `glob` helper's +orchestration boundary, via `glob::record_expansion`. Keeping recording there +leaves the expansion query free of metrics and tracing side effects while +keeping a degraded expansion visible without having to reproduce it. - **Metrics** — `netsuke_manifest_glob_expansions_total`, labelled `outcome` (`matched`, `unopenable_prefix`), and @@ -1917,9 +1911,9 @@ reproduce it. - **Tracing** — every caller-controlled path field is replaced with the stable `` marker: patterns, prefixes, and sampled relative matches. A skipped unreachable-symlink event is emitted only for the retained sample, - with no more than four such events per expansion. Metrics retain only - bounded aggregate status and reason data; errors may retain the caller's - original pattern so invalid input can be explained precisely. + with no more than four such events per expansion. Metrics retain only bounded + aggregate status and reason data; errors may retain the caller's original + pattern so invalid input can be explained precisely. ## Test isolation utilities @@ -2002,8 +1996,8 @@ at the controlled pre-persist point, returns `io::ErrorKind::IsADirectory`. When a manifest is missing, its generated contents are written to a temporary file staged in the destination directory before persistence. The tests inject -the pre-persist action to cover controlled creation orderings; they do not claim -to model arbitrary scheduler or filesystem interleavings. The fallible +the pre-persist action to cover controlled creation orderings; they do not +claim to model arbitrary scheduler or filesystem interleavings. The fallible `test_support::fs::inspect_path` probe treats `NotFound` as absence and propagates every other metadata error. @@ -2090,20 +2084,20 @@ modules share. It reads the `POLONIUS_FLAGS` variable and rejects two states: only, reported as "POLONIUS_FLAGS should not be empty". The rejection matters because every assertion built on the resolved value uses -`contains`, which an empty needle satisfies vacuously. Returning an empty string -would silently void the Polonius contract instead of failing it. On success the -resolver returns the trimmed value, and a bounded proptest in +`contains`, which an empty needle satisfies vacuously. Returning an empty +string would silently void the Polonius contract instead of failing it. On +success the resolver returns the trimmed value, and a bounded proptest in `rustflags_polonius_tests` pins that acceptance invariant: resolution succeeds exactly when a definition exists whose value is non-empty after trimming, and the resolved text is the trimmed value. Its generator emits absent, -whitespace-only, and whitespace-padded flag tokens, so an untrimmed return fails -the property rather than slipping past. +whitespace-only, and whitespace-padded flag tokens, so an untrimmed return +fails the property rather than slipping past. Because `rustflags.rs` and `rustflags_polonius_tests.rs` are children of the `makefile_test_target` test binary rather than files under `tests/`, Cargo does -not compile them as separate targets. The root declares them, and they reach the -shared helpers through `use super::{read_repo_file, target_recipe}`. Keep this -shape for further Makefile contract work: general parsing helpers belong in +not compile them as separate targets. The root declares them, and they reach +the shared helpers through `use super::{read_repo_file, target_recipe}`. Keep +this shape for further Makefile contract work: general parsing helpers belong in `tests/support/makefile.rs` once a second contract test needs them, whereas model types such as `RustflagsCase` stay private to the contract they describe. @@ -2167,9 +2161,9 @@ boundary policy. The seams described in this section follow one of three sanctioned shapes — narrow closures, `mockable::Env`, or `EnvReader` — chosen by call-site count, expected growth, and `Send + Sync` registration requirements: use -`mockable::Env` when a boundary is expected to acquire more inputs, even -before its call-site count grows. See -[ADR-008](adr-008-environment-seam-taxonomy.md) for the taxonomy. +`mockable::Env` when a boundary is expected to acquire more inputs, even before +its call-site count grows. See [ADR-008](adr-008-environment-seam-taxonomy.md) +for the taxonomy. `manifest::EnvReader` owns environment lookup for the manifest `env()` helper. Production constructs the process-backed adapter at the manifest loading @@ -2263,9 +2257,9 @@ returning `None` for anything unrecognized — hides exactly the regression a test double should catch: if the code under test starts reading a differently named variable, through a rename, a typo, or a new precedence rung, a permissive stub answers `None` and the test still passes, asserting nothing -about the new read. Recognize the panic message, `"which the test did not -declare"`, when a test starts failing after a rename; it means the test's -declarations need updating, not that the stub is broken. +about the new read. Recognize the panic message, +`"which the test did not declare"`, when a test starts failing after a rename; +it means the test's declarations need updating, not that the stub is broken. Three distinct states are representable for a key: **declared with a value** (`with_var`), **declared but unset** (`allowing`, which reports `None`), and @@ -2275,48 +2269,48 @@ distinguishable from a variable the test never expected to be read at all. `StubEnv::with_locale` and `StubEnv::without_locale` are the common constructors for `NETSUKE_LOCALE`; `strict()` starts from nothing declared. -Declaring the same key twice is well-defined: the most recent declaration -wins, in either order. `allowing` after `with_var` clears the value; `with_var` -after `allowing` restores one. Were `allowing` merely to append to the -permitted-keys list rather than clearing the stored value, it would read as -declaring the key unset while still answering with the earlier value. +Declaring the same key twice is well-defined: the most recent declaration wins, +in either order. `allowing` after `with_var` clears the value; `with_var` after +`allowing` restores one. Were `allowing` merely to append to the permitted-keys +list rather than clearing the stored value, it would read as declaring the key +unset while still answering with the earlier value. `Default` is deliberately **not** implemented for `StubEnv`. On a strict stub, "default" would have to mean "deny every read", so `StubEnv::default()` would compile and then panic at run time for the common "no locale set" case; requiring `StubEnv::without_locale()` instead makes that intent explicit at compile time. This refusal is itself a tested contract: -`tests/locale_stub_ui_tests.rs` compiles a fixture calling -`StubEnv::default()` directly with `rustc` and asserts the compile fails with -`E0599` naming the missing `default` item, guarding against the constraint -regressing to a doc-comment promise. `tests/locale_stub_strictness_tests.rs` -covers the panic, the trichotomy, and the last-declaration-wins rule with -both example-based and property tests. +`tests/locale_stub_ui_tests.rs` compiles a fixture calling `StubEnv::default()` +directly with `rustc` and asserts the compile fails with `E0599` naming the +missing `default` item, guarding against the constraint regressing to a +doc-comment promise. `tests/locale_stub_strictness_tests.rs` covers the panic, +the trichotomy, and the last-declaration-wins rule with both example-based and +property tests. #### Locale-stub UI harness and split build directories -`tests/locale_stub_ui_tests.rs` builds `test_support` with `cargo build ---message-format=json` and parses the resulting Cargo JSON messages rather -than assuming its dependencies sit beside the uplifted `test_support` rlib. -For every `compiler-artifact` message it records the parent directory of each -rlib the message names, and passes the whole set to `rustc` as `-L -dependency=` directories when compiling the UI fixtures. This keeps the -harness correct when Cargo's `build.build-dir` setting splits intermediate -artefacts — where dependency rlibs live — from the final, uplifted ones; -deriving the directories from what Cargo actually reports, rather than from a -single assumed location, means the harness does not need to special-case that -split. - -`harness_compiles_under_a_split_build_dir` is the regression test for this: -it forces a split layout with its own private `CARGO_TARGET_DIR` and +`tests/locale_stub_ui_tests.rs` builds `test_support` with +`cargo build --message-format=json` and parses the resulting Cargo JSON +messages rather than assuming its dependencies sit beside the uplifted +`test_support` rlib. For every `compiler-artifact` message it records the +parent directory of each rlib the message names, and passes the whole set to +`rustc` as `-L dependency=` directories when compiling the UI fixtures. This +keeps the harness correct when Cargo's `build.build-dir` setting splits +intermediate artefacts — where dependency rlibs live — from the final, uplifted +ones; deriving the directories from what Cargo actually reports, rather than +from a single assumed location, means the harness does not need to special-case +that split. + +`harness_compiles_under_a_split_build_dir` is the regression test for this: it +forces a split layout with its own private `CARGO_TARGET_DIR` and `CARGO_BUILD_BUILD_DIR` roots, confirms the collected dependency directories -span the split, and then compiles a fixture against them. The roots are -private to the test rather than the ambient target directory because the -`#[once]` `test_support_rlib` fixture builds concurrently for +span the split, and then compiles a fixture against them. The roots are private +to the test rather than the ambient target directory because the `#[once]` +`test_support_rlib` fixture builds concurrently for `stub_env_default_does_not_compile` and -`stub_env_builders_compile_under_the_same_harness`. Sharing a target -directory would make `harness_compiles_under_a_split_build_dir` race that -build on the uplifted rlibs and fail with version-skew errors (`E0460`). +`stub_env_builders_compile_under_the_same_harness`. Sharing a target directory +would make `harness_compiles_under_a_split_build_dir` race that build on the +uplifted rlibs and fail with version-skew errors (`E0460`). ### Manifest `env()` reader @@ -2527,12 +2521,30 @@ bounded deferred diagnostics. The diagnostic-mode resolver, resolves the JSON preference from those discovered layers and preserves the outcome for the startup boundary. +Normal command-line use requires no change. The Rust API remains an unstable +beta surface, but callers that compose configuration themselves can avoid +discovering and loading the same configuration files more than once. At the +composition boundary, call `DiscoveryOutcome::emit_diagnostics()` after +tracing is configured, then consume the outcome with `into_layers()` and pass +the cached layers to `merge_with_cached_file_layers` for the full merge. This +preserves diagnostics from the same discovery pass while avoiding repeated +file loading. + +If the composition boundary times discovery itself, call the public +`record_discovery_outcome(&clock, started, &outcome)` after the pass completes. +`started` is the `std::time::Instant` captured from the same injected +`monotony::MonotonicClock` immediately before discovery; the function records +the elapsed duration and retained outcome without rediscovering. It also +recreates the bounded `collect_diag_file_layers` tracing span, recording +`outcome=success` or `outcome=error`; a discovery failure records +`error_category=file` for an `OrthoError::File` error. + `DiscoveryOutcome::emit_diagnostics` replays the retained bounded diagnostics after the composition boundary configures tracing. Callers must explicitly replay diagnostics there; the method does not repeat environment or filesystem -access. `DiscoveryOutcome::into_layers` transfers the same -`DiscoveredLayers` to `merge_with_cached_file_layers`, which consumes the -cached layers for the full merge and prevents a second discovery pass. +access. `DiscoveryOutcome::into_layers` transfers the same `DiscoveredLayers` to +`merge_with_cached_file_layers`, which consumes the cached layers for the full +merge and prevents a second discovery pass. `make bench-config-load` exercises early JSON resolution and the cached merge with a large nested configuration payload. It protects the ownership transfer @@ -2578,8 +2590,8 @@ Configuration merge helpers: - `explicit_config_path_with_env(cli, env) -> Option` resolves explicit config selection from `--config` and `NETSUKE_CONFIG`. - `discover_file_layers(cli, env) -> DiscoveryOutcome` performs one discovery - pass and retains the discovered layers, discovery errors and bounded - deferred diagnostics for the diagnostic and merge callers. + pass and retains the discovered layers, discovery errors and bounded deferred + diagnostics for the diagnostic and merge callers. - `push_discovered_file_layers(composer, errors, discovered) -> ()` transfers the retained layers and discovery errors into the full merge composition. - `collect_file_layers_with_trace_and_env_source(directory, env_source)` runs @@ -2595,8 +2607,6 @@ Configuration merge helpers: - `retain_layers_and_resolve_json(layers)` transfers each owned file-layer value into the cached layer while recording the last valid `json` value, avoiding complete layer or JSON-value copies before the full merge. -- `json_from_matches(cli, matches, discovered) -> bool` applies an explicit - root `--json` override to the discovered value. - `cli_overrides_from_matches(matches: &ArgMatches) -> OrthoValue` extracts CLI-supplied fields, stripping defaults and non-CLI sources. - `EnvironmentLayer` converts an injected snapshot of `NETSUKE_*` values into @@ -2619,21 +2629,21 @@ pub trait ConfigEnvProvider { ``` `get` owns selector lookup, while `entries` supplies the complete snapshot for -the layered `NETSUKE_*` merge. A selector-only provider may retain the empty -default for `entries`. Full-merge adapters must return a stable owned snapshot +the layered `NETSUKE_*` merge. A selector-only provider may return an empty +vector from `entries`; full-merge adapters must return a stable owned snapshot so discovery and value merging observe one environment. Keep this port scoped to CLI configuration; runner, manifest, locale, and stdlib environment seams remain separate because their input and lifetime contracts differ. -`discovery_env_source(env)` is the crate-private adapter that projects -Netsuke's `ConfigEnvProvider` port into the `SharedEnvSource` OrthoConfig -discovery accepts. Ambient and injected entry points alike pass through this -one adapter: `ConfigStdEnvProvider` backs ambient runs, while injected entry -points pass the same `ConfigEnvProvider` value that drives selector and -`NETSUKE_*` lookups. The projection is closed — only `NETSUKE_CONFIG`, `HOME`, -`USERPROFILE`, `XDG_CONFIG_HOME`, `XDG_CONFIG_DIRS`, `APPDATA`, and -`LOCALAPPDATA` appear — so it is not a general environment-copy helper; -`EnvironmentLayer` alone enumerates the full `NETSUKE_*` value environment. +`discovery_env_source(env)` is the crate-private adapter that projects Netsuke's +`ConfigEnvProvider` port into the `SharedEnvSource` OrthoConfig discovery +accepts. Ambient and injected entry points alike pass through this one adapter: +`ConfigStdEnvProvider` backs ambient runs, while injected entry points pass the +same `ConfigEnvProvider` value that drives selector and `NETSUKE_*` lookups. +The projection is closed — only `NETSUKE_CONFIG`, `HOME`, `USERPROFILE`, +`XDG_CONFIG_HOME`, `XDG_CONFIG_DIRS`, `APPDATA`, and `LOCALAPPDATA` appear — so +it is not a general environment-copy helper; `EnvironmentLayer` alone +enumerates the full `NETSUKE_*` value environment. `explicit_config_path_with_env` is the crate-internal seam for explicit config-file selection. It evaluates the precedence chain in this order: @@ -2670,10 +2680,9 @@ uses the bare `EnvProvider` name. Tests for injected configuration discovery should provide a map-backed `ConfigEnvProvider`. End-to-end tests of the ambient `ConfigStdEnvProvider` -adapter must run in an isolated child configured with `env_clear()` followed -by `Command::env`. `EnvLock` is reserved for tests that change the process -working directory alongside `CwdGuard`; it does not justify environment -mutation. +adapter must run in an isolated child configured with `env_clear()` followed by +`Command::env`. `EnvLock` is reserved for tests that change the process working +directory alongside `CwdGuard`; it does not justify environment mutation. Unit tests that only need to verify explicit config path precedence should test `explicit_config_path_with_env` with an injected provider instead of mutating @@ -2685,15 +2694,34 @@ evaluated, and emits no tracing itself. `discover_file_layers` retains the bounded diagnostics produced by that resolution and by layer loading; `DiscoveryOutcome::emit_diagnostics` replays them after tracing is configured. -Deferred discovery diagnostics never log raw configuration paths, configuration -file names or formatted parser errors. They expose bounded `path_hash` and -presence fields, and classify load failures with the `ConfigLoadFailureKind` -enum instead of formatted error text. The unkeyed `path_hash` is a bounded -correlation identifier, not confidential concealment of a guessable path. +#### Discovery pass telemetry + +The composition boundary records each file-layer discovery pass after the pure +query returns. `DISCOVERY_TOTAL` has a bounded `outcome` label of `success` or +`error`, and `DISCOVERY_DURATION` records the elapsed duration. A failed pass +also emits a bounded `error_category` from the closed set `file`, `validation`, +`cyclic_extends`, `cli_parsing`, `gathering`, `merge`, `aggregate`, and +`other`. These metrics and events never include selectors, paths, or +configuration values. + +The workspace's recorder-backed tests exercise both outcomes, the bounded +failure classification, and the single duration sample through local +`metrics_util::DebuggingRecorder` instances. + +Deferred configuration-discovery diagnostics never log full paths, file names, +or formatted parser errors. Path values in those events are bounded to a +`path_hash` correlation identifier plus a presence indicator. Load failures are +classified with the `ConfigLoadFailureKind` enum instead of the formatted error +text. The terminal human-mode `configuration load failed` event emitted by +`config_err_to_exit` is separate: it emits bounded `operation` and +`error_category` fields. It does not emit formatted error text or paths. +`path_hash` is a bounded identifier for correlating events, not a cryptographic +guarantee. This deferred contract is distinct from terminal `configuration load failed` -records emitted by `src/main.rs`. Those terminal records identify the failed -operation and coarse error category without rendering the source error. +records emitted by `config_load::config_err_to_exit`. Those terminal records +identify the failed operation and coarse error category without rendering the +source error. #### `json` contract @@ -2707,13 +2735,13 @@ explicit root `--json` flag bypasses environment parsing. #### Workspace fallback switch seam `src/stdlib/which/workspace_switch.rs` is a leaf module holding the -`NETSUKE_WHICH_WORKSPACE` name and the domain state `WorkspaceSwitch` -(`Value`, `Absent`, `NotUnicode`) with its `enabled()` decision. The variable -is read by `EnvSnapshot::capture` through the injected `mockable::Env` -provider and stored as snapshot data; the enable/disable decision is derived -from that snapshot on demand. The cache fingerprint hashes the state — -`WorkspaceSwitch` derives `Hash` for exactly that purpose — so two resolutions -differing only in this switch never share a cache entry. +`NETSUKE_WHICH_WORKSPACE` name and the domain state `WorkspaceSwitch` (`Value`, +`Absent`, `NotUnicode`) with its `enabled()` decision. The variable is read by +`EnvSnapshot::capture` through the injected `mockable::Env` provider and stored +as snapshot data; the enable/disable decision is derived from that snapshot on +demand. The cache fingerprint hashes the state — `WorkspaceSwitch` derives +`Hash` for exactly that purpose — so two resolutions differing only in this +switch never share a cache entry. The adapter owns everything platform-specific. `env.rs` holds the `From>` conversion, the single point at @@ -2725,14 +2753,14 @@ nor `tracing`, and consulting the switch afterwards is silent. See #### Ninja program resolver seam -`resolve_ninja_program_utf8_with` in `src/runner/process/ninja_program.rs` -takes `&impl mockable::Env`, with `mockable::DefaultEnv` as the production -adapter supplied by the ambient `resolve_ninja_program_utf8` wrapper. The unit -tests inject a `MockEnv` that pins the `NETSUKE_NINJA` key, so every override -branch runs without process mutation. +`resolve_ninja_program_utf8_with` in `src/runner/process/ninja_program.rs` takes +`&impl mockable::Env`, with `mockable::DefaultEnv` as the production adapter +supplied by the ambient `resolve_ninja_program_utf8` wrapper. The unit tests +inject a `MockEnv` that pins the `NETSUKE_NINJA` key, so every override branch +runs without process mutation. -`resolve_ninja_program_with`, in the same module, takes the identical `&impl -mockable::Env` seam and converts the UTF-8 result into a general platform +`resolve_ninja_program_with`, in the same module, takes the identical +`&impl mockable::Env` seam and converts the UTF-8 result into a general platform `PathBuf`. It is compiled only under `#[cfg(test)]`: production reaches the platform-path form through `resolve_ninja_program`, which itself calls the UTF-8 resolver and converts its result, so no production path constructs a @@ -2740,9 +2768,9 @@ platform `PathBuf` independently of `resolve_ninja_program_utf8_with`. #### `which` environment capture -`EnvSnapshot::capture` (`stdlib::which::env`) reads `PATH` on every -platform, and `PATHEXT` on Windows only, through an injected -`mockable::Env` provider rather than straight from the process: +`EnvSnapshot::capture` (`stdlib::which::env`) reads `PATH` on every platform, +and `PATHEXT` on Windows only, through an injected `mockable::Env` provider +rather than straight from the process: - `capture` is the production entry point. It delegates to `capture_with_env` with `mockable::DefaultEnv`, so it is the single site that binds the @@ -2767,10 +2795,10 @@ process: `StdlibConfig::with_path_override` and `StdlibConfig::with_pathext_override` are copied into `WhichConfig`, which `WhichResolver::new` consumes whole — the resolver takes the configuration rather than its fields so a new environment seam does not lengthen the -signature again. Pinning both is what lets a behavioural test drive `which` -and `command_available` over a temporary directory with a chosen extension -list; see `tests/stdlib_which_pathext_tests.rs`, which is gated to Windows -because `PATHEXT` governs resolution only there. +signature again. Pinning both is what lets a behavioural test drive `which` and +`command_available` over a temporary directory with a chosen extension list; see +`tests/stdlib_which_pathext_tests.rs`, which is gated to Windows because +`PATHEXT` governs resolution only there. That gating has a cost worth stating: CI runs `make test` on `ubuntu-latest` only, so a `#[cfg(windows)]` test does not gate a merge. Keep host-independent @@ -2825,9 +2853,9 @@ ladders — POSIX (`HOME`, then `USERPROFILE`) and Windows (those two, then the `HOMEDRIVE`/`HOMEPATH` pair, then `HOMESHARE`). Both take an injected `read_env` closure. `home_from_env` remains the sole platform-*selection* point, and each ladder is gated to its own platform plus `test` -(`posix_home_from` is `#[cfg(any(not(windows), test))]`, `windows_home_from` -is `#[cfg(any(windows, test))]`), so a release build compiles only the ladder -it uses while the `test` arm keeps both reachable from any host. +(`posix_home_from` is `#[cfg(any(not(windows), test))]`, `windows_home_from` is +`#[cfg(any(windows, test))]`), so a release build compiles only the ladder it +uses while the `test` arm keeps both reachable from any host. #### Ladder ownership and call sites @@ -2838,50 +2866,49 @@ it uses while the `test` arm keeps both reachable from any host. environment, and `Ambient` drives `home_from_env` with whatever reader the caller supplied. The composition root lives at the registration boundary — filter registration in `stdlib::path::filters` captures the process-backed - reader once, carrying the sanctioned site-level expectation — so - `path_utils` holds no process access at all. Tests inject their own reader, - covering the `Ambient` path without touching the process environment. + reader once, carrying the sanctioned site-level expectation — so `path_utils` + holds no process access at all. Tests inject their own reader, covering the + `Ambient` path without touching the process environment. #### Ladder composition rules - Keep each ladder free of platform *selection logic*, leaving that to `home_from_env`. Gating decides only whether a ladder compiles, never which - one applies — that separation is what lets the `test` arm expose both - ladders to the CI host. + one applies — that separation is what lets the `test` arm expose both ladders + to the CI host. - Gate each ladder `#[cfg(any(windows, test))]` or its inverse, and have `home_from_env` name only the ladder it selects. Compiling both unconditionally would leave the inapplicable one dead in a release build, which `-D warnings` rejects; the previous workaround — binding both as a `(posix, windows)` function-pointer pair so the unused one counted as - referenced — was an artificial dead-code anchor and has been removed. - Bounded constants used by only one ladder (`HOME_SOURCE_DRIVE_PATH` and - `HOME_SOURCE_HOMESHARE`, both Windows-only) carry the same gate as the - ladder that reads them. + referenced — was an artificial dead-code anchor and has been removed. Bounded + constants used by only one ladder (`HOME_SOURCE_DRIVE_PATH` and + `HOME_SOURCE_HOMESHARE`, both Windows-only) carry the same gate as the ladder + that reads them. - The ladders report what the environment says. An empty value is passed - through rather than treated as unset for the single-variable readings - (`HOME`, `USERPROFILE`, `HOMESHARE`), and interpreting that is - `expanduser`'s concern, not theirs. The `HOMEDRIVE`/`HOMEPATH` pair is the - exception: it counts only when both halves are non-empty, since a bare - drive or a bare relative path is not a home directory; an incomplete pair - falls through to `HOMESHARE`. + through rather than treated as unset for the single-variable readings (`HOME`, + `USERPROFILE`, `HOMESHARE`), and interpreting that is `expanduser`'s + concern, not theirs. The `HOMEDRIVE`/`HOMEPATH` pair is the exception: it + counts only when both halves are non-empty, since a bare drive or a bare + relative path is not a home directory; an incomplete pair falls through to + `HOMESHARE`. #### Home-resolution telemetry The ladders stay pure: each *returns* the resolved home paired with a bounded `&'static str` label naming the rung that supplied it, and emits nothing. -`resolve_home` is the sole telemetry boundary, emitting a -`tracing::debug!` event for every resolution, plus an additional -`tracing::debug!` failure event when no home was available, with these -fields: +`resolve_home` is the sole telemetry boundary, emitting a `tracing::debug!` +event for every resolution, plus an additional `tracing::debug!` failure event +when no home was available, with these fields: Table: Home-resolution telemetry fields. -| Field | Meaning | -| --- | --- | -| `event` | Always `stdlib.expanduser.home`, so the events are filterable. | -| `source` | The bounded label naming what supplied the home. | -| `found` | Whether a home was resolved at all. | -| `outcome` | Present only on the failure event: `home_unavailable`. | +| Field | Meaning | +| --------- | -------------------------------------------------------------- | +| `event` | Always `stdlib.expanduser.home`, so the events are filterable. | +| `source` | The bounded label naming what supplied the home. | +| `found` | Whether a home was resolved at all. | +| `outcome` | Present only on the failure event: `home_unavailable`. | `source` is drawn from a closed set and is never derived from a value: @@ -2899,14 +2926,14 @@ Table: Home-resolution telemetry fields. the series count is fixed by the code, never by the environment: `outcome` is `found` or `home_unavailable`; `source` is the same bounded label set listed above. It increments exactly once per resolution whatever the outcome, so the -counter totals resolutions rather than events — the failure path emits a -second *debug event* but no second sample. Both the success and failure cases -are pinned by tests in `src/stdlib/path/home_tests.rs`, which capture samples +counter totals resolutions rather than events — the failure path emits a second +*debug event* but no second sample. Both the success and failure cases are +pinned by tests in `src/stdlib/path/home_tests.rs`, which capture samples through a local `metrics_util` `DebuggingRecorder` rather than the global one. -The events carry no paths and no environment values: neither the resolved -home, nor a variable's contents, nor the expanded result. Adding a rung means -adding a label to the closed set above and pinning it in the ladder tests, not +The events carry no paths and no environment values: neither the resolved home, +nor a variable's contents, nor the expanded result. Adding a rung means adding +a label to the closed set above and pinning it in the ladder tests, not recording the value that distinguished it. ### Configuration discovery module layout @@ -2959,15 +2986,14 @@ cached layers to `cli::merge_with_cached_file_layers` for the full merge. Phase-level metrics are composed in `src/observability.rs` around those two operations. -Both aggregate and phase-level configuration-load timing use the same -injected elapsed-time seam: each boundary receives -`&impl monotony::MonotonicClock`. Production supplies -`monotony::StdMonotonicClock`; tests use deterministic clocks from -`monotony::test_util`, such as `FixedMonotonicClock` and +Both aggregate and phase-level configuration-load timing use the same injected +elapsed-time seam: each boundary receives `&impl monotony::MonotonicClock`. +Production supplies `monotony::StdMonotonicClock`; tests use deterministic +clocks from `monotony::test_util`, such as `FixedMonotonicClock` and `QueuedMonotonicClock`. Do not add a local `ConfigurationLoadClock` or `SystemConfigurationLoadClock`, or call `Instant::now` directly at these -boundaries. Whenever a mockable monotonic clock is introduced, use -`monotony` as the repository-approved mechanism. The dependency choice is +boundaries. Whenever a mockable monotonic clock is introduced, use `monotony` +as the repository-approved mechanism. The dependency choice is `monotony = "0.1.0"`; its public contract keeps the production clock abstraction dependency-free while its `test-util` feature provides deterministic test clocks. @@ -2985,10 +3011,10 @@ Instruments emitted by `record_config_load_metrics`: - `netsuke_config_load_duration_seconds` — a histogram recording the elapsed duration of the configuration-load phase in seconds (one sample per startup that reaches configuration resolution). Suggested operator bucket - boundaries: `0.001, 0.005, 0.01, - 0.05, 0.1, 0.5, 1.0` seconds; configuration loading is expected to complete - in single-digit milliseconds, so buckets above one second exist only to - catch pathological filesystem or environment stalls. + boundaries: `0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1.0` seconds; configuration + loading is expected to complete in single-digit milliseconds, so buckets + above one second exist only to catch pathological filesystem or environment + stalls. Naming convention: metric names use the `netsuke_` prefix and a `snake_case` unit suffix (`_total` for counters, `_seconds` for duration histograms), @@ -3257,8 +3283,8 @@ admits string keys, so such a mapping fails earlier, inside `serde_saphyr::from_str`, and surfaces as the YAML parse diagnostic rather than the `vars` one. -`register_manifest_vars` also rejects a key that collides with `env` or -`glob`, the two helper functions the manifest loader registers directly (the +`register_manifest_vars` also rejects a key that collides with `env` or `glob`, +the two helper functions the manifest loader registers directly (the `RESERVED_VAR_NAMES` constant), with the localized `manifest.vars.reserved_name` message. This check is necessary because MiniJinja keeps functions and global variables in a single namespace — @@ -3343,8 +3369,8 @@ credentials. This mirrors the redaction rule `env_var_with` already applies to `env()` lookup failures; see [Manifest `env()` reader](#manifest-env-reader). `describe_macro_metrics` and `describe_render_metrics` register each metric's -description exactly once, guarded by `std::sync::Once`. Neither is called from a -query function: `describe_macro_metrics` runs when `make_macro_fn` builds a +description exactly once, guarded by `std::sync::Once`. Neither is called from +a query function: `describe_macro_metrics` runs when `make_macro_fn` builds a macro's registration, which is setup rather than evaluation, so the guard never sits on the invocation hot path; `describe_render_metrics` runs inside `instrument_template_render`, so `render_template` names only the @@ -3452,8 +3478,8 @@ standard output is a TTY. `make_reporter(options)` selects the base reporter, `AccessibleReporter` or `IndicatifReporter` when progress is enabled and `SilentReporter` otherwise, then wraps it in `VerboseTimingReporter` when verbose mode is active. `should_force_text_task_updates` decides whether the -indicatif reporter emits textual task updates, forcing them for accessible -mode or non-TTY standard output. +indicatif reporter emits textual task updates, forcing them for accessible mode +or non-TTY standard output. `run_with_ninja_program` (in `src/runner/mod.rs`) constructs the run's `StatusReporter` through `reporter::make_reporter` after resolving output mode @@ -3468,16 +3494,15 @@ and reporter settings, then shares it via the `ExecutionContext` it passes to reporter, and it is not part of the crate's public API. - **Permitted call-sites:** only the runner boundary in `src/runner/mod.rs` may call `make_reporter` — today solely `run_with_ninja_program`. - `runner::process`, dispatch handlers, and external embedders never - construct reporters; handlers consume the already-built reporter through - the `ExecutionContext`/`&dyn StatusReporter` only. + `runner::process`, dispatch handlers, and external embedders never construct + reporters; handlers consume the already-built reporter through the + `ExecutionContext`/`&dyn StatusReporter` only. - **Composition rules:** the caller must resolve all `ReporterOptions` inputs (output mode, progress, verbose, output prefs, stdout TTY) from - CLI/environment state before calling `make_reporter`; the module performs - no such resolution itself. The reporter is composed once per run and - shared immutably. New reporter kinds or selection policies belong in this - module beside the mode-selection logic, colocated with the output-mode - policy. + CLI/environment state before calling `make_reporter`; the module performs no + such resolution itself. The reporter is composed once per run and shared + immutably. New reporter kinds or selection policies belong in this module + beside the mode-selection logic, colocated with the output-mode policy. ### Module: `runner::process::ninja_program` @@ -3558,34 +3583,33 @@ command-line argument string; it gives the redaction helpers a dedicated type to operate on instead of passing bare `String` values around. `CommandArg` carries no redaction guarantee of its own. The same type holds -both the raw arguments read from `Command::get_args` and the values returned -by the redaction helpers, and `as_str` is available on either. The invariant -is therefore a discipline on the call site, not a property of the type: -logging paths must render only what `redact_argument` or -`redact_sensitive_args` returned. `CommandLogContext::from_command` is the -one place that observes this, redacting the collected arguments before it -builds `redacted_command`. - -An argument is treated as sensitive when it is a `key=value` pair whose -trimmed key case-insensitively matches `password`, `token`, `secret`, -`api_key`, `apikey`, `auth`, or `authorization`. Matching arguments keep the -key and replace the value with `***REDACTED***`; positional arguments with no -`=` are passed through unchanged, so a path such as `secrets.yml` is not -mangled. Widen the keyword list rather than adding a second redaction path if -new sensitive arguments appear. - -The module's doc examples are marked `ignore`. `CommandArg` and the helpers -are crate-private, and the `cfg(doctest)` re-export in `runner::process::doc` -is compiled out of the library that doctests link against, so no doctest can +both the raw arguments read from `Command::get_args` and the values returned by +the redaction helpers, and `as_str` is available on either. The invariant is +therefore a discipline on the call site, not a property of the type: logging +paths must render only what `redact_argument` or `redact_sensitive_args` +returned. `CommandLogContext::from_command` is the one place that observes +this, redacting the collected arguments before it builds `redacted_command`. + +An argument is treated as sensitive when it is a `key=value` pair whose trimmed +key case-insensitively matches `password`, `token`, `secret`, `api_key`, +`apikey`, `auth`, or `authorization`. Matching arguments keep the key and +replace the value with `***REDACTED***`; positional arguments with no `=` are +passed through unchanged, so a path such as `secrets.yml` is not mangled. Widen +the keyword list rather than adding a second redaction path if new sensitive +arguments appear. + +The module's doc examples are marked `ignore`. `CommandArg` and the helpers are +crate-private, and the `cfg(doctest)` re-export in `runner::process::doc` is +compiled out of the library that doctests link against, so no doctest can import them. Behaviour is covered by the unit tests in the module instead. ### Module: `runner` target selection `BuildTargets<'a>` is a borrowing newtype over the requested target list, constructed by `BuildTargets::new` and read through `as_slice`. It exposes no -`is_empty`: the accessor existed but had no callers anywhere in the -workspace, so it was removed; call `as_slice().is_empty()` where that -question needs asking. +`is_empty`: the accessor existed but had no callers anywhere in the workspace, +so it was removed; call `as_slice().is_empty()` where that question needs +asking. ### Module: `runner::process::command_env` @@ -3741,9 +3765,9 @@ user-facing error. The boundary receives `&impl monotony::MonotonicClock`. Production passes `StdMonotonicClock`; tests pass deterministic clocks from -`monotony::test_util`. Keep elapsed-time measurement on this injected -contract; do not call `Instant::now` or introduce a configuration-specific -clock abstraction. +`monotony::test_util`. Keep elapsed-time measurement on this injected contract; +do not call `Instant::now` or introduce a configuration-specific clock +abstraction. `src/observability.rs` owns the phase-level instrumentation for the two configuration-loading boundaries in `src/main.rs`. Keep configuration loading @@ -3769,13 +3793,13 @@ labels. The `netsuke_` prefix identifies the public startup-attempt family. process-wide `metrics_util::debugging::DebuggingRecorder` after tracing starts. It retains only the bounded configuration-load series above (phase-level and startup-attempt), so unrelated workload histograms cannot accumulate samples -until shutdown. Tests must use -`metrics::with_local_recorder` with a local recorder instead. -`emit_metrics_snapshot()` drains and logs that configuration-load aggregate at -command completion. After a successful configuration merge, `finish_run` gates -it on merged `verbose`; if diagnostic-mode resolution or the full merge fails -before a merged configuration exists, it uses parsed CLI `verbose` instead. JSON -sets tracing to `OFF`, so JSON runs suppress this snapshot. +until shutdown. Tests must use `metrics::with_local_recorder` with a local +recorder instead. `emit_metrics_snapshot()` drains and logs that +configuration-load aggregate at command completion. After a successful +configuration merge, `finish_run` gates it on merged `verbose`; if +diagnostic-mode resolution or the full merge fails before a merged +configuration exists, it uses parsed CLI `verbose` instead. JSON sets tracing to +`OFF`, so JSON runs suppress this snapshot. The lifecycle is exactly-once: a `Once` guards global recorder installation and a `OnceLock` stores the snapshotter, so a second `init_metrics()` call is a diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index d2a2b0281..86f1a5a8e 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -263,19 +263,19 @@ Each entry in the `rules` list is a mapping that defines a reusable action. with space-separated, POSIX-shell-quoted input and output paths using the [`shell-quote`](https://docs.rs/shell-quote/latest/shell_quote/) crate (Sh mode) before hashing the action. Standalone `$in` and `$out` tokens are - resolved at the same boundary, while tokens inside backticks are preserved. - A scalar command is emitted unchanged. A list is lowered to brace groups - that evaluate each entry through a shell-quoted `eval` payload and are joined - by `&&`. The groups run in declaration order in one shell process and stop - at the first non-zero exit, so working directory, environment, and shell - variables carry forward. The `eval` boundary keeps an entry's inline - comments or trailing control operators from consuming the generated group - terminator. A failed entry emits a bounded action/entry marker for the - runner to include in the failure diagnostic. The resulting command must be - parsable by [shlex](https://docs.rs/shlex/latest/shlex/) (POSIX mode). An - empty command list is rejected during manifest deserialization. Plain command - strings remain shell text; authors should use structured recipes or explicit - quoting helpers for arbitrary variables. + resolved at the same boundary, while tokens inside backticks are preserved. A + scalar command is emitted unchanged. A list is lowered to brace groups that + evaluate each entry through a shell-quoted `eval` payload and are joined by + `&&`. The groups run in declaration order in one shell process and stop at + the first non-zero exit, so working directory, environment, and shell + variables carry forward. The `eval` boundary keeps an entry's inline comments + or trailing control operators from consuming the generated group terminator. + A failed entry emits a bounded action/entry marker for the runner to include + in the failure diagnostic. The resulting command must be parsable by + [shlex](https://docs.rs/shlex/latest/shlex/) (POSIX mode). An empty command + list is rejected during manifest deserialization. Plain command strings + remain shell text; authors should use structured recipes or explicit quoting + helpers for arbitrary variables. - `script`: A multi-line script declared with the YAML `|` block style. The entire block is passed to an interpreter. If the first line begins with `#!` @@ -689,12 +689,11 @@ schema defined in Section 2. They will be defined in a dedicated module, `src/ast/mod.rs`, and annotated with `#[derive(Deserialize)]` (and `Debug`) to enable automatic deserialization and easy debugging. -The authoritative live AST contract is -[src/ast/mod.rs](../src/ast/mod.rs). Fields and types marked `FUTURE` in the -snippet below are forward-looking API sketches. -`Target.description` is implemented optional discovery metadata; the remaining -forward-looking fields describe the intended schema once the roadmap tasks -land and are not assertions about the current codebase. +The authoritative live AST contract is [src/ast/mod.rs](../src/ast/mod.rs). +Fields and types marked `FUTURE` in the snippet below are forward-looking API +sketches. `Target.description` is implemented optional discovery metadata; the +remaining forward-looking fields describe the intended schema once the roadmap +tasks land and are not assertions about the current codebase. Rust @@ -833,9 +832,9 @@ selectors; command lists are executed in order, while path-like fields are interpreted only at the manifest-to-IR boundary.* `StringOrList` owns the conversions that only need to know its own shape: -`map_each` applies a function to every contained string, and `to_string_vec` -and `as_single` build on it. Path conversion deliberately does not live here. -The AST models the manifest's surface syntax, in which `sources`, `deps` and +`map_each` applies a function to every contained string, and `to_string_vec` and +`as_single` build on it. Path conversion deliberately does not live here. The +AST models the manifest's surface syntax, in which `sources`, `deps` and `order_only_deps` are plain strings; only manifest-to-IR lowering decides they name files on disk, so `src/ir/from_manifest_support.rs::to_paths` performs that interpretation at the boundary. Keeping `camino` out of `src/ast/mod.rs` @@ -923,12 +922,12 @@ parsing and template evaluation cleanly separated. ### 3.4 Design Decisions -The AST structures are implemented in `src/ast/mod.rs` and derive `Deserialize`. -Unknown fields are rejected to surface user errors early. `StringOrList` -provides a default `Empty` variant, so optional lists are trivial to represent. -The manifest version is parsed using the `semver` crate to validate that it -follows semantic versioning rules. Global and target variable maps now share the -`ManifestMap` alias: +The AST structures are implemented in `src/ast/mod.rs` and derive +`Deserialize`. Unknown fields are rejected to surface user errors early. +`StringOrList` provides a default `Empty` variant, so optional lists are +trivial to represent. The manifest version is parsed using the `semver` crate +to validate that it follows semantic versioning rules. Global and target +variable maps now share the `ManifestMap` alias: ```rust type ManifestMap = serde_json::Map; @@ -1124,15 +1123,13 @@ providing a secure bridge to the underlying system. supported. This provides globbing support not available in Ninja itself, which does not support globbing.[^3] - The metadata check that filters directories out of the results runs - through a capability opened at the pattern's literal directory prefix - (`src/` for `src/**/*.c`) rather than at an ambient root; the match walk - itself remains the `glob` crate's own, which traverses the filesystem - ambiently. + The metadata check that filters directories out of the results runs through a + capability opened at the pattern's literal directory prefix (`src/` for + `src/**/*.c`) rather than at an ambient root; the match walk itself remains + the `glob` crate's own, which traverses the filesystem ambiently. [ADR-010](adr-010-scope-glob-capability-to-literal-prefix.md) records this - decision; see the - [developer's guide](developers-guide.md#capability-scope) for the prefix - computation and symlink-handling rules. + decision; see the [developer's guide](developers-guide.md#capability-scope) + for the prefix computation and symlink-handling rules. - `python_version(requirement: &str) -> Result`: An example of a domain-specific helper function that demonstrates the extensibility of this architecture. This function would execute `python --version` or @@ -1276,9 +1273,9 @@ Implementation notes: registration boundary: `register_expanduser` in `stdlib::path::filters` captures a process-backed reader closure once and injects it into the pure resolution ladders in `path_utils`. Those ladders never touch the ambient - environment directly; they only consult whatever reader they are given. - Tests supply their own readers, exercising the ladders without reaching the - process environment. + environment directly; they only consult whatever reader they are given. Tests + supply their own readers, exercising the ladders without reaching the process + environment. - `with_suffix` removes dotted suffix segments (default `n = 1`) before appending the provided suffix. @@ -1346,13 +1343,13 @@ state, and the cache-relevant options (`all`, `canonical`, `cwd_mode`). Including the workspace switch keeps a fallback hit cached while the search was enabled from answering a resolution made with it disabled. Entries are validated once at insertion; cache reads no longer re-probe executability, -keeping the hot path lean. Because `fresh` only controls bypass behaviour, it is -stripped from the cache key so fresh lookups still repopulate the cache for +keeping the hot path lean. Because `fresh` only controls bypass behaviour, it +is stripped from the cache key so fresh lookups still repopulate the cache for subsequent calls. The fingerprint means environment changes invalidate keys without cloning large strings, and the helper remains pure because all inputs still derive from the manifest, the stdlib configuration, or the captured -environment. Callers can request a bypass with `fresh=true` when they -need to observe recent toolchain changes during a long session. +environment. Callers can request a bypass with `fresh=true` when they need to +observe recent toolchain changes during a long session. Cache capacity defaults to 64 entries, covering typical PATH sizes without overcommitting memory, and can be tuned via @@ -2012,10 +2009,10 @@ This transformation involves several steps: `ir::BuildEdge` linking the target to the action identifier and transfer the `phony` and `always` flags. `sources` are lowered into the edge's explicit input list so recipe interpolation and Ninja `$in` see only material inputs. - `deps` are lowered into a separate `implicit_deps` list, which maps to Ninja's - implicit dependency syntax (`|`) so Ninja orders and rebuilds them without - exposing them as recipe arguments; `order_only_deps` remains separate and - maps to Ninja's `||` class. + `deps` are lowered into a separate `implicit_deps` list, which maps to + Ninja's implicit dependency syntax (`|`) so Ninja orders and rebuilds them + without exposing them as recipe arguments; `order_only_deps` remains + separate and maps to Ninja's `||` class. FUTURE: @@ -2024,8 +2021,8 @@ This transformation involves several steps: remains discovery-only metadata: it is not part of the IR and does not replace the referenced rule's description for Ninja progress. Env-aware action hashing will include resolved environment bindings alongside the - recipe and file set so otherwise identical actions remain distinct when their - execution environment differs. + recipe and file set so otherwise identical actions remain distinct when + their execution environment differs. 4. **Graph Validation:** As the graph is constructed, perform validation checks. This includes ensuring that every rule referenced by a target exists in the @@ -2047,9 +2044,9 @@ This transformation involves several steps: and visitation map. Keys are cloned from the `targets` map so traversal leaves the input graph untouched. Missing dependencies encountered during traversal are logged, collected, and returned alongside any cycle to aid - diagnostics. +diagnostics. -### 5.4 Ninja File Synthesis (`ninja_gen/mod.rs`) +### 5.4 Ninja file synthesis (`src/ninja_gen.rs`) The final step is to synthesize the `build.ninja` file from the `BuildGraph` IR. This process is a straightforward, mechanical translation from the IR data @@ -2122,14 +2119,14 @@ structures to the Ninja file syntax. no Ninja edge produces sidecar content. The generated result is a bundle, not merely a string: generation is an - effect-free query that returns the main Ninja text and its - `.netsuke/dyndep` sidecars. Each runner command then materializes those - sidecars through an injected effective-working-directory capability before - it writes or runs the main file. The main file declares - `ninja_required_version = 1.10` only when it contains such staged serial - ordering. `.netsuke/serial` and `.netsuke/dyndep` are reserved for generated - state. `serial` applies only to direct implicit dependencies; it does not - delay an independently reachable node elsewhere in the graph. + effect-free query that returns the main Ninja text and its `.netsuke/dyndep` + sidecars. Each runner command then materializes those sidecars through an + injected effective-working-directory capability before it writes or runs the + main file. The main file declares `ninja_required_version = 1.10` only when + it contains such staged serial ordering. `.netsuke/serial` and + `.netsuke/dyndep` are reserved for generated state. `serial` applies only to + direct implicit dependencies; it does not delay an independently reachable + node elsewhere in the graph. Figure: Runner-owned serial dyndep bundle generation and execution. @@ -2166,11 +2163,11 @@ sequenceDiagram The runner holds a capability-scoped exclusive lease on the dyndep directory from sidecar materialization through Ninja consumption or generated-output consumption. While the lease is held, stale `.tmp` files are removed and -retention preserves the current bundle plus at most 32 obsolete `.dd` files -and 1 MiB of obsolete `.dd` bytes. `build` and `generate` prune after +retention preserves the current bundle plus at most 32 obsolete `.dd` files and +1 MiB of obsolete `.dd` bytes. `build` and `generate` prune after materialization; `clean` prunes only after successful `ninja -t clean` and not -on failure. Sidecars remain immutable and content-addressed. Consequently, -an older arbitrary `generate --output` manifest may lose its sidecars after a +on failure. Sidecars remain immutable and content-addressed. Consequently, an +older arbitrary `generate --output` manifest may lose its sidecars after a later command and must be regenerated. See [ADR-012](adr-012-bound-dyndep-sidecar-retention.md) for this policy. @@ -2209,9 +2206,9 @@ representation portable. optional key-value pairs or flags, keeping the generator easy to scan. - Integration tests snapshot the generated Ninja file with `insta` and execute the Ninja binary to validate structure and no-op behaviour. - Serial-ordering tests additionally use real Ninja to prove declaration - order, failure short-circuiting, shared-work reuse, and unrelated-branch - concurrency. [ADR-011](adr-011-use-ninja-dyndep-for-serial-dependency-ordering.md) + Serial-ordering tests additionally use real Ninja to prove declaration order, + failure short-circuiting, shared-work reuse, and unrelated-branch concurrency. + [ADR-011](adr-011-use-ninja-dyndep-for-serial-dependency-ordering.md) records why staged dyndep is used instead of order-only gates, pools, or recursive Ninja invocations. @@ -2272,10 +2269,10 @@ The command construction follows this pattern: 4. The request's `CommandEnv` is applied. Overrides are **additive**: each pair is written with `Command::env` and nothing is cleared, because Ninja needs the ambient environment to function and `env_clear` would make a test - environment diverge from production in ways unrelated to what the test - pins. `CommandEnv::inherit()` sets nothing at all, so production spawns - exactly the environment Netsuke itself received. This is the only supported - way to vary a child's environment: Netsuke never mutates its own process + environment diverge from production in ways unrelated to what the test pins. + `CommandEnv::inherit()` sets nothing at all, so production spawns exactly + the environment Netsuke itself received. This is the only supported way to + vary a child's environment: Netsuke never mutates its own process environment to influence a subprocess. 5. Standard I/O streams (`stdin`, `stdout`, `stderr`) are configured using @@ -2284,20 +2281,19 @@ The command construction follows this pattern: to the user's console, potentially with additional formatting or status updates from Netsuke itself. -Stream routing follows the `stderr_mode` policy carried by the request. -Outside JSON mode (`StderrMode::Forward`) the child's standard output and -error are piped and forwarded to Netsuke's own streams concurrently: stdout is -drained on the main thread while a separate thread forwards stderr, so users -see Ninja's messages as it produces them. No relative ordering is guaranteed -between the two streams. In JSON diagnostics mode the request -carries `StderrMode::Suppress`, which drains both streams to `io::sink()`: -stdout carries only the versioned result document and stderr only the -diagnostic document, keeping both machine-readable. The runner derives the -policy from the CLI's JSON setting via -`StderrMode::from_json_enabled(cli.json)`; the process layer consumes the -request's `stderr_mode` field and never re-derives it. A non-zero exit status -or failure to spawn the process is reported as an `io::Error` for the CLI to -surface. +Stream routing follows the `stderr_mode` policy carried by the request. Outside +JSON mode (`StderrMode::Forward`) the child's standard output and error are +piped and forwarded to Netsuke's own streams concurrently: stdout is drained on +the main thread while a separate thread forwards stderr, so users see Ninja's +messages as it produces them. No relative ordering is guaranteed between the +two streams. In JSON diagnostics mode the request carries +`StderrMode::Suppress`, which drains both streams to `io::sink()`: stdout +carries only the versioned result document and stderr only the diagnostic +document, keeping both machine-readable. The runner derives the policy from the +CLI's JSON setting via `StderrMode::from_json_enabled(cli.json)`; the process +layer consumes the request's `stderr_mode` field and never re-derives it. A +non-zero exit status or failure to spawn the process is reported as an +`io::Error` for the CLI to surface. The `ninja_subprocess` span and its spawn and exit events carry `env_override_count` and `path_overridden`, derived from the prepared @@ -2355,11 +2351,11 @@ error-prone task that requires specialized knowledge. Netsuke's design makes identified path substitution safe by default. Netsuke quotes the `ins`/`outs` path values before action hashing and Ninja synthesis; arbitrary Jinja values and handwritten shell fragments remain the manifest -author's responsibility. By integrating `shell-quote` into IR command -lowering, before action hashing and Ninja file synthesis, -Netsuke protects users from a common and dangerous class of errors by default. -This approach embodies a deeper form of user-friendliness: one that anticipates -and mitigates risks on the user's behalf. +author's responsibility. By integrating `shell-quote` into IR command lowering, +before action hashing and Ninja file synthesis, Netsuke protects users from a +common and dangerous class of errors by default. This approach embodies a +deeper form of user-friendliness: one that anticipates and mitigates risks on +the user's behalf. ## Section 7: A Framework for Friendly and Actionable Error Reporting @@ -2760,21 +2756,20 @@ then passes the cached layers to `cli::merge_with_cached_file_layers` for the full merge. Those query functions do not install a recorder or own configuration-load metrics. `src/observability.rs` owns the phase recorder and bounded phase/outcome vocabulary, while `src/config_load.rs` owns the -startup-attempt series. -The application installs an in-process `DebuggingRecorder`; it does not open a -metrics listener as a side effect of a command invocation. +startup-attempt series. The application installs an in-process +`DebuggingRecorder`; it does not open a metrics listener as a side effect of a +command invocation. Metric labels are closed sets: phase-level series use `diag_mode` or `merge`, and both phase-level and startup-attempt counters use `success` or `failure`. Recorder-snapshot tests assert these labels through the same closed phase and -outcome vocabulary. -The startup-attempt duration has no labels. Paths, configuration values, and -formatted source errors remain in user-facing diagnostics where appropriate; -they must not become metric labels. When a run reaches configuration loading, -the recorder emits its aggregate snapshot only for verbose runs, after command -completion or a configuration-load exit. -The full metric names, phase boundaries, and test-recorder rules are documented -in the configuration-load observability section of the +outcome vocabulary. The startup-attempt duration has no labels. Paths, +configuration values, and formatted source errors remain in user-facing +diagnostics where appropriate; they must not become metric labels. When a run +reaches configuration loading, the recorder emits its aggregate snapshot only +for verbose runs, after command completion or a configuration-load exit. The +full metric names, phase boundaries, and test-recorder rules are documented in +the configuration-load observability section of the [developer's guide](developers-guide.md). `src/locale_catalogues.rs` is the authoritative registry of shipped catalogues. @@ -2914,74 +2909,65 @@ when the variable is unset, empty, or non-UTF-8. ### 8.4.1 Configuration File Discovery **Figure: Configuration Discovery and Merge Flow** — This diagram illustrates -how Netsuke discovers and merges configuration from multiple sources. The flow -begins with CLI parsing, checks for project and user configuration files, and -applies layered merging with increasing precedence: defaults < discovered -config files < environment variables < CLI flags. The final merged -configuration determines runtime behaviour. Accessible text description follows -below. +the cached single-pass configuration flow. Netsuke parses the CLI and directory +option, then either loads an explicit selection or runs OrthoConfig discovery +rooted by that directory, with an optional project-scope fallback. The +`DiscoveryOutcome` retains layers and diagnostics for replay before +`into_layers()` hands the cached layers to the full merge via +`merge_with_cached_file_layers()`. Accessible text description follows below. ```mermaid flowchart LR - A[Start Netsuke] --> B[Parse CLI into Cli] - B --> C{Project config exists?} - - C -->|Yes| D[Load project config via ConfigDiscovery] - C -->|No| E[No project config layer] - - D --> F{User config exists?} - E --> F - - F -->|Yes| G[Load user config via ConfigDiscovery] - F -->|No| H[No user config layer] - - G --> I[Merge defaults + project + user] - H --> I[Merge defaults + project + user] - - I --> J[Apply environment NETSUKE_... overrides] - J --> K[Apply CLI flag overrides] - K --> L[Resolved merged config] - L --> M[Run Netsuke with final behaviour] + A[Start Netsuke] --> B["Parse CLI and --directory"] + B --> C[Resolve JSON mode and DiscoveryOutcome] + C --> D{Explicit config selector?} + + D -->|Yes| E[Load explicit config] + D -->|No| F["Run OrthoConfig discovery rooted by --directory"] + + F --> G{Project-scope layer already included?} + G -->|Yes| H[Retain discovered layers] + G -->|No| I[Append project-scope file only when omitted] + I --> H + E --> H + + H --> J[Retain DiscoveryOutcome layers, errors, and diagnostics] + J --> K[Configure tracing] + K --> L["emit_diagnostics()"] + L --> M["into_layers()"] + M --> N["merge_with_cached_file_layers()"] + N --> O[Run Netsuke with final behaviour] ``` 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`. -`discover_file_layers(...)` performs one discovery pass, applying the -`-C/--directory` flag as the project-discovery root, and returns a -`DiscoveryOutcome`. Its `DiscoveredLayers` owns the discovered layers, -discovery errors and bounded deferred diagnostics. +`discover_file_layers(...)` performs one overall discovery pass, applying the +`-C/--directory` flag as the project-discovery root. Its automatic path first +runs the OrthoConfig scan, then loads the project-scope file as a second pass +when the scan did not include it. The function returns a `DiscoveryOutcome`; its +`DiscoveredLayers` owns the discovered layers, discovery errors, and bounded +deferred diagnostics. Diagnostic-mode resolution uses - -`resolve_json_and_layers_outcome_with_env(...)` to resolve JSON from those -discovered layers and retain the outcome for the startup boundary. -It -returns deferred diagnostics instead of emitting tracing while resolving. -`DiscoveryOutcome::emit_diagnostics()` replays the retained diagnostics after -tracing is configured without repeating environment or filesystem access. -`collect_file_layers_with_trace_and_env_source(...)` performs the underlying -discovery scan and retains bounded project-scope trace metadata. -`DiscoveryOutcome::into_layers()` transfers the same discovered layers to -`merge_with_cached_file_layers(...)`, which consumes them for the full merge -and prevents a second discovery pass. The standalone -`merge_with_config_and_env(...)` path performs discovery, emits diagnostics -and delegates to `merge_with_cached_file_layers(...)`. - -Because OrthoConfig 0.9.0 exposes only an owned `MergeLayer::into_value()` -accessor, discovery derives its JSON preference while transferring each owned -file value into the cached file layer. This preserves the cached values for -the merge without cloning complete layers or JSON values. The startup -composition root creates `ConfigStdEnvProvider`; `ConfigurationLoadContext` -carries an injected `ConfigEnvProvider` through both early resolution and the -cached merge. +`resolve_json_and_layers_outcome_with_env(...)`, which returns +`(OrthoResult, DiscoveryOutcome)` without emitting diagnostics. The +composition boundary calls `DiscoveryOutcome::emit_diagnostics()` after tracing +is configured, replaying the retained diagnostics without repeating environment +or filesystem access. `collect_file_layers_with_trace_and_env_source(...)` +performs the underlying discovery scan and retains bounded project-scope trace +metadata. `DiscoveryOutcome::into_layers()` transfers the same discovered +layers to `merge_with_cached_file_layers(...)`, which consumes them for the +full merge and prevents a second discovery pass. The standalone +`merge_with_config_and_env(...)` path performs discovery, emits diagnostics and +delegates to `merge_with_cached_file_layers(...)`. Deferred bounded discovery diagnostics are retained only for replay after the startup tracing boundary is configured. They do not contain raw paths or file names and replay does not repeat environment or filesystem access. Discovery and configuration errors remain separate: diagnostic-mode resolution returns -its discovery or JSON-validation error, while the full merge caller handles -the retained discovery errors together with merge errors. +its discovery or JSON-validation error, while the full merge caller handles the +retained discovery errors together with merge errors. **Figure: Explicit Config Selector Resolution** — This diagram shows how Netsuke chooses the configuration file before automatic discovery. Netsuke @@ -3023,12 +3009,12 @@ flowchart TD #### Discovery scopes and layered merging -Configuration discovery searches multiple scopes and composes all discovered -files into layers using OrthoConfig's `compose_layers()`. The single -`discover_file_layers` pass retains the resulting layers and ensures that -project-scope layers have higher precedence than user- and system-scope -layers. After file layers are merged, environment variables and CLI arguments -override the merged result, ensuring explicit user intent always wins. +Automatic configuration discovery first calls OrthoConfig's `compose_layers()` +to retain its primary automatic match. If that match does not include the +project-scope file, a second pass loads that file directly and appends its +layers. Thus, `compose_layers()` does not merge every scope, and it does not by +itself establish project/user/system precedence. Environment variables and CLI +arguments then override the file layers. 1. **Explicit override**: `--config ` and `NETSUKE_CONFIG` are evaluated in that precedence order before discovery. These explicit selectors bypass @@ -3057,15 +3043,10 @@ override the merged result, ensuring explicit user intent always wins. Directory system config directories, defaults to `/etc/xdg/netsuke/config.toml` when `XDG_CONFIG_DIRS` is unset) -**Scope precedence**: When configuration files exist in multiple scopes, -OrthoConfig's discovery order gives **project scope highest precedence**, then -user scope, then system scope (project > user > system). This matches user -expectations: project-local configuration should override user-global defaults, -which in turn override system-wide settings. The `-C/--directory` flag anchors -project-scope discovery to the specified directory while leaving user-scope and -system-scope lookup unchanged, ensuring user-global and system-wide -configuration remain available even when operating on a project in a different -directory. +The primary scanner's platform-specific search determines which automatic file +is retained first. The `-C/--directory` flag anchors the project-scope fallback +to the specified directory while leaving the scanner's user- and system-scope +lookups unchanged. **Layer merge precedence** (lowest to highest): @@ -3091,29 +3072,26 @@ manual flag repetition. environment variables via Figment and CLI overrides extracted from `ArgMatches`. - The `config_discovery()` function uses OrthoConfig's builder API with the - application name, environment selector, and an environment source selected at - the CLI composition root. Ambient runs use `ProcessEnv`; injected runs use a - closed `MapEnv` projected from Netsuke's environment port, preventing tests - from falling through to host directories. -- A missing optional candidate means no configuration layer and therefore - built-in defaults. A candidate that exists but cannot load is retained as an - error when no candidate succeeds, so malformed configuration and a missing - `extends` parent are never mistaken for absence. + application name, injected discovery environment, and optional project-root + anchor, 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. The v0.9.0 adapter projects only - the documented discovery keys into OrthoConfig, while `EnvironmentLayer` - retains the complete `NETSUKE_*` value merge boundary. -- Configuration files use TOML. OrthoConfig's optional YAML provider remains - disabled; Netsukefile YAML continues to be parsed by the separate - `serde-saphyr` manifest boundary. + resolution take an injected `&impl ConfigEnvProvider` in the public + `*_with_env` entry points. Production wrappers supply `ConfigStdEnvProvider`; + tests supply an in-memory `ConfigEnvProvider` without mutating the process + environment. Startup obtains a `DiscoveryOutcome` from + `resolve_json_and_layers_outcome_with_env`, emits its deferred diagnostics, + then passes `into_layers()` to `merge_with_cached_file_layers`, so file + discovery and loading happen once. 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. - Explicit config selection is handled outside OrthoConfig's built-in discovery override surface so Netsuke keeps its custom project-over-user precedence for - automatic discovery. If an explicit selector is set, the - selected file is loaded directly and bypasses discovery, but still - participates in the normal precedence ladder: defaults < file < environment < - CLI. + automatic discovery. If an explicit selector is set, the selected file is + loaded directly and bypasses discovery, but still participates in the normal + precedence ladder: defaults < file < environment < CLI. - Relative paths passed to `--config` are resolved against the process current working directory, not the `-C/--directory` anchor. This keeps config-file selection aligned with normal shell path semantics while `-C` continues to @@ -3125,9 +3103,9 @@ The CLI definition doubles as the source for user documentation. Release automation now calls `cargo-orthohelp` explicitly through `scripts/generate-release-help.sh`; ordinary Cargo builds do not supply the release manual page or PowerShell help. `cargo-orthohelp` remains the release -source for those artefacts. Separately, `build.rs` generates Bash, Elvish, Fish, -PowerShell, and Zsh completion assets from `Cli::command()`. The completion -files are staged as portable shell-completion sidecars under +source for those artefacts. Separately, `build.rs` generates Bash, Elvish, +Fish, PowerShell, and Zsh completion assets from `Cli::command()`. The +completion files are staged as portable shell-completion sidecars under `completions//` in release archives. The build script also performs the localization key audit against Fluent bundles. @@ -3161,22 +3139,27 @@ resulting `.deb` and `.rpm` archives both declare a runtime dependency on `leynos/shared-actions`; Windows staging also carries the PowerShell help files as release artefacts alongside the MSI package. Every standalone release archive also carries the generated shell completion sidecars under -`completions//`. The composite shells out to a -Cyclopts-driven script that reads the `.github/release-staging.toml` -configuration (Tom's Obvious, Minimal Language (TOML)), merges the `[common]` -configuration with the target-specific overrides, and copies the configured -artefacts into a fresh `dist/{bin}_{platform}_{arch}` directory. It installs -Astral's Python package manager (uv) with `astral-sh/setup-uv`, double-checks -the tool is present, and only then launches the Python entry point so workflows -stay declarative. The helper writes SHA-256 sums for every staged file and -exports a JSON map of the artefact outputs, allowing the workflow to hydrate -downstream steps without hard-coded path logic. Figure 8.1 summarizes the -configuration entities, including optional keys reserved for templated -directories and explicit artefact destinations that the helper can adopt -without breaking compatibility. +`completions//`. The composite shells out to a Cyclopts-driven script +that reads the `.github/release-staging.toml` configuration (Tom's Obvious, +Minimal Language (TOML)), merges the `[common]` configuration with the +target-specific overrides, and copies the configured artefacts into a fresh +`dist/{bin}_{platform}_{arch}` directory. It installs Astral's Python package +manager (uv) with `astral-sh/setup-uv`, double-checks the tool is present, and +only then launches the Python entry point so workflows stay declarative. The +helper writes SHA-256 sums for every staged file and exports a JSON map of the +artefact outputs, allowing the workflow to hydrate downstream steps without +hard-coded path logic. Figure 8.1 summarizes the configuration entities, +including optional keys reserved for templated directories and explicit +artefact destinations that the helper can adopt without breaking compatibility. Figure 8.1: Entity relationship for the staging configuration schema. +For screen readers: `COMMON` defines shared release-staging settings and +contains common `ArtefactConfig` entries. `TARGETS` defines platform-specific +settings and contains target-specific `ArtefactConfig` entries. Each +`ArtefactConfig` defines an artefact source, required status, and output or +destination settings. + ```mermaid %% Figure 8.1: Entity relationship for the staging configuration schema. erDiagram @@ -3277,7 +3260,7 @@ goal. 3. Implement the AST-to-IR transformation logic, including basic validation like checking for rule existence. - 4. Implement the IR-to-Ninja file generator (`ninja_gen/mod.rs`). + 4. Implement the IR-to-Ninja file generator (`src/ninja_gen.rs`). 5. Implement the `std::process::Command` logic to invoke `ninja`. diff --git a/docs/users-guide.md b/docs/users-guide.md index 44da963a6..5e2eeca51 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -65,13 +65,12 @@ licence. Ninja must be installed separately when using the macOS or Windows installer. The Windows MSI installs to `C:\Program Files\netsuke` and does not update `PATH`. -The MSI installer supports pre-release SemVer versions such as -`0.1.0-beta2`: the pre-release suffix cannot be represented in an MSI -product version, so the installer carries the numeric release triple -(`0.1.0`) while the full version remains in the package and release names. -Because successive pre-releases share that numeric version, installing a -later pre-release MSI replaces the existing installation for that version -series rather than installing alongside it. +The MSI installer supports pre-release SemVer versions such as `0.1.0-beta2`: +the pre-release suffix cannot be represented in an MSI product version, so the +installer carries the numeric release triple (`0.1.0`) while the full version +remains in the package and release names. Because successive pre-releases share +that numeric version, installing a later pre-release MSI replaces the existing +installation for that version series rather than installing alongside it. SHA-256 checksum files accompany standalone binaries and staged help, completion, and licence files. Installer packages do not have checksum sidecars @@ -83,8 +82,8 @@ sidecars under `completions//` for Bash, Elvish, Fish, PowerShell, and Zsh. These files are portable and separate from the executable and installer payloads. To use one, extract the matching archive and copy the file for the chosen shell into that shell's normal completion directory, or load it through -the shell's documented completion mechanism. The package installation -commands above do not install completion files; completion directory names and +the shell's documented completion mechanism. The package installation commands +above do not install completion files; completion directory names and activation steps vary by shell and platform. Install the current source checkout with Cargo. The clone supplies both the @@ -308,28 +307,27 @@ Rules may also provide `description`, text used for Ninja's progress display. Targets and actions may also provide `description`, but with a different purpose: a target or action description is discovery metadata surfaced by `netsuke help targets` (see -[Generate and inspect artefacts](#generate-and-inspect-artefacts)). It does -not affect Ninja progress output, which stays driven by the referenced rule's +[Generate and inspect artefacts](#generate-and-inspect-artefacts)). It does not +affect Ninja progress output, which stays driven by the referenced rule's `description`. A `command` list runs its entries in declaration order and stops at the first -non-zero exit, so entries share the fail-fast behaviour of a handwritten -`&&` chain. The command field is a `StringOrList`: a scalar remains one shell +non-zero exit, so entries share the fail-fast behaviour of a handwritten `&&` +chain. The command field is a `StringOrList`: a scalar remains one shell command, while a YAML sequence is rendered and lowered one entry at a time. This applies equally to rules, direct targets, and actions. Each entry sees the same Jinja context, including `{{ ins }}` and `{{ outs }}`; those two -placeholders are resolved later to the concrete target's shell-quoted input -and output paths. An empty command list is rejected when the manifest is -parsed. +placeholders are resolved later to the concrete target's shell-quoted input and +output paths. An empty command list is rejected when the manifest is parsed. At execution time, each list entry is evaluated inside its own brace group and the groups are joined with `&&`. The entry is passed to `eval` as a shell-quoted payload, so an inline `#` comment or a trailing control operator -such as `&` cannot consume the generated group's closing boundary. Brace -groups run in the current shell rather than a subshell: a changed working -directory, environment assignment, or shell variable can therefore be used by -later entries. A failed entry stops the chain, and the diagnostic identifies -the generated action and one-based list-entry positions, for example +such as `&` cannot consume the generated group's closing boundary. Brace groups +run in the current shell rather than a subshell: a changed working directory, +environment assignment, or shell variable can therefore be used by later +entries. A failed entry stops the chain, and the diagnostic identifies the +generated action and one-based list-entry positions, for example `netsuke command-list failure: action HASH, entry 2`. @@ -368,8 +366,8 @@ targets: ``` Prefer a `command` list for a short, ordered sequence of distinct commands. -Prefer `script` when the logic needs multi-line structure or shell -constructs such as loops, conditionals, or variable assignment. +Prefer `script` when the logic needs multi-line structure or shell constructs +such as loops, conditionals, or variable assignment. The v0.1.0-beta2 `script` implementation invokes `/bin/sh -e`; it is not currently a portable PowerShell abstraction. Prefer `command` or @@ -396,9 +394,9 @@ A target supports these fields: - `phony`: marks a logical target that does not represent a file. - `always`: forces the recipe to run whenever the target is requested. - `description`: an optional human-readable summary of the public operation - the target performs. It is discovery metadata shown by `netsuke help - targets`; it never replaces a referenced rule's `description` in Ninja - progress output. + the target performs. It is discovery metadata shown by + `netsuke help targets`; it never replaces a referenced rule's `description` + in Ninja progress output. `name`, `sources`, `deps`, and `order_only_deps` accept either one string or a list of strings. @@ -462,11 +460,10 @@ same ordered entry point. Netsuke uses Ninja's `dyndep` support for serial lists with two or more dependencies, and generated builds containing staged serial ordering require -Ninja 1.10 or newer. -`netsuke generate`, `build`, and `clean` materialize the generated sidecars -under `.netsuke/dyndep` in the effective working directory before writing or -invoking the generated Ninja file. The sidecars are immutable and -content-addressed. Each sidecar-capable command retains the current bundle, +Ninja 1.10 or newer. `netsuke generate`, `build`, and `clean` materialize the +generated sidecars under `.netsuke/dyndep` in the effective working directory +before writing or invoking the generated Ninja file. The sidecars are immutable +and content-addressed. Each sidecar-capable command retains the current bundle, then at most 32 obsolete `.dd` files and 1 MiB of obsolete `.dd` bytes. Stale `.tmp` files are removed while the exclusive sidecar-directory lease is held. `build` and `generate` prune after materialization; `clean` prunes only after @@ -474,10 +471,10 @@ successful `ninja -t clean`, and does not prune when clean fails. An older arbitrary manifest written with `generate --output` may lose its referenced sidecars after a later command. Regenerate that manifest before -using it if retention has removed any of its sidecars. -The paths `.netsuke/serial` and `.netsuke/dyndep` must not occur in any user -graph path, including outputs, inputs, implicit dependencies, and order-only -dependencies; they are reserved for Netsuke-generated gates and sidecars. +using it if retention has removed any of its sidecars. The paths +`.netsuke/serial` and `.netsuke/dyndep` must not occur in any user graph path, +including outputs, inputs, implicit dependencies, and order-only dependencies; +they are reserved for Netsuke-generated gates and sidecars. When migrating an existing manifest, see the [v0.1.0 migration guide](v0-1-0-migration-guide.md#opting-into-serial-dependency-ordering) @@ -539,6 +536,20 @@ Matching is case-sensitive. `*` and `?` do not cross directory separators; use are returned. The [quick-start guide](quickstart.md) shows a complete runnable example. +Patterns may be absolute or relative to the working directory, including +parent-relative patterns such as `glob('../shared/*.h')`. Expansion is scoped +to the pattern's longest literal directory prefix — the text up to the first +`*`, `?`, `[` or `{`, trimmed back to the last separator, so `src/` for +`src/**/*.c`. If that prefix does not exist, or names something that is not a +directory, the call returns an empty list rather than failing. A symbolic-link +literal prefix, such as `src/link/*.c`, cannot establish the capability and +causes expansion to fail. A match is skipped rather than reported as an error +when the metadata lookup cannot resolve a symbolic link — the match itself or a +directory reached on the way to it — because it is unreadable within the +prefix, dangling, or resolves outside that prefix. A cyclic symbolic link is +reported as an error rather than skipped, since it describes a broken tree +rather than a missing file. + ### Define reusable macros Macros return rendered text and can accept default arguments: @@ -570,22 +581,21 @@ defaults: is absent. The same helper is also available as a filter. On Windows, a name without an extension is matched against the effective -`PATHEXT`, the same list the shell uses — so `which('cargo')` finds -`cargo.exe` provided `.exe` is among those entries. A custom `PATHEXT` may -legitimately omit it, in which case it is not a candidate. +`PATHEXT`, the same list the shell uses — so `which('cargo')` finds `cargo.exe` +provided `.exe` is among those entries. A custom `PATHEXT` may legitimately +omit it, in which case it is not a candidate. `PATHEXT` falls back to the built-in list only when it is unset or when no -entry survives normalization — that is, every entry is empty or whitespace. -Any other value is used as given, however unusual. The built-in list, in -order: +entry survives normalization — that is, every entry is empty or whitespace. Any +other value is used as given, however unusual. The built-in list, in order: -`.com`, `.exe`, `.bat`, `.cmd`, `.vbs`, `.vbe`, `.js`, `.jse`, `.wsf`, -`.wsh`, `.msc` +`.com`, `.exe`, `.bat`, `.cmd`, `.vbs`, `.vbe`, `.js`, `.jse`, `.wsf`, `.wsh`, +`.msc` The fallback exists because an empty effective list would match nothing and -report every command missing. Entries are matched case-insensitively and -tried in the order the list gives them. A name that already carries an -extension is used as written. +report every command missing. Entries are matched case-insensitively and tried +in the order the list gives them. A name that already carries an extension is +used as written. `command_available(name, **kwargs)` returns a boolean and is better for complementary branches: @@ -762,9 +772,9 @@ environment. `path | expanduser` expands a leading `~` against the home directory, resolved from `HOME` then `USERPROFILE` on POSIX hosts, and from `HOME`, `USERPROFILE`, the `HOMEDRIVE`/`HOMEPATH` pair, then `HOMESHARE` on Windows. The Windows pair counts only when both halves are non-empty; an -incomplete pair falls through to `HOMESHARE`. Named-user forms such as -`~alice` are unsupported, and when no home directory resolves at all, the -filter fails rather than passing the `~` through silently. +incomplete pair falls through to `HOMESHARE`. Named-user forms such as `~alice` +are unsupported, and when no home directory resolves at all, the filter fails +rather than passing the `~` through silently. ## Use the command-line interface @@ -918,18 +928,18 @@ textual outline and a `