From ab8609a0caaeb113222274905f10d24efcbb9acc Mon Sep 17 00:00:00 2001 From: ciregenz Date: Fri, 31 Jul 2026 17:01:04 -0700 Subject: [PATCH] [eric] workflows: null session pointers whose session is gone, so a loss renders as empty not blank --- .../apps/workflows/reconcile_references.py | 67 +++++++++ backend/apps/workflows/scheduler.py | 3 + backend/tests/test_reference_integrity.py | 141 ++++++++++++++++++ 3 files changed, 211 insertions(+) create mode 100644 backend/apps/workflows/reconcile_references.py create mode 100644 backend/tests/test_reference_integrity.py diff --git a/backend/apps/workflows/reconcile_references.py b/backend/apps/workflows/reconcile_references.py new file mode 100644 index 00000000..ab2ccb8e --- /dev/null +++ b/backend/apps/workflows/reconcile_references.py @@ -0,0 +1,67 @@ +"""Null the workflow pointers whose session no longer exists. + +A pointer to a missing thing is worse than no pointer at all. The UI reads `edit_agent_session_id`, +asks for a session that is gone, gets nothing back, and renders a blank panel with no explanation. +No pointer renders the designed empty state instead, which is the thing a user can actually read. + +This runs once at boot. It cannot recover a deleted transcript, and it is not a substitute for +never losing one; it is the wall that keeps a loss from showing up as a broken-looking screen. +""" + +import logging +from typing import Dict, List + +from pydantic import BaseModel, ConfigDict +from typeguard import typechecked + +from backend.apps.workflows import storage +from backend.apps.workflows.owned_sessions import OWNED_SESSION_FIELDS + +logger = logging.getLogger(__name__) + + +class ReconcileReport(BaseModel): + """What the sweep found, so a silent heal still leaves a trace worth reading.""" + + model_config = ConfigDict(validate_assignment=True) + + workflows_scanned: int = 0 + pointers_cleared: int = 0 + workflows_touched: List[str] = [] + + +@typechecked +def reconcile_workflow_sessions() -> ReconcileReport: + """Clear sticky session pointers whose file is gone. Safe to run repeatedly.""" + from backend.apps.agents.manager.session.session_store import load_session_data + + report = ReconcileReport() + resolved: Dict[str, bool] = {} + # Trashed ones too, or restoring a workflow hands the user back a pointer that already dangles. + for wf in list(storage.list_workflows()) + list(storage.list_deleted_workflows()): + report.workflows_scanned += 1 + dirty = False + for field in OWNED_SESSION_FIELDS: + sid = getattr(wf, field, None) + if not isinstance(sid, str) or not sid: + continue + if sid not in resolved: + try: + resolved[sid] = load_session_data(sid) is not None + except Exception: + # Unreadable is not the same as missing; leave the pointer rather than guess. + resolved[sid] = True + if resolved[sid]: + continue + setattr(wf, field, None) + report.pointers_cleared += 1 + dirty = True + if dirty: + report.workflows_touched.append(wf.id) + storage.save_workflow(wf) + if report.pointers_cleared: + logger.info( + "reconcile: cleared %d dangling session pointer(s) across %d of %d workflow(s)", + report.pointers_cleared, len(report.workflows_touched), report.workflows_scanned, + ) + return report diff --git a/backend/apps/workflows/scheduler.py b/backend/apps/workflows/scheduler.py index 671329f0..c4d88ba6 100644 --- a/backend/apps/workflows/scheduler.py +++ b/backend/apps/workflows/scheduler.py @@ -491,6 +491,9 @@ async def start() -> None: if _loop_task is not None: return _mark_stuck_runs_failed() + # A pointer at a session that is gone renders as a blank panel; nulling it renders the empty state. + from backend.apps.workflows.reconcile_references import reconcile_workflow_sessions + reconcile_workflow_sessions() reconcile_on_startup() _loop_task = asyncio.create_task(_loop()) diff --git a/backend/tests/test_reference_integrity.py b/backend/tests/test_reference_integrity.py new file mode 100644 index 00000000..68e77503 --- /dev/null +++ b/backend/tests/test_reference_integrity.py @@ -0,0 +1,141 @@ +"""Referential integrity for workflows: no transcript outlives a hard delete, no pointer outlives +its session. + +Two failures of the same class, both seen live: + - Purge removed the workflow and its runs and left the chat transcripts on disk forever. The user + asked for an irreversible delete and the conversation survived it. + - 0 of 5 workflow chat pointers resolved in the real packaged store, so the UI read a pointer, + got nothing, and rendered a blank panel instead of an empty state. + +The last test here is the one that matters most: it pins OWNED_SESSION_FIELDS against the model, so +adding a fourth sticky session pointer and forgetting to clean it up fails CI instead of quietly +leaking a transcript six months from now. + +Run: + cd backend && .venv/bin/python -m pytest tests/test_reference_integrity.py -v +""" + +from __future__ import annotations + +import pytest + +from backend.apps.workflows import storage +from backend.apps.workflows.models import Workflow +from backend.apps.workflows.owned_sessions import ( + OWNED_SESSION_FIELDS, + REFERENCED_SESSION_FIELDS, + owned_session_ids, + purge_owned_sessions, +) +from backend.apps.workflows.reconcile_references import reconcile_workflow_sessions + + +@pytest.fixture(autouse=True) +def p_wf_env(isolated_workflows_data, reset_scheduler_state): + yield + + +def p_session_on_disk(sid: str) -> None: + from backend.apps.agents.manager.session.session_store import save_session + save_session(sid, {"id": sid, "name": "t", "messages": []}) + + +def p_session_exists(sid: str) -> bool: + from backend.apps.agents.manager.session.session_store import load_session_data + return load_session_data(sid) is not None + + +def test_owned_ids_collects_every_owned_pointer(make_wf): + wf = make_wf( + edit_agent_session_id="s-edit", + schedule_agent_session_id="s-sched", + last_test_session_id="s-test", + ) + assert sorted(owned_session_ids(wf)) == ["s-edit", "s-sched", "s-test"] + + +def test_the_originating_chat_is_never_owned(make_wf): + """source_session_id is the user's own chat that the workflow was generated from. Deleting it + on purge would destroy a real conversation nobody asked to lose.""" + wf = make_wf(source_session_id="s-users-own-chat") + assert owned_session_ids(wf) == [] + + +@pytest.mark.asyncio +async def test_purge_takes_the_transcripts_with_it(make_wf): + wf = make_wf(edit_agent_session_id="s-edit", last_test_session_id="s-test") + for sid in ("s-edit", "s-test"): + p_session_on_disk(sid) + assert p_session_exists("s-edit") + + removed = await purge_owned_sessions(wf) + + assert removed == 2 + assert not p_session_exists("s-edit"), "a hard delete must not leave the conversation behind" + assert not p_session_exists("s-test") + + +@pytest.mark.asyncio +async def test_purge_spares_the_originating_chat(make_wf): + wf = make_wf(edit_agent_session_id="s-edit", source_session_id="s-users-own-chat") + p_session_on_disk("s-edit") + p_session_on_disk("s-users-own-chat") + + await purge_owned_sessions(wf) + + assert not p_session_exists("s-edit") + assert p_session_exists("s-users-own-chat"), "the user's own chat outlives the workflow" + + +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) + + report = reconcile_workflow_sessions() + + assert report.pointers_cleared == 1 + assert report.workflows_scanned == 1 + assert storage.get_workflow(wf.id).edit_agent_session_id is None + + +def test_reconcile_leaves_a_live_pointer_alone(make_wf): + """The discriminating half. A sweep that nulls everything would destroy working history.""" + p_session_on_disk("s-alive") + wf = make_wf(edit_agent_session_id="s-alive") + storage.save_workflow(wf) + + report = reconcile_workflow_sessions() + + assert report.pointers_cleared == 0 + assert storage.get_workflow(wf.id).edit_agent_session_id == "s-alive" + + +def test_reconcile_covers_trashed_workflows(make_wf): + """Otherwise restoring from Trash hands back a pointer that already dangles.""" + from datetime import datetime + wf = make_wf(edit_agent_session_id="s-vanished", deleted_at=datetime.now()) + storage.save_workflow(wf) + + reconcile_workflow_sessions() + + assert storage.get_workflow(wf.id).edit_agent_session_id is None + + +def test_reconcile_is_idempotent(make_wf): + wf = make_wf(edit_agent_session_id="s-vanished") + storage.save_workflow(wf) + reconcile_workflow_sessions() + assert reconcile_workflow_sessions().pointers_cleared == 0 + + +def test_every_session_pointer_on_the_model_is_classified(): + """THE seal on this bug class. Any new *_session_id field on Workflow must be declared either + owned (purged with the workflow) or referenced (spared). Forgetting leaks a transcript past a + hard delete, and this fails loudly instead of letting that ship.""" + declared = set(OWNED_SESSION_FIELDS) | set(REFERENCED_SESSION_FIELDS) + on_model = {f for f in Workflow.model_fields if f.endswith("_session_id")} + missing = on_model - declared + assert not missing, ( + f"{sorted(missing)} is a session pointer nobody classified. Add it to OWNED_SESSION_FIELDS " + f"(deleted with the workflow) or REFERENCED_SESSION_FIELDS (survives it)." + )