mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
feat(skills): org skills are editable in place; local edits survive org updates
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 <skill>".
- 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.
This commit is contained in:
parent
e19ac3b745
commit
981feb6730
7 changed files with 370 additions and 45 deletions
|
|
@ -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]:
|
||||
|
|
|
|||
|
|
@ -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 <skill>`. "
|
||||
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":
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 <name>` 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]:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 <skill>``. 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:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue