mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-05 17:27:42 +02:00
[eric] workflows: null session pointers whose session is gone, so a loss renders as empty not blank
This commit is contained in:
@@ -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
|
||||
@@ -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())
|
||||
|
||||
|
||||
@@ -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)."
|
||||
)
|
||||
Reference in New Issue
Block a user