Make a leading **/ in an exclude pattern match at the top level (closes #303)

An unanchored exclude pattern such as **/node_modules or **/*.pyc
needed a / before the name, so it excluded sub/node_modules and
sub/y.pyc but backed up node_modules/ and x.pyc directly under the
snapshot path. An unanchored pattern already matches at every depth,
so the leading **/ is now dropped when the pattern is compiled and,
as in gitignore, the pattern matches the top level too.

Judgement call: an anchored /**/x is left as it was; the issue covers
patterns that start with **/.

Model: opus-5-5

ARCHITECTURE.md

@@ -279,8 +279,8 @@ Key methods:
 - `Scan(ctx, path, snapshotID)` → Main scan operation
 
 Scan phases:
-1. **Phase 1**: Walk directory, identify files needing processing
+1. **Phase 0**: Detect deleted files from previous snapshots
-2. **Phase 1b**: Count as deleted the files that the previous snapshot of the same name lists and the walk did not find; an excluded file is not counted
+2. **Phase 1**: Walk directory, identify files needing processing
 3. **Phase 2**: Process files (chunk → pack → upload)
 
 #### SnapshotManager
@@ -320,13 +320,13 @@ CreateSnapshot(opts)
     │       │
     │       ├─► scanner.Scan(ctx, path, snapshotID)
     │       │       │
+    │       │       ├─► Phase 0: detectDeletedFiles()
+    │       │       │
     │       │       ├─► Phase 1: scanPhase()
     │       │       │       Walk directory
     │       │       │       Check file metadata changes
     │       │       │       Build list of files to process
     │       │       │
-    │       │       ├─► Phase 1b: detectDeletedFiles()
-    │       │       │
     │       │       └─► Phase 2: processPhase()
     │       │               For each file:
     │       │                   chunker.ChunkReaderStreaming()

README.md

@@ -349,11 +349,9 @@ the backup destination store. Blobs are NOT touched — deleting blobs
 requires reading every remaining remote manifest (the destination store
 may hold snapshots this host doesn't know about), which is what
 `vaultik prune` does. On success the command prints the exact `vaultik
-prune` invocation to run as a follow-up. An ID that names no snapshot in
+prune` invocation to run as a follow-up. Local row cleanup (files,
-the local index or on the destination store is an error; with
+chunks, blobs the snapshot was the last referrer for) runs
-`--local-only`, the snapshot must be in the local index. Local row
+automatically. If the destination store is unreachable, the local-DB
-cleanup (files, chunks, blobs the snapshot was the last referrer for)
-runs automatically. If the destination store is unreachable, the local-DB
 removal still completes and a warning is emitted; run `vaultik snapshot
 remove <snapshot-id>` again once the store is reachable to remove the
 snapshot's metadata from it (`vaultik prune` does not). To wipe everything

TODO.md

@@ -22,27 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to
 
 # Completed Steps
 
-- 2026-10-13: Counted a file as deleted only when the previous snapshot
+- 2026-10-13: Made an exclude pattern starting with `**/` also match
-  of the same name had it
+  directly under the snapshot path
-  ([issue #304](https://git.eeqj.de/sneak/vaultik/issues/304)). The
+  ([issue #303](https://git.eeqj.de/sneak/vaultik/issues/303)).
-  deleted figure of a backup counted every file in the local index under
+  `**/node_modules` and `**/*.pyc`, the patterns `config.example.yml`
-  the source path that the walk did not find. That took in files the
+  uses, excluded `sub/node_modules` and `sub/y.pyc` but backed up
-  snapshot excludes but another snapshot backs up, and files deleted
+  `node_modules/` and `x.pyc` at the top of the path, since the pattern
-  long ago, which every later backup, and the first backup of another
+  required a `/` before the name. As in gitignore, such a pattern now
-  name over the same path, reported as deleted again. A file is now
+  matches at every depth, the top included.
-  counted only when the previous completed snapshot of the same name
-  lists it and the walk did not find it, and an excluded file is never
-  counted.
-
-- 2026-10-13: Made `snapshot remove` fail for a snapshot that does not
-  exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)).
-  The command never checked the ID: it reported the snapshot removed
-  from the local database and from the destination store and exited 0,
-  under `--json`, `--dry-run` and `--local-only` too. An ID that names
-  no snapshot in the local index or on the destination store is now an
-  error; with `--local-only`, the snapshot must be in the local index.
-  The command now reports a removal from either side only when one
-  happened, and `remote_removed` is true only when metadata was deleted.
 
 - 2026-10-12: Bound the local index to the destination an `s3.*` config
   names ([issue #298](https://git.eeqj.de/sneak/vaultik/issues/298)).

internal/cli/snapshot.go

@@ -255,12 +255,8 @@ manifest (the destination store may hold snapshots this host doesn't
 know about), which is what 'vaultik prune' does. On success the command
 prints the exact 'vaultik prune' invocation to run as a follow-up.
 
-An ID that names no snapshot in the local index or on the destination
-store is an error.
-
 Use --local-only to skip the remote half (e.g. when you want to forget a
-snapshot locally without touching the destination store); the snapshot
+snapshot locally without touching the destination store).
-must then be in the local index.
 
 If the remote is unreachable, the local-database removal still completes
 and a warning is emitted; run 'vaultik snapshot remove <snapshot-id>' again

internal/database/files.go

@@ -277,50 +277,6 @@ func (r *FileRepository) ListUnderPath(
 	return files, rows.Err()
 }
 
-// ListUnderPathInSnapshot returns the files that ListUnderPath returns
-// for path and that the snapshot lists, in no particular order.
-func (r *FileRepository) ListUnderPathInSnapshot(
-	ctx context.Context, path, snapshotID string,
-) ([]*File, error) {
-	path = strings.TrimRight(path, "/")
-	dirPrefix := path + "/"
-
-	query := `
-		SELECT f.id, f.path, f.source_path, f.mtime, f.mtime_nsec, f.size,
-			f.mode, f.uid, f.gid, f.link_target
-		FROM files f
-		JOIN snapshot_files sf ON sf.file_id = f.id
-		WHERE sf.snapshot_id = ?
-		  AND (f.path = ? OR substr(f.path, 1, length(?)) = ?)
-	`
-
-	rows, err := r.db.conn.QueryContext(ctx, query,
-		snapshotID, path, dirPrefix, dirPrefix)
-	if err != nil {
-		return nil, fmt.Errorf("querying files: %w", err)
-	}
-
-	defer func() {
-		err := rows.Close()
-		if err != nil {
-			Fatalf("failed to close rows: %v", err)
-		}
-	}()
-
-	var files []*File
-
-	for rows.Next() {
-		file, err := r.scanFileRows(rows)
-		if err != nil {
-			return nil, fmt.Errorf("scanning file: %w", err)
-		}
-
-		files = append(files, file)
-	}
-
-	return files, rows.Err()
-}
-
 // ListIDsWithChunksNotInUploadedBlobs returns the IDs of the files whose
 // path starts with prefix and that list at least one chunk held by no
 // blob whose upload has completed (uploaded_ts set). A new snapshot

internal/database/files_test.go

@@ -212,65 +212,6 @@ func TestFileRepositoryListUnderPath(t *testing.T) {
 	}
 }
 
-// ListUnderPathInSnapshot leaves out files outside path and files the
-// snapshot does not list, though another snapshot lists them.
-func TestFileRepositoryListUnderPathInSnapshot(t *testing.T) {
-	t.Parallel()
-
-	db, cleanup := setupTestDB(t)
-	defer cleanup()
-
-	ctx := context.Background()
-	files := database.NewFileRepository(db)
-	snapshots := database.NewSnapshotRepository(db)
-
-	listed := map[string][]string{
-		"one": {"/src/a.txt", "/src2/c.txt"},
-		"two": {"/src/b.txt"},
-	}
-
-	for snapshotID, paths := range listed {
-		err := snapshots.Create(ctx, nil, &database.Snapshot{
-			ID:        types.SnapshotID(snapshotID),
-			Hostname:  "host",
-			StartedAt: time.Now(),
-		})
-		if err != nil {
-			t.Fatalf("failed to create snapshot %s: %v", snapshotID, err)
-		}
-
-		for _, path := range paths {
-			err := files.Create(ctx, nil, &database.File{
-				Path:  types.FilePath(path),
-				MTime: time.Now().Truncate(time.Second),
-				Mode:  0644,
-			})
-			if err != nil {
-				t.Fatalf("failed to create %s: %v", path, err)
-			}
-
-			err = snapshots.AddFile(ctx, nil, snapshotID, path)
-			if err != nil {
-				t.Fatalf("failed to add %s to %s: %v", path, snapshotID, err)
-			}
-		}
-	}
-
-	found, err := files.ListUnderPathInSnapshot(ctx, "/src", "one")
-	if err != nil {
-		t.Fatalf("failed to list files: %v", err)
-	}
-
-	got := make([]string, 0, len(found))
-	for _, f := range found {
-		got = append(got, f.Path.String())
-	}
-
-	if !slices.Equal(got, []string{"/src/a.txt"}) {
-		t.Errorf("got %q, want only /src/a.txt", got)
-	}
-}
-
 func TestFileRepositorySymlink(t *testing.T) {
 	t.Parallel()
 

internal/snapshot/exclude_test.go

@@ -531,3 +531,50 @@ func TestExcludePatterns_UnanchoredPatternFile(t *testing.T) {
 	require.Equal(t, 1, result.FilesScanned,
 		"Unanchored pattern for file should match anywhere")
 }
+
+func TestExcludePatterns_DoubleStarDirectoryAtEveryDepth(t *testing.T) {
+	t.Parallel()
+
+	// As in gitignore, a leading **/ also matches directly under the source dir
+	fs := setupAnchoredTestFS(t)
+
+	scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/projectname"})
+	defer cleanup()
+
+	ctx := context.Background()
+	createSnapshotRecord(ctx, t, repos, "test-snapshot")
+
+	result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
+	require.NoError(t, err)
+
+	// **/projectname should exclude BOTH:
+	// - /backup/projectname/file.txt
+	// - /backup/otherproject/projectname/file.txt
+	// Total files: 4, excluded: 2
+	require.Equal(t, 2, result.FilesScanned,
+		"**/ directory pattern should match at the top level and nested")
+}
+
+func TestExcludePatterns_DoubleStarFileAtEveryDepth(t *testing.T) {
+	t.Parallel()
+
+	// As in gitignore, a leading **/ also matches directly under the source dir
+	fs := setupAnchoredTestFS(t)
+
+	scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/*.txt"})
+	defer cleanup()
+
+	ctx := context.Background()
+	createSnapshotRecord(ctx, t, repos, "test-snapshot")
+
+	result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
+	require.NoError(t, err)
+
+	// **/*.txt should exclude ALL .txt files:
+	// - /backup/file.txt
+	// - /backup/projectname/file.txt
+	// - /backup/otherproject/projectname/file.txt
+	// Total files: 4, excluded: 3
+	require.Equal(t, 1, result.FilesScanned,
+		"**/ file pattern should match at the top level and nested")
+}

internal/snapshot/module.go

@@ -16,9 +16,6 @@ type ScannerParams struct {
 	Fs             afero.Fs
 	Exclude        []string // Exclude patterns (combined global + snapshot-specific)
 	SkipErrors     bool     // Skip file read errors (log loudly but continue)
-	// PreviousSnapshotID is the most recent completed snapshot of the
-	// same name, "" for none.
-	PreviousSnapshotID string
 }
 
 // Module exports backup functionality as an fx module.
@@ -47,18 +44,17 @@ func provideScannerFactory(
 		}
 
 		return NewScanner(ScannerConfig{
-			FS:                 params.Fs,
+			FS:               params.Fs,
-			ChunkSize:          cfg.ChunkSize.Int64(),
+			ChunkSize:        cfg.ChunkSize.Int64(),
-			Repositories:       repos,
+			Repositories:     repos,
-			Storage:            storer,
+			Storage:          storer,
-			MaxBlobSize:        cfg.BlobSizeLimit.Int64(),
+			MaxBlobSize:      cfg.BlobSizeLimit.Int64(),
-			CompressionLevel:   cfg.CompressionLevel,
+			CompressionLevel: cfg.CompressionLevel,
-			AgeRecipients:      cfg.AgeRecipients,
+			AgeRecipients:    cfg.AgeRecipients,
-			EnableProgress:     params.EnableProgress,
+			EnableProgress:   params.EnableProgress,
-			UI:                 params.UI,
+			UI:               params.UI,
-			Exclude:            excludes,
+			Exclude:          excludes,
-			SkipErrors:         params.SkipErrors,
+			SkipErrors:       params.SkipErrors,
-			PreviousSnapshotID: params.PreviousSnapshotID,
 		})
 	}
 }

internal/snapshot/scanner.go

@@ -58,10 +58,6 @@ type Scanner struct {
 	compressionLevel int
 	ageRecipient     string
 	snapshotID       string // Current snapshot being processed
-	// previousSnapshotID is the most recent completed snapshot of the
-	// same name, "" for none. Only a file it lists can be counted as
-	// deleted.
-	previousSnapshotID string
 	// currentSourcePath is the source directory being scanned, stored with
 	// each file record.
 	currentSourcePath string
@@ -133,9 +129,6 @@ type ScannerConfig struct {
 	// SkipErrors skips files that cannot be opened or read (log loudly but
 	// continue); packer, database, encryption, and upload errors still abort.
 	SkipErrors bool
-	// PreviousSnapshotID is the most recent completed snapshot of the
-	// same name, "" for none.
-	PreviousSnapshotID string
 }
 
 // ScanResult contains the results of a scan operation. Files and bytes
@@ -211,7 +204,6 @@ func NewScanner(cfg ScannerConfig) *Scanner {
 		compiledExclude:    compiledExclude,
 		progress:           progress,
 		skipErrors:         cfg.SkipErrors,
-		previousSnapshotID: cfg.PreviousSnapshotID,
 		ui:                 uiw,
 		pendingChunkHashes: make(map[string]struct{}),
 	}
@@ -263,9 +255,8 @@ func (s *Scanner) Scan(
 
 	filesToProcess := scanResult.FilesToProcess
 
-	// Phase 1b: Detect deleted files by comparing the previous snapshot
+	// Phase 1b: Detect deleted files by comparing DB against scanned files
-	// of this name against scanned files
+	err = s.detectDeletedFilesFromMap(ctx, knownFiles, existingFiles, result)
-	err = s.detectDeletedFiles(ctx, path, existingFiles, result)
 	if err != nil {
 		return nil, fmt.Errorf("detecting deleted files: %w", err)
 	}
@@ -445,8 +436,9 @@ func (s *Scanner) summarizeScanPhase(
 }
 
 // loadKnownFiles loads the known files at and beneath path from the
-// database into a map for fast lookup. This avoids per-file database
+// database into a map for fast lookup. Every loaded file the scan does
-// queries during the scan phase.
+// not find is counted as deleted. This avoids per-file database queries
+// during the scan phase.
 func (s *Scanner) loadKnownFiles(
 	ctx context.Context, path string,
 ) (map[string]*database.File, error) {
@@ -1941,28 +1933,21 @@ func (s *Scanner) queueFileForBatchInsert(
 	})
 }
 
-// detectDeletedFiles counts as deleted each file at or beneath path that
+// detectDeletedFilesFromMap finds files that existed in previous snapshots
-// the previous snapshot of this name lists and the walk did not find.
+// but no longer exist. Uses pre-loaded maps to avoid any filesystem or
-// An excluded file is not counted: the walk skips it whether or not it
+// database access.
-// is still on disk.
+func (s *Scanner) detectDeletedFilesFromMap(
-func (s *Scanner) detectDeletedFiles(
 	ctx context.Context,
-	path string,
+	knownFiles map[string]*database.File,
 	existingFiles map[string]struct{},
 	result *ScanResult,
 ) error {
-	if s.previousSnapshotID == "" {
+	if len(knownFiles) == 0 {
 		return nil
 	}
 
-	previousFiles, err := s.repos.Files.ListUnderPathInSnapshot(
+	// Check each known file against the enumerated set (no filesystem access needed)
-		ctx, path, s.previousSnapshotID)
+	for path, file := range knownFiles {
-	if err != nil {
-		return fmt.Errorf("listing files of snapshot %s: %w",
-			s.previousSnapshotID, err)
-	}
-
-	for _, file := range previousFiles {
 		// Check context cancellation periodically
 		select {
 		case <-ctx.Done():
@@ -1970,16 +1955,13 @@ func (s *Scanner) detectDeletedFiles(
 		default:
 		}
 
-		filePath := file.Path.String()
+		// Check if the file exists in our enumerated set
-
+		if _, exists := existingFiles[path]; !exists {
-		_, exists := existingFiles[filePath]
+			// File has been deleted
-		if exists || s.shouldExclude(filePath, path) {
+			result.FilesDeleted++
-			continue
+			result.BytesDeleted += file.Size
+			log.Debug("Detected deleted file", "path", path, "size", file.Size)
 		}
-
-		result.FilesDeleted++
-		result.BytesDeleted += file.Size
-		log.Debug("Detected deleted file", "path", filePath, "size", file.Size)
 	}
 
 	if result.FilesDeleted > 0 {
@@ -2031,6 +2013,14 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
 		// Remove trailing slash if present (directory indicator)
 		pattern = strings.TrimSuffix(pattern, "/")
 
+		// A leading "**/" demands a "/" before the name and so misses an
+		// entry directly under the source path. An unanchored pattern
+		// already matches at every depth, so drop the prefix: as in
+		// gitignore, "**/x" then also matches a top-level "x".
+		if !anchored {
+			pattern = strings.TrimPrefix(pattern, "**/")
+		}
+
 		// Compile the glob pattern
 		// For patterns without path separators, we need to match them as components
 		// e.g., ".git" should match ".git" anywhere in the path

internal/vaultik/fault_injection_test.go

@@ -661,18 +661,17 @@ func faultScannerFactory(
 ) snapshot.ScannerFactory {
 	return func(params snapshot.ScannerParams) *snapshot.Scanner {
 		return snapshot.NewScanner(snapshot.ScannerConfig{
-			FS:                 params.Fs,
+			FS:               params.Fs,
-			Storage:            storer,
+			Storage:          storer,
-			ChunkSize:          faultChunkSize,
+			ChunkSize:        faultChunkSize,
-			MaxBlobSize:        faultMaxBlobSize,
+			MaxBlobSize:      faultMaxBlobSize,
-			CompressionLevel:   cfg.CompressionLevel,
+			CompressionLevel: cfg.CompressionLevel,
-			AgeRecipients:      cfg.AgeRecipients,
+			AgeRecipients:    cfg.AgeRecipients,
-			Repositories:       repos,
+			Repositories:     repos,
-			EnableProgress:     params.EnableProgress,
+			EnableProgress:   params.EnableProgress,
-			UI:                 params.UI,
+			UI:               params.UI,
-			Exclude:            params.Exclude,
+			Exclude:          params.Exclude,
-			SkipErrors:         params.SkipErrors,
+			SkipErrors:       params.SkipErrors,
-			PreviousSnapshotID: params.PreviousSnapshotID,
 		})
 	}
 }

internal/vaultik/remove_snapshot_test.go

@@ -3,21 +3,17 @@ package vaultik_test
 import (
 	"bytes"
 	"context"
-	"database/sql"
 	"io"
 	"strings"
 	"sync"
 	"testing"
-	"time"
 
 	"github.com/klauspost/compress/zstd"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/snapshot"
 	"sneak.berlin/go/vaultik/internal/storage"
-	"sneak.berlin/go/vaultik/internal/types"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
 
@@ -218,37 +214,6 @@ func addBlob(t *testing.T, store *testStorer, hash string) {
 	require.NoError(t, err)
 }
 
-// newLocalIndex returns an in-memory local index holding a completed
-// snapshot row for each of ids.
-func newLocalIndex(t *testing.T, ids ...string) *database.Repositories {
-	t.Helper()
-
-	ctx := context.Background()
-
-	db, err := database.New(ctx, ":memory:")
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = db.Close() })
-
-	repos := database.NewRepositories(db)
-
-	for _, id := range ids {
-		completedAt := time.Now().UTC()
-		snap := &database.Snapshot{
-			ID:          types.SnapshotID(id),
-			Hostname:    snapHostname,
-			StartedAt:   completedAt,
-			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)
-	}
-
-	return repos
-}
-
 // ============================================================================
 // Unit Tests for RemoveSnapshot
 // ============================================================================
@@ -268,7 +233,6 @@ func TestRemoveSnapshot_LocalOnly_PreservesRemote(t *testing.T) {
 	addBlob(t, store, blobA)
 
 	tv := vaultik.NewForTesting(store)
-	tv.Repositories = newLocalIndex(t, "snapshot-001")
 
 	opts := &vaultik.RemoveOptions{Force: true, LocalOnly: true}
 	result, err := tv.RemoveSnapshot("snapshot-001", opts)
@@ -304,7 +268,6 @@ func TestRemoveSnapshot_DefaultRemovesMetadataNotBlobs(t *testing.T) {
 	addBlob(t, store, blobShared)
 
 	tv := vaultik.NewForTesting(store)
-	tv.Repositories = newLocalIndex(t, "snapshot-001", "snapshot-002")
 
 	opts := &vaultik.RemoveOptions{Force: true}
 	result, err := tv.RemoveSnapshot("snapshot-001", opts)
@@ -353,84 +316,6 @@ func TestRemoveSnapshot_DryRun(t *testing.T) {
 	assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]")
 }
 
-// TestRemoveSnapshot_UnknownIDIsError checks that an ID naming no
-// snapshot in the local index or on the destination store is an error
-// under every option, and that nothing is reported as removed.
-func TestRemoveSnapshot_UnknownIDIsError(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	for name, opts := range map[string]vaultik.RemoveOptions{
-		"force":      {Force: true},
-		"json":       {JSON: true},
-		"dry-run":    {Force: true, DryRun: true},
-		"local-only": {Force: true, LocalOnly: true},
-	} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			store := newTestStorer()
-			addManifest(t, store, "snapshot-001", nil)
-
-			tv := vaultik.NewForTesting(store)
-			tv.Repositories = newLocalIndex(t, "snapshot-001")
-
-			result, err := tv.RemoveSnapshot("snapshot-999", &opts)
-
-			require.ErrorContains(t, err, "snapshot not found")
-			assert.False(t, result.RemoteRemoved)
-			assert.Empty(t, tv.Stdout.String())
-			assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
-		})
-	}
-}
-
-// TestRemoveSnapshot_NoMetadataOnDestination checks that removing a
-// snapshot whose metadata is not on the destination store does not
-// report metadata removed from it.
-func TestRemoveSnapshot_NoMetadataOnDestination(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	tv := vaultik.NewForTesting(newTestStorer())
-	tv.Repositories = newLocalIndex(t, "snapshot-001")
-
-	result, err := tv.RemoveSnapshot("snapshot-001",
-		&vaultik.RemoveOptions{Force: true})
-
-	require.NoError(t, err)
-	assert.False(t, result.RemoteRemoved)
-
-	out := tv.Stdout.String()
-	assert.Contains(t, out, "Removed snapshot 'snapshot-001' from local database")
-	assert.NotContains(t, out, "Removed snapshot metadata from remote storage")
-}
-
-// TestRemoveSnapshot_NotInLocalIndex checks that removing a snapshot
-// only the destination store holds removes its metadata there without
-// reporting a removal from the local database.
-func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	store := newTestStorer()
-	addManifest(t, store, "snapshot-001", nil)
-
-	tv := vaultik.NewForTesting(store)
-	tv.Repositories = newLocalIndex(t)
-
-	result, err := tv.RemoveSnapshot("snapshot-001",
-		&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")
-}
-
 func TestRemoveAllSnapshots_RequiresForce(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()

internal/vaultik/snapshot.go

@@ -23,7 +23,6 @@ var (
 	errNoSnapshotsInConfig  = errors.New("no snapshots configured")
 	errSnapshotVerifyFailed = errors.New("verification failed")
 	errRemoveAllNeedsForce  = errors.New("--all requires --force")
-	errSnapshotNotFound     = errors.New("snapshot not found")
 	errInvalidTableName     = errors.New("invalid table name")
 	errNoAgeRecipients      = errors.New(
 		"creating a snapshot needs at least one public key in " +
@@ -214,18 +213,12 @@ func (v *Vaultik) createNamedSnapshot(
 		return err
 	}
 
-	previousSnapshotID, err := v.previousSnapshotID(snapName)
-	if err != nil {
-		return err
-	}
-
 	scanner := v.ScannerFactory(snapshot.ScannerParams{
-		EnableProgress:     !opts.Cron,
+		EnableProgress: !opts.Cron,
-		UI:                 v.UI,
+		UI:             v.UI,
-		Fs:                 v.Fs,
+		Fs:             v.Fs,
-		Exclude:            v.Config.GetExcludes(snapName),
+		Exclude:        v.Config.GetExcludes(snapName),
-		SkipErrors:         opts.SkipErrors,
+		SkipErrors:     opts.SkipErrors,
-		PreviousSnapshotID: previousSnapshotID,
 	})
 
 	snapshotID, err := v.SnapshotManager.CreateSnapshotWithName(
@@ -260,24 +253,6 @@ func (v *Vaultik) createNamedSnapshot(
 	return nil
 }
 
-// previousSnapshotID returns the ID of the most recent completed snapshot
-// named snapName in the local index, or "" when there is none.
-func (v *Vaultik) previousSnapshotID(snapName string) (string, error) {
-	snapshots, err := v.Repositories.Snapshots.ListRecent(v.ctx, listRecentLimit)
-	if err != nil {
-		return "", fmt.Errorf("listing snapshots: %w", err)
-	}
-
-	for _, s := range snapshots {
-		if s.CompletedAt != nil &&
-			parseSnapshotName(s.ID.String(), s.Hostname.String()) == snapName {
-			return s.ID.String(), nil
-		}
-	}
-
-	return "", nil
-}
-
 // resolveSnapshotPaths resolves source directories to absolute paths
 // with symlink resolution.
 func (v *Vaultik) resolveSnapshotPaths(snapName string) ([]string, error) {
@@ -668,7 +643,7 @@ func (v *Vaultik) confirmAndExecutePurge(
 				"snapshot_id", snapshotID, "error", err)
 		}
 
-		_, err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))
+		err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))
 		if err != nil {
 			return fmt.Errorf("deleting snapshot %s from remote: %w", snapshotID, err)
 		}
@@ -1178,11 +1153,6 @@ func (v *Vaultik) RemoveSnapshot(
 		return result, err
 	}
 
-	inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts)
-	if err != nil {
-		return result, err
-	}
-
 	if opts.DryRun {
 		return v.removeSnapshotDryRun(result, snapshotID, opts)
 	}
@@ -1191,13 +1161,11 @@ func (v *Vaultik) RemoveSnapshot(
 		return result, nil
 	}
 
-	if inLocalIndex {
+	log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
-		log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
 
-		err = v.deleteSnapshotFromLocalDB(snapshotID)
+	err = v.deleteSnapshotFromLocalDB(snapshotID)
-		if err != nil {
+	if err != nil {
-			return result, fmt.Errorf("removing from local database: %w", err)
+		return result, fmt.Errorf("removing from local database: %w", err)
-		}
 	}
 
 	if !opts.LocalOnly {
@@ -1216,11 +1184,9 @@ func (v *Vaultik) RemoveSnapshot(
 		return result, v.outputRemoveJSON(result)
 	}
 
-	if inLocalIndex {
+	v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID)
-		v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID)
-	}
 
-	if result.RemoteRemoved {
+	if !opts.LocalOnly && result.RemoteRemoved {
 		v.printlnStdout("Removed snapshot metadata from remote storage")
 		v.stdoutf("\nNote: The removed snapshot's blobs remain on the remote. "+
 			"Run '%s' to delete any blobs no longer referenced by any "+
@@ -1230,45 +1196,6 @@ func (v *Vaultik) RemoveSnapshot(
 	return result, nil
 }
 
-// findSnapshotToRemove reports whether the local index has a row for
-// snapshotID, and returns errSnapshotNotFound when there is nothing to
-// remove: no row and, unless LocalOnly is set, no metadata on the
-// destination store either. The destination store is consulted only
-// when the local index has no row, so a snapshot in the local index can
-// still be removed from it while the destination store is unreachable.
-func (v *Vaultik) findSnapshotToRemove(
-	snapshotID string, opts *RemoveOptions,
-) (bool, error) {
-	if v.Repositories != nil {
-		snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID)
-		if err != nil {
-			return false, fmt.Errorf("looking up snapshot in local index: %w", err)
-		}
-
-		if snap != nil {
-			return true, nil
-		}
-	}
-
-	if opts.LocalOnly {
-		return false, fmt.Errorf("%w in the local index: %s",
-			errSnapshotNotFound, snapshotID)
-	}
-
-	objects, err := v.remoteSnapshotObjects(snapshot.RemoteSnapshotKey(snapshotID))
-	if err != nil {
-		return false, fmt.Errorf("looking up snapshot on destination store: %w", err)
-	}
-
-	if len(objects) == 0 {
-		return false, fmt.Errorf(
-			"%w in the local index or on the destination store: %s",
-			errSnapshotNotFound, snapshotID)
-	}
-
-	return false, nil
-}
-
 // removeSnapshotDryRun reports what RemoveSnapshot would do without
 // making changes.
 func (v *Vaultik) removeSnapshotDryRun(
@@ -1328,14 +1255,14 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
 // didn't finish and to run `vaultik snapshot remove` for the snapshot
 // again once the destination store is reachable (`vaultik prune` never
 // removes snapshot metadata). Returns true when the remote removal
-// succeeded and deleted something.
+// succeeded.
 func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) bool {
 	log.Info("Removing snapshot metadata from remote storage",
 		"snapshot_id", snapshotID)
 
 	remoteKey := snapshot.RemoteSnapshotKey(snapshotID)
 
-	deleted, err := v.deleteRemoteSnapshotByKey(remoteKey)
+	err := v.deleteRemoteSnapshotByKey(remoteKey)
 	if err != nil {
 		log.Warn("Could not remove snapshot metadata from remote storage; "+
 			"run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+
@@ -1353,7 +1280,7 @@ func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) b
 		return false
 	}
 
-	return deleted > 0
+	return true
 }
 
 // RemoveAllSnapshots removes every snapshot known to the local
@@ -1645,7 +1572,7 @@ func (v *Vaultik) removeAllLocalSnapshots(
 		}
 
 		if !opts.LocalOnly {
-			_, err = v.deleteRemoteSnapshotByKey(
+			err = v.deleteRemoteSnapshotByKey(
 				snapshot.RemoteSnapshotKey(snapshotID))
 			if err != nil {
 				log.Warn("Failed to remove snapshot metadata from remote",
@@ -1671,7 +1598,7 @@ func (v *Vaultik) removeAllOrphanRemotes(
 	for _, key := range orphanRemoteKeys {
 		log.Info("Removing orphan remote snapshot", "remote_key", key)
 
-		_, err := v.deleteRemoteSnapshotByKey(key)
+		err := v.deleteRemoteSnapshotByKey(key)
 		if err != nil {
 			log.Warn("Failed to remove orphan from remote",
 				"remote_key", key, "error", err)
@@ -1718,44 +1645,34 @@ func (v *Vaultik) deleteSnapshotFromLocalDB(snapshotID string) error {
 }
 
 // deleteRemoteSnapshotByKey removes everything under
-// metadata/<remoteKey>/ on the destination store and returns how many
+// metadata/<remoteKey>/ on the destination store. The argument is a
-// objects it deleted. The argument is a remote key (double-SHA256
+// remote key (double-SHA256 derivation), not a human snapshot ID;
-// derivation), not a human snapshot ID; callers that have a human ID
+// callers that have a human ID must hash via snapshot.RemoteSnapshotKey
-// must hash via snapshot.RemoteSnapshotKey first.
+// first.
-func (v *Vaultik) deleteRemoteSnapshotByKey(remoteKey string) (int, error) {
+func (v *Vaultik) deleteRemoteSnapshotByKey(remoteKey string) error {
-	objectsToDelete, err := v.remoteSnapshotObjects(remoteKey)
+	prefix := fmt.Sprintf("metadata/%s/", remoteKey)
-	if err != nil {
+	objectCh := v.Storage.ListStream(v.ctx, prefix)
-		return 0, err
+
+	var objectsToDelete []string
+
+	for object := range objectCh {
+		if object.Err != nil {
+			return fmt.Errorf("listing objects: %w", object.Err)
+		}
+
+		objectsToDelete = append(objectsToDelete, object.Key)
 	}
 
-	for i, key := range objectsToDelete {
+	for _, key := range objectsToDelete {
-		err = v.Storage.Delete(v.ctx, key)
+		err := v.Storage.Delete(v.ctx, key)
 		if err != nil {
-			return i, fmt.Errorf("removing %s: %w", key, err)
+			return fmt.Errorf("removing %s: %w", key, err)
 		}
 
 		log.Debug("Deleted remote object", "key", key)
 	}
 
-	return len(objectsToDelete), nil
+	return nil
-}
-
-// remoteSnapshotObjects lists the keys of every object under
-// metadata/<remoteKey>/ on the destination store.
-func (v *Vaultik) remoteSnapshotObjects(remoteKey string) ([]string, error) {
-	prefix := fmt.Sprintf("metadata/%s/", remoteKey)
-
-	var keys []string
-
-	for object := range v.Storage.ListStream(v.ctx, prefix) {
-		if object.Err != nil {
-			return nil, fmt.Errorf("listing objects: %w", object.Err)
-		}
-
-		keys = append(keys, object.Key)
-	}
-
-	return keys, nil
 }
 
 // outputRemoveJSON outputs the removal result as JSON

internal/vaultik/snapshot_deleted_count_test.go

@@ -1,149 +0,0 @@
-package vaultik_test
-
-import (
-	"bytes"
-	"context"
-	"fmt"
-	"os"
-	"path/filepath"
-	"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/ui"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// These tests cover https://git.eeqj.de/sneak/vaultik/issues/304: a
-// backup counts a file as deleted only when the previous snapshot of
-// the same name had it and it is gone, and never counts an excluded
-// file.
-
-// deletedCountEnv is a backup setup over the source directory src whose
-// user-facing output is kept in out. Each test configures its snapshots
-// with setSnapshot.
-type deletedCountEnv struct {
-	v   *vaultik.Vaultik
-	out *bytes.Buffer
-	src string
-}
-
-// newDeletedCountEnv writes files, keyed by path relative to src.
-func newDeletedCountEnv(t *testing.T, files map[string][]byte) *deletedCountEnv {
-	t.Helper()
-
-	ctx := context.Background()
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	srcDir := filepath.Join(tempDir, "src")
-
-	for rel, content := range files {
-		path := filepath.Join(srcDir, rel)
-		require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0o755))
-		require.NoError(t, afero.WriteFile(fs, path, content, 0o644))
-	}
-
-	cfg := faultTestConfig()
-	cfg.IndexPath = filepath.Join(tempDir, "index.sqlite")
-	cfg.Snapshots = map[string]config.SnapshotConfig{}
-
-	store, err := storage.NewFileStorer(filepath.Join(tempDir, "remote"))
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, cfg.IndexPath)
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = db.Close() })
-
-	repos := database.NewRepositories(db)
-	out := &bytes.Buffer{}
-	v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
-	v.UI = ui.NewWithColor(out, false)
-
-	return &deletedCountEnv{v: v, out: out, src: srcDir}
-}
-
-// setSnapshot configures the snapshot name to back up the path rel
-// beneath src, with the given excludes.
-func (e *deletedCountEnv) setSnapshot(name, rel string, exclude ...string) {
-	e.v.Config.Snapshots[name] = config.SnapshotConfig{
-		Paths:   []string{filepath.Join(e.src, rel)},
-		Exclude: exclude,
-	}
-}
-
-// backUp runs a backup of the named snapshot and returns its output.
-func (e *deletedCountEnv) backUp(t *testing.T, name string) string {
-	t.Helper()
-
-	e.out.Reset()
-	require.NoError(t, e.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
-		Snapshots: []string{name},
-	}))
-
-	return e.out.String()
-}
-
-// A file this snapshot excludes is not counted as deleted: neither one
-// that another snapshot backs up, nor one that the previous snapshot of
-// this name had before the exclude was added.
-func TestBackupDoesNotCountExcludedFileAsDeleted(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	env := newDeletedCountEnv(t, map[string][]byte{
-		"top.txt":       []byte("top"),
-		"cache/tmp.bin": []byte("cache"),
-		"photos/a.jpg":  []byte("photo a"),
-		"photos/b.jpg":  []byte("photo b"),
-		"photos/c.jpg":  []byte("photo c"),
-	})
-	env.setSnapshot("photos", "photos")
-	env.setSnapshot("home", "", "/photos")
-
-	env.backUp(t, "photos")
-	assert.Contains(t, env.backUp(t, "home"), filesLine(2, 2, 0))
-
-	env.setSnapshot("home", "", "/photos", "/cache")
-	assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
-}
-
-// A file removed from disk is counted as deleted by the next backup of
-// the snapshot that had it, and not by any later backup of that
-// snapshot, nor by the first backup of another name over the same path.
-func TestBackupCountsDeletedFileOnce(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	aContent := bytes.Repeat([]byte("a"), 1000)
-	bContent := bytes.Repeat([]byte("b"), 2000)
-	keptContent := bytes.Repeat([]byte("k"), 500)
-
-	env := newDeletedCountEnv(t, map[string][]byte{
-		"a.txt":    aContent,
-		"b.txt":    bContent,
-		"kept.txt": keptContent,
-	})
-	env.setSnapshot("home", "")
-	env.setSnapshot("other", "")
-
-	assert.Contains(t, env.backUp(t, "home"), filesLine(3, 3, 0))
-
-	require.NoError(t, os.Remove(filepath.Join(env.src, "a.txt")))
-	require.NoError(t, os.Remove(filepath.Join(env.src, "b.txt")))
-
-	summary := env.backUp(t, "home")
-	assert.Contains(t, summary,
-		"Files: 1 examined, 0 backed up, 1 unchanged, 2 deleted.")
-	assert.Contains(t, summary,
-		fmt.Sprintf("Data: %s total (%s backed up), %s deleted.",
-			env.v.UI.Size(int64(len(keptContent))), env.v.UI.Size(0),
-			env.v.UI.Size(int64(len(aContent)+len(bContent)))))
-
-	assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
-	assert.Contains(t, env.backUp(t, "other"), filesLine(1, 0, 1))
-}