Skip to content

fix: make transcript rewrite atomic to prevent data loss on crash - #8077

Closed
tomqiaozc wants to merge 1 commit into
NousResearch:mainfrom
tomqiaozc:fix/atomic-transcript-rewrite
Closed

fix: make transcript rewrite atomic to prevent data loss on crash#8077
tomqiaozc wants to merge 1 commit into
NousResearch:mainfrom
tomqiaozc:fix/atomic-transcript-rewrite

Conversation

@tomqiaozc

Copy link
Copy Markdown

Summary

  • Add SessionDB.rewrite_messages() that runs DELETE + all INSERTs inside a single _execute_write() transaction — a crash during rewrite rolls back to the original messages instead of leaving an empty or partial transcript
  • Use tempfile + fsync + os.replace for JSONL rewrites — a crash mid-write preserves the original file intact
  • Replaces the previous non-atomic clear_messages() + N × append_message() sequence in rewrite_transcript()

Test plan

  • test_sqlite_rewrite_is_atomic — verifies rewrite_messages works end-to-end
  • test_jsonl_rewrite_preserves_original_on_error — verifies original JSONL file survives when os.replace fails
  • test_reasoning_survives_rewrite — existing regression test still passes
  • All 62 session tests pass, all 9 dedup tests pass

Closes #8029

🤖 Generated with Claude Code

rewrite_transcript (used by /retry, /undo, /compress) performed
clear+reinsert as separate SQLite transactions and used bare open("w")
for JSONL. A crash between the clear and completion of inserts lost the
entire conversation history.

- Add SessionDB.rewrite_messages() that runs DELETE + INSERT inside a
  single _execute_write() transaction — crash rolls back to original
- Use tempfile + fsync + os.replace for JSONL writes — crash preserves
  the original file intact

Closes NousResearch#8029

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #14774 — same root cause (non-atomic transcript rewrite in /retry, /undo, /compress causing data loss on crash). #14774 is the active fix for #8029.

@alt-glitch alt-glitch added the duplicate This issue or pull request already exists label Apr 28, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the careful durability work. This is already implemented on current main — automated hermes-sweeper review.

  • gateway/session.py:2307-2327 delegates transcript replacement to the canonical database implementation and returns failure to callers when that write fails.
  • hermes_state.py:3632-3678 performs the delete, reinsert, and metadata updates inside one _execute_write transaction; its rollback behavior protects the original transcript if an insert fails.
  • tests/gateway/test_session.py:1536-1576 explicitly forces a mid-rewrite failure and verifies the preexisting messages survive.
  • The JSONL fallback addressed by this PR no longer participates in transcript rewrites: commit 351fdcc6e removed it, and gateway/session.py:2329-2342 now uses state.db only.

The transactional implementation shipped in 2f9243c333 and is contained in release v2026.5.28. The duplicate relationship noted by @alt-glitch with #14774 is consistent with the current implementation.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: non-atomic transcript rewrite causes data loss on crash (/retry, /undo, /compress)

3 participants