Skip to content

fix(hindsight): preserve existing HINDSIGHT_LLM_API_KEY on blank re-setup input - #15465

Closed
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/hindsight-preserve-existing-key
Closed

fix(hindsight): preserve existing HINDSIGHT_LLM_API_KEY on blank re-setup input#15465
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/hindsight-preserve-existing-key

Conversation

@briandevans

Copy link
Copy Markdown
Contributor

What does this PR do?

Re-running `hermes memory setup` and pressing Enter at the "LLM API key:" prompt silently blanked out the previously-configured key. The prompt loop read the empty `llm_key`, stamped it into `env_writes` unconditionally, and then the write-back loop below overwrote the existing `HINDSIGHT_LLM_API_KEY=` line in `/.hermes/.env` with `HINDSIGHT_LLM_API_KEY=` (empty). The materialised profile env at `/.hindsight/profiles/hermes.env` then wrote `HINDSIGHT_API_LLM_API_KEY=` — the Hindsight daemon came up unauthenticated.

The existing comment ("always write explicitly including empty so the provider sees '' rather than a missing variable") documents why the variable is always emitted, but it didn't distinguish "user explicitly blanks the key" from "user pressed Enter to keep their existing key".

Fix

Before writing to `env_writes`, treat blank input as "keep existing": read the current value out of `~/.hermes/.env` and use that if present. Explicit blanking of a previously-set key still requires a deliberate action (deleting the value in `.env` directly, or typing a different key), which matches how every other setup prompt in Hermes behaves.

The "always emit the variable" contract is preserved — `env_writes[...] = llm_key` still runs unconditionally; only the SOURCE of `llm_key` changed for the blank-input case.

```python
if not llm_key:
existing = _load_simple_env(Path(hermes_home) / ".env")
llm_key = existing.get("HINDSIGHT_LLM_API_KEY", "")
env_writes["HINDSIGHT_LLM_API_KEY"] = llm_key
```

Test plan

  • This repairs `tests/plugins/memory/test_hindsight_provider.py::TestPostSetup::test_local_embedded_setup_preserves_existing_key_when_input_left_blank`, one of the long-standing baseline CI failures across every open PR (same cohort that test(model-switch): assert fetch_api_models kwargs match current signature (#15243) #15246 cleared for `test_custom_provider_model_switch`)
  • No new tests needed — the existing regression test explicitly asserts the preserved-key behaviour and was already failing on clean `origin/main` waiting for this fix
  • Verified regression guard: temporarily removed the new preserve-existing block; the test correctly failed with `'HINDSIGHT_API_LLM_API_KEY=existing-key' in 'HINDSIGHT_API_LLM_API_KEY=...'`. Restored → all 3 TestPostSetup tests pass, full hindsight suite 75/75 green.
  • Pre-push discipline applied: no new bare `except Exception`, no `int()` on user data, no unused imports

Not in scope

Related

…etup input

Re-running ``hermes memory setup`` and pressing Enter at the "LLM API
key:" prompt silently blanked out the previously-configured key.  The
prompt loop read the empty ``llm_key``, stamped it into ``env_writes``
unconditionally, and then the write-back loop below overwrote the
existing ``HINDSIGHT_LLM_API_KEY=<value>`` line in ``~/.hermes/.env``
with ``HINDSIGHT_LLM_API_KEY=`` (empty).  The materialised profile
env at ``~/.hindsight/profiles/hermes.env`` then wrote
``HINDSIGHT_API_LLM_API_KEY=`` — the daemon came up unauthenticated.

The existing comment ("always write explicitly including empty so the
provider sees "" rather than a missing variable") documents why the
variable is always emitted, but it didn't distinguish "user explicitly
blanks the key" from "user pressed Enter to keep their existing key".

### Fix

Before writing to ``env_writes``, treat blank input as "keep existing":
read the current value out of ``~/.hermes/.env`` and use that if
present.  Explicit blanking of a previously-set key still requires a
deliberate action (deleting the value in ``.env`` directly, or typing
a different key), which matches how every other setup prompt in
Hermes behaves.

The "always emit the variable" contract is preserved — ``env_writes[...]
= llm_key`` still runs unconditionally; only the SOURCE of ``llm_key``
changed for the blank-input case.

### Tests

This repairs
``tests/plugins/memory/test_hindsight_provider.py::TestPostSetup::test_local_embedded_setup_preserves_existing_key_when_input_left_blank``,
one of the long-standing baseline CI failures across every open PR
(the same cohort that NousResearch#15246 cleared for
``test_custom_provider_model_switch``).  No new tests needed — the
existing regression test explicitly asserts the preserved-key
behaviour and was already failing on clean ``origin/main``.

**Verified regression guard**: temporarily deleted the new
preserve-existing block; the test correctly failed with
``'HINDSIGHT_API_LLM_API_KEY=existing-key\\n' in
'HINDSIGHT_API_LLM_API_KEY=\\n...'``.  Restored → all 3 TestPostSetup
tests pass, full hindsight suite 75/75 green.

No new bare ``except Exception``, no ``int()`` on user data, no unused
imports — pre-push discipline checklist applied.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 25, 2026 01:57

Copilot AI 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.

Pull request overview

Fixes hermes memory setup for the Hindsight local_embedded mode so that pressing Enter at the “LLM API key” prompt no longer overwrites an already-configured HINDSIGHT_LLM_API_KEY with an empty value in ~/.hermes/.env, which previously propagated to an unauthenticated embedded daemon environment.

Changes:

  • Treat blank “LLM API key” input as “keep existing” by reading the current value from hermes_home/.env before writing back.
  • Preserve the existing “always emit the variable” behavior by still writing HINDSIGHT_LLM_API_KEY unconditionally (but with the preserved value when applicable).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels Apr 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #15233 — same root cause: blank input in hindsight post_setup() overwrites existing HINDSIGHT_LLM_API_KEY. Also related to #15309.

@briandevans

Copy link
Copy Markdown
Contributor Author

Closing as superseded — @LeonSGP43's #15233 predates this by ~10 hours and is strictly more thorough:

  • Checks three key sources (env var, ~/.hermes/.env, and the embedded profile env) vs my one (~/.hermes/.env only) — catches users who configured via HINDSIGHT_LLM_API_KEY env var but never persisted to .env
  • Adds a UX hint at the prompt: LLM API key (current: ...XXXX, blank to keep): so users know pressing Enter preserves the existing key (mine was silent about the fallback)

Both our fixes satisfy the same failing regression test (test_local_embedded_setup_preserves_existing_key_when_input_left_blank), but the broader fallback chain actually matters in practice — I only caught the single-source case.

Thanks @alt-glitch for the cross-reference. Closing in favour of #15233.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants