Fix warm binary request cache hits (#44)

* Fix warm binary request cache hits

* Cover cache identity edge cases

* Verify global pnpm package ownership

* Reject stale unowned pnpm cache targets

Author: pirate

abxpkg/binprovider.py

@@ -1326,6 +1326,15 @@ def record_matches(record: object) -> bool:
                 list,
             ):
                 return False
+            requested_path = Path(str(bin_name)).expanduser()
+            cached_name = typed_record.get("bin_name")
+            cached_name_matches = cached_name == str(bin_name) or (
+                requested_path.is_absolute()
+                and isinstance(cached_name, str)
+                and Path(cached_name).name == requested_path.name
+                and os.path.abspath(os.path.expanduser(cached_abspath))
+                == os.path.abspath(os.path.expanduser(str(requested_path)))
+            )
             fingerprint_paths: list[Path] = []
             for raw_fingerprint in raw_fingerprints:
                 if not isinstance(raw_fingerprint, dict):
@@ -1337,7 +1346,7 @@ def record_matches(record: object) -> bool:
                 fingerprint_paths.append(Path(fingerprint_path))
             return (
                 typed_record.get("provider_name") == self.name
-                and typed_record.get("bin_name") == str(bin_name)
+                and cached_name_matches
                 and Path(cached_abspath).expanduser().resolve(strict=False)
                 == resolved_abspath
                 and typed_record.get("resolved_provider_name") == exec_provider.name

abxpkg/binprovider_pnpm.py

@@ -830,16 +830,63 @@ def cached_binary_state_mismatch(
         bin_name: BinName,
         cached_record: Mapping[str, object],
     ) -> bool:
-        if self.install_root is None:
-            return False
-        modules_dir = self.install_root / "node_modules"
-        for package in self._package_names_from_install_args(
+        package_names = self._package_names_from_install_args(
             self.get_install_args(bin_name, quiet=True, no_cache=True),
-        ):
-            if not (modules_dir / package / "package.json").exists():
-                return True
+        )
+        if self.install_root is not None:
+            modules_dir = self.install_root / "node_modules"
+            for package in package_names:
+                if not (modules_dir / package / "package.json").exists():
+                    return True
+            installed_version = self._installed_package_version(str(bin_name))
+        else:
+            raw_abspath = cached_record.get("abspath")
+            installed_version = (
+                self._installed_abspath_package_version(
+                    Path(raw_abspath),
+                    package_names=set(package_names),
+                )
+                if isinstance(raw_abspath, str)
+                else None
+            )
+        raw_cached_version = cached_record.get("loaded_version")
+        cached_version = (
+            SemVer.parse(raw_cached_version)
+            if isinstance(raw_cached_version, (str, bytes))
+            else None
+        )
+        if installed_version is None:
+            return True
+        if cached_version is not None and installed_version != cached_version:
+            return True
         return False
 
+    @classmethod
+    def _installed_abspath_package_version(
+        cls,
+        abspath: Path,
+        *,
+        package_names: set[str],
+    ) -> SemVer | None:
+        """Read the package version behind a global pnpm executable without pnpm."""
+        package_target = cls.host_projection_target(abspath) or abspath.resolve(
+            strict=False,
+        )
+        for parent in package_target.parents:
+            package_json = parent / "package.json"
+            try:
+                package = json.loads(package_json.read_text())
+            except (OSError, json.JSONDecodeError):
+                continue
+            if (
+                not isinstance(package, dict)
+                or package.get("name") not in package_names
+            ):
+                continue
+            version = package.get("version")
+            return SemVer.parse(version) if isinstance(version, str) else None
+        return None
+
     def _node_modules_dir(self) -> Path | None:
         if self.install_root:
             return self.install_root / "node_modules"
@@ -1351,6 +1398,22 @@ def default_abspath_handler(
             no_cache=no_cache,
         )
 
+    def _get_version_at_abspath(
+        self,
+        bin_name: BinName,
+        installed_abspath: HostBinPath,
+        *,
+        quiet: bool,
+    ) -> SemVer | None:
+        installed_package_version = self._installed_package_version(str(bin_name))
+        if installed_package_version is not None:
+            return installed_package_version
+        return super()._get_version_at_abspath(
+            bin_name,
+            installed_abspath,
+            quiet=quiet,
+        )
+
     def default_version_handler(
         self,
         bin_name: BinName,

abxpkg/config.py

@@ -569,6 +569,10 @@ def _cached_records(
     *,
     require_executable: bool = True,
 ):
+    requested_path = os.path.expanduser(os.fspath(binary_name))
+    requested_abspath = (
+        os.path.abspath(requested_path) if os.path.isabs(requested_path) else None
+    )
     for provider_name in provider_names:
         default_root = os.path.join(lib_dir, provider_name)
         provider_roots = dict.fromkeys(
@@ -629,10 +633,22 @@ def _cached_records(
                     if isinstance(raw_fingerprints, list) and raw_fingerprints
                     else None
                 )
+                record_name = (
+                    record.get("bin_name") if isinstance(record, dict) else None
+                )
+                record_name_matches = record_name == binary_name or (
+                    requested_abspath is not None
+                    and isinstance(record_name, str)
+                    and os.path.basename(record_name)
+                    == os.path.basename(requested_path)
+                    and isinstance(record_abspath, str)
+                    and os.path.abspath(os.path.expanduser(record_abspath))
+                    == requested_abspath
+                )
                 if (
                     isinstance(record, dict)
                     and record.get("provider_name") == provider_name
-                    and record.get("bin_name") == binary_name
+                    and record_name_matches
                     and isinstance(record_abspath, str)
                     and os.path.isabs(record_abspath)
                     and (not require_executable or os.access(record_abspath, os.X_OK))

tests/test_binary_service.py

@@ -449,6 +449,107 @@ def request_projections(record: dict[str, Any]) -> dict[str, Any]:
     )
 
 
+def test_binary_service_reuses_absolute_path_request_projection(
+    tmp_path: Path,
+) -> None:
+    from abxpkg.binary_service import BinaryRequestEvent, BinaryService
+
+    lib_dir = tmp_path / "lib"
+    provider = EnvProvider(install_root=lib_dir / "env")
+    loaded = provider.load("python3")
+    assert loaded is not None
+    projected_path = provider.project_binary(loaded, "python3")
+    assert projected_path is not None
+
+    run_id = 0
+
+    async def run() -> None:
+        nonlocal run_id
+        run_id += 1
+        bus = abxbus.EventBus(name=f"test_absolute_request_projection_{run_id}")
+        BinaryService(bus, auto_install=False, lib_dir=lib_dir)
+        await bus.emit(
+            BinaryRequestEvent(
+                name=str(projected_path),
+                binproviders="env",
+            ),
+        ).now()
+        await bus.wait_until_idle()
+        await bus.destroy(clear=False)
+
+    asyncio.run(run())
+
+    binary_load_calls = 0
+
+    def count_binary_loads(frame: Any, event: str, arg: Any) -> None:
+        del arg
+        nonlocal binary_load_calls
+        if event == "call" and frame.f_code is BinaryService._load.__code__:
+            binary_load_calls += 1
+
+    sys.setprofile(count_binary_loads)
+    threading.setprofile(count_binary_loads)
+    try:
+        asyncio.run(run())
+    finally:
+        sys.setprofile(None)
+        threading.setprofile(None)
+
+    assert binary_load_calls == 0
+
+
+def test_binary_service_reuses_normalized_absolute_path_request_projection(
+    tmp_path: Path,
+) -> None:
+    from abxpkg.binary_service import BinaryRequestEvent, BinaryService
+
+    lib_dir = tmp_path / "lib"
+    provider = EnvProvider(install_root=lib_dir / "env")
+    loaded = provider.load("python3")
+    assert loaded is not None
+    projected_path = provider.project_binary(loaded, "python3")
+    assert projected_path is not None
+    requested_path = (
+        projected_path.parent / ".." / projected_path.parent.name / projected_path.name
+    )
+
+    run_id = 0
+
+    async def run() -> None:
+        nonlocal run_id
+        run_id += 1
+        bus = abxbus.EventBus(name=f"test_normalized_request_projection_{run_id}")
+        BinaryService(bus, auto_install=False, lib_dir=lib_dir)
+        await bus.emit(
+            BinaryRequestEvent(
+                name=str(requested_path),
+                binproviders="env",
+            ),
+        ).now()
+        await bus.wait_until_idle()
+        await bus.destroy(clear=False)
+
+    asyncio.run(run())
+
+    binary_load_calls = 0
+
+    def count_binary_loads(frame: Any, event: str, arg: Any) -> None:
+        del arg
+        nonlocal binary_load_calls
+        if event == "call" and frame.f_code is BinaryService._load.__code__:
+            binary_load_calls += 1
+
+    sys.setprofile(count_binary_loads)
+    threading.setprofile(count_binary_loads)
+    try:
+        asyncio.run(run())
+    finally:
+        sys.setprofile(None)
+        threading.setprofile(None)
+
+    assert binary_load_calls == 0
+
+
 def test_binary_event_env_does_not_prepend_shared_host_projections(
     tmp_path: Path,
 ) -> None:

tests/test_pnpmprovider.py

@@ -853,6 +853,101 @@ def test_provider_direct_min_version_revalidates_old_install_and_upgrades(
                     min_release_age=3,
                 ).update("zx", min_version=SemVer("999.0.0"))
 
+    def test_literal_version_override_cannot_replace_installed_package_version(
+        self,
+        test_machine,
+    ):
+        test_machine.require_tool("node")
+        with tempfile.TemporaryDirectory() as tmpdir:
+            install_root = Path(tmpdir) / "pnpm"
+            provider = PnpmProvider(
+                install_root=install_root,
+                postinstall_scripts=True,
+                min_release_age=0,
+            ).get_provider_with_overrides(
+                overrides={
+                    "zx": {
+                        "install_args": ["zx@7.2.3"],
+                        "version": "999.0.0",
+                    },
+                },
+            )
+
+            installed = provider.install("zx", min_version=SemVer("1.0.0"))
+
+            assert installed is not None
+            package_json = install_root / "node_modules" / "zx" / "package.json"
+            assert package_json.exists()
+            import json as _json
+
+            observed_version = _json.loads(package_json.read_text())["version"]
+            assert installed.loaded_version == SemVer("7.2.3")
+            assert observed_version == "7.2.3"
+            assert str(installed.loaded_version) != "999.0.0"
+
+    def test_global_cache_compares_installed_package_version(self, tmp_path):
+        global_root = tmp_path / "global"
+        package_dir = global_root / ".pnpm" / "zx@7.2.3" / "node_modules" / "zx"
+        executable = package_dir / "build" / "cli.js"
+        executable.parent.mkdir(parents=True)
+        executable.write_text("#!/usr/bin/env node\n")
+        executable.chmod(0o755)
+        (executable.parent / "package.json").write_text(
+            '{"name": "not-zx", "version": "999.0.0"}',
+        )
+        (package_dir / "package.json").write_text(
+            '{"name": "zx", "version": "7.2.3"}',
+        )
+        launcher = global_root / "bin" / "zx"
+        launcher.parent.mkdir()
+        launcher.write_text(
+            '#!/bin/sh\nexec node "$basedir/../.pnpm/zx@7.2.3/node_modules/zx/build/cli.js" "$@"\n',
+        )
+        launcher.chmod(0o755)
+        provider = PnpmProvider(install_root=None)
+
+        assert provider.host_projection_target(launcher) == executable
+
+        assert provider.cached_binary_state_mismatch(
+            "zx",
+            {"abspath": str(launcher), "loaded_version": "999.0.0"},
+        )
+        assert not provider.cached_binary_state_mismatch(
+            "zx",
+            {"abspath": str(launcher), "loaded_version": "7.2.3"},
+        )
+
+    def test_global_cache_rejects_executable_without_owned_package(self, tmp_path):
+        global_root = tmp_path / "global"
+        executable = (
+            global_root
+            / ".pnpm"
+            / "not-zx@7.2.3"
+            / "node_modules"
+            / "not-zx"
+            / "build"
+            / "cli.js"
+        )
+        executable.parent.mkdir(parents=True)
+        executable.write_text("#!/usr/bin/env node\n")
+        executable.chmod(0o755)
+        (executable.parent.parent / "package.json").write_text(
+            '{"name": "not-zx", "version": "7.2.3"}',
+        )
+        launcher = global_root / "bin" / "zx"
+        launcher.parent.mkdir()
+        launcher.write_text(
+            '#!/bin/sh\nexec node "$basedir/../.pnpm/not-zx@7.2.3/node_modules/not-zx/build/cli.js" "$@"\n',
+        )
+        launcher.chmod(0o755)
+        provider = PnpmProvider(install_root=None)
+
+        assert provider.host_projection_target(launcher) == executable
+        assert provider.cached_binary_state_mismatch(
+            "zx",
+            {"abspath": str(launcher), "loaded_version": "7.2.3"},
+        )
+
     def test_provider_defaults_and_binary_overrides_enforce_min_release_age(
         self,
         test_machine,