* 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
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.
* 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>
* fix: coerce null endpoint_url when delivering task result to a session
* fix: also coerce null model so the session insert satisfies NOT NULL
* test: cover task session delivery on an empty database
Joshua Valderrama