fix(core): address review feedback on FastEmbed cache fix

- Use explicit ``is not None`` instead of ``or`` in _resolve_cache_dir so
  an empty-string override doesn't silently fall through to the default.
  Empty string is an invalid path; letting FastEmbed surface the error
  is clearer than swapping it out for ~/.basic-memory/fastembed_cache.
- Drop the ``os.getenv("HOME", Path.home())`` dance in resolve_data_dir.
  ``Path.home()`` already reads $HOME on POSIX and %USERPROFILE% on
  Windows, so the env check was redundant and made tests that patched
  os.name to simulate Windows on non-Windows hosts fragile.
- Switch default_fastembed_cache_dir to ``os.getenv`` for consistency
  with the rest of config.py.
- Fix test_default_fastembed_cache_dir_never_falls_back_to_tmp so it no
  longer mis-fires on Linux CI. ``tempfile.gettempdir()`` is ``/tmp``
  there and pytest's ``tmp_path`` lives under it, so the previous
  ``startswith(tempfile.gettempdir())`` guard flagged legitimate paths.
  Compare exact paths against FastEmbed's concrete
  ``<tempdir>/fastembed_cache`` fallback instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Drew Cain <groksrc@gmail.com>

Author: groksrc

src/basic_memory/config.py

@@ -54,16 +54,16 @@ def resolve_data_dir() -> Path:
     """Resolve the Basic Memory data directory.
 
     Single source of truth for the per-user state directory. Honors
-    BASIC_MEMORY_CONFIG_DIR so each process/worktree can isolate config
-    and database state; otherwise falls back to ``~/.basic-memory``.
+    ``BASIC_MEMORY_CONFIG_DIR`` so each process/worktree can isolate config
+    and database state; otherwise falls back to ``<user home>/.basic-memory``.
 
-    Works cross-platform: ``Path.home()`` returns ``%USERPROFILE%`` on
-    Windows and ``$HOME`` on macOS/Linux.
+    Cross-platform: ``Path.home()`` reads ``$HOME`` on POSIX and
+    ``%USERPROFILE%`` on Windows, so there's no need to check ``$HOME``
+    explicitly here.
     """
     if config_dir := os.getenv("BASIC_MEMORY_CONFIG_DIR"):
         return Path(config_dir)
-    home = os.getenv("HOME", Path.home())
-    return Path(home) / DATA_DIR_NAME
+    return Path.home() / DATA_DIR_NAME
 
 
 def default_fastembed_cache_dir() -> str:
@@ -83,7 +83,7 @@ def default_fastembed_cache_dir() -> str:
       ONNX load raises ``NO_SUCHFILE``. Persisting the cache under the
       per-user data directory works identically on macOS, Linux, and Windows.
     """
-    if env_override := os.environ.get("FASTEMBED_CACHE_PATH"):
+    if env_override := os.getenv("FASTEMBED_CACHE_PATH"):
         return env_override
     return str(resolve_data_dir() / "fastembed_cache")
 

src/basic_memory/repository/embedding_provider_factory.py

@@ -23,8 +23,17 @@
 
 
 def _resolve_cache_dir(app_config: BasicMemoryConfig) -> str:
-    """Resolve the effective FastEmbed cache dir for this config."""
-    return app_config.semantic_embedding_cache_dir or default_fastembed_cache_dir()
+    """Resolve the effective FastEmbed cache dir for this config.
+
+    Uses an explicit ``is not None`` check — an empty string override from
+    config or ``BASIC_MEMORY_SEMANTIC_EMBEDDING_CACHE_DIR`` is an invalid
+    path, not a request to fall back to the default, and FastEmbed's error
+    message is clearer than silently swapping in a different directory.
+    """
+    configured = app_config.semantic_embedding_cache_dir
+    if configured is not None:
+        return configured
+    return default_fastembed_cache_dir()
 
 
 def _available_cpu_count() -> int | None:

tests/test_config.py

@@ -335,15 +335,30 @@ def test_default_fastembed_cache_dir_env_override(self, tmp_path, monkeypatch):
 
         assert default_fastembed_cache_dir() == str(custom)
 
-    def test_default_fastembed_cache_dir_never_falls_back_to_tmp(self, config_home, monkeypatch):
-        """Regression guard for #741: default must not point at /tmp/fastembed_cache."""
+    def test_default_fastembed_cache_dir_never_falls_back_to_fastembed_tmp_default(
+        self, config_home, monkeypatch
+    ):
+        """Regression guard for #741.
+
+        FastEmbed's own fallback when ``cache_dir`` is ``None`` is
+        ``<system tmp>/fastembed_cache`` — the exact path that disappears in
+        sandboxed MCP runtimes (Codex CLI). Ensure Basic Memory's resolver
+        never lands on that path.
+
+        Compared as exact paths rather than ``startswith(tempfile.gettempdir())``
+        because the test runner itself can legitimately live under ``/tmp``
+        (pytest's ``tmp_path`` does on Linux CI), and that's not the bug we
+        care about here.
+        """
         monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
         monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)
 
-        resolved = default_fastembed_cache_dir()
+        resolved = Path(default_fastembed_cache_dir())
 
-        assert "/tmp/fastembed_cache" not in resolved
-        assert not resolved.startswith(tempfile.gettempdir())
+        # Must equal the data-dir-relative default.
+        assert resolved == config_home / ".basic-memory" / "fastembed_cache"
+        # And must not equal FastEmbed's own <tempdir>/fastembed_cache fallback.
+        assert resolved != Path(tempfile.gettempdir()) / "fastembed_cache"
 
 
 class TestConfigManager: