Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/reference/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ places, the environment wins.
| `SURREAL_MEMORY_API_KEY` | `src/surreal_memory/unified_config.py` |
| `SURREAL_MEMORY_BRAIN` | `src/surreal_memory/cli/_helpers.py`, `src/surreal_memory/cli/commands/brain.py`, `src/surreal_memory/unified_config.py` |
| `SURREAL_MEMORY_DASHBOARD_CACHE_TTL` | `src/surreal_memory/server/dashboard_cache.py` |
| `SURREAL_MEMORY_DIR` | `src/surreal_memory/cli/config.py`, `src/surreal_memory/cli/update_check.py`, `src/surreal_memory/engine/reasoning_injection.py`, +3 more |
| `SURREAL_MEMORY_DIR` | `src/surreal_memory/cli/config.py`, `src/surreal_memory/cli/update_check.py`, `src/surreal_memory/engine/reasoning_injection.py`, +4 more |
| `SURREAL_MEMORY_DISABLE_SUPERSEDED_FILTER` | `src/surreal_memory/mcp/recall_handler.py` |
| `SURREAL_MEMORY_EMBEDDING_API_KEY` | `src/surreal_memory/engine/embedding/bge_m3_embedding.py` |
| `SURREAL_MEMORY_EMBEDDING_DIMENSION` | `src/surreal_memory/engine/embedding/bge_m3_embedding.py`, `src/surreal_memory/unified_config.py` |
Expand Down
19 changes: 17 additions & 2 deletions src/surreal_memory/utils/consolidation_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,23 @@


def _lock_path(brain_id: str = "") -> Path:
"""Get the lock file path, optionally per-brain."""
base = Path.home() / ".surrealmemory"
"""Get the lock file path, optionally per-brain.

Honours ``SURREAL_MEMORY_DIR`` like the rest of the data directory does
(``cli/config.get_default_data_dir``, ``cli/update_check._get_cache_path``).
Hard-coding ``Path.home()`` here made the lock the one piece of state that
a redirected data dir could not move: anything run against a throwaway
brain still created and deleted files in the operator's real
``~/.surrealmemory``.

The trade-off is worth naming: two processes pointed at different
``SURREAL_MEMORY_DIR`` values now take different locks and no longer
exclude one another. That is the intended reading -- they are working on
separate data directories -- but it does mean the lock is per data dir
rather than per machine.
"""
env_dir = os.environ.get("SURREAL_MEMORY_DIR")
base = Path(env_dir).resolve() if env_dir else Path.home() / ".surrealmemory"
base.mkdir(parents=True, exist_ok=True)
safe_name = brain_id.replace("/", "_").replace("\\", "_") if brain_id else ""
filename = f"consolidation-{safe_name}.lock" if safe_name else "consolidation.lock"
Expand Down
37 changes: 29 additions & 8 deletions tests/unit/test_multi_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import json
import os
import time
from pathlib import Path
from unittest.mock import AsyncMock, MagicMock, patch

import pytest
Expand Down Expand Up @@ -120,22 +121,42 @@ def test_agent_tag_coexists_with_user_tags(self) -> None:
class TestConsolidationLock:
"""Test file-based consolidation lock.

_lock_path() is a real, fixed file under ~/.surrealmemory/ shared across
the whole process tree — not test-isolated. Under pytest-xdist's default
per-test scheduling, two of these tests can land on different worker
processes and race on that one file (reproduced live: a different subset
fails each full-suite run, 100% pass in isolation). xdist_group pins the
whole class to one worker, same fix as the aiosqlite leak-guard pair.
_lock_path() ignored SURREAL_MEMORY_DIR and derived its directory from
Path.home() alone. When #74 added xdist_group here, that made these tests
share one real lock file across the process tree and race on it, exactly as
the note it left said. #121 then gave the suite a session-scoped $HOME
redirect for unrelated reasons, which hands every xdist worker its own home
and therefore its own lock file -- so the race has not been reachable since,
and the note quietly stopped describing the present.

What the redirect does not do is make the intent explicit: a test that
relies on $HOME being someone else's problem is one refactor away from
writing into a real home again. The fixture below points SURREAL_MEMORY_DIR
at a per-test directory and the first test asserts the redirection, because
the failure mode is silent. xdist_group is kept as a guard against shared
state being reintroduced.
"""

@pytest.fixture(autouse=True)
def _cleanup_lock(self) -> None:
"""Remove lock file before/after each test."""
def _cleanup_lock(self, tmp_path, monkeypatch) -> None: # type: ignore[no-untyped-def]
"""Point the lock at a per-test directory, and leave nothing behind."""
monkeypatch.setenv("SURREAL_MEMORY_DIR", str(tmp_path))
lock = _lock_path()
lock.unlink(missing_ok=True)
yield # type: ignore[misc]
lock.unlink(missing_ok=True)

def test_lock_lives_in_the_configured_data_dir(self, tmp_path) -> None: # type: ignore[no-untyped-def]
"""SURREAL_MEMORY_DIR must move the lock, like it moves everything else.

The autouse fixture already redirects it; this asserts the redirection
rather than assuming it, because the failure mode is silent — the lock
simply appears in the real home directory instead.
"""
assert _lock_path().parent == tmp_path.resolve()
assert _lock_path("some-brain").parent == tmp_path.resolve()
assert Path.home() / ".surrealmemory" != _lock_path().parent

def test_acquire_fresh_lock(self) -> None:
"""Should acquire lock when no lock exists."""
assert acquire_consolidation_lock() is True
Expand Down
Loading