Verify config precedence invariant (#291)

src/cli/config_merge.rs

@@ -361,6 +361,10 @@ pub fn merge_with_config(cli: &Cli, matches: &ArgMatches) -> OrthoResult<Cli> {
     Ok(merged)
 }
 
+#[cfg(test)]
+#[path = "config_path_precedence_tests.rs"]
+mod config_path_precedence_tests;
+
 #[cfg(test)]
 #[path = "config_merge_tests.rs"]
 mod tests;

src/cli/config_merge_tests.rs

@@ -331,65 +331,3 @@ fn env_config_path_returns_path_when_var_set() {
     let result = env_config_path(|name| env.var_os(name), "__NETSUKE_TEST_VAR");
     assert_eq!(result, Some(std::path::PathBuf::from("/tmp/foo.toml")));
 }
-
-// -----------------------------------------------------------------------
-// resolve_config_path
-// -----------------------------------------------------------------------
-
-#[rstest]
-#[case::cli_wins_over_env(
-    (Some("/env/path.toml"), None),
-    Some("/cli/path.toml"),
-    Some("/cli/path.toml"),
-    "cli.config should take precedence over CONFIG_ENV_VAR"
-)]
-#[case::env_wins_over_legacy(
-    (Some("/env/path.toml"), Some("/legacy/path.toml")),
-    None,
-    Some("/env/path.toml"),
-    "CONFIG_ENV_VAR should take precedence over CONFIG_ENV_VAR_LEGACY"
-)]
-#[case::legacy_used_when_primary_absent(
-    (None, Some("/legacy/path.toml")),
-    None,
-    Some("/legacy/path.toml"),
-    "CONFIG_ENV_VAR_LEGACY should be used when primary env var absent"
-)]
-#[case::cli_wins_over_both_env_vars(
-    (Some("/env/path.toml"), Some("/legacy/path.toml")),
-    Some("/cli/path.toml"),
-    Some("/cli/path.toml"),
-    "cli.config should win over both env vars"
-)]
-fn resolve_config_path_precedence(
-    #[case] env_vars: (Option<&'static str>, Option<&'static str>),
-    #[case] cli_config: Option<&'static str>,
-    #[case] expected: Option<&'static str>,
-    #[case] msg: &'static str,
-) {
-    let mut env = TestEnv::default();
-    if let Some(value) = env_vars.0 {
-        env = env.with_var(CONFIG_ENV_VAR, value);
-    }
-    if let Some(value) = env_vars.1 {
-        env = env.with_var(CONFIG_ENV_VAR_LEGACY, value);
-    }
-    let cli = Cli {
-        config: cli_config.map(std::path::PathBuf::from),
-        ..Cli::default()
-    };
-    assert_eq!(
-        resolve_config_path(&cli, |name| env.var_os(name)),
-        expected.map(std::path::PathBuf::from),
-        "{msg}",
-    );
-}
-
-#[test]
-fn resolve_config_path_returns_none_when_all_absent() {
-    let cli = Cli::default();
-    assert!(
-        resolve_config_path(&cli, |name| TestEnv::default().var_os(name)).is_none(),
-        "should return None when no selector is active"
-    );
-}

src/cli/config_path_precedence_tests.rs

@@ -0,0 +1,122 @@
+//! Tests for explicit config path selector precedence.
+
+use super::*;
+use proptest::prelude::*;
+use rstest::rstest;
+use std::collections::HashMap;
+use std::ffi::OsString;
+use std::path::PathBuf;
+
+#[derive(Default)]
+struct TestEnv {
+    values: HashMap<&'static str, OsString>,
+}
+
+impl TestEnv {
+    fn with_var(mut self, name: &'static str, value: impl Into<OsString>) -> Self {
+        self.values.insert(name, value.into());
+        self
+    }
+
+    fn var_os(&self, name: &str) -> Option<OsString> {
+        self.values.get(name).cloned()
+    }
+}
+
+fn precedence_winner(
+    cli_config: Option<PathBuf>,
+    env_config: Option<PathBuf>,
+    legacy_config: Option<PathBuf>,
+) -> Option<PathBuf> {
+    cli_config.or(env_config).or(legacy_config)
+}
+
+fn resolve_config_path_with_selectors(
+    cli_config: Option<PathBuf>,
+    env_config: Option<PathBuf>,
+    legacy_config: Option<PathBuf>,
+) -> Option<PathBuf> {
+    let mut env = TestEnv::default();
+    if let Some(value) = env_config {
+        env = env.with_var(CONFIG_ENV_VAR, value.into_os_string());
+    }
+    if let Some(value) = legacy_config {
+        env = env.with_var(CONFIG_ENV_VAR_LEGACY, value.into_os_string());
+    }
+    let cli = Cli {
+        config: cli_config,
+        ..Cli::default()
+    };
+    resolve_config_path(&cli, |name| env.var_os(name))
+}
+
+/// For selectors S1 (`cli.config`), S2 (`NETSUKE_CONFIG`), and S3
+/// (`NETSUKE_CONFIG_PATH`), the resolved path must be S1 when present, else S2
+/// when present, else S3 when present, else `None`. These cases exhaustively
+/// enumerate the 2^3 selector presence states.
+#[rstest]
+#[case::all_absent(None, None, None, None)]
+#[case::legacy_only(None, None, Some("/legacy/path.toml"), Some("/legacy/path.toml"))]
+#[case::env_only(None, Some("/env/path.toml"), None, Some("/env/path.toml"))]
+#[case::env_wins_over_legacy(
+    None,
+    Some("/env/path.toml"),
+    Some("/legacy/path.toml"),
+    Some("/env/path.toml")
+)]
+#[case::cli_only(Some("/cli/path.toml"), None, None, Some("/cli/path.toml"))]
+#[case::cli_wins_over_legacy_alone(
+    Some("/cli/path.toml"),
+    None,
+    Some("/legacy/path.toml"),
+    Some("/cli/path.toml")
+)]
+#[case::cli_wins_over_env(
+    Some("/cli/path.toml"),
+    Some("/env/path.toml"),
+    None,
+    Some("/cli/path.toml")
+)]
+#[case::cli_wins_over_both_env_vars(
+    Some("/cli/path.toml"),
+    Some("/env/path.toml"),
+    Some("/legacy/path.toml"),
+    Some("/cli/path.toml")
+)]
+fn resolve_config_path_precedence(
+    #[case] cli_config: Option<&'static str>,
+    #[case] env_config: Option<&'static str>,
+    #[case] legacy_config: Option<&'static str>,
+    #[case] expected: Option<&'static str>,
+) {
+    assert_eq!(
+        resolve_config_path_with_selectors(
+            cli_config.map(PathBuf::from),
+            env_config.map(PathBuf::from),
+            legacy_config.map(PathBuf::from),
+        ),
+        expected.map(PathBuf::from),
+    );
+}
+
+fn path_selector() -> impl Strategy<Value = Option<PathBuf>> {
+    proptest::option::of("[A-Za-z0-9._/-]{1,64}".prop_map(PathBuf::from))
+}
+
+proptest! {
+    #[test]
+    fn resolve_config_path_obeys_precedence_invariant(
+        cli_config in path_selector(),
+        env_config in path_selector(),
+        legacy_config in path_selector(),
+    ) {
+        let expected = precedence_winner(
+            cli_config.clone(),
+            env_config.clone(),
+            legacy_config.clone(),
+        );
+        let actual = resolve_config_path_with_selectors(cli_config, env_config, legacy_config);
+
+        prop_assert_eq!(actual, expected);
+    }
+}

tests/cli_tests/config_selection.rs

@@ -260,6 +260,23 @@ fn write_optional_config(
     .with_cli_config(ConfigFile::new("cli.toml", "theme = \"unicode\"\n"))
     .with_env_config(ConfigFile::new("env.toml", "theme = \"ascii\"\n")),
 )]
+#[case::config_flag_takes_precedence_over_legacy_env(
+    ConfigSelectionCase::new(
+        ThemePreference::Unicode,
+        "--config should win over NETSUKE_CONFIG_PATH",
+    )
+    .with_cli_config(ConfigFile::new("cli.toml", "theme = \"unicode\"\n"))
+    .with_legacy_config(ConfigFile::new("legacy.toml", "theme = \"ascii\"\n")),
+)]
+#[case::config_flag_takes_precedence_over_both_env_vars(
+    ConfigSelectionCase::new(
+        ThemePreference::Unicode,
+        "--config should win over both config environment variables",
+    )
+    .with_cli_config(ConfigFile::new("cli.toml", "theme = \"unicode\"\n"))
+    .with_env_config(ConfigFile::new("env.toml", "theme = \"ascii\"\n"))
+    .with_legacy_config(ConfigFile::new("legacy.toml", "theme = \"ascii\"\n")),
+)]
 #[case::config_flag_values_still_overridden_by_cli_preferences(
     ConfigSelectionCase::new(
         ThemePreference::Ascii,