diff --git a/routes/history_routes.py b/routes/history_routes.py index 378fab35f..59ed6674e 100644 --- a/routes/history_routes.py +++ b/routes/history_routes.py @@ -490,7 +490,13 @@ def setup_history_routes(session_manager) -> APIRouter: # Copy messages up to keep_count msgs_to_copy = source.history[:keep_count] for msg in msgs_to_copy: - new_session.add_message(ChatMessage(msg.role, msg.content, msg.metadata)) + # Copy the metadata dict. Sharing it would let the fork's + # persistence (add_message -> _persist_message stamps + # _db_id/timestamp onto the dict) mutate the SOURCE session's + # in-memory messages, corrupting their _db_id and breaking + # edit/delete-by-id on the original conversation. + meta = dict(msg.metadata) if isinstance(msg.metadata, dict) else None + new_session.add_message(ChatMessage(msg.role, msg.content, meta)) try: from src.event_bus import fire_event fire_event("session_created", getattr(source, 'owner', None)) diff --git a/tests/test_fork_session_metadata.py b/tests/test_fork_session_metadata.py new file mode 100644 index 000000000..cd278da1d --- /dev/null +++ b/tests/test_fork_session_metadata.py @@ -0,0 +1,84 @@ +"""Forking a session must not mutate the source session's messages. + +ChatMessage.metadata is a dict. add_message() -> _persist_message() stamps +_db_id (and timestamp) onto that dict in place. The fork handler used to pass +the source message's metadata dict by reference into the new session, so +persisting the fork rewrote the SOURCE messages' _db_id — breaking +edit/delete-by-id on the original conversation. The fork must copy the dict. +""" +import asyncio +from types import SimpleNamespace + +from core.models import ChatMessage +import routes.history_routes as mod + + +class _FakeSession: + def __init__(self, name="", owner=None): + self.name = name + self.owner = owner + self.endpoint_url = "" + self.model = "" + self.history = [] + + def add_message(self, message): + # Mirror _persist_message: stamp the in-memory message's metadata. + if message.metadata is None: + message.metadata = {} + message.metadata["_db_id"] = f"new-{len(self.history)}" + self.history.append(message) + + +class _FakeSessionManager: + def __init__(self, source): + self.sessions = {"src-id": source} + self.created = None + + def create_session(self, session_id=None, name=None, endpoint_url=None, + model=None, rag=False, owner=None): + self.created = _FakeSession(name=name, owner=owner) + return self.created + + def save_sessions(self): + pass + + +def _fork_handler(router): + for route in router.routes: + if "/fork" in getattr(route, "path", "") and "POST" in getattr(route, "methods", set()): + return route.endpoint + raise AssertionError("fork route not found") + + +def test_fork_does_not_corrupt_source_message_metadata(monkeypatch): + monkeypatch.setattr(mod, "_verify_session_owner", lambda *a, **k: None) + + source = _FakeSession(name="Original", owner="alice") + source.history = [ + ChatMessage("user", "hi", {"_db_id": "src-0"}), + ChatMessage("assistant", "yo", {"_db_id": "src-1"}), + ] + sm = _FakeSessionManager(source) + + req = SimpleNamespace() + + async def _json(): + return {"keep_count": 2} + + req.json = _json + + router = mod.setup_history_routes(sm) + fork = _fork_handler(router) + result = asyncio.run(fork(request=req, session_id="src-id")) + + assert result["status"] == "ok" + assert result["kept"] == 2 + + # The forked session got its own metadata dicts... + new_session = sm.created + assert new_session.history[0].metadata is not source.history[0].metadata + assert new_session.history[1].metadata is not source.history[1].metadata + + # ...and the source session's _db_id values are untouched. + assert source.history[0].metadata["_db_id"] == "src-0" + assert source.history[1].metadata["_db_id"] == "src-1"