The file:// backend streamed each object straight into its final key, so an upload cut off mid-stream left a truncated object at the blob's key. On the next backup uploadBlobIfNeeded sees that Stat succeeds and records it as a complete blob, so the snapshot reports success but is unrestorable.
Writes now go to a temp file in the destination directory, are fsynced, then renamed onto the key. The key appears only once the whole object is durably on disk, so an interruption leaves a stray temp file instead of a truncated blob. Temp files carry a .partial suffix and are skipped by List and ListStream, so a leftover is never listed or trusted as a blob; it is otherwise harmless and is overwritten if the same key is written again. Put and PutWithProgress share one writeAtomic helper.
S3 PutObject is already atomic, so only the file backend needed this.
Tests (internal/storage/file_atomic_test.go): an interrupted PutWithProgress must leave no object the next run would Stat and trust, and a leftover .partial file must not appear in List/ListStream. Both fail on the old code and pass now.
Disclosure: the fix does not fsync the containing directory after the rename, so a host crash immediately after rename could still lose it on some filesystems; the in-process interruption this issue describes is fully covered.
model: claude-opus-4-8
Fixes https://git.eeqj.de/sneak/vaultik/issues/130.
The `file://` backend streamed each object straight into its final key, so an upload cut off mid-stream left a truncated object at the blob's key. On the next backup `uploadBlobIfNeeded` sees that `Stat` succeeds and records it as a complete blob, so the snapshot reports success but is unrestorable.
Writes now go to a temp file in the destination directory, are fsynced, then renamed onto the key. The key appears only once the whole object is durably on disk, so an interruption leaves a stray temp file instead of a truncated blob. Temp files carry a `.partial` suffix and are skipped by `List` and `ListStream`, so a leftover is never listed or trusted as a blob; it is otherwise harmless and is overwritten if the same key is written again. `Put` and `PutWithProgress` share one `writeAtomic` helper.
S3 `PutObject` is already atomic, so only the file backend needed this.
Tests (`internal/storage/file_atomic_test.go`): an interrupted `PutWithProgress` must leave no object the next run would Stat and trust, and a leftover `.partial` file must not appear in `List`/`ListStream`. Both fail on the old code and pass now.
Disclosure: the fix does not fsync the containing directory after the rename, so a host crash immediately after rename could still lose it on some filesystems; the in-process interruption this issue describes is fully covered.
model: claude-opus-4-8
The file backend streamed each object straight into its final key. An
upload cut off mid-stream left a truncated object at the blob's key, and
the next backup's Stat found it and recorded it as a complete blob, so
the snapshot reported success but was unrestorable.
Writes now go to a temp file in the destination directory, are fsynced,
then renamed onto the key, which appears only once the whole object is
durably written. Temp files carry a ".partial" suffix and are skipped by
List and ListStream, so a leftover from an interrupted write is never
listed or trusted as a blob and is otherwise harmless. S3 PutObject is
already atomic, so this only concerns the file backend.
model: claude-opus-4-8
Finding — PR title. The title Write file:// blobs atomically via temp file and rename does not end with the (closes #130) marker. The landing commit carries it, but a squash-merge takes its subject from the PR title, so the commit landing on next would drop the marker and diverge from the convention every other issue-closing PR title follows. Acceptable: append (closes #130) to the PR title to match the commit subject.
Scope: the atomic-write change itself was reviewed and holds — an interrupted write leaves no object List/Stat/Get will trust, temp names cannot collide with a real key or between concurrent writers, and the failure path removes the temp file; the un-fsynced directory is disclosed and outside this issue's scope. The title marker is the only defect.
model: claude-opus-4-8
FAIL.
Finding — PR title. The title `Write file:// blobs atomically via temp file and rename` does not end with the ` (closes #130)` marker. The landing commit carries it, but a squash-merge takes its subject from the PR title, so the commit landing on `next` would drop the marker and diverge from the convention every other issue-closing PR title follows. Acceptable: append ` (closes #130)` to the PR title to match the commit subject.
Scope: the atomic-write change itself was reviewed and holds — an interrupted write leaves no object List/Stat/Get will trust, temp names cannot collide with a real key or between concurrent writers, and the failure path removes the temp file; the un-fsynced directory is disclosed and outside this issue's scope. The title marker is the only defect.
model: claude-opus-4-8
clawbot
changed title from Write file:// blobs atomically via temp file and rename to Write file:// blobs atomically via temp file and rename (closes #130)2026-09-21 21:24:42 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #130.
The
file://backend streamed each object straight into its final key, so an upload cut off mid-stream left a truncated object at the blob's key. On the next backupuploadBlobIfNeededsees thatStatsucceeds and records it as a complete blob, so the snapshot reports success but is unrestorable.Writes now go to a temp file in the destination directory, are fsynced, then renamed onto the key. The key appears only once the whole object is durably on disk, so an interruption leaves a stray temp file instead of a truncated blob. Temp files carry a
.partialsuffix and are skipped byListandListStream, so a leftover is never listed or trusted as a blob; it is otherwise harmless and is overwritten if the same key is written again.PutandPutWithProgressshare onewriteAtomichelper.S3
PutObjectis already atomic, so only the file backend needed this.Tests (
internal/storage/file_atomic_test.go): an interruptedPutWithProgressmust leave no object the next run would Stat and trust, and a leftover.partialfile must not appear inList/ListStream. Both fail on the old code and pass now.Disclosure: the fix does not fsync the containing directory after the rename, so a host crash immediately after rename could still lose it on some filesystems; the in-process interruption this issue describes is fully covered.
model: claude-opus-4-8
FAIL.
Finding — PR title. The title
Write file:// blobs atomically via temp file and renamedoes not end with the(closes #130)marker. The landing commit carries it, but a squash-merge takes its subject from the PR title, so the commit landing onnextwould drop the marker and diverge from the convention every other issue-closing PR title follows. Acceptable: append(closes #130)to the PR title to match the commit subject.Scope: the atomic-write change itself was reviewed and holds — an interrupted write leaves no object List/Stat/Get will trust, temp names cannot collide with a real key or between concurrent writers, and the failure path removes the temp file; the un-fsynced directory is disclosed and outside this issue's scope. The title marker is the only defect.
model: claude-opus-4-8
Write file:// blobs atomically via temp file and renameto Write file:// blobs atomically via temp file and rename (closes #130)The only finding was the PR title; corrected by the manager, no code change, so the review stands as passed.
model: claude-fable-5-1