Name the S3 keys when storage_url moves to s3:// first (closes #428)

On a config that loads, `config set storage_url s3://...` is refused
until `s3.access_key_id` and `s3.secret_access_key` are set. The refusal
now says to set those two keys first, then `storage_url`.

Model: opus-5-5

README.md

@@ -448,8 +448,6 @@ 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/config_test.go

@@ -268,7 +268,7 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
 		{configWithoutRecipients, "snapshots.home.paths", "/srv",
 			"into []string"},
 		{configWithoutRecipients, "chunk_size", "fast",
-			`chunk_size: "fast" is not a size`},
+			"invalid size format"},
 		// Values of the right type that fail validation.
 		{configWithoutRecipients, "compression_level", "99",
 			"compression_level must be between 1 and 19"},

internal/cli/snapshot.go

@@ -288,10 +288,6 @@ 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/config/config.go

@@ -295,8 +295,7 @@ func Load(path string) (*Config, error) {
 }
 
 // Parse reads YAML config data as Load does, without validating it. It
-// refuses a key that no setting has and a size setting whose value is not a
+// refuses a key that no setting has, applies default values for optional
-// size, naming the setting. It applies default values for optional
 // fields, fills in ${...} references using smartconfig, and expands ~ in
 // paths.
 func Parse(data []byte) (*Config, error) {
@@ -321,9 +320,8 @@ func Parse(data []byte) (*Config, error) {
 		return nil, fmt.Errorf("failed to load config file: %w", err)
 	}
 
-	// A ${...} reference can bring in keys that are not in the file, and
+	// A ${...} reference can bring in keys that are not in the file.
-	// is the value of a size setting only once it is filled in.
+	err = checkFilledKeys(sc.Data())
-	err = checkFilledConfig(sc.Data())
 	if err != nil {
 		return nil, err
 	}

internal/config/keys.go

@@ -129,11 +129,10 @@ func checkKeys(node *yaml.Node, typ reflect.Type, path string,
 	return nil
 }
 
-// checkFilledConfig checks data, the config after smartconfig has filled in
+// checkFilledKeys is checkKeys for data, the config after smartconfig has
-// its ${...} references: its keys with checkKeys, then its size settings
+// filled in its ${...} references. A key that a reference brings in is not
-// with checkSizes. A key that a reference brings in is not in the file, and
+// in the file, and the copy that Encode makes has no line numbers.
-// the copy that Encode makes has no line numbers.
+func checkFilledKeys(data map[string]any) error {
-func checkFilledConfig(data map[string]any) error {
 	var filled yaml.Node
 
 	err := filled.Encode(data)
@@ -141,13 +140,8 @@ func checkFilledConfig(data map[string]any) error {
 		return fmt.Errorf("failed to marshal config data: %w", err)
 	}
 
-	err = checkKeys(&filled, reflect.TypeFor[Config](), "",
+	return checkKeys(&filled, reflect.TypeFor[Config](), "",
 		map[*yaml.Node]bool{}, map[nodeAndType]bool{})
-	if err != nil {
-		return err
-	}
-
-	return checkSizes(&filled, nil)
 }
 
 // checkMappingKeys is checkKeys for a map.

internal/config/size.go

@@ -4,20 +4,13 @@ import (
 	"errors"
 	"fmt"
 	"math"
-	"reflect"
-	"slices"
-	"strconv"
-	"strings"
 
 	"github.com/dustin/go-humanize"
-	"gopkg.in/yaml.v3"
 )
 
 var (
 	errSizeType     = errors.New("size must be a number or string")
 	errSizeTooLarge = errors.New("size exceeds maximum supported value")
-	errNotASize     = errors.New(
-		"is not a size; use a number of bytes or a unit such as 10MB")
 )
 
 // Size represents a byte size that can be specified in configuration files.
@@ -39,15 +32,6 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
 		return nil
 	}
 
-	// A number too large for an int64 decodes as a float64. Its text can be
-	// 1e+20, which is not a size string.
-	var floatVal float64
-
-	err = unmarshal(&floatVal)
-	if err == nil && floatVal > math.MaxInt64 {
-		return errSizeTooLarge
-	}
-
 	// Try to unmarshal as string
 	var strVal string
 
@@ -56,66 +40,17 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
 		return errSizeType
 	}
 
-	size, err := ParseSize(strVal)
+	// Parse the string using go-humanize
+	bytes, err := humanize.ParseBytes(strVal)
 	if err != nil {
-		return err
+		return fmt.Errorf("invalid size format: %w", err)
 	}
 
-	*s = size
+	if bytes > math.MaxInt64 {
-
+		return fmt.Errorf("%w: %s", errSizeTooLarge, strVal)
-	return nil
-}
-
-// checkSizes returns an error naming the first size setting in node whose
-// value is not a size. node is the config, or the part of it at the dotted
-// keys, after smartconfig has filled in its ${...} references.
-func checkSizes(node *yaml.Node, keys []string) error {
-	typ, err := SettingType(keys)
-	if err != nil {
-		// Unknown keys are refused before this check, so keys is in one of
-		// two places. The env section holds no settings. A value of the
-		// wrong shape, such as a map given for hostname or inside exclude,
-		// is reported later by the YAML decoder.
-		return nil //nolint:nilerr // not an error here
 	}
 
-	if typ == reflect.TypeFor[Size]() {
+	*s = Size(bytes)
-		var size Size
-
-		err = node.Decode(&size)
-		if err != nil {
-			return fmt.Errorf("%s: %w", strings.Join(keys, "."), err)
-		}
-
-		return nil
-	}
-
-	switch node.Kind {
-	case yaml.DocumentNode:
-		for _, child := range node.Content {
-			err = checkSizes(child, keys)
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.MappingNode:
-		for i := 0; i+1 < len(node.Content); i += 2 {
-			err = checkSizes(node.Content[i+1],
-				slices.Concat(keys, []string{node.Content[i].Value}))
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.SequenceNode:
-		for i, child := range node.Content {
-			err = checkSizes(child, slices.Concat(keys, []string{strconv.Itoa(i)}))
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.ScalarNode, yaml.AliasNode:
-		// A scalar holds no settings, and the filled config has no aliases.
-	}
 
 	return nil
 }
@@ -135,17 +70,16 @@ func (s Size) String() string {
 	return humanize.Bytes(uint64(s))
 }
 
-// ParseSize parses a size string, such as "10MB", into a Size value. An
+// ParseSize parses a size string into a Size value
-// error quotes s.
 func ParseSize(s string) (Size, error) {
-	bytes, err := humanize.ParseBigBytes(s)
+	bytes, err := humanize.ParseBytes(s)
 	if err != nil {
-		return 0, fmt.Errorf("%q %w", s, errNotASize)
+		return 0, fmt.Errorf("invalid size format: %w", err)
 	}
 
-	if !bytes.IsInt64() {
+	if bytes > math.MaxInt64 {
 		return 0, fmt.Errorf("%w: %s", errSizeTooLarge, s)
 	}
 
-	return Size(bytes.Int64()), nil
+	return Size(bytes), nil
 }

internal/config/size_test.go

@@ -1,120 +0,0 @@
-package config //nolint:testpackage // checks errNotASize and errSizeTooLarge
-
-import (
-	"errors"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-)
-
-// TestParseRefusesValueThatIsNotASize checks that a size setting whose
-// value is not a size fails the parse with an error naming the setting and
-// quoting the value, also when a ${...} reference fills the value in.
-func TestParseRefusesValueThatIsNotASize(t *testing.T) {
-	t.Parallel()
-
-	sizePath := filepath.Join(t.TempDir(), "size")
-
-	err := os.WriteFile(sizePath, []byte("lots"), 0o600)
-	if err != nil {
-		t.Fatalf("write size: %v", err)
-	}
-
-	tests := []struct {
-		name string
-		yaml string
-		want string
-	}{
-		{"chunk_size", keysTestConfig + "chunk_size: banana\n",
-			`chunk_size: "banana" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"blob_size_limit", keysTestConfig + "blob_size_limit: 10XB\n",
-			`blob_size_limit: "10XB" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"s3.part_size", strings.Replace(keysTestConfig, "  region:",
-			"  part_size: big\n  region:", 1),
-			`s3.part_size: "big" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"from a reference",
-			keysTestConfig + "chunk_size: ${FILE:" + sizePath + "}\n",
-			`chunk_size: "lots" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := Parse([]byte(tt.yaml))
-			if !errors.Is(err, errNotASize) {
-				t.Fatalf("Parse() error = %v, want errNotASize", err)
-			}
-
-			if err.Error() != tt.want {
-				t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
-			}
-		})
-	}
-}
-
-// TestParseRefusesSizeTooLarge checks that a size setting whose value is
-// too large for a size fails the parse with an error naming the setting.
-// YAML reads the second value as a float.
-func TestParseRefusesSizeTooLarge(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		value string
-		want  string
-	}{
-		{"100EB", "chunk_size: size exceeds maximum supported value: 100EB"},
-		{"99999999999999999999",
-			"chunk_size: size exceeds maximum supported value"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.value, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := Parse([]byte(keysTestConfig + "chunk_size: " + tt.value + "\n"))
-			if !errors.Is(err, errSizeTooLarge) {
-				t.Fatalf("Parse() error = %v, want errSizeTooLarge", err)
-			}
-
-			if err.Error() != tt.want {
-				t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
-			}
-		})
-	}
-}
-
-// TestParseAcceptsSizes checks that size settings given as a number of
-// bytes, as a unit, and through a ${...} reference still parse.
-func TestParseAcceptsSizes(t *testing.T) {
-	t.Parallel()
-
-	sizePath := filepath.Join(t.TempDir(), "size")
-
-	err := os.WriteFile(sizePath, []byte("20MB"), 0o600)
-	if err != nil {
-		t.Fatalf("write size: %v", err)
-	}
-
-	data := strings.Replace(keysTestConfig, "  region:",
-		"  part_size: 6000000\n  region:", 1) +
-		"chunk_size: ${FILE:" + sizePath + "}\n" +
-		"blob_size_limit: 1GB\n"
-
-	cfg, err := Parse([]byte(data))
-	if err != nil {
-		t.Fatalf("Parse() unexpected error: %v", err)
-	}
-
-	if cfg.ChunkSize != 20_000_000 || cfg.BlobSizeLimit != 1_000_000_000 ||
-		cfg.S3.PartSize != 6_000_000 {
-		t.Errorf("chunk_size, blob_size_limit, s3.part_size = %d, %d, %d, "+
-			"want 20000000, 1000000000, 6000000",
-			cfg.ChunkSize, cfg.BlobSizeLimit, cfg.S3.PartSize)
-	}
-}

internal/vaultik/remove_snapshot_test.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"database/sql"
-	"encoding/json"
 	"io"
 	"strings"
 	"sync"
@@ -473,171 +472,6 @@ 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
@@ -672,38 +506,6 @@ 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,14 +1424,11 @@ type RemoveOptions struct {
 	LocalOnly bool // Skip remote cleanup; only touch the local index
 }
 
-// RemoveResult contains the result of a snapshot removal. For a single
+// RemoveResult contains the result of a snapshot removal
-// 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"`
@@ -1461,35 +1458,25 @@ 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, resolvedID, remoteKey, err := v.findSnapshotToRemove(
+	inLocalIndex, err := v.checkSnapshotExists(snapshotID, opts.LocalOnly)
-		snapshotID, opts.LocalOnly)
 	if err != nil {
 		return result, err
 	}
 
-	result.SnapshotID = resolvedID
-	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 opts.DryRun {
-		return v.removeSnapshotDryRun(result, shown, opts)
+		return v.removeSnapshotDryRun(result, snapshotID, opts)
 	}
 
-	if !v.confirmRemoveSnapshot(shown, opts) {
+	if !v.confirmRemoveSnapshot(snapshotID, opts) {
 		return result, ErrNotConfirmed
 	}
 
@@ -1502,16 +1489,16 @@ func (v *Vaultik) RemoveSnapshot(
 	}
 
 	if inLocalIndex {
-		log.Info("Removing snapshot from local database", "snapshot_id", resolvedID)
+		log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
 
-		err = v.deleteSnapshotFromLocalDB(resolvedID)
+		err = v.deleteSnapshotFromLocalDB(snapshotID)
 		if err != nil {
 			return result, fmt.Errorf("removing from local database: %w", err)
 		}
 	}
 
 	result.RemoteRemoved, err = v.removeSnapshotRemote(
-		shown, remoteKey, inLocalIndex, markerErr, opts)
+		snapshotID, inLocalIndex, markerErr, opts)
 	if err != nil {
 		return result, err
 	}
@@ -1530,7 +1517,7 @@ func (v *Vaultik) RemoveSnapshot(
 
 	if inLocalIndex {
 		v.UI.Infof("Removed snapshot '%s' from local database.",
-			v.UI.Snapshot(resolvedID))
+			v.UI.Snapshot(snapshotID))
 	}
 
 	if result.RemoteRemoved {
@@ -1543,52 +1530,6 @@ 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
@@ -1698,22 +1639,19 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
 }
 
 // removeSnapshotRemote strips the snapshot's metadata from the
-// destination store. remoteKey is the resolved metadata directory name
+// destination store. It returns true only when the delete finished
-// to delete, as returned by findSnapshotToRemove. shown names the
+// without an error and removed at least one object; under LocalOnly it
-// snapshot in the log records and the warning: its ID, or its full
+// does nothing. markerErr is the result of checkDestinationMarker; when
-// remote key when the ID is not known. It returns true only when the
+// it is set, nothing is deleted and it is the failure. When the snapshot
-// delete finished without an error and removed at least one object;
+// is only on the destination store, a failure is returned as an error,
-// under LocalOnly it does nothing. markerErr is the result of
+// because that removal is all the command does. When the snapshot was in
-// checkDestinationMarker; when it is set, nothing is deleted and it is
+// the local index, its removal there has already happened, so a failure
-// the failure. When the snapshot is only on the destination store, a
+// is only a warning telling the user to run `vaultik snapshot remove`
-// failure is returned as an error, because that removal is all the
+// for the snapshot again once the destination store is reachable
-// command does. When the snapshot was in the local index, its removal
+// (`vaultik prune` never removes snapshot metadata that has its
-// there has already happened, so a failure is only a warning telling
+// manifest).
-// 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(
-	shown, remoteKey string, inLocalIndex bool, markerErr error,
+	snapshotID string, inLocalIndex bool, markerErr error,
 	opts *RemoveOptions,
 ) (bool, error) {
 	if opts.LocalOnly {
@@ -1721,13 +1659,14 @@ func (v *Vaultik) removeSnapshotRemote(
 	}
 
 	log.Info("Removing snapshot metadata from remote storage",
-		"snapshot_id", shown)
+		"snapshot_id", snapshotID)
 
 	var deleted int
 
 	err := markerErr
 	if err == nil {
-		deleted, err = v.deleteRemoteSnapshotByKey(remoteKey)
+		deleted, err = v.deleteRemoteSnapshotByKey(
+			snapshot.RemoteSnapshotKey(snapshotID))
 	}
 
 	if err != nil && !inLocalIndex {
@@ -1739,14 +1678,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", shown, "error", err)
+			"snapshot_id", snapshotID, "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, shown)
+				err, snapshotRemoveCommandHint, snapshotID)
 		}
 
 		return false, nil
@@ -1841,23 +1780,6 @@ 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