Keep the local index when remote nuke cannot list or find the destination (closes #292)

`remote nuke` took a destination store it could not list, or one
without `vaultik-destination.txt` while the local index held
snapshots, as a warning: it then cleared the local snapshot records,
and for an unplugged volume's empty mount point reported the store
empty and exited zero. The bulk remove nuke runs now fails on either
before it removes anything; only a local-only remove still goes on
with a warning. It also deletes each snapshot's remote metadata before
its local record, so a failed delete keeps the snapshot in the local
index under its name instead of leaving it on the destination store
as a remote-only snapshot.

Model: opus-5-5

internal/vaultik/destination_marker_test.go

@@ -152,6 +152,23 @@ func TestPurgeRefusesDestinationWithoutMarker(t *testing.T) {
 	assertUntouched(ctx, t, repos, storeDir)
 }
 
+// TestNukeRemoteRefusesDestinationWithoutMarker checks that remote nuke
+// fails, deletes no local snapshot record and does not report the
+// destination empty 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")
+
+	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,39 @@ 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 does not report the destination empty.
+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")
+
+	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/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)
 	}