fix(install): make the staged skill bundle writable so a read-only package installs (#2453)

This commit is contained in:
Benjamin S. Leveritt
2026-08-13 14:17:42 +01:00
committed by safishamsi
parent 5e4ab1dfa0
commit 613c45cc7e
2 changed files with 40 additions and 0 deletions
+8
View File
@@ -17,6 +17,7 @@ import os
import platform
import re
import shutil
import stat
import sys
from pathlib import Path
from typing import NoReturn
@@ -165,6 +166,13 @@ def _install_skill_references(skill_dst: Path, refs_src: Path) -> None:
shutil.rmtree(refs_staged)
try:
shutil.copytree(refs_src, refs_staged)
# copytree preserves the source's mode bits, and a packaged bundle can
# be read-only: a Nix store path, a root-owned site-packages, a
# container image layer. Renaming a directory needs write permission on
# the directory itself, to update its ".." entry, so the os.replace
# below would fail with EACCES. Restore owner-write on the staged copy.
for path in (refs_staged, *refs_staged.rglob("*")):
path.chmod(path.stat().st_mode | stat.S_IWUSR)
if refs_dst.exists():
shutil.rmtree(refs_dst)
os.replace(refs_staged, refs_dst)
+32
View File
@@ -514,3 +514,35 @@ def test_amp_user_install_carries_references(tmp_path, monkeypatch):
main()
assert not skill_dir.exists()
def test_install_from_read_only_package_dir(tmp_path, fake_bundle):
"""Install succeeds when the packaged bundle is read-only.
Nix store paths are mode 0o555, as are root-owned site-packages and
container image layers. copytree preserves those bits onto references.tmp,
and renaming a directory needs write permission on the directory itself to
update its ".." entry so the staging rename fails with EACCES unless the
staged copy is made writable first.
"""
platform = fake_bundle
bundle = mainmod._PLATFORM_CONFIG[platform]["skill_refs"]
refs_src = PKG_DIR / "skills" / bundle / "references"
modes = {p: p.stat().st_mode for p in (refs_src, *refs_src.rglob("*"))}
for path in modes:
path.chmod(0o555 if path.is_dir() else 0o444)
try:
_install(tmp_path, platform)
finally:
for path, mode in modes.items():
path.chmod(mode)
skill_dir = tmp_path / ".claude" / "skills" / "graphify"
refs = skill_dir / "references"
assert refs.is_dir()
assert (refs / "query.md").read_text() == "# query fragment\n"
assert not (skill_dir / "references.tmp").exists()
# The installed sidecar must stay writable, or the next install cannot
# rmtree it to swap in a new one.
assert os.access(refs, os.W_OK)