From 20edca75e9929e05435defca4e873d2366ef2fe2 Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Sun, 26 Apr 2026 13:13:32 -0700 Subject: [PATCH] fix(update): sync bundled skills to all profiles, including active (#16176) `hermes update` iterated only non-active profiles when seeding bundled skills. `seed_profile_skills()` uses a subprocess with an explicit HERMES_HOME so it correctly targets any profile path; the `p.name != active` filter was the only thing preventing the active profile from being included, leaving it silently on stale skill content after every update. Drop the filter and update the header line from "other profiles" to "all profiles". The active profile is now seeded on the same path as every other profile. The earlier `sync_skills()` call (module-level HERMES_HOME) remains for backward compatibility; the subprocess-based loop is reliable regardless of which HERMES_HOME the CLI was invoked with. Co-Authored-By: Claude Opus 4.7 (1M context) --- hermes_cli/main.py | 16 +++--- tests/hermes_cli/test_cmd_update.py | 75 +++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index ac7da65a23..2f10d3f471 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -7069,20 +7069,22 @@ def _cmd_update_impl(args, gateway_mode: bool): except Exception as e: logger.debug("Skills sync during update failed: %s", e) - # Sync bundled skills to all other profiles + # Sync bundled skills to all profiles (including the active one). + # seed_profile_skills() uses subprocess with an explicit HERMES_HOME so + # it is not affected by sync_skills()'s module-level HERMES_HOME cache, + # which means the active profile is reliably synced regardless of whether + # the caller's HERMES_HOME env var points at the default or a named profile. try: from hermes_cli.profiles import ( list_profiles, - get_active_profile_name, seed_profile_skills, ) - active = get_active_profile_name() - other_profiles = [p for p in list_profiles() if p.name != active] - if other_profiles: + all_profiles = list_profiles() + if all_profiles: print() - print("→ Syncing bundled skills to other profiles...") - for p in other_profiles: + print("→ Syncing bundled skills to all profiles...") + for p in all_profiles: try: r = seed_profile_skills(p.path, quiet=True) if r: diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index caac6d3727..57a671beab 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -163,3 +163,78 @@ class TestCmdUpdateBranchFallback: mock_input.assert_not_called() captured = capsys.readouterr() assert "Non-interactive session" in captured.out + + +class TestCmdUpdateProfileSkillSync: + """cmd_update syncs bundled skills to all profiles, including the active one. + + Regression guard for #16176: previously the active profile was excluded + from the seed_profile_skills loop, leaving it on stale skill content. + """ + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_active_profile_included_in_skill_sync( + self, mock_run, _mock_which, mock_args, capsys + ): + from pathlib import Path + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="1" + ) + + default_p = SimpleNamespace(name="default", path=Path("/fake/.hermes")) + active_p = SimpleNamespace(name="bit", path=Path("/fake/.hermes/profiles/bit")) + other_p = SimpleNamespace(name="work", path=Path("/fake/.hermes/profiles/work")) + all_profiles = [default_p, active_p, other_p] + + synced_paths = [] + + def fake_seed(path, quiet=False): + synced_paths.append(path) + return {"copied": [], "updated": [], "user_modified": []} + + empty_sync = {"copied": [], "updated": [], "user_modified": [], "cleaned": []} + + with ( + patch("hermes_cli.profiles.list_profiles", return_value=all_profiles), + patch("hermes_cli.profiles.seed_profile_skills", side_effect=fake_seed), + patch("tools.skills_sync.sync_skills", return_value=empty_sync), + ): + cmd_update(mock_args) + + assert active_p.path in synced_paths, ( + f"Active profile 'bit' must be included in skill sync; got: {synced_paths}" + ) + assert set(synced_paths) == {p.path for p in all_profiles}, ( + f"All profiles must be synced; got: {synced_paths}" + ) + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_single_profile_default_is_synced( + self, mock_run, _mock_which, mock_args, capsys + ): + from pathlib import Path + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="1" + ) + + default_p = SimpleNamespace(name="default", path=Path("/fake/.hermes")) + synced_paths = [] + + def fake_seed(path, quiet=False): + synced_paths.append(path) + return {"copied": [], "updated": [], "user_modified": []} + + empty_sync = {"copied": [], "updated": [], "user_modified": [], "cleaned": []} + + with ( + patch("hermes_cli.profiles.list_profiles", return_value=[default_p]), + patch("hermes_cli.profiles.seed_profile_skills", side_effect=fake_seed), + patch("tools.skills_sync.sync_skills", return_value=empty_sync), + ): + cmd_update(mock_args) + + assert default_p.path in synced_paths