Files
odysseus/tests/test_task_scheduler_session_delivery.py

116 lines
3.9 KiB
Python
Raw Normal View History

"""Regression tests for task-result delivery into chat sessions (issue #326)."""
import asyncio
2026-06-04 01:27:29 +02:00
import sys
import types as _types
import pytest
sqlalchemy = pytest.importorskip("sqlalchemy")
if not isinstance(sqlalchemy, _types.ModuleType):
pytest.skip("sqlalchemy is stubbed in this environment", allow_module_level=True)
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
from tests.helpers.import_state import clear_fake_database_modules
2026-06-04 01:27:29 +02:00
clear_fake_database_modules()
2026-06-04 01:27:29 +02:00
import core.database as cdb
from core.database import Base, Session as DbSession
fix: session context drifting — messages leaking between chats (#135) (#267) * docs: add implementation plan for fixing chat context drifting (#135) * fix: make Session.history immutable + fix {}.history crash - Session.history now exposes a COPY of the internal _history list - add_message() replaces history with a fresh copy each time - get_context_messages() derives from _history directly - replace_messages() updates both _history and history - truncate_messages() updates both _history and history - _persist_message() line 207: fixed {}.history fallback crash - Added 11 tests for session isolation and edge cases Addresses #135 root cause #1: shared mutable references * fix: task scheduler uses SessionManager methods instead of overwriting sessions - Added ensure_task_session() to SessionManager (checks cache first) - Task scheduler now uses ensure_task_session() instead of direct dict assignment - Task scheduler now uses SessionManager.add_message() for message persistence - Removed direct sess_obj.history.append() that was silently losing data Addresses #135 root causes #2 and #3 * fix: add age guard to cleanup_empty_sessions — don't delete sessions <1h old Prevents the cleanup task from deleting sessions that were just created and haven't received any messages yet (message_count == 0). Addresses #135 root cause #5 * test: comprehensive session isolation tests (10/10 passing) * refactor: consolidate _session_manager into singleton pattern - Added set_session_manager_instance / get_session_manager_instance to core/models - kept backward-compat aliases (set_session_manager, get_session_manager) - session_manager.py re-exports the singleton functions - ai_interaction.set_session_manager now syncs with the core singleton - context_compactor uses get_session_manager_instance() instead of getattr hack - app.py initializes the singleton once Addresses #135 root cause #4: fragile global wiring * test: add concurrent session isolation integration tests Verifies: - Concurrent add_message to different sessions doesn't cross-contaminate - Rapid parallel writes maintain isolation - Read-write concurrent access is safe All 3 async tests pass, proving the immutable history fix works under concurrency * fix: pre-import core.models in conftest to prevent test pollution test_agent_loop.py stubs sys.modules['core.models'] = MagicMock() at module level during collection. Any test collected after it imports Session as a MagicMock. Pre-importing core.models in conftest.py before test_agent_loop.py's module-level code runs prevents this. * fix: make .history authoritative mutable list, address PR review Per review feedback: keep .history as the authoritative mutable list so existing code doing .history.pop(), .history = [...], etc. still works. Fix the cross-contamination bug by ensuring __post_init__() gives each Session its OWN unique history list (never shared). Changes: - core/models.py: .history IS the authoritative list. _history aliases it. Each Session gets its own list in __post_init__. - core/session_manager.py: add_message() delegates to Session.add_message() instead of appending directly — no double-append, single source of truth. - tests/test_session_manager.py: updated test to reflect that .history references see new messages (same list, not a snapshot). - docs/plans/2026-06-01-fix-chat-context-drifting.md: removed (not for shipping — useful design context but too much process/doc to ship). All 272 tests pass (3 pre-existing failures unrelated). * Fix session manager message persistence * Fix session history alias regressions * Fix session history aliasing and task delivery
2026-06-09 21:12:52 +08:00
from core.models import ChatMessage as MemChatMessage
from src.task_scheduler import TaskScheduler
Scope core.* module stubs to the test, not the module (#1513) Three test files (test_auth_regressions, test_auth_event_loop, test_null_owner_gates) install stubs for core.database / core.auth / src.endpoint_resolver at module-import time, so they outlive the file and are still present in sys.modules when later-collected test files try to import the real modules. The stubs are minimal (a handful of MagicMock attrs) so the import chain that follows fails with ImportError on the very next real import. test_companion_pairing also leaks, with a twist: its _DBStub subclass returns a MagicMock for *any* attribute including dunders, so the next test that does `from core.database import *` reads `__all__` as a MagicMock and dies with 'Item in __all__ must be str, not MagicMock'. Move the stub installation into an autouse fixture per file and register each stub with monkeypatch.setitem so sys.modules is restored to its pre-test state on teardown. Tighten _DBStub to refuse dunder names so __all__ stays undefined. _CAPTURED is cleared per test so the mint-token assertions see a fresh dict. Before: 3 test files fail at collection time (test_chat_image_routing, test_context_compactor, test_webhook_ssrf_resilience). After: 0 collection errors. 1365/1370 pass, 1 skip, 4 unrelated pre-existing failures (verified against origin/main baseline). Out of scope: test_task_scheduler_session_delivery:: test_session_delivery_survives_empty_database also fails in the full suite due to order-dependent state from a different test file. That's a separate leak with a different root cause.
2026-06-03 06:23:40 +01:00
# This test needs the real core.database (real SQLAlchemy Base/ChatMessage).
# test_null_owner_gates.py no longer leaks its stubs (per-test fixture cleanup
# since PR #1513), but several other files still install core.database stubs
# at module level without teardown (test_model_routes, test_companion_readonly,
# test_endpoint_probing, test_vault_password_not_in_argv). When any of those
# are collected before us, core.database is a stub and Base is a MagicMock.
# Skip in that case — the test passes correctly in isolation or when collected
# before the stubbing files.
Fix test suite: ESM module loading and stub isolation (#844) * Fix test suite: ESM loading and stub isolation (refs #605) Three targeted fixes to reduce suite failures from 9 → 1: 1. package.json: add "type": "module" so Node loads static/js/** as ES modules. Fixes 7 tests in test_compare_js.py and test_reply_recipients_js.py that fail with "SyntaxError: Unexpected token 'export'". 2. test_null_owner_gates.py: add Base and ChatMessage to the core.database stub. Without Base the scheduler test cannot import at collection time; without ChatMessage core/__init__.py fails mid-load when session_manager.py tries to import it, leaving core partially initialised in sys.modules and poisoning the auth manager migration test that runs later in the same file. 3. test_task_scheduler_session_delivery.py: skip gracefully when core.database is stubbed (Base is a MagicMock) rather than crashing. The test passes correctly when run in isolation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Scope ESM declaration to static/js/ and document isolation workaround Per review feedback on #844: 1. Move "type": "module" from root package.json to static/js/package.json. The root package.json had no type field (defaulted to CJS) and should stay that way — vendored UMD bundles in static/lib/ use require() internally and would break if Node ever tried to load them as ES modules. Node resolves the nearest package.json, so adding it in static/js/ scopes the ESM declaration to just the files the JS unit tests actually load (compare/state.js, emailLibrary/replyRecipients.js). 2. Expand the module-level skip comment in test_task_scheduler_session_delivery to document that it is a temporary isolation workaround, explain root cause (test_null_owner_gates installs a module-level sys.modules stub with no cleanup), record before/after suite numbers, and note the clean path (refactor to fixture-scoped stub). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-02 12:29:29 +10:00
if type(Base).__name__ == "MagicMock":
pytest.skip("core.database is stubbed — run this file in isolation", allow_module_level=True)
def _make_db():
engine = create_engine("sqlite:///:memory:")
Base.metadata.create_all(engine)
return sessionmaker(bind=engine)()
def _make_task():
return _types.SimpleNamespace(
id="task-1",
name="Chat Sessions Tidy",
prompt="tidy",
output_target="session",
endpoint_url=None,
model=None,
session_id=None,
owner=None,
crew_member_id=None,
)
2026-06-04 01:27:29 +02:00
def test_session_delivery_survives_empty_database(monkeypatch):
"""On a fresh/wiped database there is no session to inherit endpoint/model
from, so _resolve_defaults returns None. The delivery must still persist a
session instead of crashing on the NOT NULL constraint (issue #326)."""
2026-06-04 01:27:29 +02:00
monkeypatch.setitem(sys.modules, "core.database", cdb)
parent = sys.modules.get("core")
if parent is not None:
monkeypatch.setattr(parent, "database", cdb, raising=False)
db = _make_db()
scheduler = TaskScheduler.__new__(TaskScheduler)
scheduler._session_manager = None
asyncio.run(scheduler._deliver_task_result(_make_task(), "done", db))
sessions = db.query(DbSession).all()
assert len(sessions) == 1
assert sessions[0].endpoint_url == ""
assert sessions[0].model == ""
fix: session context drifting — messages leaking between chats (#135) (#267) * docs: add implementation plan for fixing chat context drifting (#135) * fix: make Session.history immutable + fix {}.history crash - Session.history now exposes a COPY of the internal _history list - add_message() replaces history with a fresh copy each time - get_context_messages() derives from _history directly - replace_messages() updates both _history and history - truncate_messages() updates both _history and history - _persist_message() line 207: fixed {}.history fallback crash - Added 11 tests for session isolation and edge cases Addresses #135 root cause #1: shared mutable references * fix: task scheduler uses SessionManager methods instead of overwriting sessions - Added ensure_task_session() to SessionManager (checks cache first) - Task scheduler now uses ensure_task_session() instead of direct dict assignment - Task scheduler now uses SessionManager.add_message() for message persistence - Removed direct sess_obj.history.append() that was silently losing data Addresses #135 root causes #2 and #3 * fix: add age guard to cleanup_empty_sessions — don't delete sessions <1h old Prevents the cleanup task from deleting sessions that were just created and haven't received any messages yet (message_count == 0). Addresses #135 root cause #5 * test: comprehensive session isolation tests (10/10 passing) * refactor: consolidate _session_manager into singleton pattern - Added set_session_manager_instance / get_session_manager_instance to core/models - kept backward-compat aliases (set_session_manager, get_session_manager) - session_manager.py re-exports the singleton functions - ai_interaction.set_session_manager now syncs with the core singleton - context_compactor uses get_session_manager_instance() instead of getattr hack - app.py initializes the singleton once Addresses #135 root cause #4: fragile global wiring * test: add concurrent session isolation integration tests Verifies: - Concurrent add_message to different sessions doesn't cross-contaminate - Rapid parallel writes maintain isolation - Read-write concurrent access is safe All 3 async tests pass, proving the immutable history fix works under concurrency * fix: pre-import core.models in conftest to prevent test pollution test_agent_loop.py stubs sys.modules['core.models'] = MagicMock() at module level during collection. Any test collected after it imports Session as a MagicMock. Pre-importing core.models in conftest.py before test_agent_loop.py's module-level code runs prevents this. * fix: make .history authoritative mutable list, address PR review Per review feedback: keep .history as the authoritative mutable list so existing code doing .history.pop(), .history = [...], etc. still works. Fix the cross-contamination bug by ensuring __post_init__() gives each Session its OWN unique history list (never shared). Changes: - core/models.py: .history IS the authoritative list. _history aliases it. Each Session gets its own list in __post_init__. - core/session_manager.py: add_message() delegates to Session.add_message() instead of appending directly — no double-append, single source of truth. - tests/test_session_manager.py: updated test to reflect that .history references see new messages (same list, not a snapshot). - docs/plans/2026-06-01-fix-chat-context-drifting.md: removed (not for shipping — useful design context but too much process/doc to ship). All 272 tests pass (3 pre-existing failures unrelated). * Fix session manager message persistence * Fix session history alias regressions * Fix session history aliasing and task delivery
2026-06-09 21:12:52 +08:00
def test_session_delivery_uses_in_memory_messages_with_manager(monkeypatch):
"""Manager delivery must not construct the SQLAlchemy ChatMessage model."""
monkeypatch.setitem(sys.modules, "core.database", cdb)
parent = sys.modules.get("core")
if parent is not None:
monkeypatch.setattr(parent, "database", cdb, raising=False)
class RecordingManager:
def __init__(self):
self.messages = []
def add_message(self, session_id, message):
assert isinstance(message, MemChatMessage)
self.messages.append((session_id, message))
db = _make_db()
manager = RecordingManager()
scheduler = TaskScheduler.__new__(TaskScheduler)
scheduler._session_manager = manager
task = _make_task()
task.session_id = "existing-session"
task.endpoint_url = "http://endpoint"
task.model = "test-model"
asyncio.run(scheduler._deliver_task_result(task, "done", db))
assert [message.role for _, message in manager.messages] == [
"user",
"assistant",
]
assert [message.content for _, message in manager.messages] == [
"tidy",
"done",
]
assert all(session_id == "existing-session" for session_id, _ in manager.messages)
assert all(
message.metadata == {"model": "test-model"}
for _, message in manager.messages
)