Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions agent/agent_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ def init_agent(
# would mangle the escape sequences. None = use builtins.print.
agent._print_fn = None
agent.background_review_callback = None # Optional sync callback for gateway delivery
agent.memory_notifications = "on" # Memory update notifications: "off", "on", "verbose"
agent.skip_context_files = skip_context_files
agent.load_soul_identity = load_soul_identity
agent.pass_session_id = pass_session_id
Expand Down
140 changes: 121 additions & 19 deletions agent/background_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,18 +237,25 @@
def summarize_background_review_actions(
review_messages: List[Dict],
prior_snapshot: List[Dict],
notification_mode: str = "on",
) -> List[str]:
"""Build the human-facing action summary for a background review pass.

Walks the review agent's session messages and collects "successful tool
action" descriptions to surface to the user (e.g. "Memory updated").
Tool messages already present in ``prior_snapshot`` are skipped so we
don't re-surface stale results from the prior conversation that the
review agent inherited via ``conversation_history`` (issue #14944).
Walks the review agent's session messages and collects successful memory
and skill-management actions to surface to the user. Tool messages already
present in ``prior_snapshot`` are skipped so stale inherited results are
not re-surfaced as fresh background work (issue #14944).

Matching is by ``tool_call_id`` when available, with a content-equality
fallback for tool messages that lack one.
``notification_mode`` controls display detail:
- ``off``: return no actions.
- ``on``: generic "Memory updated"/tool messages.
- ``verbose``: include compact content previews from tool-call arguments.
"""
mode = str(notification_mode or "on").lower()
if mode == "off":
return []
verbose = mode == "verbose"

existing_tool_call_ids = set()
existing_tool_contents = set()
for prior in prior_snapshot or []:
Expand All @@ -262,6 +269,42 @@ def summarize_background_review_actions(
if isinstance(content, str):
existing_tool_contents.add(content)

# Map review-agent tool results back to the calls that produced them. The
# result JSON only says "Entry added"; the call arguments contain action,
# 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":
continue
for tc in msg.get("tool_calls", []) or []:
if not isinstance(tc, dict):
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 = {}
if tcid:
call_details[tcid] = {
"tool": fn_name,
"action": args.get("action", "?"),
"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] = []
for msg in review_messages or []:
if not isinstance(msg, dict) or msg.get("role") != "tool":
Expand All @@ -273,26 +316,84 @@ 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 all_tool_call_ids and tcid not in call_details:
continue
try:
data = json.loads(msg.get("content", "{}"))
except (json.JSONDecodeError, TypeError):
continue
if not isinstance(data, dict) or not data.get("success"):
continue
message = data.get("message", "")
target = data.get("target", "")
if "created" in message.lower():
actions.append(message)
elif "updated" in message.lower():
actions.append(message)
elif "added" in message.lower() or (target and "add" in message.lower()):
label = "Memory" if target == "memory" else "User profile" if target == "user" else target
actions.append(f"{label} updated")
elif "Entry added" in message:
label = "Memory" if target == "memory" else "User profile" if target == "user" else target
actions.append(f"{label} updated")
elif "removed" in message.lower() or "replaced" in message.lower():
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:
label = "Memory" if target == "memory" else "User profile" if target == "user" else target
else:
continue

if verbose:
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:
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}")
elif action == "replace" and content:
preview = content[:max_preview] + ("…" if len(content) > max_preview else "")
actions.append(f"{label} ✏️ {preview}")
elif action == "remove" and old_text:
preview = old_text[:60] + ("…" if len(old_text) > 60 else "")
actions.append(f"{label} ➖ {preview}")
else:
actions.append(f"{label} updated")
elif (
"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
):
actions.append(f"{label} updated")
return actions

Expand Down Expand Up @@ -522,6 +623,7 @@ def _bg_review_auto_deny(command, description, **kwargs):
actions = summarize_background_review_actions(
review_messages,
messages_snapshot,
notification_mode=getattr(agent, "memory_notifications", "on"),
)

if actions:
Expand Down
8 changes: 8 additions & 0 deletions gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -14697,6 +14697,14 @@ def _bg_review_send(message: str) -> None:
_pdc = getattr(_status_adapter, "_post_delivery_callbacks", None)
if _pdc is not None:
_pdc[session_key] = _release_bg_review_messages
# Memory update notifications in chat. Config: display.memory_notifications
# off — no chat notification (still logged to stdout)
# on — generic "💾 Memory updated" (default)
# verbose — content preview: "💾 Memory ➕ Hermes Repo..."
_mem_notif = user_config.get("display", {}).get("memory_notifications")
if isinstance(_mem_notif, bool):
_mem_notif = "on" if _mem_notif else "off"
agent.memory_notifications = str(_mem_notif).lower() if _mem_notif else "on"

# ------------------------------------------------------------------
# Clarify callback: present a clarify prompt and block on a response.
Expand Down
7 changes: 6 additions & 1 deletion run_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -1411,10 +1411,15 @@ def _cleanup_task_resources(self, task_id: str) -> None:
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,
Expand Down
4 changes: 3 additions & 1 deletion tests/run_agent/test_background_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,11 @@ def close(self):
# 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)
Expand Down Expand Up @@ -146,6 +147,7 @@ def fake_summarize(review_messages, prior_snapshot):
]
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):
Expand Down
32 changes: 30 additions & 2 deletions tools/skill_manager_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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},
}


Expand Down Expand Up @@ -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]:
Expand Down
Loading