From 3ad9d3258738fd5070f62bca46e797a7e3d9f317 Mon Sep 17 00:00:00 2001 From: Robert Sigmundsson Date: Thu, 16 Jul 2026 22:27:26 +0200 Subject: [PATCH] fix(cli): honor SURREAL_MEMORY_BRAIN env when --brain omitted CLI commands resolved the brain as `brain or config.current_brain` before calling get_storage(), always passing a non-None name. That made get_storage() take its explicit-arg branch and skip its own env-var check, so SURREAL_MEMORY_BRAIN was silently ignored and commands ran against the wrong brain (falling back to 'default'). Add a central resolve_brain(brain, config, *, default=None) helper (explicit-arg > env var > default/config) and route get_storage() plus all ten CLI call sites through it: consolidate, decay, lifecycle (tools.py); export, import (shortcuts.py); brain list/export/import/health (brain.py). Also drops the duplicated SURREAL_MEMORY_BRAIN lookup in get_storage(). Add tests/unit/test_cli_resolve_brain.py covering the priority order and the empty-env-var edge case. (cherry picked from commit 1bdc7b2e427c4d414f064e203f4c1687364cc48e) --- src/surreal_memory/cli/_helpers.py | 27 ++++++--- src/surreal_memory/cli/commands/brain.py | 11 ++-- src/surreal_memory/cli/commands/shortcuts.py | 9 +-- src/surreal_memory/cli/commands/tools.py | 8 +-- tests/unit/test_cli_resolve_brain.py | 59 ++++++++++++++++++++ 5 files changed, 94 insertions(+), 20 deletions(-) create mode 100644 tests/unit/test_cli_resolve_brain.py diff --git a/src/surreal_memory/cli/_helpers.py b/src/surreal_memory/cli/_helpers.py index b2ec817b..6bbd602d 100755 --- a/src/surreal_memory/cli/_helpers.py +++ b/src/surreal_memory/cli/_helpers.py @@ -5,6 +5,7 @@ import asyncio import json import logging +import os from collections.abc import Coroutine from pathlib import Path from typing import Any, TypeVar @@ -18,6 +19,24 @@ T = TypeVar("T") + +def resolve_brain(brain: str | None, config: CLIConfig, *, default: str | None = None) -> str: + """Resolve the effective brain name: explicit arg > env var > default/config. + + Every CLI command that needs the brain name before calling ``get_storage`` + (e.g. to echo progress) must resolve it through this helper rather than + ``brain or config.current_brain`` — that pattern always produces a + non-``None`` name, which makes ``get_storage`` skip its own env-var check + (see its docstring) and silently ignores ``SURREAL_MEMORY_BRAIN``. + """ + if brain is not None: + return brain + env_brain = os.environ.get("SURREAL_MEMORY_BRAIN") + if env_brain: + return env_brain + return default if default is not None else config.current_brain + + # Track storages created during a CLI command so we can close them before # the event loop shuts down (prevents "Event loop is closed" noise from # aiosqlite's background thread). @@ -84,13 +103,7 @@ async def get_storage( Storage instance (local JSON, local SQLite, or remote shared) """ # Priority: explicit arg > env var > config file - if brain_name is None: - import os - - env_brain = os.environ.get("SURREAL_MEMORY_BRAIN") or os.environ.get("SURREAL_MEMORY_BRAIN") - name = env_brain or config.current_brain - else: - name = brain_name + name = resolve_brain(brain_name, config) # Remote shared mode (via server) use_shared = (config.is_shared_mode or force_shared) and not force_local and not force_sqlite diff --git a/src/surreal_memory/cli/commands/brain.py b/src/surreal_memory/cli/commands/brain.py index 371056ea..beeefc3d 100755 --- a/src/surreal_memory/cli/commands/brain.py +++ b/src/surreal_memory/cli/commands/brain.py @@ -13,6 +13,7 @@ get_config, get_storage, output_result, + resolve_brain, run_async, ) from surreal_memory.safety.freshness import analyze_freshness @@ -36,8 +37,8 @@ def brain_list( config = get_config() brains = config.list_brains() # Effective brain: env var overrides config (matches CLI get_storage resolution) - env_brain = os.environ.get("SURREAL_MEMORY_BRAIN") or os.environ.get("SURREAL_MEMORY_BRAIN") - current = env_brain or config.current_brain + env_brain = os.environ.get("SURREAL_MEMORY_BRAIN") + current = resolve_brain(None, config) if json_output: result: dict[str, Any] = {"brains": brains, "current": current} @@ -158,7 +159,7 @@ def brain_export( async def _export() -> None: config = get_config() - brain_name = name or config.current_brain + brain_name = resolve_brain(name, config) brain_path = get_brain_path_auto(config, brain_name) if not brain_path.exists(): @@ -302,8 +303,8 @@ async def _import() -> None: if not typer.confirm("Continue importing?"): raise typer.Exit(0) - brain_name = name or data.get("brain_name", "imported") config = get_config() + brain_name = resolve_brain(name, config, default=data.get("brain_name", "imported")) if brain_name in config.list_brains(): typer.secho( @@ -565,7 +566,7 @@ def brain_health( async def _health() -> dict[str, Any]: config = get_config() - brain_name = name or config.current_brain + brain_name = resolve_brain(name, config) brain_path = get_brain_path_auto(config, brain_name) if not brain_path.exists(): diff --git a/src/surreal_memory/cli/commands/shortcuts.py b/src/surreal_memory/cli/commands/shortcuts.py index 2e1cb6b1..056d687f 100755 --- a/src/surreal_memory/cli/commands/shortcuts.py +++ b/src/surreal_memory/cli/commands/shortcuts.py @@ -8,7 +8,7 @@ import typer -from surreal_memory.cli._helpers import get_config, get_storage, run_async +from surreal_memory.cli._helpers import get_config, get_storage, resolve_brain, run_async from surreal_memory.core.memory_types import Priority, TypedMemory, suggest_memory_type from surreal_memory.utils.timeutils import utcnow @@ -269,7 +269,7 @@ def export_brain_cmd( async def _export() -> None: config = get_config() - brain_name = brain or config.current_brain + brain_name = resolve_brain(brain, config) storage = await get_storage(config, brain_name=brain_name) snapshot = await storage.export_brain(brain_name) @@ -335,8 +335,9 @@ async def _import() -> None: data = json.loads(input_path.read_text()) - brain_name = brain or data.get("brain_name", "imported") - storage = await get_storage(config=get_config(), brain_name=brain_name) + config = get_config() + brain_name = resolve_brain(brain, config, default=data.get("brain_name", "imported")) + storage = await get_storage(config=config, brain_name=brain_name) incoming_snapshot = BrainSnapshot( brain_id=data.get("brain_id", brain_name), diff --git a/src/surreal_memory/cli/commands/tools.py b/src/surreal_memory/cli/commands/tools.py index b25a023a..049eee39 100755 --- a/src/surreal_memory/cli/commands/tools.py +++ b/src/surreal_memory/cli/commands/tools.py @@ -7,7 +7,7 @@ import typer -from surreal_memory.cli._helpers import get_config, get_storage, run_async +from surreal_memory.cli._helpers import get_config, get_storage, resolve_brain, run_async def mcp() -> None: @@ -433,7 +433,7 @@ def decay( async def _decay() -> None: config = get_config() - brain_name = brain or config.current_brain + brain_name = resolve_brain(brain, config) typer.echo(f"Applying decay to brain '{brain_name}'...") if dry_run: @@ -558,7 +558,7 @@ def consolidate( async def _consolidate() -> None: config = get_config() - brain_name = brain or config.current_brain + brain_name = resolve_brain(brain, config) typer.echo(f"Consolidating brain '{brain_name}' (strategy: {strategy})...") if dry_run: @@ -1061,7 +1061,7 @@ def lifecycle( async def _lifecycle() -> None: config = get_config() - brain_name = brain or config.current_brain + brain_name = resolve_brain(brain, config) storage = await get_storage(config, brain_name=brain_name) if action == "status": diff --git a/tests/unit/test_cli_resolve_brain.py b/tests/unit/test_cli_resolve_brain.py new file mode 100644 index 00000000..8840d4bb --- /dev/null +++ b/tests/unit/test_cli_resolve_brain.py @@ -0,0 +1,59 @@ +"""Tests for CLI brain-name resolution priority (explicit arg > env var > config). + +Regression coverage for the bug where ``brain_name = brain or config.current_brain`` +at CLI command call sites always produced a non-None name, causing ``get_storage`` +to skip its own env-var check and silently ignore ``SURREAL_MEMORY_BRAIN``. +""" + +from __future__ import annotations + +from surreal_memory.cli._helpers import resolve_brain +from surreal_memory.cli.config import CLIConfig + + +def _config(current_brain: str = "default") -> CLIConfig: + return CLIConfig(current_brain=current_brain) + + +class TestResolveBrain: + def test_explicit_arg_wins_over_everything(self, monkeypatch) -> None: + monkeypatch.setenv("SURREAL_MEMORY_BRAIN", "env-brain") + config = _config("config-brain") + + assert resolve_brain("explicit-brain", config) == "explicit-brain" + + def test_env_var_wins_when_no_explicit_arg(self, monkeypatch) -> None: + monkeypatch.setenv("SURREAL_MEMORY_BRAIN", "env-brain") + config = _config("config-brain") + + assert resolve_brain(None, config) == "env-brain" + + def test_config_current_brain_used_when_no_explicit_arg_or_env(self, monkeypatch) -> None: + monkeypatch.delenv("SURREAL_MEMORY_BRAIN", raising=False) + config = _config("config-brain") + + assert resolve_brain(None, config) == "config-brain" + + def test_empty_env_var_treated_as_unset(self, monkeypatch) -> None: + monkeypatch.setenv("SURREAL_MEMORY_BRAIN", "") + config = _config("config-brain") + + assert resolve_brain(None, config) == "config-brain" + + def test_default_param_used_over_config_when_no_explicit_arg_or_env(self, monkeypatch) -> None: + monkeypatch.delenv("SURREAL_MEMORY_BRAIN", raising=False) + config = _config("config-brain") + + assert resolve_brain(None, config, default="imported-brain") == "imported-brain" + + def test_env_var_wins_over_default_param(self, monkeypatch) -> None: + monkeypatch.setenv("SURREAL_MEMORY_BRAIN", "env-brain") + config = _config("config-brain") + + assert resolve_brain(None, config, default="imported-brain") == "env-brain" + + def test_explicit_arg_wins_over_default_param(self, monkeypatch) -> None: + monkeypatch.setenv("SURREAL_MEMORY_BRAIN", "env-brain") + config = _config("config-brain") + + assert resolve_brain("explicit-brain", config, default="imported-brain") == "explicit-brain"