From 4cf9d80fba1eddd0a187e6f29d4531c8f2dc1610 Mon Sep 17 00:00:00 2001 From: Wolfram Ravenwolf Date: Fri, 10 Apr 2026 10:32:32 +0200 Subject: [PATCH] feat(display): verbose skill change notifications with content previews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When display.memory_notifications is set to 'verbose', skill_manage notifications now show meaningful change details instead of just the generic tool message. Before (verbose mode): πŸ’Ύ πŸ“ Patched SKILL.md in skill 'gogcli' (1 replacement). After (verbose mode): πŸ’Ύ πŸ“ Skill 'gogcli' patched: "old pitfall text..." β†’ "new pitfall text..." Changes: - skill_manager_tool.py: _patch_skill() now includes old/new string previews (truncated to 200 chars) in the result via '_change' key. _create_skill() and _edit_skill() include skill description from frontmatter for verbose create/edit notifications. - run_agent.py: Background review notification builder now reads the '_change' dict from skill tool results and formats descriptive notifications per action type (patch β†’ oldβ†’new diff, create/edit β†’ description preview). Falls back to generic message when _change data is unavailable (backwards compatible). This is especially useful when subagents patch skills, since neither the user nor the parent agent can see what the subagent changed. --- agent/background_review.py | 57 ++++++++++++++++++----- run_agent.py | 7 ++- tests/run_agent/test_background_review.py | 4 +- tools/skill_manager_tool.py | 32 ++++++++++++- 4 files changed, 85 insertions(+), 15 deletions(-) diff --git a/agent/background_review.py b/agent/background_review.py index 5d3fc2bf5e8..2c9703ba68b 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -274,6 +274,7 @@ def summarize_background_review_actions( # target, and content previews. Restricting to notify_tools also prevents # helper tools from surfacing as memory work just because they succeeded. notify_tools = {"memory", "skill_manage"} + all_tool_call_ids: set = set() call_details: dict = {} for msg in review_messages or []: if not isinstance(msg, dict) or msg.get("role") != "assistant": @@ -283,13 +284,15 @@ def summarize_background_review_actions( continue fn = tc.get("function", {}) or {} fn_name = fn.get("name", "") + tcid = tc.get("id") + if tcid: + all_tool_call_ids.add(tcid) if fn_name not in notify_tools: continue try: args = json.loads(fn.get("arguments", "{}")) except (json.JSONDecodeError, TypeError): args = {} - tcid = tc.get("id") if tcid: call_details[tcid] = { "tool": fn_name, @@ -297,6 +300,9 @@ def summarize_background_review_actions( "target": args.get("target", "memory"), "content": args.get("content", ""), "old_text": args.get("old_text", ""), + "name": args.get("name", ""), + "old_string": args.get("old_string", ""), + "new_string": args.get("new_string", ""), } actions: List[str] = [] @@ -310,7 +316,7 @@ def summarize_background_review_actions( content_str = msg.get("content") if isinstance(content_str, str) and content_str in existing_tool_contents: continue - if tcid and call_details and tcid not in call_details: + if tcid and all_tool_call_ids and tcid not in call_details: continue try: data = json.loads(msg.get("content", "{}")) @@ -319,10 +325,22 @@ def summarize_background_review_actions( if not isinstance(data, dict) or not data.get("success"): continue message = data.get("message", "") - target = data.get("target", "") detail = call_details.get(tcid, {}) + target = data.get("target", "") or detail.get("target", "") is_skill = detail.get("tool") == "skill_manage" + message_lower = message.lower() + if not verbose: + if "created" in message_lower: + actions.append(message) + continue + if "updated" in message_lower: + actions.append(message) + continue + if is_skill and "patched" in message_lower: + actions.append(message) + continue + if is_skill: label = "Skill" elif target: @@ -334,9 +352,30 @@ def summarize_background_review_actions( action = detail.get("action", "") content = detail.get("content", "") old_text = detail.get("old_text", "") + skill_name = detail.get("name", "") max_preview = 120 if is_skill: - actions.append(f"πŸ“ {message}" if message else f"Skill {action}") + change = data.get("_change", {}) + old_string = change.get("old", "") or detail.get("old_string", "") + new_string = change.get("new", "") or detail.get("new_string", "") + description = change.get("description", "") + if action == "patch" and (old_string or new_string): + old_preview = old_string[:80].replace("\n", " ") + ( + "…" if len(old_string) > 80 else "" + ) + new_preview = new_string[:80].replace("\n", " ") + ( + "…" if len(new_string) > 80 else "" + ) + actions.append( + f"πŸ“ Skill '{skill_name}' patched: " + f"\"{old_preview}\" β†’ \"{new_preview}\"" + ) + elif action == "create" and description: + actions.append(f"πŸ“ Skill '{skill_name}' created: {description}") + elif action == "edit" and description: + actions.append(f"πŸ“ Skill '{skill_name}' rewritten: {description}") + else: + actions.append(f"πŸ“ {message}" if message else f"Skill {action}") elif action == "add" and content: preview = content[:max_preview] + ("…" if len(content) > max_preview else "") actions.append(f"{label} βž• {preview}") @@ -348,14 +387,10 @@ def summarize_background_review_actions( actions.append(f"{label} βž– {preview}") else: actions.append(f"{label} updated") - elif "created" in message.lower(): - actions.append(message) - elif "updated" in message.lower(): - actions.append(message) elif ( - "added" in message.lower() - or "replaced" in message.lower() - or "removed" in message.lower() + "added" in message_lower + or "replaced" in message_lower + or "removed" in message_lower or (target and "add" in message.lower()) or "Entry added" in message ): diff --git a/run_agent.py b/run_agent.py index a97f6c9c0aa..94d3be3e674 100644 --- a/run_agent.py +++ b/run_agent.py @@ -1411,10 +1411,15 @@ class AIAgent: def _summarize_background_review_actions( review_messages: List[Dict], prior_snapshot: List[Dict], + notification_mode: str = "on", ) -> List[str]: """Forwarder β€” see ``agent.background_review.summarize_background_review_actions``.""" from agent.background_review import summarize_background_review_actions - return summarize_background_review_actions(review_messages, prior_snapshot) + return summarize_background_review_actions( + review_messages, + prior_snapshot, + notification_mode=notification_mode, + ) def _spawn_background_review( self, diff --git a/tests/run_agent/test_background_review.py b/tests/run_agent/test_background_review.py index f4b0faff7f5..b512497c1cf 100644 --- a/tests/run_agent/test_background_review.py +++ b/tests/run_agent/test_background_review.py @@ -115,10 +115,11 @@ def test_background_review_summarizer_receives_captured_messages_after_close(mon # must have snapshot them before this runs. self._session_messages = [] - def fake_summarize(review_messages, prior_snapshot): + def fake_summarize(review_messages, prior_snapshot, notification_mode="on"): events.append("summarize") captured["review_messages"] = list(review_messages) captured["prior_snapshot"] = list(prior_snapshot) + captured["notification_mode"] = notification_mode return [] monkeypatch.setattr(run_agent_module, "AIAgent", FakeReviewAgent) @@ -146,6 +147,7 @@ def test_background_review_summarizer_receives_captured_messages_after_close(mon ] assert captured["review_messages"] == [review_tool_message] assert captured["prior_snapshot"] == messages_snapshot + assert captured["notification_mode"] == "on" def test_background_review_installs_auto_deny_approval_callback(monkeypatch): diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index 020c3a0155a..e3f48b2b6ea 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -598,11 +598,22 @@ def _create_skill(name: str, content: str, category: str = None) -> Dict[str, An shutil.rmtree(skill_dir, ignore_errors=True) return {"success": False, "error": scan_error} + # Extract description from frontmatter for verbose notifications + _desc = "" + try: + _fm_end = re.search(r'\n---\s*\n', content[3:]) + if _fm_end: + _parsed = yaml.safe_load(content[3:_fm_end.start() + 3]) + _desc = str(_parsed.get("description", ""))[:120] + except Exception: + pass + result = { "success": True, "message": f"Skill '{name}' created.", "path": str(skill_dir.relative_to(SKILLS_DIR)), "skill_md": str(skill_md), + "_change": {"description": _desc}, } if category: result["category"] = category @@ -639,10 +650,21 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]: _atomic_write_text(skill_md, original_content) return {"success": False, "error": scan_error} + # Extract description from new content for verbose notifications + _desc = "" + try: + _fm_end = re.search(r'\n---\s*\n', content[3:]) + if _fm_end: + _parsed = yaml.safe_load(content[3:_fm_end.start() + 3]) + _desc = str(_parsed.get("description", ""))[:120] + except Exception: + pass + return { "success": True, - "message": f"Skill '{name}' updated.", + "message": f"Skill '{name}' updated (full rewrite).", "path": str(existing["path"]), + "_change": {"description": _desc}, } @@ -734,10 +756,16 @@ def _patch_skill( _atomic_write_text(target, original_content) return {"success": False, "error": scan_error} - return { + result = { "success": True, "message": f"Patched {'SKILL.md' if not file_path else file_path} in skill '{name}' ({match_count} replacement{'s' if match_count > 1 else ''}).", } + # Include change previews for verbose notifications + result["_change"] = { + "old": old_string[:200] + ("…" if len(old_string) > 200 else ""), + "new": new_string[:200] + ("…" if len(new_string) > 200 else ""), + } + return result def _delete_skill(name: str, absorbed_into: Optional[str] = None) -> Dict[str, Any]: