Warn loudly when config-shaping env vars arrive via component .env files (not honored)

[kylebernhardy]

Fixes #1513 — pivoted to warn-don't-honor per the design discussion below (top-level config controls components, not vice versa).

Summary

HARPER_DEFAULT_CONFIG/HARPER_CONFIG/HARPER_SET_CONFIG delivered via a component's .env (loadEnv) silently no-op'd: the config is composed and memoized before components load. The original PR applied them in a pre-composition pass; review (kriszyp) correctly flagged that as inverting the config relationship. The pivot keeps the detection and fixes the actual pain — the silence:

• Boot pre-pass (componentEnvPrepass.ts): the same component/.env scan (loadEnv files globs, config-file precedence, escape protection) now emits an actionable warning per config-shaping var found — naming the variable, the file, and the supported channels (process environment / harper-config.yaml) — without touching process.env.
• loadEnv plugin: warns at component-load time for the same keys, covering components deployed after boot (which the boot pre-pass can't see).

Where to look

• The warning texts — they're the feature now; wording suggestions welcome.
• The pre-pass never throws (config composition, including install, must survive bad component dirs) — unchanged posture from the previous revision.

Testing

10 unit tests on the pre-pass (detection reach: globs, config-file precedence, hidden dirs, escape protection; warns-without-applying incl. when a real process env var coexists; encrypted values) + loadEnv warning test + integration test flipped: the fixture delivers HARPER_CONFIG only via .env and asserts /mcp stays unmounted. Verified live on a real instance: both warnings appear in the boot log and the config is not shaped.

Generated by an LLM (Claude Fable 5); pivot direction approved by kriszyp on this PR.

[gemini-code-assist]

Code Review

This pull request introduces a pre-pass mechanism (config/componentEnvPrepass.ts) to parse and apply config-shaping environment variables from component .env files to process.env before the root configuration is composed and memoized, resolving issue #1513. It also includes corresponding integration and unit tests, and updates conflict handling in resources/loadEnv.ts. The reviewer identified a high-severity issue where relative rootPath configurations could resolve incorrectly relative to the current working directory instead of the HarperDB base path, and provided a code suggestion to use env.getHdbBasePath() during resolution.

[claude]

Reviewed; no blockers in the final revision — a warn-but-fall-through in loadEnv.ts (missing continue after the config-shaping warning, caught by @heskew after the first pass) was fixed in 7b05a84 before merge.

[kriszyp]

Question on the overall design direction here: do we really want components to be able to alter top-level config? My instinct is the opposite direction — the top-level config should control components, not vice versa. Letting a component's .env (via loadEnv) shape HARPER_CONFIG/HARPER_SET_CONFIG/HARPER_DEFAULT_CONFIG on the composed config feels like it inverts that relationship: a component can now reach up and mutate instance-wide config that other components and the top-level config file also depend on.

I get that this closes a real gap (#1513 — these vars silently no-op today if only delivered via a component's .env), but I'd like to understand the motivating use case a bit more before this direction gets locked in. Is there a reason a component needs to shape top-level config rather than just consuming its own scoped config?

[kylebernhardy]

Good question — context: this addresses #1513, filed by Nathan's agent (tps-flint), which hit the sharp edge directly: it delivered HARPER_SET_CONFIG via a component .env and the value silently no-op'd because config is composed and memoized before components load. The pain in that report is really the silence — nothing tells you the channel doesn't work.

Totally understandable that we don't want components inverting the config relationship — top-level config controlling components is the right direction, and honoring these vars from a component .env (even narrowly, in a pre-pass) does cut against that.

Proposal: pivot this PR to warn-don't-honor. Keep the early detection of HARPER_CONFIG/HARPER_SET_CONFIG/HARPER_DEFAULT_CONFIG in component .env files, but instead of applying them, log a prominent warning naming the variable, the component, and the supported alternatives (process environment / instance config). That fixes the actual failure mode from #1513 — the silent no-op becomes an actionable message — while keeping config strictly top-down. If that direction works for you I'll rework the PR accordingly (the detection pre-pass and tests mostly carry over; the docs PR #564 shrinks to documenting the warning).

Comment drafted by an LLM (Claude Fable 5) on Kyle's direction.

[kriszyp]

pivot this PR to warn-don't-honor

Love it! Do it!

[kylebernhardy]

Pivoted in 752cdd7: detection stays, application is gone — the pre-pass and loadEnv now emit actionable warnings (variable + file + supported channels) and process.env is never touched. Integration test flipped to assert /mcp stays unmounted; verified live that both warnings fire. PR title/description updated. Comment generated by an LLM (Claude Fable 5).

[kriszyp]

Solid diagnostic feature — correctly reverses the original approach (warn instead of silently applying) after review feedback, careful about never throwing, and doesn't leak env-var values into the warning (only names + file paths, which matters since HARPER_CONFIG/HARPER_SET_CONFIG payloads can carry secrets). Well tested at both the unit and integration level — the integration test actually boots a real instance with a fixture component and asserts the not-honored config doesn't take effect, not just that a log line fires.

One drift risk worth resolving before merge: CONFIG_SHAPING_ENV_VARS in componentEnvPrepass.ts is a second, independently-maintained list of the same three vars already named (as the ConfigSource type union) in harperConfigEnvVars.ts. There's no shared export today, so this isn't a trivial one-line fix — but it's exactly the failure mode this PR exists to eliminate: if a fourth config-shaping var is ever added to harperConfigEnvVars.ts, nothing forces this list to follow, and the new var would silently regress to the pre-#1513 behavior (no warning, no visibility) for anyone shipping it via .env. Would you take exporting a shared array from harperConfigEnvVars.ts (or at minimum a comment pointing back at this list) as part of this PR, or would a tracked follow-up be fine?

Also worth a quick confirmation: initConfig(force = true) re-runs this whole block since it's gated on flatConfigObj === undefined || force, not a separate one-time flag. Is initConfig(true) ever invoked mid-process (a config-reload path)? applyRuntimeEnvVarConfig right below it is explicitly documented as unsafe to run twice — since the pre-pass is stateless (scan + warn, no process.env writes) I don't think it shares that constraint, but a one-line confirmation would close the loop.

[heskew]

Cross-PR coordination with #1726 (#1618 — OptionsWatcher composing runtime env config over root-config reads). cc @kriszyp.

The boot pre-pass correctly never touches process.env, but the loadEnv-plugin path for the config-shaping trio still injects. In resources/loadEnv.ts, the CONFIG_SHAPING_ENV_VARS branch logs the warning but has no continue, so — absent a pre-existing value — it falls through to process.env[key] = value. A component loaded via loadEnv therefore still lands its .env-delivered HARPER_CONFIG/HARPER_SET_CONFIG/HARPER_DEFAULT_CONFIG in process.env; the "process.env is never touched" property holds only for the pre-pass, not this path.

That's harmless for this PR as-is — config is composed/memoized before loadEnv, so nothing re-reads it. But #1726 changes exactly that: it re-composes from process.env on every root-config read, so a trio var injected here would then be honored, quietly re-inverting the top-down relationship this PR establishes.

Suggestion: add a continue after the config-shaping warn (warn and skip the assignment), plus a test asserting the var never reaches process.env. Enforcing at the injection point — rather than defensively in each consumer — keeps the invariant true for every downstream reader, makes the PR's own "don't honor" property literally true on this path too, and lets #1726 safely rely on "the trio only reaches process.env via the sanctioned channels."

@kriszyp — since you set the top-down direction: does enforce-at-injection here match your intent, or would you rather each consumer defend itself (e.g. #1726 composing from a boot snapshot of process.env)? I'd lean enforce-once-here.

Comment generated by an LLM (Claude Opus 4.8, 1M context).

[kylebernhardy]

@heskew Good catch — fixed in 7b05a84, enforce-at-injection as you leaned: the config-shaping branch now warns and continues, so the trio never reaches process.env from a component .env on any path (the new unit test asserts that even with loadEnv override: true, and it was verified to fail against the fall-through before the fix). That makes this PR's "not honored" property injection-point-true, so #1726's re-composition can rely on the trio arriving only via sanctioned channels regardless of how @kriszyp answers the consumer-defense question. All suites green.

Comment generated by an LLM (Claude Fable 5).

[github-actions]

Patch cherry-pick: conflict

Cherry-pick onto v5.1 produced conflicts on commit(s): 0e80b6bdbe8b46e5162d55419d53ffac952df7bc

The conflict markers are committed on branch cherry-pick/v5.1/pr-1580.
A pull request has been opened to land this patch: #1754

[Ethan-Arrowood]

LGTM — the warn-don't-honor pivot was the right call, and the injection-point enforcement makes it future-proof against #1726. Red checks are unrelated (main's lint fixed by #1753, and a storage flake).

_{sent with Claude Fable 5}