Skip to content

fix: make session history save atomic and surface corrupt files - #83

Merged
shauryagangrade merged 1 commit into
shauryagangrade:mainfrom
aryansk:fix/atomic-history-save
Aug 14, 2026
Merged

fix: make session history save atomic and surface corrupt files#83
shauryagangrade merged 1 commit into
shauryagangrade:mainfrom
aryansk:fix/atomic-history-save

Conversation

@aryansk

@aryansk aryansk commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Closes #53.

Problem

gcode/history.py wrote session history in place and swallowed every exception in save (bare except Exception: pass). An interrupted or failed save could leave a partial session file — and load then returned None, so the whole conversation looked like it never existed. Silent data loss.

Change

  • Atomic save: save() now writes to a temp file in the same directory (tempfile.mkstemp) and os.replace()s it into place, so a crash or interrupt can never leave a partial file. On failure the temp file is removed and a warning naming the path is printed to stderr — save still never crashes the REPL.
  • Corrupt load surfaces: load() now prints a warning naming the file and suggesting the user inspect/remove it when a session file exists but cannot be parsed, instead of silently returning None. A genuinely missing file still returns None silently (no history yet is not an error).

save is called once per turn (not per keystroke), so a per-failure warning is not spammy.

Tests

  • test_save_write_failure_resilience: patched os.replace to fail — asserts no exception, no partial file at the real path, no stray .tmp file, and a warning naming the path.
  • test_load_corrupt_json_warns_with_path: invalid JSON — asserts None plus a warning containing the path and "corrupt".
  • test_load_missing_file_no_warning: missing file — asserts None with no stderr output.

Validation

  • Full suite: 105 passed, 1 skipped (3 new history tests).
  • ruff check . clean, ruff format --check . clean, mypy gcode clean, bandit clean, compileall clean.
  • git diff --check clean; rebased onto current main.

save() wrote the session file in place and swallowed every exception, so
an interrupted or failed save could leave a partial file that load()
then silently treated as 'no history' — silent data loss. Save now writes
to a temp file in the same directory and os.replace()s it into place
(no partial file on crash), cleans up the temp file on failure, and
prints a warning to stderr instead of passing. load() now reports the
path and suggests removing the file when a session file exists but
cannot be parsed; a genuinely missing file stays silent. Fixes shauryagangrade#53.
@shauryagangrade
shauryagangrade marked this pull request as ready for review August 14, 2026 17:46
@shauryagangrade
shauryagangrade merged commit 6d0b65e into shauryagangrade:main Aug 14, 2026
6 checks passed
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.

[Bug]: History save silently swallows errors; a corrupt file reads back as "no history"

2 participants