From 72f6ebecbdd09144174fd5a2cc1eab541e29a738 Mon Sep 17 00:00:00 2001 From: Drew Cain Date: Tue, 10 Mar 2026 13:05:05 -0500 Subject: [PATCH 1/3] Invalidate config cache when file is modified by another process Add mtime-based cache validation so long-lived processes (like the MCP stdio server) detect when the config file has been modified externally (e.g. by `bm project set-cloud` in a separate terminal). This is a cheap os.stat() call per config access that only triggers a re-read when the file mtime has actually changed. Fixes #660 Co-Authored-By: Claude Opus 4.6 Signed-off-by: Drew Cain --- src/basic_memory/config.py | 38 +++++- test-int/conftest.py | 1 + tests/cli/conftest.py | 1 + tests/cli/test_json_output.py | 1 + tests/cli/test_project_add_with_local_path.py | 1 + tests/cli/test_project_list_and_ls.py | 1 + tests/cli/test_project_set_cloud_local.py | 12 ++ tests/cli/test_workspace_commands.py | 2 + tests/conftest.py | 1 + tests/services/test_project_service.py | 5 + tests/test_config.py | 113 ++++++++++++++++++ 11 files changed, 171 insertions(+), 5 deletions(-) diff --git a/src/basic_memory/config.py b/src/basic_memory/config.py index 006c9df7d..0042e9ccd 100644 --- a/src/basic_memory/config.py +++ b/src/basic_memory/config.py @@ -629,6 +629,10 @@ def data_dir_path(self) -> Path: # Module-level cache for configuration _CONFIG_CACHE: Optional[BasicMemoryConfig] = None +# Track config file mtime so cross-process changes (e.g. `bm project set-cloud` +# in a separate terminal) invalidate the cache in long-lived processes like the +# MCP stdio server. +_CONFIG_MTIME: Optional[float] = None class ConfigManager: @@ -662,13 +666,30 @@ def load_config(self) -> BasicMemoryConfig: Environment variables take precedence over file config values, following Pydantic Settings best practices. - Uses module-level cache for performance across ConfigManager instances. + Uses module-level cache with file mtime validation so that + cross-process config changes (e.g. `bm project set-cloud` in a + separate terminal) are picked up by long-lived processes like + the MCP stdio server. """ - global _CONFIG_CACHE + global _CONFIG_CACHE, _CONFIG_MTIME - # Return cached config if available + # Trigger: cached config exists but the on-disk file may have been + # modified by another process (CLI command in a different terminal). + # Why: the MCP server is long-lived; without this check it would + # serve stale project routing forever. + # Outcome: cheap os.stat() per access; re-read only when mtime differs. if _CONFIG_CACHE is not None: - return _CONFIG_CACHE + try: + current_mtime = self.config_file.stat().st_mtime + except OSError: + current_mtime = None + + if current_mtime is not None and current_mtime == _CONFIG_MTIME: + return _CONFIG_CACHE + + # mtime changed or file gone — invalidate and fall through to re-read + _CONFIG_CACHE = None + _CONFIG_MTIME = None if self.config_file.exists(): try: @@ -723,6 +744,12 @@ def load_config(self) -> BasicMemoryConfig: _CONFIG_CACHE = BasicMemoryConfig(**merged_data) + # Record mtime so subsequent calls detect cross-process changes + try: + _CONFIG_MTIME = self.config_file.stat().st_mtime + except OSError: + _CONFIG_MTIME = None + # Re-save to normalize legacy config into current format if needs_resave: # Create backup before overwriting so users can revert if needed @@ -753,10 +780,11 @@ def load_config(self) -> BasicMemoryConfig: def save_config(self, config: BasicMemoryConfig) -> None: """Save configuration to file and invalidate cache.""" - global _CONFIG_CACHE + global _CONFIG_CACHE, _CONFIG_MTIME save_basic_memory_config(self.config_file, config) # Invalidate cache so next load_config() reads fresh data _CONFIG_CACHE = None + _CONFIG_MTIME = None @property def projects(self) -> Dict[str, str]: diff --git a/test-int/conftest.py b/test-int/conftest.py index 8a98c94ea..b0f6e2f4c 100644 --- a/test-int/conftest.py +++ b/test-int/conftest.py @@ -258,6 +258,7 @@ def config_manager(app_config: BasicMemoryConfig, config_home) -> ConfigManager: from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None config_manager = ConfigManager() # Update its paths to use the test directory diff --git a/tests/cli/conftest.py b/tests/cli/conftest.py index 4b94e0746..f6c6f3265 100644 --- a/tests/cli/conftest.py +++ b/tests/cli/conftest.py @@ -25,6 +25,7 @@ def isolated_home(tmp_path, monkeypatch) -> Path: from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None monkeypatch.setenv("HOME", str(tmp_path)) if os.name == "nt": diff --git a/tests/cli/test_json_output.py b/tests/cli/test_json_output.py index 0928c27a6..76ab38c31 100644 --- a/tests/cli/test_json_output.py +++ b/tests/cli/test_json_output.py @@ -350,6 +350,7 @@ def _write(config_data: dict): from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/cli/test_project_add_with_local_path.py b/tests/cli/test_project_add_with_local_path.py index 1f93cf611..8d350e75b 100644 --- a/tests/cli/test_project_add_with_local_path.py +++ b/tests/cli/test_project_add_with_local_path.py @@ -27,6 +27,7 @@ def mock_config(tmp_path, monkeypatch): from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/cli/test_project_list_and_ls.py b/tests/cli/test_project_list_and_ls.py index be9f583d6..70abecf73 100644 --- a/tests/cli/test_project_list_and_ls.py +++ b/tests/cli/test_project_list_and_ls.py @@ -29,6 +29,7 @@ def _write(config_data: dict) -> Path: from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/cli/test_project_set_cloud_local.py b/tests/cli/test_project_set_cloud_local.py index b38c7dea5..0479a6516 100644 --- a/tests/cli/test_project_set_cloud_local.py +++ b/tests/cli/test_project_set_cloud_local.py @@ -22,6 +22,7 @@ def mock_config(tmp_path, monkeypatch): from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -68,6 +69,7 @@ def test_set_cloud_no_credentials(self, runner, tmp_path, monkeypatch): from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -91,6 +93,7 @@ def test_set_cloud_with_oauth_session(self, runner, tmp_path, monkeypatch): from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -161,11 +164,13 @@ def test_set_local_clears_workspace_id(self, runner, mock_config): # Manually set workspace_id on the project config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None config_data = json.loads(mock_config.read_text()) config_data["projects"]["research"]["mode"] = "cloud" config_data["projects"]["research"]["workspace_id"] = "11111111-1111-1111-1111-111111111111" mock_config.write_text(json.dumps(config_data, indent=2)) config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None # Set back to local result = runner.invoke(app, ["project", "set-local", "research"]) @@ -173,6 +178,7 @@ def test_set_local_clears_workspace_id(self, runner, mock_config): # Verify workspace_id was cleared config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None updated_data = json.loads(mock_config.read_text()) assert updated_data["projects"]["research"]["workspace_id"] is None assert updated_data["projects"]["research"]["mode"] == "local" @@ -187,6 +193,7 @@ def test_set_cloud_with_workspace_stores_workspace_id(self, runner, mock_config, from basic_memory.schemas.cloud import WorkspaceInfo config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None async def fake_get_available_workspaces(): return [ @@ -210,6 +217,7 @@ async def fake_get_available_workspaces(): # Verify workspace_id was persisted config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None updated_data = json.loads(mock_config.read_text()) assert ( updated_data["projects"]["research"]["workspace_id"] @@ -222,6 +230,7 @@ def test_set_cloud_with_workspace_not_found(self, runner, mock_config, monkeypat from basic_memory.schemas.cloud import WorkspaceInfo config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None async def fake_get_available_workspaces(): return [ @@ -249,17 +258,20 @@ def test_set_cloud_uses_default_workspace_when_no_flag(self, runner, mock_config from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None # Set default_workspace in config config_data = json.loads(mock_config.read_text()) config_data["default_workspace"] = "global-default-tenant-id" mock_config.write_text(json.dumps(config_data, indent=2)) config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None result = runner.invoke(app, ["project", "set-cloud", "research"]) assert result.exit_code == 0 # Verify workspace_id was set from default config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None updated_data = json.loads(mock_config.read_text()) assert updated_data["projects"]["research"]["workspace_id"] == "global-default-tenant-id" diff --git a/tests/cli/test_workspace_commands.py b/tests/cli/test_workspace_commands.py index a1285a71c..af411c6c9 100644 --- a/tests/cli/test_workspace_commands.py +++ b/tests/cli/test_workspace_commands.py @@ -76,6 +76,7 @@ def _setup_config(self, monkeypatch): monkeypatch.setenv("HOME", str(temp_path)) monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(config_dir)) basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None config_manager = ConfigManager() test_config = BasicMemoryConfig( @@ -106,6 +107,7 @@ async def fake_get_available_workspaces(context=None): # Verify config was updated basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None config = ConfigManager().config assert config.default_workspace == "11111111-1111-1111-1111-111111111111" diff --git a/tests/conftest.py b/tests/conftest.py index f19fb2425..1801fffc4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -138,6 +138,7 @@ def config_manager(app_config: BasicMemoryConfig, config_home: Path, monkeypatch from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None # Create a new ConfigManager that uses the test home directory config_manager = ConfigManager() diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index c5a86e86f..c0e61d086 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -778,6 +778,7 @@ async def test_add_project_with_project_root_sanitizes_paths( from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None test_cases = [ # (project_name, user_path, expected_sanitized_name) @@ -845,6 +846,7 @@ async def test_add_project_with_project_root_rejects_escape_attempts( from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None # All of these should succeed by being sanitized to paths under project_root # The sanitization removes dangerous patterns, so they don't escape @@ -931,6 +933,7 @@ async def test_add_project_with_project_root_normalizes_case( from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None test_cases = [ # (input_path, expected_normalized_path) @@ -985,6 +988,7 @@ async def test_add_project_with_project_root_detects_case_collisions( from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None # First, create a project with lowercase path first_project = "documents-project" @@ -1159,6 +1163,7 @@ async def test_add_project_nested_validation_with_project_root( from basic_memory import config as config_module config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None parent_project_name = f"cloud-parent-{os.urandom(4).hex()}" child_project_name = f"cloud-child-{os.urandom(4).hex()}" diff --git a/tests/test_config.py b/tests/test_config.py index a8d6488cf..ede1c4f92 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -213,6 +213,7 @@ def test_stale_default_project_loaded_from_file(self, config_home, monkeypatch): } config_manager.config_file.write_text(json.dumps(config_data, indent=2)) basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None loaded = config_manager.load_config() assert loaded.default_project == "research" @@ -238,6 +239,7 @@ def test_config_file_without_default_project_key(self, config_home, monkeypatch) } config_manager.config_file.write_text(json.dumps(config_data, indent=2)) basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None loaded = config_manager.load_config() assert loaded.default_project == "work" @@ -545,6 +547,7 @@ def test_backward_compatibility_loading_old_format_config(self): import basic_memory.config basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None # Should load successfully with migration to ProjectEntry config = config_manager.load_config() @@ -585,6 +588,7 @@ def test_backward_compatibility_migrates_project_modes_and_cloud_projects(self): import basic_memory.config basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None config = config_manager.load_config() @@ -617,6 +621,7 @@ def test_legacy_cloud_mode_key_is_stripped_on_normalization_save(self): import basic_memory.config basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None loaded = config_manager.load_config() assert isinstance(loaded, BasicMemoryConfig) @@ -647,6 +652,7 @@ def test_migration_creates_backup_of_old_config(self): import basic_memory.config basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None config_manager.load_config() @@ -678,6 +684,7 @@ def test_no_backup_when_config_is_current_format(self): import basic_memory.config basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None config_manager.load_config() @@ -1099,6 +1106,7 @@ def test_backward_compat_loading_old_format_without_project_modes(self): import basic_memory.config basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None # Should load successfully with migration config = config_manager.load_config() @@ -1174,6 +1182,111 @@ def test_workspace_fields_round_trip(self): assert loaded.projects["main"].workspace_id is None +class TestConfigCacheMtimeInvalidation: + """Test that config cache is invalidated when file is modified externally.""" + + def test_cache_returns_same_config_when_file_unchanged(self, config_home): + """Verify cache hit when config file mtime has not changed.""" + import basic_memory.config + + basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None + + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) + config_manager = ConfigManager() + config_manager.config_dir = temp_path / "basic-memory" + config_manager.config_file = config_manager.config_dir / "config.json" + config_manager.config_dir.mkdir(parents=True, exist_ok=True) + + test_config = BasicMemoryConfig( + projects={"main": {"path": str(temp_path / "main")}}, + default_project="main", + ) + config_manager.save_config(test_config) + + # First load populates cache + config1 = config_manager.load_config() + assert config1.default_project == "main" + + # Second load should return cached config (same object) + config2 = config_manager.load_config() + assert config1 is config2 + + def test_cache_invalidated_when_file_modified(self, config_home): + """Verify cache miss when config file is modified by another process.""" + import json + import os + import time + + import basic_memory.config + + basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None + + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) + config_manager = ConfigManager() + config_manager.config_dir = temp_path / "basic-memory" + config_manager.config_file = config_manager.config_dir / "config.json" + config_manager.config_dir.mkdir(parents=True, exist_ok=True) + + test_config = BasicMemoryConfig( + projects={"main": {"path": str(temp_path / "main")}}, + default_project="main", + ) + config_manager.save_config(test_config) + + # First load populates cache + config1 = config_manager.load_config() + assert config1.get_project_mode("main") == ProjectMode.LOCAL + + # Simulate external process modifying the config file + config_data = json.loads(config_manager.config_file.read_text()) + config_data["projects"]["main"]["mode"] = "cloud" + + # Ensure mtime actually changes (some filesystems have 1s granularity) + time.sleep(0.05) + config_manager.config_file.write_text(json.dumps(config_data, indent=2)) + # Force mtime change on filesystems with coarse granularity + new_mtime = os.path.getmtime(config_manager.config_file) + 1 + os.utime(config_manager.config_file, (new_mtime, new_mtime)) + + # Next load should detect mtime change and re-read + config2 = config_manager.load_config() + assert config2.get_project_mode("main") == ProjectMode.CLOUD + assert config1 is not config2 + + def test_save_config_resets_mtime(self, config_home): + """Verify save_config clears both cache and mtime.""" + import basic_memory.config + + basic_memory.config._CONFIG_CACHE = None + basic_memory.config._CONFIG_MTIME = None + + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) + config_manager = ConfigManager() + config_manager.config_dir = temp_path / "basic-memory" + config_manager.config_file = config_manager.config_dir / "config.json" + config_manager.config_dir.mkdir(parents=True, exist_ok=True) + + test_config = BasicMemoryConfig( + projects={"main": {"path": str(temp_path / "main")}}, + ) + config_manager.save_config(test_config) + + # Load to populate cache + config_manager.load_config() + assert basic_memory.config._CONFIG_CACHE is not None + assert basic_memory.config._CONFIG_MTIME is not None + + # Save should clear both + config_manager.save_config(test_config) + assert basic_memory.config._CONFIG_CACHE is None + assert basic_memory.config._CONFIG_MTIME is None + + class TestLocalSyncPathMigration: """Test migration that promotes local_sync_path into path for cloud projects.""" From b3fc169babbf38954662fd151ebc4f889ec5a0dd Mon Sep 17 00:00:00 2001 From: Drew Cain Date: Tue, 10 Mar 2026 14:15:58 -0500 Subject: [PATCH 2/3] Add file size to cache invalidation check Addresses Codex review feedback: on filesystems with coarse mtime granularity (1s resolution), two writes within the same second would share the same mtime. Adding file size to the check catches config changes even when mtime doesn't change, since config edits almost always change file size. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Drew Cain --- src/basic_memory/config.py | 34 +++++++++++++------ test-int/conftest.py | 1 + tests/cli/conftest.py | 1 + tests/cli/test_json_output.py | 1 + tests/cli/test_project_add_with_local_path.py | 1 + tests/cli/test_project_list_and_ls.py | 1 + tests/cli/test_project_set_cloud_local.py | 12 +++++++ tests/cli/test_workspace_commands.py | 2 ++ tests/conftest.py | 1 + tests/services/test_project_service.py | 5 +++ tests/test_config.py | 15 +++++++- 11 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/basic_memory/config.py b/src/basic_memory/config.py index 0042e9ccd..fc4451d5a 100644 --- a/src/basic_memory/config.py +++ b/src/basic_memory/config.py @@ -629,10 +629,12 @@ def data_dir_path(self) -> Path: # Module-level cache for configuration _CONFIG_CACHE: Optional[BasicMemoryConfig] = None -# Track config file mtime so cross-process changes (e.g. `bm project set-cloud` +# Track config file mtime+size so cross-process changes (e.g. `bm project set-cloud` # in a separate terminal) invalidate the cache in long-lived processes like the -# MCP stdio server. +# MCP stdio server. Using both mtime and size guards against coarse-granularity +# filesystems where two writes within the same second share the same mtime. _CONFIG_MTIME: Optional[float] = None +_CONFIG_SIZE: Optional[int] = None class ConfigManager: @@ -671,25 +673,33 @@ def load_config(self) -> BasicMemoryConfig: separate terminal) are picked up by long-lived processes like the MCP stdio server. """ - global _CONFIG_CACHE, _CONFIG_MTIME + global _CONFIG_CACHE, _CONFIG_MTIME, _CONFIG_SIZE # Trigger: cached config exists but the on-disk file may have been # modified by another process (CLI command in a different terminal). # Why: the MCP server is long-lived; without this check it would # serve stale project routing forever. - # Outcome: cheap os.stat() per access; re-read only when mtime differs. + # Outcome: cheap os.stat() per access; re-read only when mtime or size differs. if _CONFIG_CACHE is not None: try: - current_mtime = self.config_file.stat().st_mtime + st = self.config_file.stat() + current_mtime = st.st_mtime + current_size = st.st_size except OSError: current_mtime = None + current_size = None - if current_mtime is not None and current_mtime == _CONFIG_MTIME: + if ( + current_mtime is not None + and current_mtime == _CONFIG_MTIME + and current_size == _CONFIG_SIZE + ): return _CONFIG_CACHE - # mtime changed or file gone — invalidate and fall through to re-read + # mtime/size changed or file gone — invalidate and fall through to re-read _CONFIG_CACHE = None _CONFIG_MTIME = None + _CONFIG_SIZE = None if self.config_file.exists(): try: @@ -744,11 +754,14 @@ def load_config(self) -> BasicMemoryConfig: _CONFIG_CACHE = BasicMemoryConfig(**merged_data) - # Record mtime so subsequent calls detect cross-process changes + # Record mtime+size so subsequent calls detect cross-process changes try: - _CONFIG_MTIME = self.config_file.stat().st_mtime + st = self.config_file.stat() + _CONFIG_MTIME = st.st_mtime + _CONFIG_SIZE = st.st_size except OSError: _CONFIG_MTIME = None + _CONFIG_SIZE = None # Re-save to normalize legacy config into current format if needs_resave: @@ -780,11 +793,12 @@ def load_config(self) -> BasicMemoryConfig: def save_config(self, config: BasicMemoryConfig) -> None: """Save configuration to file and invalidate cache.""" - global _CONFIG_CACHE, _CONFIG_MTIME + global _CONFIG_CACHE, _CONFIG_MTIME, _CONFIG_SIZE save_basic_memory_config(self.config_file, config) # Invalidate cache so next load_config() reads fresh data _CONFIG_CACHE = None _CONFIG_MTIME = None + _CONFIG_SIZE = None @property def projects(self) -> Dict[str, str]: diff --git a/test-int/conftest.py b/test-int/conftest.py index b0f6e2f4c..07f5af384 100644 --- a/test-int/conftest.py +++ b/test-int/conftest.py @@ -259,6 +259,7 @@ def config_manager(app_config: BasicMemoryConfig, config_home) -> ConfigManager: config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None config_manager = ConfigManager() # Update its paths to use the test directory diff --git a/tests/cli/conftest.py b/tests/cli/conftest.py index f6c6f3265..6d1c635c6 100644 --- a/tests/cli/conftest.py +++ b/tests/cli/conftest.py @@ -26,6 +26,7 @@ def isolated_home(tmp_path, monkeypatch) -> Path: config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None monkeypatch.setenv("HOME", str(tmp_path)) if os.name == "nt": diff --git a/tests/cli/test_json_output.py b/tests/cli/test_json_output.py index 76ab38c31..7a33988ea 100644 --- a/tests/cli/test_json_output.py +++ b/tests/cli/test_json_output.py @@ -351,6 +351,7 @@ def _write(config_data: dict): config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/cli/test_project_add_with_local_path.py b/tests/cli/test_project_add_with_local_path.py index 8d350e75b..aace06ad8 100644 --- a/tests/cli/test_project_add_with_local_path.py +++ b/tests/cli/test_project_add_with_local_path.py @@ -28,6 +28,7 @@ def mock_config(tmp_path, monkeypatch): config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/cli/test_project_list_and_ls.py b/tests/cli/test_project_list_and_ls.py index 70abecf73..02b2c5572 100644 --- a/tests/cli/test_project_list_and_ls.py +++ b/tests/cli/test_project_list_and_ls.py @@ -30,6 +30,7 @@ def _write(config_data: dict) -> Path: config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/cli/test_project_set_cloud_local.py b/tests/cli/test_project_set_cloud_local.py index 0479a6516..39a507848 100644 --- a/tests/cli/test_project_set_cloud_local.py +++ b/tests/cli/test_project_set_cloud_local.py @@ -23,6 +23,7 @@ def mock_config(tmp_path, monkeypatch): config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -70,6 +71,7 @@ def test_set_cloud_no_credentials(self, runner, tmp_path, monkeypatch): config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -94,6 +96,7 @@ def test_set_cloud_with_oauth_session(self, runner, tmp_path, monkeypatch): config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -165,12 +168,14 @@ def test_set_local_clears_workspace_id(self, runner, mock_config): # Manually set workspace_id on the project config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None config_data = json.loads(mock_config.read_text()) config_data["projects"]["research"]["mode"] = "cloud" config_data["projects"]["research"]["workspace_id"] = "11111111-1111-1111-1111-111111111111" mock_config.write_text(json.dumps(config_data, indent=2)) config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None # Set back to local result = runner.invoke(app, ["project", "set-local", "research"]) @@ -179,6 +184,7 @@ def test_set_local_clears_workspace_id(self, runner, mock_config): # Verify workspace_id was cleared config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None updated_data = json.loads(mock_config.read_text()) assert updated_data["projects"]["research"]["workspace_id"] is None assert updated_data["projects"]["research"]["mode"] == "local" @@ -194,6 +200,7 @@ def test_set_cloud_with_workspace_stores_workspace_id(self, runner, mock_config, config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None async def fake_get_available_workspaces(): return [ @@ -218,6 +225,7 @@ async def fake_get_available_workspaces(): # Verify workspace_id was persisted config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None updated_data = json.loads(mock_config.read_text()) assert ( updated_data["projects"]["research"]["workspace_id"] @@ -231,6 +239,7 @@ def test_set_cloud_with_workspace_not_found(self, runner, mock_config, monkeypat config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None async def fake_get_available_workspaces(): return [ @@ -259,6 +268,7 @@ def test_set_cloud_uses_default_workspace_when_no_flag(self, runner, mock_config config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None # Set default_workspace in config config_data = json.loads(mock_config.read_text()) @@ -266,6 +276,7 @@ def test_set_cloud_uses_default_workspace_when_no_flag(self, runner, mock_config mock_config.write_text(json.dumps(config_data, indent=2)) config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None result = runner.invoke(app, ["project", "set-cloud", "research"]) assert result.exit_code == 0 @@ -273,5 +284,6 @@ def test_set_cloud_uses_default_workspace_when_no_flag(self, runner, mock_config # Verify workspace_id was set from default config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None updated_data = json.loads(mock_config.read_text()) assert updated_data["projects"]["research"]["workspace_id"] == "global-default-tenant-id" diff --git a/tests/cli/test_workspace_commands.py b/tests/cli/test_workspace_commands.py index af411c6c9..ef5616acc 100644 --- a/tests/cli/test_workspace_commands.py +++ b/tests/cli/test_workspace_commands.py @@ -77,6 +77,7 @@ def _setup_config(self, monkeypatch): monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(config_dir)) basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None config_manager = ConfigManager() test_config = BasicMemoryConfig( @@ -108,6 +109,7 @@ async def fake_get_available_workspaces(context=None): # Verify config was updated basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None config = ConfigManager().config assert config.default_workspace == "11111111-1111-1111-1111-111111111111" diff --git a/tests/conftest.py b/tests/conftest.py index 1801fffc4..4c2b261ba 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -139,6 +139,7 @@ def config_manager(app_config: BasicMemoryConfig, config_home: Path, monkeypatch config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None # Create a new ConfigManager that uses the test home directory config_manager = ConfigManager() diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index c0e61d086..8e58f8480 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -779,6 +779,7 @@ async def test_add_project_with_project_root_sanitizes_paths( config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None test_cases = [ # (project_name, user_path, expected_sanitized_name) @@ -847,6 +848,7 @@ async def test_add_project_with_project_root_rejects_escape_attempts( config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None # All of these should succeed by being sanitized to paths under project_root # The sanitization removes dangerous patterns, so they don't escape @@ -934,6 +936,7 @@ async def test_add_project_with_project_root_normalizes_case( config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None test_cases = [ # (input_path, expected_normalized_path) @@ -989,6 +992,7 @@ async def test_add_project_with_project_root_detects_case_collisions( config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None # First, create a project with lowercase path first_project = "documents-project" @@ -1164,6 +1168,7 @@ async def test_add_project_nested_validation_with_project_root( config_module._CONFIG_CACHE = None config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None parent_project_name = f"cloud-parent-{os.urandom(4).hex()}" child_project_name = f"cloud-child-{os.urandom(4).hex()}" diff --git a/tests/test_config.py b/tests/test_config.py index ede1c4f92..7b698c11a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -214,6 +214,7 @@ def test_stale_default_project_loaded_from_file(self, config_home, monkeypatch): config_manager.config_file.write_text(json.dumps(config_data, indent=2)) basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None loaded = config_manager.load_config() assert loaded.default_project == "research" @@ -240,6 +241,7 @@ def test_config_file_without_default_project_key(self, config_home, monkeypatch) config_manager.config_file.write_text(json.dumps(config_data, indent=2)) basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None loaded = config_manager.load_config() assert loaded.default_project == "work" @@ -548,6 +550,7 @@ def test_backward_compatibility_loading_old_format_config(self): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None # Should load successfully with migration to ProjectEntry config = config_manager.load_config() @@ -589,6 +592,7 @@ def test_backward_compatibility_migrates_project_modes_and_cloud_projects(self): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None config = config_manager.load_config() @@ -622,6 +626,7 @@ def test_legacy_cloud_mode_key_is_stripped_on_normalization_save(self): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None loaded = config_manager.load_config() assert isinstance(loaded, BasicMemoryConfig) @@ -653,6 +658,7 @@ def test_migration_creates_backup_of_old_config(self): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None config_manager.load_config() @@ -685,6 +691,7 @@ def test_no_backup_when_config_is_current_format(self): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None config_manager.load_config() @@ -1107,6 +1114,7 @@ def test_backward_compat_loading_old_format_without_project_modes(self): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None # Should load successfully with migration config = config_manager.load_config() @@ -1191,6 +1199,7 @@ def test_cache_returns_same_config_when_file_unchanged(self, config_home): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None with tempfile.TemporaryDirectory() as temp_dir: temp_path = Path(temp_dir) @@ -1223,6 +1232,7 @@ def test_cache_invalidated_when_file_modified(self, config_home): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None with tempfile.TemporaryDirectory() as temp_dir: temp_path = Path(temp_dir) @@ -1263,6 +1273,7 @@ def test_save_config_resets_mtime(self, config_home): basic_memory.config._CONFIG_CACHE = None basic_memory.config._CONFIG_MTIME = None + basic_memory.config._CONFIG_SIZE = None with tempfile.TemporaryDirectory() as temp_dir: temp_path = Path(temp_dir) @@ -1280,11 +1291,13 @@ def test_save_config_resets_mtime(self, config_home): config_manager.load_config() assert basic_memory.config._CONFIG_CACHE is not None assert basic_memory.config._CONFIG_MTIME is not None + assert basic_memory.config._CONFIG_SIZE is not None - # Save should clear both + # Save should clear all cache state config_manager.save_config(test_config) assert basic_memory.config._CONFIG_CACHE is None assert basic_memory.config._CONFIG_MTIME is None + assert basic_memory.config._CONFIG_SIZE is None class TestLocalSyncPathMigration: From 6e7cfc746fa36b15ba967279b46f8cf6effe4635 Mon Sep 17 00:00:00 2001 From: Drew Cain Date: Tue, 10 Mar 2026 21:03:26 -0500 Subject: [PATCH 3/3] fix(test): pin mtime+size when injecting config cache in write_note tests The mtime+size cache guard introduced in this branch would discard injected _CONFIG_CACHE values because float("inf") never matches any real file mtime. Use the actual config file's mtime and size so the guard sees a match and keeps the injected config. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Drew Cain --- tests/mcp/test_tool_write_note.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/mcp/test_tool_write_note.py b/tests/mcp/test_tool_write_note.py index ed95886c9..a4d29b8d8 100644 --- a/tests/mcp/test_tool_write_note.py +++ b/tests/mcp/test_tool_write_note.py @@ -1257,6 +1257,11 @@ async def test_write_note_config_overwrite_default_true( # Set config to allow overwrites by default app_config.write_note_overwrite_default = True config_module._CONFIG_CACHE = app_config + # Pin mtime+size to the on-disk file so the cache guard sees a match + # and keeps our injected config instead of re-reading from disk. + _st = config_manager.config_file.stat() + config_module._CONFIG_MTIME = _st.st_mtime + config_module._CONFIG_SIZE = _st.st_size try: await write_note( @@ -1281,6 +1286,9 @@ async def test_write_note_config_overwrite_default_true( # Restore config app_config.write_note_overwrite_default = False config_module._CONFIG_CACHE = app_config + _st = config_manager.config_file.stat() + config_module._CONFIG_MTIME = _st.st_mtime + config_module._CONFIG_SIZE = _st.st_size @pytest.mark.asyncio async def test_write_note_new_note_unaffected(self, app, test_project):