fix(profile): preserve user-added skills and cron jobs on install --force / update

[mohsinzahid]

Fixes #25120

What

hermes profile install --force and hermes profile update both advertise "user data preserved" in their help text, but _copy_dist_payload() rmtree'd + recopied every directory the distribution ships. Anything the user added inside skills/ after install was silently deleted — including skills installed with hermes skills install, which writes into the same tree. User-added cron/ jobs were deleted the same way.

This is the destructive replacement reported in #25120, implemented along the lines of its "Option B — additive merge", with one refinement for distribution-owned skills (below).

How

A new _replace_dir_merging_user_entries() replaces the wholesale rmtree + copytree in _copy_dist_payload():

• a shipped directory that is a skill root (contains SKILL.md) still replaces its counterpart wholesale, so files dropped or renamed by the new version don't linger inside distribution-owned skills;
• any other shipped directory merges recursively — this covers category folders like skills/devops/ shared by the distribution and hermes skills install --category devops (the exact layout in Bug: hermes profile install/update destructively replaces local skills despite distribution_owned flag #25120's repro);
• shipped files overwrite their counterparts;
• entries the distribution doesn't ship are left untouched.

One semantics change to be aware of: a skill the distribution removes in a new version now lingers in the profile instead of being deleted as a side effect of the mirror. If cleanup of dist-removed skills matters, the follow-up would be payload bookkeeping in the installed manifest (record shipped entries at install; delete previously-shipped-now-absent ones on update). Happy to iterate in that direction if you prefer.

Also removed: the ignore= lambda previously passed to copytree, which was dead code — it only filtered names when the visited directory equaled the staged root, but that copytree started one level below the root, so the predicate never fired. Top-level filtering already happens via the name in USER_OWNED_EXCLUDE guard at the top of the loop; TestNestedUserOwnedExcludeNotFiltered passes unchanged.

The module docstring's "Update semantics" section and the profile update parser description are updated to state the merge behavior, which makes the existing "(user data preserved)" help text on --force and update accurate.

How to test

New tests in tests/hermes_cli/test_profile_distribution.py::TestUserAdditionsSurviveReinstall — written first, red on main, green with this change:

• test_force_install_preserves_user_added_skill
• test_update_preserves_user_added_skill — dist's own skill still receives its new content
• test_update_preserves_user_skill_in_shared_category_dir — Bug: hermes profile install/update destructively replaces local skills despite distribution_owned flag #25120's skills/<category>/<custom-skill> case
• test_update_preserves_user_added_cron_job
• test_update_still_mirrors_dist_shipped_skill_dir — guard: stale files inside a dist-shipped skill must NOT linger (passes before and after)

pytest tests/hermes_cli/test_profile_distribution.py    # 74 passed

End-to-end repro (fails on stock main at e71d746, fixed with this branch):

repro script

#!/usr/bin/env bash
set -euo pipefail
HERMES_CMD="${HERMES_CMD:-hermes}"
TMP=$(mktemp -d)
trap 'rm -rf "$TMP"' EXIT
export HERMES_HOME="$TMP/hermes-home"
mkdir -p "$HERMES_HOME"

DIST="$TMP/demo-dist"
mkdir -p "$DIST/skills/dist-skill"
cat > "$DIST/distribution.yaml" <<'EOF'
name: demodist
version: 0.1.0
description: minimal repro distribution
EOF
printf -- '---\nname: dist-skill\ndescription: shipped by the distribution\n---\n# dist skill\n' \
  > "$DIST/skills/dist-skill/SKILL.md"
printf 'soul\n' > "$DIST/SOUL.md"

$HERMES_CMD profile install "$DIST" --yes >/dev/null

PROFILE="$HERMES_HOME/profiles/demodist"
mkdir -p "$PROFILE/skills/my-skill"
printf -- '---\nname: my-skill\ndescription: added by the user\n---\n# my skill\n' \
  > "$PROFILE/skills/my-skill/SKILL.md"
echo "skills before reinstall: $(ls "$PROFILE/skills" | tr '\n' ' ')"

$HERMES_CMD profile install "$DIST" --yes --force >/dev/null

echo "skills after reinstall:  $(ls "$PROFILE/skills" | tr '\n' ' ')"
if [ -d "$PROFILE/skills/my-skill" ]; then
  echo "OK: user-added skill preserved"
else
  echo "BUG: user-added skill was DELETED"
fi

Before this change: BUG: user-added skill was DELETED. After: OK: user-added skill preserved.

Platforms

macOS (Darwin 25.2.0), Python 3.11.15. Full scripts/run_tests.sh tests/hermes_cli/ run: the only failures (11 tests across 4 gateway/service test files, plus 2 provider files with collection errors) are identical on a pristine main worktree in this environment — pre-existing and unrelated to this change. The code path touched here is platform-independent (pathlib/shutil only).

🤖 Generated with Claude Code

[liuhao1024]

Code Review — Positive Verification

Reviewed the full diff across hermes_cli/profile_distribution.py, hermes_cli/subcommands/profile.py, and tests/hermes_cli/test_profile_distribution.py.

What was checked:

• _replace_dir_merging_user_entries correctly distinguishes distribution-owned skill dirs (containing SKILL.md) from shared category dirs — skill dirs are replaced wholesale (removing stale files), category dirs are merged recursively
• Edge cases handled: symlink-to-file at dest (unlinked before copy), file-at-dest where dir expected (unlinked), dir-at-dest where file expected (rmtree)
• The old _copy_dist_payload ignore=USER_OWNED_EXCLUDE lambda is fully replaced — no orphaned code
• 5 test cases cover: force install preserves user skill, update preserves user skill, shared category dir merging, update preserves user cron job, update mirrors dist-shipped skill dir
• Help text updated to document the new preservation behavior

Verdict: Clean. The merge-instead-of-mirror approach prevents data loss for users who install additional skills via hermes skills install or create custom cron jobs. The SKILL.md detection heuristic is correct — distribution-shipped skill directories always contain a SKILL.md, so they get wholesale replacement (preventing stale files), while non-skill directories get recursive merge. Good test coverage.

[tonydwb]

Code Review Summary

Verdict: Approved

Summary

Fixes hermes update/install --force to preserve user-added skills and cron jobs that were installed after the distribution was first deployed. Previously, distribution updates would wipe user additions inside distribution-owned directories.

Changes

• Adds _replace_dir_merging_user_entries() that recursively merges directories, preserving user entries while replacing distribution-owned skill roots wholesale.
• Distribution directories containing SKILL.md are replaced wholesale; other directories are merged recursively.

Review

• Good fix with clear documentation of the preservation semantics.
• No security concerns.
• Clean approach that distinguishes skill roots from general category folders.

---

Reviewed by Hermes Agent (batch cron)

[teknium1]

Thanks for isolating a real destructive update path. Current main still removes skills/ and cron/ wholesale in hermes_cli/profile_distribution.py:573-585, so the preservation direction is valuable.

Problems

• hermes_cli/profile_distribution.py:581 — the new file-copy branch calls shutil.copy2(child, node) without first removing a destination symlink. Unlike the directory branch at :570-573, this follows a file symlink. In a recursively merged user-preserved directory, an update can therefore overwrite the symlink target outside the profile tree.

Suggested changes

• Unlink destination symlinks before copying files and add a regression test covering a nested preserved-directory symlink.
• Update website/docs/user-guide/profile-distributions.md:247 and :356-358; they still describe skills/ and cron/ as replaced rather than merged.

Automated hermes-sweeper review.

[teknium1]

node.is_dir() is false for a symlink to a file, so copy2() follows that symlink and overwrites its target. Unlink node when node.is_symlink() before copying, and add a regression test using a nested symlink in a recursively merged directory.

[mohsinzahid]

@teknium1 Thanks — addressed on exact head dc032d0f89ac8ebacad238bae23d078deb5ee357.

• The copy path now unlinks a destination symlink/file before copy2(), so an external symlink target is not overwritten.
• Added the requested nested symlink regression in a recursively merged cron directory; it proves the external target remains unchanged.
• Updated the profile-distribution docs for recursive skills/cron merge-preservation semantics.
• Local verification passed: nested regression (1 passed), TestUserAdditionsSurviveReinstall (6 passed), full tests/hermes_cli/test_profile_distribution.py (75 passed), Ruff on the modified Python files, git diff --check, the static symlink guard, profile update --help, and the website build (with only existing broken-link/anchor warnings).

Could you please re-review this exact head?