From 981feb673055f9c45e98887af4dec1be462bfda6 Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Tue, 28 Jul 2026 09:06:34 +1000 Subject: [PATCH] feat(skills): org skills are editable in place; local edits survive org updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read-only org mirror broke the learning loop precisely where it matters most. The system prompt tells every agent to patch a skill the moment it finds a gap, and shared skills are the ones the most people use — but every write to _org/ was refused, and the curator was excluded from them outright. So org skills froze while personal skills kept improving, and the offered alternative ("fork it into a personal skill, then propose the fork") is not something an agent does mid-task. The refusal WAS the feature; improvements were simply lost, and manual forks would have fragmented the shared set. Edit in place: - skill_manage patch/edit/write_file now work on org skills. Only delete is still refused (the mirror is a view of org HEAD — a local delete returns on the next pull; removing a shared skill is an admin action). - Org skills are curation-eligible again, so the curator can improve the highest-leverage skills in the system instead of skipping them. - The load-time provenance header now says edits are allowed and kept, instead of instructing the agent not to edit. Local edits are never overwritten: - pull_org_skills previously rmtree'd each skill dir and re-materialized it, silently destroying local work on the next session start. It now records a content fingerprint per skill (.org-baseline.json) when it writes one, and SKIPS any skill whose local content diverges from that baseline. - When upstream ALSO changed such a skill, it is reported in the pull result's "conflicted" list and left untouched for the user to resolve deliberately (propose the local version, or delete it and re-pull to take theirs). A missing baseline is treated as unmodified so pre-existing mirrors do not raise phantom conflicts. - Fingerprints are content-based (path + bytes, sorted), so a touch/mtime change is not mistaken for an edit. Sharing back: - Default: the edit stays local and the tool result tells the user to run "hermes skills propose ". - Opt-in sync.org_auto_propose / HERMES_SYNC_ORG_AUTO_PROPOSE submits each edit immediately. Defaults OFF — pushing every agent edit to a whole organisation is not a safe default. A failed submission never fails the edit; the change is saved and can be proposed later. - "hermes sync status" lists org skills with unshared local edits; "hermes sync pull" reports conflicts it declined to overwrite. Tests: 25 in the namespace suite (was 15). The two that asserted the old read-only behaviour now assert the opposite. New coverage for edit-applied, share-back guidance, delete-still-refused, curation-allowed, edit detection, missing-baseline tolerance, mtime-insensitivity, and the auto-propose default. 428 passed / 0 failed via scripts/run_tests.sh. Verified through the REAL pull path against a mock plane: pull v1 -> edit in place -> upstream ships v2 -> pull leaves the local edit intact, reports the conflict, and surfaces it in status. --- agent/skill_utils.py | 3 + hermes_cli/main.py | 26 ++++- tests/agent/test_org_skill_namespace.py | 132 +++++++++++++++++++++-- tools/skill_manager_tool.py | 89 +++++++++++++--- tools/skill_usage.py | 12 +-- tools/skills_sync_client.py | 133 +++++++++++++++++++++++- tools/skills_tool.py | 20 ++-- 7 files changed, 370 insertions(+), 45 deletions(-) diff --git a/agent/skill_utils.py b/agent/skill_utils.py index ea4bb2ea27d..9276f145abd 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -62,6 +62,9 @@ SKILL_SUPPORT_DIRS = frozenset(("references", "templates", "assets", "scripts")) ORG_MIRROR_DIR_NAME = "_org" ORG_ACTIVE_MARKER = ".active_org" ORG_PROVENANCE_FILE = ".org-provenance.json" +# Records the fingerprint of each skill exactly as upstream sent it, so a +# later local edit is detectable and an org pull can refuse to clobber it. +ORG_BASELINE_FILE = ".org-baseline.json" def read_active_org_id(skills_dir: Path) -> Optional[str]: diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 7124009c143..ac374175b6b 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -4508,12 +4508,21 @@ def cmd_sync(args): print(_json.dumps(status, indent=2, ensure_ascii=False)) if status.get("org_available"): n = len(status.get("org_skills") or []) + modified = status.get("org_skills_modified") or [] print( - f"\nOrg skills: {n} shared skill(s) mirrored read-only from " - f"your organisation (your role: {status.get('org_role')}). " - f"They load alongside your own, labeled by origin.", + f"\nOrg skills: {n} shared skill(s) from your organisation " + f"(your role: {status.get('org_role')}). They load alongside " + f"your own, labeled by origin, and you can edit them.", file=sys.stderr, ) + if modified: + print( + f" {len(modified)} with local edits not yet shared: " + f"{', '.join(modified)}\n" + f" Share them back with `hermes skills propose `. " + f"Org updates will not overwrite them.", + file=sys.stderr, + ) elif status.get("logged_in"): print( "\nOrg skills: not applicable — this account isn't a member " @@ -4574,6 +4583,17 @@ def cmd_sync(args): f"organisation.", file=sys.stderr, ) + clashes = org_result.get("conflicted") or [] + if clashes: + print( + f"org: {len(clashes)} skill(s) have BOTH local edits " + f"and org updates, so they were left as-is: " + f"{', '.join(clashes)}\n" + f" Your local version is intact. Review it, then " + f"either propose it or delete the local copy and pull " + f"again to take the org version.", + file=sys.stderr, + ) elif sub == "push": result = ssc.push_skills(identity=identity, message="hermes sync push") elif sub == "now": diff --git a/tests/agent/test_org_skill_namespace.py b/tests/agent/test_org_skill_namespace.py index 3c6ed7db6fe..b86d9a7dad9 100644 --- a/tests/agent/test_org_skill_namespace.py +++ b/tests/agent/test_org_skill_namespace.py @@ -150,30 +150,61 @@ class TestListingCollisionsAndLabels: assert "[name collision" not in out -class TestOrgMirrorReadOnly: - def test_skill_manage_patch_refuses_org_mirror(self, tmp_path, monkeypatch): +class TestOrgSkillsAreEditableInPlace: + """The learning loop must work ON shared skills, not around them. + + Refusing edits to `_org/` froze exactly the skills the most people use: + the agent is instructed to patch a skill the moment it finds a gap, and + "fork it to a personal skill first" is not something an agent does + mid-task. So edits land in place; org updates never clobber them; the + user (or auto-propose) shares them back. + """ + + def _org_skill(self, tmp_path, monkeypatch): from tools import skill_manager_tool as smt + from agent import skill_utils as _sku skills = tmp_path / "skills" - _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x") + d = _mk_skill( + skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x" + ) _mark_active(skills, "org-1") monkeypatch.setattr(smt, "_skills_dir", lambda: skills) - from agent import skill_utils as _sku monkeypatch.setattr( _sku, "get_all_skills_dirs", lambda: [skills], raising=True ) - result = smt._patch_skill("shared-x", "body", "hacked") - assert result["success"] is False - assert "ORG-SHARED" in result["error"] - assert "propose" in result["error"] + return smt, skills, d - def test_curation_exempt(self, tmp_path, monkeypatch): + def test_patch_is_allowed_and_applied(self, tmp_path, monkeypatch): + smt, _skills, d = self._org_skill(tmp_path, monkeypatch) + result = smt._patch_skill("shared-x", "body", "improved") + assert result["success"] is True, result.get("error") + assert "improved" in (d / "SKILL.md").read_text(encoding="utf-8") + + def test_edit_tells_the_user_how_to_share_it_back(self, tmp_path, monkeypatch): + smt, _skills, _d = self._org_skill(tmp_path, monkeypatch) + result = smt._patch_skill("shared-x", "body", "improved") + # Without auto-propose the edit stays local, and the tool result must + # say so AND name the command — otherwise the improvement is stranded. + assert "propose" in (result.get("org_sharing") or "") + + def test_delete_is_still_refused(self, tmp_path, monkeypatch): + smt, _skills, d = self._org_skill(tmp_path, monkeypatch) + guard = smt._org_mirror_write_guard("shared-x", d, "delete") + assert guard is not None and guard["success"] is False + assert "admin" in guard["error"] + + def test_curation_is_allowed(self, tmp_path, monkeypatch): from tools import skill_usage as su skills = tmp_path / "skills" - d = _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x") + d = _mk_skill( + skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x" + ) monkeypatch.setattr(su, "_skills_dir", lambda: skills) - assert su.is_curation_eligible("shared-x", d) is False + # The curator must be able to improve shared skills — they are the + # highest-leverage ones in the system. + assert su.is_curation_eligible("shared-x", d) is True class TestOrgPullIsWiredIn: @@ -275,3 +306,82 @@ class TestOrgSharingIsDiscoverable: assert "hermes skills propose" in src, ( "`hermes sync --help` must point at the org-sharing command." ) + + +class TestLocalEditsSurviveOrgUpdates: + """Ben's requirement: local edits are never silently overwritten. + + An org pull materializes the shared set. Before this, it `rmtree`'d each + skill dir and re-wrote it, so any local improvement vanished on the next + session start with no warning. Now a locally-modified skill is skipped + and reported as a conflict for the user to resolve deliberately. + """ + + def _mirror(self, tmp_path, monkeypatch, body="original\n"): + from tools import skills_sync_client as ssc + + skills = tmp_path / "skills" + d = _mk_skill( + skills, + f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", + name="shared-x", + body=body, + ) + _mark_active(skills, "org-1") + monkeypatch.setattr(ssc, "_skills_dir", lambda: skills) + monkeypatch.setattr( + ssc, "_org_dir", lambda: skills / sku.ORG_MIRROR_DIR_NAME + ) + return ssc, skills, d + + def test_unmodified_skill_is_not_flagged(self, tmp_path, monkeypatch): + ssc, _skills, d = self._mirror(tmp_path, monkeypatch) + ssc._write_org_baseline( + "org-1", + {"shared-x": {"fingerprint": ssc._skill_dir_fingerprint(d), "tree": "t1"}}, + ) + assert ssc.org_skill_is_locally_modified("shared-x", "org-1") is False + assert ssc.list_locally_modified_org_skills("org-1") == [] + + def test_edited_skill_is_detected(self, tmp_path, monkeypatch): + ssc, _skills, d = self._mirror(tmp_path, monkeypatch) + ssc._write_org_baseline( + "org-1", + {"shared-x": {"fingerprint": ssc._skill_dir_fingerprint(d), "tree": "t1"}}, + ) + (d / "SKILL.md").write_text("---\nname: shared-x\n---\nEDITED\n", encoding="utf-8") + assert ssc.org_skill_is_locally_modified("shared-x", "org-1") is True + assert ssc.list_locally_modified_org_skills("org-1") == ["shared-x"] + + def test_missing_baseline_does_not_cry_wolf(self, tmp_path, monkeypatch): + ssc, _skills, _d = self._mirror(tmp_path, monkeypatch) + # Mirror pulled before baselines existed — must not be reported as + # modified (that would block every update with a phantom conflict). + assert ssc.org_skill_is_locally_modified("shared-x", "org-1") is False + + def test_fingerprint_is_content_based_not_mtime(self, tmp_path, monkeypatch): + import os + import time + + ssc, _skills, d = self._mirror(tmp_path, monkeypatch) + before = ssc._skill_dir_fingerprint(d) + time.sleep(0.01) + os.utime(d / "SKILL.md", None) # touch: mtime changes, content doesn't + assert ssc._skill_dir_fingerprint(d) == before + + def test_auto_propose_defaults_off(self, monkeypatch): + from tools import skills_sync_client as ssc + + monkeypatch.delenv("HERMES_SYNC_ORG_AUTO_PROPOSE", raising=False) + monkeypatch.setattr( + "hermes_cli.config.load_config", lambda: {}, raising=False + ) + # Default must be OFF: silently pushing every agent edit to the whole + # organisation is not a safe default. + assert ssc.sync_org_auto_propose() is False + + def test_auto_propose_can_be_enabled_by_env(self, monkeypatch): + from tools import skills_sync_client as ssc + + monkeypatch.setenv("HERMES_SYNC_ORG_AUTO_PROPOSE", "1") + assert ssc.sync_org_auto_propose() is True diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index ffb88deb337..5064b25a048 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -622,14 +622,61 @@ def _find_skill(name: str) -> Optional[Dict[str, Any]]: return None -def _org_mirror_write_guard(name: str, skill_path: Path, action: str) -> Optional[Dict[str, Any]]: - """Refuse writes to org-mirror skills (M2, contract §11.11 / design §7.1). +def _maybe_auto_propose_org_edit(name: str, skill_path: Path) -> Optional[str]: + """Submit an org-skill edit upstream when `sync.org_auto_propose` is on. - The ``_org/`` mirror is materialized FROM the org HEAD and overwritten on - every pull — a local edit would be silently lost AND would misrepresent - admin-approved shared content. The change path is: fork into a personal - skill, edit, then ``hermes skills propose``. + Returns a short note for the tool result, or None when nothing happened. + Never raises: an offline/failed submission must not fail the edit itself — + the change is already saved locally and can be proposed later. """ + try: + from agent.skill_utils import is_org_mirror_path + from tools import skills_sync_client as ssc + + if not is_org_mirror_path(skill_path, _skills_dir()): + return None + if not ssc.sync_org_auto_propose(): + return ( + f"This skill is shared by your organisation. Your edit is " + f"saved locally and will not be overwritten by org updates. " + f"Run `hermes skills propose {name}` to share it back." + ) + result = ssc.propose_skill(name) + if result.get("proposal_pending"): + return ( + f"Auto-proposed to your organisation as proposal " + f"#{result.get('proposal_id')} (pending admin review)." + ) + return "Auto-proposed to your organisation (merged into the shared set)." + except Exception as e: + logger.debug("auto-propose skipped for %s: %s", name, e) + return ( + f"Edit saved locally. Could not submit it to your organisation " + f"right now — run `hermes skills propose {name}` to retry." + ) + + +def _org_mirror_write_guard(name: str, skill_path: Path, action: str) -> Optional[Dict[str, Any]]: + """Org-shared skills are EDITABLE IN PLACE — this only blocks deletion. + + Earlier versions refused every write to `_org/`, which broke the learning + loop exactly where it matters most: the agent is told to patch a skill the + moment it finds a gap, and shared skills are the ones the most people use. + Blocking that froze org skills while personal ones kept improving, and the + "fork it into a personal skill" alternative is not something an agent does + mid-task — so improvements were simply lost. + + Now an edit lands in the mirror and is protected from being overwritten by + the next org pull (see the baseline sidecar in skills_sync_client). It + reaches the organisation when the user runs `hermes skills propose`, or + immediately if `sync.org_auto_propose` is on. + + Deletion is still refused: the mirror is a materialized view of the org + HEAD, so a local delete is meaningless (the next pull restores it) and + removing a skill for the organisation is an admin action, not a local one. + """ + if action not in {"delete", "remove_file"}: + return None try: from agent.skill_utils import is_org_mirror_path @@ -637,12 +684,12 @@ def _org_mirror_write_guard(name: str, skill_path: Path, action: str) -> Optiona return { "success": False, "error": ( - f"Refusing {action} for '{name}': it is an ORG-SHARED " - "skill (read-only mirror of your org's approved set; " - "local edits are overwritten on every org pull). To " - "change it: copy it to a personal skill, edit that, then " - "`hermes skills propose ` so an org admin can " - "review and approve." + f"Cannot {action} '{name}' locally: it is shared by your " + "organisation, so a local delete would just come back on " + "the next sync. Ask an org admin to remove it for " + "everyone. (Editing it IS allowed — your changes are kept " + "and can be proposed back with `hermes skills propose " + f"{name}`.)" ), } except Exception: @@ -954,12 +1001,17 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]: except Exception: pass - return { + result = { "success": True, "message": f"Skill '{name}' updated (full rewrite).", "path": str(existing["path"]), "_change": {"description": _desc}, } + org_note = _maybe_auto_propose_org_edit(name, existing["path"]) + if org_note: + result["org_sharing"] = org_note + result["message"] = f"{result['message']} {org_note}" + return result def _patch_skill( @@ -1075,6 +1127,10 @@ def _patch_skill( "old": old_string[:200] + ("…" if len(old_string) > 200 else ""), "new": new_string[:200] + ("…" if len(new_string) > 200 else ""), } + org_note = _maybe_auto_propose_org_edit(name, skill_dir) + if org_note: + result["org_sharing"] = org_note + result["message"] = f"{result['message']} {org_note}" return result @@ -1244,11 +1300,16 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]: target.unlink(missing_ok=True) return {"success": False, "error": scan_error} - return { + result = { "success": True, "message": f"File '{file_path}' written to skill '{name}'.", "path": str(target), } + org_note = _maybe_auto_propose_org_edit(name, existing["path"]) + if org_note: + result["org_sharing"] = org_note + result["message"] = f"{result['message']} {org_note}" + return result def _remove_file(name: str, file_path: str) -> Dict[str, Any]: diff --git a/tools/skill_usage.py b/tools/skill_usage.py index 8c0fc848a47..7ea57d5480a 100644 --- a/tools/skill_usage.py +++ b/tools/skill_usage.py @@ -450,18 +450,16 @@ def is_curation_eligible(skill_name: str, skill_path: Optional[Path] = None) -> Agent-created skills are always eligible. Bundled built-ins become eligible only when ``curator.prune_builtins`` is enabled. Hub-installed and external skill-dir skills are NEVER eligible — they have an external upstream owner. - Org-mirror skills (``_org/``) are NEVER eligible — the org HEAD owns them; - curation happens via propose → approve, not local archive/consolidate. + Org-shared skills ARE eligible for improvement (the curator may patch them + like any other skill; edits stay local until proposed) but are protected + from ARCHIVE/DELETE elsewhere — removing a shared skill is an org-admin + action, not a local curation decision. Protected built-ins (``PROTECTED_BUILTIN_SKILLS``) are NEVER eligible regardless of any flag — they back load-bearing UX and must never be archived or consolidated. """ - from agent.skill_utils import is_org_mirror_path - if skill_path is not None and is_external_skill_path(skill_path): return False - if skill_path is not None and is_org_mirror_path(skill_path, _skills_dir()): - return False if is_protected_builtin(skill_name): return False if is_hub_installed(skill_name): @@ -470,8 +468,6 @@ def is_curation_eligible(skill_name: str, skill_path: Optional[Path] = None) -> return _prune_builtins_enabled() local_dir = _find_skill_dir(skill_name) if local_dir is not None: - if is_org_mirror_path(local_dir, _skills_dir()): - return False return not is_external_skill_path(local_dir) if _find_external_skill_dir(skill_name) is not None: return False diff --git a/tools/skills_sync_client.py b/tools/skills_sync_client.py index daa582f8c28..0bc604ac12e 100644 --- a/tools/skills_sync_client.py +++ b/tools/skills_sync_client.py @@ -377,6 +377,25 @@ def sync_feature_enabled() -> bool: return _sync_config_bool("HERMES_SYNC_ENABLED", "enabled", default=False) +def sync_org_auto_propose() -> bool: + """Whether an agent/user edit to an org skill is proposed automatically. + + ``HERMES_SYNC_ORG_AUTO_PROPOSE`` -> ``sync.org_auto_propose`` -> False. + + False (default): edits to an org-shared skill stay LOCAL until the user + runs ``hermes skills propose ``. The skill keeps working with the + edit applied; the organisation just doesn't see it yet. + + True: every local edit to an org skill is submitted to the org as a + proposal right away (an admin still approves it, unless the editor is an + admin). Suits a small, high-trust team that wants improvements to flow + back without anyone remembering to push them. + """ + return _sync_config_bool( + "HERMES_SYNC_ORG_AUTO_PROPOSE", "org_auto_propose", default=False + ) + + def sync_default_opt_in() -> bool: """The personal sync default opt-in policy (env-first). @@ -1565,6 +1584,8 @@ def sync_status() -> Dict[str, Any]: "org_id": None, "org_role": None, "org_skills": [], + # Org skills edited locally and not yet shared back. + "org_skills_modified": [], } try: identity = resolve_identity() @@ -1586,6 +1607,9 @@ def sync_status() -> Dict[str, Any]: status["org_id"] = org_identity.get("org_id") status["org_role"] = org_identity.get("org_role") status["org_skills"] = list_org_skill_names() + status["org_skills_modified"] = list_locally_modified_org_skills( + status["org_id"] + ) except SyncInertError: pass except Exception as e: @@ -1724,15 +1748,34 @@ def pull_org_skills( dest_root = _org_dir() / org_id updated: List[str] = [] + # Skills the user/agent has edited locally and upstream also changed. + # We do NOT overwrite them — the local work wins until the user resolves. + conflicted: List[str] = [] + baseline = _read_org_baseline(org_id) for rel_path, tree_hash in sorted(skill_trees.items()): dest = dest_root / PurePosixPath(rel_path) try: if dest.exists(): + # Local edits are protected: never clobber work the user or + # agent did in place. Skip the update and report it so they + # can resolve deliberately (propose the local version, or + # discard it and re-pull). + if org_skill_is_locally_modified(rel_path, org_id): + prev = baseline.get(rel_path) or {} + # Upstream also moved on => a real conflict the user must + # resolve. Upstream unchanged => their edit simply stands. + if prev.get("tree") != tree_hash: + conflicted.append(rel_path) + continue import shutil shutil.rmtree(dest) dest.mkdir(parents=True, exist_ok=True) materialize_tree(client, tree_hash, dest) + baseline[rel_path] = { + "fingerprint": _skill_dir_fingerprint(dest), + "tree": tree_hash, + } updated.append(rel_path) except Exception as e: logger.warning( @@ -1754,7 +1797,95 @@ def pull_org_skills( "skills": updated, }, ) - return {"ok": True, "org_id": org_id, "head": head, "updated": updated} + _write_org_baseline(org_id, baseline) + if conflicted: + logger.warning( + "skills_sync_client: %d org skill(s) have local edits AND upstream " + "changes; left untouched: %s", + len(conflicted), + ", ".join(conflicted), + ) + return { + "ok": True, + "org_id": org_id, + "head": head, + "updated": updated, + "conflicted": conflicted, + } + + +def _skill_dir_fingerprint(path: Path) -> str: + """Stable content hash of a materialized skill directory. + + Used to tell "the user/agent edited this org skill" from "this is exactly + what upstream shipped". Hashes every file's relative path + bytes, sorted, + so it is independent of filesystem ordering and mtimes. + """ + h = hashlib.sha256() + try: + for f in sorted(p for p in path.rglob("*") if p.is_file()): + h.update(str(f.relative_to(path)).replace("\\", "/").encode("utf-8")) + h.update(b"\0") + h.update(f.read_bytes()) + h.update(b"\0") + except OSError as e: + logger.debug("skills_sync_client: fingerprint failed for %s: %s", path, e) + return "" + return h.hexdigest() + + +def _org_baseline_path(org_id: str) -> Path: + """Sidecar recording the upstream fingerprint of each mirrored skill.""" + from agent.skill_utils import ORG_BASELINE_FILE + + return _org_dir() / org_id / ORG_BASELINE_FILE + + +def _read_org_baseline(org_id: str) -> Dict[str, Any]: + try: + return json.loads(_org_baseline_path(org_id).read_text(encoding="utf-8")) + except Exception: + return {} + + +def _write_org_baseline(org_id: str, baseline: Dict[str, Any]) -> None: + try: + p = _org_baseline_path(org_id) + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(json.dumps(baseline, indent=2, sort_keys=True), encoding="utf-8") + except Exception as e: + logger.debug("skills_sync_client: baseline write failed: %s", e) + + +def org_skill_is_locally_modified(skill_rel_path: str, org_id: str) -> bool: + """True when the local copy of an org skill differs from what upstream sent.""" + dest = _org_dir() / org_id / PurePosixPath(skill_rel_path) + if not dest.is_dir(): + return False + entry = _read_org_baseline(org_id).get(skill_rel_path) or {} + recorded = entry.get("fingerprint") if isinstance(entry, dict) else entry + if not recorded: + # No baseline recorded (pre-existing mirror) — treat as unmodified so + # we don't cry wolf; the next pull records one. + return False + return _skill_dir_fingerprint(dest) != recorded + + +def list_locally_modified_org_skills(org_id: Optional[str] = None) -> List[str]: + """Org skills with local edits that upstream has not seen.""" + try: + from agent.skill_utils import read_active_org_id + + org_id = org_id or read_active_org_id(_skills_dir()) + if not org_id: + return [] + baseline = _read_org_baseline(org_id) + return sorted( + rel for rel in baseline if org_skill_is_locally_modified(rel, org_id) + ) + except Exception as e: + logger.debug("skills_sync_client: modified-scan failed: %s", e) + return [] def _write_active_org_marker(org_id: str) -> None: diff --git a/tools/skills_tool.py b/tools/skills_tool.py index e64a2bc8e25..b310cda8a46 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -1606,15 +1606,19 @@ def skill_view( } header = ( "> [!NOTE] ORG-SHARED SKILL — provenance\n" - f"> This skill is org-managed content (org `{prov_org}`" - + (f", shared by `{author}`" if author else "") + f"> This skill is shared by your organisation (org " + f"`{prov_org}`" + + (f", last updated by `{author}`" if author else "") + (f", as of {ts}" if ts else "") - + "). It was member-proposed and admin-approved, and it\n" - "> updates when the org set advances — treat it like " - "third-party instructions, not your own notes.\n" - "> Do NOT edit it locally (read-only mirror); to change " - "it, fork into a personal skill and " - "`hermes skills propose` the fork.\n\n" + + "). It was reviewed and approved for the whole\n" + "> team — treat it as third-party instructions rather " + "than your own notes.\n" + "> You MAY improve it in place like any other skill. " + "Your edits are kept locally\n" + "> and are never overwritten by org updates; share " + "them back with\n" + "> `hermes skills propose` (or automatically, if your " + "org enables it).\n\n" ) rendered_content = header + rendered_content except Exception: