harden #5504: dedup audit tombstone item when .bak + state.db row both survive
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 <rodboev@users.noreply.github.com>
This commit is contained in:
1 parent
63d0c00f8d
commit
6c88c0360c
2 files changed
+41
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in new issue
Block a user