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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions agent/background_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@
logger = logging.getLogger(__name__)


# Empty preserves CLI agents; aliases support non-gateway direct-chat callers.
_DIRECT_CHAT_TYPES = frozenset({"", "dm", "direct", "private"})


def background_review_allowed(chat_type: Any) -> bool:
"""Return whether automatic owner-scoped background review is safe."""
normalized = "" if chat_type is None else str(chat_type).strip().lower()
return normalized in _DIRECT_CHAT_TYPES


# ---------------------------------------------------------------------------
# Background-review aux-model selector + routed digest.
#
Expand Down
9 changes: 8 additions & 1 deletion run_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -1611,7 +1611,14 @@ def _spawn_background_review(
here so existing tests that patch ``run_agent.threading.Thread``
keep working.
"""
from agent.background_review import spawn_background_review_thread
from agent.background_review import (
background_review_allowed,
spawn_background_review_thread,
)

if not background_review_allowed(getattr(self, "_chat_type", False)):
return

from tools.thread_context import propagate_context_to_thread
target, _prompt = spawn_background_review_thread(
self,
Expand Down
64 changes: 64 additions & 0 deletions tests/run_agent/test_background_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

from __future__ import annotations

from unittest.mock import Mock

import pytest

import agent.background_review as background_review
import run_agent as run_agent_module
from run_agent import AIAgent

Expand Down Expand Up @@ -29,6 +34,7 @@ def _bare_agent() -> AIAgent:
agent.background_review_callback = None
agent.status_callback = None
agent._safe_print = lambda *_args, **_kwargs: None
agent._chat_type = None
return agent


Expand All @@ -40,6 +46,64 @@ def start(self):
self._target()


@pytest.mark.parametrize(
("chat_type", "expected"),
[
(None, True),
("", True),
(False, False),
(0, False),
("dm", True),
(" DM ", True),
("direct", True),
("private", True),
("group", False),
("channel", False),
("forum", False),
("thread", False),
("room", False),
("webhook", False),
("unknown", False),
],
)
def test_background_review_allowed_for_direct_chats_only(chat_type, expected):
assert background_review.background_review_allowed(chat_type) is expected


@pytest.mark.parametrize(
"chat_type",
["group", "channel", "forum", "thread", "room", "webhook", "unknown"],
)
def test_background_review_does_not_start_for_multi_user_chat(monkeypatch, chat_type):
thread = Mock()
monkeypatch.setattr(run_agent_module.threading, "Thread", thread)

agent = _bare_agent()
agent._chat_type = chat_type
AIAgent._spawn_background_review(
agent,
messages_snapshot=[{"role": "user", "content": "hello"}],
review_memory=True,
)

thread.assert_not_called()


def test_background_review_does_not_start_without_chat_type(monkeypatch):
thread = Mock()
monkeypatch.setattr(run_agent_module.threading, "Thread", thread)

agent = _bare_agent()
del agent._chat_type
AIAgent._spawn_background_review(
agent,
messages_snapshot=[{"role": "user", "content": "hello"}],
review_memory=True,
)

thread.assert_not_called()


def test_background_review_shuts_down_memory_provider_before_close(monkeypatch):
events = []

Expand Down
1 change: 1 addition & 0 deletions tests/run_agent/test_background_review_cache_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def _make_agent_stub(agent_cls):
agent.platform = "test"
agent.provider = "openai"
agent.session_id = "sess-123"
agent._chat_type = None
agent.quiet_mode = True
agent._memory_store = None
agent._memory_enabled = True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def _make_agent_stub(agent_cls):
agent.platform = "test"
agent.provider = "openai"
agent.session_id = "sess-123"
agent._chat_type = None
agent.quiet_mode = True
agent._memory_store = None
agent._memory_enabled = True
Expand Down
30 changes: 30 additions & 0 deletions tests/run_agent/test_codex_app_server_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,36 @@ def fake_run_turn(self, user_input: str, **kwargs):
# Counter should be reset after the review fires
assert agent._iters_since_skill == 0

def test_group_review_guard_preserves_external_memory_sync(self, monkeypatch):
def fake_run_turn(self, user_input: str, **kwargs):
return TurnResult(
final_text="done",
projected_messages=[{"role": "assistant", "content": "done"}],
tool_iterations=10,
turn_id="t1",
thread_id="th1",
)

monkeypatch.setattr(CodexAppServerSession, "run_turn", fake_run_turn)
monkeypatch.setattr(
CodexAppServerSession, "ensure_started", lambda self: "th1"
)

agent = _make_codex_agent(chat_type="group")
agent._skill_nudge_interval = 10
agent._iters_since_skill = 0
agent.valid_tool_names = set(getattr(agent, "valid_tool_names", set()))
agent.valid_tool_names.add("skill_manage")
agent._memory_manager = MagicMock()
agent._memory_manager.build_system_prompt.return_value = ""

with patch.object(run_agent.threading, "Thread") as thread:
result = agent.run_conversation("do tool work")

thread.assert_not_called()
agent._memory_manager.sync_all.assert_called_once()
assert agent._memory_manager.sync_all.call_args.kwargs["messages"] == result["messages"]

def test_background_review_signature_never_breaks(self, fake_session):
"""Even when no trigger fires, the helper must never call
_spawn_background_review with the wrong signature. Run a turn,
Expand Down