Skip to content

Commit 6147df9

Browse files
authored
fix(config): Fix env var priority over file config (#467)
The `load_envs` function was merging in the wrong order, passing the file-based config as the argument to `merge`. In schematic, the argument to `merge` takes precedence over the receiver, so this caused file configuration to silently override environment variables — the opposite of the intended behavior. The fix swaps the receiver and argument: environment variables now merge into the file-based config, correctly taking precedence over any file-level settings. Signed-off-by: Jean Mertz <git@jeanmertz.com>
1 parent 2719a85 commit 6147df9

2 files changed

Lines changed: 20 additions & 4 deletions

File tree

crates/jp_config/src/util.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,12 @@ pub fn load_partials_with_inheritance(
5353
///
5454
/// Returns an error if merging the partials fails, which returns a
5555
/// [`schematic::MergeError`].
56-
pub fn load_envs(base: PartialAppConfig) -> Result<PartialAppConfig, BoxedError> {
56+
pub fn load_envs(mut base: PartialAppConfig) -> Result<PartialAppConfig, BoxedError> {
5757
trace!("Loading environment variable configuration.");
58-
let mut partial = PartialAppConfig::from_envs()?;
59-
partial.merge(&(), base)?;
58+
let envs = PartialAppConfig::from_envs()?;
59+
base.merge(&(), envs)?;
6060

61-
Ok(partial)
61+
Ok(base)
6262
}
6363

6464
/// Tries to find a configuration file in a load path.

crates/jp_config/src/util_tests.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,22 @@ fn test_load_envs() {
9595
);
9696
}
9797

98+
#[test]
99+
#[serial(env_vars)]
100+
fn test_load_envs_overrides_file_config() {
101+
let _env = EnvVarGuard::set("JP_CFG_PROVIDERS_LLM_OPENROUTER_API_KEY_ENV", "FROM_ENV");
102+
103+
let mut file_config = PartialAppConfig::empty();
104+
file_config.providers.llm.openrouter.api_key_env = Some("FROM_FILE".to_owned());
105+
106+
let merged = load_envs(file_config).unwrap();
107+
assert_eq!(
108+
merged.providers.llm.openrouter.api_key_env,
109+
Some("FROM_ENV".to_owned()),
110+
"environment variables should override file config"
111+
);
112+
}
113+
98114
#[test]
99115
fn test_build() {
100116
let error = build(PartialAppConfig::default_values(&()).unwrap().unwrap()).unwrap_err();

0 commit comments

Comments
 (0)