adde94e430
* fix: closed document no longer stays active and leaks into new chats (#1160) Closing a document tab calls _detachDocFromSession: a doc with content is PATCHed to session_id="" (unlinked, session_id -> NULL, is_active stays True), an empty one is DELETEd. But the in-memory active-document pointer (tool_implementations._active_document_id) was never cleared on either path. The chat doc-injection last-resort looks up that pointer by id and injects it when `not cand.session_id or cand.session_id == session`. An unlinked doc has session_id NULL, so the stale pointer re-surfaced a closed document in later, unrelated chats — the agent kept reading/suggesting edits to a doc the user had closed. Fix: add clear_active_document(doc_id) and call it when a document is unlinked (PATCH session_id="") or deleted, so the pointer no longer resurrects a closed document. clear_active_document only clears when the id matches (or no id), so a different active doc is left untouched. Covered by tests/test_active_document_clear.py (4 cases). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: add route-level regression for #1160 (detach/delete clears active doc) Per review: prove the actual API path, not just the helper. Drives PATCH /api/document/{id} (session_id="") and DELETE /api/document/{id} through TestClient against a temp SQLite DB under real owner routing, and asserts get_active_document() is cleared (and untouched when a different document is closed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: make #1160 route regression hang-proof and dev-DB-independent The route test could hang in other environments: it set DATABASE_URL at import time, which is ignored if core.database was already imported, so it fell back to the real dev DB and could contend for its locks (maintainer saw it hang, exit 124). Rebind to a DEDICATED temporary SQLite engine (NullPool) and patch the document route module's SessionLocal to it via an autouse fixture — so the test never touches the dev DB and is independent of import order. Runs in ~0.3s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: drive #1160 route regression without TestClient (fixes local hang) The route test used Starlette TestClient (middleware app + threadpool), which hung in the maintainer's environment. Rework it to call the async route handlers directly — extracted from the router — with a minimal fake request against a temp-SQLite-patched SessionLocal. Same real coverage (handler + DB + owner routing), but it completes reliably (~0.3s) with no TestClient/threadpool. Verified the maintainer's exact batch now passes: pytest tests/test_document_close_clears_active_route.py \ tests/test_active_document_clear.py \ tests/test_document_tool_owner_scope.py -> 14 passed Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
40 lines
1.3 KiB
Python
40 lines
1.3 KiB
Python
"""Issue #1160 — a closed document must not stay 'active' and leak into new chats.
|
|
|
|
Closing a document tab detaches it (session_id -> NULL) or deletes it, but the
|
|
in-memory active-document pointer was never cleared, so the last-resort doc
|
|
injection re-surfaced the closed doc in later, unrelated chats. The document
|
|
routes now call clear_active_document() on detach/delete; this pins that helper.
|
|
"""
|
|
|
|
from src.tool_implementations import (
|
|
set_active_document,
|
|
get_active_document,
|
|
clear_active_document,
|
|
)
|
|
|
|
|
|
def test_clear_matching_id_resets_pointer():
|
|
set_active_document("doc-123")
|
|
assert get_active_document() == "doc-123"
|
|
assert clear_active_document("doc-123") is True
|
|
assert get_active_document() is None
|
|
|
|
|
|
def test_clear_non_matching_id_leaves_other_active_doc():
|
|
set_active_document("doc-abc")
|
|
# Closing a DIFFERENT document must not clobber the currently active one.
|
|
assert clear_active_document("doc-xyz") is False
|
|
assert get_active_document() == "doc-abc"
|
|
|
|
|
|
def test_clear_without_id_clears_unconditionally():
|
|
set_active_document("doc-abc")
|
|
assert clear_active_document() is True
|
|
assert get_active_document() is None
|
|
|
|
|
|
def test_clear_when_already_none_is_safe():
|
|
set_active_document(None)
|
|
assert clear_active_document("doc-123") is False
|
|
assert get_active_document() is None
|