diff --git a/tests/tools/test_tool_manifest_cache.py b/tests/tools/test_tool_manifest_cache.py index 6ddd118bb44..75bb760de88 100644 --- a/tests/tools/test_tool_manifest_cache.py +++ b/tests/tools/test_tool_manifest_cache.py @@ -64,6 +64,19 @@ class TestManifestCache: assert loaded is not None assert loaded == ["tools.alpha", "tools.beta"] + def test_cache_miss_when_tools_dir_differs(self, tool_dir, cache_path, tmp_path): + """A manifest built for one checkout must not serve another checkout.""" + module = _load_registry_module() + module._save_manifest_cache(cache_path, tools_dir=tool_dir, module_names=["tools.alpha", "tools.beta"]) + other = tmp_path / "other" / "tools" + other.mkdir(parents=True) + # Mirror the exact files/mtimes so only tools_dir differs. + import shutil + for p in tool_dir.glob("*.py"): + dest = other / p.name + shutil.copy2(p, dest) + assert module._load_manifest_cache(cache_path, tools_dir=other) is None + def test_cache_miss_when_file_missing(self, tool_dir, cache_path): module = _load_registry_module() assert module._load_manifest_cache(cache_path, tools_dir=tool_dir) is None diff --git a/tools/registry.py b/tools/registry.py index 8eb18bc6e00..b2f09fb1584 100644 --- a/tools/registry.py +++ b/tools/registry.py @@ -98,7 +98,11 @@ def _save_manifest_cache(cache_path: Path, tools_dir: Path, module_names: List[s "module_names": module_names, "mtimes": _collect_tool_file_mtimes(tools_dir), } - cache_path.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n", encoding="utf-8") + # Atomic replace so a concurrent reader (gateway + CLI + cron can all + # cold-start at once) never sees a torn/partial JSON file. + tmp_path = cache_path.with_name(cache_path.name + f".tmp{os.getpid()}") + tmp_path.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n", encoding="utf-8") + os.replace(tmp_path, cache_path) except Exception: logger.debug("Failed to save tool manifest cache", exc_info=True) @@ -113,6 +117,12 @@ def _load_manifest_cache(cache_path: Path, tools_dir: Path) -> Optional[List[str return None if not isinstance(data, dict) or data.get("version") != 1: return None + # A shared HERMES_HOME can be used by several checkouts (main clone + + # worktrees). The manifest is only valid for the tools dir it was built + # from — otherwise identical mtimes across copies could serve a stale + # module list from a different tree. + if data.get("tools_dir") != str(tools_dir): + return None cached_mtimes = data.get("mtimes", {}) current_mtimes = _collect_tool_file_mtimes(tools_dir) if cached_mtimes != current_mtimes: