From 407b6cedbd60f17a9af81854de8e34448a1e74d0 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 23 Sep 2026 16:29:21 +0530 Subject: [PATCH] refactor(sessions): let the delete transaction be the only export-drift fence --delete-after-verified had three guards after the merge: #119935's pre-delete message re-count, #120065's caller-side `previous != snapshot` compare between exported items, and #120065's expected_display_messages compare inside delete_session's BEGIN IMMEDIATE. Only the last one is race-free: it compares the exact display rows against the store at DELETE time, so a same-count rewrite, an append after the re-count, or inter-item drift is refused there. The two caller-side checks run outside the transaction and can only ever duplicate a verdict the fence already gives, so drop them and merge the snapshots straight into expected_messages. Verified with the S1 probe (--race): compacted_fires false, same_count_fires false with the caller-side guards removed. Co-authored-by: JoaoMarcos44 Co-authored-by: John Paul Soliva --- hermes_cli/sessions_cmd.py | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/hermes_cli/sessions_cmd.py b/hermes_cli/sessions_cmd.py index 92c678ee3f..e433b71836 100644 --- a/hermes_cli/sessions_cmd.py +++ b/hermes_cli/sessions_cmd.py @@ -547,27 +547,15 @@ def _export_markdown_single(db, args, export_one, output_dir, lineage_is_logical f"to {exported_items[0][1] if n == 1 else output_dir}") if not args.delete_after_verified: return + # The file only proves it matches the dict it was written from. Whether the store still holds that history + # is decided inside delete_session's transaction (expected_display_messages), so no caller-side re-read. expected_messages = {} for data, exported_path, snapshots in exported_items: ok, reason = verify_export_file(exported_path, data) - # The file only proves it matches the dict it was written from; the delete removes what the store holds - # now, so re-count the store just before it (like the adoption retire loop, outside its transaction). - exported = len(data.get("messages") or []) - shown = sum(len(db.get_messages(sid, include_compacted=True)) - for sid in data.get("lineage_session_ids") or [data["id"]]) - if ok and shown != exported: - ok, reason = False, (f"the session changed after it was exported ({shown} messages now, {exported} in " - "the file); run the export again") if not ok: print(f"Export verification failed; not deleting session '{data.get('id')}': {reason}") return - for covered_id, snapshot in snapshots.items(): - previous = expected_messages.get(covered_id) - if previous is not None and previous != snapshot: - print(f"Export verification failed; not deleting session '{data.get('id')}': " - f"session '{covered_id}' changed while the export set was being built") - return - expected_messages[covered_id] = snapshot + expected_messages.update(snapshots) if not db.delete_session( resolved_session_id, sessions_dir=_sessions_dir(), expected_delete_ids=delete_target_ids, expected_display_messages=expected_messages,