diff --git a/backend/apps/workflows/owned_sessions.py b/backend/apps/workflows/owned_sessions.py index 8a6d11d2..d63668a3 100644 --- a/backend/apps/workflows/owned_sessions.py +++ b/backend/apps/workflows/owned_sessions.py @@ -42,26 +42,49 @@ def owned_session_ids(wf: Workflow) -> List[str]: @typechecked -async def purge_owned_sessions(wf: Workflow) -> int: - """Close and delete the workflow's own chats. Returns how many were removed. - - Best-effort per session: one unreadable file must not strand the rest, because a partial purge - is the state that leaves a transcript behind. - """ +async def drop_session(sid: str) -> bool: + """Close one chat and delete its file. Best effort: one unreadable file must not strand the rest.""" import logging from backend.apps.agents.manager.session.session_store import delete_session_file logger = logging.getLogger(__name__) + try: + from backend.apps.agents.agent_manager import agent_manager + await agent_manager.close_session(sid) + except Exception: + logger.debug("could not close session %s", sid, exc_info=True) + try: + delete_session_file(sid) + return True + except Exception: + logger.warning("could not delete session file %s", sid, exc_info=True) + return False + + +@typechecked +async def purge_owned_sessions(wf: Workflow) -> int: + """Close and delete the workflow's own chats. Returns how many were removed. + + Best-effort per session, because a partial purge is the state that leaves a transcript behind. + """ removed = 0 for sid in owned_session_ids(wf): - try: - from backend.apps.agents.agent_manager import agent_manager - await agent_manager.close_session(sid) - except Exception: - logger.debug("could not close session %s during purge", sid, exc_info=True) - try: - delete_session_file(sid) + if await drop_session(sid): removed += 1 - except Exception: - logger.warning("could not delete session file %s during purge", sid, exc_info=True) return removed + + +@typechecked +async def retire_previous_test_session(wf: Workflow) -> None: + """Drop the old test chat before a new one takes its place. + + `last_test_session_id` holds exactly one pointer, so a second test used to strand the first + session: an orphan card sitting on the user's canvas that nothing references any more, and a + transcript that outlives a hard delete of the workflow, which is the very leak this module + exists to prevent. + """ + sid = getattr(wf, "last_test_session_id", None) + if not isinstance(sid, str) or not sid: + return + await drop_session(sid) + wf.last_test_session_id = None diff --git a/backend/apps/workflows/workflows.py b/backend/apps/workflows/workflows.py index ac11df7e..8efefa4b 100644 --- a/backend/apps/workflows/workflows.py +++ b/backend/apps/workflows/workflows.py @@ -1254,7 +1254,10 @@ async def test_run_workflow(workflow_id: str, body: dict): set_workflow_approval_step, ) from backend.apps.workflows import executor + from backend.apps.workflows.owned_sessions import retire_previous_test_session + # One test card at a time: the previous test's chat is about to lose the only pointer to it, so retire it now rather than leave an orphan card on the canvas. + await retire_previous_test_session(wf) # Like a real run, the test must attach to the dashboard the user is watching, else its browser tools have no card to drive and the test "runs" but visibly does nothing. Prefer the live active dashboard over the workflow's stored home. from backend.apps.agents.core.ws_manager import ws_manager as p_wsm test_dashboard_id = p_wsm.active_dashboard_id or executor.resolve_workflow_dashboard_id(wf) diff --git a/backend/tests/test_reference_integrity.py b/backend/tests/test_reference_integrity.py index 68e77503..903e4004 100644 --- a/backend/tests/test_reference_integrity.py +++ b/backend/tests/test_reference_integrity.py @@ -26,6 +26,7 @@ from backend.apps.workflows.owned_sessions import ( REFERENCED_SESSION_FIELDS, owned_session_ids, purge_owned_sessions, + retire_previous_test_session, ) from backend.apps.workflows.reconcile_references import reconcile_workflow_sessions @@ -87,6 +88,31 @@ async def test_purge_spares_the_originating_chat(make_wf): assert p_session_exists("s-users-own-chat"), "the user's own chat outlives the workflow" +@pytest.mark.asyncio +async def test_a_second_test_run_retires_the_first(make_wf): + """Live: building one workflow ran five tests and left five orphan cards on the canvas. Each new + test overwrites last_test_session_id, so the displaced chat had no pointer left and would have + survived a hard delete of the workflow.""" + wf = make_wf(last_test_session_id="s-test-1") + p_session_on_disk("s-test-1") + + await retire_previous_test_session(wf) + + assert wf.last_test_session_id is None + assert not p_session_exists("s-test-1"), "the displaced test chat must not outlive its pointer" + + +@pytest.mark.asyncio +async def test_retiring_with_no_previous_test_is_a_no_op(make_wf): + """The discriminating half: the first test of a workflow must not try to drop anything.""" + wf = make_wf(edit_agent_session_id="s-edit") + p_session_on_disk("s-edit") + + await retire_previous_test_session(wf) + + assert p_session_exists("s-edit"), "retiring a test must never touch the edit chat" + + def test_reconcile_nulls_a_pointer_whose_session_is_gone(make_wf): wf = make_wf(edit_agent_session_id="s-vanished") storage.save_workflow(wf)