snapshot remove accepts a remote-only snapshot's remote key (closes #400)

A host with no local index could remove a remote-only snapshot only by
its plain snapshot ID, which it does not have. snapshot remove passed
its argument through RemoteSnapshotKey, rehashing a remote key so it
never matched. It now resolves a remote key, or an unambiguous leading
part of it, with the same resolveSnapshotRemoteKey that verify and
restore use. The prompt, the dry run and the warning after a failed
remote delete name the snapshot it resolved to: its ID when known,
otherwise its full remote key. Under --json, snapshot_id holds the ID
only when it is known, and remote_key holds the full key.

Model: deepseek-v4-pro (change); opus-5-5 (rebase onto next, rework)

README.md

@@ -448,6 +448,8 @@ remove the snapshot's metadata from it (`vaultik prune` does not). For a
 snapshot that is only on the destination store, that failure is an error. To
 wipe everything on the destination in one go, use `vaultik remote nuke --force`.
 
+- Accepts a snapshot ID, or — for a snapshot only on the destination store — its
+  remote key (or an unambiguous leading part of it) as shown by `snapshot list`.
 - `--local-only`: Skip remote cleanup; only touch the local index
 - `--dry-run`: Show what would be deleted without deleting
 - `--force`: Skip confirmation prompt

internal/cli/snapshot.go

@@ -288,6 +288,10 @@ know about), which is what 'vaultik prune' does. The command prints the
 'vaultik prune' invocation to run as a follow-up only when it removed the
 snapshot's metadata from the destination store, and never under --json.
 
+The snapshot may be named by its ID or, on a host with no local index, by
+the remote key that 'snapshot list' prints for a remote-only snapshot (an
+unambiguous leading part is enough).
+
 An ID that names no snapshot in the local index or on the destination
 store is an error.
 

internal/vaultik/remove_snapshot_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"database/sql"
+	"encoding/json"
 	"io"
 	"strings"
 	"sync"
@@ -472,6 +473,171 @@ func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
 	assert.Contains(t, out, "Removed snapshot metadata from remote storage")
 }
 
+// TestRemoveSnapshot_RemoteOnlyByRemoteKey checks that a snapshot only
+// on the destination store, on a host with no local index, is removed by
+// the remote key that 'snapshot list' prints for such a snapshot, and by
+// an unambiguous leading part of it — the same identifiers verify and
+// restore accept.
+func TestRemoveSnapshot_RemoteOnlyByRemoteKey(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
+
+	identifiers := map[string]string{
+		"full key":     remoteKey,
+		"leading part": remoteKey[:8],
+	}
+
+	for name, identifier := range identifiers {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			store := newTestStorer()
+			addManifest(t, store, "snapshot-001", nil)
+
+			// No local index: a host that sees the snapshot only remotely.
+			tv := vaultik.NewForTesting(store)
+			tv.Repositories = newLocalIndex(t)
+
+			result, err := tv.RemoveSnapshot(identifier,
+				&vaultik.RemoveOptions{Force: true})
+
+			require.NoError(t, err)
+			assert.True(t, result.RemoteRemoved)
+			assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
+
+			out := tv.Stdout.String()
+			assert.NotContains(t, out, "from local database")
+			assert.Contains(t, out, "Removed snapshot metadata from remote storage")
+		})
+	}
+}
+
+// TestRemoveSnapshot_LocalIndexByRemoteKey checks that a remote key (or
+// its unambiguous leading part) that names a snapshot the local index
+// holds removes that snapshot's local row and its remote metadata
+// together. `snapshot list --json` prints a remote key for every
+// snapshot, including local ones.
+func TestRemoveSnapshot_LocalIndexByRemoteKey(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
+
+	identifiers := map[string]string{
+		"full key":     remoteKey,
+		"leading part": remoteKey[:8],
+	}
+
+	for name, identifier := range identifiers {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			store := newTestStorer()
+			addManifest(t, store, "snapshot-001", nil)
+
+			// A backup wrote to this destination, so it carries the
+			// marker, and the local index holds the snapshot.
+			require.NoError(t, store.Put(context.Background(), markerFile,
+				bytes.NewReader(nil)))
+
+			tv := vaultik.NewForTesting(store)
+			repos := newLocalIndex(t, "snapshot-001")
+			tv.Repositories = repos
+
+			result, err := tv.RemoveSnapshot(identifier,
+				&vaultik.RemoveOptions{Force: true})
+
+			require.NoError(t, err)
+			assert.True(t, result.RemoteRemoved)
+			assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
+
+			row, err := repos.Snapshots.GetByID(
+				context.Background(), "snapshot-001")
+			require.NoError(t, err)
+			assert.Nil(t, row)
+
+			out := tv.Stdout.String()
+			assert.Contains(t, out,
+				"Removed snapshot 'snapshot-001' from local database")
+			assert.Contains(t, out, "Removed snapshot metadata from remote storage")
+		})
+	}
+}
+
+// TestRemoveSnapshot_DryRunByLeadingPart checks that a dry run by a
+// leading part of a remote key names the snapshot that part resolved to:
+// by its ID when the local index holds the snapshot, otherwise by its
+// full remote key.
+func TestRemoveSnapshot_DryRunByLeadingPart(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
+
+	for name, tc := range map[string]struct {
+		localIDs []string
+		shown    string
+	}{
+		"in local index": {localIDs: []string{"snapshot-001"}, shown: "snapshot-001"},
+		"remote only":    {localIDs: nil, shown: remoteKey},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			store := newTestStorer()
+			addManifest(t, store, "snapshot-001", nil)
+
+			// A backup wrote to this destination, so it carries the marker.
+			require.NoError(t, store.Put(context.Background(), markerFile,
+				bytes.NewReader(nil)))
+
+			tv := vaultik.NewForTesting(store)
+			tv.Repositories = newLocalIndex(t, tc.localIDs...)
+
+			_, err := tv.RemoveSnapshot(remoteKey[:8],
+				&vaultik.RemoveOptions{DryRun: true})
+
+			require.NoError(t, err)
+			assert.Contains(t, tv.Stdout.String(),
+				"Would remove snapshot: "+tc.shown+".")
+			assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
+		})
+	}
+}
+
+// TestRemoveSnapshot_JSONDryRunByLeadingPart checks that a JSON dry run
+// by a leading part of a remote key prints the ID and the full remote key
+// of the snapshot that part resolved to.
+func TestRemoveSnapshot_JSONDryRunByLeadingPart(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
+
+	store := newTestStorer()
+	addManifest(t, store, "snapshot-001", nil)
+
+	// A backup wrote to this destination, so it carries the marker.
+	require.NoError(t, store.Put(context.Background(), markerFile,
+		bytes.NewReader(nil)))
+
+	tv := vaultik.NewForTesting(store)
+	tv.Repositories = newLocalIndex(t, "snapshot-001")
+
+	_, err := tv.RemoveSnapshot(remoteKey[:8],
+		&vaultik.RemoveOptions{DryRun: true, JSON: true})
+	require.NoError(t, err)
+
+	var printed map[string]any
+
+	require.NoError(t, json.Unmarshal(tv.Stdout.Bytes(), &printed))
+	assert.Equal(t, "snapshot-001", printed["snapshot_id"])
+	assert.Equal(t, remoteKey, printed["remote_key"])
+	assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
+}
+
 // TestRemoveSnapshot_NotInLocalIndexDeleteFails checks that removing a
 // snapshot only the destination store holds fails when its metadata
 // cannot be deleted there, under --force and under --json: nothing was
@@ -506,6 +672,38 @@ func TestRemoveSnapshot_NotInLocalIndexDeleteFails(t *testing.T) {
 	}
 }
 
+// TestRemoveSnapshot_LocalIndexByLeadingPartDeleteFails checks the
+// warning when a snapshot the local index holds is removed by a leading
+// part of its remote key and the delete on the destination store fails.
+// The warning names the snapshot by its ID, not by the part given.
+func TestRemoveSnapshot_LocalIndexByLeadingPartDeleteFails(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	store := newTestStorer()
+	addManifest(t, store, "snapshot-001", nil)
+
+	// A backup wrote to this destination, so it carries the marker.
+	require.NoError(t, store.Put(context.Background(), markerFile,
+		bytes.NewReader(nil)))
+
+	faulty := faultstore.New(store)
+	faulty.OnDelete = func(string) bool { return true }
+
+	tv := vaultik.NewForTesting(faulty)
+	tv.Repositories = newLocalIndex(t, "snapshot-001")
+
+	result, err := tv.RemoveSnapshot(
+		snapshot.RemoteSnapshotKey("snapshot-001")[:8],
+		&vaultik.RemoveOptions{Force: true})
+
+	require.NoError(t, err)
+	assert.False(t, result.RemoteRemoved)
+	assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
+	assert.Contains(t, tv.Stdout.String(),
+		"Run 'vaultik snapshot remove snapshot-001' again")
+}
+
 func TestRemoveAllSnapshots_RequiresForce(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()

internal/vaultik/snapshot.go

@@ -1424,11 +1424,14 @@ type RemoveOptions struct {
 	LocalOnly bool // Skip remote cleanup; only touch the local index
 }
 
-// RemoveResult contains the result of a snapshot removal
+// RemoveResult contains the result of a snapshot removal. For a single
+// snapshot, SnapshotID is its ID, empty when the ID is not known, and
+// RemoteKey is the full remote key of its metadata.
 //
 //nolint:tagliatelle // snake_case is the established JSON output format
 type RemoveResult struct {
 	SnapshotID       string   `json:"snapshot_id,omitempty"`
+	RemoteKey        string   `json:"remote_key,omitempty"`
 	SnapshotsRemoved []string `json:"snapshots_removed,omitempty"`
 	RemoteRemoved    bool     `json:"remote_removed,omitempty"`
 	DryRun           bool     `json:"dry_run,omitempty"`
@@ -1458,25 +1461,35 @@ const snapshotRemoveCommandHint = "vaultik snapshot remove"
 func (v *Vaultik) RemoveSnapshot(
 	snapshotID string, opts *RemoveOptions,
 ) (*RemoveResult, error) {
-	result := &RemoveResult{
+	result := &RemoveResult{}
-		SnapshotID: snapshotID,
-	}
 
 	err := v.checkStorageBinding()
 	if err != nil {
 		return result, err
 	}
 
-	inLocalIndex, err := v.checkSnapshotExists(snapshotID, opts.LocalOnly)
+	inLocalIndex, resolvedID, remoteKey, err := v.findSnapshotToRemove(
+		snapshotID, opts.LocalOnly)
 	if err != nil {
 		return result, err
 	}
 
-	if opts.DryRun {
+	result.SnapshotID = resolvedID
-		return v.removeSnapshotDryRun(result, snapshotID, opts)
+	result.RemoteKey = remoteKey
+
+	// The dry run, the prompt and removeSnapshotRemote name the snapshot
+	// that snapshotID resolved to: by its ID, or by its full remote key
+	// when the ID is not known.
+	shown := resolvedID
+	if shown == "" {
+		shown = remoteKey
 	}
 
-	if !v.confirmRemoveSnapshot(snapshotID, opts) {
+	if opts.DryRun {
+		return v.removeSnapshotDryRun(result, shown, opts)
+	}
+
+	if !v.confirmRemoveSnapshot(shown, opts) {
 		return result, ErrNotConfirmed
 	}
 
@@ -1489,16 +1502,16 @@ func (v *Vaultik) RemoveSnapshot(
 	}
 
 	if inLocalIndex {
-		log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
+		log.Info("Removing snapshot from local database", "snapshot_id", resolvedID)
 
-		err = v.deleteSnapshotFromLocalDB(snapshotID)
+		err = v.deleteSnapshotFromLocalDB(resolvedID)
 		if err != nil {
 			return result, fmt.Errorf("removing from local database: %w", err)
 		}
 	}
 
 	result.RemoteRemoved, err = v.removeSnapshotRemote(
-		snapshotID, inLocalIndex, markerErr, opts)
+		shown, remoteKey, inLocalIndex, markerErr, opts)
 	if err != nil {
 		return result, err
 	}
@@ -1517,7 +1530,7 @@ func (v *Vaultik) RemoveSnapshot(
 
 	if inLocalIndex {
 		v.UI.Infof("Removed snapshot '%s' from local database.",
-			v.UI.Snapshot(snapshotID))
+			v.UI.Snapshot(resolvedID))
 	}
 
 	if result.RemoteRemoved {
@@ -1530,6 +1543,52 @@ func (v *Vaultik) RemoveSnapshot(
 	return result, nil
 }
 
+// findSnapshotToRemove finds the snapshot that identifier names. It
+// returns whether the local index holds that snapshot, its snapshot ID,
+// and the remote key of its metadata on the destination store. The ID is
+// known when identifier is a snapshot ID or the local index holds the
+// snapshot; otherwise it is empty.
+//
+// A snapshot ID is looked up by checkSnapshotExists. Under localOnly the
+// store is not consulted, so every identifier is looked up that way and
+// a remote key is not found. Otherwise a remote key, or a leading part
+// of one, is resolved by resolveSnapshotRemoteKey, as restore and verify
+// do. That fails with errSnapshotKeyNotFound or errSnapshotKeyAmbiguous
+// when the identifier matches no snapshot on the store, or more than
+// one. The store is listed only after checkDestinationMarker passes, as
+// in checkSnapshotExists. The local row is the one whose snapshot ID
+// hashes to the resolved key, as in RemoveAllSnapshots, so removing by
+// remote key removes that row too.
+func (v *Vaultik) findSnapshotToRemove(
+	identifier string, localOnly bool,
+) (bool, string, string, error) {
+	if localOnly || !isRemoteKeyOrPrefix(identifier) {
+		inLocalIndex, err := v.checkSnapshotExists(identifier, localOnly)
+		if err != nil {
+			return false, "", "", err
+		}
+
+		return inLocalIndex, identifier, snapshot.RemoteSnapshotKey(identifier), nil
+	}
+
+	err := v.checkDestinationMarker()
+	if err != nil {
+		return false, "", "", err
+	}
+
+	remoteKey, err := v.resolveSnapshotRemoteKey(identifier)
+	if err != nil {
+		return false, "", "", err
+	}
+
+	localID, err := v.localSnapshotIDForRemoteKey(remoteKey)
+	if err != nil {
+		return false, "", "", err
+	}
+
+	return localID != "", localID, remoteKey, nil
+}
+
 // checkSnapshotExists returns errSnapshotNotFound when the local index
 // has no row for snapshotID and, unless localOnly is set, the
 // destination store has no metadata for it either. inLocalIndex reports
@@ -1639,19 +1698,22 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
 }
 
 // removeSnapshotRemote strips the snapshot's metadata from the
-// destination store. It returns true only when the delete finished
+// destination store. remoteKey is the resolved metadata directory name
-// without an error and removed at least one object; under LocalOnly it
+// to delete, as returned by findSnapshotToRemove. shown names the
-// does nothing. markerErr is the result of checkDestinationMarker; when
+// snapshot in the log records and the warning: its ID, or its full
-// it is set, nothing is deleted and it is the failure. When the snapshot
+// remote key when the ID is not known. It returns true only when the
-// is only on the destination store, a failure is returned as an error,
+// delete finished without an error and removed at least one object;
-// because that removal is all the command does. When the snapshot was in
+// under LocalOnly it does nothing. markerErr is the result of
-// the local index, its removal there has already happened, so a failure
+// checkDestinationMarker; when it is set, nothing is deleted and it is
-// is only a warning telling the user to run `vaultik snapshot remove`
+// the failure. When the snapshot is only on the destination store, a
-// for the snapshot again once the destination store is reachable
+// failure is returned as an error, because that removal is all the
-// (`vaultik prune` never removes snapshot metadata that has its
+// command does. When the snapshot was in the local index, its removal
-// manifest).
+// there has already happened, so a failure is only a warning telling
+// the user to run `vaultik snapshot remove` for the snapshot again once
+// the destination store is reachable (`vaultik prune` never removes
+// snapshot metadata that has its manifest).
 func (v *Vaultik) removeSnapshotRemote(
-	snapshotID string, inLocalIndex bool, markerErr error,
+	shown, remoteKey string, inLocalIndex bool, markerErr error,
 	opts *RemoveOptions,
 ) (bool, error) {
 	if opts.LocalOnly {
@@ -1659,14 +1721,13 @@ func (v *Vaultik) removeSnapshotRemote(
 	}
 
 	log.Info("Removing snapshot metadata from remote storage",
-		"snapshot_id", snapshotID)
+		"snapshot_id", shown)
 
 	var deleted int
 
 	err := markerErr
 	if err == nil {
-		deleted, err = v.deleteRemoteSnapshotByKey(
+		deleted, err = v.deleteRemoteSnapshotByKey(remoteKey)
-			snapshot.RemoteSnapshotKey(snapshotID))
 	}
 
 	if err != nil && !inLocalIndex {
@@ -1678,14 +1739,14 @@ func (v *Vaultik) removeSnapshotRemote(
 		log.Warn("Could not remove snapshot metadata from remote storage; "+
 			"run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+
 			"again once the remote is reachable",
-			"snapshot_id", snapshotID, "error", err)
+			"snapshot_id", shown, "error", err)
 
 		// The UI writes to stdout, which under --json holds only the
 		// document; the log record above is the warning on stderr.
 		if v.UI != nil && !opts.JSON {
 			v.UI.Warningf("Could not remove snapshot metadata from remote: "+
 				"%v. Run '%s %s' again once the remote is reachable.",
-				err, snapshotRemoveCommandHint, snapshotID)
+				err, snapshotRemoveCommandHint, shown)
 		}
 
 		return false, nil
@@ -1780,6 +1841,23 @@ func (v *Vaultik) localSnapshotIDs() ([]string, error) {
 	return ids, nil
 }
 
+// localSnapshotIDForRemoteKey returns the human snapshot ID of the local
+// index row whose RemoteSnapshotKey is key, or "" when no row has it.
+func (v *Vaultik) localSnapshotIDForRemoteKey(key string) (string, error) {
+	ids, err := v.localSnapshotIDs()
+	if err != nil {
+		return "", fmt.Errorf("listing local snapshots: %w", err)
+	}
+
+	for _, id := range ids {
+		if snapshot.RemoteSnapshotKey(id) == key {
+			return id, nil
+		}
+	}
+
+	return "", nil
+}
+
 // listAllRemoteSnapshotKeys collects the hashed remote keys
 // (subdirectories under metadata/) currently present in the
 // destination store. Returns (nil, err) when the store cannot be