Make snapshot rm clean up the remote by default

snapshot rm <id> now does the full cleanup: removes the local index entry, strips the snapshot's metadata from the destination store, and prunes any blobs that were only referenced by the just-removed manifest. The --remote flag is retired; --local-only opts out for the rare case where the user wants to forget a snapshot locally without touching the remote. If the destination store is unreachable, the local-DB removal still completes and a warning is emitted; the user can rerun 'vaultik prune' to retry the remote half later. RemoveAllSnapshots gets the same treatment: after deleting every snapshot's metadata (local + remote + orphan keys), an automatic blob prune sweep removes the now-unreferenced blob set.
2026-06-28 06:10:26 +02:00
parent 017ad7d3a6
commit b39d765374
5 changed files with 208 additions and 117 deletions
@@ -973,11 +973,11 @@ func (v *Vaultik) syncWithRemote() error {

 // RemoveOptions contains options for the snapshot remove command
 type RemoveOptions struct {
-	Force  bool
-	DryRun bool
-	JSON   bool
-	Remote bool // Also remove metadata from remote storage
-	All    bool // Remove all snapshots (requires Force)
+	Force     bool
+	DryRun    bool
+	JSON      bool
+	LocalOnly bool // Skip remote cleanup; only touch the local index
+	All       bool // Remove all snapshots (requires Force)
 }

 // RemoveResult contains the result of a snapshot removal
@@ -985,11 +985,17 @@ type RemoveResult struct {
 	SnapshotID       string   `json:"snapshot_id,omitempty"`
 	SnapshotsRemoved []string `json:"snapshots_removed,omitempty"`
 	RemoteRemoved    bool     `json:"remote_removed,omitempty"`
+	BlobsDeleted     int      `json:"blobs_deleted,omitempty"`
+	BytesFreed       int64    `json:"bytes_freed,omitempty"`
 	DryRun           bool     `json:"dry_run,omitempty"`
 }

-// RemoveSnapshot removes a snapshot from the local database and optionally from remote storage
-// Note: This does NOT remove blobs. Use 'vaultik prune' to remove orphaned blobs.
+// RemoveSnapshot removes a snapshot from the local index database and,
+// unless LocalOnly is set, also strips the snapshot's metadata from the
+// destination store and prunes any blobs that are no longer referenced
+// by any remaining remote snapshot. When the remote is unreachable the
+// command still completes the local-DB removal and warns; callers can
+// retry remote cleanup later with `vaultik prune`.
 func (v *Vaultik) RemoveSnapshot(snapshotID string, opts *RemoveOptions) (*RemoveResult, error) {
 	result := &RemoveResult{
 		SnapshotID: snapshotID,
@@ -999,8 +1005,8 @@ func (v *Vaultik) RemoveSnapshot(snapshotID string, opts *RemoveOptions) (*Remov
 		result.DryRun = true
 		if !opts.JSON {
 			v.printfStdout("Would remove snapshot: %s\n", snapshotID)
-			if opts.Remote {
-				v.printlnStdout("Would also remove from remote storage")
+			if !opts.LocalOnly {
+				v.printlnStdout("Would also remove metadata and any unique blobs from remote storage")
 			}
 			v.printlnStdout("[Dry run - no changes made]")
 		}
@@ -1010,12 +1016,11 @@ func (v *Vaultik) RemoveSnapshot(snapshotID string, opts *RemoveOptions) (*Remov
 		return result, nil
 	}

-	// Confirm unless --force is used (skip in JSON mode - require --force)
 	if !opts.Force && !opts.JSON {
-		if opts.Remote {
-			v.printfStdout("Remove snapshot '%s' from local database and remote storage? [y/N] ", snapshotID)
+		if opts.LocalOnly {
+			v.printfStdout("Remove snapshot '%s' from local database (remote untouched)? [y/N] ", snapshotID)
 		} else {
-			v.printfStdout("Remove snapshot '%s' from local database? [y/N] ", snapshotID)
+			v.printfStdout("Remove snapshot '%s' from local database AND remote storage (including any blobs unique to this snapshot)? [y/N] ", snapshotID)
 		}
 		var confirm string
 		if _, err := v.scanStdin(&confirm); err != nil {
@@ -1030,45 +1035,90 @@ func (v *Vaultik) RemoveSnapshot(snapshotID string, opts *RemoveOptions) (*Remov

 	log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)

-	// Remove from local database
 	if err := v.deleteSnapshotFromLocalDB(snapshotID); err != nil {
 		return result, fmt.Errorf("removing from local database: %w", err)
 	}

-	// If --remote, also remove from remote storage
-	if opts.Remote {
+	if !opts.LocalOnly {
 		log.Info("Removing snapshot metadata from remote storage", "snapshot_id", snapshotID)
-		if err := v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID)); err != nil {
-			return result, fmt.Errorf("removing from remote storage: %w", err)
+		remoteKey := snapshot.RemoteSnapshotKey(snapshotID)
+		if err := v.deleteRemoteSnapshotByKey(remoteKey); err != nil {
+			// Per design: warn-and-proceed; the local-DB removal has
+			// already happened, so the user can retry remote cleanup
+			// with `vaultik prune` later.
+			log.Warn("Could not remove snapshot metadata from remote storage", "error", err)
+			if v.UI != nil {
+				v.UI.Warning("Could not remove snapshot metadata from remote: %v. Run 'vaultik prune' once the remote is reachable to finish cleanup.", err)
+			}
+		} else {
+			result.RemoteRemoved = true
+			blobsDeleted, bytesFreed, pruneErr := v.pruneUnreferencedBlobsAfterRemoval()
+			if pruneErr != nil {
+				log.Warn("Failed to prune unreferenced blobs after snapshot removal", "error", pruneErr)
+				if v.UI != nil {
+					v.UI.Warning("Snapshot metadata removed, but blob cleanup failed: %v. Run 'vaultik prune' to retry.", pruneErr)
+				}
+			} else {
+				result.BlobsDeleted = blobsDeleted
+				result.BytesFreed = bytesFreed
+			}
 		}
-		result.RemoteRemoved = true
 	}

-	// Clean up the local rows that just became orphaned (files, chunks,
-	// blob_chunks, blobs no longer referenced by any snapshot). This
-	// used to be a separate `vaultik snapshot prune` step; running it
-	// inline means `snapshot remove` leaves no ghost rows behind.
 	if v.SnapshotManager != nil {
 		if err := v.SnapshotManager.CleanupOrphanedData(v.ctx); err != nil {
 			log.Warn("Failed to clean up orphaned local data after removal", "error", err)
 		}
 	}

-	// Output result
 	if opts.JSON {
 		return result, v.outputRemoveJSON(result)
 	}

-	// Print summary
 	v.printfStdout("Removed snapshot '%s' from local database\n", snapshotID)
-	if opts.Remote {
+	if !opts.LocalOnly && result.RemoteRemoved {
 		v.printlnStdout("Removed snapshot metadata from remote storage")
-		v.printlnStdout("\nNote: Remote blobs were not removed. Run 'vaultik prune' to remove orphaned blobs.")
+		if result.BlobsDeleted > 0 {
+			v.printfStdout("Removed %d unreferenced blob(s) (%s freed)\n",
+				result.BlobsDeleted, humanize.Bytes(uint64(result.BytesFreed)))
+		} else {
+			v.printlnStdout("No blobs unique to this snapshot were found.")
+		}
 	}

 	return result, nil
 }

+// pruneUnreferencedBlobsAfterRemoval deletes blobs no longer referenced
+// by any remaining remote manifest. Used by RemoveSnapshot /
+// RemoveAllSnapshots after metadata has been stripped from the
+// destination store; with the just-removed snapshot's manifest gone,
+// any blobs that were only referenced by it become unreferenced and
+// are swept here.
+func (v *Vaultik) pruneUnreferencedBlobsAfterRemoval() (int, int64, error) {
+	referenced, err := v.collectReferencedBlobs()
+	if err != nil {
+		return 0, 0, fmt.Errorf("collecting referenced blobs: %w", err)
+	}
+
+	allBlobs, err := v.listAllRemoteBlobs()
+	if err != nil {
+		return 0, 0, fmt.Errorf("listing remote blobs: %w", err)
+	}
+
+	unreferenced, totalSize := v.findUnreferencedBlobs(allBlobs, referenced)
+	if len(unreferenced) == 0 {
+		return 0, 0, nil
+	}
+
+	result := &PruneBlobsResult{BlobsFound: len(unreferenced)}
+	log.Info("Pruning unreferenced blobs after snapshot removal",
+		"count", len(unreferenced),
+		"size", humanize.Bytes(uint64(totalSize)))
+	v.deleteUnreferencedBlobs(unreferenced, allBlobs, result)
+	return result.BlobsDeleted, result.BytesFreed, nil
+}
+
 // RemoveAllSnapshots removes every snapshot known to the local
 // database from the local index, and (with --remote) every snapshot
 // metadata directory in remote storage. Both sides are processed so a
@@ -1176,7 +1226,7 @@ func (v *Vaultik) listAllRemoteSnapshotKeys() ([]string, error) {
 func (v *Vaultik) handleRemoveAllDryRun(localSnaps, orphanRemoteKeys []string, opts *RemoveOptions) (*RemoveResult, error) {
 	result := &RemoveResult{DryRun: true}
 	result.SnapshotsRemoved = append(result.SnapshotsRemoved, localSnaps...)
-	if opts.Remote {
+	if !opts.LocalOnly {
 		result.SnapshotsRemoved = append(result.SnapshotsRemoved, orphanRemoteKeys...)
 	}
 	if !opts.JSON {
@@ -1184,13 +1234,16 @@ func (v *Vaultik) handleRemoveAllDryRun(localSnaps, orphanRemoteKeys []string, o
 		for _, id := range localSnaps {
 			v.printfStdout("  %s\n", id)
 		}
-		if opts.Remote && len(orphanRemoteKeys) > 0 {
-			v.printfStdout("Would also remove %d orphan remote snapshot key(s):\n", len(orphanRemoteKeys))
-			for _, key := range orphanRemoteKeys {
-				v.printfStdout("  %s\n", key)
+		if !opts.LocalOnly {
+			if len(orphanRemoteKeys) > 0 {
+				v.printfStdout("Would also remove %d orphan remote snapshot key(s):\n", len(orphanRemoteKeys))
+				for _, key := range orphanRemoteKeys {
+					v.printfStdout("  %s\n", key)
+				}
+			} else {
+				v.printlnStdout("Would also remove from remote storage")
 			}
-		} else if opts.Remote {
-			v.printlnStdout("Would also remove from remote storage")
+			v.printlnStdout("Would then prune all unreferenced blobs from remote storage")
 		}
 		v.printlnStdout("[Dry run - no changes made]")
 	}
@@ -1200,11 +1253,11 @@ func (v *Vaultik) handleRemoveAllDryRun(localSnaps, orphanRemoteKeys []string, o
 	return result, nil
 }

-// executeRemoveAll deletes every local snapshot (and, with --remote,
-// every corresponding remote metadata directory plus any orphan remote
-// keys that don't match a local snapshot).
+// executeRemoveAll deletes every local snapshot and, unless LocalOnly
+// is set, every corresponding remote metadata directory plus any
+// orphan remote keys, then prunes the resulting set of unreferenced
+// blobs from the destination store.
 func (v *Vaultik) executeRemoveAll(localSnaps, orphanRemoteKeys []string, opts *RemoveOptions) (*RemoveResult, error) {
-	// --all requires --force
 	if !opts.Force {
 		return nil, fmt.Errorf("--all requires --force")
 	}
@@ -1212,6 +1265,7 @@ func (v *Vaultik) executeRemoveAll(localSnaps, orphanRemoteKeys []string, opts *
 	log.Info("Removing all snapshots", "local_count", len(localSnaps), "orphan_remote_count", len(orphanRemoteKeys))

 	result := &RemoveResult{}
+	remoteErrors := 0
 	for _, snapshotID := range localSnaps {
 		log.Info("Removing snapshot", "snapshot_id", snapshotID)

@@ -1220,33 +1274,44 @@ func (v *Vaultik) executeRemoveAll(localSnaps, orphanRemoteKeys []string, opts *
 			continue
 		}

-		if opts.Remote {
+		if !opts.LocalOnly {
 			if err := v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID)); err != nil {
-				log.Error("Failed to remove from remote", "snapshot_id", snapshotID, "error", err)
-				continue
+				log.Warn("Failed to remove snapshot metadata from remote", "snapshot_id", snapshotID, "error", err)
+				remoteErrors++
 			}
 		}

 		result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID)
 	}

-	if opts.Remote {
+	if !opts.LocalOnly {
 		for _, key := range orphanRemoteKeys {
 			log.Info("Removing orphan remote snapshot", "remote_key", key)
 			if err := v.deleteRemoteSnapshotByKey(key); err != nil {
-				log.Error("Failed to remove orphan from remote", "remote_key", key, "error", err)
+				log.Warn("Failed to remove orphan from remote", "remote_key", key, "error", err)
+				remoteErrors++
 				continue
 			}
 			result.SnapshotsRemoved = append(result.SnapshotsRemoved, key)
 		}
+
+		if remoteErrors == 0 {
+			result.RemoteRemoved = true
+			blobsDeleted, bytesFreed, pruneErr := v.pruneUnreferencedBlobsAfterRemoval()
+			if pruneErr != nil {
+				log.Warn("Failed to prune unreferenced blobs after bulk removal", "error", pruneErr)
+				if v.UI != nil {
+					v.UI.Warning("Bulk metadata removal succeeded, but blob cleanup failed: %v. Run 'vaultik prune' to retry.", pruneErr)
+				}
+			} else {
+				result.BlobsDeleted = blobsDeleted
+				result.BytesFreed = bytesFreed
+			}
+		} else if v.UI != nil {
+			v.UI.Warning("Some remote metadata deletions failed; skipping automatic blob prune. Run 'vaultik prune' once the remote is healthy.")
+		}
 	}

-	if opts.Remote {
-		result.RemoteRemoved = true
-	}
-
-	// Clean up everything that just became orphaned locally so the
-	// index database doesn't carry 39k ghost rows after a wipe.
 	if v.SnapshotManager != nil {
 		if err := v.SnapshotManager.CleanupOrphanedData(v.ctx); err != nil {
 			log.Warn("Failed to clean up orphaned local data after bulk removal", "error", err)
@@ -1258,9 +1323,14 @@ func (v *Vaultik) executeRemoveAll(localSnaps, orphanRemoteKeys []string, opts *
 	}

 	v.printfStdout("Removed %d snapshot(s)\n", len(result.SnapshotsRemoved))
-	if opts.Remote {
+	if !opts.LocalOnly && result.RemoteRemoved {
 		v.printlnStdout("Removed snapshot metadata from remote storage")
-		v.printlnStdout("\nNote: Remote blobs were not removed. Run 'vaultik prune' to remove orphaned blobs.")
+		if result.BlobsDeleted > 0 {
+			v.printfStdout("Removed %d unreferenced blob(s) (%s freed)\n",
+				result.BlobsDeleted, humanize.Bytes(uint64(result.BytesFreed)))
+		} else {
+			v.printlnStdout("No unreferenced blobs were found.")
+		}
 	}

 	return result, nil