fix(profiles): clone re-creates NTFS skill junctions instead of deep-copying them

`hermes profile create --clone/--clone-all` copies the source home with
`shutil.copytree(symlinks=True)`. POSIX symlinks survive that (b7192b1cb0),
but an NTFS junction is a reparse point, not a symlink: `os.path.islink()`
is False and copytree descends into it, so a `skills/foo` junction into a
`skills.external_dirs` root became a physical copy in the clone. The clone's
copy and the external original are then two same-named candidates and
`_locate_skill()` refuses to guess ("Ambiguous skill name"), which also
aborts Kanban worker spawns carrying the name (`Unknown skill(s)`).

`_copytree_keep_junctions` wraps both clone copies: an `ignore` callback
excludes junction entries (`st_reparse_tag == IO_REPARSE_TAG_MOUNT_POINT`)
and the recorded targets are re-created with `_winapi.CreateJunction`. A
junction whose target is gone is skipped with a warning (CreateJunction
requires an existing target), so a dangling link never fails the clone.
Off Windows the predicate short-circuits and behaviour is unchanged.

The export staging copy deliberately keeps deep-copying: `_scrub_export_secrets`
rewrites staged files in place, and a preserved junction would let that
redaction write through into the user's external originals.

Co-authored-by: KoNit-K <konit.block@protonmail.com>
This commit is contained in:
teknium1
2026-09-18 00:44:04 -07:00
committed by Teknium
parent c55b5bb6ce
commit b08fbfa0d7
2 changed files with 99 additions and 5 deletions

View File

@@ -874,10 +874,53 @@ def _materialize_symlinked_files(profile_dir: Path) -> List[str]:
return done
def _junction_target(path: str) -> Optional[str]:
"""Target of an NTFS directory junction, else ``None``. A junction is a reparse point, not a
symlink: ``os.path.islink()`` is False and ``shutil.copytree(symlinks=True)`` descends into it."""
if os.name != "nt":
return None
try:
if os.lstat(path).st_reparse_tag != stat.IO_REPARSE_TAG_MOUNT_POINT:
return None
target = os.readlink(path)
except OSError:
return None
# readlink hands back the substitute name; CreateJunction rejects the ``\\?\`` spelling.
if target.startswith("\\\\?\\UNC\\"):
return "\\" + target[7:]
return target[4:] if target.startswith("\\\\?\\") else target
def _copytree_keep_junctions(src: Path, dst: Path, ignore, dirs_exist_ok: bool = False) -> None:
"""``shutil.copytree(symlinks=True)`` that re-creates NTFS junctions as junctions instead of
traversing them. A ``skills/foo`` junction into a ``skills.external_dirs`` root copied as a
physical tree is a second same-named candidate and ``_locate_skill`` refuses to guess (#113471).
A junction whose target is gone is skipped with a warning, never a crash."""
junctions: Dict[str, str] = {}
def _ignore(directory: str, names: List[str]) -> set:
ignored = set(ignore(directory, names))
for name in names:
target = _junction_target(os.path.join(directory, name))
if target is not None:
junctions[os.path.join(directory, name)] = target
ignored.add(name)
return ignored
shutil.copytree(src, dst, symlinks=True, dirs_exist_ok=dirs_exist_ok, ignore=_ignore)
if junctions:
import _winapi # Windows-only stdlib module; only reachable once a junction was seen
for link, target in junctions.items():
try:
_winapi.CreateJunction(target, os.path.join(dst, os.path.relpath(link, src)))
except OSError as exc:
logger.warning("clone: skipped junction %s -> %s (%s)", link, target, exc)
def _clone_all_into(source_dir: Path, profile_dir: Path, canon: str) -> None:
"""--clone-all: full copytree minus infrastructure/history, then strip runtime files
and cloned single-use OAuth grants."""
shutil.copytree(source_dir, profile_dir, symlinks=True, ignore=_clone_all_copytree_ignore(source_dir))
_copytree_keep_junctions(source_dir, profile_dir, _clone_all_copytree_ignore(source_dir))
materialized = _materialize_symlinked_files(profile_dir)
if materialized:
logger.info("profile %s: materialized symlinked %s so the clone never writes through to %s",
@@ -919,10 +962,7 @@ def _bootstrap_profile_dir(profile_dir: Path, source_dir: Optional[Path],
_clone_file(source_dir, profile_dir, relpath)
source_skills = source_dir / "skills"
if source_skills.is_dir():
shutil.copytree(
source_skills, profile_dir / "skills", symlinks=True, dirs_exist_ok=True,
ignore=_non_exportable_entries,
)
_copytree_keep_junctions(source_skills, profile_dir / "skills", _non_exportable_entries, dirs_exist_ok=True)
for relpath in _CLONE_SUBDIR_FILES:
_clone_file(source_dir, profile_dir, relpath)
if sync_imports:

View File

@@ -242,6 +242,60 @@ class TestCreateProfile:
(default_home / "config.yaml").write_text("model: changed")
assert yaml.safe_load((synced / "config.yaml").read_text())["model"] == "test"
@staticmethod
def _home_with_linked_skill(profile_env):
"""Source home: ``skills/foo`` links into an ``external_dirs`` root, ``skills/local`` is physical."""
default_home = profile_env / ".hermes"
external = profile_env / "agents-skills"
(external / "foo").mkdir(parents=True)
(external / "foo" / "SKILL.md").write_text("# external foo\n", encoding="utf-8")
(default_home / "skills" / "local").mkdir(parents=True)
(default_home / "skills" / "local" / "SKILL.md").write_text("# local\n", encoding="utf-8")
(default_home / "config.yaml").write_text(f"model: test\nskills:\n external_dirs:\n - {external}\n")
return default_home, external
@pytest.mark.parametrize("clone_kwargs", [{"clone_config": True}, {"clone_all": True}])
def test_clone_recreates_skill_junctions_and_skips_dangling_ones(self, profile_env, monkeypatch, clone_kwargs):
"""A junctioned skill stays a link (one candidate with its external original), a dangling
junction is skipped without failing the clone. The reparse-point predicate and CreateJunction
are Windows-only; simulate both so the copy/re-create contract runs on every host."""
default_home, external = self._home_with_linked_skill(profile_env)
# copytree sees plain directories (what a junction looks like to os.stat on Windows).
(default_home / "skills" / "foo").mkdir()
(default_home / "skills" / "foo" / "SKILL.md").write_text("# a physical copy would come from here\n")
(default_home / "skills" / "gone").mkdir()
targets = {str(default_home / "skills" / "foo"): str(external / "foo"),
str(default_home / "skills" / "gone"): str(profile_env / "nowhere")}
monkeypatch.setattr(profiles, "_junction_target", lambda path: targets.get(path), raising=False)
def _create_junction(target, dst):
if not os.path.isdir(target):
raise OSError("target missing") # what _winapi.CreateJunction does for a dangling junction
os.symlink(target, dst, target_is_directory=True)
monkeypatch.setitem(sys.modules, "_winapi", types.SimpleNamespace(CreateJunction=_create_junction))
clone = create_profile("clone", no_alias=True, **clone_kwargs)
foo = clone / "skills" / "foo"
assert foo.is_symlink() and foo.resolve() == (external / "foo").resolve()
assert (foo / "SKILL.md").read_text(encoding="utf-8") == "# external foo\n"
assert (clone / "skills" / "local" / "SKILL.md").is_file()
assert not (clone / "skills" / "gone").exists()
from tools.skills_tool import _collect_skill_candidates
assert len(_collect_skill_candidates("foo", None, [clone / "skills", external])) == 1
@pytest.mark.windows_only
def test_clone_keeps_real_ntfs_junction(self, profile_env):
import _winapi
default_home, external = self._home_with_linked_skill(profile_env)
_winapi.CreateJunction(str(external / "foo"), str(default_home / "skills" / "foo"))
clone = create_profile("clone", clone_config=True, no_alias=True)
foo = clone / "skills" / "foo"
assert os.lstat(foo).st_reparse_tag == profiles.stat.IO_REPARSE_TAG_MOUNT_POINT
assert foo.resolve() == (external / "foo").resolve()
from tools.skills_tool import _collect_skill_candidates
assert len(_collect_skill_candidates("foo", None, [clone / "skills", external])) == 1
def test_sync_imports_requires_a_clone_source(self, profile_env):
with pytest.raises(ValueError, match="--sync-imports requires"):
create_profile("lonely", sync_imports=True, no_alias=True)