Skip to content

fix(gateway): make rewrite_transcript() JSONL write atomic - #20012

Closed
vominh1919 wants to merge 1 commit into
NousResearch:mainfrom
vominh1919:fix/session-atomic-rewrite
Closed

fix(gateway): make rewrite_transcript() JSONL write atomic#20012
vominh1919 wants to merge 1 commit into
NousResearch:mainfrom
vominh1919:fix/session-atomic-rewrite

Conversation

@vominh1919

Copy link
Copy Markdown
Contributor

Problem

SessionStore.rewrite_transcript() overwrites the JSONL transcript file with a plain open(..., 'w') + loop. If the process is killed mid-write (SIGKILL, OOM, power loss), the file is left truncated. load_transcript() then loads incomplete conversation history, which can cause the agent to lose context.

This function is called by /retry, /undo, and /compress — operations where losing history is particularly harmful.

Fix

Replace the direct write_text with the same tempfile + fsync + atomic_replace pattern already used by _sessions_save() in the same file. The old file is preserved until the new one is fully written and synced to disk.

Before vs After

Scenario Before After
Crash during /retry Truncated JSONL, lost history Old file preserved, no data loss
Crash during /compress Partial transcript Atomic swap, consistent state

Tests

This is a crash-safety fix. The atomic write pattern (tempfile.mkstemp + os.fdopen + fsync + atomic_replace) is already battle-tested in _sessions_save() at line 722 of the same file.

rewrite_transcript() overwrites the JSONL transcript file with a plain
open(..., 'w') + loop. If the process is killed mid-write (SIGKILL,
OOM, power loss), the file is left truncated, causing load_transcript()
to load an incomplete conversation history.

Use the same tempfile + fsync + atomic_replace pattern already used by
_sessions_save() in the same file. This ensures the old file is
preserved until the new one is fully written and synced to disk.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels May 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #15085 — same atomic JSONL rewrite fix for rewrite_transcript(). Also competes with #8065.

@teknium1

Copy link
Copy Markdown
Contributor

This looks implemented on current main by the later DB-only transcript refactor. Automated hermes-sweeper review found that the JSONL rewrite path this PR hardens no longer exists.

  • gateway/session.py:1302 now has rewrite_transcript() call self._db.replace_messages(session_id, messages) only; it no longer opens or rewrites a JSONL transcript file.
  • gateway/session.py:1314 documents and implements DB-only transcript loading, so load_transcript() no longer consumes the legacy JSONL fallback that could be left truncated.
  • hermes_state.py:2327 / hermes_state.py:800 show the replacement path runs inside a SQLite transaction with commit/rollback handling.
  • The mainline removal landed in 351fdcc6e6d763bd5d405d90d467a6d52eabf1f5 (refactor(gateway): stop writing JSONL in append_to_transcript / rewrite_transcript), contained in v2026.5.28.

Thanks for the crash-safety fix; the same failure mode is now addressed by removing the JSONL rewrite/load path entirely rather than making that legacy write atomic.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 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 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.

3 participants