Files
odysseus/tests/test_document_close_clears_active_route.py
lekt8 adde94e430 fix: closed document stays active & leaks into new chats (#1160) (#1238)
* 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>
2026-06-03 01:47:13 +09:00

94 lines
3.3 KiB
Python

"""Issue #1160 — route-level regression for clearing the active-document pointer.
Exercises the REAL ``PATCH /api/document/{id}`` (session_id="") and
``DELETE /api/document/{id}`` handlers, proving that closing a document's tab
(detach or delete) clears the in-memory active-document pointer under the actual
owner/session routing — not just the helper in isolation.
Calls the route handler callables DIRECTLY (extracted from the router) instead of
through Starlette's TestClient. The TestClient path spun up a middleware app +
threadpool that could hang in some environments; calling the async handler with a
minimal fake request keeps the same real coverage (handler + DB + owner routing)
while completing reliably everywhere.
"""
import tempfile
import uuid
from types import SimpleNamespace
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
from sqlalchemy.pool import NullPool
from unittest.mock import MagicMock
import core.database as cdb
import routes.document_routes as droutes
from core.database import Document
from core.database import Session as DbSession
from routes.document_helpers import DocumentPatch
from src.tool_implementations import set_active_document, get_active_document
_TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
_ENGINE = create_engine(
f"sqlite:///{_TMPDB.name}",
connect_args={"check_same_thread": False},
poolclass=NullPool,
)
cdb.Base.metadata.create_all(_ENGINE)
_TS = sessionmaker(bind=_ENGINE, autoflush=False, autocommit=False)
droutes.SessionLocal = _TS # route handlers resolve SessionLocal at call time
def _req():
return SimpleNamespace(state=SimpleNamespace(current_user="tester"))
def _endpoint(method, path):
router = droutes.setup_document_routes(MagicMock(), None)
for r in router.routes:
if getattr(r, "path", None) == path and method in getattr(r, "methods", set()):
return r.endpoint
raise RuntimeError(f"{method} {path} not found")
def _make_doc():
sid = "s-" + uuid.uuid4().hex[:8]
db = _TS()
try:
db.add(DbSession(id=sid, owner="tester", name="s", model="m", endpoint_url="http://x"))
doc = Document(
id=str(uuid.uuid4()), session_id=sid, title="t",
language="markdown", current_content="hi", version_count=1,
is_active=True, owner="tester",
)
db.add(doc)
db.commit()
return doc.id
finally:
db.close()
async def test_patch_unlink_clears_active_document():
patch_document = _endpoint("PATCH", "/api/document/{doc_id}")
doc_id = _make_doc()
set_active_document(doc_id)
await patch_document(_req(), doc_id, DocumentPatch(session_id=""))
assert get_active_document() is None
async def test_delete_clears_active_document():
delete_document = _endpoint("DELETE", "/api/document/{doc_id}")
doc_id = _make_doc()
set_active_document(doc_id)
await delete_document(_req(), doc_id)
assert get_active_document() is None
async def test_unlinking_a_different_doc_leaves_pointer():
patch_document = _endpoint("PATCH", "/api/document/{doc_id}")
active_id = _make_doc()
other_id = _make_doc()
set_active_document(active_id)
await patch_document(_req(), other_id, DocumentPatch(session_id=""))
assert get_active_document() == active_id