Skip to content

fix(env_loader): prune previously-seeded keys when removed from .env - #18784

Open
rayzhux wants to merge 1 commit into
NousResearch:mainfrom
rayzhux:fix/env-loader-ghost-prune
Open

fix(env_loader): prune previously-seeded keys when removed from .env#18784
rayzhux wants to merge 1 commit into
NousResearch:mainfrom
rayzhux:fix/env-loader-ghost-prune

Conversation

@rayzhux

@rayzhux rayzhux commented May 2, 2026

Copy link
Copy Markdown

Summary

load_hermes_dotenv() calls python-dotenv's load_dotenv(override=True), which writes keys present in the file but never removes keys you deleted from .env. Long-running Hermes processes (gateway, dashboard, CLI sessions) keep stale values in os.environ for the lifetime of the process. The same key can ghost across SIGUSR1-triggered config reloads and explicit re-invocations of load_hermes_dotenv() inside the same process. Restarting the process is the only mitigation today.

Real-world failure mode

A key deleted from ~/.hermes/.env continued routing browser_navigate calls to its old endpoint for 8 days across three long-lived CLI processes. macOS ps eEww -p <pid> returns nothing for env on non-self processes, so detecting the ghost requires lldb or a process restart — there is no surfaced signal.

Fix

Track keys that load_hermes_dotenv itself put into os.environ in a per-process _HERMES_SEEDED_KEYS set, populated only when a key was absent from os.environ before our load. Before each subsequent load:

  1. Parse the .env file(s) with dotenv_values to compute declared keys.
  2. Pop any seeded key not in that declared set, and drop it from the registry.
  3. Then run load_dotenv as before.

Genuine shell exports (keys we never seeded) are never popped — only keys whose presence in os.environ originated from one of our own load_dotenv calls. If any .env file fails to parse, pruning is skipped to avoid clobbering keys whose declaration we couldn't read.

A bare FOO line (no =) is treated as undeclared. dotenv_values returns it as FOO -> None but load_dotenv does not actually seed it, so counting it as declared would let a previously-seeded value linger. FOO= (empty value) parses as FOO -> \"\" and IS seeded, so it stays declared.

Relationship to #18734

#18734 addresses a related but distinct bug: 12-factor precedence between shell-exported env vars and .env file values. That PR flips the default to override=False with a HERMES_DOTENV_OVERRIDE=1 opt-in.

This PR is complementary. It fixes the multi-load-in-same-process ghost under either precedence default — when override=True is in effect (or when override=False but .env did originally seed a value), a deletion in .env now actually takes effect on reload instead of pinning the old value forever.

Test plan

Six new test cases plus an autouse fixture that snapshots and restores the seeded-keys registry between tests:

  • test_seeded_key_removed_from_file_is_pruned_on_reload
  • test_shell_exported_key_never_pruned
  • test_seeded_key_value_change_propagates_on_reload
  • test_bare_key_without_value_is_pruned_on_reload
  • test_empty_value_keeps_key_declared
  • test_seeded_key_pruned_when_only_in_one_of_two_files

All 11 tests in tests/hermes_cli/test_env_loader.py pass:

```
$ pytest tests/hermes_cli/test_env_loader.py
11 passed in 1.51s
```

🤖 Generated with Claude Code

Problem
-------
load_hermes_dotenv() calls python-dotenv's load_dotenv(override=True),
which writes keys present in the file but never *removes* keys you
deleted from .env. Long-running Hermes processes (gateway, dashboard,
CLI sessions) keep stale values in os.environ for the lifetime of the
process. The same key can ghost across SIGUSR1-triggered config reloads
and across explicit re-invocations of load_hermes_dotenv() inside the
same process. Restarting the process is the only mitigation.

Real-world failure: a key deleted from ~/.hermes/.env continued routing
browser_navigate calls to its old endpoint for 8 days across three
long-lived CLI processes. macOS `ps eEww -p <pid>` returns nothing for
env on non-self processes, so detecting the ghost requires lldb or
process restart.

Fix
---
Track keys that load_hermes_dotenv itself put into os.environ (per-
process _HERMES_SEEDED_KEYS set, populated only when a key was absent
from os.environ before our load). Before each subsequent load:

  1. Parse the .env file(s) with dotenv_values to compute declared keys.
  2. Pop any seeded key not in that declared set, and drop it from
     the registry.
  3. Then run load_dotenv as before.

Genuine shell exports (keys we never seeded) are never popped — only
keys whose presence in os.environ originated from one of our own
load_dotenv calls. If any .env file fails to parse, pruning is skipped
to avoid clobbering keys whose declaration we couldn't read.

A bare `FOO` line (no `=`) is treated as undeclared. dotenv_values
returns it as `FOO -> None` but load_dotenv does not actually seed it,
so counting it as declared would let a previously-seeded value linger.
`FOO=` (empty value) parses as `FOO -> ""` and IS seeded, so it stays
declared.

Tests
-----
6 new test cases plus an autouse fixture that snapshots and restores
the seeded-keys registry between tests:

  - test_seeded_key_removed_from_file_is_pruned_on_reload
  - test_shell_exported_key_never_pruned
  - test_seeded_key_value_change_propagates_on_reload
  - test_bare_key_without_value_is_pruned_on_reload
  - test_empty_value_keeps_key_declared
  - test_seeded_key_pruned_when_only_in_one_of_two_files

All 11 tests in tests/hermes_cli/test_env_loader.py pass.

Relationship to PR NousResearch#18734
-------------------------
PR NousResearch#18734 addresses a related but distinct bug: 12-factor precedence
between shell-exported env vars and .env file values. That PR flips the
default to override=False with a HERMES_DOTENV_OVERRIDE=1 opt-in.

This PR is complementary. It fixes the multi-load-in-same-process ghost
under either precedence default — when override=True is in effect (or
when override=False but .env did originally seed a value), a deletion
in .env now actually takes effect on reload instead of pinning the
old value forever.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles labels May 2, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying a real long-lived-process gap. Current main still reloads through load_hermes_dotenv() on gateway turns (gateway/run.py:1336-1340), while the helper loads user .env with override=True (hermes_cli/env_loader.py:245-247) and has no deletion tracking.

Problems

  • The added newly_seeded expression records only names absent before loading (hermes_cli/env_loader.py:239, PR right side). If the shell starts with FOO=shell and .env has FOO=dotenv, the loader overwrites the shell value but does not track it. Removing FOO from .env then leaves FOO=dotenv, so the reported ghost persists for that important case.
  • Current main now also loads .op.env and applies external/managed secret sources after the user/project files (hermes_cli/env_loader.py:249-268, :273-302); the proposed declared-key pass covers only user/project paths.

Suggested changes

  • Track each overwritten key’s prior state and restore it when its dotenv declaration disappears; add the shell-overwrite/delete regression test.
  • Reconcile the implementation with current secret-source and managed-environment ordering before salvage.

This is an automated hermes-sweeper review.

Comment thread hermes_cli/env_loader.py
# already seeded) are not added a second time, and shell exports
# remain ineligible for pruning.
newly_seeded = (set(os.environ.keys()) - pre_load_keys) & declared_keys
_HERMES_SEEDED_KEYS.update(newly_seeded)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tracks only names that were absent before loading. With shell FOO=shell and .env FOO=dotenv, override=True replaces the shell value but FOO is not registered; deleting FOO later leaves dotenv ghosted. Preserve the prior value/missing state for overwritten keys and restore it on removal.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants