Skip to content

fix(core): clean up orphaned temp files on atomic write failure - #6068

Open
SHAI-nikhil-chaudhary wants to merge 1 commit into
odysseus-dev:devfrom
SHAI-nikhil-chaudhary:fix/6001-atomic-write-cleanup
Open

fix(core): clean up orphaned temp files on atomic write failure#6068
SHAI-nikhil-chaudhary wants to merge 1 commit into
odysseus-dev:devfrom
SHAI-nikhil-chaudhary:fix/6001-atomic-write-cleanup

Conversation

@SHAI-nikhil-chaudhary

@SHAI-nikhil-chaudhary SHAI-nikhil-chaudhary commented Aug 16, 2026

Copy link
Copy Markdown

Summary

atomic_write_json / atomic_write_text write to a uniquely-named .tmp.<uuid> file and then
os.replace() it onto the target. If anything fails between creating the temp file and the
successful replace, the temp file is left on disk. The two cases that reach this in practice:

  • Serialization failure. json.dump() raises (e.g. a set in the payload) after the temp file
    has already been opened, so a partial or empty .tmp.<uuid> is orphaned.
  • Replace failure. os.replace() raises (permissions, cross-device, target locked), leaving the
    fully-written temp file behind.

Because the suffix is a UUID, every failure orphans a new file rather than overwriting the last
one, so a repeatedly-failing write accumulates junk next to the real data file indefinitely.

This PR wraps the write-and-replace in try/finally so the temp path is unlinked on any failure
path. The success path is unchanged: after os.replace() succeeds the temp path no longer exists,
and the cleanup is a no-op guarded against FileNotFoundError. The exception still propagates —
callers see the same TypeError / OSError they see today.

Target branch

  • This PR targets dev, not main.

Linked Issue

Fixes #6001

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)
  • New feature (non-breaking — adds new behaviour)
  • Breaking change (changes or removes existing behaviour)
  • Refactor / cleanup (behaviour unchanged)
  • Documentation only
  • CI / tooling / configuration

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end.

How to Test

1. Confirm the app still writes normally.

docker compose up

Exercise any flow that persists state through atomic_io (e.g. saving settings), then confirm the
target file is written and no .tmp.* files are left beside it:

ls <data-dir> | grep '\.tmp\.'

2. Serialization failure — atomic_write_json.

From the project root:

import glob
from core.atomic_io import atomic_write_json

try:
    atomic_write_json("test_target.json", {"key": {1, 2, 3}})  # set is not JSON-serializable
except TypeError:
    pass

print("leftover:", glob.glob("test_target.json.tmp.*"))

Expected: leftover: []. On dev today this prints one orphaned temp file, and a second run
prints two.

3. Replace failure — atomic_write_text.

Force os.replace() to fail after a successful write:

import glob, os
from unittest.mock import patch
from core.atomic_io import atomic_write_text

with patch("core.atomic_io.os.replace", side_effect=PermissionError("boom")):
    try:
        atomic_write_text("test_target.txt", "hello")
    except PermissionError:
        pass

print("leftover:", glob.glob("test_target.txt.tmp.*"))

Expected: leftover: [], and test_target.txt is either absent or still holds its previous
contents — the failed write must not have clobbered it.

4. Confirm the exception is unchanged. Both snippets above must still raise (TypeError,
PermissionError). Cleanup must not swallow the error.

Visual / UI changes

N/A — this touches core/atomic_io.py only; nothing renders.

  • Screenshot or short clip
  • Style match
  • No new component patterns
  • I am not an LLM agent submitting a bulk PR.

@github-actions github-actions Bot added needs work PR description incomplete — please update before review ready for review Description complete — ready for maintainer review and removed needs work PR description incomplete — please update before review labels Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed atomic writes accumulate UUID temp files

1 participant