HindsightEmbedded forwarded every LLM/daemon setting on every construction, using placeholder defaults for the ones the caller never mentioned. The embed manager merges the caller's config over the profile .env and copies any non-None HINDSIGHT_* entry into the daemon environment, so a client built without credentials overwrote a key inherited from the profile or the parent shell -- and _register_profile then persisted the placeholders back into the profile's .env file, leaving a profile configured for anthropic recorded as groq on disk. llm_provider, llm_api_key, llm_model, log_level and idle_timeout now default to None and are omitted when not passed, so the daemon resolves them from the profile .env, then the parent environment, then its own defaults. An explicit empty string remains an override, which is how a local LLM service with no authentication clears an inherited key.
This commit is contained in:
Sanderhoff-alt
@@ -64,15 +64,26 @@ class HindsightEmbedded:
|
||||
- create_directive(), list_directives(), etc.
|
||||
- And all async variants (aretain, arecall, areflect, etc.)
|
||||
|
||||
Only the settings you pass explicitly are forwarded to the daemon. Anything
|
||||
left at its default is resolved by the daemon instead, in this order: the
|
||||
profile's .env file, then the parent process environment, then the daemon's
|
||||
own default. That is what lets a client constructed without credentials run
|
||||
against a profile (or a shell) that already has them configured, rather than
|
||||
overwriting them with placeholders (#3253).
|
||||
|
||||
Args:
|
||||
profile: Profile name for data isolation (default: "default")
|
||||
llm_provider: LLM provider ("groq", "openai", "ollama", "gemini", "anthropic", "lmstudio")
|
||||
llm_api_key: API key for the LLM provider
|
||||
llm_model: Model name to use
|
||||
llm_provider: LLM provider ("groq", "openai", "ollama", "gemini", "anthropic",
|
||||
"lmstudio"). Omit to inherit; the server default is "openai".
|
||||
llm_api_key: API key for the LLM provider. Omit to inherit; pass "" to
|
||||
explicitly run without a key (local services that need no auth).
|
||||
llm_model: Model name to use. Omit to inherit; the server picks a default
|
||||
for the resolved provider.
|
||||
llm_base_url: Optional custom base URL for LLM API
|
||||
database_url: Optional database URL override (default: profile-specific pg0)
|
||||
idle_timeout: Seconds before daemon auto-exits when idle (default: 0, disabled)
|
||||
log_level: Daemon log level (default: "info")
|
||||
idle_timeout: Seconds before daemon auto-exits when idle. Omit to inherit
|
||||
(daemon default: 0, disabled).
|
||||
log_level: Daemon log level. Omit to inherit (daemon default: "info").
|
||||
ui: Whether to start the control plane web UI alongside the daemon (default: False)
|
||||
ui_port: Port for the UI. Defaults to daemon_port + 10000.
|
||||
ui_hostname: Hostname to bind the UI to. Defaults to "0.0.0.0".
|
||||
@@ -81,13 +92,13 @@ class HindsightEmbedded:
|
||||
def __init__(
|
||||
self,
|
||||
profile: str = "default",
|
||||
llm_provider: str = "groq",
|
||||
llm_api_key: str = "",
|
||||
llm_model: str = "openai/gpt-oss-120b",
|
||||
llm_provider: Optional[str] = None,
|
||||
llm_api_key: Optional[str] = None,
|
||||
llm_model: Optional[str] = None,
|
||||
llm_base_url: Optional[str] = None,
|
||||
database_url: Optional[str] = None,
|
||||
idle_timeout: int = 0,
|
||||
log_level: str = "info",
|
||||
idle_timeout: Optional[int] = None,
|
||||
log_level: Optional[str] = None,
|
||||
ui: bool = False,
|
||||
ui_port: Optional[int] = None,
|
||||
ui_hostname: str = "0.0.0.0",
|
||||
@@ -95,29 +106,50 @@ class HindsightEmbedded:
|
||||
"""
|
||||
Initialize the embedded client (daemon starts on first use).
|
||||
|
||||
Every LLM/daemon setting left as None is omitted from the daemon config so
|
||||
the daemon resolves it from the profile .env, then the parent environment,
|
||||
then its own default.
|
||||
|
||||
Args:
|
||||
profile: Profile name for data isolation
|
||||
llm_provider: LLM provider
|
||||
llm_api_key: API key for the LLM provider
|
||||
llm_model: Model name to use
|
||||
llm_provider: LLM provider. Omit to inherit.
|
||||
llm_api_key: API key for the LLM provider. Omit to inherit; pass "" to
|
||||
explicitly run without a key.
|
||||
llm_model: Model name to use. Omit to inherit.
|
||||
llm_base_url: Optional custom base URL for LLM API
|
||||
database_url: Optional database URL override
|
||||
idle_timeout: Seconds before daemon auto-exits when idle (0 = disabled)
|
||||
log_level: Daemon log level
|
||||
idle_timeout: Seconds before daemon auto-exits when idle (0 = disabled).
|
||||
Omit to inherit.
|
||||
log_level: Daemon log level. Omit to inherit.
|
||||
ui: Whether to start the control plane web UI alongside the daemon
|
||||
ui_port: Port for the UI (defaults to daemon_port + 10000)
|
||||
ui_hostname: Hostname to bind the UI to (defaults to "0.0.0.0")
|
||||
"""
|
||||
self.profile = profile
|
||||
|
||||
# Build config dict for daemon (matches CLI format)
|
||||
self.config = {
|
||||
"HINDSIGHT_API_LLM_PROVIDER": llm_provider,
|
||||
"HINDSIGHT_API_LLM_API_KEY": llm_api_key,
|
||||
"HINDSIGHT_API_LLM_MODEL": llm_model,
|
||||
"HINDSIGHT_API_LOG_LEVEL": log_level,
|
||||
"HINDSIGHT_EMBED_DAEMON_IDLE_TIMEOUT": str(idle_timeout),
|
||||
}
|
||||
# Build the config dict for the daemon (matches CLI format), omitting
|
||||
# every setting the caller did not specify. An omitted key is inherited
|
||||
# by the daemon from the profile .env / parent environment; sending a
|
||||
# placeholder instead would overwrite it, and _register_profile would
|
||||
# then persist that placeholder into the profile's .env file (#3253).
|
||||
# An explicit "" is still an override — that is how a local LLM service
|
||||
# with no authentication clears an inherited API key.
|
||||
self.config: dict[str, str] = {}
|
||||
|
||||
if llm_provider is not None:
|
||||
self.config["HINDSIGHT_API_LLM_PROVIDER"] = llm_provider
|
||||
|
||||
if llm_api_key is not None:
|
||||
self.config["HINDSIGHT_API_LLM_API_KEY"] = llm_api_key
|
||||
|
||||
if llm_model is not None:
|
||||
self.config["HINDSIGHT_API_LLM_MODEL"] = llm_model
|
||||
|
||||
if log_level is not None:
|
||||
self.config["HINDSIGHT_API_LOG_LEVEL"] = log_level
|
||||
|
||||
if idle_timeout is not None:
|
||||
self.config["HINDSIGHT_EMBED_DAEMON_IDLE_TIMEOUT"] = str(idle_timeout)
|
||||
|
||||
if llm_base_url:
|
||||
self.config["HINDSIGHT_API_LLM_BASE_URL"] = llm_base_url
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
"""Configuration forwarding rules for HindsightEmbedded.
|
||||
|
||||
Regression coverage for #3253: a setting the caller does not pass must be left
|
||||
out of the daemon config, so the daemon can resolve it from the profile's .env
|
||||
file or the parent environment instead of receiving a client-side placeholder
|
||||
that overwrites it — and that the daemon then persists back into the profile.
|
||||
"""
|
||||
|
||||
import json
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from hindsight import HindsightEmbedded
|
||||
from hindsight_embed.daemon_embed_manager import DaemonEmbedManager
|
||||
|
||||
LLM_PROVIDER = "HINDSIGHT_API_LLM_PROVIDER"
|
||||
LLM_API_KEY = "HINDSIGHT_API_LLM_API_KEY"
|
||||
LLM_MODEL = "HINDSIGHT_API_LLM_MODEL"
|
||||
LOG_LEVEL = "HINDSIGHT_API_LOG_LEVEL"
|
||||
IDLE_TIMEOUT = "HINDSIGHT_EMBED_DAEMON_IDLE_TIMEOUT"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def temp_home(tmp_path, monkeypatch):
|
||||
"""Isolate HOME so profile .env files never touch the real user profile.
|
||||
|
||||
USERPROFILE is set as well because Path.home() consults it on Windows.
|
||||
"""
|
||||
home = tmp_path / "home"
|
||||
home.mkdir()
|
||||
monkeypatch.setenv("HOME", str(home))
|
||||
monkeypatch.setenv("USERPROFILE", str(home))
|
||||
return home
|
||||
|
||||
|
||||
def _write_profile(home, name, port, env_contents=None):
|
||||
"""Create a registered profile, optionally with a pre-populated .env file."""
|
||||
profile_dir = home / ".hindsight" / "profiles"
|
||||
profile_dir.mkdir(parents=True, exist_ok=True)
|
||||
(profile_dir / "metadata.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"version": 1,
|
||||
"profiles": {
|
||||
name: {
|
||||
"port": port,
|
||||
"created_at": "2024-01-01T00:00:00+00:00",
|
||||
"last_used": "2024-01-01T00:00:00+00:00",
|
||||
}
|
||||
},
|
||||
}
|
||||
)
|
||||
)
|
||||
env_path = profile_dir / f"{name}.env"
|
||||
if env_contents is not None:
|
||||
env_path.write_text(env_contents)
|
||||
return env_path
|
||||
|
||||
|
||||
def _daemon_env(client):
|
||||
"""Run the real daemon start path with Popen stubbed, returning the child env.
|
||||
|
||||
Asserting on client.config alone would not catch a regression in how the
|
||||
embed manager merges that config with the profile and the parent
|
||||
environment, which is where the reported bug actually surfaced.
|
||||
"""
|
||||
manager = DaemonEmbedManager()
|
||||
captured: dict[str, dict[str, str]] = {}
|
||||
spawned = [False]
|
||||
|
||||
def fake_popen(cmd, env, **kwargs):
|
||||
captured["env"] = env
|
||||
spawned[0] = True
|
||||
process = MagicMock()
|
||||
process.pid = 12345
|
||||
return process
|
||||
|
||||
with (
|
||||
patch("hindsight_embed.daemon_embed_manager.subprocess.Popen", side_effect=fake_popen),
|
||||
patch("hindsight_embed.daemon_embed_manager.time.sleep"),
|
||||
patch.object(manager, "_clear_port", return_value=True),
|
||||
patch.object(manager, "_find_api_command", return_value=["hindsight-api"]),
|
||||
patch.object(manager, "is_running", side_effect=lambda profile="": spawned[0]),
|
||||
patch("hindsight_embed.daemon_embed_manager.platform.system", return_value="Linux"),
|
||||
):
|
||||
assert manager.ensure_running(client.config, client.profile)
|
||||
|
||||
return captured["env"]
|
||||
|
||||
|
||||
def test_nothing_is_forwarded_when_nothing_is_specified(temp_home):
|
||||
assert HindsightEmbedded(profile="test").config == {}
|
||||
|
||||
|
||||
def test_explicitly_passed_settings_are_forwarded(temp_home):
|
||||
client = HindsightEmbedded(
|
||||
profile="test",
|
||||
llm_provider="openai",
|
||||
llm_api_key="sk-real",
|
||||
llm_model="gpt-4o-mini",
|
||||
log_level="debug",
|
||||
idle_timeout=300,
|
||||
)
|
||||
|
||||
assert client.config == {
|
||||
LLM_PROVIDER: "openai",
|
||||
LLM_API_KEY: "sk-real",
|
||||
LLM_MODEL: "gpt-4o-mini",
|
||||
LOG_LEVEL: "debug",
|
||||
IDLE_TIMEOUT: "300",
|
||||
}
|
||||
|
||||
|
||||
def test_empty_api_key_is_forwarded_as_an_override(temp_home):
|
||||
"""An empty string is an explicit choice, not an omission.
|
||||
|
||||
Local LLM services that need no authentication rely on it to clear a key
|
||||
inherited from the environment.
|
||||
"""
|
||||
assert HindsightEmbedded(profile="test", llm_api_key="").config[LLM_API_KEY] == ""
|
||||
|
||||
|
||||
def test_idle_timeout_zero_is_forwarded(temp_home):
|
||||
"""0 is falsy but meaningful ("never auto-exit"), so it must survive."""
|
||||
assert HindsightEmbedded(profile="test", idle_timeout=0).config[IDLE_TIMEOUT] == "0"
|
||||
|
||||
|
||||
def test_omitted_key_inherits_the_parent_environment(temp_home, monkeypatch):
|
||||
monkeypatch.setenv(LLM_API_KEY, "sk-parent")
|
||||
_write_profile(temp_home, "inherit-env", 9871)
|
||||
|
||||
env = _daemon_env(HindsightEmbedded(profile="inherit-env", llm_provider="openai"))
|
||||
|
||||
assert env[LLM_API_KEY] == "sk-parent"
|
||||
|
||||
|
||||
def test_omitted_settings_inherit_the_profile_env(temp_home, monkeypatch):
|
||||
for var in (LLM_PROVIDER, LLM_API_KEY, LLM_MODEL):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
env_path = _write_profile(
|
||||
temp_home,
|
||||
"prod",
|
||||
9872,
|
||||
"HINDSIGHT_API_LLM_PROVIDER=anthropic\n"
|
||||
"HINDSIGHT_API_LLM_MODEL=claude-sonnet-4-20250514\n"
|
||||
"HINDSIGHT_API_LLM_API_KEY=sk-ant-prod\n",
|
||||
)
|
||||
|
||||
env = _daemon_env(HindsightEmbedded(profile="prod"))
|
||||
|
||||
assert env[LLM_PROVIDER] == "anthropic"
|
||||
assert env[LLM_MODEL] == "claude-sonnet-4-20250514"
|
||||
assert env[LLM_API_KEY] == "sk-ant-prod"
|
||||
|
||||
# A successful start rewrites the profile's .env; it must not come back with
|
||||
# client-side placeholders in place of the configured values.
|
||||
persisted = env_path.read_text()
|
||||
assert "HINDSIGHT_API_LLM_PROVIDER=anthropic" in persisted
|
||||
assert "HINDSIGHT_API_LLM_MODEL=claude-sonnet-4-20250514" in persisted
|
||||
assert "HINDSIGHT_API_LLM_API_KEY=sk-ant-prod" in persisted
|
||||
|
||||
|
||||
def test_explicit_empty_key_overrides_the_parent_environment(temp_home, monkeypatch):
|
||||
monkeypatch.setenv(LLM_API_KEY, "sk-parent")
|
||||
_write_profile(temp_home, "no-auth", 9873)
|
||||
|
||||
env = _daemon_env(
|
||||
HindsightEmbedded(profile="no-auth", llm_provider="lmstudio", llm_api_key="")
|
||||
)
|
||||
|
||||
assert env[LLM_API_KEY] == ""
|
||||
|
||||
|
||||
def test_explicit_settings_still_win_over_the_profile(temp_home, monkeypatch):
|
||||
monkeypatch.delenv(LLM_PROVIDER, raising=False)
|
||||
_write_profile(temp_home, "override", 9874, "HINDSIGHT_API_LLM_PROVIDER=anthropic\n")
|
||||
|
||||
env = _daemon_env(HindsightEmbedded(profile="override", llm_provider="openai"))
|
||||
|
||||
assert env[LLM_PROVIDER] == "openai"
|
||||
Reference in New Issue
Block a user