Skip to content

gateway: rewrite transcripts atomically to avoid mid-write corruption - #11083

Closed
Xowiek wants to merge 1 commit into
NousResearch:mainfrom
Xowiek:fix/atomic-transcript-rewrites
Closed

gateway: rewrite transcripts atomically to avoid mid-write corruption#11083
Xowiek wants to merge 1 commit into
NousResearch:mainfrom
Xowiek:fix/atomic-transcript-rewrites

Conversation

@Xowiek

@Xowiek Xowiek commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Summary

SessionStore.rewrite_transcript() was rewriting legacy JSONL transcripts in-place with "w" mode. If Hermes crashed or a serialization error happened mid-rewrite, the transcript file could be left truncated even though the previous version was still valid.

This patch makes transcript rewrites atomic by writing to a temporary sibling file, flushing and fsyncing it, and then swapping it into place with os.replace().

Why

This is a small reliability fix, but it closes a real failure mode in a sensitive code path used by /retry, /undo, and /compress.

Before this change:

  • a mid-write failure could partially overwrite the JSONL transcript
  • legacy sessions that still rely on JSONL fallback could lose usable history
  • subsequent load_transcript() calls could observe truncated state

After this change:

  • the old transcript remains intact unless the replacement file is fully written
  • interruptions and serialization failures no longer corrupt the existing JSONL transcript
  • rewrite behavior is consistent with the repo’s other atomic-write patterns

Changes

  • update gateway/session.py so rewrite_transcript() uses:
    • temp file in the same directory
    • flush() + os.fsync()
    • atomic os.replace()
    • temp-file cleanup on failure
  • add regression coverage in tests/gateway/test_session.py for a simulated mid-write serialization failure

Test coverage

Ran:

uv run --extra dev pytest tests\gateway\test_session.py -k "test_rewrite_failure_preserves_existing_transcript" -v

Result:
PASSED

Also verified the broader test_session.py gateway session tests completed successfully.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the contribution, @Xowiek!

Closing this as a duplicate of #4985 (by @binhnt92), which targets the same fix/feature. We're consolidating on that PR for review.

If you want to help push it over the line, please jump in there — or if you think your approach is better for a specific reason that isn't covered in the other PR, let us know and we can reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants