chore: uptick

tests/agent/test_auxiliary_named_custom_providers.py

@@ -427,3 +427,68 @@ class TestProvidersDictApiModeAnthropicMessages:
         assert isinstance(sync_client, OpenAI)
         async_client, _ = resolve_provider_client("localchat", async_mode=True)
         assert isinstance(async_client, AsyncOpenAI)
+
+
+class TestCustomProviderAliasCollision:
+    """A user-declared custom_providers entry whose name matches a built-in
+    *alias* (not a canonical provider) must win over the built-in.
+
+    Regression guard for #15743: users who defined fallback_model pointing at
+    a custom_providers entry named ``kimi`` were having requests routed to
+    the built-in kimi-coding endpoint because ``_normalize_aux_provider``
+    rewrote ``kimi`` → ``kimi-coding`` before the named-custom lookup.
+    """
+
+    def test_custom_named_kimi_wins_over_builtin_alias(self, tmp_path):
+        _write_config(tmp_path, {
+            "model": {"provider": "openrouter", "default": "anthropic/claude-sonnet-4.6"},
+            "custom_providers": [
+                {
+                    "name": "kimi",
+                    "base_url": "https://my-custom-kimi.example.com/v1",
+                    "api_key": "my-kimi-key",
+                    "models": {"my-kimi-model": {"context_length": 200000}},
+                },
+            ],
+        })
+        from agent.auxiliary_client import resolve_provider_client
+        from openai import OpenAI
+        client, model = resolve_provider_client("kimi", model="my-kimi-model", raw_codex=True)
+        assert isinstance(client, OpenAI)
+        assert "my-custom-kimi.example.com" in str(client.base_url)
+        assert client.api_key == "my-kimi-key"
+        assert model == "my-kimi-model"
+
+    def test_bare_kimi_without_custom_still_routes_to_builtin(self, tmp_path, monkeypatch):
+        """Regression guard: bare 'kimi' with no custom entry must still
+        reach the built-in kimi-coding provider."""
+        _write_config(tmp_path, {
+            "model": {"provider": "openrouter", "default": "anthropic/claude-sonnet-4.6"},
+        })
+        monkeypatch.setenv("KIMI_API_KEY", "builtin-kimi-key")
+        from agent.auxiliary_client import resolve_provider_client
+        client, _ = resolve_provider_client("kimi", model="kimi-k2-0905-preview", raw_codex=True)
+        assert client is not None
+        base_url = str(client.base_url)
+        # Built-in kimi-coding points at api.moonshot.ai
+        assert "moonshot" in base_url or "kimi" in base_url, f"unexpected base_url {base_url!r}"
+
+    def test_explicit_overrides_applied_on_api_key_branch(self, tmp_path, monkeypatch):
+        """Explicit base_url/api_key from the caller must override the
+        registered provider's defaults on the API-key branch.  Used by
+        _try_activate_fallback to route a fallback through a built-in
+        provider name but targeting a user-supplied endpoint."""
+        _write_config(tmp_path, {
+            "model": {"provider": "openrouter", "default": "anthropic/claude-sonnet-4.6"},
+        })
+        monkeypatch.setenv("KIMI_API_KEY", "builtin-kimi-key")
+        from agent.auxiliary_client import resolve_provider_client
+        from openai import OpenAI
+        client, _ = resolve_provider_client(
+            "kimi-coding", model="kimi-k2", raw_codex=True,
+            explicit_base_url="https://override.example.com",
+            explicit_api_key="override-key",
+        )
+        assert isinstance(client, OpenAI)
+        assert "override.example.com" in str(client.base_url)
+        assert client.api_key == "override-key"

tests/agent/test_context_compressor.py

@@ -640,6 +640,30 @@ class TestCompressWithClient:
                 for tc in msg["tool_calls"]:
                     assert tc["id"] in answered_ids
 
+    def test_sanitizer_matches_responses_call_id_when_id_differs(self, compressor):
+        msgs = [
+            {
+                "role": "assistant",
+                "content": "",
+                "tool_calls": [
+                    {
+                        "id": "fc_123",
+                        "call_id": "call_123",
+                        "response_item_id": "fc_123",
+                        "type": "function",
+                        "function": {"name": "search_files", "arguments": "{}"},
+                    }
+                ],
+            },
+            {"role": "tool", "tool_call_id": "call_123", "content": "result"},
+        ]
+
+        sanitized = compressor._sanitize_tool_pairs(msgs)
+
+        assert [m.get("tool_call_id") for m in sanitized if m.get("role") == "tool"] == [
+            "call_123"
+        ]
+
     def test_summary_role_avoids_consecutive_user_messages(self):
         """Summary role should alternate with the last head message to avoid consecutive same-role messages."""
         mock_client = MagicMock()
@@ -1119,6 +1143,34 @@ class TestTokenBudgetTailProtection:
         # At least one old tool result should have been pruned
         assert pruned >= 1
 
+    def test_prune_short_conv_protects_entire_tail(self, budget_compressor):
+        """Regression guard for PR #17025.
+
+        When ``len(messages) <= protect_tail_count`` and a token budget is
+        also set, every message must be protected. The previous code used
+        ``min(protect_tail_count, len(result) - 1)`` which capped the floor
+        one below the full length, leaving the oldest message eligible for
+        pruning.
+        """
+        c = budget_compressor
+        # 4 messages, protect_tail_count=4 -- nothing should be pruned.
+        # Oldest message is a large tool result; on the buggy path it falls
+        # outside the protected window and gets summarized.
+        messages = [
+            {"role": "tool", "content": "x" * 5000, "tool_call_id": "c0"},
+            {"role": "assistant", "content": "ack"},
+            {"role": "user", "content": "recent"},
+            {"role": "assistant", "content": "reply"},
+        ]
+        result, pruned = c._prune_old_tool_results(
+            messages,
+            protect_tail_count=4,
+            protect_tail_tokens=1_000_000,  # budget large enough to protect all
+        )
+        assert pruned == 0
+        # Tool result at index 0 must be preserved verbatim
+        assert result[0]["content"] == "x" * 5000
+
     def test_prune_without_token_budget_uses_message_count(self, budget_compressor):
         """Without protect_tail_tokens, falls back to message-count behavior."""
         c = budget_compressor

tests/agent/test_curator.py

@@ -86,9 +86,22 @@ def test_curator_config_overrides(curator_env, monkeypatch):
 # should_run_now
 # ---------------------------------------------------------------------------
 
-def test_first_run_always_eligible(curator_env):
+def test_first_run_defers(curator_env):
+    """The FIRST observation of the curator (fresh install, no state file)
+    must NOT trigger an immediate run. The curator is designed to run after
+    a full ``interval_hours`` of skill activity, not on the first background
+    tick after installation. Fixes #18373.
+    """
     c = curator_env["curator"]
-    assert c.should_run_now() is True
+    # No state file — should defer and seed last_run_at.
+    assert c.should_run_now() is False
+    state = c.load_state()
+    assert state.get("last_run_at") is not None, (
+        "first observation should seed last_run_at so the interval clock "
+        "starts ticking instead of firing immediately next tick"
+    )
+    # A second immediate call still returns False (seeded, not yet stale).
+    assert c.should_run_now() is False
 
 
 def test_recent_run_blocks(curator_env):
@@ -265,6 +278,77 @@ def test_run_review_records_state(curator_env):
     assert state["last_run_summary"] is not None
 
 
+def test_dry_run_does_not_advance_state(curator_env, monkeypatch):
+    """Dry-run previews must not bump last_run_at or run_count. A preview
+    shouldn't defer the next scheduled real pass or look like a real run in
+    `hermes curator status`. Fixes #18373.
+    """
+    c = curator_env["curator"]
+    skills_dir = curator_env["home"] / "skills"
+    _write_skill(skills_dir, "a")
+
+    # Stub the LLM so the test doesn't need a provider.
+    monkeypatch.setattr(
+        c, "_run_llm_review",
+        lambda prompt: {
+            "final": "", "summary": "dry preview", "model": "", "provider": "",
+            "tool_calls": [], "error": None,
+        },
+    )
+
+    c.run_curator_review(synchronous=True, dry_run=True)
+    state = c.load_state()
+    assert state.get("last_run_at") is None, "dry-run must not seed last_run_at"
+    assert state.get("run_count", 0) == 0, "dry-run must not bump run_count"
+    assert "dry-run" in (state.get("last_run_summary") or ""), (
+        "dry-run summary should be labeled so status output is unambiguous"
+    )
+
+
+def test_dry_run_injects_report_only_banner(curator_env, monkeypatch):
+    """The dry-run prompt must carry a banner instructing the LLM not to
+    call any mutating tool. This is defense in depth — the caller also
+    skips automatic transitions — but the LLM prompt is the only guard
+    against the model calling skill_manage directly."""
+    c = curator_env["curator"]
+    skills_dir = curator_env["home"] / "skills"
+    _write_skill(skills_dir, "a")
+
+    captured = {}
+    def _stub(prompt):
+        captured["prompt"] = prompt
+        return {"final": "", "summary": "s", "model": "", "provider": "",
+                "tool_calls": [], "error": None}
+    monkeypatch.setattr(c, "_run_llm_review", _stub)
+
+    c.run_curator_review(synchronous=True, dry_run=True)
+    assert "DRY-RUN" in captured["prompt"]
+    assert "DO NOT" in captured["prompt"]
+
+
+def test_dry_run_skips_automatic_transitions(curator_env, monkeypatch):
+    """Dry-run must not call apply_automatic_transitions — the auto pass
+    archives skills deterministically, and a preview must not touch the
+    filesystem."""
+    c = curator_env["curator"]
+    skills_dir = curator_env["home"] / "skills"
+    _write_skill(skills_dir, "a")
+
+    called = {"n": 0}
+    def _explode(*_a, **_kw):
+        called["n"] += 1
+        return {"checked": 0, "marked_stale": 0, "archived": 0, "reactivated": 0}
+    monkeypatch.setattr(c, "apply_automatic_transitions", _explode)
+    monkeypatch.setattr(
+        c, "_run_llm_review",
+        lambda p: {"final": "", "summary": "s", "model": "", "provider": "",
+                   "tool_calls": [], "error": None},
+    )
+
+    c.run_curator_review(synchronous=True, dry_run=True)
+    assert called["n"] == 0, "dry-run must skip apply_automatic_transitions"
+
+
 def test_run_review_synchronous_invokes_llm_stub(curator_env, monkeypatch):
     c = curator_env["curator"]
     skills_dir = curator_env["home"] / "skills"
@@ -327,12 +411,32 @@ def test_maybe_run_curator_runs_when_eligible(curator_env, monkeypatch):
     c = curator_env["curator"]
     skills_dir = curator_env["home"] / "skills"
     _write_skill(skills_dir, "a")
+    # Seed last_run_at far in the past so the interval gate opens — the
+    # "no state" path intentionally defers the first run now (#18373).
+    long_ago = datetime.now(timezone.utc) - timedelta(hours=c.get_interval_hours() * 2)
+    c.save_state({"last_run_at": long_ago.isoformat(), "paused": False})
     # Force idle over threshold
     result = c.maybe_run_curator(idle_for_seconds=99999.0)
     assert result is not None
     assert "started_at" in result
 
 
+def test_maybe_run_curator_defers_on_fresh_install(curator_env):
+    """Fresh install (no curator state file) must NOT fire the curator on
+    the first gateway tick. The first observation seeds last_run_at and
+    returns None. Fixes #18373."""
+    c = curator_env["curator"]
+    skills_dir = curator_env["home"] / "skills"
+    _write_skill(skills_dir, "a")
+    # Infinite idle — the only thing that should block the run is the new
+    # deferred-first-run gate.
+    result = c.maybe_run_curator(idle_for_seconds=99999.0)
+    assert result is None
+    # And the next tick still defers (we seeded last_run_at to "now").
+    result2 = c.maybe_run_curator(idle_for_seconds=99999.0)
+    assert result2 is None
+
+
 def test_maybe_run_curator_swallows_exceptions(curator_env, monkeypatch):
     c = curator_env["curator"]
 
@@ -363,6 +467,19 @@ def test_state_atomic_write_no_tmp_leftovers(curator_env):
         assert not p.name.startswith(".curator_state_"), f"tmp leftover: {p.name}"
 
 
+def test_state_preserves_last_report_path(curator_env):
+    c = curator_env["curator"]
+    c.save_state({
+        "last_run_at": "2026-04-30T12:00:00+00:00",
+        "last_run_summary": "ok",
+        "last_report_path": "/tmp/curator-report",
+        "paused": False,
+        "run_count": 1,
+    })
+    state = c.load_state()
+    assert state["last_report_path"] == "/tmp/curator-report"
+
+
 def test_curator_review_prompt_has_invariants():
     """Core invariants must be in the review prompt text."""
     from agent.curator import CURATOR_REVIEW_PROMPT

tests/agent/test_curator_backup.py

@@ -0,0 +1,316 @@
+"""Tests for agent/curator_backup.py — snapshot + rollback of the skills tree."""
+
+from __future__ import annotations
+
+import importlib
+import json
+import os
+import sys
+import tarfile
+import tempfile
+from pathlib import Path
+
+import pytest
+
+
+@pytest.fixture
+def backup_env(monkeypatch, tmp_path):
+    """Isolate HERMES_HOME + reload modules so every test starts clean."""
+    home = tmp_path / ".hermes"
+    home.mkdir()
+    (home / "skills").mkdir()
+    monkeypatch.setenv("HERMES_HOME", str(home))
+    monkeypatch.setattr(Path, "home", lambda: tmp_path)
+
+    # Reload so get_hermes_home picks up the env var fresh.
+    import hermes_constants
+    importlib.reload(hermes_constants)
+    from agent import curator_backup
+    importlib.reload(curator_backup)
+    return {"home": home, "skills": home / "skills", "cb": curator_backup}
+
+
+def _write_skill(skills_dir: Path, name: str, body: str = "body") -> Path:
+    d = skills_dir / name
+    d.mkdir(parents=True, exist_ok=True)
+    (d / "SKILL.md").write_text(
+        f"---\nname: {name}\ndescription: t\nversion: 1.0\n---\n\n{body}\n",
+        encoding="utf-8",
+    )
+    return d
+
+
+# ---------------------------------------------------------------------------
+# snapshot_skills
+# ---------------------------------------------------------------------------
+
+def test_snapshot_creates_tarball_and_manifest(backup_env):
+    cb = backup_env["cb"]
+    _write_skill(backup_env["skills"], "alpha")
+    _write_skill(backup_env["skills"], "beta")
+
+    snap = cb.snapshot_skills(reason="test")
+    assert snap is not None, "snapshot should succeed with a populated skills dir"
+    assert (snap / "skills.tar.gz").exists()
+    manifest = json.loads((snap / "manifest.json").read_text())
+    assert manifest["reason"] == "test"
+    assert manifest["skill_files"] == 2
+    assert manifest["archive_bytes"] > 0
+
+
+def test_snapshot_excludes_backups_dir_itself(backup_env):
+    """The backup must NOT contain .curator_backups/ — that would recurse
+    with every subsequent snapshot and balloon disk usage."""
+    cb = backup_env["cb"]
+    _write_skill(backup_env["skills"], "alpha")
+    snap1 = cb.snapshot_skills(reason="first")
+    assert snap1 is not None
+    snap2 = cb.snapshot_skills(reason="second")
+    assert snap2 is not None
+    with tarfile.open(snap2 / "skills.tar.gz") as tf:
+        names = tf.getnames()
+    assert not any(n.startswith(".curator_backups") for n in names), (
+        "second snapshot must not contain the first snapshot recursively"
+    )
+
+
+def test_snapshot_excludes_hub_dir(backup_env):
+    """.hub/ is managed by the skills hub. Rolling it back would break
+    lockfile invariants, so the snapshot omits it entirely."""
+    cb = backup_env["cb"]
+    hub = backup_env["skills"] / ".hub"
+    hub.mkdir()
+    (hub / "lock.json").write_text("{}")
+    _write_skill(backup_env["skills"], "alpha")
+    snap = cb.snapshot_skills(reason="t")
+    assert snap is not None
+    with tarfile.open(snap / "skills.tar.gz") as tf:
+        names = tf.getnames()
+    assert not any(n.startswith(".hub") for n in names)
+
+
+def test_snapshot_disabled_returns_none(backup_env, monkeypatch):
+    cb = backup_env["cb"]
+    monkeypatch.setattr(cb, "is_enabled", lambda: False)
+    _write_skill(backup_env["skills"], "alpha")
+    assert cb.snapshot_skills() is None
+    # And no backup dir should have been created
+    assert not (backup_env["skills"] / ".curator_backups").exists()
+
+
+def test_snapshot_uniquifies_when_same_second(backup_env, monkeypatch):
+    """Two snapshots in the same wallclock second must not clobber each
+    other. The module appends a counter to the second snapshot's id."""
+    cb = backup_env["cb"]
+    _write_skill(backup_env["skills"], "alpha")
+    frozen = "2026-05-01T12-00-00Z"
+    monkeypatch.setattr(cb, "_utc_id", lambda now=None: frozen)
+    s1 = cb.snapshot_skills(reason="a")
+    s2 = cb.snapshot_skills(reason="b")
+    assert s1 is not None and s2 is not None
+    assert s1.name == frozen
+    assert s2.name == f"{frozen}-01"
+
+
+def test_snapshot_prunes_to_keep_count(backup_env, monkeypatch):
+    cb = backup_env["cb"]
+    _write_skill(backup_env["skills"], "alpha")
+    monkeypatch.setattr(cb, "get_keep", lambda: 3)
+
+    # Create 5 snapshots with monotonically increasing fake ids
+    ids = [f"2026-05-0{i}T00-00-00Z" for i in range(1, 6)]
+    for i, fid in enumerate(ids):
+        monkeypatch.setattr(cb, "_utc_id", lambda now=None, _f=fid: _f)
+        cb.snapshot_skills(reason=f"n{i}")
+
+    remaining = sorted(p.name for p in (backup_env["skills"] / ".curator_backups").iterdir())
+    # Newest 3 kept (lex order == date order for this id format)
+    assert remaining == ids[2:], f"expected newest 3, got {remaining}"
+
+
+# ---------------------------------------------------------------------------
+# list_backups / _resolve_backup
+# ---------------------------------------------------------------------------
+
+def test_list_backups_empty(backup_env):
+    cb = backup_env["cb"]
+    assert cb.list_backups() == []
+
+
+def test_list_backups_returns_manifest_data(backup_env):
+    cb = backup_env["cb"]
+    _write_skill(backup_env["skills"], "alpha")
+    cb.snapshot_skills(reason="m1")
+    rows = cb.list_backups()
+    assert len(rows) == 1
+    assert rows[0]["reason"] == "m1"
+    assert rows[0]["skill_files"] == 1
+
+
+def test_resolve_backup_newest_when_no_id(backup_env, monkeypatch):
+    cb = backup_env["cb"]
+    _write_skill(backup_env["skills"], "alpha")
+    ids = ["2026-05-01T00-00-00Z", "2026-05-02T00-00-00Z"]
+    for fid in ids:
+        monkeypatch.setattr(cb, "_utc_id", lambda now=None, _f=fid: _f)
+        cb.snapshot_skills()
+    resolved = cb._resolve_backup(None)
+    assert resolved is not None
+    assert resolved.name == "2026-05-02T00-00-00Z", (
+        "resolve(None) must return newest regular snapshot"
+    )
+
+
+def test_resolve_backup_unknown_id_returns_none(backup_env):
+    cb = backup_env["cb"]
+    _write_skill(backup_env["skills"], "alpha")
+    cb.snapshot_skills()
+    assert cb._resolve_backup("not-an-id") is None
+
+
+# ---------------------------------------------------------------------------
+# rollback
+# ---------------------------------------------------------------------------
+
+def test_rollback_restores_deleted_skill(backup_env):
+    """The whole point of this feature: user loses a skill, rollback
+    brings it back."""
+    cb = backup_env["cb"]
+    skills = backup_env["skills"]
+    user_skill = _write_skill(skills, "my-personal-workflow", body="important content")
+    cb.snapshot_skills(reason="pre-simulated-curator")
+
+    # Simulate curator archiving it out of existence
+    import shutil as _sh
+    _sh.rmtree(user_skill)
+    assert not user_skill.exists()
+
+    ok, msg, _ = cb.rollback()
+    assert ok, f"rollback failed: {msg}"
+    assert user_skill.exists(), "my-personal-workflow should be restored"
+    assert "important content" in (user_skill / "SKILL.md").read_text()
+
+
+def test_rollback_is_itself_undoable(backup_env):
+    """A rollback creates its own safety snapshot before replacing the
+    tree, so the user can undo a mistaken rollback. The safety snapshot
+    is a real tarball with reason='pre-rollback to <id>' — it's
+    listed by list_backups() just like any other snapshot and can be
+    restored the same way."""
+    cb = backup_env["cb"]
+    skills = backup_env["skills"]
+    _write_skill(skills, "v1")
+    cb.snapshot_skills(reason="snapshot-of-v1")
+
+    # Overwrite with a new skill state
+    import shutil as _sh
+    _sh.rmtree(skills / "v1")
+    _write_skill(skills, "v2")
+
+    ok, _, _ = cb.rollback()
+    assert ok
+    assert (skills / "v1").exists()
+
+    # list_backups should show a safety snapshot tagged "pre-rollback to <target-id>"
+    rows = cb.list_backups()
+    pre_rollback_entries = [r for r in rows if "pre-rollback" in (r.get("reason") or "")]
+    assert len(pre_rollback_entries) >= 1, (
+        f"expected a pre-rollback safety snapshot in list_backups(), got: "
+        f"{[(r.get('id'), r.get('reason')) for r in rows]}"
+    )
+    # And the transient staging dir must be gone (it's implementation detail)
+    backups_dir = skills / ".curator_backups"
+    staging_dirs = [p for p in backups_dir.iterdir() if p.name.startswith(".rollback-staging-")]
+    assert staging_dirs == [], (
+        f"staging dir should be cleaned up on success, got: {staging_dirs}"
+    )
+
+
+def test_rollback_no_snapshots_returns_error(backup_env):
+    cb = backup_env["cb"]
+    ok, msg, _ = cb.rollback()
+    assert not ok
+    assert "no matching backup" in msg.lower() or "no snapshot" in msg.lower()
+
+
+def test_rollback_rejects_unsafe_tarball(backup_env, monkeypatch):
+    """Tarballs with absolute paths or .. components must be refused even
+    if someone crafts a malicious snapshot. Defense in depth — normal
+    curator snapshots never produce these."""
+    cb = backup_env["cb"]
+    skills = backup_env["skills"]
+    _write_skill(skills, "alpha")
+    cb.snapshot_skills(reason="legit")
+
+    # Hand-craft a malicious tarball replacing the legit one
+    rows = cb.list_backups()
+    snap_dir = Path(rows[0]["path"])
+    mal = snap_dir / "skills.tar.gz"
+    mal.unlink()
+    with tarfile.open(mal, "w:gz") as tf:
+        evil = tempfile.NamedTemporaryFile(delete=False, suffix=".md")
+        evil.write(b"evil")
+        evil.close()
+        tf.add(evil.name, arcname="../../etc/evil.md")
+        os.unlink(evil.name)
+
+    ok, msg, _ = cb.rollback()
+    assert not ok
+    assert "unsafe" in msg.lower() or "refus" in msg.lower() or "extract" in msg.lower()
+
+
+# ---------------------------------------------------------------------------
+# Integration with run_curator_review
+# ---------------------------------------------------------------------------
+
+def test_real_run_takes_pre_snapshot(backup_env, monkeypatch):
+    """A real (non-dry) curator pass must snapshot the tree before calling
+    apply_automatic_transitions. This is the safety net #18373 asked for."""
+    cb = backup_env["cb"]
+    skills = backup_env["skills"]
+    _write_skill(skills, "alpha")
+
+    # Reload curator module against the freshly-env'd hermes_constants
+    from agent import curator
+    importlib.reload(curator)
+
+    # Stub out LLM review and auto transitions — we only care about the
+    # snapshot side-effect.
+    monkeypatch.setattr(
+        curator, "_run_llm_review",
+        lambda p: {"final": "", "summary": "s", "model": "", "provider": "",
+                   "tool_calls": [], "error": None},
+    )
+    monkeypatch.setattr(
+        curator, "apply_automatic_transitions",
+        lambda now=None: {"checked": 1, "marked_stale": 0, "archived": 0, "reactivated": 0},
+    )
+
+    curator.run_curator_review(synchronous=True)
+    # Pre-run snapshot should exist
+    rows = cb.list_backups()
+    assert any(r.get("reason") == "pre-curator-run" for r in rows), (
+        f"expected a pre-curator-run snapshot, got {[r.get('reason') for r in rows]}"
+    )
+
+
+def test_dry_run_skips_snapshot(backup_env, monkeypatch):
+    """Dry-run previews must not spend disk on a snapshot — they don't
+    mutate anything, so there's nothing to back up."""
+    cb = backup_env["cb"]
+    skills = backup_env["skills"]
+    _write_skill(skills, "alpha")
+
+    from agent import curator
+    importlib.reload(curator)
+    monkeypatch.setattr(
+        curator, "_run_llm_review",
+        lambda p: {"final": "", "summary": "s", "model": "", "provider": "",
+                   "tool_calls": [], "error": None},
+    )
+
+    curator.run_curator_review(synchronous=True, dry_run=True)
+    rows = cb.list_backups()
+    assert not any(r.get("reason") == "pre-curator-run" for r in rows), (
+        "dry-run must not create a pre-run snapshot"
+    )

tests/agent/test_curator_reports.py

@@ -270,3 +270,167 @@ def test_state_transitions_captured_in_report(curator_env):
     assert "State transitions" in md
     assert "getting-old" in md
     assert "active → stale" in md
+
+
+# ---------------------------------------------------------------------------
+# Cron job skill reference rewriting (curator ↔ cron integration)
+# ---------------------------------------------------------------------------
+#
+# When the curator consolidates skill X into umbrella Y during a run, any
+# cron job that listed X in its ``skills`` field would fail to load X at
+# run time — the scheduler logs a warning and skips it, so the scheduled
+# job runs without the instructions it was scheduled to follow. These
+# tests verify that _write_run_report calls into cron.jobs to repair
+# those references and records what it did in both run.json and
+# cron_rewrites.json.
+
+
+@pytest.fixture
+def curator_env_with_cron(curator_env, monkeypatch):
+    """Extend curator_env with an initialized + repointed cron.jobs module."""
+    home = curator_env["home"]
+    (home / "cron").mkdir(exist_ok=True)
+    (home / "cron" / "output").mkdir(exist_ok=True)
+
+    import importlib
+    import cron.jobs as jobs_mod
+    importlib.reload(jobs_mod)
+    monkeypatch.setattr(jobs_mod, "HERMES_DIR", home)
+    monkeypatch.setattr(jobs_mod, "CRON_DIR", home / "cron")
+    monkeypatch.setattr(jobs_mod, "JOBS_FILE", home / "cron" / "jobs.json")
+    monkeypatch.setattr(jobs_mod, "OUTPUT_DIR", home / "cron" / "output")
+
+    return {**curator_env, "jobs": jobs_mod}
+
+
+def test_curator_rewrites_cron_skills_when_skill_consolidated(curator_env_with_cron):
+    """A skill consolidated into an umbrella should be rewritten in any
+    cron job's skills list; the rewrite should be visible in run.json
+    and cron_rewrites.json."""
+    curator = curator_env_with_cron["curator"]
+    jobs = curator_env_with_cron["jobs"]
+
+    # Create a cron job that depends on a soon-to-be-consolidated skill
+    job = jobs.create_job(
+        prompt="",
+        schedule="every 1h",
+        skills=["foo"],
+        name="foo-watcher",
+    )
+
+    # Simulate a curator pass that consolidated `foo` → `foo-umbrella`
+    before = [{"name": "foo", "state": "active", "pinned": False}]
+    after = [{"name": "foo-umbrella", "state": "active", "pinned": False}]
+
+    run_dir = curator._write_run_report(
+        started_at=datetime.now(timezone.utc),
+        elapsed_seconds=3.0,
+        auto_counts={"checked": 1, "marked_stale": 0, "archived": 0, "reactivated": 0},
+        auto_summary="no changes",
+        before_report=before,
+        before_names={"foo"},
+        after_report=after,
+        llm_meta=_make_llm_meta(
+            final="Consolidated foo into foo-umbrella.",
+            tool_calls=[
+                {
+                    "name": "skill_manage",
+                    "arguments": json.dumps({
+                        "action": "write_file",
+                        "name": "foo-umbrella",
+                        "file_path": "references/foo.md",
+                        "file_content": "from foo",
+                    }),
+                },
+            ],
+        ),
+    )
+
+    # Cron job is rewritten on disk
+    loaded = jobs.get_job(job["id"])
+    assert loaded["skills"] == ["foo-umbrella"]
+    assert loaded["skill"] == "foo-umbrella"
+
+    # Rewrite is recorded in run.json
+    payload = json.loads((run_dir / "run.json").read_text())
+    assert payload["cron_rewrites"]["jobs_updated"] == 1
+    assert payload["counts"]["cron_jobs_rewritten"] == 1
+    rewrites = payload["cron_rewrites"]["rewrites"]
+    assert len(rewrites) == 1
+    assert rewrites[0]["mapped"] == {"foo": "foo-umbrella"}
+
+    # Separate cron_rewrites.json is written for convenience
+    cron_file = run_dir / "cron_rewrites.json"
+    assert cron_file.exists()
+    detail = json.loads(cron_file.read_text())
+    assert detail["jobs_updated"] == 1
+
+    # Markdown surfaces the change
+    md = (run_dir / "REPORT.md").read_text()
+    assert "Cron job skill references rewritten" in md
+    assert "foo-watcher" in md
+    assert "foo-umbrella" in md
+
+
+def test_curator_drops_pruned_skill_from_cron_job(curator_env_with_cron):
+    """A pruned (no-umbrella) skill should be dropped from the cron
+    job's skill list entirely — there's no forwarding target."""
+    curator = curator_env_with_cron["curator"]
+    jobs = curator_env_with_cron["jobs"]
+
+    job = jobs.create_job(
+        prompt="",
+        schedule="every 1h",
+        skills=["keep", "stale-one"],
+    )
+
+    before = [{"name": "stale-one", "state": "active", "pinned": False}]
+    after: list = []  # stale-one was archived with no target
+
+    run_dir = curator._write_run_report(
+        started_at=datetime.now(timezone.utc),
+        elapsed_seconds=1.0,
+        auto_counts={"checked": 1, "marked_stale": 0, "archived": 1, "reactivated": 0},
+        auto_summary="1 archived",
+        before_report=before,
+        before_names={"stale-one"},
+        after_report=after,
+        llm_meta=_make_llm_meta(),  # no tool calls → classifier marks it pruned
+    )
+
+    loaded = jobs.get_job(job["id"])
+    assert loaded["skills"] == ["keep"]
+
+    payload = json.loads((run_dir / "run.json").read_text())
+    assert payload["cron_rewrites"]["jobs_updated"] == 1
+    rewrites = payload["cron_rewrites"]["rewrites"]
+    assert rewrites[0]["dropped"] == ["stale-one"]
+
+
+def test_curator_report_has_no_cron_section_when_nothing_changes(curator_env_with_cron):
+    """When the curator run doesn't touch any skills, cron jobs are
+    untouched and cron_rewrites.json is not even written."""
+    curator = curator_env_with_cron["curator"]
+    jobs = curator_env_with_cron["jobs"]
+
+    jobs.create_job(prompt="", schedule="every 1h", skills=["foo"])
+
+    run_dir = curator._write_run_report(
+        started_at=datetime.now(timezone.utc),
+        elapsed_seconds=1.0,
+        auto_counts={"checked": 0, "marked_stale": 0, "archived": 0, "reactivated": 0},
+        auto_summary="no changes",
+        before_report=[{"name": "foo", "state": "active", "pinned": False}],
+        before_names={"foo"},
+        after_report=[{"name": "foo", "state": "active", "pinned": False}],
+        llm_meta=_make_llm_meta(),
+    )
+
+    # No rewrites → no separate file, no section in md
+    assert not (run_dir / "cron_rewrites.json").exists()
+    md = (run_dir / "REPORT.md").read_text()
+    assert "Cron job skill references rewritten" not in md
+
+    payload = json.loads((run_dir / "run.json").read_text())
+    assert payload["cron_rewrites"]["jobs_updated"] == 0
+    assert payload["counts"]["cron_jobs_rewritten"] == 0

tests/agent/test_moonshot_schema.py

@@ -115,9 +115,15 @@ class TestMissingTypeFilled:
 
 
 class TestAnyOfParentType:
-    """Rule 2: type must not appear at the anyOf parent level."""
+    """Rule 2: type must not appear at the anyOf parent level.
 
-    def test_parent_type_stripped_when_anyof_present(self):
+    When an anyOf contains a null-type branch, Moonshot rejects it.
+    The sanitizer collapses the anyOf: single non-null branch is promoted,
+    multiple non-null branches have null removed from the list.
+    """
+
+    def test_anyof_null_branch_collapsed_to_single_type(self):
+        """anyOf [string, null] → plain string (anyOf removed)."""
         params = {
             "type": "object",
             "properties": {
@@ -132,25 +138,46 @@ class TestAnyOfParentType:
         }
         out = sanitize_moonshot_tool_parameters(params)
         from_format = out["properties"]["from_format"]
-        assert "type" not in from_format
-        assert "anyOf" in from_format
+        # null branch removed, anyOf collapsed to the single non-null type
+        assert "anyOf" not in from_format
+        assert from_format["type"] == "string"
 
-    def test_anyof_children_missing_type_get_filled(self):
+    def test_anyof_multiple_non_null_preserved(self):
+        """anyOf [string, integer] (no null) → kept as-is with parent type stripped."""
         params = {
             "type": "object",
             "properties": {
-                "value": {
+                "mode": {
                     "anyOf": [
                         {"type": "string"},
-                        {"description": "A typeless option"},
+                        {"type": "integer"},
                     ],
                 },
             },
         }
         out = sanitize_moonshot_tool_parameters(params)
-        children = out["properties"]["value"]["anyOf"]
-        assert children[0]["type"] == "string"
-        assert "type" in children[1]
+        mode = out["properties"]["mode"]
+        assert "anyOf" in mode
+        assert "type" not in mode  # parent type stripped
+
+    def test_anyof_enum_with_null_collapsed(self):
+        """anyOf [{enum: [...], type: string}, {type: null}] → enum + type only."""
+        params = {
+            "type": "object",
+            "properties": {
+                "db_type": {
+                    "anyOf": [
+                        {"enum": ["mysql", "postgresql", ""]},
+                        {"type": "null"},
+                    ],
+                },
+            },
+        }
+        out = sanitize_moonshot_tool_parameters(params)
+        db_type = out["properties"]["db_type"]
+        assert "anyOf" not in db_type
+        assert db_type["type"] == "string"
+        assert db_type["enum"] == ["mysql", "postgresql"]  # "" stripped by enum cleanup
 
 
 class TestTopLevelGuarantees:
@@ -226,7 +253,7 @@ class TestRealWorldMCPShape:
     """End-to-end: a realistic MCP-style schema that used to 400 on Moonshot."""
 
     def test_combined_rewrites(self):
-        # Shape: missing type on a property, anyOf with parent type, array
+        # Shape: missing type on a property, anyOf with parent type + null, array
         # items without type — all in one tool.
         params = {
             "type": "object",
@@ -248,7 +275,125 @@ class TestRealWorldMCPShape:
         }
         out = sanitize_moonshot_tool_parameters(params)
         assert out["properties"]["query"]["type"] == "string"
-        assert "type" not in out["properties"]["filter"]
-        assert out["properties"]["filter"]["anyOf"][0]["type"] == "string"
+        # anyOf with null collapsed to plain type
+        assert "anyOf" not in out["properties"]["filter"]
+        assert out["properties"]["filter"]["type"] == "string"
         assert out["properties"]["tags"]["items"]["type"] == "string"
         assert out["required"] == ["query"]
+
+
+class TestEnumNullStripping:
+    """Rule 3: Moonshot rejects null/empty-string inside enum arrays."""
+
+    def test_enum_null_value_stripped(self):
+        """enum containing Python None must have it removed for Moonshot."""
+        params = {
+            "type": "object",
+            "properties": {
+                "db_type": {
+                    "type": "string",
+                    "enum": ["mysql", "postgresql", None],
+                },
+            },
+        }
+        out = sanitize_moonshot_tool_parameters(params)
+        db_type = out["properties"]["db_type"]
+        assert None not in db_type["enum"]
+        assert "mysql" in db_type["enum"]
+        assert "postgresql" in db_type["enum"]
+
+    def test_enum_empty_string_stripped(self):
+        """enum containing empty string '' must have it removed for Moonshot."""
+        params = {
+            "type": "object",
+            "properties": {
+                "db_type": {
+                    "type": "string",
+                    "enum": ["mysql", "postgresql", ""],
+                },
+            },
+        }
+        out = sanitize_moonshot_tool_parameters(params)
+        db_type = out["properties"]["db_type"]
+        assert "" not in db_type["enum"]
+        assert db_type["enum"] == ["mysql", "postgresql"]
+
+    def test_enum_all_null_becomes_no_enum(self):
+        """enum that only had null/empty values is dropped entirely."""
+        params = {
+            "type": "object",
+            "properties": {
+                "val": {
+                    "type": "string",
+                    "enum": [None, ""],
+                },
+            },
+        }
+        out = sanitize_moonshot_tool_parameters(params)
+        assert "enum" not in out["properties"]["val"]
+
+    def test_dataslayer_db_type_after_mcp_normalize(self):
+        """Real-world: dataslayer db_type anyOf+enum after MCP normalization."""
+        # This is the exact shape after _normalize_mcp_input_schema runs:
+        # anyOf collapsed, but enum still has null + empty string
+        params = {
+            "type": "object",
+            "properties": {
+                "datasource": {"type": "string"},
+                "db_type": {
+                    "enum": ["mysql", "mariadb", "postgresql", "sqlserver", "oracle", "", None],
+                    "type": "string",
+                    "nullable": True,
+                    "default": None,
+                },
+            },
+            "required": ["datasource"],
+        }
+        out = sanitize_moonshot_tool_parameters(params)
+        db_type = out["properties"]["db_type"]
+        assert "nullable" not in db_type, "nullable keyword must be stripped"
+        assert None not in db_type["enum"]
+        assert "" not in db_type["enum"]
+        assert db_type["enum"] == ["mysql", "mariadb", "postgresql", "sqlserver", "oracle"]
+        assert db_type["type"] == "string"
+
+    def test_enum_on_object_type_not_stripped(self):
+        """enum on non-scalar types (object) should NOT be touched."""
+        params = {
+            "type": "object",
+            "properties": {
+                "config": {
+                    "type": "object",
+                    "properties": {},
+                    "enum": [{}, None],
+                },
+            },
+        }
+        out = sanitize_moonshot_tool_parameters(params)
+        # object-typed enum should pass through unchanged
+        assert "enum" in out["properties"]["config"]
+
+    def test_anyof_collapse_still_runs_nullable_and_enum_cleanup(self):
+        """After anyOf collapses to a single non-null branch, the merged
+        node must still have ``nullable`` stripped and null/empty-string
+        values removed from enum — not skipped by the early anyOf return.
+        """
+        params = {
+            "type": "object",
+            "properties": {
+                "db_type": {
+                    "anyOf": [
+                        {"enum": ["mysql", "postgresql", "", None]},
+                        {"type": "null"},
+                    ],
+                    "nullable": True,
+                },
+            },
+        }
+        out = sanitize_moonshot_tool_parameters(params)
+        db_type = out["properties"]["db_type"]
+        assert "anyOf" not in db_type
+        assert "nullable" not in db_type, "nullable must be stripped after anyOf collapse"
+        assert db_type["type"] == "string"
+        assert db_type["enum"] == ["mysql", "postgresql"], \
+            "null/empty enum values must be stripped after anyOf collapse"

tests/agent/test_skill_utils.py

@@ -0,0 +1,58 @@
+"""Tests for agent/skill_utils.py — extract_skill_conditions metadata handling."""
+
+from agent.skill_utils import extract_skill_conditions
+
+
+def test_metadata_as_dict_with_hermes():
+    """Normal case: metadata is a dict containing hermes keys."""
+    frontmatter = {
+        "metadata": {
+            "hermes": {
+                "fallback_for_toolsets": ["toolset_a"],
+                "requires_toolsets": ["toolset_b"],
+                "fallback_for_tools": ["tool_x"],
+                "requires_tools": ["tool_y"],
+            }
+        }
+    }
+    result = extract_skill_conditions(frontmatter)
+    assert result["fallback_for_toolsets"] == ["toolset_a"]
+    assert result["requires_toolsets"] == ["toolset_b"]
+    assert result["fallback_for_tools"] == ["tool_x"]
+    assert result["requires_tools"] == ["tool_y"]
+
+
+def test_metadata_as_string_does_not_crash():
+    """Bug case: metadata is a non-dict truthy value (e.g. a YAML string)."""
+    frontmatter = {"metadata": "some text"}
+    result = extract_skill_conditions(frontmatter)
+    assert result == {
+        "fallback_for_toolsets": [],
+        "requires_toolsets": [],
+        "fallback_for_tools": [],
+        "requires_tools": [],
+    }
+
+
+def test_metadata_as_none():
+    """metadata key is present but set to null/None."""
+    frontmatter = {"metadata": None}
+    result = extract_skill_conditions(frontmatter)
+    assert result == {
+        "fallback_for_toolsets": [],
+        "requires_toolsets": [],
+        "fallback_for_tools": [],
+        "requires_tools": [],
+    }
+
+
+def test_metadata_missing_entirely():
+    """metadata key is absent from frontmatter."""
+    frontmatter = {"name": "my-skill", "description": "Does stuff."}
+    result = extract_skill_conditions(frontmatter)
+    assert result == {
+        "fallback_for_toolsets": [],
+        "requires_toolsets": [],
+        "fallback_for_tools": [],
+        "requires_tools": [],
+    }

tests/agent/test_tool_guardrails.py

@@ -0,0 +1,238 @@
+"""Pure tool-call guardrail primitive tests."""
+
+import json
+
+from agent.tool_guardrails import (
+    ToolCallGuardrailConfig,
+    ToolCallGuardrailController,
+    ToolCallSignature,
+    canonical_tool_args,
+)
+
+
+def test_tool_call_signature_hashes_canonical_nested_unicode_args_without_exposing_raw_args():
+    args_a = {
+        "z": [{"β": "☤", "a": 1}],
+        "a": {"y": 2, "x": "secret-token-value"},
+    }
+    args_b = {
+        "a": {"x": "secret-token-value", "y": 2},
+        "z": [{"a": 1, "β": "☤"}],
+    }
+
+    assert canonical_tool_args(args_a) == canonical_tool_args(args_b)
+    sig_a = ToolCallSignature.from_call("web_search", args_a)
+    sig_b = ToolCallSignature.from_call("web_search", args_b)
+
+    assert sig_a == sig_b
+    assert len(sig_a.args_hash) == 64
+    metadata = sig_a.to_metadata()
+    assert metadata == {"tool_name": "web_search", "args_hash": sig_a.args_hash}
+    assert "secret-token-value" not in json.dumps(metadata)
+    assert "☤" not in json.dumps(metadata)
+
+
+def test_default_config_is_soft_warning_only_with_hard_stop_disabled():
+    cfg = ToolCallGuardrailConfig()
+
+    assert cfg.warnings_enabled is True
+    assert cfg.hard_stop_enabled is False
+    assert cfg.exact_failure_warn_after == 2
+    assert cfg.same_tool_failure_warn_after == 3
+    assert cfg.no_progress_warn_after == 2
+    assert cfg.exact_failure_block_after == 5
+    assert cfg.same_tool_failure_halt_after == 8
+    assert cfg.no_progress_block_after == 5
+
+
+def test_config_parses_nested_warn_and_hard_stop_thresholds():
+    cfg = ToolCallGuardrailConfig.from_mapping(
+        {
+            "warnings_enabled": False,
+            "hard_stop_enabled": True,
+            "warn_after": {
+                "exact_failure": 3,
+                "same_tool_failure": 4,
+                "idempotent_no_progress": 5,
+            },
+            "hard_stop_after": {
+                "exact_failure": 6,
+                "same_tool_failure": 7,
+                "idempotent_no_progress": 8,
+            },
+        }
+    )
+
+    assert cfg.warnings_enabled is False
+    assert cfg.hard_stop_enabled is True
+    assert cfg.exact_failure_warn_after == 3
+    assert cfg.same_tool_failure_warn_after == 4
+    assert cfg.no_progress_warn_after == 5
+    assert cfg.exact_failure_block_after == 6
+    assert cfg.same_tool_failure_halt_after == 7
+    assert cfg.no_progress_block_after == 8
+
+
+def test_default_repeated_identical_failed_call_warns_without_blocking():
+    controller = ToolCallGuardrailController()
+    args = {"query": "same"}
+
+    decisions = []
+    for _ in range(5):
+        assert controller.before_call("web_search", args).action == "allow"
+        decisions.append(
+            controller.after_call("web_search", args, '{"error":"boom"}', failed=True)
+        )
+
+    assert decisions[0].action == "allow"
+    assert [d.action for d in decisions[1:]] == ["warn", "warn", "warn", "warn"]
+    assert {d.code for d in decisions[1:]} == {"repeated_exact_failure_warning"}
+    assert controller.before_call("web_search", args).action == "allow"
+    assert controller.halt_decision is None
+
+
+def test_hard_stop_enabled_blocks_repeated_exact_failure_before_next_execution():
+    controller = ToolCallGuardrailController(
+        ToolCallGuardrailConfig(
+            hard_stop_enabled=True,
+            exact_failure_warn_after=2,
+            exact_failure_block_after=2,
+            same_tool_failure_halt_after=99,
+        )
+    )
+    args = {"query": "same"}
+
+    assert controller.before_call("web_search", args).action == "allow"
+    first = controller.after_call("web_search", args, '{"error":"boom"}', failed=True)
+    assert first.action == "allow"
+
+    assert controller.before_call("web_search", args).action == "allow"
+    second = controller.after_call("web_search", args, '{"error":"boom"}', failed=True)
+    assert second.action == "warn"
+    assert second.code == "repeated_exact_failure_warning"
+
+    blocked = controller.before_call("web_search", args)
+    assert blocked.action == "block"
+    assert blocked.code == "repeated_exact_failure_block"
+    assert blocked.count == 2
+
+
+def test_success_resets_exact_signature_failure_streak():
+    controller = ToolCallGuardrailController(
+        ToolCallGuardrailConfig(hard_stop_enabled=True, exact_failure_block_after=2, same_tool_failure_halt_after=99)
+    )
+    args = {"query": "same"}
+
+    controller.after_call("web_search", args, '{"error":"boom"}', failed=True)
+    controller.after_call("web_search", args, '{"ok":true}', failed=False)
+
+    assert controller.before_call("web_search", args).action == "allow"
+    controller.after_call("web_search", args, '{"error":"boom"}', failed=True)
+    assert controller.before_call("web_search", args).action == "allow"
+
+
+def test_same_tool_varying_args_warns_by_default_without_halting():
+    controller = ToolCallGuardrailController(
+        ToolCallGuardrailConfig(same_tool_failure_warn_after=2, same_tool_failure_halt_after=3)
+    )
+
+    first = controller.after_call("terminal", {"command": "cmd-1"}, '{"exit_code":1}', failed=True)
+    second = controller.after_call("terminal", {"command": "cmd-2"}, '{"exit_code":1}', failed=True)
+    third = controller.after_call("terminal", {"command": "cmd-3"}, '{"exit_code":1}', failed=True)
+    fourth = controller.after_call("terminal", {"command": "cmd-4"}, '{"exit_code":1}', failed=True)
+
+    assert first.action == "allow"
+    assert [second.action, third.action, fourth.action] == ["warn", "warn", "warn"]
+    assert {second.code, third.code, fourth.code} == {"same_tool_failure_warning"}
+    assert controller.halt_decision is None
+
+
+def test_hard_stop_enabled_halts_same_tool_varying_args_failure_streak():
+    controller = ToolCallGuardrailController(
+        ToolCallGuardrailConfig(
+            hard_stop_enabled=True,
+            exact_failure_block_after=99,
+            same_tool_failure_warn_after=2,
+            same_tool_failure_halt_after=3,
+        )
+    )
+
+    first = controller.after_call("terminal", {"command": "cmd-1"}, '{"exit_code":1}', failed=True)
+    assert first.action == "allow"
+    second = controller.after_call("terminal", {"command": "cmd-2"}, '{"exit_code":1}', failed=True)
+    assert second.action == "warn"
+    assert second.code == "same_tool_failure_warning"
+    third = controller.after_call("terminal", {"command": "cmd-3"}, '{"exit_code":1}', failed=True)
+    assert third.action == "halt"
+    assert third.code == "same_tool_failure_halt"
+    assert third.count == 3
+
+
+def test_idempotent_no_progress_repeated_result_warns_without_blocking_by_default():
+    controller = ToolCallGuardrailController(
+        ToolCallGuardrailConfig(no_progress_warn_after=2, no_progress_block_after=2)
+    )
+    args = {"path": "/tmp/same.txt"}
+    result = "same file contents"
+
+    for _ in range(4):
+        assert controller.before_call("read_file", args).action == "allow"
+        decision = controller.after_call("read_file", args, result, failed=False)
+
+    assert decision.action == "warn"
+    assert decision.code == "idempotent_no_progress_warning"
+    assert controller.before_call("read_file", args).action == "allow"
+    assert controller.halt_decision is None
+
+
+def test_hard_stop_enabled_blocks_idempotent_no_progress_future_repeat():
+    controller = ToolCallGuardrailController(
+        ToolCallGuardrailConfig(
+            hard_stop_enabled=True,
+            no_progress_warn_after=2,
+            no_progress_block_after=2,
+        )
+    )
+    args = {"path": "/tmp/same.txt"}
+    result = "same file contents"
+
+    assert controller.before_call("read_file", args).action == "allow"
+    assert controller.after_call("read_file", args, result, failed=False).action == "allow"
+    assert controller.before_call("read_file", args).action == "allow"
+    warn = controller.after_call("read_file", args, result, failed=False)
+    assert warn.action == "warn"
+    assert warn.code == "idempotent_no_progress_warning"
+
+    blocked = controller.before_call("read_file", args)
+    assert blocked.action == "block"
+    assert blocked.code == "idempotent_no_progress_block"
+
+
+def test_mutating_or_unknown_tools_are_not_blocked_for_repeated_identical_success_output_by_default():
+    controller = ToolCallGuardrailController(
+        ToolCallGuardrailConfig(no_progress_warn_after=2, no_progress_block_after=2)
+    )
+
+    for _ in range(3):
+        assert controller.before_call("write_file", {"path": "/tmp/x", "content": "x"}).action == "allow"
+        assert controller.after_call("write_file", {"path": "/tmp/x", "content": "x"}, "ok", failed=False).action == "allow"
+        assert controller.before_call("custom_tool", {"x": 1}).action == "allow"
+        assert controller.after_call("custom_tool", {"x": 1}, "ok", failed=False).action == "allow"
+
+
+def test_reset_for_turn_clears_bounded_guardrail_state():
+    controller = ToolCallGuardrailController(
+        ToolCallGuardrailConfig(hard_stop_enabled=True, exact_failure_block_after=2, no_progress_block_after=2)
+    )
+    controller.after_call("web_search", {"query": "same"}, '{"error":"boom"}', failed=True)
+    controller.after_call("web_search", {"query": "same"}, '{"error":"boom"}', failed=True)
+    controller.after_call("read_file", {"path": "/tmp/x"}, "same", failed=False)
+    controller.after_call("read_file", {"path": "/tmp/x"}, "same", failed=False)
+
+    assert controller.before_call("web_search", {"query": "same"}).action == "block"
+    assert controller.before_call("read_file", {"path": "/tmp/x"}).action == "block"
+
+    controller.reset_for_turn()
+
+    assert controller.before_call("web_search", {"query": "same"}).action == "allow"
+    assert controller.before_call("read_file", {"path": "/tmp/x"}).action == "allow"