diff --git a/docs/adr-004-explicit-config-selection-outside-orthoconfig.md b/docs/adr-004-explicit-config-selection-outside-orthoconfig.md new file mode 100644 index 00000000..22f1005e --- /dev/null +++ b/docs/adr-004-explicit-config-selection-outside-orthoconfig.md @@ -0,0 +1,115 @@ +# ADR 004: Explicit config selection outside OrthoConfig + +## Status + +Accepted. + +Accepted: 2026-05-31. Netsuke will resolve explicit configuration file +selection in `src/cli/config_merge.rs` rather than delegate this behaviour +to OrthoConfig's built-in discovery attributes. + +## Date + +2026-05-31. + +## Context and problem statement + +Netsuke needs an explicit configuration selector for operators who want one +known configuration file to control a run. The public selector order is +`--config` > `NETSUKE_CONFIG` > `NETSUKE_CONFIG_PATH` > automatic +discovery, where `NETSUKE_CONFIG_PATH` remains a backward-compatible alias +only. + +The existing merge pipeline is deliberately two-pass. It first resolves +early diagnostic JSON preferences from file layers, so startup errors can be +emitted in the requested format. It then runs the full OrthoConfig-backed +merge for the final `Cli` value. Automatic discovery also has Netsuke-specific +precedence requirements: project configuration must outrank user +configuration, and a missed project `.netsuke.toml` requires a direct +second-pass project load. + +OrthoConfig can discover configuration files, but its built-in discovery +attribute does not own Netsuke's `--config` spelling, legacy-environment +compatibility, early diagnostic merge, or project-over-user second pass. +Putting explicit selection into OrthoConfig would either expose +Netsuke-specific policy through a generic library API or force Netsuke to work +around library-owned behaviour in the CLI adapter. + +## Decision drivers + +- Keep Netsuke's command-line contract in the CLI adapter that owns the + command-line spelling. +- Preserve the existing two-pass merge pipeline for early diagnostic JSON + resolution and final configuration merging. +- Keep `OrthoConfig` responsible for generic layer composition, not + Netsuke-specific selector precedence. +- Support `NETSUKE_CONFIG_PATH` only as a compatibility alias behind + `NETSUKE_CONFIG`. +- Make explicit selection fail closed: an invalid selected file must not fall + through to automatic discovery. + +## Options considered + +### Option A: use OrthoConfig's built-in discovery attribute + +This would let OrthoConfig own the config-path selector and merge discovered +files as part of its normal derived merge behaviour. + +It was rejected because Netsuke needs the public spelling `--config`, the +environment precedence `NETSUKE_CONFIG` before `NETSUKE_CONFIG_PATH`, and +the two-pass diagnostic path. OrthoConfig's generic discovery machinery cannot +express those Netsuke-specific semantics without broadening its API around one +consumer's policy. + +### Option B: add Netsuke-specific explicit selection to OrthoConfig + +This would extend OrthoConfig, so Netsuke could delegate the selector order and +legacy alias handling to the library. + +It was rejected because the policy is part of Netsuke's CLI contract rather +than OrthoConfig's domain. Baking `NETSUKE_CONFIG`, `NETSUKE_CONFIG_PATH`, or +Netsuke's project-scope fallback into OrthoConfig would invert the dependency: +the generic merge library would know too much about one adapter. + +### Option C: resolve explicit selection in `config_merge.rs` + +This keeps explicit path selection beside Netsuke's CLI merge code. Private +helpers resolve the selector, load file layers for the diagnostic pass, and +push the same layers into the full merge composer. + +It is accepted because it keeps the boundary clear. OrthoConfig remains the +layer-composition engine, while Netsuke's CLI adapter owns how user input, +environment aliases, diagnostics, and automatic discovery are combined. + +## Decision outcome + +Netsuke resolves explicit configuration paths in `src/cli/config_merge.rs`. + +- `resolve_config_path` applies `--config` > `NETSUKE_CONFIG` > + `NETSUKE_CONFIG_PATH`, ignoring empty environment values. +- `env_config_path` reads one environment variable through an injected lookup + function, so precedence tests do not mutate process-global environment. +- `collect_diag_file_layers` mirrors the file-loading path for early diagnostic + JSON resolution and propagates explicit-load failures. +- `push_layers_result` drains successful layer loads into the merge composer, or + records the load error for final diagnostics. +- Automatic discovery remains the fallback only when no explicit selector is + present. + +## Consequences + +- The CLI adapter has a small amount of Netsuke-specific orchestration logic, + but the rules are visible and testable where the public contract is defined. +- OrthoConfig does not gain Netsuke-specific configuration selector semantics. +- Explicit selected files fail closed. A missing or invalid file reports the + selected-file error instead of silently inheriting a discovered file. +- Future changes to selector precedence must update `config_merge.rs`, the + developer guide, the design document, and this ADR together. + +## Related documents + +- [`docs/developers-guide.md`](developers-guide.md) +- [`docs/execplans/3-11-3-expose-config-path-and-netsuke-config.md`][execplan] +- [`docs/netsuke-design.md`](netsuke-design.md) + +[execplan]: execplans/3-11-3-expose-config-path-and-netsuke-config.md 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 aa0954a9..ba450d97 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 @@ -646,12 +646,15 @@ the JSON value that feeds the merge pipeline. /// /// Precedence: `--config` > `NETSUKE_CONFIG` > `NETSUKE_CONFIG_PATH`. /// Empty environment values are ignored. -fn resolve_config_path(cli: &Cli) -> Option { +fn resolve_config_path(cli: &Cli, var_os: F) -> Option +where + F: Fn(&str) -> Option, +{ cli.config .as_ref() - .map(PathBuf::from) - .or_else(|| env_config_path(CONFIG_ENV_VAR)) - .or_else(|| env_config_path(CONFIG_ENV_VAR_LEGACY)) + .cloned() + .or_else(|| env_config_path(&var_os, CONFIG_ENV_VAR)) + .or_else(|| env_config_path(var_os, CONFIG_ENV_VAR_LEGACY)) } ``` @@ -684,20 +687,20 @@ error from that direct load is appended to `errors`. ### Updated `collect_diag_file_layers` signature ```rust -fn collect_diag_file_layers(cli: &Cli) -> Vec> +fn collect_diag_file_layers(cli: &Cli) -> OrthoResult>> ``` This mirrors the same resolution order for early diag-JSON evaluation. An explicit config path is resolved first and, when `load_layers_from_path()` succeeds, its layers are returned immediately. If that explicit load fails, the -helper returns an empty vector and does not continue into automatic discovery, -so an invalid explicit selector cannot inherit a discovered diagnostic -preference. Without an explicit selector, the helper uses the same +helper propagates the `OrthoError` and does not continue into automatic +discovery, so an invalid explicit selector cannot inherit a discovered +diagnostic preference. Without an explicit selector, the helper uses the same `config_discovery(cli.directory.as_deref())` path as `push_file_layers()`, returns the first-pass file layers when the project file was already discovered, and only falls back to `project_scope_layers()` when the first pass -missed the project `.netsuke.toml`. If the direct project load fails, the -helper returns the first-pass layers instead of propagating the error. +missed the project `.netsuke.toml`. If the direct project load fails, that +error is propagated to the diagnostic merge caller. ### Fluent key (`src/localization/keys.rs`)