From 8ce7e8e04342ddd6451477f0a6052223aaad7d12 Mon Sep 17 00:00:00 2001 From: Leynos Date: Thu, 16 Apr 2026 03:16:33 +0000 Subject: [PATCH 1/3] feat(cli): add --config flag and NETSUKE_CONFIG env var to specify config file Introduce a new command-line flag `--config ` and a new environment variable `NETSUKE_CONFIG` to allow users to explicitly specify a configuration file. This bypasses the automatic config discovery mechanism and loads the specified file directly. The legacy `NETSUKE_CONFIG_PATH` remains supported as a silently deprecated alias, with precedence: - `--config` flag (highest) - `NETSUKE_CONFIG` env variable (new documented alias) - `NETSUKE_CONFIG_PATH` env variable (legacy) - automatic discovery (lowest) The `config` field is added to the `Cli` struct with proper annotations (`#[serde(skip)]` and clap arguments) to exclude it from OrthoConfig serialization and merging pipelines, preventing it from leaking as a config preference. The merge pipeline functions are updated to honor the explicit config path, loading that file directly and skipping discovery when specified. Errors on missing or invalid explicit config files are propagated. Localization keys for the flag help text are added with English and Spanish translations. The user guide and design documentation are updated to describe the new flag and environment variable and the precedence rules. Comprehensive integration and behavioral tests verify correct loading precedence, error handling, and backward compatibility. This completes roadmap item 3.11.3, enhancing config file specification clarity and control for users. Co-authored-by: devboxerhub[bot] --- docs/developers-guide.md | 9 +- ...3-expose-config-path-and-netsuke-config.md | 682 ++++++++++++++++++ 2 files changed, 686 insertions(+), 5 deletions(-) create mode 100644 docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 258ce98e..e8a13f4a 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -158,9 +158,8 @@ let _cwd_guard = CwdGuard::acquire()?; std::env::set_current_dir(temp.path())?; ``` -Acquire `EnvLock` and then `CwdGuard` so Rust drops them in reverse -declaration order: `CwdGuard` restores the CWD first, and `EnvLock` releases -second. +Acquire `EnvLock` and then `CwdGuard` so Rust drops them in reverse declaration +order: `CwdGuard` restores the CWD first, and `EnvLock` releases second. ### `restore_many` and `restore_many_locked` @@ -354,8 +353,8 @@ Versioning and compatibility rules: - `cli_overrides_from_matches` must continue to emit a JSON object, even when no CLI override is present. - `is_empty_value` treats only the empty object `{}` as "no CLI overrides". - Downstream tooling must not replace an empty object with `null`, `[]`, or - any other sentinel. + Downstream tooling must not replace an empty object with `null`, `[]`, or any + other sentinel. - Additional properties are ignored by `diag_json` resolution and may be present because the same layer object also participates in full config merging. diff --git a/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md b/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md new file mode 100644 index 00000000..5b9a0f6a --- /dev/null +++ b/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md @@ -0,0 +1,682 @@ +# 3.11.3. Expose `--config ` and `NETSUKE_CONFIG` + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work +proceeds. + +Status: DRAFT + +## Purpose / big picture + +After this work, a Netsuke user can point the tool at an arbitrary +configuration file in two new, visible ways: + +1. A CLI flag: `netsuke --config /path/to/config.toml build` +2. An environment variable: `NETSUKE_CONFIG=/path/to/config.toml netsuke build` + +Both surfaces bypass automatic discovery and load the specified file directly. +The existing `NETSUKE_CONFIG_PATH` environment variable continues to work as a +silent alias for backward compatibility, but `NETSUKE_CONFIG` becomes the +documented, user-facing name. + +The repository also ships an annotated sample configuration file at +`docs/sample-netsuke.toml` that documents every supported key, so users have a +starting point without reading source code. + +Observable success means all of the following hold simultaneously: + +- `netsuke --config /tmp/custom.toml build` loads the custom file instead of + the discovered one. +- `NETSUKE_CONFIG=/tmp/custom.toml netsuke build` does the same. +- The legacy `NETSUKE_CONFIG_PATH` still works when `NETSUKE_CONFIG` is not + set. +- When both are set, `NETSUKE_CONFIG` takes precedence over + `NETSUKE_CONFIG_PATH`. +- `netsuke --help` shows the `--config` flag with localised help text. +- `docs/sample-netsuke.toml` is a valid, parsable config file with comments + explaining every key. +- `make check-fmt`, `make lint`, and `make test` all pass. +- The roadmap entry 3.11.3 is checked off. + +## Constraints + +- Keep `Cli` as the concrete clap and OrthoConfig merge root. Do not + restructure the derive hierarchy. +- Preserve backward compatibility with `NETSUKE_CONFIG_PATH`. Removing it + would break existing CI pipelines and user workflows. +- Preserve the standard precedence ladder: defaults < config files < + environment variables < CLI flags. The `--config` flag is a file-selection + mechanism, not a value-level override: it selects which file to load, but the + file's values still sit below environment and CLI in the precedence chain. +- Keep all source files below the 400-line limit per `AGENTS.md`. +- Keep all new user-facing strings localizable via Fluent. Update both + `en-US` and `es-ES` bundles. +- Add `build.rs` symbol anchors for any new public helpers. +- Do not use OrthoConfig's built-in `discovery(...)` attribute on the `Cli` + struct. Netsuke manages its own discovery through `config_discovery()` in + `src/cli/config_merge.rs` because OrthoConfig's `compose_layers()` returns + only the first found file, and Netsuke's two-pass approach is needed for + correct project-over-user precedence. The new `--config` flag must integrate + with this custom discovery path, not replace it. +- The `--config` flag must use a long-form argument only. The short `-c` is + not assigned because `-C` (uppercase) is already taken by `--directory` and + the visual similarity would cause confusion. +- Mark roadmap item 3.11.3 done only after all validation gates pass. + +## Tolerances (exception triggers) + +- Scope: if implementation requires more than 16 files changed or more than + 700 net new lines, stop and escalate before proceeding. +- Interface: if the change requires altering the signature of + `merge_with_config` or `resolve_merged_diag_json` in a way that breaks + existing callers, stop and escalate. +- Dependencies: if a new external crate dependency is required, stop and + escalate. +- Iterations: if `make lint` or `make test` still fail after three focused + fix-and-rerun cycles within a single stage, stop, document the blocker, and + escalate. +- Ambiguity: if the interaction between `--config`, `NETSUKE_CONFIG`, and + `NETSUKE_CONFIG_PATH` creates an unresolvable precedence conflict, stop and + present options with trade-offs. + +## Risks + +- Risk: adding a `--config` field to the `Cli` struct may interact with + OrthoConfig's hidden `--config-path` flag, creating a clap conflict or + ambiguity. Severity: medium. Likelihood: medium. Mitigation: OrthoConfig's + hidden `--config-path` is only injected when the struct uses + `discovery(...)`, which Netsuke does not. Netsuke's `Cli` already manages its + own discovery externally, so adding a plain `config: Option` field + should not collide. Verify during Stage A by running `netsuke --help` and + confirming no duplicate flags. + +- Risk: the `cli_overrides_from_matches` function strips fields not + explicitly set on the command line. A new `config` field must be handled + correctly there — it should be stripped from value-level overrides because it + is a meta-field (selects which file to load), not a config preference. + Severity: high. Likelihood: high. Mitigation: add `"config"` to the exclusion + list in `cli_overrides_from_matches` during Stage B. + +- Risk: the `config` field will be serialized by `sanitize_value` and included + in the merge pipeline if not handled carefully. Because it is a file + selector, not a preference, including it in the merged `Cli` output would be + confusing and could cause issues if the merged struct is re-serialized. + Severity: medium. Likelihood: high. Mitigation: mark the field with + `#[serde(skip)]` so it does not participate in OrthoConfig serialization. + Keep it as a clap-only, parse-time field. + +- Risk: backward compatibility between `NETSUKE_CONFIG` and + `NETSUKE_CONFIG_PATH` could create confusion if both are set to different + files. Severity: low. Likelihood: low. Mitigation: define a clear precedence: + `--config` (CLI) > `NETSUKE_CONFIG` (env) > `NETSUKE_CONFIG_PATH` (env, + legacy). Document this in the user guide. + +- Risk: the new `--config` flag could be passed alongside `-C` / + `--directory`, creating ambiguity about which directory anchors the config + path. Severity: low. Likelihood: medium. Mitigation: `--config` accepts an + absolute or relative path resolved against the process working directory (not + `-C`). This matches the semantics of `NETSUKE_CONFIG_PATH` and avoids + surprising interactions. Document this clearly. + +- Risk: new Fluent keys need both `en-US` and `es-ES` translations or the + build-time audit will fail. Severity: high. Likelihood: high. Mitigation: add + keys to both bundles in Stage C. Use a reasonable Spanish translation (or a + close English fallback with a `TODO(l10n)` comment if unsure) and verify with + `make lint`. + +## Progress + +- [ ] Stage A: add `--config` CLI field and wire it into discovery. +- [ ] Stage B: support `NETSUKE_CONFIG` environment variable alongside legacy + `NETSUKE_CONFIG_PATH`. +- [ ] Stage C: add Fluent localization keys and `build.rs` anchoring. +- [ ] Stage D: add `rstest` integration tests for `--config` and + `NETSUKE_CONFIG`. +- [ ] Stage E: add `rstest-bdd` behavioural tests. +- [ ] Stage F: ship annotated sample config and update documentation. +- [ ] Stage G: validation, roadmap update, and evidence capture. + +## Surprises & discoveries + +(None yet — this section will be populated during implementation.) + +## Decision log + +- Decision: use a plain `config: Option` field on `Cli` with + `#[serde(skip)]` rather than OrthoConfig's + `discovery(config_cli_long = "config", config_cli_visible = true)` attribute. + Rationale: Netsuke's two-pass discovery in `config_merge.rs` is required for + correct project-over-user file precedence. OrthoConfig's built-in + `compose_layers()` returns only the first found file and cannot express this. + Introducing the `discovery(...)` attribute would replace Netsuke's custom + pipeline, and the interaction between a `discovery()`-generated flag and the + existing manual `config_discovery()` builder is untested and risky. A plain + clap field avoids the coupling. Date/Author: 2026-04-16 / planning agent. + +- Decision: keep `NETSUKE_CONFIG_PATH` as a silent backward-compatible alias. + Rationale: CI pipelines may already use it. Removing it would be a breaking + change for no user benefit. The new `NETSUKE_CONFIG` env var takes precedence + when both are set. Date/Author: 2026-04-16 / planning agent. + +- Decision: `--config` uses long-form only (no `-c` short flag). + Rationale: `-C` (uppercase) is already assigned to `--directory`. A lowercase + `-c` for a different flag would cause visual confusion, particularly in + documentation and error messages. Long-form `--config` is unambiguous. + Date/Author: 2026-04-16 / planning agent. + +- Decision: `--config` path is resolved against the process working + directory, not the `-C` directory. Rationale: this matches the existing + `NETSUKE_CONFIG_PATH` semantics and the user's shell expectations. The `-C` + flag re-anchors project-scope discovery and manifest lookup, but the config + file path is specified before any directory change is applied. Date/Author: + 2026-04-16 / planning agent. + +## Outcomes & retrospective + +(To be completed after implementation.) + +## Context and orientation + +Read these files in order before changing code. + +1. `src/cli/mod.rs` — the `Cli` struct (lines 40–142). This is the clap + parser and OrthoConfig merge root. It defines the current `CONFIG_ENV_VAR` + constant (`"NETSUKE_CONFIG_PATH"`) and the `ENV_PREFIX` constant + (`"NETSUKE_"`). The new `config` field will be added here. + +2. `src/cli/config_merge.rs` — the merge pipeline. The key functions are: + + - `config_discovery(directory)` (line 38): builds a `ConfigDiscovery` + using `CONFIG_ENV_VAR`. This function must be updated to also accept an + explicit config path from `--config`. + - `push_file_layers(composer, errors, directory)` (line 176): two-pass + file discovery. When an explicit config path is provided, this function + should load that file directly and skip all discovery. + - `collect_diag_file_layers(directory)` (line 111): mirrors + `push_file_layers` for early diag-JSON resolution. Must also honour + `--config`. + - `merge_with_config(cli, matches)` (line 264): the top-level merge + entry point. The `cli.config` field is read here to decide whether to + use explicit loading or discovery. + +3. `src/cli/config.rs` — the `CliConfig` typed view. The `config` field does + NOT belong here because it is a file selector, not a runtime preference. + +4. `src/cli_l10n.rs` — localization helpers. `flag_help_key()` (line 122) + maps argument IDs to Fluent keys. A new mapping for `"config"` must be added. + +5. `src/localization/keys.rs` — Fluent key constants. A new key + `CLI_FLAG_CONFIG_HELP` must be added. + +6. `locales/en-US/messages.ftl` and `locales/es-ES/messages.ftl` — Fluent + bundles. New messages for the `--config` flag help text. + +7. `build.rs` — symbol anchoring. If any new public helper is exposed from + `src/cli/mod.rs` or `src/cli/config_merge.rs`, a `const _` anchor must be + added. + +8. `tests/cli_tests/config_discovery.rs` — existing integration tests for + config discovery. New tests for `--config` and `NETSUKE_CONFIG` will be + added here or in a neighbouring file. + +9. `tests/features/configuration_discovery.feature` and + `tests/bdd/steps/configuration_discovery.rs` — existing BDD coverage for + config discovery. New scenarios will extend this feature file. + +10. `docs/users-guide.md` (lines 543–630) — user-facing configuration + documentation. Must be updated to describe `--config` and + `NETSUKE_CONFIG`. + +11. `docs/netsuke-design.md` (lines 2030–2111) — design decisions section + 8.4. Must be updated to record the new config override surface. + +12. `docs/roadmap.md` (lines 296–298) — roadmap item 3.11.3. Must be marked + done after all gates pass. + +## Plan of work + +### Stage A. Add `--config` CLI field and wire it into discovery + +Add a new `config: Option` field to the `Cli` struct in +`src/cli/mod.rs`. This field: + +- accepts a file path via `--config `; +- is marked `#[serde(skip)]` so it does not participate in OrthoConfig + serialization or merging (it is a meta-field, not a preference); +- is excluded from the override detection in `cli_overrides_from_matches` + by virtue of `#[serde(skip)]` (serde-skipped fields do not appear in the + serialized JSON, so they cannot leak into the merge pipeline); +- has localised help text via the existing localization infrastructure. + +Update the default impl for `Cli` to set `config: None`. + +Then, update the merge pipeline in `src/cli/config_merge.rs`: + +1. Add a new constant: `const CONFIG_ENV_VAR_NEW: &str = "NETSUKE_CONFIG";` + (or rename the semantics — see below). + +2. Create a helper `fn resolve_config_path(cli: &Cli) -> Option` + that implements the precedence: `cli.config` (from `--config`) > + `NETSUKE_CONFIG` env var > `NETSUKE_CONFIG_PATH` env var > `None` (use + discovery). This helper reads environment variables directly + (`std::env::var_os`) because the env-var layer in the merge pipeline is for + preference values, not for file selection. + +3. Update `config_discovery()` to accept an `Option<&Path>` for the explicit + config path. When `Some`, skip `ConfigDiscovery` entirely and load the file + directly via `load_config_file_as_chain`. + +4. Update `push_file_layers()` to call `resolve_config_path` and, when a + path is returned, load that single file instead of running two-pass + discovery. When the explicit path does not exist or fails to parse, + propagate the error rather than falling back to discovery — the user + explicitly requested this file. + +5. Update `collect_diag_file_layers()` with the same explicit-path logic + so `resolve_merged_diag_json` honours `--config` and `NETSUKE_CONFIG`. + +6. Update `merge_with_config()` — the existing code passes + `cli.directory.as_deref()` into `push_file_layers`. Now also pass the + resolved config path. The function signature of `push_file_layers` will gain + an `explicit_config: Option<&Path>` parameter. + +Acceptance for Stage A: + +- `cargo check` succeeds. +- `netsuke --help` shows the `--config` flag (with English help text as a + placeholder until Stage C). +- `netsuke --config /nonexistent.toml build` reports an error about the + missing file rather than falling back to discovery. +- `netsuke --config build` loads the specified file. + +### Stage B. Support `NETSUKE_CONFIG` environment variable + +Update `resolve_config_path()` to check `NETSUKE_CONFIG` before +`NETSUKE_CONFIG_PATH`. The full precedence for file selection is: + +```plaintext +--config (CLI flag, highest) +NETSUKE_CONFIG (env var, new user-facing name) +NETSUKE_CONFIG_PATH (env var, legacy silent alias) +automatic discovery (lowest, two-pass project > user) +``` + +Update the `has_explicit_config` checks in `push_file_layers` and +`collect_diag_file_layers` to also check `NETSUKE_CONFIG`. Currently these +check `CONFIG_ENV_VAR` (`NETSUKE_CONFIG_PATH`); they must now check both env +vars and the CLI field. + +Acceptance for Stage B: + +- `NETSUKE_CONFIG=/tmp/custom.toml netsuke build` uses the custom file. +- `NETSUKE_CONFIG_PATH=/tmp/legacy.toml netsuke build` still works. +- When both are set to different files, `NETSUKE_CONFIG` wins. +- `cargo check` succeeds. + +### Stage C. Fluent localization keys and `build.rs` anchoring + +1. Add a new key constant in `src/localization/keys.rs`: + + ```rust + CLI_FLAG_CONFIG_HELP => "cli.flag.config.help", + ``` + +2. Add the Fluent message to `locales/en-US/messages.ftl`: + + ```ftl + cli.flag.config.help = Path to a configuration file, bypassing automatic discovery. + ``` + +3. Add the Fluent message to `locales/es-ES/messages.ftl`: + + ```ftl + cli.flag.config.help = Ruta a un archivo de configuración, omitiendo la detección automática. + ``` + +4. Update `flag_help_key()` in `src/cli_l10n.rs` to map `"config"` to + `keys::CLI_FLAG_CONFIG_HELP`. + +5. If `resolve_config_path` or any other new helper is made `pub` and + used from `build.rs`-compiled modules, add a `const _` symbol anchor in + `build.rs::assert_symbols_linked()`. If the helpers remain `pub(super)` or + private to `config_merge.rs`, no anchor is needed. + +Acceptance for Stage C: + +- `make lint` passes (including `cargo doc` and the build-time Fluent + audit). +- `netsuke --help` shows the `--config` flag with localised English text. + +### Stage D. `rstest` integration tests + +Add integration tests in `tests/cli_tests/config_discovery.rs` (or a +neighbouring file if the 400-line limit is reached). Tests should follow the +existing pattern: acquire `EnvLock`, create temp directories, write config +files, parse with `parse_with_localizer_from`, merge with `merge_with_config`, +and assert on the merged `Cli` struct. + +Test cases: + +1. `config_flag_loads_specified_file` — write a custom config file with a + distinctive theme, pass `--config `, assert theme matches. + +2. `config_flag_skips_project_discovery` — place a project `.netsuke.toml` + in the current directory with theme `ascii`, pass `--config` pointing to a + file with theme `unicode`, assert `unicode` wins. + +3. `config_flag_with_nonexistent_file_produces_error` — pass `--config` + pointing to a path that does not exist, assert the merge returns an error. + +4. `netsuke_config_env_loads_specified_file` — set `NETSUKE_CONFIG` to a + custom file, parse without `--config`, assert the custom file is loaded. + +5. `netsuke_config_env_takes_precedence_over_legacy` — set + `NETSUKE_CONFIG` to file A and `NETSUKE_CONFIG_PATH` to file B (with + different themes), assert file A wins. + +6. `config_flag_takes_precedence_over_netsuke_config_env` — set + `NETSUKE_CONFIG` to file A, pass `--config` pointing to file B, assert file + B wins. + +7. `config_flag_values_still_overridden_by_env_and_cli_preferences` — + custom config sets theme `ascii`, env sets `NETSUKE_THEME=unicode`, assert + theme is `unicode` (the file is loaded, but preference-level env vars still + override preference values from the file). + +Use `rstest` parameterization where appropriate. Use `EnvVarGuard` for all +environment mutations. Use `CwdGuard` if changing working directory. + +Acceptance for Stage D: + +- `cargo test --test cli_tests -- config` runs the new tests and they pass. +- Tests are deterministic and do not interfere with parallel execution. + +### Stage E. `rstest-bdd` behavioural tests + +Extend `tests/features/configuration_discovery.feature` with new scenarios that +prove user-observable behaviour: + +```gherkin +Scenario: Explicit config file overrides project discovery + Given a temporary workspace + And a project config file ".netsuke.toml" with theme "ascii" + And a custom config file "custom.toml" with theme "unicode" + When the CLI is parsed with "--config custom.toml" + Then parsing succeeds + And the theme preference is "unicode" + +Scenario: NETSUKE_CONFIG environment variable selects config file + Given a temporary workspace + And a project config file ".netsuke.toml" with theme "ascii" + And a custom config file "override.toml" with theme "unicode" + And the environment variable "NETSUKE_CONFIG" points to "override.toml" + When the CLI is parsed with no additional arguments + Then parsing succeeds + And the theme preference is "unicode" + +Scenario: NETSUKE_CONFIG takes precedence over NETSUKE_CONFIG_PATH + Given a temporary workspace + And a custom config file "new.toml" with theme "unicode" + And a custom config file "legacy.toml" with theme "ascii" + And the environment variable "NETSUKE_CONFIG" points to "new.toml" + And the environment variable "NETSUKE_CONFIG_PATH" points to "legacy.toml" + When the CLI is parsed with no additional arguments + Then parsing succeeds + And the theme preference is "unicode" + +Scenario: CLI config flag takes precedence over NETSUKE_CONFIG + Given a temporary workspace + And a custom config file "cli.toml" with theme "unicode" + And a custom config file "env.toml" with theme "ascii" + And the environment variable "NETSUKE_CONFIG" points to "env.toml" + When the CLI is parsed with "--config cli.toml" + Then parsing succeeds + And the theme preference is "unicode" +``` + +Add or extend step definitions in `tests/bdd/steps/configuration_discovery.rs` +as needed. The existing `write_config_file` helper and +`custom_config_with_theme` step should work for most scenarios. The +`When the CLI is parsed with "--config custom.toml"` step needs to resolve +`custom.toml` relative to the temp workspace directory before passing to the +parser — update the existing When step to handle `--config` arguments that +reference filenames in the workspace. + +Acceptance for Stage E: + +- `cargo test --test bdd_tests configuration_discovery` runs all scenarios + (old and new) and they pass. +- Scenarios read as user stories, not as unit tests in Gherkin clothing. + +### Stage F. Annotated sample config and documentation updates + +1. Create `docs/sample-netsuke.toml` — an annotated sample configuration + file that documents every supported key. Use TOML comments (`#`) to explain + each field, its default value, and valid options. The file must be parsable + by Netsuke (all values should be valid defaults or commented out). Structure + it by section: general, build behaviour, output preferences, network policy. + + Example structure: + + ```toml + # Netsuke sample configuration + # + # Place this file at .netsuke.toml in your project root, or point + # to it with --config or NETSUKE_CONFIG. + + # Enable verbose diagnostic logging and timing summaries. + # verbose = false + + # Locale for CLI messages (e.g., "en-US", "es-ES"). + # locale = "en-US" + + # CLI theme preset: "auto", "unicode", or "ascii". + # theme = "auto" + + # ... (all other fields) + ``` + +2. Update `docs/users-guide.md`: + + - In the "Configuration and Localization" section (around line 543), add + documentation for `--config ` and `NETSUKE_CONFIG`. + - Document the full config override precedence: + `--config` > `NETSUKE_CONFIG` > `NETSUKE_CONFIG_PATH` > automatic + discovery. + - Mention the sample config file and where to find it. + - Ensure the existing `NETSUKE_CONFIG_PATH` documentation is preserved + but de-emphasised as a legacy alias. + +3. Update `docs/netsuke-design.md` section 8.4: + + - Record the config override surface design decision. + - Document the interaction between `--config`, `NETSUKE_CONFIG`, and + `NETSUKE_CONFIG_PATH`. + +4. Run `make fmt` and `make markdownlint` after documentation changes. + +Acceptance for Stage F: + +- `netsuke --config docs/sample-netsuke.toml build` does not error on + config parsing (the sample file should be valid or entirely commented out). +- `make markdownlint` passes. +- `make nixie` passes (if any Mermaid diagrams were added or changed). +- A user can learn how to use `--config` and `NETSUKE_CONFIG` by reading + only the user guide. + +### Stage G. Validation, roadmap update, and evidence capture + +1. Run all validation gates using `tee` and `set -o pipefail`: + + ```sh + set -o pipefail && make fmt 2>&1 | tee /tmp/3-11-3-make-fmt.log + set -o pipefail && make check-fmt 2>&1 | tee /tmp/3-11-3-check-fmt.log + set -o pipefail && make lint 2>&1 | tee /tmp/3-11-3-make-lint.log + set -o pipefail && make test 2>&1 | tee /tmp/3-11-3-make-test.log + set -o pipefail && make markdownlint 2>&1 | tee /tmp/3-11-3-markdownlint.log + set -o pipefail && make nixie 2>&1 | tee /tmp/3-11-3-make-nixie.log + ``` + +2. Review log files for truncated output, not just exit codes. + +3. Mark roadmap item 3.11.3 done in `docs/roadmap.md`. + +4. Update the `Progress`, `Outcomes & Retrospective`, and + `Surprises & Discoveries` sections of this ExecPlan. + +Acceptance for Stage G: + +- All six validation commands exit with status 0. +- Roadmap item 3.11.3 is checked off. +- This ExecPlan status is updated to COMPLETE. + +## Interfaces and dependencies + +### New field on `Cli` (`src/cli/mod.rs`) + +```rust +/// Path to a configuration file, bypassing automatic discovery. +/// +/// When specified, Netsuke loads this file instead of searching for +/// `.netsuke.toml` in project and user scopes. The file path is resolved +/// against the process working directory. +#[arg(long, value_name = "PATH")] +#[serde(skip)] +#[ortho_config(skip_cli)] +pub config: Option, +``` + +The `#[serde(skip)]` annotation prevents the field from being serialized into +the JSON value that feeds the merge pipeline. The `#[ortho_config (skip_cli)]` +annotation (matching the existing `command` field pattern) prevents OrthoConfig +from trying to merge this field across layers. + +### Config path resolution helper (`src/cli/config_merge.rs`) + +```rust +/// Resolve the effective explicit config file path. +/// +/// Precedence: `--config` CLI flag > `NETSUKE_CONFIG` env var > +/// `NETSUKE_CONFIG_PATH` env var > `None` (use automatic discovery). +fn resolve_config_path(cli: &Cli) -> Option { + if let Some(ref path) = cli.config { + return Some(path.clone()); + } + if let Some(val) = std::env::var_os("NETSUKE_CONFIG") { + if !val.is_empty() { + return Some(PathBuf::from(val)); + } + } + if let Some(val) = std::env::var_os(CONFIG_ENV_VAR) { + if !val.is_empty() { + return Some(PathBuf::from(val)); + } + } + None +} +``` + +### Updated `push_file_layers` signature + +```rust +fn push_file_layers( + composer: &mut MergeComposer, + errors: &mut Vec>, + directory: Option<&Path>, + explicit_config: Option<&Path>, +) +``` + +When `explicit_config` is `Some`, the function loads that single file via +`load_config_file_as_chain` and pushes the resulting layers. It does not run +`config_discovery()` or the second-pass project-scope loader. When the file +does not exist or fails to parse, the error is pushed to `errors`. + +### Updated `collect_diag_file_layers` signature + +```rust +fn collect_diag_file_layers( + directory: Option<&Path>, + explicit_config: Option<&Path>, +) -> Vec> +``` + +Mirrors the `push_file_layers` change for early diag-JSON resolution. + +### Fluent key (`src/localization/keys.rs`) + +```rust +CLI_FLAG_CONFIG_HELP => "cli.flag.config.help", +``` + +### Localization mapping (`src/cli_l10n.rs`, in `flag_help_key`) + +```rust +"config" => Some(keys::CLI_FLAG_CONFIG_HELP), +``` + +## Validation and acceptance + +Quality criteria (what "done" means): + +- Tests: `make test` passes the full workspace suite, including at least 7 + new integration tests and 4 new BDD scenarios. +- Lint: `make lint` passes with zero warnings. +- Format: `make check-fmt` passes. +- Markdown: `make markdownlint` passes. +- Mermaid: `make nixie` passes. +- Sample config: `docs/sample-netsuke.toml` parses without errors. +- Docs: the user guide documents `--config`, `NETSUKE_CONFIG`, the full + precedence chain, and the sample config file. + +Quality method (how we check): + +```sh +set -o pipefail && make check-fmt 2>&1 | tee /tmp/3-11-3-check-fmt.log +set -o pipefail && make lint 2>&1 | tee /tmp/3-11-3-make-lint.log +set -o pipefail && make test 2>&1 | tee /tmp/3-11-3-make-test.log +set -o pipefail && make markdownlint 2>&1 | tee /tmp/3-11-3-markdownlint.log +set -o pipefail && make nixie 2>&1 | tee /tmp/3-11-3-make-nixie.log +``` + +## Idempotence and recovery + +All stages are safe to re-run. Config file writes are idempotent (write the +same content). Test execution is stateless. Environment variable mutations are +protected by `EnvLock` and `EnvVarGuard` RAII guards. + +If a stage fails partway through, fix the issue and re-run from the start of +that stage. No rollback is needed because no destructive operations are +performed. + +## Artifacts and notes + +### File change summary (expected) + +New files: + +- `docs/sample-netsuke.toml` — annotated sample configuration file. + +Modified files: + +- `src/cli/mod.rs` — add `config: Option` field; update `Default` + impl. +- `src/cli/config_merge.rs` — add `resolve_config_path`; update + `push_file_layers`, `collect_diag_file_layers`, and `merge_with_config` to + honour explicit config paths. +- `src/cli_l10n.rs` — add `"config"` mapping in `flag_help_key`. +- `src/localization/keys.rs` — add `CLI_FLAG_CONFIG_HELP` key. +- `locales/en-US/messages.ftl` — add `cli.flag.config.help` message. +- `locales/es-ES/messages.ftl` — add `cli.flag.config.help` message. +- `tests/cli_tests/config_discovery.rs` (or new neighbour) — add 7+ + integration tests. +- `tests/features/configuration_discovery.feature` — add 4 BDD scenarios. +- `tests/bdd/steps/configuration_discovery.rs` — extend step definitions if + needed. +- `docs/users-guide.md` — document `--config` and `NETSUKE_CONFIG`. +- `docs/netsuke-design.md` — record design decision in section 8.4. +- `docs/roadmap.md` — mark 3.11.3 done. +- `build.rs` — add symbol anchor if new public helpers are exposed. +- `src/cli/config_merge_tests.rs` — add unit tests for `resolve_config_path` + and updated `push_file_layers`. From 42c42dd115ab1ebc7bb8ec957adf683524362539 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 20 Apr 2026 22:18:38 +0000 Subject: [PATCH 2/3] feat(cli): add --config flag and NETSUKE_CONFIG env var for explicit config file selection - Introduce a visible `--config ` CLI flag to specify an explicit config file, bypassing automatic discovery. - Add `NETSUKE_CONFIG` environment variable as the documented override, keeping `NETSUKE_CONFIG_PATH` as a legacy fallback. - Centralize explicit config path resolution in `resolve_config_path()` to unify precedence handling across merging and diagnostic JSON resolution. - Update configuration discovery logic to prioritize explicit selectors (`--config` > `NETSUKE_CONFIG` > `NETSUKE_CONFIG_PATH`) before falling back to automatic discovery. - Extend integration and behavioral tests to cover CLI/environment precedence, missing file errors, and legacy support. - Provide a sample annotated config file and update documentation accordingly. This change improves user ergonomics by exposing explicit config file selection via CLI and environment, clarifies configuration precedence, and unifies implementation logic to prevent inconsistencies. Co-authored-by: devboxerhub[bot] --- ...3-expose-config-path-and-netsuke-config.md | 78 +++++- docs/netsuke-design.md | 29 ++- docs/roadmap.md | 6 +- docs/sample-netsuke.toml | 63 +++++ docs/users-guide.md | 23 +- locales/en-US/messages.ftl | 1 + locales/es-ES/messages.ftl | 1 + src/cli/config_merge.rs | 95 ++++++-- src/cli/config_merge_tests.rs | 18 +- src/cli/mod.rs | 9 +- src/cli_l10n.rs | 1 + src/localization/keys.rs | 1 + tests/bdd/steps/cli_parsing.rs | 1 + tests/cli_tests/config_selection.rs | 227 ++++++++++++++++++ tests/cli_tests/mod.rs | 1 + .../features/configuration_discovery.feature | 38 +++ 16 files changed, 536 insertions(+), 56 deletions(-) create mode 100644 docs/sample-netsuke.toml create mode 100644 tests/cli_tests/config_selection.rs diff --git a/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md b/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md index 5b9a0f6a..49644e47 100644 --- a/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md +++ b/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md @@ -5,7 +5,7 @@ This ExecPlan (execution plan) is a living document. The sections `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: DRAFT +Status: COMPLETE ## Purpose / big picture @@ -127,19 +127,33 @@ Observable success means all of the following hold simultaneously: ## Progress -- [ ] Stage A: add `--config` CLI field and wire it into discovery. -- [ ] Stage B: support `NETSUKE_CONFIG` environment variable alongside legacy +- [x] Stage A: add `--config` CLI field and wire it into discovery. +- [x] Stage B: support `NETSUKE_CONFIG` environment variable alongside legacy `NETSUKE_CONFIG_PATH`. -- [ ] Stage C: add Fluent localization keys and `build.rs` anchoring. -- [ ] Stage D: add `rstest` integration tests for `--config` and +- [x] Stage C: add Fluent localization keys and `build.rs` anchoring. +- [x] Stage D: add `rstest` integration tests for `--config` and `NETSUKE_CONFIG`. -- [ ] Stage E: add `rstest-bdd` behavioural tests. -- [ ] Stage F: ship annotated sample config and update documentation. -- [ ] Stage G: validation, roadmap update, and evidence capture. +- [x] Stage E: add `rstest-bdd` behavioural tests. +- [x] Stage F: ship annotated sample config and update documentation. +- [x] Stage G: validation, roadmap update, and evidence capture. ## Surprises & discoveries -(None yet — this section will be populated during implementation.) +- Observation: a single `resolve_config_path()` helper in + `src/cli/config_merge.rs` cleanly keeps `merge_with_config()` and + `resolve_merged_diag_json()` aligned. This removed the old implicit reliance + on `ConfigDiscovery::env_var(...)` for explicit file selection and avoided + duplicating precedence logic. Date/Author: 2026-04-20 / implementation agent. + +- Observation: the existing BDD configuration-discovery steps were already + generic enough for `NETSUKE_CONFIG` and `--config`; only the feature file and + the generic isolated-CLI env cleanup list needed changes. Date/Author: + 2026-04-20 / implementation agent. + +- Observation: explicit missing config files surface as file-layer merge + errors, so integration tests must inspect the full error chain or debug + output rather than only the top-level context string. Date/Author: 2026-04-20 + / implementation agent. ## Decision log @@ -172,9 +186,53 @@ Observable success means all of the following hold simultaneously: file path is specified before any directory change is applied. Date/Author: 2026-04-16 / planning agent. +- Decision: retire `ConfigDiscovery::env_var(...)` from Netsuke's internal + automatic-discovery helper and perform all explicit config-path selection in + `resolve_config_path()`. Rationale: once `NETSUKE_CONFIG`, + `NETSUKE_CONFIG_PATH`, and `--config` must share a single precedence ladder, + keeping the file-selection logic in one helper is simpler and prevents drift + between normal merging and early diag-JSON resolution. Date/Author: + 2026-04-20 / implementation agent. + ## Outcomes & retrospective -(To be completed after implementation.) +Implementation completed on 2026-04-20. + +Validation evidence: + +- `make fmt` +- `make check-fmt` +- `make lint` +- `make test` +- `make markdownlint` +- `make nixie` + +Key outcomes: + +- Added a visible `--config ` CLI flag on `Cli` as a parse-time-only + field (`#[serde(skip)]`), keeping it out of value merging. +- Added `NETSUKE_CONFIG` as the documented environment-variable selector while + preserving `NETSUKE_CONFIG_PATH` as a lower-precedence legacy alias. +- Centralized explicit config selection in + `src/cli/config_merge.rs::resolve_config_path()`, which now drives both full + merging and early `diag_json` resolution. +- Added focused integration coverage in + `tests/cli_tests/config_selection.rs` for CLI/env precedence, missing-file + handling, and continued value-level precedence over the selected file. +- Extended the configuration-discovery BDD feature with scenarios for + `--config`, `NETSUKE_CONFIG`, and precedence over the legacy alias. +- Added `docs/sample-netsuke.toml` and updated the user guide, design + documentation, roadmap, and this ExecPlan to reflect the final contract. + +Retrospective: + +- The main implementation risk was not in Clap or OrthoConfig integration, but + in keeping file-selection precedence consistent across the normal merge path, + early diagnostic-mode resolution, integration tests, and BDD harness. + Centralizing the logic in a single helper avoided that drift. +- The existing BDD support was already flexible enough for the new env var and + most of the CLI coverage, so the behavioural delta stayed smaller than the + plan first suggested. ## Context and orientation diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index d2738947..86ccea5a 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2179,9 +2179,10 @@ flowchart LR Netsuke configuration discovery is implemented through OrthoConfig's `ConfigDiscovery` builder in `src/cli/config_merge.rs`. The `config_discovery()` helper constructs a discovery instance rooted at -application name `"netsuke"`, honours the `NETSUKE_CONFIG_PATH` environment -variable as an explicit override, and adjusts project-root discovery when the -`-C/--directory` flag is supplied. +application name `"netsuke"` and adjusts project-root discovery when the +`-C/--directory` flag is supplied. Explicit file selection is handled before +discovery by `resolve_config_path()`, which applies the precedence `--config` > +`NETSUKE_CONFIG` > `NETSUKE_CONFIG_PATH`. #### Discovery scopes and layered merging @@ -2194,9 +2195,12 @@ override earlier ones—meaning project-scope has highest precedence among file layers. After file layers are merged, environment variables and CLI arguments override the merged result, ensuring explicit user intent always wins. -1. **Explicit override**: `NETSUKE_CONFIG_PATH` environment variable, if set. - This allows users to point to any arbitrary configuration file path, - bypassing automatic discovery entirely. +1. **Explicit override**: the first configured selector from this list: + `--config `, `NETSUKE_CONFIG`, `NETSUKE_CONFIG_PATH`. This allows + users to point to any arbitrary configuration file path, bypassing automatic + discovery entirely. `NETSUKE_CONFIG_PATH` remains supported as a + backward-compatible alias, but `NETSUKE_CONFIG` is the documented + environment variable going forward. 2. **Project scope**: Configuration files in the current working directory (or the directory specified via `-C/--directory`): @@ -2255,10 +2259,15 @@ manual flag repetition. - Configuration files use TOML format by default. JSON5 (`.json`, `.json5`) and YAML (`.yaml`, `.yml`) formats are supported when the corresponding Cargo features are enabled. -- The current implementation uses `NETSUKE_CONFIG_PATH` for the override - environment variable. Roadmap item 3.11.3 plans to expose a visible - `--config` CLI flag and rename the environment variable to `NETSUKE_CONFIG` - for improved user ergonomics. +- Explicit config selection is handled outside OrthoConfig's built-in + discovery override surface because Netsuke needs its custom two-pass + project-over-user merge behaviour for automatic discovery. The selected file + still participates in the normal precedence ladder: defaults < file < + environment < CLI flags. +- 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 + scope project discovery and manifest lookup. ### 8.5 Manual Pages diff --git a/docs/roadmap.md b/docs/roadmap.md index a49bfcb4..60668f31 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -293,9 +293,9 @@ library, and CLI ergonomics. - [x] Add integration tests for each precedence tier. See [ortho-config-users-guide.md](ortho-config-users-guide.md) and `tests/cli_tests/config_discovery.rs`. -- [ ] 3.11.3. Expose `--config ` and `NETSUKE_CONFIG`. - - [ ] Select alternative config files. - - [ ] Ship annotated sample configs in documentation. +- [x] 3.11.3. Expose `--config ` and `NETSUKE_CONFIG`. + - [x] Select alternative config files. + - [x] Ship annotated sample configs in documentation. - [ ] 3.11.4. Add regression tests for OrthoConfig precedence ladder. - [ ] Test defaults < file < env < CLI precedence. See [ortho-config-users-guide.md](ortho-config-users-guide.md). diff --git a/docs/sample-netsuke.toml b/docs/sample-netsuke.toml new file mode 100644 index 00000000..865f087a --- /dev/null +++ b/docs/sample-netsuke.toml @@ -0,0 +1,63 @@ +# Sample Netsuke configuration +# +# Copy this file to `.netsuke.toml` in a project root, or point Netsuke at it +# with `--config /path/to/file.toml` or `NETSUKE_CONFIG=/path/to/file.toml`. +# +# All settings below are commented out so the file is valid as-is. Uncomment +# only the keys you want to change. + +# Path to the manifest file to use instead of `Netsukefile`. +# file = "Netsukefile" + +# Number of parallel build jobs. Valid range: 1-64. +# jobs = 8 + +# Enable verbose diagnostics and timing summaries. +# verbose = true + +# Locale for CLI messages, for example `en-US` or `es-ES`. +# locale = "en-US" + +# Force accessible output mode on or off. +# accessible = true + +# Suppress emoji glyphs in output. +# no_emoji = false + +# Theme preset: `auto`, `unicode`, or `ascii`. +# theme = "auto" + +# Colour output policy: `auto`, `always`, or `never`. +# colour_policy = "auto" + +# Force standard stage and task progress summaries on or off. +# progress = true + +# Spinner mode: `enabled` or `disabled`. +# spinner_mode = "enabled" + +# Emit machine-readable diagnostics on stderr. +# diag_json = false + +# Diagnostic output format: `human` or `json`. +# output_format = "human" + +# Default build targets when the CLI does not name any targets. +# default_targets = ["fmt", "lint", "test"] + +# Additional URL schemes allowed for the `fetch` helper. +# fetch_allow_scheme = ["https"] + +# Hostnames allowed when `fetch_default_deny = true`. +# Supports wildcards such as `*.example.com`. +# fetch_allow_host = ["example.com", "*.example.com"] + +# Hostnames that are always blocked. +# Supports wildcards such as `*.example.com`. +# fetch_block_host = ["metadata.internal"] + +# Deny all fetch hosts by default and allow only the declared allowlist. +# fetch_default_deny = false + +# CLI-only controls such as `--config`, `--directory`, and subcommands are not +# read from config files. diff --git a/docs/users-guide.md b/docs/users-guide.md index 80765388..ba02f772 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -548,10 +548,17 @@ command-line flags. #### Configuration file discovery -Configuration files are discovered using OrthoConfig. When -`NETSUKE_CONFIG_PATH` is set, it points to a single file and bypasses all -automatic discovery. Otherwise, Netsuke searches for configuration in three -scopes: +Configuration files are discovered using OrthoConfig unless you select an +explicit file. Netsuke honours these file-selection inputs in precedence order: + +- `--config ` +- `NETSUKE_CONFIG=` +- `NETSUKE_CONFIG_PATH=` (legacy alias) +- Automatic discovery + +When one of the explicit selectors is set, Netsuke loads only that file and +skips automatic discovery. Otherwise, Netsuke searches for configuration in +three scopes: - **Project scope** — `.netsuke.toml` in the current working directory (or the directory specified by `-C` / `--directory`). @@ -571,6 +578,10 @@ the project file overrides the same field from user or system files, user-scope settings override system-scope, and fields set only in lower-precedence files are still applied. +The explicit config path is resolved against the shell's current working +directory, not the `-C` / `--directory` project anchor. The directory flag only +changes where project-scope discovery and manifest lookup begin. + #### Directory flag and project anchoring The `-C ` / `--directory ` flag re-anchors project-scope discovery to @@ -585,6 +596,10 @@ netsuke -C /path/to/project build With the flag, only `/path/to/project/.netsuke.toml` is considered for project-scope discovery; user-scope discovery is unaffected. +For a documented starting point, see +[`docs/sample-netsuke.toml`](sample-netsuke.toml), which annotates every +supported config-file key. + #### Environment variables Environment variables use the `NETSUKE_` prefix (for example, diff --git a/locales/en-US/messages.ftl b/locales/en-US/messages.ftl index 05a4c703..4c374ead 100644 --- a/locales/en-US/messages.ftl +++ b/locales/en-US/messages.ftl @@ -7,6 +7,7 @@ cli.usage = { $usage } # Root-level flag help text. cli.flag.file.help = Path to the Netsuke manifest file to use. cli.flag.directory.help = Run as if started in this directory. +cli.flag.config.help = Path to a configuration file, bypassing automatic discovery. cli.flag.jobs.help = Set the number of parallel build jobs. cli.flag.verbose.help = Enable verbose diagnostic logging and completion timing summaries. cli.flag.locale.help = Locale tag for CLI copy (for example: en-US, es-ES). diff --git a/locales/es-ES/messages.ftl b/locales/es-ES/messages.ftl index 55041d14..a81f1edf 100644 --- a/locales/es-ES/messages.ftl +++ b/locales/es-ES/messages.ftl @@ -7,6 +7,7 @@ cli.usage = { $usage } # Texto de ayuda para opciones globales. cli.flag.file.help = Ruta al archivo de manifiesto Netsuke. cli.flag.directory.help = Ejecutar como si se iniciara en este directorio. +cli.flag.config.help = Ruta a un archivo de configuración, omitiendo la detección automática. cli.flag.jobs.help = Número de trabajos de compilación en paralelo. cli.flag.verbose.help = Habilitar registro de diagnóstico detallado y resúmenes de tiempo al completar. cli.flag.locale.help = Etiqueta de idioma para la CLI (por ejemplo: en-US, es-ES). diff --git a/src/cli/config_merge.rs b/src/cli/config_merge.rs index deaf18f4..ab9518a4 100644 --- a/src/cli/config_merge.rs +++ b/src/cli/config_merge.rs @@ -9,17 +9,18 @@ use ortho_config::declarative::LayerComposition; use ortho_config::figment::{Figment, providers::Env}; use ortho_config::uncased::Uncased; use ortho_config::{ - ConfigDiscovery, LocalizationArgs, MergeComposer, MergeLayer, OrthoMergeExt, OrthoResult, - load_config_file_as_chain, sanitize_value, + ConfigDiscovery, LocalizationArgs, MergeComposer, MergeLayer, OrthoError, OrthoMergeExt, + OrthoResult, load_config_file_as_chain, sanitize_value, }; use std::borrow::Cow; +use std::io; use std::path::{Path, PathBuf}; use std::sync::Arc; use crate::localization::{self, keys}; use super::config::OutputFormat; -use super::{CONFIG_ENV_VAR, Cli, ENV_PREFIX, validation_message}; +use super::{CONFIG_ENV_VAR, CONFIG_ENV_VAR_LEGACY, Cli, ENV_PREFIX, validation_message}; /// Return the default manifest filename when none is provided. pub(super) fn default_manifest_path() -> PathBuf { @@ -31,18 +32,46 @@ fn env_provider() -> Env { Env::prefixed(ENV_PREFIX) } -/// Build the single-pass discovery used when `NETSUKE_CONFIG_PATH` is set. -/// -/// When the env var is present `compose_layers` will find it first and return -/// immediately, so project-vs-user ordering is irrelevant. +/// Build the automatic discovery used when no explicit config path is set. fn config_discovery(directory: Option<&Path>) -> ConfigDiscovery { - let mut builder = ConfigDiscovery::builder("netsuke").env_var(CONFIG_ENV_VAR); + let mut builder = ConfigDiscovery::builder("netsuke"); if let Some(dir) = directory { builder = builder.clear_project_roots().add_project_root(dir); } builder.build() } +fn env_config_path(var_name: &str) -> Option { + std::env::var_os(var_name) + .filter(|value| !value.is_empty()) + .map(PathBuf::from) +} + +fn resolve_config_path(cli: &Cli) -> Option { + cli.config + .clone() + .or_else(|| env_config_path(CONFIG_ENV_VAR)) + .or_else(|| env_config_path(CONFIG_ENV_VAR_LEGACY)) +} + +fn load_layers_from_path(path: &Path) -> OrthoResult>> { + match load_config_file_as_chain(path) { + Ok(Some(chain)) => Ok(chain + .values + .into_iter() + .map(|(value, layer_path)| MergeLayer::file(Cow::Owned(value), Some(layer_path))) + .collect()), + Ok(None) => Err(Arc::new(OrthoError::File { + path: path.to_path_buf(), + source: Box::new(io::Error::new( + io::ErrorKind::NotFound, + "explicit configuration file not found", + )), + })), + Err(err) => Err(err), + } +} + /// Return the expected project-scope config file path as a string, if /// resolvable. fn project_scope_file_str(directory: Option<&Path>) -> Option { @@ -106,21 +135,26 @@ fn diag_json_from_layer(value: &serde_json::Value) -> Option { /// /// If project-scope layer loading fails, this function falls back to the first-pass /// layers (global and user configs) rather than propagating an error. An explicit -/// `NETSUKE_CONFIG_PATH` environment variable override will also cause the function -/// to return early with the first-pass layers only. -fn collect_diag_file_layers(directory: Option<&Path>) -> Vec> { - let discovery = config_discovery(directory); +/// explicit config path will also cause the function to return early with the +/// selected file only. +fn collect_diag_file_layers(cli: &Cli) -> Vec> { + if let Some(path) = resolve_config_path(cli) + && let Ok(layers) = load_layers_from_path(&path) + { + return layers; + } + + let discovery = config_discovery(cli.directory.as_deref()); let file_layers = discovery.compose_layers().value; - let project_file = project_scope_file_str(directory); + let project_file = project_scope_file_str(cli.directory.as_deref()); let first_pass_found_project = file_layers.iter().any(|l| { l.path() .is_some_and(|p| project_file.as_deref() == Some(p.as_str())) }); - let has_explicit_config = std::env::var_os(CONFIG_ENV_VAR).is_some_and(|v| !v.is_empty()); - if first_pass_found_project || has_explicit_config { + if first_pass_found_project { file_layers } else { - match project_scope_layers(directory) { + match project_scope_layers(cli.directory.as_deref()) { Ok(project_layers) => file_layers.into_iter().chain(project_layers).collect(), Err(_) => file_layers, } @@ -148,7 +182,7 @@ fn diag_json_from_matches(cli: &Cli, matches: &ArgMatches, discovered: bool) -> pub fn resolve_merged_diag_json(cli: &Cli, matches: &ArgMatches) -> bool { let mut diag_json = Cli::default().diag_json; - let layers = collect_diag_file_layers(cli.directory.as_deref()); + let layers = collect_diag_file_layers(cli); for layer in layers { let layer_value = layer.into_value(); if let Some(layer_diag_json) = diag_json_from_layer(&layer_value) { @@ -172,20 +206,32 @@ pub fn resolve_merged_diag_json(cli: &Cli, matches: &ArgMatches) -> bool { /// /// Implements "project scope > user scope" by running a second direct load of /// the project-scope file when first-pass discovery did not include it and no -/// explicit config-path override is active. +/// explicit config path is active. fn push_file_layers( composer: &mut MergeComposer, errors: &mut Vec>, - directory: Option<&Path>, + cli: &Cli, ) { - let discovery = config_discovery(directory); + if let Some(path) = resolve_config_path(cli) { + match load_layers_from_path(&path) { + Ok(layers) => { + for layer in layers { + composer.push_layer(layer); + } + } + Err(err) => errors.push(err), + } + return; + } + + let discovery = config_discovery(cli.directory.as_deref()); let mut file_layers = discovery.compose_layers(); errors.append(&mut file_layers.required_errors); if file_layers.value.is_empty() { errors.append(&mut file_layers.optional_errors); } - let project_file = project_scope_file_str(directory); + let project_file = project_scope_file_str(cli.directory.as_deref()); let first_pass_found_project = file_layers.value.iter().any(|l| { l.path() .is_some_and(|p| project_file.as_deref() == Some(p.as_str())) @@ -195,9 +241,8 @@ fn push_file_layers( composer.push_layer(layer); } - let has_explicit_config = std::env::var_os(CONFIG_ENV_VAR).is_some_and(|v| !v.is_empty()); - if !first_pass_found_project && !has_explicit_config { - match project_scope_layers(directory) { + if !first_pass_found_project { + match project_scope_layers(cli.directory.as_deref()) { Ok(layers) => { for layer in layers { composer.push_layer(layer); @@ -271,7 +316,7 @@ pub fn merge_with_config(cli: &Cli, matches: &ArgMatches) -> OrthoResult { Err(err) => errors.push(err), } - push_file_layers(&mut composer, &mut errors, cli.directory.as_deref()); + push_file_layers(&mut composer, &mut errors, cli); let env_provider = env_provider() .map(|key| Uncased::new(key.as_str().to_ascii_uppercase())) diff --git a/src/cli/config_merge_tests.rs b/src/cli/config_merge_tests.rs index e3dc6902..76d13b06 100644 --- a/src/cli/config_merge_tests.rs +++ b/src/cli/config_merge_tests.rs @@ -8,6 +8,13 @@ use serde_json::json; use tempfile::tempdir; use test_support::{CwdGuard, EnvVarGuard}; +fn cli_with_directory(directory: &std::path::Path) -> Cli { + Cli { + directory: Some(directory.to_path_buf()), + ..Cli::default() + } +} + // --------------------------------------------------------------------------- // is_empty_value // --------------------------------------------------------------------------- @@ -156,7 +163,7 @@ fn project_scope_layers_returns_one_layer_when_file_present() { /// /// Returns (`EnvLock`, `project_dir`, `fake_home`, `env_guards`) where `env_guards` /// isolate `HOME` and platform-specific config paths (`XDG_CONFIG_HOME` on Unix, -/// `APPDATA`/`LOCALAPPDATA` on Windows) and remove `CONFIG_ENV_VAR`. +/// `APPDATA`/`LOCALAPPDATA` on Windows) and remove explicit config selectors. #[cfg(test)] #[fixture] fn isolated_config_env() -> anyhow::Result<( @@ -175,6 +182,7 @@ fn isolated_config_env() -> anyhow::Result<( EnvVarGuard::set("HOME", fake_home.path().as_os_str()), EnvVarGuard::set("XDG_CONFIG_HOME", fake_home.path().as_os_str()), EnvVarGuard::remove(CONFIG_ENV_VAR), + EnvVarGuard::remove(CONFIG_ENV_VAR_LEGACY), ]; #[cfg(windows)] @@ -183,6 +191,7 @@ fn isolated_config_env() -> anyhow::Result<( EnvVarGuard::set("APPDATA", fake_home.path().as_os_str()), EnvVarGuard::set("LOCALAPPDATA", fake_home.path().as_os_str()), EnvVarGuard::remove(CONFIG_ENV_VAR), + EnvVarGuard::remove(CONFIG_ENV_VAR_LEGACY), ]; Ok((lock, dir, fake_home, guards)) @@ -208,7 +217,8 @@ fn collect_diag_file_layers_handles_project_file_presence( .expect("write config"); } - let layers = collect_diag_file_layers(Some(dir.path())); + let cli = cli_with_directory(dir.path()); + let layers = collect_diag_file_layers(&cli); if expect_empty { ensure!(layers.is_empty(), "expected no layers when file absent"); @@ -270,7 +280,9 @@ fn push_file_layers_pushes_expected_layer_count( #[cfg(windows)] let _local_appdata_guard = EnvVarGuard::set("LOCALAPPDATA", fake_home.path().as_os_str()); let _config_guard = EnvVarGuard::remove(CONFIG_ENV_VAR); - push_file_layers(&mut composer, &mut errors, Some(dir.path())); + let _legacy_config_guard = EnvVarGuard::remove(CONFIG_ENV_VAR_LEGACY); + let cli = cli_with_directory(dir.path()); + push_file_layers(&mut composer, &mut errors, &cli); assert!(errors.is_empty(), "no required errors expected"); assert_eq!( composer.layers().len(), diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 464d928e..a9a77c1c 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -25,7 +25,8 @@ use validation::configure_validation_parsers; /// Maximum number of jobs accepted by the CLI. const MAX_JOBS: usize = 64; -const CONFIG_ENV_VAR: &str = "NETSUKE_CONFIG_PATH"; +const CONFIG_ENV_VAR: &str = "NETSUKE_CONFIG"; +const CONFIG_ENV_VAR_LEGACY: &str = "NETSUKE_CONFIG_PATH"; const ENV_PREFIX: &str = "NETSUKE_"; fn validation_message( @@ -53,6 +54,11 @@ pub struct Cli { #[arg(short = 'C', long, value_name = "DIR")] pub directory: Option, + /// Path to a configuration file, bypassing automatic discovery. + #[arg(long, value_name = "FILE")] + #[serde(skip)] + pub config: Option, + /// Set the number of parallel build jobs. /// /// Values must be between 1 and 64. @@ -160,6 +166,7 @@ impl Default for Cli { Self { file: default_manifest_path(), directory: None, + config: None, jobs: None, verbose: false, locale: None, diff --git a/src/cli_l10n.rs b/src/cli_l10n.rs index cf5d9633..61b45fe2 100644 --- a/src/cli_l10n.rs +++ b/src/cli_l10n.rs @@ -124,6 +124,7 @@ fn flag_help_key(arg_id: &str, subcommand_name: Option<&str>) -> Option<&'static None => match arg_id { "file" => Some(keys::CLI_FLAG_FILE_HELP), "directory" => Some(keys::CLI_FLAG_DIRECTORY_HELP), + "config" => Some(keys::CLI_FLAG_CONFIG_HELP), "jobs" => Some(keys::CLI_FLAG_JOBS_HELP), "verbose" => Some(keys::CLI_FLAG_VERBOSE_HELP), "locale" => Some(keys::CLI_FLAG_LOCALE_HELP), diff --git a/src/localization/keys.rs b/src/localization/keys.rs index 65adaca4..309637f9 100644 --- a/src/localization/keys.rs +++ b/src/localization/keys.rs @@ -14,6 +14,7 @@ define_keys! { CLI_USAGE => "cli.usage", CLI_FLAG_FILE_HELP => "cli.flag.file.help", CLI_FLAG_DIRECTORY_HELP => "cli.flag.directory.help", + CLI_FLAG_CONFIG_HELP => "cli.flag.config.help", CLI_FLAG_JOBS_HELP => "cli.flag.jobs.help", CLI_FLAG_VERBOSE_HELP => "cli.flag.verbose.help", CLI_FLAG_LOCALE_HELP => "cli.flag.locale.help", diff --git a/tests/bdd/steps/cli_parsing.rs b/tests/bdd/steps/cli_parsing.rs index 31778fa3..61117c7f 100644 --- a/tests/bdd/steps/cli_parsing.rs +++ b/tests/bdd/steps/cli_parsing.rs @@ -46,6 +46,7 @@ fn isolated_cli_environment(world: &TestWorld) -> Result<()> { // Clear all NETSUKE_* environment variables to prevent interference let netsuke_vars = [ + "NETSUKE_CONFIG", "NETSUKE_CONFIG_PATH", "NETSUKE_THEME", "NETSUKE_LOCALE", diff --git a/tests/cli_tests/config_selection.rs b/tests/cli_tests/config_selection.rs new file mode 100644 index 00000000..c003ede9 --- /dev/null +++ b/tests/cli_tests/config_selection.rs @@ -0,0 +1,227 @@ +//! Integration tests for explicit configuration file selection. +//! +//! These tests cover the visible `--config` flag and `NETSUKE_CONFIG` +//! environment variable, plus compatibility with the legacy +//! `NETSUKE_CONFIG_PATH` override. + +use anyhow::{Context, Result, ensure}; +use netsuke::cli_localization; +use netsuke::theme::ThemePreference; +use rstest::rstest; +use std::ffi::OsStr; +use std::fs; +use std::sync::Arc; +use tempfile::tempdir; +use test_support::{EnvVarGuard, env_lock::EnvLock}; + +/// RAII guard that restores the process working directory on drop. +struct CwdGuard(std::path::PathBuf); + +impl CwdGuard { + fn acquire() -> Result { + Ok(Self( + std::env::current_dir().context("capture current working directory")?, + )) + } +} + +impl Drop for CwdGuard { + fn drop(&mut self) { + drop(std::env::set_current_dir(&self.0)); + } +} + +fn parse_and_merge(args: &[&str]) -> Result { + let localizer = Arc::from(cli_localization::build_localizer(None)); + let (cli, matches) = netsuke::cli::parse_with_localizer_from(args, &localizer) + .context("parse CLI for config selection test")?; + netsuke::cli::merge_with_config(&cli, &matches) + .context("merge CLI with selected config")? + .with_default_command() + .pipe(Ok) +} + +trait Pipe: Sized { + fn pipe(self, f: impl FnOnce(Self) -> T) -> T { + f(self) + } +} + +impl Pipe for T {} + +fn sandbox_user_scope(home: &tempfile::TempDir) -> Result<(EnvVarGuard, EnvVarGuard, EnvVarGuard)> { + let xdg_config_home = home.path().join(".config"); + fs::create_dir_all(&xdg_config_home).context("create sandboxed XDG config home")?; + Ok(( + EnvVarGuard::set("HOME", home.path().as_os_str()), + EnvVarGuard::set("XDG_CONFIG_HOME", xdg_config_home.as_os_str()), + EnvVarGuard::set("XDG_CONFIG_DIRS", OsStr::new("")), + )) +} + +#[rstest] +fn config_flag_loads_specified_file() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let _cwd_guard = CwdGuard::acquire()?; + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create fake home directory")?; + let _user_scope = sandbox_user_scope(&home)?; + let _config_guard = EnvVarGuard::remove("NETSUKE_CONFIG"); + let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); + let _theme_guard = EnvVarGuard::remove("NETSUKE_THEME"); + + let custom = project.path().join("custom.toml"); + fs::write(&custom, "theme = \"unicode\"\n").context("write explicit config file")?; + std::env::set_current_dir(project.path()).context("change to project directory")?; + + let merged = parse_and_merge(&["netsuke", "--config", "custom.toml"])?; + ensure!( + merged.theme == Some(ThemePreference::Unicode), + "explicit --config file should be loaded" + ); + Ok(()) +} + +#[rstest] +fn config_flag_skips_project_discovery() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let _cwd_guard = CwdGuard::acquire()?; + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create fake home directory")?; + let _user_scope = sandbox_user_scope(&home)?; + let _config_guard = EnvVarGuard::remove("NETSUKE_CONFIG"); + let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); + + fs::write(project.path().join(".netsuke.toml"), "theme = \"ascii\"\n") + .context("write project config")?; + fs::write(project.path().join("custom.toml"), "theme = \"unicode\"\n") + .context("write custom config")?; + std::env::set_current_dir(project.path()).context("change to project directory")?; + + let merged = parse_and_merge(&["netsuke", "--config", "custom.toml"])?; + ensure!( + merged.theme == Some(ThemePreference::Unicode), + "explicit --config should bypass discovered project config" + ); + Ok(()) +} + +#[rstest] +fn config_flag_with_nonexistent_file_produces_error() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let _cwd_guard = CwdGuard::acquire()?; + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create fake home directory")?; + let _user_scope = sandbox_user_scope(&home)?; + let _config_guard = EnvVarGuard::remove("NETSUKE_CONFIG"); + let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); + std::env::set_current_dir(project.path()).context("change to project directory")?; + + let error = parse_and_merge(&["netsuke", "--config", "missing.toml"]) + .expect_err("missing explicit config file should fail"); + let message = format!("{error:?}"); + ensure!( + message.contains("missing.toml"), + "error should mention the missing explicit config path, got {message}" + ); + Ok(()) +} + +#[rstest] +fn netsuke_config_env_loads_specified_file() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let _cwd_guard = CwdGuard::acquire()?; + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create fake home directory")?; + let _user_scope = sandbox_user_scope(&home)?; + let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); + + let custom = project.path().join("env.toml"); + fs::write(&custom, "theme = \"unicode\"\n").context("write NETSUKE_CONFIG file")?; + let _config_guard = EnvVarGuard::set("NETSUKE_CONFIG", custom.as_os_str()); + std::env::set_current_dir(project.path()).context("change to project directory")?; + + let merged = parse_and_merge(&["netsuke"])?; + ensure!( + merged.theme == Some(ThemePreference::Unicode), + "NETSUKE_CONFIG should load the selected config file" + ); + Ok(()) +} + +#[rstest] +fn netsuke_config_env_takes_precedence_over_legacy() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let _cwd_guard = CwdGuard::acquire()?; + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create fake home directory")?; + let _user_scope = sandbox_user_scope(&home)?; + + let new_config = project.path().join("new.toml"); + let legacy_config = project.path().join("legacy.toml"); + fs::write(&new_config, "theme = \"unicode\"\n").context("write NETSUKE_CONFIG file")?; + fs::write(&legacy_config, "theme = \"ascii\"\n").context("write legacy config file")?; + let _config_guard = EnvVarGuard::set("NETSUKE_CONFIG", new_config.as_os_str()); + let _legacy_guard = EnvVarGuard::set("NETSUKE_CONFIG_PATH", legacy_config.as_os_str()); + std::env::set_current_dir(project.path()).context("change to project directory")?; + + let merged = parse_and_merge(&["netsuke"])?; + ensure!( + merged.theme == Some(ThemePreference::Unicode), + "NETSUKE_CONFIG should win over NETSUKE_CONFIG_PATH" + ); + Ok(()) +} + +#[rstest] +fn config_flag_takes_precedence_over_netsuke_config_env() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let _cwd_guard = CwdGuard::acquire()?; + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create fake home directory")?; + let _user_scope = sandbox_user_scope(&home)?; + + let cli_config = project.path().join("cli.toml"); + let env_config = project.path().join("env.toml"); + fs::write(&cli_config, "theme = \"unicode\"\n").context("write CLI-selected config")?; + fs::write(&env_config, "theme = \"ascii\"\n").context("write env-selected config")?; + let _config_guard = EnvVarGuard::set("NETSUKE_CONFIG", env_config.as_os_str()); + let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); + std::env::set_current_dir(project.path()).context("change to project directory")?; + + let merged = parse_and_merge(&["netsuke", "--config", "cli.toml"])?; + ensure!( + merged.theme == Some(ThemePreference::Unicode), + "--config should win over NETSUKE_CONFIG" + ); + Ok(()) +} + +#[rstest] +fn config_flag_values_still_overridden_by_env_and_cli_preferences() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let _cwd_guard = CwdGuard::acquire()?; + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create fake home directory")?; + let _user_scope = sandbox_user_scope(&home)?; + let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); + let _theme_guard = EnvVarGuard::set("NETSUKE_THEME", "unicode"); + + let custom = project.path().join("custom.toml"); + fs::write(&custom, "theme = \"ascii\"\n").context("write explicit config file")?; + std::env::set_current_dir(project.path()).context("change to project directory")?; + + let merged_with_cli_override = + parse_and_merge(&["netsuke", "--config", "custom.toml", "--theme", "ascii"])?; + ensure!( + merged_with_cli_override.theme == Some(ThemePreference::Ascii), + "CLI preference values should still override environment and selected config" + ); + + let merged_with_env_override = parse_and_merge(&["netsuke", "--config", "custom.toml"])?; + ensure!( + merged_with_env_override.theme == Some(ThemePreference::Unicode), + "environment preference values should still override the selected config" + ); + Ok(()) +} diff --git a/tests/cli_tests/mod.rs b/tests/cli_tests/mod.rs index d400b542..aa34140b 100644 --- a/tests/cli_tests/mod.rs +++ b/tests/cli_tests/mod.rs @@ -3,6 +3,7 @@ //! This module exercises the command-line interface defined in `netsuke::cli`. mod config_discovery; +mod config_selection; mod helpers; mod locale; mod merge; diff --git a/tests/features/configuration_discovery.feature b/tests/features/configuration_discovery.feature index 90ae21da..88da2d0f 100644 --- a/tests/features/configuration_discovery.feature +++ b/tests/features/configuration_discovery.feature @@ -43,3 +43,41 @@ Feature: Configuration file discovery and precedence When the CLI is parsed with no additional arguments Then parsing succeeds And the theme preference is "unicode" + + Scenario: Explicit config file overrides project discovery + Given a temporary workspace + And a project config file ".netsuke.toml" with theme "ascii" + And a custom config file "custom.toml" with theme "unicode" + When the CLI is parsed with "--config custom.toml" + Then parsing succeeds + And the theme preference is "unicode" + + Scenario: NETSUKE_CONFIG environment variable selects config file + Given a temporary workspace + And a project config file ".netsuke.toml" with theme "ascii" + And a custom config file "override.toml" with theme "unicode" + And the environment variable "NETSUKE_CONFIG" points to "override.toml" + When the CLI is parsed with no additional arguments + Then parsing succeeds + And the theme preference is "unicode" + + Scenario: NETSUKE_CONFIG takes precedence over NETSUKE_CONFIG_PATH + Given a temporary workspace + And a project config file ".netsuke.toml" with theme "ascii" + And a custom config file "new.toml" with theme "unicode" + And a custom config file "legacy.toml" with theme "ascii" + And the environment variable "NETSUKE_CONFIG" points to "new.toml" + And the environment variable "NETSUKE_CONFIG_PATH" points to "legacy.toml" + When the CLI is parsed with no additional arguments + Then parsing succeeds + And the theme preference is "unicode" + + Scenario: CLI config flag takes precedence over NETSUKE_CONFIG + Given a temporary workspace + And a project config file ".netsuke.toml" with theme "ascii" + And a custom config file "cli.toml" with theme "unicode" + And a custom config file "env.toml" with theme "ascii" + And the environment variable "NETSUKE_CONFIG" points to "env.toml" + When the CLI is parsed with "--config cli.toml" + Then parsing succeeds + And the theme preference is "unicode" From 22ea03b268d653e2b815226724225d94225c776f Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 21 Apr 2026 10:54:48 +0000 Subject: [PATCH 3/3] refactor(tests): introduce ConfigTestHarness to reduce test setup duplication - Added ConfigTestHarness struct to unify repeated env lock, temp dirs, cwd guard, and env var setup in CLI integration tests. - Updated tests in cli_tests/config_selection.rs and related modules to use ConfigTestHarness, improving readability and maintainability. - Fixed test cwd teardown hazards by properly dropping guards after assertions, stabilizing parallel test execution. - Simplified load_layers_from_path calls in config_merge.rs with helper push_layers_result for cleaner error handling. - Documented observations and decisions related to tests and config path handling in docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md. These changes improve test suite stability and code clarity without altering external behavior. Co-authored-by: devboxerhub[bot] --- ...3-expose-config-path-and-netsuke-config.md | 28 ++++ src/cli/config_merge.rs | 39 ++--- tests/cli_tests/config_discovery.rs | 45 ++++-- tests/cli_tests/config_selection.rs | 134 +++++++++--------- tests/cli_tests/merge.rs | 3 +- 5 files changed, 150 insertions(+), 99 deletions(-) diff --git a/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md b/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md index 49644e47..feb03c46 100644 --- a/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md +++ b/docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md @@ -155,6 +155,26 @@ Observable success means all of the following hold simultaneously: output rather than only the top-level context string. Date/Author: 2026-04-20 / implementation agent. +- Observation: the new integration coverage in + `tests/cli_tests/config_selection.rs` accumulated repeated process-level test + setup (`EnvLock`, temporary directories, sandboxed user scope, and working + directory changes). A small file-local `ConfigTestHarness` keeps those tests + under the project's readability thresholds without changing behaviour. + Date/Author: 2026-04-21 / implementation agent. + +- Observation: for test harnesses that both `chdir` into a temporary + directory and own that directory, struct field order matters because Rust + drops fields in reverse declaration order. The cwd-restoration guard must be + declared after the temporary directories so it drops first during teardown. + Date/Author: 2026-04-21 / implementation agent. + +- Observation: the `cli_tests` integration target was still flaky when the + new `ConfigTestHarness` tests passed relative `--config custom.toml` paths + under the full parallel suite. Using the helper's returned absolute paths in + the flag-based tests removed that cwd sensitivity while preserving the + precedence behaviour under test. Date/Author: 2026-04-21 / implementation + agent. + ## Decision log - Decision: use a plain `config: Option` field on `Cli` with @@ -198,6 +218,8 @@ Observable success means all of the following hold simultaneously: Implementation completed on 2026-04-20. +Follow-on maintenance completed on 2026-04-21. + Validation evidence: - `make fmt` @@ -219,6 +241,12 @@ Key outcomes: - Added focused integration coverage in `tests/cli_tests/config_selection.rs` for CLI/env precedence, missing-file handling, and continued value-level precedence over the selected file. +- Refactored `tests/cli_tests/config_selection.rs` to use a local + `ConfigTestHarness`, removing duplicated environment and working-directory + setup while preserving the existing test names, assertions, and merge calls. +- Fixed a separate cwd teardown hazard in + `tests/cli_tests/merge.rs::resolve_merged_diag_json_handles_malformed_project_config` + so the full integration suite remains stable after the harness refactor. - Extended the configuration-discovery BDD feature with scenarios for `--config`, `NETSUKE_CONFIG`, and precedence over the legacy alias. - Added `docs/sample-netsuke.toml` and updated the user guide, design diff --git a/src/cli/config_merge.rs b/src/cli/config_merge.rs index ab9518a4..4b7e3689 100644 --- a/src/cli/config_merge.rs +++ b/src/cli/config_merge.rs @@ -207,20 +207,30 @@ pub fn resolve_merged_diag_json(cli: &Cli, matches: &ArgMatches) -> bool { /// Implements "project scope > user scope" by running a second direct load of /// the project-scope file when first-pass discovery did not include it and no /// explicit config path is active. +/// +/// Drain a layer-load result onto `composer`, recording any error. +fn push_layers_result( + composer: &mut MergeComposer, + errors: &mut Vec>, + result: Result>, Arc>, +) { + match result { + Ok(layers) => { + for layer in layers { + composer.push_layer(layer); + } + } + Err(err) => errors.push(err), + } +} + fn push_file_layers( composer: &mut MergeComposer, errors: &mut Vec>, cli: &Cli, ) { if let Some(path) = resolve_config_path(cli) { - match load_layers_from_path(&path) { - Ok(layers) => { - for layer in layers { - composer.push_layer(layer); - } - } - Err(err) => errors.push(err), - } + push_layers_result(composer, errors, load_layers_from_path(&path)); return; } @@ -242,14 +252,11 @@ fn push_file_layers( } if !first_pass_found_project { - match project_scope_layers(cli.directory.as_deref()) { - Ok(layers) => { - for layer in layers { - composer.push_layer(layer); - } - } - Err(err) => errors.push(err), - } + push_layers_result( + composer, + errors, + project_scope_layers(cli.directory.as_deref()), + ); } } diff --git a/tests/cli_tests/config_discovery.rs b/tests/cli_tests/config_discovery.rs index df76b6ab..13c0ce10 100644 --- a/tests/cli_tests/config_discovery.rs +++ b/tests/cli_tests/config_discovery.rs @@ -37,7 +37,7 @@ impl Drop for CwdGuard { #[rstest] fn project_scope_config_discovered_automatically() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_dir = tempdir().context("create temporary project directory")?; let project_config = temp_dir.path().join(".netsuke.toml"); @@ -80,6 +80,7 @@ jobs = 8 merged.jobs == Some(8), "project config jobs should be discovered" ); + drop(cwd_guard); Ok(()) } @@ -110,7 +111,7 @@ fn assert_user_config_applied(merged: &netsuke::cli::Cli) -> Result<()> { #[rstest] fn user_scope_config_discovered_when_no_project_config() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_project = tempdir().context("create temporary project directory")?; let temp_home = tempdir().context("create temporary home directory")?; @@ -141,14 +142,16 @@ fn user_scope_config_discovered_when_no_project_config() -> Result<()> { .context("merge with user config")? .with_default_command(); - assert_user_config_applied(&merged) + let result = assert_user_config_applied(&merged); + drop(cwd_guard); + result } #[cfg(windows)] #[rstest] fn user_scope_config_discovered_when_no_project_config() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_project = tempdir().context("create temporary project directory")?; let temp_appdata = tempdir().context("create temporary APPDATA directory")?; @@ -179,7 +182,9 @@ fn user_scope_config_discovered_when_no_project_config() -> Result<()> { .context("merge with user config")? .with_default_command(); - assert_user_config_applied(&merged) + let result = assert_user_config_applied(&merged); + drop(cwd_guard); + result } /// Project config TOML used by both Unix and Windows precedence test variants. @@ -214,7 +219,7 @@ fn assert_project_precedence_applied(merged: &netsuke::cli::Cli) -> Result<()> { #[rstest] fn project_config_takes_precedence_over_user_config() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_project = tempdir().context("create temporary project directory")?; let temp_home = tempdir().context("create temporary home directory")?; @@ -251,14 +256,16 @@ fn project_config_takes_precedence_over_user_config() -> Result<()> { .context("merge configs")? .with_default_command(); - assert_project_precedence_applied(&merged) + let result = assert_project_precedence_applied(&merged); + drop(cwd_guard); + result } #[cfg(windows)] #[rstest] fn project_config_takes_precedence_over_user_config() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_project = tempdir().context("create temporary project directory")?; let temp_appdata = tempdir().context("create temporary APPDATA directory")?; @@ -295,13 +302,15 @@ fn project_config_takes_precedence_over_user_config() -> Result<()> { .context("merge configs")? .with_default_command(); - assert_project_precedence_applied(&merged) + let result = assert_project_precedence_applied(&merged); + drop(cwd_guard); + result } #[rstest] fn environment_variables_override_discovered_config() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_project = tempdir().context("create temporary project directory")?; // Write project-scope config @@ -344,13 +353,14 @@ output_format = "human" merged.output_format == Some(OutputFormat::Human), "project config value should apply when no env override exists" ); + drop(cwd_guard); Ok(()) } #[rstest] fn cli_flags_override_environment_and_config() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_project = tempdir().context("create temporary project directory")?; // Write project-scope config @@ -409,6 +419,7 @@ output_format = "human" merged.colour_policy == Some(ColourPolicy::Always), "environment colour_policy should apply when CLI does not override" ); + drop(cwd_guard); Ok(()) } @@ -417,7 +428,7 @@ output_format = "human" #[case("--directory")] fn directory_flag_anchors_project_discovery_to_specified_dir(#[case] flag: &str) -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_outer = tempdir().context("create outer directory")?; let temp_project = temp_outer.path().join("project"); fs::create_dir(&temp_project).context("create project subdirectory")?; @@ -458,13 +469,14 @@ jobs = 6 merged.jobs == Some(6), "config values from directory flag should be applied" ); + drop(cwd_guard); Ok(()) } #[rstest] fn config_path_env_var_bypasses_automatic_discovery() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_project = tempdir().context("create project directory")?; let temp_custom = tempdir().context("create custom config directory")?; @@ -518,6 +530,7 @@ colour_policy = "always" merged.colour_policy == Some(ColourPolicy::Always), "custom config colour_policy should be applied" ); + drop(cwd_guard); Ok(()) } @@ -573,7 +586,7 @@ fn assert_list_fields_appended(merged: &netsuke::cli::Cli) -> Result<()> { #[rstest] fn list_fields_append_across_discovered_config_env_and_cli() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_project = tempdir().context("create project directory")?; // Write project config with default_targets @@ -610,5 +623,7 @@ fetch_allow_scheme = ["https"] .context("merge with list appending")? .with_default_command(); - assert_list_fields_appended(&merged) + let result = assert_list_fields_appended(&merged); + drop(cwd_guard); + result } diff --git a/tests/cli_tests/config_selection.rs b/tests/cli_tests/config_selection.rs index c003ede9..876bb4ef 100644 --- a/tests/cli_tests/config_selection.rs +++ b/tests/cli_tests/config_selection.rs @@ -59,63 +59,82 @@ fn sandbox_user_scope(home: &tempfile::TempDir) -> Result<(EnvVarGuard, EnvVarGu )) } +struct ConfigTestHarness { + _env_lock: EnvLock, + project: tempfile::TempDir, + _home: tempfile::TempDir, + _user_scope: (EnvVarGuard, EnvVarGuard, EnvVarGuard), + _cwd_guard: CwdGuard, +} + +impl ConfigTestHarness { + fn setup() -> Result { + let env_lock = EnvLock::acquire(); + let cwd_guard = CwdGuard::acquire()?; + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create fake home directory")?; + let user_scope = sandbox_user_scope(&home)?; + std::env::set_current_dir(project.path()).context("change to project directory")?; + Ok(Self { + _env_lock: env_lock, + project, + _home: home, + _user_scope: user_scope, + _cwd_guard: cwd_guard, + }) + } + + fn write_config(&self, name: &str, content: &str) -> Result { + let path = self.project.path().join(name); + fs::write(&path, content).with_context(|| format!("write config file {name}"))?; + std::env::set_current_dir(self.project.path()).context("change to project directory")?; + Ok(path) + } +} + #[rstest] fn config_flag_loads_specified_file() -> Result<()> { - let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire()?; - let project = tempdir().context("create project directory")?; - let home = tempdir().context("create fake home directory")?; - let _user_scope = sandbox_user_scope(&home)?; + let h = ConfigTestHarness::setup()?; let _config_guard = EnvVarGuard::remove("NETSUKE_CONFIG"); let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); let _theme_guard = EnvVarGuard::remove("NETSUKE_THEME"); - let custom = project.path().join("custom.toml"); - fs::write(&custom, "theme = \"unicode\"\n").context("write explicit config file")?; - std::env::set_current_dir(project.path()).context("change to project directory")?; + let custom_path = h.write_config("custom.toml", "theme = \"unicode\"\n")?; + let custom_arg = custom_path.to_string_lossy().into_owned(); - let merged = parse_and_merge(&["netsuke", "--config", "custom.toml"])?; + let merged = parse_and_merge(&["netsuke", "--config", &custom_arg])?; ensure!( merged.theme == Some(ThemePreference::Unicode), "explicit --config file should be loaded" ); + let _project_root = h.project.path(); Ok(()) } #[rstest] fn config_flag_skips_project_discovery() -> Result<()> { - let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire()?; - let project = tempdir().context("create project directory")?; - let home = tempdir().context("create fake home directory")?; - let _user_scope = sandbox_user_scope(&home)?; + let h = ConfigTestHarness::setup()?; let _config_guard = EnvVarGuard::remove("NETSUKE_CONFIG"); let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); - fs::write(project.path().join(".netsuke.toml"), "theme = \"ascii\"\n") - .context("write project config")?; - fs::write(project.path().join("custom.toml"), "theme = \"unicode\"\n") - .context("write custom config")?; - std::env::set_current_dir(project.path()).context("change to project directory")?; + let _project_config = h.write_config(".netsuke.toml", "theme = \"ascii\"\n")?; + let custom_path = h.write_config("custom.toml", "theme = \"unicode\"\n")?; + let custom_arg = custom_path.to_string_lossy().into_owned(); - let merged = parse_and_merge(&["netsuke", "--config", "custom.toml"])?; + let merged = parse_and_merge(&["netsuke", "--config", &custom_arg])?; ensure!( merged.theme == Some(ThemePreference::Unicode), "explicit --config should bypass discovered project config" ); + let _project_root = h.project.path(); Ok(()) } #[rstest] fn config_flag_with_nonexistent_file_produces_error() -> Result<()> { - let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire()?; - let project = tempdir().context("create project directory")?; - let home = tempdir().context("create fake home directory")?; - let _user_scope = sandbox_user_scope(&home)?; + let h = ConfigTestHarness::setup()?; let _config_guard = EnvVarGuard::remove("NETSUKE_CONFIG"); let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); - std::env::set_current_dir(project.path()).context("change to project directory")?; let error = parse_and_merge(&["netsuke", "--config", "missing.toml"]) .expect_err("missing explicit config file should fail"); @@ -124,104 +143,85 @@ fn config_flag_with_nonexistent_file_produces_error() -> Result<()> { message.contains("missing.toml"), "error should mention the missing explicit config path, got {message}" ); + let _project_root = h.project.path(); Ok(()) } #[rstest] fn netsuke_config_env_loads_specified_file() -> Result<()> { - let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire()?; - let project = tempdir().context("create project directory")?; - let home = tempdir().context("create fake home directory")?; - let _user_scope = sandbox_user_scope(&home)?; + let h = ConfigTestHarness::setup()?; let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); - let custom = project.path().join("env.toml"); - fs::write(&custom, "theme = \"unicode\"\n").context("write NETSUKE_CONFIG file")?; + let custom = h.write_config("env.toml", "theme = \"unicode\"\n")?; let _config_guard = EnvVarGuard::set("NETSUKE_CONFIG", custom.as_os_str()); - std::env::set_current_dir(project.path()).context("change to project directory")?; let merged = parse_and_merge(&["netsuke"])?; ensure!( merged.theme == Some(ThemePreference::Unicode), "NETSUKE_CONFIG should load the selected config file" ); + let _project_root = h.project.path(); Ok(()) } #[rstest] fn netsuke_config_env_takes_precedence_over_legacy() -> Result<()> { - let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire()?; - let project = tempdir().context("create project directory")?; - let home = tempdir().context("create fake home directory")?; - let _user_scope = sandbox_user_scope(&home)?; - - let new_config = project.path().join("new.toml"); - let legacy_config = project.path().join("legacy.toml"); - fs::write(&new_config, "theme = \"unicode\"\n").context("write NETSUKE_CONFIG file")?; - fs::write(&legacy_config, "theme = \"ascii\"\n").context("write legacy config file")?; + let h = ConfigTestHarness::setup()?; + + let new_config = h.write_config("new.toml", "theme = \"unicode\"\n")?; + let legacy_config = h.write_config("legacy.toml", "theme = \"ascii\"\n")?; let _config_guard = EnvVarGuard::set("NETSUKE_CONFIG", new_config.as_os_str()); let _legacy_guard = EnvVarGuard::set("NETSUKE_CONFIG_PATH", legacy_config.as_os_str()); - std::env::set_current_dir(project.path()).context("change to project directory")?; let merged = parse_and_merge(&["netsuke"])?; ensure!( merged.theme == Some(ThemePreference::Unicode), "NETSUKE_CONFIG should win over NETSUKE_CONFIG_PATH" ); + let _project_root = h.project.path(); Ok(()) } #[rstest] fn config_flag_takes_precedence_over_netsuke_config_env() -> Result<()> { - let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire()?; - let project = tempdir().context("create project directory")?; - let home = tempdir().context("create fake home directory")?; - let _user_scope = sandbox_user_scope(&home)?; - - let cli_config = project.path().join("cli.toml"); - let env_config = project.path().join("env.toml"); - fs::write(&cli_config, "theme = \"unicode\"\n").context("write CLI-selected config")?; - fs::write(&env_config, "theme = \"ascii\"\n").context("write env-selected config")?; + let h = ConfigTestHarness::setup()?; + + let cli_config_path = h.write_config("cli.toml", "theme = \"unicode\"\n")?; + let cli_config_arg = cli_config_path.to_string_lossy().into_owned(); + let env_config = h.write_config("env.toml", "theme = \"ascii\"\n")?; let _config_guard = EnvVarGuard::set("NETSUKE_CONFIG", env_config.as_os_str()); let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); - std::env::set_current_dir(project.path()).context("change to project directory")?; - let merged = parse_and_merge(&["netsuke", "--config", "cli.toml"])?; + let merged = parse_and_merge(&["netsuke", "--config", &cli_config_arg])?; ensure!( merged.theme == Some(ThemePreference::Unicode), "--config should win over NETSUKE_CONFIG" ); + let _project_root = h.project.path(); Ok(()) } #[rstest] fn config_flag_values_still_overridden_by_env_and_cli_preferences() -> Result<()> { - let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire()?; - let project = tempdir().context("create project directory")?; - let home = tempdir().context("create fake home directory")?; - let _user_scope = sandbox_user_scope(&home)?; + let h = ConfigTestHarness::setup()?; let _legacy_guard = EnvVarGuard::remove("NETSUKE_CONFIG_PATH"); let _theme_guard = EnvVarGuard::set("NETSUKE_THEME", "unicode"); - let custom = project.path().join("custom.toml"); - fs::write(&custom, "theme = \"ascii\"\n").context("write explicit config file")?; - std::env::set_current_dir(project.path()).context("change to project directory")?; + let custom_path = h.write_config("custom.toml", "theme = \"ascii\"\n")?; + let custom_arg = custom_path.to_string_lossy().into_owned(); let merged_with_cli_override = - parse_and_merge(&["netsuke", "--config", "custom.toml", "--theme", "ascii"])?; + parse_and_merge(&["netsuke", "--config", &custom_arg, "--theme", "ascii"])?; ensure!( merged_with_cli_override.theme == Some(ThemePreference::Ascii), "CLI preference values should still override environment and selected config" ); - let merged_with_env_override = parse_and_merge(&["netsuke", "--config", "custom.toml"])?; + let merged_with_env_override = parse_and_merge(&["netsuke", "--config", &custom_arg])?; ensure!( merged_with_env_override.theme == Some(ThemePreference::Unicode), "environment preference values should still override the selected config" ); + let _project_root = h.project.path(); Ok(()) } diff --git a/tests/cli_tests/merge.rs b/tests/cli_tests/merge.rs index 8c683dad..de801bf3 100644 --- a/tests/cli_tests/merge.rs +++ b/tests/cli_tests/merge.rs @@ -313,7 +313,7 @@ fn cli_merge_layers_prefers_cli_then_env_then_file_for_locale( #[rstest] fn resolve_merged_diag_json_handles_malformed_project_config() -> Result<()> { let _env_lock = EnvLock::acquire(); - let _cwd_guard = CwdGuard::acquire().context("capture current working directory")?; + let cwd_guard = CwdGuard::acquire().context("capture current working directory")?; let temp_home = tempdir().context("create temporary home directory")?; let temp_project = tempdir().context("create temporary project directory")?; @@ -356,5 +356,6 @@ theme = "ascii "should honour user config output_format=json despite malformed project config" ); + drop(cwd_guard); Ok(()) }