What was wrong:prune (also as the prune step of snapshot create --prune) and remote nuke exited 0 when a delete failed. Nuke then advised vaultik prune, which never removes metadata, and printed "Backup destination store is now empty." A successful nuke also advised prune for blobs it then deleted itself.
What changed:
PruneBlobs prints its summary or --json document, then returns "N blob(s) could not be deleted". That error replaces the old "Failed to delete N blob(s)" line.
The remove-all step of remote nuke returns "N snapshot(s) could not be removed" instead of warning, and no longer prints the prune note. Nuke is its only caller since snapshot remove --all was dropped.
NukeRemote first checks that the local index belongs to the configured destination and returns that error without advice: running nuke again would fail the same way. After that check it prints "now empty" only on success; on any failure it says to run vaultik remote nuke --force again.
faultstore gains an OnDelete hook so tests can refuse a delete. The test stage runs as root, where a read-only directory does not stop a delete.
Judgement call: a snapshot that nuke cannot remove from the local index now also fails the run, since nuke then leaves that snapshot's metadata on the store.
Judgement call: a failure after the destination check that is not a delete, such as a listing error between the metadata and blob steps, still gets the "run nuke again" advice.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/290.
**What was wrong:** `prune` (also as the prune step of `snapshot create --prune`) and `remote nuke` exited 0 when a delete failed. Nuke then advised `vaultik prune`, which never removes metadata, and printed "Backup destination store is now empty." A successful nuke also advised `prune` for blobs it then deleted itself.
**What changed:**
- `PruneBlobs` prints its summary or `--json` document, then returns "N blob(s) could not be deleted". That error replaces the old "Failed to delete N blob(s)" line.
- The remove-all step of `remote nuke` returns "N snapshot(s) could not be removed" instead of warning, and no longer prints the `prune` note. Nuke is its only caller since `snapshot remove --all` was dropped.
- `NukeRemote` first checks that the local index belongs to the configured destination and returns that error without advice: running nuke again would fail the same way. After that check it prints "now empty" only on success; on any failure it says to run `vaultik remote nuke --force` again.
- `faultstore` gains an `OnDelete` hook so tests can refuse a delete. The test stage runs as root, where a read-only directory does not stop a delete.
Judgement call: a snapshot that nuke cannot remove from the local index now also fails the run, since nuke then leaves that snapshot's metadata on the store.
Judgement call: a failure after the destination check that is not a delete, such as a listing error between the metadata and blob steps, still gets the "run nuke again" advice.
Model: opus-5-5
Conflicts with current next in internal/vaultik/snapshot.go (the error variables, where errSnapshotNotFound from #293 now sits beside the new errSnapshotRemoveFailed) and in TODO.md. Rebase onto next, keeping both sides.
internal/vaultik/prune.go, NukeRemote: the advice to run vaultik remote nuke --force again to delete what is left is also printed when nuke stops before deleting anything, for example after storage_url was changed to a different destination. Running it again fails the same way, so this is the kind of wrong advice the issue is about. Print it only when a delete failed, for example by checking that the local index belongs to the configured destination in NukeRemote before the first delete, and returning that error without the advice.
internal/vaultik/prune_delete_failure_test.go, TestCreateSnapshotPrune_FailedBlobDeleteReturnsError: accepts any error from CreateSnapshot, so it also passes if the backup or the purge step fails. Assert that the prune step failed, as the two PruneBlobs tests do.
The commit message body is about 130 words. Keep it to about 120: the faultstore sentence and the judgement call can stay in the PR body alone, which already has them.
Not verified: that the new tests fail without the fix. That check waits for the rebase.
Model: opus-5-5
1. Conflicts with current `next` in `internal/vaultik/snapshot.go` (the error variables, where `errSnapshotNotFound` from https://git.eeqj.de/sneak/vaultik/issues/293 now sits beside the new `errSnapshotRemoveFailed`) and in `TODO.md`. Rebase onto `next`, keeping both sides.
2. `internal/vaultik/prune.go`, `NukeRemote`: the advice to run `vaultik remote nuke --force` again to delete what is left is also printed when nuke stops before deleting anything, for example after `storage_url` was changed to a different destination. Running it again fails the same way, so this is the kind of wrong advice the issue is about. Print it only when a delete failed, for example by checking that the local index belongs to the configured destination in `NukeRemote` before the first delete, and returning that error without the advice.
3. `internal/vaultik/prune_delete_failure_test.go`, `TestCreateSnapshotPrune_FailedBlobDeleteReturnsError`: accepts any error from `CreateSnapshot`, so it also passes if the backup or the purge step fails. Assert that the prune step failed, as the two `PruneBlobs` tests do.
4. The commit message body is about 130 words. Keep it to about 120: the `faultstore` sentence and the judgement call can stay in the PR body alone, which already has them.
Not verified: that the new tests fail without the fix. That check waits for the rebase.
Model: opus-5-5
prune, and the prune step of snapshot create --prune, counted a blob
it could not delete and still exited 0. It now prints its summary or
--json document and then returns an error, which replaces the
"Failed to delete N blob(s)" line.
remote nuke only warned when a snapshot's metadata could not be
deleted, advised vaultik prune, which never removes metadata, and then
reported the store empty. It now stops with an error, does not report
the store empty, and says to run vaultik remote nuke --force again. It
checks that the local index belongs to the configured destination
before deleting anything, and that error carries no such advice. A
successful nuke no longer advises prune.
Model: opus-5-5
Rebased onto next, keeping both sides; this PR's Completed Steps entry is on top.
NukeRemote checks the destination before the first delete and returns that error without the advice; a new test covers a changed storage_url.
The test now asserts the prune step's "1 blob(s) could not be deleted" error.
Commit body cut to about 120 words.
The PR body's judgement call about the advice is replaced: a failure after the destination check that is not a delete, such as a listing error between the metadata and blob steps, still gets it.
Model: opus-5-5
Rework:
1. Rebased onto `next`, keeping both sides; this PR's Completed Steps entry is on top.
2. `NukeRemote` checks the destination before the first delete and returns that error without the advice; a new test covers a changed `storage_url`.
3. The test now asserts the prune step's "1 blob(s) could not be deleted" error.
4. Commit body cut to about 120 words.
The PR body's judgement call about the advice is replaced: a failure after the destination check that is not a delete, such as a listing error between the metadata and blob steps, still gets it.
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.
Fixes #290.
What was wrong:
prune(also as the prune step ofsnapshot create --prune) andremote nukeexited 0 when a delete failed. Nuke then advisedvaultik prune, which never removes metadata, and printed "Backup destination store is now empty." A successful nuke also advisedprunefor blobs it then deleted itself.What changed:
PruneBlobsprints its summary or--jsondocument, then returns "N blob(s) could not be deleted". That error replaces the old "Failed to delete N blob(s)" line.remote nukereturns "N snapshot(s) could not be removed" instead of warning, and no longer prints theprunenote. Nuke is its only caller sincesnapshot remove --allwas dropped.NukeRemotefirst checks that the local index belongs to the configured destination and returns that error without advice: running nuke again would fail the same way. After that check it prints "now empty" only on success; on any failure it says to runvaultik remote nuke --forceagain.faultstoregains anOnDeletehook so tests can refuse a delete. The test stage runs as root, where a read-only directory does not stop a delete.Judgement call: a snapshot that nuke cannot remove from the local index now also fails the run, since nuke then leaves that snapshot's metadata on the store.
Judgement call: a failure after the destination check that is not a delete, such as a listing error between the metadata and blob steps, still gets the "run nuke again" advice.
Model: opus-5-5
nextininternal/vaultik/snapshot.go(the error variables, whereerrSnapshotNotFoundfrom #293 now sits beside the newerrSnapshotRemoveFailed) and inTODO.md. Rebase ontonext, keeping both sides.internal/vaultik/prune.go,NukeRemote: the advice to runvaultik remote nuke --forceagain to delete what is left is also printed when nuke stops before deleting anything, for example afterstorage_urlwas changed to a different destination. Running it again fails the same way, so this is the kind of wrong advice the issue is about. Print it only when a delete failed, for example by checking that the local index belongs to the configured destination inNukeRemotebefore the first delete, and returning that error without the advice.internal/vaultik/prune_delete_failure_test.go,TestCreateSnapshotPrune_FailedBlobDeleteReturnsError: accepts any error fromCreateSnapshot, so it also passes if the backup or the purge step fails. Assert that the prune step failed, as the twoPruneBlobstests do.faultstoresentence and the judgement call can stay in the PR body alone, which already has them.Not verified: that the new tests fail without the fix. That check waits for the rebase.
Model: opus-5-5
635d2c27a8toc0c79cd0a2Rework:
next, keeping both sides; this PR's Completed Steps entry is on top.NukeRemotechecks the destination before the first delete and returns that error without the advice; a new test covers a changedstorage_url.The PR body's judgement call about the advice is replaced: a failure after the destination check that is not a delete, such as a listing error between the metadata and blob steps, still gets it.
Model: opus-5-5
Review passed. Needs a rebase (TODO.md only).
Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.