Skip to content

Commit d0aebe8

Browse files
committed
All providers: never treat managed shims as source-of-truth for load()
Core principle (per maintainer): the symlinks/shims abxpkg writes under ``bin_dir`` are a human-convenience side-effect of install, not a source of truth. ``load()`` and every other internal lookup must always consult the underlying package manager / CLI instead. Audit result — only three providers had a "shim-first" short-circuit in ``default_abspath_handler``; the others (``docker``, ``bash`` wrappers; ``npm``/``pip``/``pnpm``/``yarn``/``uv``/``cargo``/``gem``/ ``goget`` where bin_dir IS the package manager's install location, not a shim of something upstream) were already correct. Fixed: - ``BrewProvider``: removed the initial ``bin_abspath(bin_name, PATH=str(self.bin_dir))`` short-circuit. Now always computes the brew Cellar / opt / PATH search list first and only refreshes the managed shim to match the freshly-resolved target. - ``PuppeteerProvider``: removed the ``bin_dir/<name>`` shim check before falling back to ``puppeteer-browsers list``. Now always asks the CLI first, then refreshes the shim to point at the fresh path. - ``PlaywrightProvider``: same — always asks ``playwright-core``'s ``executablePath()`` first, shim becomes output-only. Also updated the uninstall-handler docstrings for both puppeteer and playwright to reflect that dropping the shim is a cosmetic cleanup (to keep managed PATH tidy) rather than a correctness requirement for ``load()``.
1 parent 836123b commit d0aebe8

3 files changed

Lines changed: 30 additions & 19 deletions

File tree

abxpkg/binprovider_brew.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -396,15 +396,15 @@ def default_abspath_handler(
396396
if not self.PATH:
397397
return None
398398

399-
linked_bin = self._linked_bin_path(bin_name)
400-
if linked_bin is not None:
401-
linked_abspath = bin_abspath(bin_name, PATH=str(self.bin_dir))
402-
if linked_abspath:
403-
return linked_abspath
404-
399+
# Authoritative lookup: search brew's own Cellar / opt / PATH
400+
# entries for the real formula binary. The managed ``bin_dir``
401+
# shim is a convenience side-effect of install — never a source
402+
# of truth — so we always consult brew's paths first and only
403+
# refresh the shim to match the freshly-resolved target.
405404
search_paths = self._brew_search_paths(bin_name, no_cache=no_cache)
406405
abspath = bin_abspath(bin_name, PATH=search_paths)
407406
if abspath:
407+
linked_bin = self._linked_bin_path(bin_name)
408408
if linked_bin is None or Path(abspath).parent == self.bin_dir:
409409
return abspath
410410
return self._refresh_bin_link(bin_name, abspath)

abxpkg/binprovider_playwright.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -564,10 +564,11 @@ def default_abspath_handler(
564564
except Exception:
565565
return None
566566
return None
567-
if self.bin_dir is not None:
568-
link = self.bin_dir / str(bin_name)
569-
if link.exists() and os.access(link, os.X_OK):
570-
return link
567+
# Authoritative lookup: ask ``playwright-core`` where the browser
568+
# actually lives via its ``executablePath()`` API — never trust
569+
# the managed ``bin_dir`` shim as a source of truth, it may
570+
# point at a browser that was removed out-of-band. When
571+
# playwright-core reports nothing, we report nothing.
571572
resolved = self._playwright_browser_path(
572573
str(bin_name),
573574
no_cache=no_cache,
@@ -725,8 +726,11 @@ def default_uninstall_handler(
725726
install_args: InstallArgs | None = None,
726727
**context,
727728
) -> bool:
728-
# Drop the managed shim first so ``load()`` stops returning the
729-
# symlink even if the browser-dir rmtree partially fails.
729+
# Clean up the convenience shim under ``bin_dir`` alongside the
730+
# real browser removal. ``load()`` never consults this shim as a
731+
# source of truth (it always asks playwright-core directly), so
732+
# dropping it is a cosmetic cleanup to keep the managed PATH
733+
# tidy rather than a correctness requirement.
730734
if self.bin_dir is not None:
731735
(self.bin_dir / bin_name).unlink(missing_ok=True)
732736

abxpkg/binprovider_puppeteer.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -419,15 +419,19 @@ def default_abspath_handler(
419419
except Exception:
420420
return None
421421
return None
422-
bin_dir = self.bin_dir
423-
assert bin_dir is not None
424-
link_path = bin_dir / str(bin_name)
425-
if link_path.exists() and os.access(link_path, os.X_OK):
426-
return link_path
427422

423+
# Authoritative lookup: ask puppeteer-browsers where the browser
424+
# actually lives — never trust the managed ``bin_dir`` shim as a
425+
# source of truth, it may point at a browser that was removed
426+
# out-of-band. When the CLI reports nothing, we report nothing.
428427
resolved = self._resolve_installed_browser_path(str(bin_name))
429428
if not resolved or not resolved.exists():
430429
return None
430+
431+
# Refresh the convenience shim under ``bin_dir`` so ``PATH`` users
432+
# get a stable entry pointing at the freshly-resolved executable.
433+
# Fall back to the resolved path directly when the shim refresh
434+
# fails (read-only FS etc.).
431435
try:
432436
return self._refresh_symlink(str(bin_name), resolved)
433437
except OSError:
@@ -688,8 +692,11 @@ def default_uninstall_handler(
688692
install_args: InstallArgs | None = None,
689693
**context,
690694
) -> bool:
691-
# Drop the managed shim first so ``load()`` stops returning the
692-
# symlink even if the browser-dir rmtree partially fails.
695+
# Clean up the convenience shim under ``bin_dir`` alongside the
696+
# real browser removal. ``load()`` never consults this shim as a
697+
# source of truth (it always asks puppeteer-browsers directly),
698+
# so dropping it is a cosmetic cleanup to keep the managed PATH
699+
# tidy rather than a correctness requirement.
693700
if self.bin_dir is not None:
694701
bin_path = self.bin_dir / bin_name
695702
if bin_path.exists() or bin_path.is_symlink():

0 commit comments

Comments
 (0)