Cache config file layer discovery (#319)

.github/dependabot.yml

@@ -43,6 +43,7 @@ updates:
     directories:
       - "/"
       - "/test_support"
+      - "/tests/ui/cli_configuration_pass"
     open-pull-requests-limit: 5
     labels:
       - "dependencies"

benches/config_load_cached_merge.rs

@@ -24,6 +24,10 @@ impl ConfigEnvProvider for BenchmarkEnv {
     fn get(&self, _key: &str) -> Option<OsString> {
         None
     }
+
+    fn entries(&self) -> Vec<(OsString, OsString)> {
+        Vec::new()
+    }
 }
 
 /// Create a large nested configuration payload with valid build targets.

docs/v0-1-0-migration-guide.md

@@ -2,23 +2,26 @@
 
 This guide signposts the v0.1.0 beta additions: the injectable child
 environment (`CommandEnv`), the named Ninja request types, narrow process
-options (`NinjaProcessOptions`), and target/action discovery through
-`description` and `netsuke help targets`. Existing manifests remain compatible,
-and callers of the unchanged convenience wrappers compile unchanged. Rust
-callers that construct `Target` with a struct literal must add the new
+options (`NinjaProcessOptions`), target/action discovery through `description`
+and `netsuke help targets`, and cached configuration discovery. Existing
+manifests remain compatible, and callers of the unchanged convenience wrappers
+compile unchanged. The cached configuration discovery API is a breaking change
+for callers of the unstable Rust API; ordinary CLI users need no action.
+
+Rust callers that construct `Target` with a struct literal must add the new
 `description` field (set it to `None` or `Some(...)`); deserialized manifests
 remain compatible. Callers constructing `NinjaBuildRequest` or
 `NinjaToolRequest` must replace `cli: &cli` with `options: &options`; every
 other addition is opt-in.
 
 ## Netsuke is a build tool, not a library
 
-Netsuke is intended to be used as a command-line build tool. The only
-surfaces it commits to are the Netsukefile manifest format and the graph
-export. Everything else — the Rust API described below included — is
-private in intent and unstable in practice: it may change shape, or vanish,
-in any release of the beta series without a deprecation period. Reliance
-on it is conditional on tracking those changes.
+Netsuke is intended to be used as a command-line build tool. The only surfaces
+it commits to are the Netsukefile manifest format and the graph export.
+Everything else — the Rust API described below included — is private in intent
+and unstable in practice: it may change shape, or vanish, in any release of the
+beta series without a deprecation period. Reliance on it is conditional on
+tracking those changes.
 
 ## At-a-glance changes
 
@@ -30,11 +33,11 @@ Table: documented v0.1.0 additions, including `netsuke help targets`, and their
 | Convenience wrappers | Unchanged. `run_ninja` and `run_ninja_tool` behave exactly as before, inheriting the process environment. | [Users' guide](users-guide.md) |
 | Child environment | New opt-in `netsuke::runner::CommandEnv` carries additive variable overrides and an injected `PATH` for Ninja child processes. | [Users' guide](users-guide.md) |
 | Request types | New `netsuke::runner::NinjaBuildRequest` and `netsuke::runner::NinjaToolRequest` name the program, `NinjaProcessOptions`, build file, targets or tool, a child environment, and a required `stderr_mode: StderrMode` policy for the `*_with` run functions. | [Users' guide](users-guide.md) |
+| Cached CLI configuration API | Breaking for callers of the unstable Rust API: use the opt-in cached discovery flow with `ConfigEnvProvider`; `ConfigStdEnvProvider` supplies process-backed access. | [Users' guide](users-guide.md) |
 | Glob expansion | Parent-relative patterns such as `glob('../shared/*.h')` now expand. Metadata checks use a capability rooted at the pattern's longest literal directory prefix; missing or non-directory prefixes return no matches, and unresolvable symlink matches are skipped. | [Users' guide](users-guide.md) and [ADR-010](adr-010-scope-glob-capability-to-literal-prefix.md) |
 | Command recipes | Existing scalar `command` recipes are unchanged. New YAML command lists are opt-in and run in declaration order with fail-fast semantics. | [Rules and recipes](users-guide.md#rules-and-recipes) |
 | Manifest discovery | Optional target/action `description` values are shown by the new `netsuke help targets` command. Manifests without them and existing build output are unchanged. | [Users' guide](users-guide.md) |
 | Serial dependencies | New opt-in `dependency_order: serial` runs an action or target's direct `deps` list in declaration order. | [Serial dependency ordering](users-guide.md#run-direct-dependencies-serially) |
-| Cached configuration discovery | New opt-in `netsuke::cli::resolve_json_and_layers_outcome_with_env` and `netsuke::cli::merge_with_cached_file_layers` APIs let callers reuse discovered file layers. | [Users' guide](users-guide.md) |
 
 ## Nothing to change for existing callers
 
@@ -47,8 +50,8 @@ directly must pass `options: &options` and supply the required
 
 ## Opting into ordered command lists
 
-Existing scalar `command` recipes remain valid, so no migration is required.
-To run a short sequence of commands in declaration order, change a recipe to a
+Existing scalar `command` recipes remain valid, so no migration is required. To
+run a short sequence of commands in declaration order, change a recipe to a
 non-empty YAML list. The entries run in one shell process and stop at the first
 non-zero exit. See [Rules and recipes](users-guide.md#rules-and-recipes) for
 the syntax, shell semantics, and examples.
@@ -75,30 +78,42 @@ CLI working directory to UTF-8 and returns `io::ErrorKind::InvalidData` when
 it cannot. Worked examples live in the users' guide's
 "Drive Ninja with an explicit environment" section.
 
-## Reusing cached configuration discovery
+## Cached CLI configuration API
+
+Callers of the unstable Rust configuration API must update to the cached
+configuration discovery flow. `ConfigEnvProvider` is the public environment
+seam, and `ConfigStdEnvProvider` supplies process-backed access for production
+callers. Deterministic tests and other adapters can implement
+`ConfigEnvProvider` without mutating the process environment. This is a
+breaking change without a deprecation period or stable compatibility guarantee.
+
+For the normal flow:
 
-The cached configuration APIs are an opt-in flow for callers of the unstable
+1. Call `resolve_json_and_layers_outcome_with_env` with a
+   `ConfigEnvProvider`.
+2. Call `emit_diagnostics()` after tracing is configured, then call
+   `into_layers()` on the returned `DiscoveryOutcome`.
+3. Pass the resulting `DiscoveredLayers` to
+   `merge_with_cached_file_layers` with the same environment provider.
 
-Rust API. Callers that adopt this composition boundary resolve JSON mode and
-discover file layers once. `resolve_json_and_layers_outcome_with_env` returns
-`(OrthoResult<bool>, DiscoveryOutcome)`. Callers can call
-`emit_diagnostics()` after a tracing filter is installed, then call
-`into_layers()` before passing the layers to `merge_with_cached_file_layers`.
+`DiscoveryOutcome` owns the deferred diagnostics until `emit_diagnostics()` and
+the discovered layers until `into_layers()`.
 
-The standalone `resolve_merged_json_with_env` and
-`merge_with_config_and_env` functions retain their existing automatic
-discovery behaviour. The former resolves JSON mode, while the latter
-discovers and merges configuration in one call, so callers that do not need
-the cached flow require no migration.
+Reusing the same discovered layers avoids a second configuration-file discovery
+and loading pass. `merge_with_config` and `merge_with_config_and_env` remain
+standalone alternatives: each discovers and merges configuration in one call,
+so neither reuses an earlier discovery.
 
 v0.1.0 also instruments configuration loading itself. The internal
 phase-level series are `config_load_total`, labelled `phase=diag_mode|merge`
 and `outcome=success|failure`, and `config_load_duration_seconds`, labelled
 only `phase=diag_mode|merge`. The operator-facing startup-attempt series are
 `netsuke_config_load_total`, labelled only `outcome=success|failure`, and
 `netsuke_config_load_duration_seconds`, with no labels. Configuration-load
-failures add bounded `operation` and `error_category` fields. Neither
-exposes configuration paths. See the users' guide's
+failures add bounded `operation` and `error_category` fields. Cached discovery
+also records `netsuke_cli_config_discovery_total`, labelled
+`outcome=success|error`, and `netsuke_cli_config_discovery_duration_seconds`
+without labels. Neither exposes configuration paths. See the users' guide's
 [bounded configuration metrics](users-guide.md#bounded-configuration-metrics)
 and [interpret failures](users-guide.md#interpret-failures) sections.
 
@@ -115,32 +130,32 @@ The `generate` command materializes its supporting dyndep sidecars beneath
 `.netsuke/dyndep` in the effective working directory while writing the
 generated Ninja manifest. The `build` and `clean` commands materialize the
 sidecars before invoking Ninja with the generated Ninja file. User targets must
-not use `.netsuke/dyndep` or `.netsuke/serial`: both namespaces are reserved for
-Netsuke's generated state. Sidecars are immutable and content-addressed.
-Retention keeps the current bundle plus at most 32 obsolete `.dd` files and
-1 MiB of obsolete `.dd` bytes; stale `.tmp` files are cleaned while the
-exclusive sidecar-directory lease is held. `clean` prunes only after successful
+not use `.netsuke/dyndep` or `.netsuke/serial`: both namespaces are reserved
+for Netsuke's generated state. Sidecars are immutable and content-addressed.
+Retention keeps the current bundle plus at most 32 obsolete `.dd` files and 1
+MiB of obsolete `.dd` bytes; stale `.tmp` files are cleaned while the exclusive
+sidecar-directory lease is held. `clean` prunes only after successful
 `ninja -t clean`, not after a failure. An old arbitrary `generate --output`
 manifest needs regeneration only if retention has removed any of its referenced
-sidecars. See [ADR-012](adr-012-bound-dyndep-sidecar-retention.md).
-The [users' guide](users-guide.md#run-direct-dependencies-serially) documents
-the execution scope and the independent-reachability boundary.
+sidecars. See [ADR-012](adr-012-bound-dyndep-sidecar-retention.md). The
+[users' guide](users-guide.md#run-direct-dependencies-serially) documents the
+execution scope and the independent-reachability boundary.
 
 ## Discover targets and actions
 
 Target and action `description` values are optional discovery metadata. Adding
 them does not change manifest compatibility, Ninja progress text, or build
-execution: Ninja progress continues to use the referenced rule's
-`description`. Existing manifests without these fields remain valid.
+execution: Ninja progress continues to use the referenced rule's `description`.
+Existing manifests without these fields remain valid.
 
 Use the new command to inspect the selected manifest:
 
 ```sh
 netsuke help targets
 ```
 
-The command honours the usual manifest-selection options, including `--file`
-and `-C/--directory`. It loads, expands, renders, and validates the manifest
+The command honours the usual manifest-selection options, including `--file` and
+`-C/--directory`. It loads, expands, renders, and validates the manifest
 through a restricted, side-effect-free Jinja surface, then prints actions and
 targets without running recipes or creating build outputs. Queries allow only
 the lexical path filters `basename`, `dirname`, `with_suffix`, and
@@ -152,13 +167,13 @@ executable discovery through `which` and `command_available`, network and
 command helpers (`fetch`, `shell`, and `grep`), and the clock-dependent `now()`
 function. Normal build manifest rendering retains the full standard library;
 this restriction applies only to query rendering. Add `--json` to receive the
-versioned JSON result document; its
-`result.command` is `help-targets`. The command and the new descriptions are
-beta-series additions and remain subject to the stability caveat above.
+versioned JSON result document; its `result.command` is `help-targets`. The
+command and the new descriptions are beta-series additions and remain subject
+to the stability caveat above.
 
 ## Diagnostics
 
 Ninja subprocess spans and warn events carry two bounded fields,
-`env_override_count` and `path_overridden`, so environment-caused
-failures are diagnosable from logs. Variable names and values are never
-recorded; `CommandEnv`'s `Debug` output is redacted to the same counts.
+`env_override_count` and `path_overridden`, so environment-caused failures are
+diagnosable from logs. Variable names and values are never recorded;
+`CommandEnv`'s `Debug` output is redacted to the same counts.

src/cli/diag.rs

@@ -60,28 +60,25 @@ pub fn resolve_json_and_layers_outcome_with_env(
     env: &impl EnvProvider,
 ) -> (OrthoResult<bool>, DiscoveryOutcome) {
     let outcome = collect_diag_file_layers_with_env(cli, env);
-    let result = (|| {
-        if let Some(error) = outcome.first_error() {
-            return Err(Arc::clone(error));
-        }
-        let mut json = json_from_layers(&outcome);
-        if !has_cli_json_override(matches)
-            && let Some(env_json) = json_from_env(env)?
-        {
-            json = env_json;
-        }
-        Ok(json_from_matches(cli, matches, json))
-    })();
+    let has_cli_override = has_cli_json_override(matches);
+    let result = resolve_json_preference(cli, env, &outcome, has_cli_override);
     (result, outcome)
 }
 
-/// Apply the command-line JSON override to a discovered preference.
-fn json_from_matches(cli: &Cli, matches: &ArgMatches, discovered: bool) -> bool {
-    if has_cli_json_override(matches) {
-        cli.json
-    } else {
-        discovered
+/// Resolve JSON preference after file-layer discovery has completed.
+fn resolve_json_preference(
+    cli: &Cli,
+    env: &impl EnvProvider,
+    outcome: &DiscoveryOutcome,
+    has_cli_override: bool,
+) -> OrthoResult<bool> {
+    if let Some(error) = outcome.first_error() {
+        return Err(Arc::clone(error));
     }
+    if has_cli_override {
+        return Ok(cli.json);
+    }
+    json_from_env(env)?.map_or_else(|| Ok(json_from_layers(outcome)), Ok)
 }
 
 /// Determine whether `--json` was supplied on the command line.
@@ -187,7 +184,9 @@ mod tests {
         let dir = tempdir()?;
         let missing_config_path = dir.path().join("missing-netsuke.toml");
         let matches = Cli::command().get_matches_from(["netsuke"]);
-        let env = TestEnv::default().with_var("NETSUKE_CONFIG", &missing_config_path);
+        let env = TestEnv::default()
+            .with_var("NETSUKE_CONFIG", &missing_config_path)
+            .with_var(JSON_ENV_VAR, "yes");
 
         let (result, events) = with_test_subscriber(LevelFilter::TRACE, |captured| {
             let result = resolve_merged_json_with_env(&Cli::default(), &matches, &env);
@@ -196,7 +195,7 @@ mod tests {
         let error = result.expect_err("missing injected explicit config should fail");
         ensure!(
             matches!(error.as_ref(), OrthoError::File { path, .. } if path == &missing_config_path),
-            "expected missing explicit config error for {missing_config_path:?}, got {error:?}"
+            "missing explicit config should take precedence over malformed {JSON_ENV_VAR}: {error:?}"
         );
         ensure!(
             events.is_empty(),

src/cli/discovery.rs

@@ -8,25 +8,34 @@ use ortho_config::{
     MapEnv, MergeComposer, MergeLayer, OrthoResult, SharedEnvSource, load_config_file_as_chain,
 };
 use std::borrow::Cow;
-use std::ffi::OsString;
 use std::io;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use super::parser::Cli;
 
+#[path = "discovery_environment.rs"]
+mod environment;
+pub use environment::{EnvProvider, StdEnvProvider};
+
 #[path = "discovery_diagnostics.rs"]
 mod diagnostics;
+#[path = "discovery_json.rs"]
+mod json;
 #[path = "discovery_layers.rs"]
 mod layers;
 #[path = "discovery_paths.rs"]
 mod paths;
 #[path = "discovery_trace.rs"]
 mod trace;
+
+#[path = "discovery_telemetry.rs"]
+mod telemetry;
 use diagnostics::{BoundedConfigPath, ConfigLoadFailureKind, ConfigLoadWarning};
 use layers::collect_file_layers_with_trace_and_env_source;
+/// Record the discovery series for an already-timed phase at the boundary.
+pub use telemetry::record_discovery_outcome;
 use trace::{DiscoveryDiagnostics, DiscoveryTrace, FileLayerTrace};
-
 const CONFIG_ENV_VAR: &str = "NETSUKE_CONFIG";
 const DISCOVERY_ENV_KEYS: [&str; 7] = [
     CONFIG_ENV_VAR,
@@ -38,42 +47,6 @@ const DISCOVERY_ENV_KEYS: [&str; 7] = [
     "LOCALAPPDATA",
 ];
 
-/// Provides access to environment variables used during config discovery.
-///
-/// Production code uses [`StdEnvProvider`]. Tests can provide an in-memory
-/// implementation so config-selection logic does not mutate process-global
-/// environment state.
-pub trait EnvProvider {
-    /// Return the value of `key`, or `None` when the key is unset.
-    fn get(&self, key: &str) -> Option<OsString>;
-
-    /// Return all values available to the configuration environment layer.
-    ///
-    /// Providers concerned only with selector lookup may retain the empty
-    /// default. Full merge providers override this method.
-    fn entries(&self) -> Vec<(OsString, OsString)> {
-        Vec::new()
-    }
-}
-
-/// Environment provider backed by [`std::env::var_os`].
-#[derive(Debug, Default, Clone, Copy)]
-pub struct StdEnvProvider;
-
-#[expect(
-    clippy::disallowed_methods,
-    reason = "composition root: StdEnvProvider is the process-backed adapter behind the EnvProvider seam"
-)]
-impl EnvProvider for StdEnvProvider {
-    fn get(&self, key: &str) -> Option<OsString> {
-        std::env::var_os(key)
-    }
-
-    fn entries(&self) -> Vec<(OsString, OsString)> {
-        std::env::vars_os().collect()
-    }
-}
-
 /// File layers and loading errors produced by one discovery pass.
 ///
 /// The diagnostic pre-pass borrows the layers to resolve JSON output, then the
@@ -148,6 +121,11 @@ impl DiscoveryOutcome {
 
 /// Discover configuration layers once through the injected environment.
 pub(crate) fn discover_file_layers(cli: &Cli, env: &impl EnvProvider) -> DiscoveryOutcome {
+    collect_outcome(cli, env)
+}
+
+/// Run one discovery pass and retain its outcome, including deferred errors.
+fn collect_outcome(cli: &Cli, env: &impl EnvProvider) -> DiscoveryOutcome {
     let (trace, load_warning, outcome) = collect_file_layers_with_env(cli, env);
     let diagnostics = DiscoveryDiagnostics::new(trace, load_warning);
     let layers = match outcome {

src/cli/discovery_environment.rs

@@ -0,0 +1,38 @@
+//! Environment provider seam for configuration discovery and merging.
+
+use std::ffi::OsString;
+
+/// Provides access to environment variables used during config discovery.
+///
+/// Production code uses [`StdEnvProvider`]. Tests can provide an in-memory
+/// implementation so config-selection logic does not mutate process-global
+/// environment state.
+pub trait EnvProvider {
+    /// Return the value of `key`, or `None` when the key is unset.
+    fn get(&self, key: &str) -> Option<OsString>;
+
+    /// Return all values available to the configuration environment layer.
+    fn entries(&self) -> Vec<(OsString, OsString)>;
+}
+
+/// Environment provider backed by [`std::env::var_os`].
+#[derive(Debug, Default, Clone, Copy)]
+pub struct StdEnvProvider;
+
+impl EnvProvider for StdEnvProvider {
+    #[expect(
+        clippy::disallowed_methods,
+        reason = "composition root: StdEnvProvider is the process-backed adapter behind the EnvProvider seam"
+    )]
+    fn get(&self, key: &str) -> Option<OsString> {
+        std::env::var_os(key)
+    }
+
+    #[expect(
+        clippy::disallowed_methods,
+        reason = "composition root: StdEnvProvider is the process-backed adapter behind the EnvProvider seam"
+    )]
+    fn entries(&self) -> Vec<(OsString, OsString)> {
+        std::env::vars_os().collect()
+    }
+}