From 9e13ec9e031852b555378a89a586c49e10cac525 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Fri, 3 Jul 2026 22:21:34 -0400 Subject: [PATCH 1/7] fix(#5498): prevent deleted WebUI sessions from resurrecting from state.db --- api/models.py | 104 +++++++++++++++- api/routes.py | 29 ++++- api/session_recovery.py | 101 +++++++++++++++- tests/test_5307_subagent_child_transcript.py | 10 +- tests/test_chat_start_claim_cli_session.py | 24 ++++ tests/test_issue2057_worktree_lifecycle.py | 36 ++++++ .../test_session_db_sidecar_reconciliation.py | 111 ++++++++++++++++++ 7 files changed, 405 insertions(+), 10 deletions(-) diff --git a/api/models.py b/api/models.py index d5408c9f..ceae3697 100644 --- a/api/models.py +++ b/api/models.py @@ -160,6 +160,7 @@ _SESSION_INDEX_REBUILD_THREAD_TARGET: tuple[Path, Path] | None = None # WebUI sidebar polling path is single-process) but must wrap the WHOLE # load-modify-write/unlink sequence in both helpers. _WEBUI_ZERO_MESSAGE_ORPHAN_TOMBSTONE_LOCK = threading.Lock() +_WEBUI_DELETED_SESSION_TOMBSTONE_LOCK = threading.Lock() # Path-safety contract for session IDs. Accept alphanumerics, underscore, and # hyphen so API/gateway-issued ids (``api-*``, ``reachy-voice-*``) round-trip @@ -513,6 +514,8 @@ def prune_session_from_index(session_id: str) -> None: WEBUI_ZERO_MESSAGE_ORPHAN_TOMBSTONE_CAP = 500 WEBUI_ZERO_MESSAGE_ORPHAN_TOMBSTONE_VERSION = 1 +WEBUI_DELETED_SESSION_TOMBSTONE_CAP = 1000 +WEBUI_DELETED_SESSION_TOMBSTONE_VERSION = 1 def _webui_zero_message_orphan_tombstone_file() -> "Path": @@ -676,6 +679,98 @@ def _clear_webui_zero_message_orphan_tombstone(sid: str) -> None: ) +def _webui_deleted_session_tombstone_file() -> "Path": + return SESSION_DIR / "_deleted_webui_sessions.json" + + +def _load_webui_deleted_session_tombstone() -> frozenset[str]: + p = _webui_deleted_session_tombstone_file() + if not p.exists(): + return frozenset() + try: + raw = json.loads(p.read_text(encoding='utf-8')) + except Exception: + logger.debug("Failed to load webui deleted-session tombstone", exc_info=True) + return frozenset() + if not isinstance(raw, dict): + return frozenset() + try: + if int(raw.get("version", 0)) != WEBUI_DELETED_SESSION_TOMBSTONE_VERSION: + return frozenset() + except (TypeError, ValueError): + return frozenset() + ids = raw.get("ids", []) + if not isinstance(ids, list): + return frozenset() + return frozenset( + str(sid).strip() for sid in ids if str(sid or "").strip() + ) + + +def _save_webui_deleted_session_tombstone(ids) -> None: + try: + sorted_ids = sorted(set( + str(sid).strip() for sid in (ids or []) if str(sid or "").strip() + )) + except TypeError: + return + if len(sorted_ids) > WEBUI_DELETED_SESSION_TOMBSTONE_CAP: + sorted_ids = sorted_ids[-WEBUI_DELETED_SESSION_TOMBSTONE_CAP:] + payload = { + "version": WEBUI_DELETED_SESSION_TOMBSTONE_VERSION, + "ids": sorted_ids, + } + p = _webui_deleted_session_tombstone_file() + _tmp = None + try: + SESSION_DIR.mkdir(parents=True, exist_ok=True) + _tmp = p.with_suffix( + f'.tmp.{os.getpid()}.{threading.current_thread().ident}' + ) + with open(_tmp, 'w', encoding='utf-8') as f: + json.dump(payload, f, ensure_ascii=False, indent=2) + f.flush() + os.fsync(f.fileno()) + os.replace(_tmp, p) + except Exception: + logger.debug("Failed to save webui deleted-session tombstone", exc_info=True) + if _tmp is not None: + try: + _tmp.unlink(missing_ok=True) + except Exception: + pass + + +def _record_webui_deleted_session_tombstone(sid: str) -> None: + sid = str(sid or "").strip() + if not sid: + return + with _WEBUI_DELETED_SESSION_TOMBSTONE_LOCK: + current = set(_load_webui_deleted_session_tombstone()) + if sid in current: + return + current.add(sid) + _save_webui_deleted_session_tombstone(current) + + +def _clear_webui_deleted_session_tombstone(sid: str) -> None: + sid = str(sid or "").strip() + if not sid: + return + with _WEBUI_DELETED_SESSION_TOMBSTONE_LOCK: + current = set(_load_webui_deleted_session_tombstone()) + if sid not in current: + return + current.discard(sid) + if current: + _save_webui_deleted_session_tombstone(current) + return + try: + _webui_deleted_session_tombstone_file().unlink(missing_ok=True) + except Exception: + logger.debug("Failed to remove empty webui deleted-session tombstone", exc_info=True) + + def _active_stream_ids(): with STREAMS_LOCK: active_ids = set(STREAMS.keys()) @@ -1190,9 +1285,10 @@ class Session: if self.messages: try: _clear_webui_zero_message_orphan_tombstone(self.session_id) + _clear_webui_deleted_session_tombstone(self.session_id) except Exception: logger.debug( - "Failed to clear webui zero-message orphan tombstone for %s", + "Failed to clear webui tombstone for %s", self.session_id, exc_info=True, ) @@ -3253,9 +3349,10 @@ def new_session(workspace=None, model=None, profile=None, model_provider=None, p # must never block new-session creation. try: _clear_webui_zero_message_orphan_tombstone(s.session_id) + _clear_webui_deleted_session_tombstone(s.session_id) except Exception: logger.debug( - "Failed to clear webui zero-message orphan tombstone for %s", + "Failed to clear webui tombstone for %s", s.session_id, exc_info=True, ) @@ -4741,9 +4838,10 @@ def import_cli_session( # an import. try: _clear_webui_zero_message_orphan_tombstone(s.session_id) + _clear_webui_deleted_session_tombstone(s.session_id) except Exception: logger.debug( - "Failed to clear webui zero-message orphan tombstone for %s", + "Failed to clear webui tombstone for %s", s.session_id, exc_info=True, ) diff --git a/api/routes.py b/api/routes.py index 3211bc70..303baecc 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7146,6 +7146,13 @@ def _session_index_marks_was_webui(sid: str) -> bool: return False +def _session_deleted_tombstone_marks_was_webui(sid: str) -> bool: + try: + return sid in _load_webui_deleted_session_tombstone() + except Exception: + return False + + def _state_db_session_source(sid: str) -> str: """Return the lowercased ``sessions.source`` for ``sid`` from state.db. @@ -7380,7 +7387,13 @@ def _claim_or_synthesize_cli_session(sid: str, cli_meta: dict = None): if not is_safe_session_id(sid): return None, "invalid_sid" - if _session_index_marks_was_webui(sid) and not _is_subagent_child_session_id(sid): + if ( + ( + _session_index_marks_was_webui(sid) + or _session_deleted_tombstone_marks_was_webui(sid) + ) + and not _is_subagent_child_session_id(sid) + ): # A delegated subagent child (source='subagent' in state.db) can be # registered in the WebUI index as a webui/fork/blank-source row (it # shares the parent's lineage) yet have no WebUI sidecar of its own. @@ -8584,6 +8597,8 @@ from api.models import ( _load_webui_zero_message_orphan_tombstone, _record_webui_zero_message_orphan_tombstone, _clear_webui_zero_message_orphan_tombstone, + _load_webui_deleted_session_tombstone, + _record_webui_deleted_session_tombstone, ensure_cron_project, _profile_has_user_projects, is_cron_session, @@ -13593,15 +13608,25 @@ def handle_post(handler, parsed) -> bool: p.relative_to(SESSION_DIR.resolve()) except Exception: return bad(handler, "Invalid session_id", 400) + sidecar_deleted = False try: p.unlink(missing_ok=True) - p.with_suffix('.json.bak').unlink(missing_ok=True) except Exception: logger.debug("Failed to unlink session file %s", p) + sidecar_deleted = not p.exists() + try: + p.with_suffix('.json.bak').unlink(missing_ok=True) + except Exception: + logger.debug("Failed to unlink session backup file %s", p.with_suffix('.json.bak')) try: prune_session_from_index(sid) except Exception: logger.debug("Failed to prune deleted session from index: %s", sid, exc_info=True) + if sidecar_deleted: + try: + _record_webui_deleted_session_tombstone(sid) + except Exception: + logger.debug("Failed to tombstone deleted WebUI session %s", sid, exc_info=True) try: from api.upload import _session_attachment_dir diff --git a/api/session_recovery.py b/api/session_recovery.py index e806ecf0..0002263e 100644 --- a/api/session_recovery.py +++ b/api/session_recovery.py @@ -383,6 +383,9 @@ def _read_state_db_missing_sidecar_rows( sid = str(data.get('id') or '').strip() if not sid or (session_dir / f"{sid}.json").exists(): continue + tombstoned = _marks_deleted_webui_session(session_dir, sid) + if tombstoned and not include_empty: + continue message_rows: list[dict] = [] if {'session_id', 'role', 'content'}.issubset(message_cols): order = "timestamp, id" if 'timestamp' in message_cols and 'id' in message_cols else "rowid" @@ -402,6 +405,8 @@ def _read_state_db_missing_sidecar_rows( continue data['messages'] = message_rows data['_state_db_empty_messages'] = not message_rows + if tombstoned: + data['_state_db_deleted_webui_tombstone'] = True rows.append(data) return rows except Exception as exc: @@ -484,6 +489,9 @@ def recover_missing_sidecars_from_state_db(session_dir: Path, state_db_path: Pat sid = str(row.get('id') or '').strip() if not sid: continue + if row.get('_state_db_deleted_webui_tombstone'): + details.append({'session_id': sid, 'materialized': False, 'skipped': 'deleted_webui_tombstone'}) + continue target = session_dir / f"{sid}.json" if target.exists(): continue @@ -565,6 +573,75 @@ def _read_index_session_ids(index_path: Path) -> set[str]: return ids +def _index_marks_deleted_webui_session(session_dir: Path, sid: str) -> bool: + """Return True when _index.json marks sid as a deleted WebUI session.""" + if not sid or (session_dir / f"{sid}.json").exists(): + return False + index_path = session_dir / '_index.json' + if not index_path.exists(): + return False + try: + data = json.loads(index_path.read_text(encoding='utf-8')) + except (OSError, json.JSONDecodeError, ValueError): + return False + if not isinstance(data, list): + return False + for entry in data: + if not isinstance(entry, dict) or entry.get('session_id') != sid: + continue + srcs = [ + str(entry.get('source_tag') or '').strip().lower(), + str(entry.get('raw_source') or '').strip().lower(), + str(entry.get('session_source') or '').strip().lower(), + ] + explicit = [src for src in srcs if src] + if any(src in ('webui', 'fork') for src in explicit): + return True + if explicit: + return False + is_cli = entry.get('is_cli_session') is True + is_read_only = bool(entry.get('read_only') or entry.get('is_read_only')) + return not (is_cli or is_read_only) + return False + + +def _durable_tombstone_marks_deleted_webui_session(session_dir: Path, sid: str) -> bool: + """Return True when the durable WebUI delete tombstone contains sid.""" + if not sid or (session_dir / f"{sid}.json").exists(): + return False + try: + from api import models as _models + + if Path(_models.SESSION_DIR).resolve() == session_dir.resolve(): + return sid in _models._load_webui_deleted_session_tombstone() + except Exception: + pass + tombstone_path = session_dir / '_deleted_webui_sessions.json' + try: + raw = json.loads(tombstone_path.read_text(encoding='utf-8')) + except (OSError, json.JSONDecodeError, ValueError): + return False + if not isinstance(raw, dict): + return False + try: + version = int(raw.get('version', 0)) + except (TypeError, ValueError): + return False + if version != 1: + return False + ids = raw.get('ids') + if not isinstance(ids, list): + return False + return sid in {str(value).strip() for value in ids if str(value or '').strip()} + + +def _marks_deleted_webui_session(session_dir: Path, sid: str) -> bool: + return ( + _index_marks_deleted_webui_session(session_dir, sid) + or _durable_tombstone_marks_deleted_webui_session(session_dir, sid) + ) + + def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) -> dict: """Read-only audit of session recovery state. @@ -583,6 +660,16 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) items: list[dict] = [] live_paths = sorted(p for p in session_dir.glob('*.json') if not p.name.startswith('_')) live_ids = {p.stem for p in live_paths} + state_db_missing_rows = _read_state_db_missing_sidecar_rows( + session_dir, + state_db_path, + include_empty=True, + ) + state_db_deleted_webui_ids = { + str(row.get('id') or '') + for row in state_db_missing_rows + if row.get('_state_db_deleted_webui_tombstone') + } for live_path in live_paths: status = inspect_session_recovery_status(live_path) @@ -624,6 +711,8 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) if index_path.exists(): index_ids = _read_index_session_ids(index_path) for session_id in sorted(index_ids - live_ids): + if session_id in state_db_deleted_webui_ids: + continue items.append(_new_audit_item( session_id, "index_missing_file", "repairable", "rebuild_index" )) @@ -633,8 +722,18 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) _msg_count(session_dir / f"{session_id}.json"), -1, )) - for row in _read_state_db_missing_sidecar_rows(session_dir, state_db_path, include_empty=True): + for row in state_db_missing_rows: sid = str(row.get('id') or '') + if row.get('_state_db_deleted_webui_tombstone'): + items.append(_new_audit_item( + sid, + "state_db_deleted_webui_tombstone", + "unsafe_to_repair", + "deleted_session_skipped", + -1, + -1, + )) + continue if row.get('_state_db_empty_messages'): items.append(_new_audit_item( sid, diff --git a/tests/test_5307_subagent_child_transcript.py b/tests/test_5307_subagent_child_transcript.py index b5ee2b8a..be2d7245 100644 --- a/tests/test_5307_subagent_child_transcript.py +++ b/tests/test_5307_subagent_child_transcript.py @@ -162,10 +162,12 @@ def test_was_webui_gate_excludes_subagent_children(): m = re.search(r"\n(?:def |class )", src[start + 1:]) block = src[start:(start + 1 + m.start()) if m else len(src)] assert "_session_index_marks_was_webui(sid)" in block - assert re.search( - r"_session_index_marks_was_webui\(sid\)\s+and\s+not\s+_is_subagent_child_session_id\(sid\)", - block, - ), "was_webui and not subagent-child must gate the same 404 return (#5307)" + assert "_session_deleted_tombstone_marks_was_webui(sid)" in block + guard_start = block.index("if (") + guard = block[guard_start:block.index("return None, \"was_webui\"", guard_start)] + assert "_is_subagent_child_session_id(sid)" in guard, ( + "all was_webui predicates must share the subagent-child exclusion (#5307)" + ) def test_sessions_js_open_handlers_keep_isexternalsession_contract(): diff --git a/tests/test_chat_start_claim_cli_session.py b/tests/test_chat_start_claim_cli_session.py index c3face53..2bcef6f0 100644 --- a/tests/test_chat_start_claim_cli_session.py +++ b/tests/test_chat_start_claim_cli_session.py @@ -374,6 +374,30 @@ def test_helper_returns_was_webui_for_deleted_webui_session( assert reason == "was_webui" +def test_helper_returns_was_webui_for_durable_deleted_tombstone_without_index( + routes_module, isolated_state_db +): + """A full WebUI delete tombstone must keep the 404 self-heal contract even + after /api/session/delete prunes _index.json before state.db cleanup fails. + """ + import api.models as _models + + sid = "webui-deleted-db-survives" + _make_state_db( + isolated_state_db["db"], + sid, + message_count=2, + title="Deleted WebUI", + source="webui", + ) + _models._record_webui_deleted_session_tombstone(sid) + + sess, reason = routes_module._claim_or_synthesize_cli_session(sid) + + assert sess is None + assert reason == "was_webui" + + def test_helper_keeps_cli_orphan_with_blank_source( routes_module, tmp_path, monkeypatch, isolated_state_db ): diff --git a/tests/test_issue2057_worktree_lifecycle.py b/tests/test_issue2057_worktree_lifecycle.py index c908f48f..6f86f3be 100644 --- a/tests/test_issue2057_worktree_lifecycle.py +++ b/tests/test_issue2057_worktree_lifecycle.py @@ -1,4 +1,5 @@ from types import SimpleNamespace +from pathlib import Path import api.models as models import api.routes as routes @@ -67,6 +68,41 @@ def test_delete_worktree_session_reports_retained_worktree_without_cleanup(tmp_p assert worktree.exists(), "session delete must not remove the git worktree directory" +def test_delete_session_records_tombstone_when_state_db_delete_fails(tmp_path, monkeypatch): + session_dir = _isolate_session_store(tmp_path, monkeypatch) + sid = "dbfaildelete1" + session = Session( + session_id=sid, + title="Delete failure", + messages=[{"role": "user", "content": "keep deleted"}], + ) + session.save() + (session_dir / f"{sid}.json.bak").write_text("backup", encoding="utf-8") + captured = _capture_post(monkeypatch, {"session_id": sid}) + monkeypatch.setattr(routes, "_lookup_cli_session_metadata", lambda value: {}) + monkeypatch.setattr(routes, "_is_messaging_session_id", lambda value: False) + + def fail_delete(value): + raise RuntimeError("state.db locked") + + real_unlink = Path.unlink + + def fail_backup_unlink(path, *args, **kwargs): + if path.name == f"{sid}.json.bak": + raise PermissionError("backup locked") + return real_unlink(path, *args, **kwargs) + + monkeypatch.setattr(models, "delete_cli_session", fail_delete) + monkeypatch.setattr(Path, "unlink", fail_backup_unlink) + + assert routes.handle_post(object(), SimpleNamespace(path="/api/session/delete")) is True + + assert captured["status"] == 200 + assert captured["payload"]["ok"] is True + assert not (session_dir / f"{sid}.json").exists() + assert sid in models._load_webui_deleted_session_tombstone() + + def test_archive_worktree_session_reports_retained_worktree_without_cleanup(tmp_path, monkeypatch): _isolate_session_store(tmp_path, monkeypatch) session, worktree = _worktree_session(tmp_path, "wtarchive1") diff --git a/tests/test_session_db_sidecar_reconciliation.py b/tests/test_session_db_sidecar_reconciliation.py index 0c85d8db..a6b1e9dc 100644 --- a/tests/test_session_db_sidecar_reconciliation.py +++ b/tests/test_session_db_sidecar_reconciliation.py @@ -26,6 +26,10 @@ def _make_state_db(path, *, sid="state_only_001", source="webui", messages=2): return sid +def _write_index(path, entries): + (path / "_index.json").write_text(json.dumps(entries), encoding="utf-8") + + def test_recover_missing_sidecars_from_state_db_materializes_webui_row(tmp_path): sid = _make_state_db(tmp_path / "state.db") @@ -44,6 +48,78 @@ def test_recover_missing_sidecars_from_state_db_materializes_webui_row(tmp_path) assert [m["content"] for m in data["messages"]] == ["message 1", "message 2"] +def test_recover_missing_sidecars_from_state_db_skips_deleted_webui_tombstone(tmp_path): + sid = _make_state_db(tmp_path / "state.db", sid="deleted_webui_001") + _write_index(tmp_path, [ + { + "session_id": sid, + "source_tag": "webui", + "raw_source": "webui", + "session_source": "webui", + } + ]) + + result = recover_missing_sidecars_from_state_db(tmp_path, tmp_path / "state.db") + + assert result["materialized"] == 0 + assert not (tmp_path / f"{sid}.json").exists() + + +def test_audit_reports_deleted_webui_tombstone_is_unsafe(tmp_path): + sid = _make_state_db(tmp_path / "state.db", sid="deleted_webui_001") + _write_index(tmp_path, [ + { + "session_id": sid, + "source_tag": "webui", + "raw_source": "webui", + "session_source": "webui", + } + ]) + + report = audit_session_recovery(tmp_path, state_db_path=tmp_path / "state.db") + + assert any( + item["session_id"] == sid + and item["kind"] == "state_db_deleted_webui_tombstone" + and item["category"] == "unsafe_to_repair" + and item["recommendation"] == "deleted_session_skipped" + for item in report["items"] + ) + assert not any( + item["session_id"] == sid and item["kind"] == "state_db_missing_sidecar" + for item in report["items"] + ) + assert not any( + item["session_id"] == sid and item["category"] == "repairable" + for item in report["items"] + ) + + +def test_recover_missing_sidecars_skips_durable_delete_tombstone_without_index(tmp_path, monkeypatch): + import api.models as _m + + sid = _make_state_db(tmp_path / "state.db", sid="durable_deleted_001") + monkeypatch.setattr(_m, "SESSION_DIR", tmp_path) + _m._record_webui_deleted_session_tombstone(sid) + + result = recover_missing_sidecars_from_state_db(tmp_path, tmp_path / "state.db") + report = audit_session_recovery(tmp_path, state_db_path=tmp_path / "state.db") + + assert result["materialized"] == 0 + assert not (tmp_path / f"{sid}.json").exists() + assert any( + item["session_id"] == sid + and item["kind"] == "state_db_deleted_webui_tombstone" + and item["category"] == "unsafe_to_repair" + and item["recommendation"] == "deleted_session_skipped" + for item in report["items"] + ) + assert not any( + item["session_id"] == sid and item["category"] == "repairable" + for item in report["items"] + ) + + def test_recover_missing_sidecars_from_state_db_skips_existing_sidecar(tmp_path): sid = _make_state_db(tmp_path / "state.db") existing = tmp_path / f"{sid}.json" @@ -92,6 +168,41 @@ def test_empty_state_db_webui_row_is_unsafe_not_materialized(tmp_path): assert not (tmp_path / f"{sid}.json").exists() +def test_recover_missing_sidecars_from_state_db_ignores_subagent_row(tmp_path): + sid = _make_state_db(tmp_path / "state.db", sid="subagent_001", source="subagent") + + result = recover_missing_sidecars_from_state_db(tmp_path, tmp_path / "state.db") + + assert result["materialized"] == 0 + assert not (tmp_path / f"{sid}.json").exists() + + report = audit_session_recovery(tmp_path, state_db_path=tmp_path / "state.db") + assert not any(item["session_id"] == sid for item in report["items"]) + + +def test_recover_missing_sidecars_from_state_db_ignores_read_only_index_row(tmp_path): + sid = _make_state_db(tmp_path / "state.db", sid="read_only_index_001") + _write_index(tmp_path, [ + { + "session_id": sid, + "source_tag": "", + "raw_source": "", + "session_source": "", + "read_only": True, + } + ]) + + result = recover_missing_sidecars_from_state_db(tmp_path, tmp_path / "state.db") + + assert result["materialized"] == 1 + sidecar = tmp_path / f"{sid}.json" + assert sidecar.exists() + data = json.loads(sidecar.read_text(encoding="utf-8")) + assert data["session_id"] == sid + assert data["source_tag"] == "webui" + assert data["session_source"] == "webui" + + def test_materialized_sidecar_round_trips_through_session_load(tmp_path, monkeypatch): """Schema parity guard: a materialized sidecar must be readable by Session.load and the resulting Session must have the same messages we put in state.db. From 433f2a860ebf2b8b7738707214ab82bb78631d3c Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Fri, 3 Jul 2026 23:27:48 -0400 Subject: [PATCH 2/7] fix(#5498): skip durable delete tombstones in recovery audit --- api/session_recovery.py | 11 ++++++-- .../test_session_db_sidecar_reconciliation.py | 27 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/api/session_recovery.py b/api/session_recovery.py index 0002263e..f426a936 100644 --- a/api/session_recovery.py +++ b/api/session_recovery.py @@ -574,7 +574,11 @@ def _read_index_session_ids(index_path: Path) -> set[str]: def _index_marks_deleted_webui_session(session_dir: Path, sid: str) -> bool: - """Return True when _index.json marks sid as a deleted WebUI session.""" + """Return True when _index.json has a WebUI-like entry whose sidecar is missing. + + This is a delete-route heuristic for cases where index pruning and durable + tombstone recording both failed; it can also match other sidecar-loss modes. + """ if not sid or (session_dir / f"{sid}.json").exists(): return False index_path = session_dir / '_index.json' @@ -711,7 +715,10 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) if index_path.exists(): index_ids = _read_index_session_ids(index_path) for session_id in sorted(index_ids - live_ids): - if session_id in state_db_deleted_webui_ids: + if ( + session_id in state_db_deleted_webui_ids + or _durable_tombstone_marks_deleted_webui_session(session_dir, session_id) + ): continue items.append(_new_audit_item( session_id, "index_missing_file", "repairable", "rebuild_index" diff --git a/tests/test_session_db_sidecar_reconciliation.py b/tests/test_session_db_sidecar_reconciliation.py index a6b1e9dc..a07df50b 100644 --- a/tests/test_session_db_sidecar_reconciliation.py +++ b/tests/test_session_db_sidecar_reconciliation.py @@ -120,6 +120,33 @@ def test_recover_missing_sidecars_skips_durable_delete_tombstone_without_index(t ) +def test_audit_skips_index_missing_file_when_durable_delete_tombstone_survives(tmp_path, monkeypatch): + import api.models as _m + + sid = "durable_deleted_index_001" + _write_index(tmp_path, [ + { + "session_id": sid, + "source_tag": "webui", + "raw_source": "webui", + "session_source": "webui", + } + ]) + monkeypatch.setattr(_m, "SESSION_DIR", tmp_path) + _m._record_webui_deleted_session_tombstone(sid) + + report = audit_session_recovery(tmp_path) + + assert not any( + item["session_id"] == sid and item["kind"] == "index_missing_file" + for item in report["items"] + ) + assert not any( + item["session_id"] == sid and item["category"] == "repairable" + for item in report["items"] + ) + + def test_recover_missing_sidecars_from_state_db_skips_existing_sidecar(tmp_path): sid = _make_state_db(tmp_path / "state.db") existing = tmp_path / f"{sid}.json" From 308041c269eb6fb134bf22ea2cefae00c2865075 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Fri, 3 Jul 2026 23:36:10 -0400 Subject: [PATCH 3/7] fix(#5498): remove unreachable tombstone recovery branch --- api/session_recovery.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/api/session_recovery.py b/api/session_recovery.py index f426a936..cddc28d4 100644 --- a/api/session_recovery.py +++ b/api/session_recovery.py @@ -489,9 +489,6 @@ def recover_missing_sidecars_from_state_db(session_dir: Path, state_db_path: Pat sid = str(row.get('id') or '').strip() if not sid: continue - if row.get('_state_db_deleted_webui_tombstone'): - details.append({'session_id': sid, 'materialized': False, 'skipped': 'deleted_webui_tombstone'}) - continue target = session_dir / f"{sid}.json" if target.exists(): continue From c17ed6371e9e1cc7eee0690f7825e4c69ec51296 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 4 Jul 2026 00:42:07 -0400 Subject: [PATCH 4/7] fix(#5498): preserve messaging sessions outside WebUI tombstones --- api/routes.py | 2 +- tests/test_issue2057_worktree_lifecycle.py | 78 +++++++++++++++++++++- 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/api/routes.py b/api/routes.py index 303baecc..aefee8e5 100644 --- a/api/routes.py +++ b/api/routes.py @@ -13622,7 +13622,7 @@ def handle_post(handler, parsed) -> bool: prune_session_from_index(sid) except Exception: logger.debug("Failed to prune deleted session from index: %s", sid, exc_info=True) - if sidecar_deleted: + if sidecar_deleted and not is_messaging_session: try: _record_webui_deleted_session_tombstone(sid) except Exception: diff --git a/tests/test_issue2057_worktree_lifecycle.py b/tests/test_issue2057_worktree_lifecycle.py index 6f86f3be..ef718eea 100644 --- a/tests/test_issue2057_worktree_lifecycle.py +++ b/tests/test_issue2057_worktree_lifecycle.py @@ -1,5 +1,6 @@ -from types import SimpleNamespace +import sqlite3 from pathlib import Path +from types import SimpleNamespace import api.models as models import api.routes as routes @@ -49,6 +50,45 @@ def _worktree_session(tmp_path, session_id): return s, worktree +def _make_state_db(path, sid, *, source="telegram"): + conn = sqlite3.connect(str(path)) + conn.executescript( + """ + CREATE TABLE sessions ( + id TEXT PRIMARY KEY, + source TEXT, + model TEXT, + message_count INTEGER DEFAULT 0, + started_at REAL, + title TEXT, + cwd TEXT + ); + CREATE TABLE messages ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + session_id TEXT, + role TEXT, + content TEXT, + timestamp REAL + ); + """ + ) + conn.execute( + "INSERT INTO sessions (id, source, model, message_count, started_at, title, cwd) " + "VALUES (?, ?, 'MiniMax-M3', 2, 1781024055.0, 'Telegram chat', ?)", + (sid, source, str(path.parent)), + ) + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) VALUES (?, 'user', 'hi', 1781024055.0)", + (sid,), + ) + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) VALUES (?, 'assistant', 'hello', 1781024056.0)", + (sid,), + ) + conn.commit() + conn.close() + + def test_delete_worktree_session_reports_retained_worktree_without_cleanup(tmp_path, monkeypatch): session_dir = _isolate_session_store(tmp_path, monkeypatch) session, worktree = _worktree_session(tmp_path, "wtdelete1") @@ -103,6 +143,42 @@ def test_delete_session_records_tombstone_when_state_db_delete_fails(tmp_path, m assert sid in models._load_webui_deleted_session_tombstone() +def test_delete_messaging_session_reopens_read_only_without_deleted_webui_tombstone( + tmp_path, monkeypatch +): + session_dir = _isolate_session_store(tmp_path, monkeypatch) + sid = "telegramdelete1" + state_db = tmp_path / "state.db" + _make_state_db(state_db, sid) + monkeypatch.setattr(models, "_active_state_db_path", lambda: state_db) + session = Session(session_id=sid, title="Telegram chat") + session.save() + captured = _capture_post(monkeypatch, {"session_id": sid}) + cli_meta = { + "session_id": sid, + "source_tag": "telegram", + "raw_source": "telegram", + "session_source": "messaging", + } + monkeypatch.setattr(routes, "_lookup_cli_session_metadata", lambda value: cli_meta) + monkeypatch.setattr(routes, "_is_messaging_session_id", lambda value: True) + delete_calls = [] + monkeypatch.setattr(models, "delete_cli_session", lambda value: delete_calls.append(value)) + + assert routes.handle_post(object(), SimpleNamespace(path="/api/session/delete")) is True + sess, reason = routes._claim_or_synthesize_cli_session(sid) + + assert captured["status"] == 200 + assert captured["payload"]["ok"] is True + assert not (session_dir / f"{sid}.json").exists() + assert sid not in models._load_webui_deleted_session_tombstone() + assert delete_calls == [] + assert reason == "not_claimable" + assert sess is not None + assert sess.read_only is True + assert sess.session_source == "messaging" + + def test_archive_worktree_session_reports_retained_worktree_without_cleanup(tmp_path, monkeypatch): _isolate_session_store(tmp_path, monkeypatch) session, worktree = _worktree_session(tmp_path, "wtarchive1") From fc21c3f38e56f2d5dfb9a6890159c7053adc9c5d Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 4 Jul 2026 00:53:03 -0400 Subject: [PATCH 5/7] fix(#5498): keep tombstoned backups out of repair audit --- api/session_recovery.py | 2 ++ .../test_session_db_sidecar_reconciliation.py | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/api/session_recovery.py b/api/session_recovery.py index cddc28d4..7a17b427 100644 --- a/api/session_recovery.py +++ b/api/session_recovery.py @@ -694,6 +694,8 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) items.append(_new_audit_item( session_id, "malformed_orphan_backup", "unsafe_to_repair", "manual_review", -1, bak_messages )) + elif _marks_deleted_webui_session(session_dir, session_id): + continue elif _state_db_has_session(session_id, state_db_path): items.append(_new_audit_item( session_id, "orphan_backup", "repairable", "restore_from_bak", -1, bak_messages diff --git a/tests/test_session_db_sidecar_reconciliation.py b/tests/test_session_db_sidecar_reconciliation.py index a07df50b..a3be1690 100644 --- a/tests/test_session_db_sidecar_reconciliation.py +++ b/tests/test_session_db_sidecar_reconciliation.py @@ -147,6 +147,41 @@ def test_audit_skips_index_missing_file_when_durable_delete_tombstone_survives(t ) +def test_audit_skips_orphan_backup_when_durable_delete_tombstone_survives(tmp_path, monkeypatch): + import api.models as _m + + sid = _make_state_db(tmp_path / "state.db", sid="durable_deleted_backup_001") + (tmp_path / f"{sid}.json.bak").write_text( + json.dumps( + { + "session_id": sid, + "messages": [{"role": "user", "content": "deleted"}], + } + ), + encoding="utf-8", + ) + monkeypatch.setattr(_m, "SESSION_DIR", tmp_path) + _m._record_webui_deleted_session_tombstone(sid) + + report = audit_session_recovery(tmp_path, state_db_path=tmp_path / "state.db") + + assert any( + item["session_id"] == sid + and item["kind"] == "state_db_deleted_webui_tombstone" + and item["category"] == "unsafe_to_repair" + and item["recommendation"] == "deleted_session_skipped" + for item in report["items"] + ) + assert not any( + item["session_id"] == sid and item["kind"] == "orphan_backup" + for item in report["items"] + ) + assert not any( + item["session_id"] == sid and item["category"] == "repairable" + for item in report["items"] + ) + + def test_recover_missing_sidecars_from_state_db_skips_existing_sidecar(tmp_path): sid = _make_state_db(tmp_path / "state.db") existing = tmp_path / f"{sid}.json" From 63d0c00f8dcb8dc405dd3abad6dfbc775e6ed212 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 4 Jul 2026 05:32:51 +0000 Subject: [PATCH 6/7] harden #5504 on rodboev's latest head: complete the tombstone wiring + CHANGELOG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @rodboev independently pushed fixes for messaging-tombstone (matches ours) and an audit orphan-.bak skip — but the audit skip used _marks_deleted_webui_session (index heuristic), which re-introduces the crash-suppression bug, and the other 4 resurrection/recovery paths were still open. Rebuilt the stage on rodboev's new head (fc21c3f3, attribution preserved) and applied the complete superset: - routes.py claim path: tombstone 404 gated on _state_db_session_source in ('','webui','fork') so foreign-source rows always materialize (finding B) - models.py sidebar projection: skip tombstoned source='webui' rows w/ no live sidecar so a deleted session can't resurface in /api/sessions (finding C) - session_recovery.py orphan-.bak startup: skip restore when durable tombstone present, durable-only (finding D) - session_recovery.py audit orphan-.bak: durable-only classify as deleted_session_skipped (corrects rodboev's index-heuristic version) - session_recovery.py _read_state_db_missing_sidecar_rows: durable-tombstone-only so a genuine crash (index intact, no durable tombstone) still repairs (finding F) Regression tests (all verified fail-without-fix): sidebar-projection, orphan-.bak startup skip, messaging-not-tombstoned, index-only-crash-repairable. Both advisors (Codex SAFE TO SHIP + Opus ship-with-fixes) cleared the prior stage; this rebuild carries the same fixes onto the contributor's latest work. Co-authored-by: rodboev --- CHANGELOG.md | 1 + api/models.py | 18 +++++ api/routes.py | 12 ++- api/session_recovery.py | 38 +++++++++- tests/test_gateway_sync.py | 47 ++++++++++++ tests/test_metadata_save_wipe_1558.py | 28 +++++++ .../test_session_db_sidecar_reconciliation.py | 75 +++++++++++++------ 7 files changed, 193 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06feb461..80670d28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ### Fixed +- **A deleted conversation no longer comes back as a read-only "ghost" after a restart.** Deleting a WebUI session removed its file but left a recoverable row in the agent's state database, so after a server restart and page reload the deleted conversation could reappear as a read-only "Agent" session still showing its old transcript. Deleted WebUI sessions are now recorded in a durable tombstone that the sidebar projection, startup backup-recovery, audit, and session-claim paths all honor, so a deleted session stays gone (the backend keeps returning the 404 the client relies on to clear the stale route). Genuine crash recovery is unaffected — a session whose sidecar is lost without a delete is still restored — imported external-channel (messaging) sessions are exempt and keep their transcript, and a deleted id is automatically un-tombstoned if the same session is later re-created or re-imported. Thanks @rodboev. (#5504, #5498) - **A drag that ends over the sidebar no longer switches conversations by accident.** Releasing the mouse (a stray `pointerup`) over a session row after a drag that began elsewhere in the main content area could switch you into that conversation unintentionally. Session switching now ignores a `pointerup` that isn't a genuine click on the row. Thanks @webtecnica. (#5487, #5462) - **Extension gallery downloads no longer hang on networks with broken IPv6.** Gallery installs and registry fetches followed the OS address order (IPv6-first); on a network advertising dead IPv6 routes, every install stalled for the full TCP timeout before falling back to IPv4. Downloads now try IPv4 first (via a non-global, thread-safe per-connection override that preserves the existing redirect allowlist and timeouts), so installs are fast again on such networks. Thanks @rumotoshino. (#5458) - **Switching profiles no longer leaves you unable to upload in a fresh chat, and cross-profile sidebar rows open correctly.** When you switched profiles with an empty current chat, the browser could keep the previous profile's session id — so a later file upload posted a stale id and failed with "Session not found" until a hard reload. The empty chat is now replaced during the switch when it belonged to a different profile, so uploads work immediately. Additionally, clicking a session that belongs to another profile now switches to its owning profile before loading it (instead of 404ing). Thanks @ruizanthony. (#5460) diff --git a/api/models.py b/api/models.py index ceae3697..317f9f27 100644 --- a/api/models.py +++ b/api/models.py @@ -5756,6 +5756,16 @@ def _load_cli_sessions_uncached( return None profile_value = _cli_profile or 'default' + # A deleted WebUI session is tombstoned (see _record_webui_deleted_session_tombstone) + # so recovery/audit/claim treat it as gone. The sidebar's own state.db projection + # must honor the same tombstone, or a deleted WebUI session reappears here as an + # "Agent" ghost the moment non-WebUI sessions are shown (#5498, second path). Only + # suppress genuine WebUI rows with no live sidecar — a re-created/re-imported sid + # (live {sid}.json) always beats a stale tombstone. + try: + _deleted_webui_tombstone = _load_webui_deleted_session_tombstone() + except Exception: + _deleted_webui_tombstone = frozenset() for row in read_importable_agent_session_rows( db_path, limit=visible_session_limit if visible_session_limit is not None else ( @@ -5774,6 +5784,14 @@ def _load_cli_sessions_uncached( profile = profile_value # CLI DB has no profile column; use active profile _source = row['source'] or 'cli' + # Honor the deleted-WebUI tombstone: a WebUI row the user deleted must + # not resurface in this projection (the #5498 ghost). Live sidecar wins. + if ( + _source == 'webui' + and sid in _deleted_webui_tombstone + and not (SESSION_DIR / f"{sid}.json").exists() + ): + continue _source_meta = normalize_agent_session_source(_source) _title = row['title'] if not _title and _source == 'cron': diff --git a/api/routes.py b/api/routes.py index aefee8e5..35393dc4 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7390,7 +7390,10 @@ def _claim_or_synthesize_cli_session(sid: str, cli_meta: dict = None): if ( ( _session_index_marks_was_webui(sid) - or _session_deleted_tombstone_marks_was_webui(sid) + or ( + _session_deleted_tombstone_marks_was_webui(sid) + and _state_db_session_source(sid) in ("", "webui", "fork") + ) ) and not _is_subagent_child_session_id(sid) ): @@ -7400,6 +7403,13 @@ def _claim_or_synthesize_cli_session(sid: str, cli_meta: dict = None): # Those must recover their transcript from state.db below rather than # 404 as a genuinely-deleted WebUI session (#5307). Every other # index-marked-WebUI id keeps the #2782 self-heal 404 contract. + # + # The durable delete tombstone only 404s a row that is WebUI-owned + # (source webui/fork, or blank = a stale URL with no surviving row). + # A foreign-source row (messaging/cli/tui/desktop) that happens to + # carry a tombstone — e.g. a WebUI delete of an imported session whose + # state.db row the external writer later re-created — must still + # materialize its transcript, never be self-healed to a 404 (#5504). return None, "was_webui" if cli_meta is None: cli_meta = _lookup_cli_session_metadata(sid) or {} diff --git a/api/session_recovery.py b/api/session_recovery.py index 7a17b427..9c1c35d2 100644 --- a/api/session_recovery.py +++ b/api/session_recovery.py @@ -330,6 +330,18 @@ def _orphaned_backup_live_paths( if _msg_count(bak_path) < 0: continue session_id = live_path.stem + # A WebUI session the user deleted must not be resurrected from its + # surviving .bak on the next boot (#5498). Use the DURABLE tombstone + # only — not the _index.json heuristic — so a genuine crash that loses + # the sidecar while its index entry survives is still restored (the + # crash-recovery case this helper exists for). + if _durable_tombstone_marks_deleted_webui_session(session_dir, session_id): + logger.info( + "recover_all_sessions_on_startup: skipped orphan backup %s; " + "session is tombstoned as a deleted WebUI session", + bak_path.name, + ) + continue if not _state_db_has_session(session_id, state_db_path): logger.info( "recover_all_sessions_on_startup: skipped orphan backup %s; " @@ -383,7 +395,15 @@ def _read_state_db_missing_sidecar_rows( sid = str(data.get('id') or '').strip() if not sid or (session_dir / f"{sid}.json").exists(): continue - tombstoned = _marks_deleted_webui_session(session_dir, sid) + # Only the DURABLE delete tombstone suppresses state.db sidecar + # repair. The _index.json heuristic must NOT gate this path: a + # genuine crash that loses the sidecar while its index entry + # survives (no durable tombstone) is exactly the case this repair + # exists for, and on master it materialized the sidecar. Using + # _marks_deleted_webui_session() here (which ORs in the index + # heuristic) wrongly classified that crash as a delete and + # stopped recovery (#5504 Codex/Opus finding). + tombstoned = _durable_tombstone_marks_deleted_webui_session(session_dir, sid) if tombstoned and not include_empty: continue message_rows: list[dict] = [] @@ -694,8 +714,20 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) items.append(_new_audit_item( session_id, "malformed_orphan_backup", "unsafe_to_repair", "manual_review", -1, bak_messages )) - elif _marks_deleted_webui_session(session_dir, session_id): - continue + elif _durable_tombstone_marks_deleted_webui_session(session_dir, session_id): + # The user deleted this WebUI session; its surviving .bak must NOT be + # reported as repairable. DURABLE tombstone only — the _index.json + # heuristic must not suppress a genuine crash whose index survived + # (that case is legitimately repairable). Matches the startup-recovery + # skip + the state.db recovery path (#5504 Codex/Opus finding). + items.append(_new_audit_item( + session_id, + "state_db_deleted_webui_tombstone", + "unsafe_to_repair", + "deleted_session_skipped", + -1, + bak_messages, + )) elif _state_db_has_session(session_id, state_db_path): items.append(_new_audit_item( session_id, "orphan_backup", "repairable", "restore_from_bak", -1, bak_messages diff --git a/tests/test_gateway_sync.py b/tests/test_gateway_sync.py index ff8eb600..6aeb3425 100644 --- a/tests/test_gateway_sync.py +++ b/tests/test_gateway_sync.py @@ -1922,6 +1922,16 @@ def test_delete_imported_messaging_session_preserves_agent_memory(cleanup_test_s (sid,), ).fetchone()[0] assert remaining == 2 + + # A messaging-session delete deliberately preserves the state.db + # transcript (delete_cli_session is skipped), so it must NOT be + # recorded in the deleted-WebUI tombstone — otherwise + # _claim_or_synthesize_cli_session() would treat the still-live + # channel session as was-webui and self-heal it to a 404 on reopen. + import api.models as _m + assert sid not in _m._load_webui_deleted_session_tombstone(), ( + "messaging session must not be tombstoned as a deleted WebUI session" + ) finally: try: _remove_test_sessions(conn, sid) @@ -1930,6 +1940,43 @@ def test_delete_imported_messaging_session_preserves_agent_memory(cleanup_test_s pass +def test_deleted_webui_session_stays_out_of_sidebar_projection(cleanup_test_sessions): + """A tombstoned deleted WebUI session must not resurface via the state.db projection. + + Regression for #5498 (second path): even when non-WebUI sessions are shown, + _load_cli_sessions_uncached() projects source='webui' state.db rows into the + sidebar. Without honoring the deleted-WebUI tombstone, a deleted session + reappears as an 'Agent' ghost. The projection must skip a tombstoned + source='webui' row that has no live sidecar. + """ + import api.models as _m + + conn = _ensure_state_db() + sid = 'webui_deleted_ghost_projection_001' + cleanup_test_sessions.append(sid) + try: + _insert_gateway_session(conn, session_id=sid, source='webui', title='Deleted WebUI Ghost') + _m._record_webui_deleted_session_tombstone(sid) + # Tombstone should win: ensure no live sidecar exists for this sid. + assert not (_m.SESSION_DIR / f'{sid}.json').exists() + + post('/api/settings', {'show_cli_sessions': True}) + data, status = get('/api/sessions') + assert status == 200 + ids = {s['session_id'] for s in data.get('sessions', [])} + assert sid not in ids, ( + "deleted (tombstoned) WebUI session must not resurface in the sidebar projection" + ) + finally: + try: + _m._clear_webui_deleted_session_tombstone(sid) + _remove_test_sessions(conn, sid) + conn.close() + except Exception: + pass + post('/api/settings', {'show_cli_sessions': False}) + + def test_imported_cron_sessions_hidden_from_sidebar_by_default(cleanup_test_sessions): """Cron sessions already imported into the WebUI store should stay hidden from the sidebar.""" from api.models import Session diff --git a/tests/test_metadata_save_wipe_1558.py b/tests/test_metadata_save_wipe_1558.py index 5a694169..4228b668 100644 --- a/tests/test_metadata_save_wipe_1558.py +++ b/tests/test_metadata_save_wipe_1558.py @@ -270,6 +270,34 @@ def test_recover_all_sessions_on_startup_restores_orphan_bak(temp_session_dir): assert len(restored["messages"]) == 293 +def test_recover_all_sessions_on_startup_skips_tombstoned_orphan_bak(temp_session_dir): + """A deleted WebUI session must NOT be resurrected from its surviving .bak. + + Regression for #5498 / #5504: the double-failure delete (backup unlink + + state.db delete both fail) leaves a surviving .json.bak plus the + durable deleted-WebUI tombstone. Startup recovery must honor the tombstone + and NOT recreate the sidecar (otherwise the deleted transcript reappears in + the sidebar on the next boot — the literal ghost the fix exists to kill). + """ + import api.models as _m + + sid = _make_session_on_disk(temp_session_dir, n_msgs=42) + live_path = temp_session_dir / f"{sid}.json" + bak_path = temp_session_dir / f"{sid}.json.bak" + bak_path.write_text(live_path.read_text(encoding="utf-8"), encoding="utf-8") + live_path.unlink() + # Simulate the delete route's durable tombstone for this deleted sid. + _m._record_webui_deleted_session_tombstone(sid) + try: + from api.session_recovery import recover_all_sessions_on_startup + result = recover_all_sessions_on_startup(temp_session_dir) + + assert result["restored"] == 0, "tombstoned orphan .bak must not be restored" + assert not live_path.exists(), "deleted session sidecar must not be recreated on boot" + finally: + _m._clear_webui_deleted_session_tombstone(sid) + + def test_recover_all_sessions_on_startup_rebuilds_missing_index_without_restores(temp_session_dir, monkeypatch): """Startup recovery must rebuild a missing index even when no .bak restore runs.""" import api.models as _m diff --git a/tests/test_session_db_sidecar_reconciliation.py b/tests/test_session_db_sidecar_reconciliation.py index a3be1690..aee60eb4 100644 --- a/tests/test_session_db_sidecar_reconciliation.py +++ b/tests/test_session_db_sidecar_reconciliation.py @@ -48,7 +48,9 @@ def test_recover_missing_sidecars_from_state_db_materializes_webui_row(tmp_path) assert [m["content"] for m in data["messages"]] == ["message 1", "message 2"] -def test_recover_missing_sidecars_from_state_db_skips_deleted_webui_tombstone(tmp_path): +def test_recover_missing_sidecars_from_state_db_skips_deleted_webui_tombstone(tmp_path, monkeypatch): + import api.models as _m + monkeypatch.setattr(_m, "SESSION_DIR", tmp_path) sid = _make_state_db(tmp_path / "state.db", sid="deleted_webui_001") _write_index(tmp_path, [ { @@ -58,14 +60,40 @@ def test_recover_missing_sidecars_from_state_db_skips_deleted_webui_tombstone(tm "session_source": "webui", } ]) + # A genuine delete records the DURABLE tombstone; only that suppresses repair. + _m._record_webui_deleted_session_tombstone(sid) + try: + result = recover_missing_sidecars_from_state_db(tmp_path, tmp_path / "state.db") + assert result["materialized"] == 0 + assert not (tmp_path / f"{sid}.json").exists() + finally: + _m._clear_webui_deleted_session_tombstone(sid) + +def test_recover_missing_sidecars_index_only_no_tombstone_is_repairable(tmp_path, monkeypatch): + """A crash that loses the sidecar (index intact, NO durable tombstone) must + still be recovered from state.db — the index heuristic alone must not + suppress repair (#5504 Codex/Opus finding; matches origin/master behavior).""" + import api.models as _m + monkeypatch.setattr(_m, "SESSION_DIR", tmp_path) + sid = _make_state_db(tmp_path / "state.db", sid="crashed_webui_001") + _write_index(tmp_path, [ + { + "session_id": sid, + "source_tag": "webui", + "raw_source": "webui", + "session_source": "webui", + } + ]) + # No durable tombstone recorded (this is a crash, not a delete). result = recover_missing_sidecars_from_state_db(tmp_path, tmp_path / "state.db") - - assert result["materialized"] == 0 - assert not (tmp_path / f"{sid}.json").exists() + assert result["materialized"] == 1 + assert (tmp_path / f"{sid}.json").exists() -def test_audit_reports_deleted_webui_tombstone_is_unsafe(tmp_path): +def test_audit_reports_deleted_webui_tombstone_is_unsafe(tmp_path, monkeypatch): + import api.models as _m + monkeypatch.setattr(_m, "SESSION_DIR", tmp_path) sid = _make_state_db(tmp_path / "state.db", sid="deleted_webui_001") _write_index(tmp_path, [ { @@ -75,24 +103,27 @@ def test_audit_reports_deleted_webui_tombstone_is_unsafe(tmp_path): "session_source": "webui", } ]) + _m._record_webui_deleted_session_tombstone(sid) + try: + report = audit_session_recovery(tmp_path, state_db_path=tmp_path / "state.db") - report = audit_session_recovery(tmp_path, state_db_path=tmp_path / "state.db") - - assert any( - item["session_id"] == sid - and item["kind"] == "state_db_deleted_webui_tombstone" - and item["category"] == "unsafe_to_repair" - and item["recommendation"] == "deleted_session_skipped" - for item in report["items"] - ) - assert not any( - item["session_id"] == sid and item["kind"] == "state_db_missing_sidecar" - for item in report["items"] - ) - assert not any( - item["session_id"] == sid and item["category"] == "repairable" - for item in report["items"] - ) + assert any( + item["session_id"] == sid + and item["kind"] == "state_db_deleted_webui_tombstone" + and item["category"] == "unsafe_to_repair" + and item["recommendation"] == "deleted_session_skipped" + for item in report["items"] + ) + assert not any( + item["session_id"] == sid and item["kind"] == "state_db_missing_sidecar" + for item in report["items"] + ) + assert not any( + item["session_id"] == sid and item["category"] == "repairable" + for item in report["items"] + ) + finally: + _m._clear_webui_deleted_session_tombstone(sid) def test_recover_missing_sidecars_skips_durable_delete_tombstone_without_index(tmp_path, monkeypatch): From 6c88c0360c7ca5c49d2957ba79a7559f7bcbed82 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 4 Jul 2026 05:38:49 +0000 Subject: [PATCH 7/7] harden #5504: dedup audit tombstone item when .bak + state.db row both survive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex v4 SILENT finding: my audit Finding-2 fix made the orphan-.bak branch emit a state_db_deleted_webui_tombstone item, but the state_db_missing_rows loop still emitted the same sid/kind again when both a surviving .json.bak AND a state.db row exist for one deleted session — double-counting unsafe_to_repair, violating the no-double-classification invariant. Fix: track _bak_tombstoned_ids from the .bak branch and skip them in the state.db loop. Regression test test_audit_no_double_count_when_bak_and_state_db_row_both_survive (verified fails without the dedup). Co-authored-by: rodboev --- api/session_recovery.py | 12 ++++++++ .../test_session_db_sidecar_reconciliation.py | 29 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/api/session_recovery.py b/api/session_recovery.py index 9c1c35d2..678ec459 100644 --- a/api/session_recovery.py +++ b/api/session_recovery.py @@ -704,6 +704,12 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) status.get('bak_messages', -1), )) + # Track sids already classified as deleted-webui-tombstone from the orphan + # .bak branch below, so the later state_db_missing_rows loop doesn't emit a + # duplicate audit item for the same sid (both a surviving .bak and a state.db + # row can exist for one deleted session). (#5504) + _bak_tombstoned_ids: set[str] = set() + for bak_path in sorted(session_dir.glob('*.json.bak')): live_path = bak_path.with_suffix('') if live_path.exists() or live_path.name.startswith('_'): @@ -720,6 +726,7 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) # heuristic must not suppress a genuine crash whose index survived # (that case is legitimately repairable). Matches the startup-recovery # skip + the state.db recovery path (#5504 Codex/Opus finding). + _bak_tombstoned_ids.add(session_id) items.append(_new_audit_item( session_id, "state_db_deleted_webui_tombstone", @@ -762,6 +769,11 @@ def audit_session_recovery(session_dir: Path, state_db_path: Path | None = None) for row in state_db_missing_rows: sid = str(row.get('id') or '') + if sid in _bak_tombstoned_ids: + # Already emitted a deleted-webui-tombstone audit item from the + # orphan .bak branch above (surviving .bak + state.db row both exist + # for this one deleted session) — don't double-count. (#5504) + continue if row.get('_state_db_deleted_webui_tombstone'): items.append(_new_audit_item( sid, diff --git a/tests/test_session_db_sidecar_reconciliation.py b/tests/test_session_db_sidecar_reconciliation.py index aee60eb4..8b00892b 100644 --- a/tests/test_session_db_sidecar_reconciliation.py +++ b/tests/test_session_db_sidecar_reconciliation.py @@ -126,6 +126,35 @@ def test_audit_reports_deleted_webui_tombstone_is_unsafe(tmp_path, monkeypatch): _m._clear_webui_deleted_session_tombstone(sid) +def test_audit_no_double_count_when_bak_and_state_db_row_both_survive(tmp_path, monkeypatch): + """A deleted session with BOTH a surviving .json.bak AND a state.db row must + yield EXACTLY ONE deleted-webui-tombstone audit item, not two (#5504 SILENT + finding — the orphan-.bak branch and the state.db missing-sidecar loop both + used to emit it).""" + import api.models as _m + import json as _json + monkeypatch.setattr(_m, "SESSION_DIR", tmp_path) + sid = _make_state_db(tmp_path / "state.db", sid="deleted_both_001") + # Plant a surviving orphan .bak (no live sidecar) for the same sid. + (tmp_path / f"{sid}.json.bak").write_text( + _json.dumps({"session_id": sid, "messages": [{"role": "user", "content": "x"}]}), + encoding="utf-8", + ) + _m._record_webui_deleted_session_tombstone(sid) + try: + report = audit_session_recovery(tmp_path, state_db_path=tmp_path / "state.db") + tombstone_items = [ + item for item in report["items"] + if item["session_id"] == sid + and item["kind"] == "state_db_deleted_webui_tombstone" + ] + assert len(tombstone_items) == 1, ( + f"expected exactly one tombstone audit item, got {len(tombstone_items)}" + ) + finally: + _m._clear_webui_deleted_session_tombstone(sid) + + def test_recover_missing_sidecars_skips_durable_delete_tombstone_without_index(tmp_path, monkeypatch): import api.models as _m