diff --git a/scripts/verify-cache-restamp.py b/scripts/verify-cache-restamp.py new file mode 100755 index 0000000..dd9f84e --- /dev/null +++ b/scripts/verify-cache-restamp.py @@ -0,0 +1,223 @@ +#!/usr/bin/env -S uv run python +"""verify-cache-restamp.py -- regression gate for the cross-installation +stale-cache FileNotFoundError (see bundle/cache.py::_restamp_agent_source_paths). + +BUG THIS GUARDS + ``load_and_prepare_cached``'s on-disk cache + (``$AMPLIFIER_AGENT_HOME/cache/prepared///``) + is keyed ONLY by (aaa_version, sha256(bundle.md content)) -- a per-user + key, not a per-installation one. ``PreparedBundle.mount_plan["agents"] + [name]["source_path"]`` is an ABSOLUTE path baked in at cold-prepare time + against whichever installation happened to run the cold path. + + A second installation of the same version (a fresh `uv tool install` + after a prior one was removed, a dev checkout alongside a packaged + install, two side-by-side venvs, ...) warm-hits the SAME cache entry and + gets back a ``PreparedBundle`` whose agent ``source_path``s point at the + FIRST installation's site-packages tree -- which may no longer exist. + ``make_turn_handler`` reads that path directly + (``hydrate_agent_overlay(Path(entry["source_path"]))``), so the very + first turn on the second installation raises:: + + FileNotFoundError: [Errno 2] No such file or directory: + '/amplifier_agent_lib/bundle/agents/explorer.md' + + even though the SECOND installation ships that exact file at its own, + different, path. + +CONVENTION + Standalone verification script, not a pytest test -- ``tests/`` means + "e2e contract tests" in this repo (see pytest.ini_options in + pyproject.toml); this is a fast, hermetic, dependency-light regression + gate for one function, matching ``scripts/verify-wheel.py``'s own + rationale for living here instead. + +WHAT IT CHECKS + 1. ``_restamp_agent_source_paths`` overwrites a stale (nonexistent) + ``source_path`` with the current installation's real + ``AGENTS_DIR/.md`` path. + 2. It leaves an entry alone when the current installation has no file + for that agent name (nothing to restamp to -- never invents a path). + 3. It is a no-op on a mount plan with no ``"agents"`` section. + 4. End-to-end: ``load_and_prepare_cached`` against a pickled artifact + whose ``source_path`` values were built for a DIFFERENT (deleted) + directory returns a ``PreparedBundle`` whose paths are fixed up to + this process's own vendored agent files -- reading them back + succeeds instead of raising ``FileNotFoundError``. + +USAGE + ./scripts/verify-cache-restamp.py # from repo root, no arguments + uv run scripts/verify-cache-restamp.py # equivalent + +EXIT CODES + 0 all checks passed + 1 a regression was detected +""" + +from __future__ import annotations + +import asyncio +import hashlib +import json +import pickle +import sys +import tempfile +from pathlib import Path +from types import SimpleNamespace + +REPO_ROOT = Path(__file__).resolve().parents[1] +SRC_ROOT = REPO_ROOT / "src" +sys.path.insert(0, str(SRC_ROOT)) + +from amplifier_agent_lib.bundle import AGENTS_DIR # noqa: E402 +from amplifier_agent_lib.bundle.cache import ( # noqa: E402 + _ARTIFACT_NAME, + _MANIFEST_NAME, + _restamp_agent_source_paths, + cache_dir_for_version, + load_and_prepare_cached, +) + + +class Failure(Exception): + """A verification check failed, with an actionable message.""" + + +def _fake_prepared(agents: dict) -> SimpleNamespace: + """Build a minimal stand-in with the one attribute the function reads.""" + return SimpleNamespace(mount_plan={"agents": agents}) + + +def check_restamps_stale_path() -> str: + real_name = next(p.stem for p in AGENTS_DIR.glob("*.md")) + stale_path = "/nonexistent-installation-xyz/amplifier_agent_lib/bundle/agents/" + real_name + ".md" + prepared = _fake_prepared({real_name: {"name": real_name, "source_path": stale_path}}) + + _restamp_agent_source_paths(prepared) + + fixed = prepared.mount_plan["agents"][real_name]["source_path"] + expected = str(AGENTS_DIR / f"{real_name}.md") + if fixed != expected: + raise Failure(f"expected source_path restamped to {expected!r}, got {fixed!r}") + if not Path(fixed).exists(): + raise Failure(f"restamped source_path {fixed!r} does not actually exist") + return f"stale path for {real_name!r} restamped to the current installation's own file" + + +def check_leaves_unknown_agent_alone() -> str: + stale_path = "/nonexistent-installation-xyz/amplifier_agent_lib/bundle/agents/no-such-agent.md" + prepared = _fake_prepared({"no-such-agent": {"name": "no-such-agent", "source_path": stale_path}}) + + _restamp_agent_source_paths(prepared) + + unchanged = prepared.mount_plan["agents"]["no-such-agent"]["source_path"] + if unchanged != stale_path: + raise Failure(f"expected untouched (no file to restamp to), got {unchanged!r}") + return "unresolvable agent name left as-is rather than pointed at an invented path" + + +def check_noop_without_agents_section() -> str: + prepared = SimpleNamespace(mount_plan={"tools": []}) + _restamp_agent_source_paths(prepared) # must not raise + if "agents" in prepared.mount_plan: + raise Failure("an 'agents' key was unexpectedly added") + return "no-op when mount_plan carries no 'agents' section" + + +def check_end_to_end_warm_path_survives_relocation() -> str: + """Build a pickle whose paths point at a directory that no longer exists, + then confirm the warm path fixes it up rather than raising. + """ + real_names = sorted(p.stem for p in AGENTS_DIR.glob("*.md")) + if not real_names: + raise Failure(f"no vendored agent .md files found under {AGENTS_DIR}") + + aaa_version = "0.0.0-verify-cache-restamp" + + with tempfile.TemporaryDirectory() as home_tmp: + import os + + env_backup = os.environ.get("AMPLIFIER_AGENT_HOME") + os.environ["AMPLIFIER_AGENT_HOME"] = home_tmp + try: + cache_dir = cache_dir_for_version(aaa_version) + cache_dir.mkdir(parents=True, exist_ok=True) + + # A directory that is guaranteed not to exist -- simulates a + # since-removed prior installation. + deleted_install = Path(tempfile.mkdtemp()) + deleted_install.rmdir() + + stale_agents = { + name: {"name": name, "source_path": str(deleted_install / f"{name}.md")} for name in real_names + } + fake_prepared = _fake_prepared(stale_agents) + + artifact = cache_dir / _ARTIFACT_NAME + manifest = cache_dir / _MANIFEST_NAME + artifact.write_bytes(pickle.dumps(fake_prepared)) + bundle_hash = hashlib.sha256((SRC_ROOT / "amplifier_agent_lib/bundle/bundle.md").read_bytes()).hexdigest()[ + :16 + ] + manifest.write_text(json.dumps({"aaa_version": aaa_version, "bundle_sha256_prefix": bundle_hash})) + + result = asyncio.run(load_and_prepare_cached(aaa_version=aaa_version)) + + for name in real_names: + source_path = result.mount_plan["agents"][name]["source_path"] + if not Path(source_path).exists(): + raise Failure( + f"warm path for {name!r} still points at a nonexistent path: {source_path!r} " + "(cross-installation stale-cache FileNotFoundError is NOT fixed)" + ) + # Actually read it, mirroring hydrate_agent_overlay's own read. + Path(source_path).read_text(encoding="utf-8-sig") + finally: + if env_backup is None: + os.environ.pop("AMPLIFIER_AGENT_HOME", None) + else: + os.environ["AMPLIFIER_AGENT_HOME"] = env_backup + + return f"warm path survives a relocated/deleted prior installation for all {len(real_names)} agents" + + +def main() -> int: + print("verify-cache-restamp: exercising _restamp_agent_source_paths + load_and_prepare_cached") + print(f" repo root: {REPO_ROOT}\n") + + checks = [ + ("restamp a stale path", check_restamps_stale_path), + ("leave an unresolvable agent alone", check_leaves_unknown_agent_alone), + ("no-op without an agents section", check_noop_without_agents_section), + ("end-to-end: warm path survives relocation", check_end_to_end_warm_path_survives_relocation), + ] + + failures: list[tuple[str, str]] = [] + for label, fn in checks: + try: + detail = fn() + except Failure as exc: + print(f" FAIL {label}") + failures.append((label, str(exc))) + except Exception as exc: # unexpected -- still report, don't crash silently + print(f" FAIL {label} (unexpected {type(exc).__name__})") + failures.append((label, str(exc))) + else: + print(f" OK {label}: {detail}") + + print() + if failures: + for label, message in failures: + print(f"FAIL: {label}\n\n{message}\n", file=sys.stderr) + print( + f"verify-cache-restamp: FAIL -- {len(failures)} of {len(checks)} checks failed.", + file=sys.stderr, + ) + return 1 + + print(f"verify-cache-restamp: PASS -- all {len(checks)} checks passed.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/amplifier_agent_lib/bundle/cache.py b/src/amplifier_agent_lib/bundle/cache.py index 6836367..25f055d 100644 --- a/src/amplifier_agent_lib/bundle/cache.py +++ b/src/amplifier_agent_lib/bundle/cache.py @@ -7,7 +7,7 @@ prepared.pickle — pickle.dumps(PreparedBundle) manifest.json — { "aaa_version": "", "bundle_sha256_prefix": "" } -Cache key: (aaa_version, sha256(bundle.md content)). Bumping AaA version or modifying bundle.md +Cache key: (aaa_version, sha256(bundle.md content)[:16]). Editing bundle.md changes the hash and invalidates the cache automatically. This two-part key fixes the F8 failure mode where two agents with identical version strings but different manifests would share a cache directory. Corruption is treated as a cache miss and rebuilt. @@ -15,6 +15,13 @@ Cold path (Task 4): calls load_and_prepare_bundle, writes pickle + manifest. Warm path (Task 5): if artifact + manifest already exist for this key, deserialise and return directly without invoking load_and_prepare_bundle. + +Note (see ``_restamp_agent_source_paths``): the cache key above says nothing about *where* +``amplifier_agent_lib`` is installed. It is a per-$AMPLIFIER_AGENT_HOME (effectively +per-user) key, not a per-installation one, so it is shared across every installation on the +machine that happens to carry the same version and unmodified vendored ``bundle.md``. The +warm path re-stamps installation-relative absolute paths after deserializing for exactly +this reason. """ from __future__ import annotations @@ -27,7 +34,7 @@ from typing import TYPE_CHECKING from amplifier_agent_lib import persistence -from amplifier_agent_lib.bundle import BUNDLE_MD +from amplifier_agent_lib.bundle import AGENTS_DIR, BUNDLE_MD from amplifier_agent_lib.bundle.loader import load_and_prepare_bundle if TYPE_CHECKING: @@ -39,6 +46,58 @@ _MANIFEST_NAME: str = "manifest.json" +def _restamp_agent_source_paths(prepared: PreparedBundle) -> None: + """Re-resolve every agent's ``source_path`` against *this* installation. + + ``prepared.mount_plan["agents"][name]["source_path"]`` is an ABSOLUTE path + baked in at cold-prepare time by ``bundle/loader.py`` + (``str(bundle.resolve_agent_path(agent_name))``), which resolves under + whatever ``amplifier_agent_lib`` installation happened to be running at + that moment (that resolver ultimately derives from ``BUNDLE_DIR = + Path(__file__).parent``). + + The on-disk cache this value gets pickled into is keyed ONLY by + ``(aaa_version, sha256(bundle.md content))`` (see + :func:`cache_dir_for_version`) under ``$AMPLIFIER_AGENT_HOME`` -- a + per-user location, not a per-installation one. That key says nothing + about *where* ``amplifier_agent_lib`` is installed, so it is shared by + every installation on the machine that happens to carry the same version + and unmodified vendored ``bundle.md`` (a git-pinned dependency + reinstalled to a new path, a second ``uv tool install`` after the first + was removed, a dev checkout alongside a packaged install, ...). A + warm-path hit deserializes whatever absolute paths the FIRST such + installation baked in, even when the CURRENT installation lives + somewhere else entirely -- at best a silent path mismatch, at worst a + ``FileNotFoundError`` reading ``agents/.md`` out of a location that + no longer exists, surfaced deep inside ``make_turn_handler`` -> + ``hydrate_agent_overlay`` on the very first turn. + + Fix: after every warm-path deserialize, overwrite each agent's + ``source_path`` with the equivalent path resolved against *this* + process's own :data:`~amplifier_agent_lib.bundle.AGENTS_DIR` (itself + derived from ``Path(__file__)``, so it is always correct for whichever + installation is actually running -- this is the same fix already applied + to :data:`~amplifier_agent_lib.resources.BUNDLE_DIR`, extended to the + per-agent paths derived from it). Mirrors ``bundle/loader.py``'s own + cold-path rule of only stamping a ``source_path`` that is verified to + exist; an agent whose file cannot be found here is left exactly as the + cache provided it (unresolvable either way, so no worse off) rather than + silently pointing it at an unverified path. + + Mutates ``prepared.mount_plan`` in place. No-op if the mount plan carries + no ``"agents"`` section (or the pickled object predates this key's use). + """ + agents = prepared.mount_plan.get("agents") if getattr(prepared, "mount_plan", None) else None + if not isinstance(agents, dict): + return + for name, entry in agents.items(): + if not isinstance(entry, dict) or "source_path" not in entry: + continue + candidate = AGENTS_DIR / f"{name}.md" + if candidate.exists(): + entry["source_path"] = str(candidate) + + def cache_dir_for_version(aaa_version: str, bundle_path: Path | None = None) -> Path: """Return the cache directory for a specific AaA version and bundle content hash. @@ -79,7 +138,10 @@ async def load_and_prepare_cached(aaa_version: str) -> PreparedBundle: :class:`~amplifier_foundation.bundle._prepared.PreparedBundle` without invoking :func:`~amplifier_agent_lib.bundle.loader.load_and_prepare_bundle`. A corrupted pickle triggers a warning log, removes both stale files, and falls through to the - cold path. + cold path. Before returning, every agent's ``source_path`` is re-stamped against + *this* installation (see :func:`_restamp_agent_source_paths`) — the cache key is + per-user, not per-installation, so a deserialized path may point at a different + (possibly since-removed) installation. Cold path: calls :func:`~amplifier_agent_lib.bundle.loader.load_and_prepare_bundle`, writes the @@ -102,7 +164,7 @@ async def load_and_prepare_cached(aaa_version: str) -> PreparedBundle: # Warm path: both files exist — return the cached PreparedBundle directly. if artifact.exists() and manifest.exists(): try: - return pickle.loads(artifact.read_bytes()) + prepared = pickle.loads(artifact.read_bytes()) except Exception as exc: # broad: corrupt cache → rebuild logger.warning( "Cache artifact at %s is corrupted (%s); rebuilding.", @@ -111,6 +173,9 @@ async def load_and_prepare_cached(aaa_version: str) -> PreparedBundle: ) artifact.unlink(missing_ok=True) manifest.unlink(missing_ok=True) + else: + _restamp_agent_source_paths(prepared) + return prepared # Cold path: prepare from scratch and write to cache. prepared = await load_and_prepare_bundle()