diff --git a/src/basic_memory/config.py b/src/basic_memory/config.py index 006c9df7..fc4451d5 100644 --- a/src/basic_memory/config.py +++ b/src/basic_memory/config.py @@ -629,6 +629,12 @@ def data_dir_path(self) -> Path: # Module-level cache for configuration _CONFIG_CACHE: Optional[BasicMemoryConfig] = None +# 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. 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: @@ -662,13 +668,38 @@ 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, _CONFIG_SIZE - # 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 or size differs. if _CONFIG_CACHE is not None: - return _CONFIG_CACHE + try: + 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 + and current_size == _CONFIG_SIZE + ): + return _CONFIG_CACHE + + # 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: @@ -723,6 +754,15 @@ def load_config(self) -> BasicMemoryConfig: _CONFIG_CACHE = BasicMemoryConfig(**merged_data) + # Record mtime+size so subsequent calls detect cross-process changes + try: + 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: # Create backup before overwriting so users can revert if needed @@ -753,10 +793,12 @@ 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, _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 8a98c94e..07f5af38 100644 --- a/test-int/conftest.py +++ b/test-int/conftest.py @@ -258,6 +258,8 @@ 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_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 4b94e074..6d1c635c 100644 --- a/tests/cli/conftest.py +++ b/tests/cli/conftest.py @@ -25,6 +25,8 @@ 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 + 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 0928c27a..7a33988e 100644 --- a/tests/cli/test_json_output.py +++ b/tests/cli/test_json_output.py @@ -350,6 +350,8 @@ def _write(config_data: dict): from basic_memory import config as config_module 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 1f93cf61..aace06ad 100644 --- a/tests/cli/test_project_add_with_local_path.py +++ b/tests/cli/test_project_add_with_local_path.py @@ -27,6 +27,8 @@ 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_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 be9f583d..02b2c557 100644 --- a/tests/cli/test_project_list_and_ls.py +++ b/tests/cli/test_project_list_and_ls.py @@ -29,6 +29,8 @@ 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_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 b38c7dea..39a50784 100644 --- a/tests/cli/test_project_set_cloud_local.py +++ b/tests/cli/test_project_set_cloud_local.py @@ -22,6 +22,8 @@ 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_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -68,6 +70,8 @@ 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_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -91,6 +95,8 @@ 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_module._CONFIG_SIZE = None config_dir = tmp_path / ".basic-memory" config_dir.mkdir(parents=True, exist_ok=True) @@ -161,11 +167,15 @@ 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"]) @@ -173,6 +183,8 @@ 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" @@ -187,6 +199,8 @@ 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 + config_module._CONFIG_SIZE = None async def fake_get_available_workspaces(): return [ @@ -210,6 +224,8 @@ 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"] @@ -222,6 +238,8 @@ 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 + config_module._CONFIG_SIZE = None async def fake_get_available_workspaces(): return [ @@ -249,17 +267,23 @@ 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 + config_module._CONFIG_SIZE = 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 + config_module._CONFIG_SIZE = 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 + 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 a1285a71..ef5616ac 100644 --- a/tests/cli/test_workspace_commands.py +++ b/tests/cli/test_workspace_commands.py @@ -76,6 +76,8 @@ 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 + basic_memory.config._CONFIG_SIZE = None config_manager = ConfigManager() test_config = BasicMemoryConfig( @@ -106,6 +108,8 @@ 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 f19fb242..4c2b261b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -138,6 +138,8 @@ 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 + config_module._CONFIG_SIZE = None # Create a new ConfigManager that uses the test home directory config_manager = ConfigManager() diff --git a/tests/mcp/test_tool_write_note.py b/tests/mcp/test_tool_write_note.py index ed95886c..a4d29b8d 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): diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index c5a86e86..8e58f848 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -778,6 +778,8 @@ 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 + config_module._CONFIG_SIZE = None test_cases = [ # (project_name, user_path, expected_sanitized_name) @@ -845,6 +847,8 @@ 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 + 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 @@ -931,6 +935,8 @@ 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 + config_module._CONFIG_SIZE = None test_cases = [ # (input_path, expected_normalized_path) @@ -985,6 +991,8 @@ 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 + config_module._CONFIG_SIZE = None # First, create a project with lowercase path first_project = "documents-project" @@ -1159,6 +1167,8 @@ 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 + 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 a8d6488c..7b698c11 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -213,6 +213,8 @@ 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" @@ -238,6 +240,8 @@ 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" @@ -545,6 +549,8 @@ 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 + basic_memory.config._CONFIG_SIZE = None # Should load successfully with migration to ProjectEntry config = config_manager.load_config() @@ -585,6 +591,8 @@ 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 + basic_memory.config._CONFIG_SIZE = None config = config_manager.load_config() @@ -617,6 +625,8 @@ 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 + basic_memory.config._CONFIG_SIZE = None loaded = config_manager.load_config() assert isinstance(loaded, BasicMemoryConfig) @@ -647,6 +657,8 @@ 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 + basic_memory.config._CONFIG_SIZE = None config_manager.load_config() @@ -678,6 +690,8 @@ 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 + basic_memory.config._CONFIG_SIZE = None config_manager.load_config() @@ -1099,6 +1113,8 @@ 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 + basic_memory.config._CONFIG_SIZE = None # Should load successfully with migration config = config_manager.load_config() @@ -1174,6 +1190,116 @@ 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 + basic_memory.config._CONFIG_SIZE = 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 + basic_memory.config._CONFIG_SIZE = 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 + basic_memory.config._CONFIG_SIZE = 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 + assert basic_memory.config._CONFIG_SIZE is not None + + # 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: """Test migration that promotes local_sync_path into path for cloud projects."""