fix(consolidation): the lock honours SURREAL_MEMORY_DIR like the rest of the data dir - #209
Merged
acidkill merged 1 commit intoSep 7, 2026
Conversation
… of the data dir
_lock_path derived its directory from Path.home() alone, so the consolidation
lock was the one piece of state a redirected data dir could not move.
Everything else in the package already honours the variable —
cli/config.get_default_data_dir and cli/update_check._get_cache_path both read
it.
Measured on SurrealDB-independent ground, by asking _lock_path where it would
put the file:
SURREAL_MEMORY_DIR=/tmp/throwaway -> ~/.surrealmemory/consolidation-<brain>.lock
A run against a throwaway brain therefore created and deleted files in the
operator's real ~/.surrealmemory. With the change the same call returns
/tmp/throwaway/consolidation-<brain>.lock.
The trade-off is worth naming rather than discovering: 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 operating on
separate data directories — but the lock is now per data dir rather than per
machine.
The test class gets a per-test directory and a test that asserts the
redirection rather than assuming it, since the failure mode is silent: the
lock simply reappears in the real home.
Its docstring is brought up to date at the same time, and the history is worth
stating rather than flattening. When acidkill#74 added xdist_group, the note it left
was accurate: these tests did share one real lock file and did race on it.
acidkill#121 then added a session-scoped $HOME redirect to conftest for unrelated
reasons, which gives every worker its own home and therefore its own lock
file. The race has not been reachable since, and nothing updated the note. So
the isolation was already there — implicitly, resting on a fixture that exists
for another purpose, which is the problem this fixture addresses. xdist_group
is kept as a guard against shared state being reintroduced.
acidkill
approved these changes
Sep 7, 2026
acidkill
left a comment
Owner
There was a problem hiding this comment.
Reviewed the full diff. The consolidation lock now honours SURREAL_MEMORY_DIR like every other piece of data-dir state, and the tests assert the redirection instead of assuming it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_lock_pathnow honoursSURREAL_MEMORY_DIR, so the consolidation lock moves with the data directory like everything else in the package.Why
_lock_pathderived its directory fromPath.home()alone. Everything else already reads the variable —cli/config.get_default_data_dirandcli/update_check._get_cache_pathboth do — which made the consolidation lock the one piece of state a redirected data dir could not move. A run against a throwaway brain still created and deleted files in the operator's real~/.surrealmemory.Measured by asking
_lock_pathwhere it would put the file, with the variable set:A trade-off, named rather than discovered
Two processes pointed at different
SURREAL_MEMORY_DIRvalues now take different locks and no longer exclude one another. That is the intended reading — they are operating on separate data directories — but it does mean the lock is per data directory rather than per machine. If you would rather it stayed machine-wide, this is the wrong shape and I would rather know now.Changes
utils/consolidation_lock.py: readSURREAL_MEMORY_DIR, falling back toPath.home()/".surrealmemory"as before, with the trade-off recorded in the docstring.tests/unit/test_multi_agent.py: the autouse fixture pointsSURREAL_MEMORY_DIRat a per-test directory, andtest_lock_lives_in_the_configured_data_dirasserts the redirection. The failure mode is silent — the lock simply reappears in the real home — so it is worth an explicit assertion.docs/reference/config.md: regenerated withscripts/gen_config_docs.py, since that file is generated from these defaults and theDocs Freshnessjob checks it.A correction to that class's own docstring
The docstring says these tests race on one shared lock file under
xdist, and thatxdist_grouppins the class to a single worker to prevent it. That was true when #74 wrote it, and is no longer true of the suite as it stands — which is worth stating carefully, because the note was not wrong, it was overtaken.#121 later added a session-scoped
$HOMEredirect toconftestfor unrelated reasons. That gives each worker its own home directory, and therefore its own lock file, so the race stopped being reachable and nothing updated the note. Checked directly: with$HOMEredirected the lock lands under the fake home onmaintoo, before any of this change.What the redirect does not do is make the intent explicit. A test relying on
$HOMEbeing someone else's problem is one refactor away from writing to a real home again, which is why the fixture now says so in its own terms.xdist_groupis kept as a guard against shared state being reintroduced, not as a live fix.Test plan
pytest tests/unit/test_multi_agent.py— 16 passed.utils/consolidation_lock.pyreverted tomainand the tests kept, exactly one fails:test_lock_lives_in_the_configured_data_dir. The other fifteen pass either way, which is the point — this change is meant to move a file, not alter locking behaviour.pytest tests/ -m "not stress" -n 4— 7284 passed, 48 skipped, 1 xfailed, which ismain's 7283 plus the single test added here. Two tests intests/unit/test_dashboard_brains_scope.pyfail on this branch and onmainalike: they want a live database and collide with one another under-n. Both pass when that file is run on its own.ruff check src/ tests/clean;ruff format --check src/ tests/reports 739 files already formatted.mypy src/ --ignore-missing-imports— success, no issues found in 354 source files.main.CHANGELOG.mduntouched — left to the release entry, as with fix(storage): bind datetimes in time comparisons so they select by value #191–fix(memory): refresh content-derived fields on compress, restore, and refine #193.Verified by
@RobertSigmundsson