fix(learn): acknowledge /learn on the GUI surfaces and name the created skill

agent/display.py

@@ -990,6 +990,31 @@ def _cute_skill_view(a: dict, _r) -> str:
     return f"┊ 📚 skill     {_cute_trunc(label)}"
 
 
+_SKILL_MANAGE_VERBS = {
+    "create": "created", "patch": "updated", "edit": "updated",
+    "write_file": "wrote", "remove_file": "removed", "delete": "deleted",
+}
+
+
+def _cute_skill_manage(a: dict, r) -> str:
+    """Completion line naming the skill a ``skill_manage`` call created or changed. The tool
+    advertises ONE call shape — an ``operations`` array — whose first op names the line; the
+    legacy flat top-level fields still take precedence when present (old transcripts, staged replay)."""
+    ops = a.get("operations")
+    head = ops[0] if isinstance(ops, list) and ops and isinstance(ops[0], dict) else {}
+    action = str(a.get("action") or head.get("action") or "").lower()
+    name = _cute_trunc(str(a.get("name") or head.get("name") or "").strip() or "skill")
+    more = f" +{len(ops) - 1}" if isinstance(ops, list) and len(ops) > 1 else ""
+    data = safe_json_loads(r) if r else None
+    if isinstance(data, dict) and data.get("staged"):
+        verb = "staged"                 # write_approval staged it; nothing was saved yet
+    elif not _result_succeeded(r):
+        verb = action or "updated"      # failed/unknown: report the intent; the failure suffix marks the outcome
+    else:
+        verb = _SKILL_MANAGE_VERBS.get(action, action or "updated")
+    return f"┊ 📚 skill     {verb} {name}{more}"
+
+
 def _cute_cronjob(a: dict, _r) -> str:
     action = a.get("action", "?")
     if action == "create":
@@ -1052,6 +1077,7 @@ def _cute_process_manage(a: dict, _r) -> str:
     "memory": _cute_memory,
     "skills_list": lambda a, r: f"┊ 📚 skills    list {a.get('category', 'all')}",
     "skill_view": _cute_skill_view,
+    "skill_manage": _cute_skill_manage,
     "image_generate": lambda a, r: f"┊ 🎨 create    {_cute_trunc(a.get('prompt', ''))}",
     "text_to_speech": lambda a, r: f"┊ 🔊 speak     {_cute_trunc(a.get('text', ''))}",
     "vision_analyze": lambda a, r: f"┊ 👁️  vision    {_cute_trunc(a.get('question', ''))}",

tests/agent/test_display.py

@@ -200,6 +200,154 @@ def test_browser_type_cute_message_keeps_normal_text(self):
         assert text in line
 
 
+class TestCuteSkillManage:
+    """skill_manage completion lines must name the skill that was created/changed (#52085)."""
+
+    def test_create_success_names_the_skill_and_verb(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"action": "create", "name": "deploy-runbook"},
+            0.1,
+            result='{"success": true, "message": "Skill \'deploy-runbook\' created."}',
+        )
+        assert "created" in line
+        assert "deploy-runbook" in line
+
+    def test_patch_success_reports_updated(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"action": "patch", "name": "x"},
+            0.1,
+            result='{"success": true, "message": "Skill \'x\' patched."}',
+        )
+        assert "updated" in line
+        assert " x" in line
+
+    def test_failed_create_does_not_claim_created(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"action": "create", "name": "deploy-runbook"},
+            0.1,
+            result='{"success": false, "error": "A skill named deploy-runbook already exists"}',
+        )
+        assert "created" not in line
+
+    def test_missing_name_still_returns_well_formed_line(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"action": "create"},
+            0.1,
+            result='{"success": true}',
+        )
+        assert line.startswith("┊")
+        assert "created" in line
+        assert line.endswith("0.1s")
+
+    def test_blank_name_still_returns_well_formed_line(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"action": "delete", "name": "   "},
+            0.1,
+            result='{"success": true}',
+        )
+        assert line.startswith("┊")
+        assert "deleted" in line
+        assert line.endswith("0.1s")
+
+    def test_long_name_respects_configured_preview_cap(self):
+        set_tool_preview_max_len(20)
+        name = "a-very-long-skill-name-that-exceeds-the-cap"
+
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"action": "create", "name": name},
+            0.1,
+            result='{"success": true}',
+        )
+
+        assert name not in line
+        assert "..." in line
+
+    # ---- advertised call shape: {"operations": [...]} (SKILL_MANAGE_SCHEMA requires it) ----
+
+    def test_operations_array_success_names_the_skill(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"operations": [{"action": "create", "name": "deploy-runbook", "content": "..."}]},
+            0.1,
+            result='{"success": true, "message": "Skill \'deploy-runbook\' created."}',
+        )
+        assert "created" in line
+        assert "deploy-runbook" in line
+
+    def test_multi_op_batch_names_first_skill_and_marks_the_rest(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"operations": [
+                {"action": "create", "name": "first-skill", "content": "..."},
+                {"action": "patch", "name": "second-skill"},
+            ]},
+            0.1,
+            result='{"success": true}',
+        )
+        assert "first-skill" in line
+        assert "+1" in line
+
+    def test_sole_delete_batch_reports_deleted(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"operations": [{"action": "delete", "name": "gone"}]},
+            0.1,
+            result='{"success": true}',
+        )
+        assert "deleted" in line
+        assert "gone" in line
+        assert "updated" not in line
+
+    def test_failed_batch_names_intent_without_claiming_success(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"operations": [{"action": "create", "name": "dupe"}]},
+            0.1,
+            result='{"success": false, "error": "A skill named dupe already exists"}',
+        )
+        assert "created" not in line
+        assert "skill skill" not in line
+        assert "already exists" in line  # failure suffix proves it went through the real path
+
+    def test_staged_write_reports_staged_not_created(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"action": "create", "name": "staged-one"},
+            0.1,
+            result='{"success": true, "staged": true, "pending_id": "p1", "message": "Queued for approval; not yet saved."}',
+        )
+        assert "staged" in line
+        assert "created" not in line
+        assert "staged-one" in line
+
+    def test_unknown_action_verb_passes_through(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"operations": [{"action": "move", "name": "renamed-skill"}]},
+            0.1,
+            result='{"success": true}',
+        )
+        assert "move" in line
+        assert "renamed-skill" in line
+
+    def test_failed_op_without_action_falls_back_to_updated_not_skill_skill(self):
+        line = get_cute_tool_message(
+            "skill_manage",
+            {"operations": [{"name": "x"}]},
+            0.1,
+            result='{"success": false, "error": "boom"}',
+        )
+        assert "skill skill" not in line  # the round-1 bug: verb and name fallbacks collided
+        assert "updated x" in line        # neutral fallback verb + the op's name
+        assert "boom" in line             # failure marker survives
+
+
 class TestEditDiffPreview:
 
 

tests/tui_gateway/test_prompt_builtin_ack.py

@@ -0,0 +1,204 @@
+"""GUI acknowledgement for /learn, /plan, /init (#52085).
+
+The classic CLI and the messaging gateway both print an acknowledgement line
+before submitting the builder prompt as a normal turn; the TUI/desktop went
+through ``command.dispatch`` and got only ``{"type": "send", "message": ...}``
+— no ``notice`` (so no ack line) and no ``display`` (so the clients echoed the
+~5 KB model-facing prompt as the user's own bubble). These are the payload
+contracts the GUI clients render:
+
+* ``notice``  → system line (ui-tui createSlashHandler.ts, desktop slash.ts)
+* ``display`` → the chat bubble text instead of ``message``
+
+Wording is copied verbatim from the gateway acks (gateway/run_inbound.py).
+"""
+
+from __future__ import annotations
+
+import importlib
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+
+@pytest.fixture()
+def hermes_home(tmp_path, monkeypatch):
+    home = tmp_path / ".hermes"
+    home.mkdir()
+    monkeypatch.setattr(Path, "home", lambda: tmp_path)
+    monkeypatch.setenv("HERMES_HOME", str(home))
+    yield home
+
+
+@pytest.fixture()
+def server(hermes_home, monkeypatch):
+    # Mocks scoped to the initial import only (see test_protocol.py rationale).
+    with patch.dict("sys.modules", {
+        "hermes_cli.env_loader": MagicMock(),
+        "hermes_cli.banner": MagicMock(),
+    }):
+        mod = importlib.import_module("tui_gateway.server")
+    # Pin config resolution to the isolated home (sibling test files import
+    # tui_gateway.server at collection time and freeze _hermes_home to the
+    # real home — see test_goal_command.py).
+    monkeypatch.setattr(mod, "_hermes_home", hermes_home)
+    monkeypatch.setattr(mod, "_cfg_cache", None)
+    monkeypatch.setattr(mod, "_cfg_mtime", None)
+    monkeypatch.setattr(mod, "_cfg_path", None)
+    sid = "sid-prompt-builtin"
+    mod._sessions[sid] = {"session_key": sid}
+    yield mod, sid
+    mod._sessions.clear()
+    __import__("tui_gateway.server_requests", fromlist=["x"]).reset_for_tests()
+
+
+def _dispatch(server, name: str, arg: str | None = "") -> dict:
+    mod, sid = server
+    resp = mod.handle_request({
+        "id": "r1",
+        "method": "command.dispatch",
+        "params": {"name": name, "arg": arg, "session_id": sid},
+    })
+    assert "error" not in resp, resp
+    return resp["result"]
+
+
+# /learn ---------------------------------------------------------------------
+
+
+def test_learn_with_argument_acks_and_labels_the_turn(server):
+    result = _dispatch(server, "learn", "the deploy workflow")
+    assert result["type"] == "send"
+    assert result["notice"] == "Learning a skill from what you described…"
+    assert result["display"] == "/learn the deploy workflow"
+    # message stays the model-facing builder prompt, NOT what the UI shows.
+    assert "[/learn]" in result["message"]
+    assert result["message"] != result["notice"]
+    assert result["message"] != result["display"]
+
+
+def test_learn_bare_acks_this_conversation(server):
+    result = _dispatch(server, "learn")
+    assert result["type"] == "send"
+    assert result["notice"] == "Learning a skill from this conversation…"
+    assert result["display"] == "/learn"
+
+
+# /plan ----------------------------------------------------------------------
+
+
+def test_plan_with_task_acks_and_labels_the_turn(server):
+    result = _dispatch(server, "plan", "refactor the parser")
+    assert result["type"] == "send"
+    assert result["notice"] == "Planning: refactor the parser"
+    assert result["display"] == "/plan refactor the parser"
+    assert "[/plan" in result["message"]
+    assert result["message"] != result["notice"]
+
+
+def test_plan_bare_acks_conversation_context(server):
+    result = _dispatch(server, "plan")
+    assert result["notice"] == "Planning from this conversation's context…"
+    assert result["display"] == "/plan"
+
+
+def test_plan_notice_truncates_long_task(server):
+    task = "x" * 100
+    result = _dispatch(server, "plan", task)
+    assert result["notice"] == f"Planning: {'x' * 80}…"
+
+
+def test_plan_notice_collapses_a_multiline_task(server):
+    # The clients render ``notice`` as ONE system line, so a newline in the task
+    # must not reach it (nor should runs of spaces/tabs).
+    result = _dispatch(server, "plan", "fix the\nparser")
+    assert result["notice"] == "Planning: fix the parser"
+    assert "\n" not in result["notice"]
+
+
+def test_plan_truncation_applies_to_the_collapsed_task(server):
+    # 98 chars with double-space separators, 74 collapsed: over 80 raw, under 80
+    # collapsed — the ellipsis decision and the slice both use the collapsed form.
+    raw = "  ".join(["xy"] * 25)
+    collapsed = " ".join(raw.split())
+    assert len(raw) > 80 >= len(collapsed)
+    result = _dispatch(server, "plan", raw)
+    assert result["notice"] == f"Planning: {collapsed}"
+    assert "…" not in result["notice"]
+
+
+def test_plan_truncates_the_collapsed_form_when_still_too_long(server):
+    # Collapsed length still > 80, so the ellipsis stays, and the cut lands on
+    # the collapsed string — not on whitespace-padded raw text.
+    raw = "  ".join(["abcdefghij"] * 9)
+    collapsed = " ".join(raw.split())
+    assert len(collapsed) > 80
+    result = _dispatch(server, "plan", raw)
+    assert result["notice"] == f"Planning: {collapsed[:80]}…"
+
+
+# null / whitespace-only arg --------------------------------------------------
+# ``CommandDispatchParams.arg`` is ``str | None`` (contracts/tools_commands.py) and
+# ``validate_params`` defers type/required checks to handlers (contracts/registry.py), so an
+# explicit ``"arg": null`` reaches here. ``params.get("arg", "")`` does NOT default a
+# present-but-null key, so the handlers must normalise before touching the string.
+
+
+def test_learn_with_null_arg_behaves_as_bare(server):
+    result = _dispatch(server, "learn", None)
+    assert result["type"] == "send"
+    assert result["notice"] == "Learning a skill from this conversation…"
+    assert result["display"] == "/learn"
+    assert "[/learn]" in result["message"]
+
+
+def test_plan_with_null_arg_behaves_as_bare(server):
+    result = _dispatch(server, "plan", None)
+    assert result["type"] == "send"
+    assert result["notice"] == "Planning from this conversation's context…"
+    assert result["display"] == "/plan"
+
+
+def test_init_with_null_arg_behaves_as_bare(server, tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+    result = _dispatch(server, "init", None)
+    assert result["type"] == "send"
+    assert result["notice"] == "Generating AGENTS.md from a project scan…"
+    assert result["display"] == "/init"
+    assert "[/init]" in result["message"]
+
+
+def test_whitespace_only_arg_normalizes_to_the_bare_form(server):
+    result = _dispatch(server, "learn", "   ")
+    assert result["notice"] == "Learning a skill from this conversation…"
+    assert result["display"] == "/learn"
+
+
+def test_surrounding_whitespace_is_stripped_but_inner_text_kept(server):
+    multi = "\n  refactor   the parser  \n"
+    result = _dispatch(server, "plan", multi)
+    assert result["display"] == "/plan refactor   the parser"
+    # internal whitespace/newlines reach the builder verbatim, only edges are trimmed
+    assert "refactor   the parser" in result["message"]
+    assert not result["message"].startswith("\n")
+
+
+# /init ----------------------------------------------------------------------
+
+
+def test_init_notice_generates_when_no_agents_md(server, tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)  # build_init_prompt_for_cwd resolves os.getcwd()
+    result = _dispatch(server, "init")
+    assert result["type"] == "send"
+    assert result["notice"] == "Generating AGENTS.md from a project scan…"
+    assert result["display"] == "/init"
+    assert "[/init]" in result["message"]
+
+
+def test_init_notice_updates_when_agents_md_exists(server, tmp_path, monkeypatch):
+    (tmp_path / "AGENTS.md").write_text("# Existing\n\nRun `make lint`.\n", encoding="utf-8")
+    monkeypatch.chdir(tmp_path)
+    result = _dispatch(server, "init", "keep it terse")
+    assert result["notice"] == "Updating AGENTS.md from a project scan…"
+    assert result["display"] == "/init keep it terse"