feat(skills): quality gate prompt + wire up dead bump_use counter

agent/outcome_tracker.py

@@ -0,0 +1,51 @@
+"""Wire up the dead bump_use counter by scanning session messages for skill_view calls.
+
+Called once at session end from AIAgent.shutdown_memory_provider. Best-effort:
+failures are logged at DEBUG and never propagate to the caller.
+"""
+from __future__ import annotations
+
+import json
+import logging
+from typing import Any
+
+logger = logging.getLogger(__name__)
+
+
+def record_session_outcome(messages: list[dict[str, Any]]) -> None:
+    """Increment use_count for every skill viewed during the session."""
+    skill_names = _extract_skill_views(messages)
+    if not skill_names:
+        return
+    try:
+        from tools.skill_usage import bump_use
+        for name in skill_names:
+            bump_use(name)
+    except Exception as exc:
+        logger.debug("outcome_tracker: bump_use failed: %s", exc)
+
+
+def _extract_skill_views(messages: list[dict[str, Any]]) -> set[str]:
+    """Return the set of skill names loaded via skill_view in this session.
+
+    Parses OpenAI-format assistant messages (role='assistant', tool_calls=[...]).
+    Malformed entries are silently skipped.
+    """
+    seen: set[str] = set()
+    for msg in messages:
+        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
+            func = tc.get("function") or {}
+            if func.get("name") != "skill_view":
+                continue
+            try:
+                args = json.loads(func.get("arguments", "{}"))
+                name = args.get("name") or args.get("skill_name")
+                if name:
+                    seen.add(str(name))
+            except Exception:
+                pass
+    return seen

run_agent.py

@@ -1755,9 +1755,11 @@ def __init__(
 
         # Skills config: nudge interval for skill creation reminders
         self._skill_nudge_interval = 10
+        self._skill_review_extra = ""
         try:
             skills_config = _agent_cfg.get("skills", {})
             self._skill_nudge_interval = int(skills_config.get("creation_nudge_interval", 10))
+            self._skill_review_extra = str(skills_config.get("extra_review_prompt", ""))
         except Exception:
             pass
 
@@ -3470,6 +3472,8 @@ def _spawn_background_review(
             prompt = self._MEMORY_REVIEW_PROMPT
         else:
             prompt = self._SKILL_REVIEW_PROMPT
+        if review_skills and self._skill_review_extra:
+            prompt = prompt + "\n\n" + self._skill_review_extra
 
         def _run_review():
             import contextlib
@@ -4519,7 +4523,12 @@ def shutdown_memory_provider(self, messages: list = None) -> None:
                 )
             except Exception:
                 pass
-
+        try:
+            from agent.outcome_tracker import record_session_outcome
+            record_session_outcome(messages or [])
+        except Exception:
+            pass
+
     def commit_memory_session(self, messages: list = None) -> None:
         """Trigger end-of-session extraction without tearing providers down.
         Called when session_id rotates (e.g. /new, context compression);

tests/agent/test_outcome_tracker.py

@@ -0,0 +1,132 @@
+"""Tests for agent/outcome_tracker.py"""
+from __future__ import annotations
+
+import json
+from unittest.mock import patch
+
+
+def _skill_view_msg(skill_name: str) -> dict:
+    return {
+        "role": "assistant",
+        "content": None,
+        "tool_calls": [
+            {
+                "id": "call_test",
+                "type": "function",
+                "function": {
+                    "name": "skill_view",
+                    "arguments": json.dumps({"name": skill_name}),
+                },
+            }
+        ],
+    }
+
+
+def _other_tool_msg(tool_name: str) -> dict:
+    return {
+        "role": "assistant",
+        "content": None,
+        "tool_calls": [
+            {"function": {"name": tool_name, "arguments": json.dumps({"path": "/tmp/x"})}}
+        ],
+    }
+
+
+class TestExtractSkillViews:
+    def test_extracts_single_skill_name(self):
+        from agent.outcome_tracker import _extract_skill_views
+        assert _extract_skill_views([_skill_view_msg("coding-skill")]) == {"coding-skill"}
+
+    def test_extracts_multiple_unique_skills(self):
+        from agent.outcome_tracker import _extract_skill_views
+        msgs = [_skill_view_msg("skill-a"), _skill_view_msg("skill-b")]
+        assert _extract_skill_views(msgs) == {"skill-a", "skill-b"}
+
+    def test_deduplicates_repeated_views(self):
+        from agent.outcome_tracker import _extract_skill_views
+        msgs = [_skill_view_msg("skill-a"), _skill_view_msg("skill-a")]
+        assert _extract_skill_views(msgs) == {"skill-a"}
+
+    def test_ignores_other_tool_calls(self):
+        from agent.outcome_tracker import _extract_skill_views
+        msgs = [_other_tool_msg("read_file"), _other_tool_msg("bash")]
+        assert _extract_skill_views(msgs) == set()
+
+    def test_ignores_non_assistant_roles(self):
+        from agent.outcome_tracker import _extract_skill_views
+        msgs = [
+            {"role": "user", "content": "use the coding skill"},
+            {"role": "tool", "content": "skill content", "tool_call_id": "call_1"},
+        ]
+        assert _extract_skill_views(msgs) == set()
+
+    def test_handles_empty_messages(self):
+        from agent.outcome_tracker import _extract_skill_views
+        assert _extract_skill_views([]) == set()
+
+    def test_handles_malformed_arguments_gracefully(self):
+        from agent.outcome_tracker import _extract_skill_views
+        msgs = [
+            {
+                "role": "assistant",
+                "tool_calls": [
+                    {"function": {"name": "skill_view", "arguments": "not-valid-json"}}
+                ],
+            }
+        ]
+        assert _extract_skill_views(msgs) == set()
+
+    def test_handles_missing_tool_calls_key(self):
+        from agent.outcome_tracker import _extract_skill_views
+        assert _extract_skill_views([{"role": "assistant", "content": "hello"}]) == set()
+
+
+class TestRecordSessionOutcome:
+    def test_calls_bump_use_for_each_skill(self):
+        from agent.outcome_tracker import record_session_outcome
+        bumped: list[str] = []
+        with patch("tools.skill_usage.bump_use", side_effect=lambda n: bumped.append(n)):
+            record_session_outcome([_skill_view_msg("skill-a"), _skill_view_msg("skill-b")])
+        assert set(bumped) == {"skill-a", "skill-b"}
+
+    def test_no_op_on_empty_messages(self):
+        from agent.outcome_tracker import record_session_outcome
+        with patch("tools.skill_usage.bump_use") as mock_bump:
+            record_session_outcome([])
+        mock_bump.assert_not_called()
+
+    def test_no_op_when_no_skill_views(self):
+        from agent.outcome_tracker import record_session_outcome
+        with patch("tools.skill_usage.bump_use") as mock_bump:
+            record_session_outcome([_other_tool_msg("read_file")])
+        mock_bump.assert_not_called()
+
+    def test_bump_use_failure_does_not_raise(self):
+        from agent.outcome_tracker import record_session_outcome
+        with patch("tools.skill_usage.bump_use", side_effect=RuntimeError("disk full")):
+            record_session_outcome([_skill_view_msg("my-skill")])  # must not raise
+
+
+class TestShutdownHook:
+    def test_shutdown_memory_provider_calls_record_session_outcome(self):
+        """shutdown_memory_provider calls record_session_outcome after our edit.
+
+        NOTE: This test is EXPECTED TO FAIL in Task 1. The hook in run_agent.py
+        will be added in Task 2. Write the test now so Task 2 can verify it passes.
+        """
+        import run_agent as ra
+
+        recorded: list[list] = []
+
+        class FakeAgent:
+            _memory_manager = None
+            session_id = "test"
+            shutdown_memory_provider = ra.AIAgent.shutdown_memory_provider
+
+        messages = [_skill_view_msg("test-skill")]
+        with patch("agent.outcome_tracker.record_session_outcome",
+                   side_effect=lambda m: recorded.append(m)):
+            FakeAgent().shutdown_memory_provider(messages)
+
+        assert len(recorded) == 1
+        assert recorded[0] == messages

tests/agent/test_skill_review_extra.py

@@ -0,0 +1,116 @@
+"""Tests for the configurable skill review quality gate in AIAgent."""
+from __future__ import annotations
+
+from unittest.mock import MagicMock, patch
+
+
+def _fake_routed_client():
+    """Return a minimal fake provider client to satisfy AIAgent.__init__."""
+    client = MagicMock()
+    client.api_key = "sk-fake-test-key"
+    client.base_url = "https://api.openai.com/v1/"
+    client._default_headers = {}
+    return client
+
+
+def _make_agent(extra_prompt: str = ""):
+    """Construct a minimal AIAgent with mocked provider and config."""
+    import run_agent as ra
+
+    cfg = {"skills": {"extra_review_prompt": extra_prompt}} if extra_prompt else {}
+
+    with (
+        patch("hermes_cli.config.load_config", return_value=cfg),
+        patch(
+            "agent.auxiliary_client.resolve_provider_client",
+            return_value=(_fake_routed_client(), "openai"),
+        ),
+    ):
+        return ra.AIAgent(
+            model="claude-haiku-4-5-20251001",
+            quiet_mode=True,
+            skip_context_files=True,
+            max_iterations=1,
+        )
+
+
+class TestSkillReviewExtra:
+    def test_defaults_to_empty_string_when_not_configured(self):
+        agent = _make_agent()
+        assert agent._skill_review_extra == ""
+
+    def test_loaded_from_config(self):
+        agent = _make_agent("verify: is this generalizable?")
+        assert agent._skill_review_extra == "verify: is this generalizable?"
+
+    def test_extra_appended_to_skill_only_review(self):
+        """When review_skills=True and review_memory=False, extra is appended."""
+        import run_agent as ra
+
+        agent = _make_agent("my-quality-gate")
+        captured: list[str] = []
+
+        fake_review = MagicMock()
+        fake_review.run_conversation = lambda user_message, **kw: captured.append(user_message)
+
+        with patch(
+            "threading.Thread",
+            side_effect=lambda *a, target, **kw: type("T", (), {"start": lambda s: target()})(),
+        ):
+            with patch.object(ra, "AIAgent", return_value=fake_review):
+                agent._spawn_background_review(
+                    messages_snapshot=[],
+                    review_memory=False,
+                    review_skills=True,
+                )
+
+        assert len(captured) == 1
+        assert "my-quality-gate" in captured[0]
+
+    def test_extra_NOT_appended_to_memory_only_review(self):
+        """When review_skills=False, extra prompt must not appear."""
+        import run_agent as ra
+
+        agent = _make_agent("my-quality-gate")
+        captured: list[str] = []
+
+        fake_review = MagicMock()
+        fake_review.run_conversation = lambda user_message, **kw: captured.append(user_message)
+
+        with patch(
+            "threading.Thread",
+            side_effect=lambda *a, target, **kw: type("T", (), {"start": lambda s: target()})(),
+        ):
+            with patch.object(ra, "AIAgent", return_value=fake_review):
+                agent._spawn_background_review(
+                    messages_snapshot=[],
+                    review_memory=True,
+                    review_skills=False,
+                )
+
+        assert len(captured) == 1
+        assert "my-quality-gate" not in captured[0]
+
+    def test_no_extra_when_config_is_empty(self):
+        """When extra_review_prompt is empty, prompt is unchanged."""
+        import run_agent as ra
+
+        agent = _make_agent()
+        captured: list[str] = []
+
+        fake_review = MagicMock()
+        fake_review.run_conversation = lambda user_message, **kw: captured.append(user_message)
+
+        with patch(
+            "threading.Thread",
+            side_effect=lambda *a, target, **kw: type("T", (), {"start": lambda s: target()})(),
+        ):
+            with patch.object(ra, "AIAgent", return_value=fake_review):
+                agent._spawn_background_review(
+                    messages_snapshot=[],
+                    review_memory=False,
+                    review_skills=True,
+                )
+
+        assert len(captured) == 1
+        assert captured[0] == agent._SKILL_REVIEW_PROMPT