From 6c88c0360c7ca5c49d2957ba79a7559f7bcbed82 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 4 Jul 2026 05:38:49 +0000 Subject: [PATCH] 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