Stop snapshot purge when a snapshot cannot leave the local index (closes #337)

snapshot purge, and the purge step of snapshot create --prune, logged a
failed local index delete, then deleted that snapshot's metadata from
the destination store anyway and exited 0, leaving a local record whose
metadata was gone. It now stops at that snapshot with an error, before
touching its metadata, so the command exits non-zero; snapshots already
purged stay purged. This matches how the same loop already handles a
failed metadata delete.

Model: opus-5-5

README.md

@@ -339,7 +339,9 @@ local index alone, and still exits zero.
 
 **`snapshot purge`**: Remove old snapshots based on criteria. Retention is
 per-snapshot-name (`--keep-latest` keeps the latest of each name, not the
-latest globally).
+latest globally). If a snapshot cannot be removed from the local index, the
+command stops there, leaves that snapshot's metadata on the destination store
+and exits non-zero; `snapshot create --prune` fails the same way.
 * `--keep-latest`: Keep only the most recent snapshot of each name
 * `--older-than <duration>`: Remove snapshots older than duration (e.g. `30d`,
   `4w`, `6mo`, `1y`; `m` is minutes, `mo` is months)
@@ -373,9 +375,7 @@ on the destination in one go, use `vaultik remote nuke --force`.
   store — its remote key (or an unambiguous leading part of it) as shown
   by `snapshot list`. See
   [restoring on another machine](#restoring-on-another-machine).
-* Optional path arguments to restore specific files/directories (default: all).
-  A path that matches nothing in the snapshot is named in a warning, and the
-  command exits non-zero after restoring the paths that match.
+* Optional path arguments to restore specific files/directories (default: all)
 * Preserves file permissions, timestamps, ownership (ownership requires root),
   symlinks, and empty directories
 * `--verify`: After restoring, verify every restored file's chunk hashes

internal/cli/entry_snapshot_purge_test.go

@@ -0,0 +1,90 @@
+package cli //nolint:testpackage // shares runEntry and the hermetic config helpers
+
+import (
+	"context"
+	"database/sql"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/snapshot"
+	"sneak.berlin/go/vaultik/internal/types"
+)
+
+// The `snapshot purge` command word and flags used below.
+const (
+	cmdPurge       = "purge"
+	flagKeepLatest = "--keep-latest"
+	flagForce      = "--force"
+)
+
+// TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails makes
+// removing a snapshot from the local index fail and checks that
+// `snapshot purge` exits non-zero and leaves that snapshot's metadata on
+// the destination store. It used to log the error, delete the metadata
+// anyway and exit 0.
+//
+//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
+func TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails(t *testing.T) {
+	ctx := context.Background()
+	configPath := writeHermeticPruneConfig(t, false)
+
+	// writeHermeticPruneConfig puts the index and the store directory
+	// beside the config file.
+	dir := filepath.Dir(configPath)
+
+	// Two snapshots of the same name: --keep-latest purges the older.
+	olderStart := time.Date(2026, 5, 1, 9, 0, 0, 0, time.UTC)
+	newerStart := olderStart.Add(time.Hour)
+	olderID := "test-host_test_" + olderStart.Format(time.RFC3339)
+	newerID := "test-host_test_" + newerStart.Format(time.RFC3339)
+
+	db, err := database.New(ctx, filepath.Join(dir, "index.sqlite"))
+	require.NoError(t, err)
+
+	repos := database.NewRepositories(db)
+
+	manifestPaths := make(map[string]string)
+
+	for id, startedAt := range map[string]time.Time{
+		olderID: olderStart,
+		newerID: newerStart,
+	} {
+		completedAt := startedAt.Add(time.Minute)
+		snap := &database.Snapshot{
+			ID:             types.SnapshotID(id),
+			Hostname:       "test-host",
+			VaultikVersion: "test",
+			StartedAt:      startedAt,
+			CompletedAt:    &completedAt,
+		}
+
+		err = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
+			return repos.Snapshots.Create(ctx, tx, snap)
+		})
+		require.NoError(t, err)
+
+		manifestPaths[id] = filepath.Join(dir, "store", "metadata",
+			snapshot.RemoteSnapshotKey(id), "manifest.json.zst")
+		require.NoError(t, os.MkdirAll(filepath.Dir(manifestPaths[id]), 0o750))
+		require.NoError(t, os.WriteFile(manifestPaths[id], []byte("stub"), 0o600))
+	}
+
+	_, err = db.Conn().ExecContext(ctx, `
+		CREATE TRIGGER refuse_snapshot_delete BEFORE DELETE ON snapshots
+		BEGIN SELECT RAISE(ABORT, 'simulated index error'); END`)
+	require.NoError(t, err)
+	require.NoError(t, db.Close())
+
+	code, _, stderr := runEntry(t,
+		flagConfig, configPath, cmdSnapshot, cmdPurge, flagKeepLatest, flagForce)
+
+	assert.Equal(t, 1, code)
+	assert.Contains(t, stderr, "simulated index error")
+	assert.FileExists(t, manifestPaths[olderID],
+		"a snapshot still in the local index keeps its metadata")
+}

internal/cli/snapshot_restore.go

@@ -30,8 +30,6 @@ This command will restore files from the specified snapshot to the
 target directory.
 If no paths are specified, all files are restored.
 If paths are specified, only matching files/directories are restored.
-A path that matches nothing in the snapshot is named in a warning, and
-the command exits non-zero after restoring the paths that match.
 
 The snapshot may be named by its ID or, when restoring on a host with no
 local index, by the remote key that 'snapshot list' prints for a

internal/vaultik/restore.go

@@ -27,7 +27,6 @@ import (
 // Sentinel errors for restore failures.
 var (
 	errFilesFailedRestore    = errors.New("file(s) failed to restore")
-	errPathsNotInSnapshot    = errors.New("requested path(s) not in the snapshot")
 	errFilesFailedVerify     = errors.New("files failed verification")
 	errDecryptionKeyRequired = errors.New(
 		"decryption key required for restore\n\n" +
@@ -151,20 +150,16 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 	repos := database.NewRepositories(tempDB)
 
 	// Step 2: Get list of files to restore
-	files, unmatched, err := v.getFilesToRestore(v.ctx, repos, opts.Paths)
+	files, err := v.getFilesToRestore(v.ctx, repos, opts.Paths)
 	if err != nil {
 		return fmt.Errorf("getting files to restore: %w", err)
 	}
 
-	// Named before anything is restored, so the names are shown even
-	// when the restore stops on an error.
-	unmatchedErr := v.reportUnmatchedPaths(unmatched)
-
 	if len(files) == 0 {
 		log.Warn("No files found to restore")
 		v.UI.Warningf("No files found to restore.")
 
-		return unmatchedErr
+		return nil
 	}
 
 	log.Info("Found files to restore", "count", len(files))
@@ -190,20 +185,6 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 
 	result.Duration = time.Since(startTime)
 
-	return v.finishRestore(repos, files, unmatchedErr, opts, result)
-}
-
-// finishRestore prints the restore summary and the post-restore
-// warnings, runs optional verification, and converts any failed-file
-// count into an error. Otherwise it returns unmatchedErr, the error for
-// the requested paths that matched nothing (nil when there are none).
-func (v *Vaultik) finishRestore(
-	repos *database.Repositories,
-	files []*database.File,
-	unmatchedErr error,
-	opts *RestoreOptions,
-	result *RestoreResult,
-) error {
 	log.Info("Restore complete",
 		"files_restored", result.FilesRestored,
 		"bytes_restored", ubytes(result.BytesRestored),
@@ -218,6 +199,17 @@ func (v *Vaultik) finishRestore(
 		v.UI.Duration(result.Duration),
 	)
 
+	return v.finishRestore(repos, files, opts, result)
+}
+
+// finishRestore emits the post-restore warnings, runs optional
+// verification, and converts any failed-file count into an error.
+func (v *Vaultik) finishRestore(
+	repos *database.Repositories,
+	files []*database.File,
+	opts *RestoreOptions,
+	result *RestoreResult,
+) error {
 	if os.Geteuid() != 0 {
 		v.UI.Warningf("Restore did not preserve file ownership: chown(2) " +
 			"requires root. Re-run as root (e.g. with sudo) if you need " +
@@ -244,25 +236,9 @@ func (v *Vaultik) finishRestore(
 		return fmt.Errorf("%d %w", result.FilesFailed, errFilesFailedRestore)
 	}
 
-	return unmatchedErr
-}
-
-// reportUnmatchedPaths warns, by name, about each requested path that
-// matched nothing in the snapshot, and returns an error counting them,
-// or nil when there are none.
-func (v *Vaultik) reportUnmatchedPaths(unmatched []string) error {
-	if len(unmatched) == 0 {
 	return nil
 }
 
-	for _, path := range unmatched {
-		log.Warn("Requested path matches nothing in the snapshot", "path", path)
-		v.UI.Warningf("%s matches nothing in the snapshot.", v.UI.Path(path))
-	}
-
-	return fmt.Errorf("%d %w", len(unmatched), errPathsNotInSnapshot)
-}
-
 // restoreIdentities parses the configured age secret key once into every
 // identity it contains. The value may be a single key line or a whole
 // age-keygen file with several identities; all of them are returned so
@@ -864,34 +840,27 @@ func (v *Vaultik) materializeSnapshotDB(
 	return db, tempDir, nil
 }
 
-// getFilesToRestore returns the list of files to restore based on path
-// filters, and the filters, as given, that matched no file.
+// getFilesToRestore returns the list of files to restore based on path filters
 func (v *Vaultik) getFilesToRestore(
 	ctx context.Context, repos *database.Repositories, pathFilters []string,
-) ([]*database.File, []string, error) {
+) ([]*database.File, error) {
 	// If no filters, get all files
 	if len(pathFilters) == 0 {
-		files, err := repos.Files.ListAll(ctx)
-
-		return files, nil, err
+		return repos.Files.ListAll(ctx)
 	}
 
 	// Get files matching the path filters
-	var (
-		result    []*database.File
-		unmatched []string
-	)
+	var result []*database.File
 
 	seen := make(map[string]bool)
 
 	for _, filter := range pathFilters {
-		files, err := repos.Files.ListUnderPath(ctx, filepath.Clean(filter))
-		if err != nil {
-			return nil, nil, fmt.Errorf("listing files under %s: %w", filter, err)
-		}
+		// Normalize the filter path
+		filter = filepath.Clean(filter)
 
-		if len(files) == 0 {
-			unmatched = append(unmatched, filter)
+		files, err := repos.Files.ListUnderPath(ctx, filter)
+		if err != nil {
+			return nil, fmt.Errorf("listing files under %s: %w", filter, err)
 		}
 
 		for _, file := range files {
@@ -902,7 +871,7 @@ func (v *Vaultik) getFilesToRestore(
 		}
 	}
 
-	return result, unmatched, nil
+	return result, nil
 }
 
 // buildChunkToBlobMap creates a mapping from chunk hash to blob information

internal/vaultik/restore_unmatched_path_test.go

@@ -1,202 +0,0 @@
-package vaultik_test
-
-import (
-	"bytes"
-	"context"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/database"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/storage"
-	"sneak.berlin/go/vaultik/internal/storage/faultstore"
-	"sneak.berlin/go/vaultik/internal/ui"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// These tests cover https://git.eeqj.de/sneak/vaultik/issues/309: a path
-// given to restore that matches nothing in the snapshot is named in a
-// warning and makes the restore fail, after the paths that do match are
-// restored.
-
-func TestRestoreNamesUnmatchedPathAndRestoresTheMatchingOne(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	restoreDir := filepath.Join(tempDir, "restored")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-	files := writeFaultSourceTree(t, fs, dataDir)
-
-	store, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	id := fullFaultBackup(ctx, t, fs, store, cfg,
-		database.NewRepositories(db), dataDir, dbPath, "unmatched")
-	require.NoError(t, db.Close())
-
-	matched := filepath.Join(dataDir, "small.txt")
-	unmatched := filepath.Join(dataDir, "typo.txt")
-
-	var out bytes.Buffer
-
-	v := newReaderVaultik(ctx, cfg, store, nil, fs)
-	v.UI = ui.NewWithColor(&out, false)
-
-	err = v.Restore(&vaultik.RestoreOptions{
-		SnapshotID: id,
-		TargetDir:  restoreDir,
-		Paths:      []string{matched, unmatched},
-	})
-
-	require.Error(t, err, "a path that matches nothing must fail the restore")
-	assert.Contains(t, out.String(), unmatched,
-		"the path that matches nothing must be named")
-	assert.NotContains(t, out.String(), matched,
-		"the path that matches must not be reported")
-
-	got, err := afero.ReadFile(fs, filepath.Join(restoreDir, matched))
-	require.NoError(t, err, "the path that matches must still be restored")
-	assert.Equal(t, files[matched], got)
-}
-
-// When no requested path matches, nothing is restored and each path is
-// named. One path is relative. The other goes through the symlink that
-// is the snapshot's configured path, while the entries are stored under
-// the directory the symlink points to.
-func TestRestoreNamesEveryPathWhenNothingMatches(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	const snapshotName = "linked"
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	linkDir := filepath.Join(tempDir, "srclink")
-	storeDir := filepath.Join(tempDir, "remote")
-	restoreDir := filepath.Join(tempDir, "restored")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-
-	writeFaultSourceTree(t, fs, dataDir)
-	require.NoError(t, os.Symlink(dataDir, linkDir))
-
-	cfg := faultTestConfig()
-	cfg.IndexPath = dbPath
-	cfg.ChunkSize = config.Size(faultChunkSize)
-	cfg.Snapshots = map[string]config.SnapshotConfig{
-		snapshotName: {Paths: []string{linkDir}},
-	}
-
-	store, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-	v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
-
-	require.NoError(t, backUp(v, snapshotName))
-
-	id := localSnapshotID(ctx, t, repos, snapshotName)
-	require.NoError(t, db.Close())
-
-	throughLink := filepath.Join(linkDir, "sub")
-	relative := strings.TrimPrefix(filepath.Join(dataDir, "small.txt"), "/")
-
-	var out bytes.Buffer
-
-	reader := newReaderVaultik(ctx, cfg, store, nil, fs)
-	reader.UI = ui.NewWithColor(&out, false)
-
-	err = reader.Restore(&vaultik.RestoreOptions{
-		SnapshotID: id,
-		TargetDir:  restoreDir,
-		Paths:      []string{throughLink, relative},
-	})
-
-	require.Error(t, err, "a restore that matches nothing must fail")
-	assert.Contains(t, out.String(), throughLink,
-		"the path through the symlink must be named")
-	assert.Contains(t, out.String(), relative,
-		"the relative path must be named")
-
-	_, err = fs.Stat(restoreDir)
-	assert.True(t, os.IsNotExist(err), "nothing must be restored")
-}
-
-// Without --skip-errors a damaged blob stops the restore part way. The
-// path that matches nothing must still be named.
-func TestRestoreNamesUnmatchedPathWhenADamagedBlobStopsIt(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	restoreDir := filepath.Join(tempDir, "restored")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-
-	writeFaultSourceTree(t, fs, dataDir)
-
-	inner, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	id := fullFaultBackup(ctx, t, fs, inner, cfg,
-		database.NewRepositories(db), dataDir, dbPath, "damaged")
-	require.NoError(t, db.Close())
-
-	damaged := faultstore.New(inner)
-	damaged.OnGet = func(key string) faultstore.GetFault {
-		if strings.HasPrefix(key, "blobs/") {
-			return faultstore.GetCorrupt
-		}
-
-		return faultstore.GetNormal
-	}
-
-	matched := filepath.Join(dataDir, "small.txt")
-	unmatched := filepath.Join(dataDir, "typo.txt")
-
-	var out bytes.Buffer
-
-	v := newReaderVaultik(ctx, cfg, damaged, nil, fs)
-	v.UI = ui.NewWithColor(&out, false)
-
-	err = v.Restore(&vaultik.RestoreOptions{
-		SnapshotID: id,
-		TargetDir:  restoreDir,
-		Paths:      []string{matched, unmatched},
-	})
-
-	require.Error(t, err, "a damaged blob must stop the restore")
-	assert.Contains(t, err.Error(), "blob",
-		"the restore must stop on the damaged blob")
-	assert.Contains(t, out.String(), unmatched,
-		"the path that matches nothing must be named")
-}

internal/vaultik/snapshot.go

@@ -627,7 +627,8 @@ func selectSnapshotsToPurge(
 }
 
 // confirmAndExecutePurge shows deletion candidates, confirms with the
-// user, and deletes snapshots.
+// user, and deletes snapshots. It stops with an error at the first
+// snapshot it cannot delete.
 func (v *Vaultik) confirmAndExecutePurge(
 	toDelete []SnapshotInfo, force, quiet bool,
 ) error {
@@ -669,10 +670,11 @@ func (v *Vaultik) confirmAndExecutePurge(
 		snapshotID := snap.ID.String()
 		log.Info("Deleting snapshot", "id", snapshotID)
 
+		// Stop before the remote delete: a snapshot still in the local
+		// index must keep its metadata on the destination store.
 		err := v.deleteSnapshotFromLocalDB(snapshotID)
 		if err != nil {
-			log.Error("Failed to delete from local database",
-				"snapshot_id", snapshotID, "error", err)
+			return fmt.Errorf("deleting snapshot %s from local index: %w", snapshotID, err)
 		}
 
 		_, err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))