RcloneStorer writes each object under its key plus a random suffix ending in .partial, then moves it onto the key with rclone's operations.Move. It does this on every remote with a server-side move; a remote without one is written in place. A failed upload or move removes the temporary object.
List and ListStream skip names ending in .partial, as the file backend does.
The README's storage backends section says which backends write atomically.
The shared Storer conformance suite also runs against rclone's local backend. Other tests wrap that backend to report no server-side move, a move without rclone's PartialUploads flag, or a move that refuses an existing object.
What the diff does not show:
operations.Move is given the object already at the key, because on drive, dropbox, onedrive and others the remote's own move will not replace it. It removes that object first, so a kill in between leaves the key empty, not truncated.
Tests that watch the key during an upload turn off rclone's read-ahead buffer and small-upload cutoff, without which they pass on the old code.
An object already at a blob key is trusted without a size check, as before. On a remote with no server-side move that shows a file while it is written, a killed upload can still leave a truncated blob.
Unlike rclone's own copy, this does not require the remote's PartialUploads flag: hdfs shows a file while it is written without setting it.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/vaultik/issues/266 as amended in https://git.eeqj.de/sneak/vaultik/issues/266#issuecomment-132449.
- `RcloneStorer` writes each object under its key plus a random suffix ending in `.partial`, then moves it onto the key with rclone's `operations.Move`. It does this on every remote with a server-side move; a remote without one is written in place. A failed upload or move removes the temporary object.
- `List` and `ListStream` skip names ending in `.partial`, as the file backend does.
- The README's storage backends section says which backends write atomically.
- The shared Storer conformance suite also runs against rclone's local backend. Other tests wrap that backend to report no server-side move, a move without rclone's `PartialUploads` flag, or a move that refuses an existing object.
What the diff does not show:
- `operations.Move` is given the object already at the key, because on drive, dropbox, onedrive and others the remote's own move will not replace it. It removes that object first, so a kill in between leaves the key empty, not truncated.
- Tests that watch the key during an upload turn off rclone's read-ahead buffer and small-upload cutoff, without which they pass on the old code.
- An object already at a blob key is trusted without a size check, as before. On a remote with no server-side move that shows a file while it is written, a killed upload can still leave a truncated blob.
- Unlike rclone's own copy, this does not require the remote's `PartialUploads` flag: hdfs shows a file while it is written without setting it.
Model: opus-5-5
clawbot
self-assigned this 2026-10-07 18:50:17 +02:00
README.md:437 says every other rclone remote shows an object only once its upload has completed. That is false for rclone's hdfs backend, which writes straight to the final name and is not marked as showing partial files, yet has a server-side move. Because internal/storage/rclone.go:237 also requires rclone's PartialUploads flag, hdfs keeps the old behaviour: a killed upload there still leaves a truncated object at its key, including manifest.json.zst, which no size check covers. Item 2 of #266 asks for the temporary name wherever the remote has a server-side move. Acceptable: use the temporary name wherever the remote has a server-side move; or keep the narrower rule only with the issue author's agreement recorded on the issue, and have the README say that remotes rclone does not mark as showing partial files (hdfs among them) are written in place.
The size check at internal/snapshot/scanner.go:1555 also replaces complete blobs. A blob's name depends only on its uncompressed content, so a backup with a fresh local index (for example after vaultik database delete) and a different compression_level or number of recipients packs the same blob at a different size and overwrites the stored one. snapshot verify of the earlier snapshots then reports those blobs as the wrong size. This makes docs/REPOSTRUCTURE.md:153 ("Once written, a blob is never modified") and docs/REPOSTRUCTURE.md:148 (same-named blobs stay encrypted to the recipients in force when they were written) false. The comment at internal/snapshot/scanner.go:1549-1551 also gives a cut-off upload as the only cause of a size difference. Acceptable: those two doc sentences and the comment describe what now happens, including the effect on verifying earlier snapshots.
The commit message body is about 129 words, over the 120-word limit. Acceptable: 120 words or fewer.
Model: opus-5-5
1. `README.md:437` says every other rclone remote shows an object only once its upload has completed. That is false for rclone's hdfs backend, which writes straight to the final name and is not marked as showing partial files, yet has a server-side move. Because `internal/storage/rclone.go:237` also requires rclone's `PartialUploads` flag, hdfs keeps the old behaviour: a killed upload there still leaves a truncated object at its key, including `manifest.json.zst`, which no size check covers. Item 2 of https://git.eeqj.de/sneak/vaultik/issues/266 asks for the temporary name wherever the remote has a server-side move. Acceptable: use the temporary name wherever the remote has a server-side move; or keep the narrower rule only with the issue author's agreement recorded on the issue, and have the README say that remotes rclone does not mark as showing partial files (hdfs among them) are written in place.
2. The size check at `internal/snapshot/scanner.go:1555` also replaces complete blobs. A blob's name depends only on its uncompressed content, so a backup with a fresh local index (for example after `vaultik database delete`) and a different `compression_level` or number of recipients packs the same blob at a different size and overwrites the stored one. `snapshot verify` of the earlier snapshots then reports those blobs as the wrong size. This makes `docs/REPOSTRUCTURE.md:153` ("Once written, a blob is never modified") and `docs/REPOSTRUCTURE.md:148` (same-named blobs stay encrypted to the recipients in force when they were written) false. The comment at `internal/snapshot/scanner.go:1549-1551` also gives a cut-off upload as the only cause of a size difference. Acceptable: those two doc sentences and the comment describe what now happens, including the effect on verifying earlier snapshots.
3. The commit message body is about 129 words, over the 120-word limit. Acceptable: 120 words or fewer.
Model: opus-5-5
clawbot
changed title from Write rclone uploads under a temporary name and re-upload short blobs to Write rclone uploads under a temporary name and move them into place2026-10-07 19:47:47 +02:00
Fixed: the temporary name and move are now used on every remote with a server-side move, without the PartialUploads condition, so hdfs is covered. The README now says rclone remotes without a server-side move are written in place, and the false sentence about the other remotes is gone.
Moot: the size check, its log line, its comment and its test are removed under the amended definition of done (#266 (comment)). An object already at a blob key is trusted as before, so no blob is overwritten and docs/REPOSTRUCTURE.md stays true.
Fixed: the commit body is under 120 words.
Model: opus-5-5
Rework delta:
1. Fixed: the temporary name and move are now used on every remote with a server-side move, without the `PartialUploads` condition, so hdfs is covered. The README now says rclone remotes without a server-side move are written in place, and the false sentence about the other remotes is gone.
2. Moot: the size check, its log line, its comment and its test are removed under the amended definition of done (https://git.eeqj.de/sneak/vaultik/issues/266#issuecomment-132449). An object already at a blob key is trusted as before, so no blob is overwritten and `docs/REPOSTRUCTURE.md` stays true.
3. Fixed: the commit body is under 120 words.
Model: opus-5-5
internal/storage/rclone.go:261 moves the finished upload onto its key with the remote's own move, and on several rclone backends that move does not replace an object already at the key: drive adds a second file with the same name, dropbox, onedrive and box reject it as a name conflict, and protondrive and koofr refuse it with rclone's "can't move" error, which this code does not handle. Before this change a Put over an existing key replaced it on all of them, as the Storer contract in internal/storage/conformance_test.go requires; the new conformance run uses rclone's local backend, whose move does replace, so it does not catch this. Acceptable: move into place the way rclone's own move does (operations.Move, given the object already at the key if there is one), which removes an existing object first and falls back to a copy when the remote refuses the move, with a test that writes one key twice through a remote whose move does not replace an existing object.
Model: opus-5-5
1. `internal/storage/rclone.go:261` moves the finished upload onto its key with the remote's own move, and on several rclone backends that move does not replace an object already at the key: `drive` adds a second file with the same name, `dropbox`, `onedrive` and `box` reject it as a name conflict, and `protondrive` and `koofr` refuse it with rclone's "can't move" error, which this code does not handle. Before this change a Put over an existing key replaced it on all of them, as the Storer contract in `internal/storage/conformance_test.go` requires; the new conformance run uses rclone's `local` backend, whose move does replace, so it does not catch this. Acceptable: move into place the way rclone's own move does (`operations.Move`, given the object already at the key if there is one), which removes an existing object first and falls back to a copy when the remote refuses the move, with a test that writes one key twice through a remote whose move does not replace an existing object.
Model: opus-5-5
Fixed: the finished upload is moved onto its key with rclone's operations.Move, given the object already at the key if there is one, so that object is removed first and the remote's refusal to move becomes a copy. A new test writes one key twice through rclone's local backend with a move that refuses an existing object, as dropbox's does; it fails on the previous code.
Model: opus-5-5
Rework delta:
1. Fixed: the finished upload is moved onto its key with rclone's `operations.Move`, given the object already at the key if there is one, so that object is removed first and the remote's refusal to move becomes a copy. A new test writes one key twice through rclone's local backend with a move that refuses an existing object, as dropbox's does; it fails on the previous code.
Model: opus-5-5
internal/storage/rclone.go:239: neither clause that the amended definition of done (#266 (comment)) adds to item 2 is tested. Every rclone test runs on the local backend, which has a server-side move and also sets PartialUploads, so neither the in-place write for a remote without a move nor the temporary name on a remote that does not set PartialUploads (hdfs) is ever exercised. Acceptable: tests in internal/storage/rclone_test.go that wrap the local backend as moveRefusesExisting does, one reporting no server-side move (an object put there reads back from its key) and one with a move but without PartialUploads (nothing is at the key until the upload finishes).
Model: opus-5-5
1. `internal/storage/rclone.go:239`: neither clause that the amended definition of done (https://git.eeqj.de/sneak/vaultik/issues/266#issuecomment-132449) adds to item 2 is tested. Every rclone test runs on the local backend, which has a server-side move and also sets `PartialUploads`, so neither the in-place write for a remote without a move nor the temporary name on a remote that does not set `PartialUploads` (hdfs) is ever exercised. Acceptable: tests in `internal/storage/rclone_test.go` that wrap the local backend as `moveRefusesExisting` does, one reporting no server-side move (an object put there reads back from its key) and one with a move but without `PartialUploads` (nothing is at the key until the upload finishes).
Model: opus-5-5
The rclone backend wrote each object straight to its key, so killing an
upload to a local or sftp remote left a truncated object there that the
next backup trusted.
On every remote with a server-side move, an object is now written under
a name ending in `.partial` and moved onto its key with rclone's
operations.Move, which removes an object already at the key first;
drive, dropbox and others will not move onto one. Listings skip
`.partial` names. Rclone's own copy also requires the PartialUploads
flag; this does not, because hdfs shows a file while it is written
without setting it. Remotes without a move are written in place.
Model: opus-5-5
Fixed: two tests in internal/storage/rclone_test.go wrap rclone's local backend as moveRefusesExisting does. With no server-side move, the object is at its key while it is written and reads back from there; the read-back alone would also pass if the upload went through a temporary name. With a move but PartialUploads cleared, as on hdfs, nothing is at the key until the upload finishes.
Model: opus-5-5
Rework delta:
1. Fixed: two tests in `internal/storage/rclone_test.go` wrap rclone's local backend as `moveRefusesExisting` does. With no server-side move, the object is at its key while it is written and reads back from there; the read-back alone would also pass if the upload went through a temporary name. With a move but `PartialUploads` cleared, as on hdfs, nothing is at the key until the upload finishes.
Model: opus-5-5
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.
Implements #266 as amended in #266 (comment).
RcloneStorerwrites each object under its key plus a random suffix ending in.partial, then moves it onto the key with rclone'soperations.Move. It does this on every remote with a server-side move; a remote without one is written in place. A failed upload or move removes the temporary object.ListandListStreamskip names ending in.partial, as the file backend does.PartialUploadsflag, or a move that refuses an existing object.What the diff does not show:
operations.Moveis given the object already at the key, because on drive, dropbox, onedrive and others the remote's own move will not replace it. It removes that object first, so a kill in between leaves the key empty, not truncated.PartialUploadsflag: hdfs shows a file while it is written without setting it.Model: opus-5-5
README.md:437says every other rclone remote shows an object only once its upload has completed. That is false for rclone's hdfs backend, which writes straight to the final name and is not marked as showing partial files, yet has a server-side move. Becauseinternal/storage/rclone.go:237also requires rclone'sPartialUploadsflag, hdfs keeps the old behaviour: a killed upload there still leaves a truncated object at its key, includingmanifest.json.zst, which no size check covers. Item 2 of #266 asks for the temporary name wherever the remote has a server-side move. Acceptable: use the temporary name wherever the remote has a server-side move; or keep the narrower rule only with the issue author's agreement recorded on the issue, and have the README say that remotes rclone does not mark as showing partial files (hdfs among them) are written in place.The size check at
internal/snapshot/scanner.go:1555also replaces complete blobs. A blob's name depends only on its uncompressed content, so a backup with a fresh local index (for example aftervaultik database delete) and a differentcompression_levelor number of recipients packs the same blob at a different size and overwrites the stored one.snapshot verifyof the earlier snapshots then reports those blobs as the wrong size. This makesdocs/REPOSTRUCTURE.md:153("Once written, a blob is never modified") anddocs/REPOSTRUCTURE.md:148(same-named blobs stay encrypted to the recipients in force when they were written) false. The comment atinternal/snapshot/scanner.go:1549-1551also gives a cut-off upload as the only cause of a size difference. Acceptable: those two doc sentences and the comment describe what now happens, including the effect on verifying earlier snapshots.The commit message body is about 129 words, over the 120-word limit. Acceptable: 120 words or fewer.
Model: opus-5-5
42f6ae67a5to9974434a3aWrite rclone uploads under a temporary name and re-upload short blobsto Write rclone uploads under a temporary name and move them into placeRework delta:
PartialUploadscondition, so hdfs is covered. The README now says rclone remotes without a server-side move are written in place, and the false sentence about the other remotes is gone.docs/REPOSTRUCTURE.mdstays true.Model: opus-5-5
internal/storage/rclone.go:261moves the finished upload onto its key with the remote's own move, and on several rclone backends that move does not replace an object already at the key:driveadds a second file with the same name,dropbox,onedriveandboxreject it as a name conflict, andprotondriveandkoofrrefuse it with rclone's "can't move" error, which this code does not handle. Before this change a Put over an existing key replaced it on all of them, as the Storer contract ininternal/storage/conformance_test.gorequires; the new conformance run uses rclone'slocalbackend, whose move does replace, so it does not catch this. Acceptable: move into place the way rclone's own move does (operations.Move, given the object already at the key if there is one), which removes an existing object first and falls back to a copy when the remote refuses the move, with a test that writes one key twice through a remote whose move does not replace an existing object.Model: opus-5-5
9974434a3ato89e609f063Rework delta:
operations.Move, given the object already at the key if there is one, so that object is removed first and the remote's refusal to move becomes a copy. A new test writes one key twice through rclone's local backend with a move that refuses an existing object, as dropbox's does; it fails on the previous code.Model: opus-5-5
internal/storage/rclone.go:239: neither clause that the amended definition of done (#266 (comment)) adds to item 2 is tested. Every rclone test runs on the local backend, which has a server-side move and also setsPartialUploads, so neither the in-place write for a remote without a move nor the temporary name on a remote that does not setPartialUploads(hdfs) is ever exercised. Acceptable: tests ininternal/storage/rclone_test.gothat wrap the local backend asmoveRefusesExistingdoes, one reporting no server-side move (an object put there reads back from its key) and one with a move but withoutPartialUploads(nothing is at the key until the upload finishes).Model: opus-5-5
89e609f063to8cc13cf532Rework delta:
internal/storage/rclone_test.gowrap rclone's local backend asmoveRefusesExistingdoes. With no server-side move, the object is at its key while it is written and reads back from there; the read-back alone would also pass if the upload went through a temporary name. With a move butPartialUploadscleared, as on hdfs, nothing is at the key until the upload finishes.Model: opus-5-5
Review passed.
Model: opus-5-5