ARCHITECTURE.md

@@ -385,11 +385,12 @@ bucket/
 A destination without `vaultik-destination.txt` is treated as not present
 while the local index holds snapshots stored there (`checkDestinationMarker`
 in `internal/vaultik/destination_marker.go`): `snapshot create` refuses to
-write to it, `prune` and `snapshot purge` fail, and `snapshot list` and
-`snapshot remove` behave as for a destination that cannot be listed. This
-keeps those commands from taking an unplugged volume's empty mount point for
-a destination whose snapshots are all gone. The `file://` backend never
-creates its destination directory, so a write to a missing one fails.
+write to it, `prune`, `snapshot purge` and `remote nuke` fail, and
+`snapshot list` and `snapshot remove` behave as for a destination that cannot
+be listed. This keeps those commands from taking an unplugged volume's empty
+mount point for a destination whose snapshots are all gone. The `file://`
+backend never creates its destination directory, so a write to a missing one
+fails.
 
 The `{remote-key}` directory name is a one-way double SHA-256 hash of the human
 snapshot ID, so the human ID (hostname, snapshot name, timestamp) is never

README.md

@@ -454,9 +454,14 @@ since `prune` never deletes it.
 the backup destination store, whatever its name, and then
 `vaultik-destination.txt`, leaving the bucket prefix empty. Destructive
 and irreversible. This is the single supported way to wipe the entire
-destination store. If a listing afterwards still finds anything under
-`metadata/` or `blobs/`, the command names each object left and exits
-non-zero; running it again deletes what is left.
+destination store. A snapshot is removed from the local index only once
+its metadata is deleted from the destination store. If the destination
+store cannot be listed, or the local index holds snapshots and the
+destination has no `vaultik-destination.txt` (an unplugged volume's
+empty mount point), the command fails before it removes anything. If a
+listing afterwards still finds anything under `metadata/` or `blobs/`,
+the command names each object left and exits non-zero; running it again
+deletes what is left.
 * `--force`: Required to confirm destruction.
 
 **`database delete`**: Delete the local SQLite state database file
@@ -529,10 +534,11 @@ For an implementation-level view of the internals — the data model, the
 * `vaultik-destination.txt` holds one fixed line of text, with no host
   name, path or time. A destination without it is treated as not
   present while the local index holds snapshots stored there:
-  `snapshot create` refuses to write to it, `prune` and `snapshot purge`
-  fail, and `snapshot list` and `snapshot remove` behave as for a
-  destination that cannot be listed. This keeps an unplugged volume's empty mount point
-  from being taken for a destination whose snapshots are all gone. Mount
+  `snapshot create` refuses to write to it, `prune`, `snapshot purge`
+  and `remote nuke` fail, and `snapshot list` and `snapshot remove`
+  behave as for a destination that cannot be listed. This keeps an
+  unplugged volume's empty mount point from being taken for a
+  destination whose snapshots are all gone. Mount
   the volume if it is removable; if the destination was emptied on
   purpose, run `vaultik database delete` and back up again.
 * Blobs are two-level directory sharded using the first 4 hex chars of the blob hash

docs/REPOSTRUCTURE.md

@@ -27,7 +27,7 @@ The metadata subdirectory is named with the **remote key**, a one-way hash of th
 
 ## Marker File (`vaultik-destination.txt`)
 
-Every backup writes this file at the root before it uploads anything. It holds one fixed line of plain text and names no host, path or time. A destination without it is treated as not present while the local index holds snapshots stored there: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove` and `snapshot list` behave as for a destination that cannot be listed. An unplugged volume leaves an empty directory at its mount point, and without the marker that directory would read as a destination whose snapshots are all gone. `remote nuke` deletes the marker last.
+Every backup writes this file at the root before it uploads anything. It holds one fixed line of plain text and names no host, path or time. A destination without it is treated as not present while the local index holds snapshots stored there: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove`, `snapshot list` and `remote nuke` behave as for a destination that cannot be listed. An unplugged volume leaves an empty directory at its mount point, and without the marker that directory would read as a destination whose snapshots are all gone. `remote nuke` deletes the marker last.
 
 ## Blobs Directory (`blobs/`)
 

internal/vaultik/destination_marker_test.go

@@ -152,6 +152,25 @@ func TestPurgeRefusesDestinationWithoutMarker(t *testing.T) {
 	assertUntouched(ctx, t, repos, storeDir)
 }
 
+// TestNukeRemoteRefusesDestinationWithoutMarker checks that remote nuke
+// fails, deletes no local snapshot record, and neither reports the
+// destination empty nor advises running nuke again when the destination
+// has no marker.
+func TestNukeRemoteRefusesDestinationWithoutMarker(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	v, repos, out, storeDir := backUpThenUnplugMountPoint(ctx, t)
+
+	err := v.NukeRemote(true)
+	require.ErrorContains(t, err, markerFile)
+	assert.NotContains(t, out.String(), "now empty")
+	assert.NotContains(t, out.String(), "vaultik remote nuke --force")
+
+	assertUntouched(ctx, t, repos, storeDir)
+}
+
 // TestRemoveSnapshotWarnsWhenDestinationHasNoMarker checks that snapshot
 // remove warns that the metadata could not be removed from a destination
 // without a marker, instead of reporting that it was.

internal/vaultik/missing_destination_test.go

@@ -188,3 +188,41 @@ func TestPurgeSaysListingFailedOnceWhenDestinationMissing(t *testing.T) {
 	assert.Equal(t, 1, strings.Count(err.Error(), "listing remote snapshots"),
 		err.Error())
 }
+
+// TestNukeRemoteKeepsLocalIndexWhenDestinationMissing checks that remote
+// nuke fails on a destination it cannot list, leaves the local index
+// unchanged, and neither reports the destination empty nor advises
+// running nuke again.
+func TestNukeRemoteKeepsLocalIndexWhenDestinationMissing(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	v, repos, out := backUpThenUnplug(ctx, t)
+
+	filesBefore, err := repos.Files.ListAll(ctx)
+	require.NoError(t, err)
+	require.NotEmpty(t, filesBefore)
+
+	blobsBefore, err := repos.Blobs.GetAll(ctx)
+	require.NoError(t, err)
+	require.NotEmpty(t, blobsBefore)
+
+	err = v.NukeRemote(true)
+	require.ErrorIs(t, err, fs.ErrNotExist)
+	require.ErrorContains(t, err, "listing remote snapshots")
+	assert.NotContains(t, out.String(), "now empty")
+	assert.NotContains(t, out.String(), "vaultik remote nuke --force")
+
+	snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
+	require.NoError(t, err)
+	assert.Len(t, snapshots, 1, "nuke must delete no local snapshot record")
+
+	files, err := repos.Files.ListAll(ctx)
+	require.NoError(t, err)
+	assert.Len(t, files, len(filesBefore), "nuke must delete no local file record")
+
+	blobs, err := repos.Blobs.GetAll(ctx)
+	require.NoError(t, err)
+	assert.Len(t, blobs, len(blobsBefore), "nuke must delete no local blob record")
+}

internal/vaultik/nuke_remote_test.go

@@ -104,6 +104,32 @@ func TestNukeRemoteFailsWhenADeleteFails(t *testing.T) {
 	}
 }
 
+// TestNukeRemoteKeepsLocalSnapshotWhoseMetadataIsNotDeleted checks that
+// remote nuke keeps a snapshot in the local index while its metadata
+// cannot be deleted from the destination store: dropped from the local
+// index, it would be left there as a remote-only snapshot without its
+// name.
+func TestNukeRemoteKeepsLocalSnapshotWhoseMetadataIsNotDeleted(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	storeDir := filepath.Join(t.TempDir(), "store")
+	v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
+
+	store := faultstore.New(v.Storage)
+	store.OnDelete = func(key string) bool {
+		return strings.HasPrefix(key, "metadata/")
+	}
+	v.Storage = store
+
+	require.Error(t, v.NukeRemote(true))
+
+	snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
+	require.NoError(t, err)
+	assert.Len(t, snapshots, 1)
+}
+
 // TestNukeRemoteNamesWhatIsLeft checks that remote nuke, when an object
 // it does not recognise cannot be deleted, does not report the store
 // empty but names that object and fails.
@@ -154,8 +180,7 @@ func TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice(t *testing.T) {
 
 // TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved checks that a
 // snapshot remote nuke cannot remove from the local index fails the
-// run: nuke then leaves that snapshot's metadata on the destination
-// store.
+// run, and that nuke then does not report the destination store empty.
 func TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()

internal/vaultik/prune.go

@@ -55,12 +55,19 @@ func (v *Vaultik) NukeRemote(force bool) error {
 	}
 
 	// Checked before anything is deleted: running nuke again would fail
-	// the same way, so the advice below must not follow this error.
+	// the same way, so the advice below must not follow these errors.
+	// The listing also fails on a destination without the marker while
+	// the local index holds snapshots.
 	err := v.EnsureStorageBinding()
 	if err != nil {
 		return err
 	}
 
+	_, err = v.listAllRemoteSnapshotKeys()
+	if err != nil {
+		return err
+	}
+
 	err = v.emptyDestinationStore()
 	if err != nil {
 		v.UI.Warningf("Run '%s' again to delete what is left on the "+

internal/vaultik/snapshot.go

@@ -1516,7 +1516,9 @@ func (v *Vaultik) removeSnapshotRemote(
 // database from the local index, and (with --remote) every snapshot
 // metadata directory in remote storage. Both sides are processed so a
 // "remove --all" leaves nothing behind, even when the local DB and
-// remote storage have diverged.
+// remote storage have diverged. Unless LocalOnly is set, it fails
+// before it removes anything when the destination store cannot be
+// listed or is not present by checkDestinationMarker.
 func (v *Vaultik) RemoveAllSnapshots(opts *RemoveOptions) (*RemoveResult, error) {
 	err := v.EnsureStorageBinding()
 	if err != nil {
@@ -1529,11 +1531,15 @@ func (v *Vaultik) RemoveAllSnapshots(opts *RemoveOptions) (*RemoveResult, error)
 	}
 
 	// remoteKeys is the set of metadata/<key>/ subdirectories on the
-	// destination store; failures are downgraded to a warning so a
-	// permission-denied or unreachable remote can't block a local-only
-	// remove.
+	// destination store. Only for a local-only remove is a failure
+	// downgraded to a warning, so a permission-denied or unreachable
+	// remote can't block it.
 	remoteKeys, remoteErr := v.listAllRemoteSnapshotKeys()
 	if remoteErr != nil {
+		if !opts.LocalOnly {
+			return nil, remoteErr
+		}
+
 		log.Warn("Could not list remote snapshots", "error", remoteErr)
 		v.UI.Warningf("Could not list remote snapshots: %v.", remoteErr)
 	}
@@ -1595,8 +1601,7 @@ func (v *Vaultik) localSnapshotIDs() ([]string, error) {
 // listAllRemoteSnapshotKeys collects the hashed remote keys
 // (subdirectories under metadata/) currently present in the
 // destination store. Returns (nil, err) when the store cannot be
-// listed, or is not present by checkDestinationMarker; callers must
-// treat that as "no remote info available," not fatal.
+// listed, or is not present by checkDestinationMarker.
 func (v *Vaultik) listAllRemoteSnapshotKeys() ([]string, error) {
 	log.Info("Listing all remote snapshots")
 
@@ -1780,10 +1785,12 @@ func (v *Vaultik) deleteIncompleteSnapshot(snapshotIDStr string, id any) bool {
 	return true
 }
 
-// removeAllLocalSnapshots removes each known local snapshot (and, unless
-// --local-only, its remote metadata), returning the number of snapshots
-// it could not remove. A snapshot left in the local index keeps its
-// remote metadata too, since it is not an orphan remote key.
+// removeAllLocalSnapshots removes each known local snapshot's remote
+// metadata, unless --local-only, and then its local record, returning
+// the number of snapshots it could not remove. The local record goes
+// only once the metadata is deleted: dropped first, it would leave that
+// metadata on the destination store as a remote-only snapshot whose name
+// is lost.
 func (v *Vaultik) removeAllLocalSnapshots(
 	localSnaps []string, opts *RemoveOptions, result *RemoveResult,
 ) int {
@@ -1792,6 +1799,19 @@ func (v *Vaultik) removeAllLocalSnapshots(
 	for _, snapshotID := range localSnaps {
 		log.Info("Removing snapshot", "snapshot_id", snapshotID)
 
+		if !opts.LocalOnly {
+			_, err := v.deleteRemoteSnapshotByKey(
+				snapshot.RemoteSnapshotKey(snapshotID))
+			if err != nil {
+				log.Warn("Failed to remove snapshot metadata from remote",
+					"snapshot_id", snapshotID, "error", err)
+
+				failures++
+
+				continue
+			}
+		}
+
 		err := v.deleteSnapshotFromLocalDB(snapshotID)
 		if err != nil {
 			log.Error("Failed to remove from local database",
@@ -1802,17 +1822,6 @@ func (v *Vaultik) removeAllLocalSnapshots(
 			continue
 		}
 
-		if !opts.LocalOnly {
-			_, err = v.deleteRemoteSnapshotByKey(
-				snapshot.RemoteSnapshotKey(snapshotID))
-			if err != nil {
-				log.Warn("Failed to remove snapshot metadata from remote",
-					"snapshot_id", snapshotID, "error", err)
-
-				failures++
-			}
-		}
-
 		result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID)
 	}