From ef882a5595efb88f73295fb5b40fb390d5dd8505 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sun, 23 Aug 2026 23:20:13 +0800 Subject: [PATCH] fix(curator): say what pin actually does on an unmanaged skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes curator pin` guarded on is_agent_created (a filesystem-shape check), but the flag only matters when the skill carries the curator-management marker: curated_report() walks marker-carrying skills only, so auto-transitions never consider an unmanaged (pre-marker) skill at all. Pinning one recorded the flag and then printed "will bypass auto-transitions" — an effect that does not exist. Keep the write (the flag becomes meaningful after `hermes curator adopt`) and branch the message on is_curator_managed: unmanaged pins now say the skill is unmanaged and point at adopt. Unpin gets the symmetric wording. --- hermes_cli/curator.py | 19 ++++ .../hermes_cli/test_curator_pin_unmanaged.py | 100 ++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 tests/hermes_cli/test_curator_pin_unmanaged.py diff --git a/hermes_cli/curator.py b/hermes_cli/curator.py index b65d9016d1..96a53f35af 100644 --- a/hermes_cli/curator.py +++ b/hermes_cli/curator.py @@ -302,6 +302,19 @@ def _cmd_pin(args) -> int: "`hermes curator list-unmanaged` shows which skills the curator tracks." ) return 1 + if not skill_usage.is_curator_managed(args.skill): + # Unmanaged (pre-marker) skills are never touched by auto-transitions, + # so "will bypass auto-transitions" overstates what this pin does. The + # pin IS recorded (and now visible in `curator status`, #92993) but + # only becomes protective once the skill is adopted. Say so, and point + # at the handover command (#93002). + print( + f"curator: pinned '{args.skill}' (recorded; this skill is unmanaged " + "— auto-transitions never consider it. Run " + f"`hermes curator adopt {args.skill}` to put it under curator " + "management)" + ) + return 0 print(f"curator: pinned '{args.skill}' (will bypass auto-transitions)") return 0 @@ -320,6 +333,12 @@ def _cmd_unpin(args) -> int: "curation-eligible (protected built-in or external)." ) return 1 + if not skill_usage.is_curator_managed(args.skill): + print( + f"curator: unpinned '{args.skill}' (recorded; this skill is " + "unmanaged — it was never under auto-transitions to begin with)" + ) + return 0 print(f"curator: unpinned '{args.skill}'") return 0 diff --git a/tests/hermes_cli/test_curator_pin_unmanaged.py b/tests/hermes_cli/test_curator_pin_unmanaged.py new file mode 100644 index 0000000000..8688919354 --- /dev/null +++ b/tests/hermes_cli/test_curator_pin_unmanaged.py @@ -0,0 +1,100 @@ +"""Pin/unpin messaging on unmanaged skills (#92993). + +`hermes curator pin ` on an unmanaged skill (curation-eligible but no +`created_by` marker — the pre-marker population `list-unmanaged` shows) used +to print "will bypass auto-transitions" and exit 0. But `curated_report()` +only walks marker-carrying skills, so auto-transitions never consider an +unmanaged skill at all: the pin is recorded yet inert, and the message +claimed an effect that does not exist. The fix keeps the write (the flag +becomes meaningful after `hermes curator adopt`) and makes the message say +what actually happened. +""" + +from __future__ import annotations + +from types import SimpleNamespace + + +def _ns(skill: str) -> SimpleNamespace: + return SimpleNamespace(skill=skill) + + +def _stub(monkeypatch, *, managed: bool): + """Point the CLI at stubbed skill_usage surfaces. + + ``managed`` drives ``is_curator_managed`` — the policy flag the new + branch reads. ``is_agent_created`` stays True so the existing bundled/ + hub refusal guard passes and the code reaches the managed check. + """ + import tools.skill_usage as skill_usage + + calls: list[tuple[str, bool]] = [] + monkeypatch.setattr(skill_usage, "is_agent_created", lambda name: True) + monkeypatch.setattr(skill_usage, "is_curator_managed", lambda name: managed) + monkeypatch.setattr( + skill_usage, + "set_pinned", + lambda name, pinned: calls.append((name, pinned)), + ) + return calls + + +def test_pin_unmanaged_records_flag_and_prints_adopt_hint(monkeypatch, capsys): + import hermes_cli.curator as curator_cli + + calls = _stub(monkeypatch, managed=False) + + rc = curator_cli._cmd_pin(_ns("legacy-skill")) + + # The write still happens — the flag becomes meaningful after adopt. + assert calls == [("legacy-skill", True)] + assert rc == 0 + out = capsys.readouterr().out + assert "unmanaged" in out + assert "hermes curator adopt legacy-skill" in out + # The old lie must be gone: the pin does NOT bypass anything here. + assert "will bypass auto-transitions" not in out + + +def test_pin_managed_keeps_bypass_message(monkeypatch, capsys): + import hermes_cli.curator as curator_cli + + calls = _stub(monkeypatch, managed=True) + + rc = curator_cli._cmd_pin(_ns("agent-skill")) + + assert calls == [("agent-skill", True)] + assert rc == 0 + out = capsys.readouterr().out + assert "will bypass auto-transitions" in out + assert "unmanaged" not in out + + +def test_unpin_unmanaged_says_it_was_never_managed(monkeypatch, capsys): + import hermes_cli.curator as curator_cli + + calls = _stub(monkeypatch, managed=False) + + rc = curator_cli._cmd_unpin(_ns("legacy-skill")) + + assert calls == [("legacy-skill", False)] + assert rc == 0 + out = capsys.readouterr().out + assert "unmanaged" in out + assert "never under auto-transitions" in out + + +def test_pin_still_refuses_bundled_skills(monkeypatch, capsys): + import hermes_cli.curator as curator_cli + import tools.skill_usage as skill_usage + + calls = _stub(monkeypatch, managed=True) + # _stub leaves is_agent_created True; override AFTER so the refusal + # guard fires before the managed check. + monkeypatch.setattr(skill_usage, "is_agent_created", lambda name: False) + + rc = curator_cli._cmd_pin(_ns("bundled-skill")) + + assert rc == 1 + assert calls == [] + assert "cannot pin" in capsys.readouterr().out