fix(skills_sync): honor .no-bundled-skills marker for DESCRIPTION.md copy (finish PR #15)

tests/tools/test_skills_sync.py

@@ -483,7 +483,14 @@ def test_collision_prints_reset_hint(self, tmp_path, capsys):
         assert "hermes skills reset new-skill" in captured
 
     def test_nonexistent_bundled_dir(self, tmp_path):
-        with patch("tools.skills_sync._get_bundled_dir", return_value=tmp_path / "nope"):
+        # Patch SKILLS_DIR to a clean tmp path so the marker check inside
+        # sync_skills() doesn't pick up the developer's live HERMES_HOME
+        # opt-out marker and short-circuit before the missing-bundled-dir
+        # path runs.
+        skills_dir = tmp_path / "profile" / "skills"
+        skills_dir.mkdir(parents=True)
+        with patch("tools.skills_sync._get_bundled_dir", return_value=tmp_path / "nope"), \
+             patch("tools.skills_sync.SKILLS_DIR", skills_dir):
             result = sync_skills(quiet=True)
         assert result == {
             "copied": [], "updated": [], "skipped": 0,
@@ -732,3 +739,63 @@ def test_reset_no_op_when_already_clean(self, tmp_path):
             post_manifest = _read_manifest()
             assert "google-workspace" in post_manifest
         assert (skills_dir / "productivity" / "google-workspace" / "SKILL.md").exists()
+
+
+class TestNoBundledSkillsMarker:
+    """Regression: sync_skills must honor the .no-bundled-skills opt-out marker.
+
+    PR #15 added the marker check at every documented caller but the
+    DESCRIPTION.md copy loop inside sync_skills() still ran unconditionally
+    when sync_skills() was called directly (e.g. seed_profile_skills() on a
+    profile created before the marker was written). That left stale category
+    DESCRIPTION.md stubs in opted-out profile skill dirs.
+    """
+
+    def _setup_bundled(self, tmp_path):
+        bundled = tmp_path / "bundled_skills"
+        (bundled / "apple").mkdir(parents=True)
+        (bundled / "apple" / "DESCRIPTION.md").write_text(
+            "---\ndescription: Apple\n---\n"
+        )
+        (bundled / "apple" / "imessage").mkdir()
+        (bundled / "apple" / "imessage" / "SKILL.md").write_text(
+            "---\nname: imessage\n---\n# iMessage\n"
+        )
+        return bundled
+
+    def test_marker_skips_skill_and_description_copy(self, tmp_path):
+        bundled = self._setup_bundled(tmp_path)
+        skills_dir = tmp_path / "profile" / "skills"
+        skills_dir.mkdir(parents=True)
+        manifest_file = skills_dir / ".bundled_manifest"
+        # Marker lives at HERMES_HOME root (skills_dir.parent in this test).
+        from hermes_cli.profiles import NO_BUNDLED_SKILLS_MARKER
+        (skills_dir.parent / NO_BUNDLED_SKILLS_MARKER).write_text("opt-out\n")
+
+        with patch("tools.skills_sync._get_bundled_dir", return_value=bundled), \
+             patch("tools.skills_sync.SKILLS_DIR", skills_dir), \
+             patch("tools.skills_sync.MANIFEST_FILE", manifest_file):
+            result = sync_skills(quiet=True)
+
+        assert result.get("skipped_opt_out") is True
+        assert result["copied"] == []
+        # The bug we're guarding: no DESCRIPTION.md must land in the profile.
+        assert not (skills_dir / "apple" / "DESCRIPTION.md").exists()
+        # And no SKILL.md either.
+        assert not (skills_dir / "apple" / "imessage" / "SKILL.md").exists()
+
+    def test_no_marker_proceeds_normally(self, tmp_path):
+        """Sanity check: without the marker sync still copies as before."""
+        bundled = self._setup_bundled(tmp_path)
+        skills_dir = tmp_path / "profile" / "skills"
+        skills_dir.mkdir(parents=True)
+        manifest_file = skills_dir / ".bundled_manifest"
+
+        with patch("tools.skills_sync._get_bundled_dir", return_value=bundled), \
+             patch("tools.skills_sync.SKILLS_DIR", skills_dir), \
+             patch("tools.skills_sync.MANIFEST_FILE", manifest_file):
+            result = sync_skills(quiet=True)
+
+        assert result.get("skipped_opt_out") is not True
+        assert (skills_dir / "apple" / "DESCRIPTION.md").exists()
+        assert (skills_dir / "apple" / "imessage" / "SKILL.md").exists()

tools/skills_sync.py

@@ -176,10 +176,39 @@ def sync_skills(quiet: bool = False) -> dict:
     """
     Sync bundled skills into ~/.hermes/skills/ using the manifest.
 
+    Honors the ``.no-bundled-skills`` opt-out marker at ``HERMES_HOME``: when
+    present, the entire sync (SKILL.md *and* category DESCRIPTION.md files)
+    is skipped. This is defense-in-depth — every documented caller (gateway
+    startup, ``hermes update``, ``seed_profile_skills``) is supposed to check
+    the marker before calling us, but historically the DESCRIPTION.md
+    loop below ran unconditionally even when the SKILL.md path was
+    correctly bypassed at the caller, leaving stale category stubs in
+    opted-out profile skills/ dirs (see PR #15 follow-up).
+
     Returns:
         dict with keys: copied (list), updated (list), skipped (int),
-                        user_modified (list), cleaned (list), total_bundled (int)
+                        user_modified (list), cleaned (list), total_bundled (int).
+        When the opt-out marker is set, returns a zero-effect dict with
+        ``skipped_opt_out=True`` so callers can distinguish "skipped due to
+        marker" from "nothing to do".
     """
+    # Defense-in-depth marker check. SKILLS_DIR's parent is HERMES_HOME by
+    # construction (module-level constant), so resolving the marker relative
+    # to it works regardless of how callers patch globals in tests.
+    try:
+        from hermes_cli.profiles import NO_BUNDLED_SKILLS_MARKER
+        marker = SKILLS_DIR.parent / NO_BUNDLED_SKILLS_MARKER
+        if marker.exists():
+            return {
+                "copied": [], "updated": [], "skipped": 0,
+                "user_modified": [], "cleaned": [], "total_bundled": 0,
+                "skipped_opt_out": True,
+            }
+    except Exception:
+        # Best-effort — if the import fails (circular import edge cases,
+        # partial installs) fall through to the normal sync path.
+        pass
+
     bundled_dir = _get_bundled_dir()
     if not bundled_dir.exists():
         return {