Background review cache-busting bug fixes

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #25434 fix(memory): hit prefix cache in background review fork (salvage #17276 + #25427)

  merged to main

  files: hermes_cli/plugins.py, run_agent.py, scripts/release.py, tests/hermes_cli/test_plugins.py, tests/run_agent/test_background_review.py, tests/run_agent/test_background_review_cache_parity.py, tests/run_agent/test_background_review_toolset_restriction.py

  Show the diff

  • hermes_cli/plugins.py

    @@ -1339,6 +1339,21 @@ def invoke_hook(hook_name: str, **kwargs: Any) -> List[Any]:
     
     
     
    +_thread_tool_whitelist = threading.local()
    +
    +
    +def set_thread_tool_whitelist(
    +    allowed: Optional[Set[str]],
    +    deny_msg_fmt: str = "Tool '{tool_name}' denied: not in this thread's tool whitelist",
    +) -> None:
    +    _thread_tool_whitelist.allowed = allowed
    +    _thread_tool_whitelist.fmt = deny_msg_fmt
    +
    +
    +def clear_thread_tool_whitelist() -> None:
    +    _thread_tool_whitelist.allowed = None
    +
    +
     def get_pre_tool_call_block_message(
         tool_name: str,
         args: Optional[Dict[str, Any]],
    @@ -1357,6 +1372,11 @@ def get_pre_tool_call_block_message(
         directive wins.  Invalid or irrelevant hook return values are
         silently ignored so existing observer-only hooks are unaffected.
         """
    +    allowed = getattr(_thread_tool_whitelist, "allowed", None)
    +    if allowed is not None and tool_name not in allowed:
    +        fmt = getattr(_thread_tool_whitelist, "fmt", "Tool '{tool_name}' denied")
    +        return fmt.format(tool_name=tool_name)
    +
         hook_results = invoke_hook(
             "pre_tool_call",
             tool_name=tool_name,

  • run_agent.py

    @@ -4289,7 +4289,6 @@ def _bg_review_auto_deny(command, description, **kwargs):
                             api_key=_parent_runtime.get("api_key") or None,
                             credential_pool=[redacted:secret-assignment] "_credential_pool", None),
                             parent_session_id=self.session_id,
    -                        enabled_toolsets=["memory", "skills"],
                         )
                         review_agent._memory_write_origin = "background_review"
                         review_agent._memory_write_context = "background_review"
    @@ -4306,11 +4305,59 @@ def _bg_review_auto_deny(command, description, **kwargs):
                         # _vprint and leak past the stdout redirect (they go via
                         # _print_fn/status_callback, which bypass sys.stdout).
                         review_agent.suppress_status_output = True
    +                    # Inherit the parent's cached system prompt verbatim so
    +                    # the review fork's outbound HTTP request hits the same
    +                    # Anthropic/OpenRouter prefix cache the parent warmed.
    +                    # Without this, the fork rebuilds the system prompt from
    +                    # scratch (fresh _hermes_now() timestamp, fresh
    +                    # session_id, narrower toolset → different skills_prompt)
    +                    # and the byte-exact prefix-cache key misses. See
    +                    # issue #25322 and PR #17276 for the full analysis +
    +                    # measured impact (~26% end-to-end cost reduction on
    +                    # Sonnet 4.5).
    +                    review_agent._cached_system_prompt = self._cached_system_prompt
    +                    # Defensive: pin session_start + session_id to the
    +                    # parent's so any code path that re-renders parts of
    +                    # the system prompt (compression, plugin hooks) still
    +                    # produces byte-identical output. The cached-prompt
    +                    # assignment above already short-circuits the normal
    +                    # rebuild path, but these pins guarantee parity even
    +                    # if a future code path bypasses the cache.
    +                    review_agent.session_start = self.session_start
    +                    review_agent.session_id = self.session_id
    +
    +                    from model_tools import get_tool_definitions
    +                    from hermes_cli.plugins import (
    +                        set_thread_tool_whitelist,
    +                        clear_thread_tool_whitelist,
    +                    )
     
    -                    review_agent.run_conversation(
    -                        user_message=prompt,
    -                        conversation_history=messages_snapshot,
    +                    review_whitelist = {
    +                        t["function"]["name"]
    +                        for t in get_tool_definitions(
    +                            enabled_toolsets=["memory", "skills"],
    +                            quiet_mode=True,
    +                        )
    +                    }
    +                    set_thread_tool_whitelist(
    +                        review_whitelist,
    +                        deny_msg_fmt=(
    +                            "Background review denied non-whitelisted tool: "
    +                            "{tool_name}. Only memory/skill tools are allowed."
    +                        ),
                         )
    +                    try:
    +                        review_agent.run_conversation(
    +                            user_message=(
    +                                prompt
    +                                + "\n\nYou can only call memory and skill "
    +                                "management tools. Other tools will be denied "
    +                                "at runtime — do not attempt them."
    +                            ),
    +                            conversation_history=messages_snapshot,
    +                        )
    +                    finally:
    +                        clear_thread_tool_whitelist()
     
                     # Scan the review agent's messages for successful tool actions
                     # and surface a compact summary to the user. Tool messages

  • scripts/release.py

    @@ -41,6 +41,10 @@
     AUTHOR_MAP = {
         # teknium (multiple emails)
         "[redacted:email]": "teknium1",
    +    "[email protected]": "WorldWriter",
    +    "[redacted:email]": "WorldWriter",
    +    "[email protected]": "simpolism",
    +    "[redacted:email]": "simpolism",
         "[redacted:email]": "vKongv",
         "[redacted:email]": "discodirector",
         "[redacted:email]": "TruaShamu",

  • tests/hermes_cli/test_plugins.py

    @@ -538,6 +538,95 @@ def test_first_valid_block_wins(self, monkeypatch):
             assert get_pre_tool_call_block_message("terminal", {}) == "first blocker"
     
     
    +class TestThreadToolWhitelist:
    +    """Tests for the thread-local tool whitelist used by background review forks."""
    +
    +    def test_allowed_tool_passes_through_to_hooks(self, monkeypatch):
    +        from hermes_cli.plugins import (
    +            set_thread_tool_whitelist,
    +            clear_thread_tool_whitelist,
    +        )
    +
    +        monkeypatch.setattr(
    +            "hermes_cli.plugins.invoke_hook",
    +            lambda hook_name, **kwargs: [],
    +        )
    +        set_thread_tool_whitelist({"memory", "skill_manage"})
    +        try:
    +            assert get_pre_tool_call_block_message("memory", {}) is None
    +        finally:
    +            clear_thread_tool_whitelist()
    +
    +    def test_disallowed_tool_blocked_with_message(self, monkeypatch):
    +        from hermes_cli.plugins import (
    +            set_thread_tool_whitelist,
    +            clear_thread_tool_whitelist,
    +        )
    +
    +        monkeypatch.setattr(
    +            "hermes_cli.plugins.invoke_hook",
    +            lambda hook_name, **kwargs: [],
    +        )
    +        set_thread_tool_whitelist(
    +            {"memory"}, deny_msg_fmt="denied: {tool_name}"
    +        )
    +        try:
    +            msg = get_pre_tool_call_block_message("terminal", {})
    +            assert msg == "denied: terminal"
    +        finally:
    +            clear_thread_tool_whitelist()
    +
    +    def test_clear_restores_unrestricted_behavior(self, monkeypatch):
    +        from hermes_cli.plugins import (
    +            set_thread_tool_whitelist,
    +            clear_thread_tool_whitelist,
    +        )
    +
    +        monkeypatch.setattr(
    +            "hermes_cli.plugins.invoke_hook",
    +            lambda hook_name, **kwargs: [],
    +        )
    +        set_thread_tool_whitelist({"memory"})
    +        clear_thread_tool_whitelist()
    +        # After clearing, any tool should pass through to plugin hooks (which
    +        # return [] here, so result is None).
    +        assert get_pre_tool_call_block_message("terminal", {}) is None
    +
    +    def test_whitelist_is_thread_local(self, monkeypatch):
    +        """Setting a whitelist in one thread must NOT leak into another."""
    +        import threading
    +
    +        from hermes_cli.plugins import (
    +            set_thread_tool_whitelist,
    +            clear_thread_tool_whitelist,
    +        )
    +
    +        monkeypatch.setattr(
    +            "hermes_cli.plugins.invoke_hook",
    +            lambda hook_name, **kwargs: [],
    +        )
    +
    +        # Main thread: install a restrictive whitelist.
    +        set_thread_tool_whitelist({"memory"})
    +        try:
    +            assert get_pre_tool_call_block_message("terminal", {}) is not None
    +
    +            # Worker thread: should NOT inherit main thread's whitelist.
    +            result = {}
    +
    +            def worker():
    +                result["msg"] = get_pre_tool_call_block_message("terminal", {})
    +
    +            t = threading.Thread(target=worker)
    +            t.start()
    +            t.join()
    +            assert result["msg"] is None, (
    +                "thread-local whitelist leaked across threads"
    +            )
    +        finally:
    +            clear_thread_tool_whitelist()
    +
    +
     # ── TestPluginContext ──────────────────────────────────────────────────────
     
     

  • tests/run_agent/test_background_review.py

    @@ -20,6 +20,9 @@ def _bare_agent() -> AIAgent:
         agent._memory_store = object()
         agent._memory_enabled = True
         agent._user_profile_enabled = False
    +    agent._cached_system_prompt = "test-cached-system-prompt"
    +    import datetime as _dt
    +    agent.session_start = _dt.datetime(2026, 1, 1, 12, 0, 0)
         agent._MEMORY_REVIEW_PROMPT = "review memory"
         agent._SKILL_REVIEW_PROMPT = "review skills"
         agent._COMBINED_REVIEW_PROMPT = "review both"

  • tests/run_agent/test_background_review_cache_parity.py

    @@ -0,0 +1,185 @@
    +"""Tests that the background review fork inherits the parent's cached system prompt.
    +
    +Regression coverage for issue #25322 (and PR #17276's first root cause): the
    +background review's outbound HTTP request must carry the same system bytes as
    +the parent's so Anthropic/OpenRouter's exact-prefix cache key matches.
    +
    +Without this, every review rebuilds the system prompt from scratch — fresh
    +``_hermes_now()`` timestamp, fresh ``session_id``, and a different skills
    +prompt under the (former) narrow toolset — and the prefix-cache miss costs
    +roughly the full uncached system-prompt cost per nudge (~26% end-to-end on
    +Sonnet 4.5 per the contributor's measurement).
    +"""
    +
    +from unittest.mock import patch
    +
    +
    +def _make_agent_stub(agent_cls):
    +    """Create a minimal AIAgent-like object with just enough state for _spawn_background_review."""
    +    agent = object.__new__(agent_cls)
    +    agent.model = "test-model"
    +    agent.platform = "test"
    +    agent.provider = "openai"
    +    agent.session_id = "sess-123"
    +    agent.quiet_mode = True
    +    agent._memory_store = None
    +    agent._memory_enabled = True
    +    agent._user_profile_enabled = False
    +    agent._memory_nudge_interval = 5
    +    agent._skill_nudge_interval = 5
    +    agent.background_review_callback = None
    +    agent.status_callback = None
    +    agent._cached_system_prompt = (
    +        "PARENT-SYSTEM-PROMPT-BYTES — must be inherited verbatim "
    +        "for prefix-cache parity"
    +    )
    +    import datetime as _dt
    +    agent.session_start = _dt.datetime(2026, 1, 1, 12, 0, 0)
    +    agent._MEMORY_REVIEW_PROMPT = "review memory"
    +    agent._SKILL_REVIEW_PROMPT = "review skills"
    +    agent._COMBINED_REVIEW_PROMPT = "review both"
    +    return agent
    +
    +
    +class _SyncThread:
    +    """Drop-in replacement for threading.Thread that runs the target inline."""
    +
    +    def __init__(self, *, target=None, daemon=None, name=None):
    +        self._target = target
    +
    +    def start(self):
    +        if self._target:
    +            self._target()
    +
    +
    +class _ReviewAgentRecorder:
    +    """Stand-in for the review-fork AIAgent that records the prompt assignment."""
    +
    +    def __init__(self, *args, **kwargs):
    +        self._cached_system_prompt = None
    +        self._memory_write_origin = None
    +        self._memory_write_context = None
    +        self._memory_store = None
    +        self._memory_enabled = None
    +        self._user_profile_enabled = None
    +        self._memory_nudge_interval = None
    +        self._skill_nudge_interval = None
    +        self.suppress_status_output = None
    +
    +    def run_conversation(self, *args, **kwargs):
    +        raise RuntimeError("stop after recording state — don't actually call the API")
    +
    +    def shutdown_memory_provider(self):
    +        pass
    +
    +    def close(self):
    +        pass
    +
    +
    +def test_review_fork_inherits_parent_cached_system_prompt():
    +    """The review fork's _cached_system_prompt must equal the parent's byte-for-byte.
    +
    +    Anthropic's prefix cache keys on exact bytes; any divergence (timestamp
    +    minute tick, fresh session_id, narrower skills_prompt) shifts the key
    +    and forces a full re-cache. Inheriting the parent's cached prompt is
    +    the cheap, mechanical fix.
    +    """
    +    import run_agent
    +
    +    agent = _make_agent_stub(run_agent.AIAgent)
    +
    +    captured = {}
    +    parent_prompt = agent._cached_system_prompt
    +
    +    # Hook the assignment site: record what gets put on the review agent.
    +    real_recorder_init = _ReviewAgentRecorder.__init__
    +
    +    def _recorder_init(self, *args, **kwargs):
    +        real_recorder_init(self, *args, **kwargs)
    +        # The actual production code assigns _cached_system_prompt AFTER __init__,
    +        # so we need to capture it on attribute set. Use a property-style sentinel
    +        # via __setattr__ on this instance.
    +
    +    with patch.object(run_agent, "AIAgent", _ReviewAgentRecorder), \
    +         patch("threading.Thread", _SyncThread):
    +        # Wrap the recorder's __setattr__ so we can see the _cached_system_prompt
    +        # write that _spawn_background_review performs after construction.
    +        orig_setattr = _ReviewAgentRecorder.__setattr__
    +
    +        def _spy_setattr(self, name, value):
    +            if name == "_cached_system_prompt":
    +                captured["written_prompt"] = value
    +            orig_setattr(self, name, value)
    +
    +        with patch.object(_ReviewAgentRecorder, "__setattr__", _spy_setattr):
    +            agent._spawn_background_review(
    +                messages_snapshot=[],
    +                review_memory=True,
    +                review_skills=False,
    +            )
    +
    +    assert "written_prompt" in captured, (
    +        "_spawn_background_review never assigned _cached_system_prompt on the review agent"
    +    )
    +    assert captured["written_prompt"] == parent_prompt, (
    +        f"Review fork's _cached_system_prompt diverged from parent's. "
    +        f"Got {captured['written_prompt']!r}, expected {parent_prompt!r}. "
    +        "This breaks Anthropic/OpenRouter prefix-cache parity (#25322)."
    +    )
    +
    +
    +def test_review_fork_pins_session_start_and_session_id():
    +    """Defensive complement to cached-system-prompt inheritance.
    +
    +    Even though ``_cached_system_prompt`` inheritance short-circuits the
    +    normal rebuild path, pinning ``session_start`` and ``session_id`` to
    +    the parent's guarantees byte-identical output from any code path that
    +    re-renders parts of the system prompt (compression, plugin hooks).
    +    """
    +    import run_agent
    +
    +    agent = _make_agent_stub(run_agent.AIAgent)
    +
    +    captured = {}
    +
    +    class _Recorder:
    +        def __init__(self, *args, **kwargs):
    +            self._cached_system_prompt = None
    +            self._memory_write_origin = None
    +            self._memory_write_context = None
    +            self._memory_store = None
    +            self._memory_enabled = None
    +            self._user_profile_enabled = None
    +            self._memory_nudge_interval = None
    +            self._skill_nudge_interval = None
    +            self.suppress_status_output = None
    +            self.session_start = None
    +            self.session_id = None
    +
    +        def run_conversation(self, *args, **kwargs):
    +            captured["session_start"] = self.session_start
    +            captured["session_id"] = self.session_id
    +            raise RuntimeError("stop after recording")
    +
    +        def shutdown_memory_provider(self):
    +            pass
    +
    +        def close(self):
    +            pass
    +
    +    with patch.object(run_agent, "AIAgent", _Recorder), \
    +         patch("threading.Thread", _SyncThread):
    +        agent._spawn_background_review(
    +            messages_snapshot=[],
    +            review_memory=True,
    +            review_skills=False,
    +        )
    +
    +    assert captured.get("session_start") == agent.session_start, (
    +        "Review fork did not inherit parent's session_start — "
    +        "system-prompt rebuild paths would diverge."
    +    )
    +    assert captured.get("session_id") == agent.session_id, (
    +        "Review fork did not inherit parent's session_id — "
    +        "system-prompt rebuild paths would diverge."
    +    )

  • tests/run_agent/test_background_review_toolset_restriction.py

    @@ -1,8 +1,16 @@
    -"""Tests that the background review agent is restricted to memory+skills toolsets.
    -
    -Regression coverage for issue #15204: the background skill-review agent
    -inherited the full default toolset, allowing it to perform non-skill side
    -effects (terminal, send_message, delegate_task, etc.).
    +"""Tests that the background review agent restricts tools at runtime, not at schema time.
    +
    +Regression coverage for issue #15204 (the background skill-review agent must
    +not perform non-skill side effects like terminal, send_message, delegate_task)
    +combined with issue #25322 / PR #17276 (the review fork must hit the parent's
    +Anthropic/OpenRouter prefix cache).
    +
    +Reconciling the two: the fork now inherits the parent's full ``tools`` schema
    +so the cache-key matches, and enforces the memory+skills restriction at
    +runtime via a thread-local whitelist on the existing
    +``get_pre_tool_call_block_message`` gate. Safety is preserved mechanically
    +(any non-whitelisted dispatch is blocked) without the schema-level narrowing
    +that caused the prefix-cache miss.
     """
     
     import threading
    @@ -24,6 +32,9 @@ def _make_agent_stub(agent_cls):
         agent._skill_nudge_interval = 5
         agent.background_review_callback = None
         agent.status_callback = None
    +    agent._cached_system_prompt = None
    +    import datetime as _dt
    +    agent.session_start = _dt.datetime(2026, 1, 1, 12, 0, 0)
         agent._MEMORY_REVIEW_PROMPT = "review memory"
         agent._SKILL_REVIEW_PROMPT = "review skills"
         agent._COMBINED_REVIEW_PROMPT = "review both"
    @@ -41,15 +52,20 @@ def start(self):
                 self._target()
     
     
    -def test_background_review_agent_uses_restricted_toolsets():
    -    """The review agent must only have access to 'memory' and 'skills' toolsets."""
    +def test_background_review_does_not_narrow_toolset_schema():
    +    """The review fork must NOT pass enabled_toolsets to AIAgent.
    +
    +    Narrowing the schema diverges the ``tools`` cache key from the parent's,
    +    which sits above ``system`` in Anthropic's cache hierarchy and forces a
    +    full prefix-cache miss on every review (see #25322, PR #17276).
    +    """
         import run_agent
     
         agent = _make_agent_stub(run_agent.AIAgent)
         captured = {}
     
         def _capture_init(self, *args, **kwargs):
    -        captured["enabled_toolsets"] = kwargs.get("enabled_toolsets")
    +        captured["enabled_toolsets"] = kwargs.get("enabled_toolsets", "UNSET")
             raise RuntimeError("stop after capturing init args")
     
         with patch.object(run_agent.AIAgent, "__init__", _capture_init), \
    @@ -61,11 +77,71 @@ def _capture_init(self, *args, **kwargs):
             )
     
         assert "enabled_toolsets" in captured, "AIAgent.__init__ was not called"
    -    assert sorted(captured["enabled_toolsets"]) == ["memory", "skills"]
    +    # The kwarg must be absent — letting AIAgent inherit the default full
    +    # toolset so the schema bytes match the parent's.
    +    assert captured["enabled_toolsets"] == "UNSET", (
    +        f"Review fork narrowed the toolset schema (got {captured['enabled_toolsets']!r}), "
    +        "which breaks prefix-cache parity with the parent."
    +    )
    +
    +
    +def test_background_review_installs_thread_local_whitelist():
    +    """The review fork must install a memory/skills-only thread-local whitelist.
    +
    +    The schema-level toolset narrowing was lifted (for prefix-cache parity),
    +    so #15204's safety contract now relies on the runtime whitelist gate to
    +    deny terminal/send_message/delegate_task at dispatch time. Verify the
    +    whitelist is set with exactly the memory+skills tool names.
    +    """
    +    import run_agent
    +    from hermes_cli import plugins as _plugins
    +
    +    captured = {}
    +
    +    def _capture_whitelist(whitelist, deny_msg_fmt=None):
    +        captured["whitelist"] = set(whitelist)
    +        captured["deny_msg_fmt"] = deny_msg_fmt
    +        # Stop here — we just want to see what gets installed.
    +        raise RuntimeError("stop after capturing whitelist")
    +
    +    agent = _make_agent_stub(run_agent.AIAgent)
    +
    +    def _no_init(self, *args, **kwargs):
    +        # Don't crash AIAgent.__init__; let execution flow reach
    +        # set_thread_tool_whitelist.
    +        return None
    +
    +    with patch.object(run_agent.AIAgent, "__init__", _no_init), \
    +         patch.object(_plugins, "set_thread_tool_whitelist", _capture_whitelist), \
    +         patch("threading.Thread", _SyncThread):
    +        agent._spawn_background_review(
    +            messages_snapshot=[],
    +            review_memory=True,
    +            review_skills=False,
    +        )
    +
    +    assert "whitelist" in captured, "set_thread_tool_whitelist was not called"
    +    whitelist = captured["whitelist"]
    +    # memory + skills tools must be allowed
    +    assert "memory" in whitelist
    +    assert "skill_manage" in whitelist
    +    assert "skill_view" in whitelist
    +    assert "skills_list" in whitelist
    +    # dangerous tools must NOT be in the whitelist
    +    assert "terminal" not in whitelist
    +    assert "send_message" not in whitelist
    +    assert "delegate_task" not in whitelist
    +    assert "web_search" not in whitelist
    +    assert "execute_code" not in whitelist
     
     
     def test_background_review_agent_tools_are_limited():
    -    """Verify the resolved memory+skills toolsets only contain memory and skill tools."""
    +    """Verify the resolved memory+skills toolsets only contain memory and skill tools.
    +
    +    Sanity check on the source of truth for what the runtime whitelist is
    +    derived from — if a future PR adds e.g. `terminal` to the `memory`
    +    toolset, the review-fork safety contract silently breaks.
    +    """
         from toolsets import resolve_multiple_toolsets
     
         expected_tools = set(resolve_multiple_toolsets(["memory", "skills"]))

• #64379 fix(background_review): inherit parent's reasoning_config for Anthropic cache parity (salvage #30532)

  merged to main

  files: agent/background_review.py, tests/run_agent/test_background_review_cache_parity.py

  Show the diff

  • agent/background_review.py

    @@ -696,6 +696,19 @@ def _bg_review_auto_deny(command, description, **kwargs):
                 if isinstance(_rt.get("command"), str) and _rt["command"]:
                     _fork_kwargs["acp_command"] = _rt["command"]
                     _fork_kwargs["acp_args"] = _rt.get("args") or []
    +            # Match parent's reasoning config so the fork's ``thinking`` /
    +            # ``output_config`` are byte-identical in the request body —
    +            # Anthropic's cache key is namespaced by ``thinking`` presence.
    +            # Same-model path only: when routed to a different aux model the
    +            # cache is cold regardless (parity buys nothing) and the parent's
    +            # effort vocabulary may not be valid for the routed model/provider
    +            # (e.g. OpenRouter ``extra_body.reasoning.effort`` is forwarded
    +            # unclamped; codex_responses passes ``max``/``ultra`` through
    +            # unmapped except on gpt-5.6/xAI). Let the routed fork use
    +            # provider defaults — matching the ``not _routed`` gate on
    +            # _cached_system_prompt below.
    +            if not _routed:
    +                _fork_kwargs["reasoning_config"] = getattr(agent, "reasoning_config", None)
                 review_agent = AIAgent(
                     model=_rt.get("model") or agent.model,
                     max_iterations=16,

  • tests/run_agent/test_background_review_cache_parity.py

    @@ -41,6 +41,9 @@ def _make_agent_stub(agent_cls):
         # Non-None so the test catches a missing-kwarg regression.
         agent.enabled_toolsets = ["memory", "skills", "terminal"]
         agent.disabled_toolsets = ["spotify", "feishu_doc"]
    +    # Non-None so the test catches reasoning_config NOT being inherited —
    +    # which would put the fork into a different Anthropic cache namespace.
    +    agent.reasoning_config = {"enabled": True, "effort": "medium"}
         return agent
     
     
    @@ -55,28 +58,52 @@ def start(self):
                 self._target()
     
     
    -class _ReviewAgentRecorder:
    -    """Stand-in for the review-fork AIAgent that records the prompt assignment."""
    +def _make_recorder_class(captured=None, record_on_run=()):
    +    """Build a Recorder class standing in for the review-fork AIAgent.
     
    -    def __init__(self, *args, **kwargs):
    -        self._cached_system_prompt = None
    -        self._memory_write_origin = None
    -        self._memory_write_context = None
    -        self._memory_store = None
    -        self._memory_enabled = None
    -        self._user_profile_enabled = None
    -        self._memory_nudge_interval = None
    -        self._skill_nudge_interval = None
    -        self.suppress_status_output = None
    +    Keeps the stub attribute list in ONE place: when
    +    ``_spawn_background_review`` starts touching a new fork attribute, only
    +    this factory needs the extra stub — not one copy per test.
     
    -    def run_conversation(self, *args, **kwargs):
    -        raise RuntimeError("stop after recording state — don't actually call the API")
    +    ``captured`` (dict): if given, ``__init__`` stores the full constructor
    +    kwargs under ``captured["init_kwargs"]`` so tests can assert on both
    +    kwarg values and kwarg *presence*.
    +    ``record_on_run``: instance attribute names copied into ``captured`` when
    +    ``run_conversation`` fires — for values the production code assigns
    +    after construction.
    +    """
    +
    +    class _Recorder:
    +        def __init__(self, *args, **kwargs):
    +            if captured is not None:
    +                captured["init_kwargs"] = dict(kwargs)
    +            self._cached_system_prompt = None
    +            self._memory_write_origin = None
    +            self._memory_write_context = None
    +            self._memory_store = None
    +            self._memory_enabled = None
    +            self._user_profile_enabled = None
    +            self._memory_nudge_interval = None
    +            self._skill_nudge_interval = None
    +            self.suppress_status_output = None
    +            self.session_start = None
    +            self.session_id = None
     
    -    def shutdown_memory_provider(self):
    -        pass
    +        def run_conversation(self, *args, **kwargs):
    +            if captured is not None:
    +                for _name in record_on_run:
    +                    captured[_name] = getattr(self, _name)
    +            raise RuntimeError(
    +                "stop after recording — don't actually call the API"
    +            )
     
    -    def close(self):
    -        pass
    +        def shutdown_memory_provider(self):
    +            pass
    +
    +        def close(self):
    +            pass
    +
    +    return _Recorder
     
     
     def test_review_fork_inherits_parent_cached_system_prompt():
    @@ -94,27 +121,21 @@ def test_review_fork_inherits_parent_cached_system_prompt():
         captured = {}
         parent_prompt = agent._cached_system_prompt
     
    -    # Hook the assignment site: record what gets put on the review agent.
    -    real_recorder_init = _ReviewAgentRecorder.__init__
    +    _Recorder = _make_recorder_class()
     
    -    def _recorder_init(self, *args, **kwargs):
    -        real_recorder_init(self, *args, **kwargs)
    -        # The actual production code assigns _cached_system_prompt AFTER __init__,
    -        # so we need to capture it on attribute set. Use a property-style sentinel
    -        # via __setattr__ on this instance.
    -
    -    with patch.object(run_agent, "AIAgent", _ReviewAgentRecorder), \
    +    with patch.object(run_agent, "AIAgent", _Recorder), \
              patch("threading.Thread", _SyncThread):
    -        # Wrap the recorder's __setattr__ so we can see the _cached_system_prompt
    -        # write that _spawn_background_review performs after construction.
    -        orig_setattr = _ReviewAgentRecorder.__setattr__
    +        # The production code assigns _cached_system_prompt AFTER __init__,
    +        # so wrap the recorder's __setattr__ to see that post-construction
    +        # write from _spawn_background_review.
    +        orig_setattr = _Recorder.__setattr__
     
             def _spy_setattr(self, name, value):
                 if name == "_cached_system_prompt":
                     captured["written_prompt"] = value
                 orig_setattr(self, name, value)
     
    -        with patch.object(_ReviewAgentRecorder, "__setattr__", _spy_setattr):
    +        with patch.object(_Recorder, "__setattr__", _spy_setattr):
                 agent._spawn_background_review(
                     messages_snapshot=[],
                     review_memory=True,
    @@ -144,31 +165,9 @@ def test_review_fork_pins_session_start_and_session_id():
         agent = _make_agent_stub(run_agent.AIAgent)
     
         captured = {}
    -
    -    class _Recorder:
    -        def __init__(self, *args, **kwargs):
    -            self._cached_system_prompt = None
    -            self._memory_write_origin = None
    -            self._memory_write_context = None
    -            self._memory_store = None
    -            self._memory_enabled = None
    -            self._user_profile_enabled = None
    -            self._memory_nudge_interval = None
    -            self._skill_nudge_interval = None
    -            self.suppress_status_output = None
    -            self.session_start = None
    -            self.session_id = None
    -
    -        def run_conversation(self, *args, **kwargs):
    -            captured["session_start"] = self.session_start
    -            captured["session_id"] = self.session_id
    -            raise RuntimeError("stop after recording")
    -
    -        def shutdown_memory_provider(self):
    -            pass
    -
    -        def close(self):
    -            pass
    +    _Recorder = _make_recorder_class(
    +        captured, record_on_run=("session_start", "session_id")
    +    )
     
         with patch.object(run_agent, "AIAgent", _Recorder), \
              patch("threading.Thread", _SyncThread):
    @@ -195,31 +194,40 @@ def test_review_fork_inherits_parent_toolset_config():
         agent = _make_agent_stub(run_agent.AIAgent)
     
         captured = {}
    +    _Recorder = _make_recorder_class(captured)
     
    -    class _Recorder:
    -        def __init__(self, *args, **kwargs):
    -            captured["enabled_toolsets"] = kwargs.get("enabled_toolsets")
    -            captured["disabled_toolsets"] = kwargs.get("disabled_toolsets")
    -            self._cached_system_prompt = None
    -            self._memory_write_origin = None
    -            self._memory_write_context = None
    -            self._memory_store = None
    -            self._memory_enabled = None
    -            self._user_profile_enabled = None
    -            self._memory_nudge_interval = None
    -            self._skill_nudge_interval = None
    -            self.suppress_status_output = None
    -            self.session_start = None
    -            self.session_id = None
    +    with patch.object(run_agent, "AIAgent", _Recorder), \
    +         patch("threading.Thread", _SyncThread):
    +        agent._spawn_background_review(
    +            messages_snapshot=[],
    +            review_memory=True,
    +            review_skills=False,
    +        )
     
    -        def run_conversation(self, *args, **kwargs):
    -            raise RuntimeError("stop after recording — don't actually call the API")
    +    init_kwargs = captured.get("init_kwargs", {})
    +    assert init_kwargs.get("enabled_toolsets") == agent.enabled_toolsets, (
    +        f"enabled_toolsets mismatch: {init_kwargs.get('enabled_toolsets')!r} "
    +        f"vs expected {agent.enabled_toolsets!r}"
    +    )
    +    assert init_kwargs.get("disabled_toolsets") == agent.disabled_toolsets, (
    +        f"disabled_toolsets mismatch: {init_kwargs.get('disabled_toolsets')!r} "
    +        f"vs expected {agent.disabled_toolsets!r}"
    +    )
     
    -        def shutdown_memory_provider(self):
    -            pass
     
    -        def close(self):
    -            pass
    +def test_review_fork_inherits_parent_reasoning_config():
    +    """``reasoning_config`` parity on the default (non-routed) path.
    +
    +    The fork must inherit the parent's value so the request body's
    +    ``thinking`` / ``output_config`` match — Anthropic's cache is
    +    namespaced by ``thinking`` presence.
    +    """
    +    import run_agent
    +
    +    agent = _make_agent_stub(run_agent.AIAgent)
    +
    +    captured = {}
    +    _Recorder = _make_recorder_class(captured)
     
         with patch.object(run_agent, "AIAgent", _Recorder), \
              patch("threading.Thread", _SyncThread):
    @@ -229,11 +237,62 @@ def close(self):
                 review_skills=False,
             )
     
    -    assert captured.get("enabled_toolsets") == agent.enabled_toolsets, (
    -        f"enabled_toolsets mismatch: {captured.get('enabled_toolsets')!r} "
    -        f"vs expected {agent.enabled_toolsets!r}"
    +    init_kwargs = captured.get("init_kwargs", {})
    +    assert init_kwargs.get("reasoning_config") == agent.reasoning_config, (
    +        f"reasoning_config mismatch: {init_kwargs.get('reasoning_config')!r} "
    +        f"vs expected {agent.reasoning_config!r}"
         )
    -    assert captured.get("disabled_toolsets") == agent.disabled_toolsets, (
    -        f"disabled_toolsets mismatch: {captured.get('disabled_toolsets')!r} "
    -        f"vs expected {agent.disabled_toolsets!r}"
    +
    +
    +def test_routed_review_fork_does_not_inherit_reasoning_config():
    +    """Routed aux path: the fork must NOT inherit the parent's reasoning_config.
    +
    +    When ``auxiliary.background_review.{provider,model}`` routes the review
    +    to a different model, cache parity is moot (the cache is cold on that
    +    model regardless) and the parent's effort vocabulary may be invalid for
    +    the routed model/provider (OpenRouter ``extra_body.reasoning.effort`` is
    +    forwarded unclamped; codex_responses passes ``max``/``ultra`` through
    +    unmapped except on gpt-5.6/xAI). The routed fork must fall back to
    +    provider defaults, mirroring the ``not _routed`` gate on
    +    ``_cached_system_prompt`` inheritance.
    +    """
    +    import run_agent
    +    import agent.background_review as bg_review
    +
    +    agent_stub = _make_agent_stub(run_agent.AIAgent)
    +
    +    captured = {}
    +    _Recorder = _make_recorder_class(captured)
    +
    +    routed_runtime = {
    +        "provider": "openrouter",
    +        "model": "aux-cheap-model",
    +        "api_key": "[redacted:secret-assignment]",
    +        "base_url": None,
    +        "api_mode": None,
    +        "credential_pool": None,
    +        "request_overrides": {},
    +        "max_tokens": None,
    +        "command": None,
    +        "args": [],
    +        "routed": True,
    +    }
    +
    +    with patch.object(run_agent, "AIAgent", _Recorder), \
    +         patch.object(bg_review, "_resolve_review_runtime",
    +                      return_value=routed_runtime), \
    +         patch("threading.Thread", _SyncThread):
    +        agent_stub._spawn_background_review(
    +            messages_snapshot=[],
    +            review_memory=True,
    +            review_skills=False,
    +        )
    +
    +    init_kwargs = captured.get("init_kwargs", {})
    +    assert "reasoning_config" not in init_kwargs, (
    +        f"Routed review fork was passed the parent's reasoning_config "
    +        f"({init_kwargs.get('reasoning_config')!r}). On the routed path the "
    +        "cache is cold (no parity benefit) and the parent's effort value may "
    +        "be invalid for the routed model/provider — it must be omitted so "
    +        "the fork uses provider defaults."
         )