Report blobs shallow verify could not check apart from missing ones (closes #318)

Shallow snapshot verify counted every error from checking a blob or the
encrypted database as "missing" and printed the error nowhere, so a
permission error or an S3 403 read as data loss. Now only a not-found
answer from storage counts as missing. Any other error is reported as
could not be checked, with the error storage returned, in the text
report and under --json (the new unchecked list and
database_check_error field), and the run fails. Under --quiet, which
drops the report, each of those errors is logged as a warning on
stderr.

The database check moved into its own small function because the verify
function was at the linter's length limit.

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
-2. **Phase 1b**: Count as deleted the files that the previous snapshot of the same name lists and the walk did not find; a file that is excluded, or inside an excluded directory, is not counted
+1. **Phase 0**: Detect deleted files from previous snapshots
+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()

Dockerfile

@@ -5,16 +5,7 @@
 FROM golangci/golangci-lint:v2.14.0@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint
 WORKDIR /src
 COPY go.mod go.sum ./
-# Without the time limit a stalled connection to the module proxy holds
-# this step, and every build on the host sharing it, until the kernel
-# drops the connection, which can take an hour. The limit is well above
-# the three minutes a working download can take on a busy build host.
-RUN for attempt in 1 2 3; do \
-        timeout 300 go mod download && exit 0; \
-        echo "go mod download attempt $attempt failed or ran over 300s" >&2; \
-    done; \
-    echo "module download failed: giving up after 3 attempts" >&2; \
-    exit 1
+RUN go mod download
 COPY . .
 # `golangci-lint run` silently ignores an unknown top-level key in
 # .golangci.yml, such as a misspelt `linters:`; `config verify` fails on it.
@@ -27,13 +18,7 @@ RUN golangci-lint run --config .golangci.yml ./...
 FROM golang:1.26.1@sha256:cd78d88e00afadbedd272f977d375a6247455f3a4b1178f8ae8bbcb201743a8a AS test
 WORKDIR /src
 COPY go.mod go.sum ./
-# Limited and retried as in the lint phase above.
-RUN for attempt in 1 2 3; do \
-        timeout 300 go mod download && exit 0; \
-        echo "go mod download attempt $attempt failed or ran over 300s" >&2; \
-    done; \
-    echo "module download failed: giving up after 3 attempts" >&2; \
-    exit 1
+RUN go mod download
 COPY . .
 RUN go test -timeout 90s -race -cover ./... || \
     { echo "--- Rerunning with -v for details ---"; \
@@ -51,13 +36,7 @@ RUN apk add --no-cache git
 RUN git config --system --add safe.directory /src
 WORKDIR /src
 COPY go.mod go.sum ./
-# Limited and retried as in the lint phase above.
-RUN for attempt in 1 2 3; do \
-        timeout 300 go mod download && exit 0; \
-        echo "go mod download attempt $attempt failed or ran over 300s" >&2; \
-    done; \
-    echo "module download failed: giving up after 3 attempts" >&2; \
-    exit 1
+RUN go mod download
 COPY . .
 
 # The VERSION build arg when one is given, otherwise

README.md

@@ -294,8 +294,7 @@ needed.
 of the local index and the backup destination store — with timestamps
 and three sizes per snapshot (compressed remote size; total
 uncompressed chunk size; size of the chunks that snapshot stored that
-were not stored before). Both chunk sizes are recorded when the
-snapshot's backup runs.
+were not stored before, recorded when its backup ran).
 
 Listing the destination store does **not** require the age secret key,
 so it works in vaultik's intended configuration, where the backed-up
@@ -376,9 +375,7 @@ on the destination in one go, use `vaultik remote nuke --force`.
 * 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
-  match. A file skipped under `--skip-errors` is reported as failed to
-  restore and is not verified.
+* `--verify`: After restoring, verify every file's chunk hashes match
 
 **`prune`**: Tidy up everything that isn't needed. Runs three passes:
 (1) reconcile the local index against the destination store — any
@@ -389,12 +386,10 @@ manifest on the destination store to compute the still-referenced blob
 set and delete any blob not in that set. Step (3) reads all remote
 manifests — network cost scales with the number of snapshots. `snapshot
 create --prune` runs the same cleanup automatically; this is the
-manual entry point for the same work. If a blob cannot be deleted, the
-command (and `snapshot create --prune`) exits non-zero.
+manual entry point for the same work.
 * `--force`: Skip confirmation prompt
 * `--json`: Output stats as JSON. Also skips the confirmation prompt, as
-  `--force` does. A blob that cannot be deleted is counted in
-  `blobs_failed`, and the document is printed before the non-zero exit.
+  `--force` does.
 
 **`info`**: Display system configuration, storage settings, encryption
 recipients, and local database statistics.
@@ -416,8 +411,7 @@ since `prune` never deletes it.
 **`remote nuke`**: Delete every snapshot's metadata and every blob from the
 backup destination store, leaving the bucket prefix empty. Destructive and
 irreversible. This is the single supported way to wipe the entire
-destination store. If anything cannot be deleted, the command exits
-non-zero; running it again deletes what is left.
+destination store.
 * `--force`: Required to confirm destruction.
 
 **`database delete`**: Delete the local SQLite state database file

cmd/vaultik/dockermoddownload_test.go

@@ -1,45 +0,0 @@
-package main_test
-
-import (
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// TestDockerfileBoundsEveryModuleDownload fails unless every
-// `go mod download` in the Dockerfile runs under a time limit, is tried
-// a fixed number of times, and then fails the build with a message that
-// names the module download (issue #351). Like the version tests, this
-// parses the Dockerfile; that a stalled download really is cut off and
-// retried was checked by hand.
-func TestDockerfileBoundsEveryModuleDownload(t *testing.T) {
-	t.Parallel()
-
-	var downloads []string
-
-	for _, instruction := range instructions(t, productDockerfile) {
-		if strings.Contains(instruction, "go mod download") {
-			downloads = append(downloads, instruction)
-		}
-	}
-
-	require.NotEmpty(t, downloads, "%s must download modules",
-		productDockerfile)
-
-	for _, download := range downloads {
-		assert.Contains(t, download, "timeout 300 go mod download",
-			"%s: a module download must have a time limit: %s",
-			productDockerfile, download)
-		assert.Contains(t, download, "for attempt in 1 2 3",
-			"%s: a module download must be retried: %s",
-			productDockerfile, download)
-		assert.Contains(t, download, "module download failed",
-			"%s: a module download that never succeeds must say so: %s",
-			productDockerfile, download)
-		assert.True(t, strings.HasSuffix(download, "exit 1"),
-			"%s: a module download that never succeeds must fail the"+
-				" build: %s", productDockerfile, download)
-	}
-}

docs/DATAMODEL.md

@@ -90,7 +90,7 @@ Stores information about packed, compressed, and encrypted blob files.
 - `created_ts` (INTEGER NOT NULL) - Creation timestamp
 - `finished_ts` (INTEGER) - Finalization timestamp (NULL if in progress)
 - `uncompressed_size` (INTEGER NOT NULL DEFAULT 0) - Total size of chunks before compression
-- `compressed_size` (INTEGER NOT NULL DEFAULT 0) - Size after compression and encryption. Once uploaded, the size of the object in storage: when the blob was already stored, under another compression level or other recipients, this can differ from the size of the local copy
+- `compressed_size` (INTEGER NOT NULL DEFAULT 0) - Size after compression and encryption
 - `uploaded_ts` (INTEGER) - Upload completion timestamp (NULL if not uploaded)
 
 **Purpose:** Tracks blob lifecycle from creation through upload. The UUID primary key allows immediate association of chunks with blobs.
@@ -122,7 +122,6 @@ Tracks backup snapshots.
 - `new_chunk_size` (INTEGER) - Total size of those chunks before compression; `snapshot list` shows it as NEW CHUNK SIZE
 - `blob_count` (INTEGER) - Number of blobs this snapshot created
 - `total_size` (INTEGER) - Total size of all files
-- `uncompressed_chunk_size` (INTEGER) - Total size before compression of the distinct chunks the snapshot's files reference, recorded when its backup ran; `snapshot list` shows it as UNCOMPRESSED SIZE
 - `blob_size` (INTEGER) - Total compressed size of all referenced blobs
 - `blob_uncompressed_size` (INTEGER) - Total uncompressed size of all referenced blobs
 - `compression_ratio` (REAL) - Compression ratio achieved
@@ -185,7 +184,7 @@ Tracks blob upload metrics.
    - `UPDATE blobs SET blob_hash = ?, finished_ts = ?` - Finalize blob after packing
 
 4. **Upload**
-   - `UPDATE blobs SET uploaded_ts = ?, compressed_size = ?` - Mark blob as uploaded, with the size of the object in storage (the upload is skipped when the blob is already stored)
+   - `UPDATE blobs SET uploaded_ts = ?` - Mark blob as uploaded
    - `INSERT INTO uploads` - Record upload metrics with snapshot_id
    - `INSERT INTO snapshot_blobs` - Associate blob with snapshot
 

internal/database/blob_chunks.go

@@ -58,7 +58,12 @@ func (r *BlobChunkRepository) GetByBlobID(
 		return nil, fmt.Errorf("querying blob chunks: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	var blobChunks []*BlobChunk
 

internal/database/blobs.go

@@ -83,7 +83,12 @@ func (r *BlobRepository) GetAll(ctx context.Context) (map[string]*Blob, error) {
 		return nil, fmt.Errorf("querying blobs: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	out := make(map[string]*Blob)
 
@@ -154,16 +159,13 @@ func (r *BlobRepository) UpdateFinished(
 	return nil
 }
 
-// UpdateUploaded marks a blob as uploaded and records the size of the
-// stored object as its compressed size. That size can differ from the
-// local copy's when an identical blob was already stored with another
-// compression level or other recipients.
+// UpdateUploaded marks a blob as uploaded
 func (r *BlobRepository) UpdateUploaded(
-	ctx context.Context, tx *sql.Tx, id string, storedSize int64,
+	ctx context.Context, tx *sql.Tx, id string,
 ) error {
 	query := `
-		UPDATE blobs
-		SET uploaded_ts = ?, compressed_size = ?
+		UPDATE blobs 
+		SET uploaded_ts = ?
 		WHERE id = ?
 	`
 
@@ -171,9 +173,9 @@ func (r *BlobRepository) UpdateUploaded(
 
 	var err error
 	if tx != nil {
-		_, err = tx.ExecContext(ctx, query, now, storedSize, id)
+		_, err = tx.ExecContext(ctx, query, now, id)
 	} else {
-		_, err = r.db.ExecWithLog(ctx, query, now, storedSize, id)
+		_, err = r.db.ExecWithLog(ctx, query, now, id)
 	}
 
 	if err != nil {

internal/database/blobs_test.go

@@ -122,8 +122,8 @@ func TestBlobRepositoryUpdates(t *testing.T) {
 		t.Errorf("expected compressed size 500, got %d", updated.CompressedSize)
 	}
 
-	// Test UpdateUploaded, with a stored object larger than the local copy
-	err = repo.UpdateUploaded(ctx, nil, blob.ID.String(), 600)
+	// Test UpdateUploaded
+	err = repo.UpdateUploaded(ctx, nil, blob.ID.String())
 	if err != nil {
 		t.Fatalf("failed to update blob as uploaded: %v", err)
 	}
@@ -141,10 +141,6 @@ func TestBlobRepositoryUpdates(t *testing.T) {
 	if uploaded.UploadedTS.Before(now.Add(-1 * time.Second)) {
 		t.Error("uploaded timestamp should be around test time")
 	}
-
-	if uploaded.CompressedSize != 600 {
-		t.Errorf("expected the stored size 600, got %d", uploaded.CompressedSize)
-	}
 }
 
 func TestBlobRepositoryDuplicate(t *testing.T) {

internal/database/chunk_files.go

@@ -61,7 +61,12 @@ func (r *ChunkFileRepository) GetByChunkHash(
 		return nil, fmt.Errorf("querying chunk files: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	return r.scanChunkFiles(rows)
 }
@@ -82,7 +87,12 @@ func (r *ChunkFileRepository) GetByFilePath(
 		return nil, fmt.Errorf("querying chunk files: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	return r.scanChunkFiles(rows)
 }
@@ -102,7 +112,12 @@ func (r *ChunkFileRepository) GetByFileID(
 		return nil, fmt.Errorf("querying chunk files: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	return r.scanChunkFiles(rows)
 }

internal/database/chunks.go

@@ -107,7 +107,12 @@ func (r *ChunkRepository) GetByHashes(
 		return nil, fmt.Errorf("querying chunks: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	var chunks []*Chunk
 
@@ -147,7 +152,12 @@ func (r *ChunkRepository) ListUnpacked(
 		return nil, fmt.Errorf("querying unpacked chunks: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	var chunks []*Chunk
 

internal/database/chunks_ext.go

@@ -38,7 +38,12 @@ func (r *ChunkRepository) list(ctx context.Context, query string) ([]*Chunk, err
 		return nil, fmt.Errorf("querying chunks: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	var chunks []*Chunk
 

internal/database/errors.go

@@ -0,0 +1,12 @@
+package database
+
+import (
+	"fmt"
+	"os"
+)
+
+// Fatalf prints an error message to stderr and exits with status 1
+func Fatalf(format string, args ...any) {
+	fmt.Fprintf(os.Stderr, "FATAL: "+format+"\n", args...)
+	os.Exit(1)
+}

internal/database/file_chunks.go

@@ -62,7 +62,12 @@ func (r *FileChunkRepository) GetByPath(
 		return nil, fmt.Errorf("querying file chunks: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	return r.scanFileChunks(rows)
 }
@@ -83,7 +88,12 @@ func (r *FileChunkRepository) GetByFileID(
 		return nil, fmt.Errorf("querying file chunks: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	return r.scanFileChunks(rows)
 }
@@ -107,7 +117,12 @@ func (r *FileChunkRepository) GetByPathTx(
 		return nil, fmt.Errorf("querying file chunks: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	fileChunks, err := r.scanFileChunks(rows)
 	LogSQL("GetByPathTx", "Complete", path, "count", len(fileChunks))

internal/database/files.go

@@ -175,7 +175,12 @@ func (r *FileRepository) ListModifiedSince(
 		return nil, fmt.Errorf("querying files: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	var files []*File
 
@@ -251,46 +256,12 @@ func (r *FileRepository) ListUnderPath(
 		return nil, fmt.Errorf("querying files: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
-
-	var files []*File
-
-	for rows.Next() {
-		file, err := r.scanFileRows(rows)
+	defer func() {
+		err := rows.Close()
 		if err != nil {
-			return nil, fmt.Errorf("scanning file: %w", err)
+			Fatalf("failed to close rows: %v", err)
 		}
-
-		files = append(files, file)
-	}
-
-	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() { _ = rows.Close() }()
+	}()
 
 	var files []*File
 
@@ -333,7 +304,12 @@ func (r *FileRepository) ListIDsWithChunksNotInUploadedBlobs(
 		return nil, fmt.Errorf("querying files: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	var ids []types.FileID
 
@@ -364,7 +340,12 @@ func (r *FileRepository) ListAll(ctx context.Context) ([]*File, error) {
 		return nil, fmt.Errorf("querying files: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	var files []*File
 

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/database/repositories.go

@@ -106,24 +106,22 @@ func (r *Repositories) WithReadTx(ctx context.Context, fn TxFunc) error {
 
 	defer func() {
 		if p := recover(); p != nil {
-			// The panic is the failure to report, so a rollback error
-			// is dropped.
-			_ = tx.Rollback()
+			rollbackErr := tx.Rollback()
+			if rollbackErr != nil {
+				Fatalf("failed to rollback transaction: %v", rollbackErr)
+			}
 
 			panic(p)
+		} else if err != nil {
+			rollbackErr := tx.Rollback()
+			if rollbackErr != nil {
+				Fatalf("failed to rollback transaction: %v", rollbackErr)
+			}
 		}
 	}()
 
 	err = fn(ctx, tx)
 	if err != nil {
-		// Once ctx is cancelled, database/sql rolls the transaction
-		// back itself and Rollback returns sql.ErrTxDone.
-		rollbackErr := tx.Rollback()
-		if rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) {
-			return fmt.Errorf("%w; rolling back transaction: %w",
-				err, rollbackErr)
-		}
-
 		return err
 	}
 

internal/database/repositories_test.go

@@ -411,41 +411,3 @@ func TestRepositoriesReadTransaction(t *testing.T) {
 		t.Error("expected to retrieve file in read transaction")
 	}
 }
-
-// When the context is cancelled during a read transaction's body,
-// database/sql rolls the transaction back itself, so WithReadTx's own
-// rollback finds it already done. WithReadTx must return the body's error
-// alone, without that rollback.
-func TestRepositoriesReadTransactionContextCancelled(t *testing.T) {
-	t.Parallel()
-
-	db, cleanup := setupTestDB(t)
-	defer cleanup()
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	repos := database.NewRepositories(db)
-
-	err := repos.WithReadTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
-		cancel()
-
-		// database/sql rolls back from a goroutine of its own; wait
-		// until it has, so the test does not depend on that race.
-		for {
-			_, err := tx.ExecContext(context.WithoutCancel(ctx), "SELECT 1")
-			if errors.Is(err, sql.ErrTxDone) {
-				return ctx.Err()
-			}
-
-			time.Sleep(time.Millisecond)
-		}
-	})
-	if !errors.Is(err, context.Canceled) {
-		t.Fatalf("expected context.Canceled, got: %v", err)
-	}
-
-	if errors.Is(err, sql.ErrTxDone) {
-		t.Fatalf("the already-done rollback was reported: %v", err)
-	}
-}

internal/database/schema/001.sql

@@ -89,7 +89,6 @@ CREATE TABLE IF NOT EXISTS snapshots (
     new_chunk_size INTEGER NOT NULL DEFAULT 0,
     blob_count INTEGER NOT NULL DEFAULT 0,
     total_size INTEGER NOT NULL DEFAULT 0,
-    uncompressed_chunk_size INTEGER NOT NULL DEFAULT 0,
     blob_size INTEGER NOT NULL DEFAULT 0,
     blob_uncompressed_size INTEGER NOT NULL DEFAULT 0,
     compression_ratio REAL NOT NULL DEFAULT 1.0,

internal/database/snapshots.go

@@ -120,11 +120,7 @@ func (r *SnapshotRepository) UpdateCounts(
 	return nil
 }
 
-// UpdateExtendedStats updates extended statistics for a snapshot. It also
-// records the total plaintext size of the distinct chunks the snapshot's
-// files reference. Only the snapshot's own backup can take that total: a
-// file's row is shared by every snapshot that lists it, and a later
-// backup of a changed file rewrites the row with the new chunks.
+// UpdateExtendedStats updates extended statistics for a snapshot
 func (r *SnapshotRepository) UpdateExtendedStats(
 	ctx context.Context,
 	tx *sql.Tx,
@@ -149,17 +145,7 @@ func (r *SnapshotRepository) UpdateExtendedStats(
 		    compression_level = ?,
 		    upload_bytes = ?,
 		    upload_duration_ms = ?,
-		    new_chunk_size = ?,
-		    uncompressed_chunk_size = (
-		        SELECT COALESCE(SUM(c.size), 0)
-		        FROM (
-		            SELECT DISTINCT fc.chunk_hash
-		            FROM snapshot_files sf
-		            JOIN file_chunks fc ON sf.file_id = fc.file_id
-		            WHERE sf.snapshot_id = snapshots.id
-		        ) sc
-		        JOIN chunks c ON sc.chunk_hash = c.chunk_hash
-		    )
+		    new_chunk_size = ?
 		WHERE id = ?
 	`
 
@@ -255,7 +241,12 @@ func (r *SnapshotRepository) GetOnlySnapshot(ctx context.Context) (*Snapshot, er
 		return nil, fmt.Errorf("querying snapshots: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	snapshots, err := r.scanSnapshotRows(rows)
 	if err != nil {
@@ -290,7 +281,12 @@ func (r *SnapshotRepository) ListRecent(
 		return nil, fmt.Errorf("querying snapshots: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	return r.scanSnapshotRows(rows)
 }
@@ -505,7 +501,12 @@ func (r *SnapshotRepository) GetBlobHashes(
 		return nil, fmt.Errorf("querying blob hashes: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	var blobs []string
 
@@ -569,22 +570,30 @@ func (r *SnapshotRepository) GetSnapshotBlobSizes(
 	return compressed, uncompressed, nil
 }
 
-// GetSnapshotUncompressedChunkSize returns the total plaintext size of the
-// distinct chunks the snapshot's files reference, as the snapshot's backup
-// recorded it (see UpdateExtendedStats).
+// GetSnapshotUncompressedChunkSize returns the sum of plaintext sizes of all unique
+// chunks referenced by a snapshot (via snapshot_files → file_chunks → chunks).
 func (r *SnapshotRepository) GetSnapshotUncompressedChunkSize(
 	ctx context.Context, snapshotID string,
 ) (int64, error) {
-	query := `SELECT uncompressed_chunk_size FROM snapshots WHERE id = ?`
+	query := `
+		SELECT COALESCE(SUM(c.size), 0)
+		FROM (
+			SELECT DISTINCT fc.chunk_hash
+			FROM snapshot_files sf
+			JOIN file_chunks fc ON sf.file_id = fc.file_id
+			WHERE sf.snapshot_id = ?
+		) sc
+		JOIN chunks c ON sc.chunk_hash = c.chunk_hash
+	`
 
-	var uncompressedChunkSize int64
+	var totalSize int64
 
-	err := r.db.conn.QueryRowContext(ctx, query, snapshotID).Scan(&uncompressedChunkSize)
+	err := r.db.conn.QueryRowContext(ctx, query, snapshotID).Scan(&totalSize)
 	if err != nil {
 		return 0, fmt.Errorf("querying uncompressed chunk size: %w", err)
 	}
 
-	return uncompressedChunkSize, nil
+	return totalSize, nil
 }
 
 // GetSnapshotNewChunkSize returns the total plaintext size of the chunks the
@@ -625,7 +634,12 @@ func (r *SnapshotRepository) GetIncompleteSnapshots(
 		return nil, fmt.Errorf("querying incomplete snapshots: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	return r.scanSnapshotRows(rows)
 }
@@ -648,7 +662,12 @@ func (r *SnapshotRepository) GetIncompleteByHostname(
 		return nil, fmt.Errorf("querying incomplete snapshots: %w", err)
 	}
 
-	defer func() { _ = rows.Close() }()
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			Fatalf("failed to close rows: %v", err)
+		}
+	}()
 
 	// Same column set as every other multi-row snapshot query, so the
 	// shared scanner applies — including its timestamp normalization.

internal/s3/client.go

@@ -18,13 +18,6 @@ import (
 	"github.com/aws/smithy-go/logging"
 )
 
-// ErrNoContinuationToken is returned by ListObjects and ListObjectsStream
-// when the server marks a page of a listing truncated but sends no
-// continuation token to fetch the rest with, as a server answering V2
-// listings V1-style does.
-var ErrNoContinuationToken = errors.New(
-	"server marked the listing truncated but sent no continuation token")
-
 // Client wraps the AWS S3 client for vaultik operations.
 // It provides a simplified interface for S3 operations with automatic
 // prefix handling and connection management. All operations are performed
@@ -221,7 +214,7 @@ func (c *Client) ListObjects(ctx context.Context, prefix string) ([]string, erro
 	})
 
 	for paginator.HasMorePages() {
-		page, err := nextListPage(ctx, paginator)
+		page, err := paginator.NextPage(ctx)
 		if err != nil {
 			return nil, err
 		}
@@ -242,26 +235,6 @@ func (c *Client) ListObjects(ctx context.Context, prefix string) ([]string, erro
 	return keys, nil
 }
 
-// nextListPage fetches the next page of a listing. The SDK paginator takes
-// a page marked truncated without a continuation token for the last page
-// and stops without an error, so such a page is ErrNoContinuationToken
-// here instead.
-func nextListPage(
-	ctx context.Context, paginator *s3.ListObjectsV2Paginator,
-) (*s3.ListObjectsV2Output, error) {
-	page, err := paginator.NextPage(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	truncated := aws.ToBool(page.IsTruncated)
-	if truncated && aws.ToString(page.NextContinuationToken) == "" {
-		return nil, ErrNoContinuationToken
-	}
-
-	return page, nil
-}
-
 // HeadObject checks if an object exists in S3.
 // Returns true if the object exists, false otherwise.
 // The key is automatically prefixed with the configured prefix.
@@ -327,7 +300,7 @@ func (c *Client) ListObjectsStream(
 		})
 
 		for paginator.HasMorePages() {
-			page, err := nextListPage(ctx, paginator)
+			page, err := paginator.NextPage(ctx)
 			if err != nil {
 				ch <- ObjectInfo{Err: err}
 

internal/snapshot/blob_temp_file_test.go

@@ -1,62 +0,0 @@
-//nolint:testpackage // exercises the unexported cleanupBlobTempFile
-package snapshot
-
-import (
-	"errors"
-	"os"
-	"testing"
-
-	"github.com/spf13/afero"
-	"sneak.berlin/go/vaultik/internal/blob"
-)
-
-// A blob temp file that cannot be closed or removed must make the cleanup
-// return an error, not end the process.
-func TestCleanupBlobTempFileReturnsErrors(t *testing.T) {
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	s := &Scanner{fs: fs}
-
-	tests := []struct {
-		name string
-		// prepare runs before the cleanup and makes its close or remove
-		// fail.
-		prepare func(tempFile afero.File) error
-		want    error
-	}{
-		{
-			name:    "close fails",
-			prepare: func(tempFile afero.File) error { return tempFile.Close() },
-			want:    os.ErrClosed,
-		},
-		{
-			name: "remove fails",
-			prepare: func(tempFile afero.File) error {
-				return fs.Remove(tempFile.Name())
-			},
-			want: os.ErrNotExist,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			tempFile, err := afero.TempFile(fs, t.TempDir(), "blob-*.tmp")
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			err = tt.prepare(tempFile)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			err = s.cleanupBlobTempFile(&blob.WithReader{TempFile: tempFile})
-			if !errors.Is(err, tt.want) {
-				t.Fatalf("expected an error wrapping %v, got: %v", tt.want, err)
-			}
-		})
-	}
-}

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,
-			ChunkSize:          cfg.ChunkSize.Int64(),
-			Repositories:       repos,
-			Storage:            storer,
-			MaxBlobSize:        cfg.BlobSizeLimit.Int64(),
-			CompressionLevel:   cfg.CompressionLevel,
-			AgeRecipients:      cfg.AgeRecipients,
-			EnableProgress:     params.EnableProgress,
-			UI:                 params.UI,
-			Exclude:            excludes,
-			SkipErrors:         params.SkipErrors,
-			PreviousSnapshotID: params.PreviousSnapshotID,
+			FS:               params.Fs,
+			ChunkSize:        cfg.ChunkSize.Int64(),
+			Repositories:     repos,
+			Storage:          storer,
+			MaxBlobSize:      cfg.BlobSizeLimit.Int64(),
+			CompressionLevel: cfg.CompressionLevel,
+			AgeRecipients:    cfg.AgeRecipients,
+			EnableProgress:   params.EnableProgress,
+			UI:               params.UI,
+			Exclude:          excludes,
+			SkipErrors:       params.SkipErrors,
 		})
 	}
 }

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
@@ -214,7 +207,6 @@ func NewScanner(cfg ScannerConfig) *Scanner {
 		compiledExclude:    compiledExclude,
 		progress:           progress,
 		skipErrors:         cfg.SkipErrors,
-		previousSnapshotID: cfg.PreviousSnapshotID,
 		ui:                 uiw,
 		pendingChunkHashes: make(map[string]struct{}),
 	}
@@ -266,9 +258,8 @@ func (s *Scanner) Scan(
 
 	filesToProcess := scanResult.FilesToProcess
 
-	// Phase 1b: Detect deleted files by comparing the previous snapshot
-	// of this name against scanned files
-	err = s.detectDeletedFiles(ctx, path, existingFiles, result)
+	// Phase 1b: Detect deleted files by comparing DB against scanned files
+	err = s.detectDeletedFilesFromMap(ctx, knownFiles, existingFiles, result)
 	if err != nil {
 		return nil, fmt.Errorf("detecting deleted files: %w", err)
 	}
@@ -448,8 +439,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
-// queries during the scan phase.
+// database into a map for fast lookup. Every loaded file the scan does
+// 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) {
@@ -1525,8 +1517,7 @@ func (s *Scanner) finalizeProcessPhase(ctx context.Context, result *ScanResult)
 			// snapshot references has uploaded_ts set, so deduplication and
 			// interrupted-run repair treat these blobs as trustworthy.
 			err = s.repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
-				err := s.repos.Blobs.UpdateUploaded(
-					ctx, tx, b.ID, b.Compressed)
+				err := s.repos.Blobs.UpdateUploaded(ctx, tx, b.ID)
 				if err != nil {
 					return fmt.Errorf("marking blob uploaded: %w", err)
 				}
@@ -1562,28 +1553,23 @@ func (s *Scanner) handleBlobReady(
 	blobPath := fmt.Sprintf("blobs/%s/%s/%s",
 		finishedBlob.Hash[:2], finishedBlob.Hash[2:4], finishedBlob.Hash)
 
-	storedSize, blobExists, err := s.uploadBlobIfNeeded(
+	blobExists, err := s.uploadBlobIfNeeded(
 		ctx, blobPath, blobWithReader, startTime, result)
 	if err != nil {
-		cleanupErr := s.cleanupBlobTempFile(blobWithReader)
+		s.cleanupBlobTempFile(blobWithReader)
 
-		return errors.Join(
-			fmt.Errorf("uploading blob %s: %w", finishedBlob.Hash, err), cleanupErr)
+		return fmt.Errorf("uploading blob %s: %w", finishedBlob.Hash, err)
 	}
 
-	err = s.recordBlobMetadata(
-		ctx, finishedBlob, storedSize, blobExists, startTime)
+	err = s.recordBlobMetadata(ctx, finishedBlob, blobExists, startTime)
 	if err != nil {
-		cleanupErr := s.cleanupBlobTempFile(blobWithReader)
+		s.cleanupBlobTempFile(blobWithReader)
 
-		return errors.Join(err, cleanupErr)
-	}
-
-	err = s.cleanupBlobTempFile(blobWithReader)
-	if err != nil {
 		return err
 	}
 
+	s.cleanupBlobTempFile(blobWithReader)
+
 	// Chunks from this blob are now committed to DB - remove from pending set
 	s.removePendingChunkHashes(blobWithReader.InsertedChunkHashes)
 
@@ -1597,22 +1583,20 @@ func (s *Scanner) handleBlobReady(
 }
 
 // uploadBlobIfNeeded uploads the blob to storage if it doesn't already
-// exist. It returns the size of the object in storage, which for an
-// existing object can differ from the local copy's, and whether it
-// already existed.
+// exist, returns whether it existed
 func (s *Scanner) uploadBlobIfNeeded(
 	ctx context.Context,
 	blobPath string,
 	blobWithReader *blob.WithReader,
 	startTime time.Time,
 	result *ScanResult,
-) (int64, bool, error) {
+) (bool, error) {
 	finishedBlob := blobWithReader.FinishedBlob
 
 	// Check if blob already exists (deduplication after restart)
 	destination := s.storage.Info().Location
 
-	stored, err := s.storage.Stat(ctx, blobPath)
+	_, err := s.storage.Stat(ctx, blobPath)
 	if err == nil {
 		log.Info("Blob already exists in storage, skipping upload",
 			"hash", finishedBlob.Hash,
@@ -1621,7 +1605,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 			s.ui.Hex(finishedBlob.Hash), s.ui.Size(finishedBlob.Compressed),
 			s.ui.Path(destination))
 
-		return stored.Size, true, nil
+		return true, nil
 	}
 
 	s.ui.Beginf("Uploading blob %s (%s) to %s.",
@@ -1635,7 +1619,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 	if err != nil {
 		log.Error("Failed to upload blob", "hash", finishedBlob.Hash, "error", err)
 
-		return 0, false, fmt.Errorf("uploading blob to storage: %w", err)
+		return false, fmt.Errorf("uploading blob to storage: %w", err)
 	}
 
 	uploadDuration := time.Since(startTime)
@@ -1666,7 +1650,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 		stats.BytesUploaded.Add(finishedBlob.Compressed)
 	}
 
-	return finishedBlob.Compressed, false, nil
+	return false, nil
 }
 
 // makeUploadProgressCallback creates a progress callback for blob uploads.
@@ -1736,7 +1720,6 @@ func (s *Scanner) makeUploadProgressCallback(
 func (s *Scanner) recordBlobMetadata(
 	ctx context.Context,
 	finishedBlob *blob.FinishedBlob,
-	storedSize int64,
 	blobExists bool,
 	startTime time.Time,
 ) error {
@@ -1748,8 +1731,7 @@ func (s *Scanner) recordBlobMetadata(
 	uploadDuration := time.Since(startTime)
 
 	return s.repos.WithTx(ctx, func(txCtx context.Context, tx *sql.Tx) error {
-		err := s.repos.Blobs.UpdateUploaded(
-			txCtx, tx, finishedBlob.ID, storedSize)
+		err := s.repos.Blobs.UpdateUploaded(txCtx, tx, finishedBlob.ID)
 		if err != nil {
 			return fmt.Errorf("updating blob upload timestamp: %w", err)
 		}
@@ -1780,22 +1762,20 @@ func (s *Scanner) recordBlobMetadata(
 }
 
 // cleanupBlobTempFile closes and removes the blob's temporary file
-func (s *Scanner) cleanupBlobTempFile(blobWithReader *blob.WithReader) error {
+func (s *Scanner) cleanupBlobTempFile(blobWithReader *blob.WithReader) {
 	if blobWithReader.TempFile != nil {
 		tempName := blobWithReader.TempFile.Name()
 
 		err := blobWithReader.TempFile.Close()
 		if err != nil {
-			return fmt.Errorf("closing blob temp file: %w", err)
+			log.Fatal("Failed to close temp file", "file", tempName, "error", err)
 		}
 
 		err = s.fs.Remove(tempName)
 		if err != nil {
-			return fmt.Errorf("removing blob temp file: %w", err)
+			log.Fatal("Failed to remove temp file", "file", tempName, "error", err)
 		}
 	}
-
-	return nil
 }
 
 // streamingChunkInfo tracks chunk metadata collected during streaming
@@ -1962,28 +1942,21 @@ func (s *Scanner) queueFileForBatchInsert(
 	})
 }
 
-// detectDeletedFiles counts as deleted each file at or beneath path that
-// the previous snapshot of this name lists and the walk did not find.
-// A file that is excluded, or inside an excluded directory, is not
-// counted: the walk skips it whether or not it is still on disk.
-func (s *Scanner) detectDeletedFiles(
+// detectDeletedFilesFromMap finds files that existed in previous snapshots
+// but no longer exist. Uses pre-loaded maps to avoid any filesystem or
+// database access.
+func (s *Scanner) detectDeletedFilesFromMap(
 	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(
-		ctx, path, s.previousSnapshotID)
-	if err != nil {
-		return fmt.Errorf("listing files of snapshot %s: %w",
-			s.previousSnapshotID, err)
-	}
-
-	for _, file := range previousFiles {
+	// Check each known file against the enumerated set (no filesystem access needed)
+	for path, file := range knownFiles {
 		// Check context cancellation periodically
 		select {
 		case <-ctx.Done():
@@ -1991,16 +1964,13 @@ func (s *Scanner) detectDeletedFiles(
 		default:
 		}
 
-		filePath := file.Path.String()
-
-		_, exists := existingFiles[filePath]
-		if exists || s.excludedOrInExcludedDir(filePath, path) {
-			continue
+		// Check if the file exists in our enumerated set
+		if _, exists := existingFiles[path]; !exists {
+			// File has been deleted
+			result.FilesDeleted++
+			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 {
@@ -2011,20 +1981,6 @@ func (s *Scanner) detectDeletedFiles(
 	return nil
 }
 
-// excludedOrInExcludedDir reports whether filePath, which is at or
-// beneath rootPath, or any directory between rootPath and it, matches an
-// exclude pattern. A pattern such as */.local/share/Trash matches the
-// directory but not the files inside it, which the walk skips too.
-func (s *Scanner) excludedOrInExcludedDir(filePath, rootPath string) bool {
-	for p := filePath; len(p) > len(rootPath); p = filepath.Dir(p) {
-		if s.shouldExclude(p, rootPath) {
-			return true
-		}
-	}
-
-	return false
-}
-
 // wrapPermissionError augments permission errors with platform-specific
 // remediation instructions. On macOS, TCC-protected directories (Calendars,
 // Reminders, Photos, etc.) return EPERM unless the running application has

internal/storage/faultstore/faultstore.go

@@ -1,16 +1,15 @@
 // Package faultstore provides a storage.Storer wrapper that injects
 // faults on demand, so tests can reproduce the failure modes a real
 // backend exhibits: an upload that fails partway, a backend that reports
-// success while storing nothing, reads that return corrupt or truncated
-// bytes, and a delete the backend refuses. It is the seam called for by
-// the fault-injection tests (sneak/vaultik issue 72) and is meant to be
-// reused by future tests rather than re-implemented per case.
+// success while storing nothing, and reads that return corrupt or
+// truncated bytes. It is the seam called for by the fault-injection
+// tests (sneak/vaultik issue 72) and is meant to be reused by future
+// tests rather than re-implemented per case.
 //
-// The wrapper delegates every method to the inner Storer. Three hooks
-// change that: OnPut decides the fate of each write, OnGet decides how
-// each read's bytes are returned, and OnDelete decides whether each
-// delete fails. All are keyed by the object key, so a test can fault
-// only blobs, only metadata, or a single object.
+// The wrapper delegates every method to the inner Storer. Two hooks
+// change that: OnPut decides the fate of each write, and OnGet decides
+// how each read's bytes are returned. Both are keyed by the object key,
+// so a test can fault only blobs, only metadata, or a single object.
 package faultstore
 
 import (
@@ -26,10 +25,6 @@ import (
 // ErrInjectedUpload is returned by a Put the OnPut hook chose to fail.
 var ErrInjectedUpload = errors.New("faultstore: injected upload failure")
 
-// ErrInjectedDelete is returned by a Delete the OnDelete hook chose to
-// fail.
-var ErrInjectedDelete = errors.New("faultstore: injected delete failure")
-
 // PutAction is the disposition OnPut assigns to a write.
 type PutAction int
 
@@ -71,11 +66,6 @@ type Storer struct {
 	// OnGet, when set, is consulted for every Get with the object key
 	// and damages the returned bytes accordingly.
 	OnGet func(key string) GetFault
-
-	// OnDelete, when set, is consulted before every Delete with the
-	// object key. Returning true fails the delete and leaves the object
-	// in place.
-	OnDelete func(key string) bool
 }
 
 // New wraps inner. inner must be non-nil.
@@ -164,12 +154,8 @@ func (f *Storer) Stat(ctx context.Context, key string) (*storage.ObjectInfo, err
 	return f.inner.Stat(ctx, key)
 }
 
-// Delete removes the object unless OnDelete fails the delete.
+// Delete delegates unchanged.
 func (f *Storer) Delete(ctx context.Context, key string) error {
-	if f.OnDelete != nil && f.OnDelete(key) {
-		return fmt.Errorf("%w for %q", ErrInjectedDelete, key)
-	}
-
 	return f.inner.Delete(ctx, key)
 }
 

internal/storage/s3_test.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"errors"
-	"io"
 	"net/http"
 	"net/http/httptest"
 	"slices"
@@ -23,16 +22,13 @@ import (
 // s3TestBucket is the bucket created for each in-process S3 server.
 const s3TestBucket = "test-bucket"
 
-// Credentials for the tests' S3 clients. The test servers accept any.
+// Credentials for the tests that build a storer from a config.Config. The
+// in-process S3 server accepts any.
 const (
 	s3TestAccessKeyID     = "key"
 	s3TestSecretAccessKey = "secret"
 )
 
-// s3TestRegion is the region of the S3 clients built here with
-// s3.NewClient. The test servers ignore it.
-const s3TestRegion = "us-east-1"
-
 // newS3Storer builds an s3:// backend backed by a fresh in-process
 // S3 server. It reuses the same in-memory S3 harness (gofakes3 + s3mem
 // over httptest) that internal/s3 and the not-found regression test use,
@@ -59,7 +55,7 @@ func newS3Storer(t *testing.T) storage.Storer {
 		Bucket:          s3TestBucket,
 		AccessKeyID:     "test",
 		SecretAccessKey: "test",
-		Region:          s3TestRegion,
+		Region:          "us-east-1",
 	})
 	if err != nil {
 		t.Fatalf("new client: %v", err)
@@ -97,68 +93,6 @@ func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
 	}
 }
 
-// TestS3ListingCutShortIsAnError checks that List and ListStream fail on a
-// listing page marked truncated that carries no continuation token, as a
-// server answering V2 listings V1-style sends it. Taken as the last page,
-// it would hide every later key, and prune would treat the blobs of the
-// snapshots listed there as unreferenced.
-func TestS3ListingCutShortIsAnError(t *testing.T) {
-	t.Parallel()
-
-	const (
-		prefix        = "metadata/"
-		truncatedPage = `<?xml version="1.0" encoding="UTF-8"?>
-<ListBucketResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
-  <Name>test-bucket</Name>
-  <Prefix>metadata/</Prefix>
-  <KeyCount>1</KeyCount>
-  <MaxKeys>1</MaxKeys>
-  <IsTruncated>true</IsTruncated>
-  <Contents>
-    <Key>metadata/snap/manifest.json.zst</Key>
-    <Size>8</Size>
-  </Contents>
-</ListBucketResult>`
-	)
-
-	srv := httptest.NewServer(http.HandlerFunc(
-		func(w http.ResponseWriter, _ *http.Request) {
-			_, _ = io.WriteString(w, truncatedPage)
-		}))
-	t.Cleanup(srv.Close)
-
-	client, err := s3.NewClient(context.Background(), s3.Config{
-		Endpoint:        srv.URL,
-		Bucket:          s3TestBucket,
-		AccessKeyID:     s3TestAccessKeyID,
-		SecretAccessKey: s3TestSecretAccessKey,
-		Region:          s3TestRegion,
-	})
-	if err != nil {
-		t.Fatalf("new client: %v", err)
-	}
-
-	storer := storage.NewS3Storer(client)
-	ctx := context.Background()
-
-	keys, err := storer.List(ctx, prefix)
-	if !errors.Is(err, s3.ErrNoContinuationToken) {
-		t.Errorf("List = %q, %v; want ErrNoContinuationToken", keys, err)
-	}
-
-	var streamErr error
-
-	for obj := range storer.ListStream(ctx, prefix) {
-		if obj.Err != nil {
-			streamErr = obj.Err
-		}
-	}
-
-	if !errors.Is(streamErr, s3.ErrNoContinuationToken) {
-		t.Errorf("ListStream error = %v, want ErrNoContinuationToken", streamErr)
-	}
-}
-
 // TestS3URLPrefixKeyLayout pins the bucket keys an s3:// URL reads and
 // writes: the README's remote storage layout, with the prefix joined to
 // each key by one "/". s3://b/p and s3://b/p/ must be the same

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,
-			Storage:            storer,
-			ChunkSize:          faultChunkSize,
-			MaxBlobSize:        faultMaxBlobSize,
-			CompressionLevel:   cfg.CompressionLevel,
-			AgeRecipients:      cfg.AgeRecipients,
-			Repositories:       repos,
-			EnableProgress:     params.EnableProgress,
-			UI:                 params.UI,
-			Exclude:            params.Exclude,
-			SkipErrors:         params.SkipErrors,
-			PreviousSnapshotID: params.PreviousSnapshotID,
+			FS:               params.Fs,
+			Storage:          storer,
+			ChunkSize:        faultChunkSize,
+			MaxBlobSize:      faultMaxBlobSize,
+			CompressionLevel: cfg.CompressionLevel,
+			AgeRecipients:    cfg.AgeRecipients,
+			Repositories:     repos,
+			EnableProgress:   params.EnableProgress,
+			UI:               params.UI,
+			Exclude:          params.Exclude,
+			SkipErrors:       params.SkipErrors,
 		})
 	}
 }

internal/vaultik/nuke_remote_test.go

@@ -5,14 +5,12 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/snapshot"
-	"sneak.berlin/go/vaultik/internal/storage/faultstore"
 )
 
 // TestNukeRemoteLeavesNoFiles checks that remote nuke leaves no file
@@ -25,7 +23,7 @@ func TestNukeRemoteLeavesNoFiles(t *testing.T) {
 
 	ctx := context.Background()
 	storeDir := filepath.Join(t.TempDir(), "store")
-	v, repos, out := backUpToFileDestination(ctx, t, storeDir)
+	v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
 
 	snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
 	require.NoError(t, err)
@@ -63,82 +61,4 @@ func TestNukeRemoteLeavesNoFiles(t *testing.T) {
 		})
 	require.NoError(t, err)
 	assert.Empty(t, files)
-
-	// Nuke deletes the blobs itself, so advice to run prune is wrong.
-	assert.Contains(t, out.String(), "Backup destination store is now empty.")
-	assert.NotContains(t, out.String(), "vaultik prune")
-}
-
-// TestNukeRemoteFailsWhenADeleteFails checks that remote nuke returns an
-// error when a snapshot's metadata or a blob cannot be deleted, does not
-// report the store empty, and advises running nuke again: prune deletes
-// no metadata.
-func TestNukeRemoteFailsWhenADeleteFails(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	for _, refusedPrefix := range []string{"metadata/", "blobs/"} {
-		t.Run(refusedPrefix, func(t *testing.T) {
-			t.Parallel()
-
-			ctx := context.Background()
-			storeDir := filepath.Join(t.TempDir(), "store")
-			v, _, out := backUpToFileDestination(ctx, t, storeDir)
-
-			store := faultstore.New(v.Storage)
-			store.OnDelete = func(key string) bool {
-				return strings.HasPrefix(key, refusedPrefix)
-			}
-			v.Storage = store
-
-			require.Error(t, v.NukeRemote(true))
-
-			assert.NotContains(t, out.String(), "now empty")
-			assert.Contains(t, out.String(), "vaultik remote nuke --force")
-			assert.NotContains(t, out.String(), "vaultik prune")
-		})
-	}
-}
-
-// TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice checks that
-// remote nuke, when the local index belongs to a different destination
-// than the configured one, fails without advising to run nuke again:
-// that run would fail the same way.
-func TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	storeDir := filepath.Join(t.TempDir(), "store")
-	v, _, out := backUpToFileDestination(ctx, t, storeDir)
-
-	v.Config.StorageURL = "file://" + filepath.Join(t.TempDir(), "elsewhere")
-
-	err := v.NukeRemote(true)
-
-	require.ErrorContains(t, err, "bound to a different backup destination")
-	assert.NotContains(t, out.String(), "vaultik remote nuke --force")
-}
-
-// TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved checks that a
-// snapshot remote nuke cannot remove from the local index fails the
-// run: nuke then leaves that snapshot's metadata on the destination
-// store.
-func TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	storeDir := filepath.Join(t.TempDir(), "store")
-	v, _, out := backUpToFileDestination(ctx, t, storeDir)
-
-	_, err := v.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.Error(t, v.NukeRemote(true))
-
-	assert.NotContains(t, out.String(), "now empty")
-	assert.Contains(t, out.String(), "vaultik remote nuke --force")
 }

internal/vaultik/prune.go

@@ -19,14 +19,6 @@ type PruneOptions struct {
 var errNukeRequiresForce = errors.New(
 	"nuke requires --force (this deletes ALL remote snapshots and blobs)")
 
-// errBlobDeleteFailed reports unreferenced blobs prune could not delete.
-var errBlobDeleteFailed = errors.New("blob(s) could not be deleted")
-
-// nukeCommandHint is the command suggested when a remote nuke could not
-// delete everything: running it again deletes what is left, which
-// `vaultik prune` does not do for snapshot metadata.
-const nukeCommandHint = "vaultik remote nuke --force"
-
 // metadataDirName is the top-level remote directory holding snapshot
 // metadata.
 const metadataDirName = "metadata"
@@ -47,29 +39,6 @@ func (v *Vaultik) NukeRemote(force bool) error {
 		return errNukeRequiresForce
 	}
 
-	// Checked before anything is deleted: running nuke again would fail
-	// the same way, so the advice below must not follow this error.
-	err := v.EnsureStorageBinding()
-	if err != nil {
-		return err
-	}
-
-	err = v.emptyDestinationStore()
-	if err != nil {
-		v.UI.Warningf("Run '%s' again to delete what is left on the "+
-			"backup destination store.", nukeCommandHint)
-
-		return err
-	}
-
-	v.UI.Completef("Backup destination store is now empty.")
-
-	return nil
-}
-
-// emptyDestinationStore does the deleting for NukeRemote and stops at
-// the first step that fails.
-func (v *Vaultik) emptyDestinationStore() error {
 	v.UI.Beginf("Removing all snapshot metadata from backup destination store.")
 
 	_, err := v.RemoveAllSnapshots(&RemoveOptions{Force: true})
@@ -93,6 +62,8 @@ func (v *Vaultik) emptyDestinationStore() error {
 		}
 	}
 
+	v.UI.Completef("Backup destination store is now empty.")
+
 	return nil
 }
 
@@ -195,17 +166,14 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
 	v.deleteUnreferencedBlobs(unreferencedBlobs, allBlobs, result)
 
 	if opts.JSON {
-		err = v.outputPruneBlobsJSON(result)
-		if err != nil {
-			return err
-		}
-	} else {
-		v.stdoutf("\nDeleted %d blob(s) totaling %s\n",
-			result.BlobsDeleted, ubytes(result.BytesFreed))
+		return v.outputPruneBlobsJSON(result)
 	}
 
+	v.stdoutf("\nDeleted %d blob(s) totaling %s\n",
+		result.BlobsDeleted, ubytes(result.BytesFreed))
+
 	if result.BlobsFailed > 0 {
-		return fmt.Errorf("%d %w", result.BlobsFailed, errBlobDeleteFailed)
+		v.stdoutf("Failed to delete %d blob(s)\n", result.BlobsFailed)
 	}
 
 	return nil

internal/vaultik/prune_delete_failure_test.go

@@ -1,98 +0,0 @@
-package vaultik_test
-
-import (
-	"context"
-	"encoding/json"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/storage/faultstore"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// refuseBlobDeletes is an OnDelete hook that fails every blob delete.
-func refuseBlobDeletes(key string) bool {
-	return strings.HasPrefix(key, "blobs/")
-}
-
-// TestPruneBlobs_FailedDeleteReturnsError checks that prune returns an
-// error, so the command exits non-zero, when an unreferenced blob cannot
-// be deleted.
-func TestPruneBlobs_FailedDeleteReturnsError(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	env := newListEnv(t)
-
-	// The store holds no manifest, so nothing references this blob.
-	addBlob(t, env.store.testStorer, testBlobHashA)
-
-	store := faultstore.New(env.store)
-	store.OnDelete = refuseBlobDeletes
-	env.v.Storage = store
-
-	err := env.v.PruneBlobs(&vaultik.PruneOptions{Force: true})
-
-	require.ErrorContains(t, err, "1 blob(s) could not be deleted")
-}
-
-// TestPruneBlobs_JSONFailedDeleteReturnsError checks that prune --json
-// prints its document and then returns an error when an unreferenced
-// blob cannot be deleted.
-func TestPruneBlobs_JSONFailedDeleteReturnsError(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	env := newListEnv(t)
-	addBlob(t, env.store.testStorer, testBlobHashA)
-
-	store := faultstore.New(env.store)
-	store.OnDelete = refuseBlobDeletes
-	env.v.Storage = store
-
-	err := env.v.PruneBlobs(&vaultik.PruneOptions{JSON: true})
-
-	require.Error(t, err, "prune must fail when a blob cannot be deleted")
-
-	var result vaultik.PruneBlobsResult
-
-	require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &result),
-		"stdout must hold the JSON document, got:\n%s", env.stdout.String())
-	assert.Equal(t, 1, result.BlobsFailed)
-	assert.Equal(t, 0, result.BlobsDeleted)
-}
-
-// TestCreateSnapshotPrune_FailedBlobDeleteReturnsError checks that the
-// prune step of snapshot create --prune fails the run when an
-// unreferenced blob cannot be deleted.
-func TestCreateSnapshotPrune_FailedBlobDeleteReturnsError(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	storeDir := filepath.Join(t.TempDir(), "store")
-	v, _, _ := backUpToFileDestination(ctx, t, storeDir)
-
-	// No snapshot references this blob, so the prune step deletes it.
-	orphan := filepath.Join(storeDir, "blobs", testBlobHashA[:2],
-		testBlobHashA[2:4], testBlobHashA)
-	require.NoError(t, os.MkdirAll(filepath.Dir(orphan), 0o750))
-	require.NoError(t, os.WriteFile(orphan, []byte("orphan"), 0o600))
-
-	store := faultstore.New(v.Storage)
-	store.OnDelete = refuseBlobDeletes
-	v.Storage = store
-
-	err := v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
-		Cron:  true,
-		Prune: true,
-	})
-
-	require.ErrorContains(t, err, "1 blob(s) could not be deleted")
-	assert.FileExists(t, orphan)
-}

internal/vaultik/remove_snapshot_test.go

@@ -476,8 +476,7 @@ func TestRemoveAllSnapshots_WithForce(t *testing.T) {
 	out := tv.Stdout.String()
 	assert.Contains(t, out, "Removed 2 snapshot(s)")
 	assert.Contains(t, out, "Removed snapshot metadata from remote storage")
-	// Remote nuke, the only caller, deletes the blobs next itself.
-	assert.NotContains(t, out, "vaultik prune")
+	assert.Contains(t, out, "vaultik prune")
 }
 
 func TestRemoveAllSnapshots_DryRun(t *testing.T) {

internal/vaultik/restore.go

@@ -103,12 +103,11 @@ type RestoreResult struct {
 	BlobsDownloaded int
 	BytesDownloaded int64
 	Duration        time.Duration
-	FilesFailed     int
-	FailedFiles     []string // Paths of files that failed to restore
 	// Verification results (only populated if Verify option is set)
-	FilesVerified     int
-	BytesVerified     int64
-	VerifyFailedFiles []string // Paths of restored files that failed verification
+	FilesVerified int
+	BytesVerified int64
+	FilesFailed   int
+	FailedFiles   []string // Paths of files that failed verification
 }
 
 // Restore restores files from a snapshot to the target directory
@@ -310,22 +309,11 @@ func (v *Vaultik) restoreAllFiles(
 		return nil, err
 	}
 
-	plan, unplanned, err := newRestorePlan(
-		v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
+	plan, err := newRestorePlan(v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
 	if err != nil {
 		return nil, fmt.Errorf("building restore plan: %w", err)
 	}
 
-	// Ranging over files rather than the map reports them in a stable order.
-	for _, file := range files {
-		if planErr, ok := unplanned[file.ID]; ok {
-			err = v.handleRestoreFileError(plan, opts, result, file, file.ID, planErr)
-			if err != nil {
-				return nil, err
-			}
-		}
-	}
-
 	filesByID, totalBytesExpected := indexRestoreFiles(files)
 
 	v.UI.Beginf("Restoring %s files (%s) to %s.",
@@ -659,34 +647,20 @@ func (v *Vaultik) handleRestoreVerification(
 	opts *RestoreOptions,
 	result *RestoreResult,
 ) error {
-	// A file that failed to restore was not written and is already
-	// reported as such, so it is not verified.
-	failedToRestore := make(map[string]bool, len(result.FailedFiles))
-	for _, path := range result.FailedFiles {
-		failedToRestore[path] = true
-	}
-
-	restored := make([]*database.File, 0, len(files))
-	for _, file := range files {
-		if !failedToRestore[file.Path.String()] {
-			restored = append(restored, file)
-		}
-	}
-
-	err := v.verifyRestoredFiles(v.ctx, repos, restored, opts.TargetDir, result)
+	err := v.verifyRestoredFiles(v.ctx, repos, files, opts.TargetDir, result)
 	if err != nil {
 		return fmt.Errorf("verification failed: %w", err)
 	}
 
-	if len(result.VerifyFailedFiles) > 0 {
+	if result.FilesFailed > 0 {
 		v.UI.Errorf("Verification failed: %s files did not match expected checksums.",
-			v.UI.Count(len(result.VerifyFailedFiles)))
+			v.UI.Count(result.FilesFailed))
 
-		for _, path := range result.VerifyFailedFiles {
+		for _, path := range result.FailedFiles {
 			v.UI.Detailf("%s", v.UI.Path(path))
 		}
 
-		return fmt.Errorf("%d %w", len(result.VerifyFailedFiles), errFilesFailedVerify)
+		return fmt.Errorf("%d %w", result.FilesFailed, errFilesFailedVerify)
 	}
 
 	v.UI.Completef("Verified %s files (%s).",
@@ -717,8 +691,6 @@ func (v *Vaultik) downloadSnapshotDB(
 		return nil, "", fmt.Errorf("downloading %s: %w", dbKey, err)
 	}
 
-	reader = newCloseOnCancelReader(v.ctx, reader)
-
 	defer func() { _ = reader.Close() }()
 
 	// Decrypt and decompress straight from the storage stream, then stream
@@ -1448,8 +1420,8 @@ func (v *Vaultik) verifyRestoredFiles(
 		if err != nil {
 			log.Error("File verification failed", "path", file.Path, "error", err)
 
-			result.VerifyFailedFiles = append(
-				result.VerifyFailedFiles, file.Path.String())
+			result.FilesFailed++
+			result.FailedFiles = append(result.FailedFiles, file.Path.String())
 		}
 
 		bytesProcessed += file.Size
@@ -1465,7 +1437,7 @@ func (v *Vaultik) verifyRestoredFiles(
 	log.Info("Verification complete",
 		"files_verified", result.FilesVerified,
 		"bytes_verified", ubytes(result.BytesVerified),
-		"files_failed", len(result.VerifyFailedFiles),
+		"files_failed", result.FilesFailed,
 	)
 
 	return nil

internal/vaultik/restore_interrupt_test.go

@@ -207,37 +207,30 @@ func TestRestoreSkipErrorsCancelDuringBlobDownload(t *testing.T) {
 		"the cancel was reported as a failed file")
 }
 
-// stallingStorer wraps a Storer so that each download of a key chosen by
-// stalls hands over the first half of the object and then stalls,
-// ignoring the context, as a file:// read on a hung USB stick or network
-// share does. Only closing the returned reader ends the stall.
-type stallingStorer struct {
+// stallingBlobStorer wraps a Storer so that every blob download hands
+// over the first half of the blob and then stalls, ignoring the context,
+// as a file:// blob on a hung USB stick or network share does. Only
+// closing the returned reader ends the stall.
+type stallingBlobStorer struct {
 	storage.Storer
 
-	stalls  func(key string) bool
 	once    sync.Once
 	stalled chan struct{}
 }
 
-func newStallingStorer(
-	inner storage.Storer, stalls func(key string) bool,
-) *stallingStorer {
-	return &stallingStorer{
-		Storer:  inner,
-		stalls:  stalls,
-		stalled: make(chan struct{}),
-	}
+func newStallingBlobStorer(inner storage.Storer) *stallingBlobStorer {
+	return &stallingBlobStorer{Storer: inner, stalled: make(chan struct{})}
 }
 
-func (s *stallingStorer) Get(
+func (s *stallingBlobStorer) Get(
 	ctx context.Context, key string,
 ) (io.ReadCloser, error) {
 	rc, err := s.Storer.Get(ctx, key)
-	if err != nil || !s.stalls(key) {
+	if err != nil || !strings.HasPrefix(key, "blobs/") {
 		return rc, err
 	}
 
-	object, err := io.ReadAll(rc)
+	blob, err := io.ReadAll(rc)
 	_ = rc.Close()
 
 	if err != nil {
@@ -249,7 +242,7 @@ func (s *stallingStorer) Get(
 	pr, pw := io.Pipe()
 
 	go func() {
-		_, _ = pw.Write(object[:len(object)/2])
+		_, _ = pw.Write(blob[:len(blob)/2])
 
 		s.once.Do(func() { close(s.stalled) })
 	}()
@@ -257,26 +250,17 @@ func (s *stallingStorer) Get(
 	return pr, nil
 }
 
-// TestInterruptDuringStalledReadCleansTempDir stops a restore and a deep
-// verify while a read of a blob or of the snapshot database is stalled
-// on a source that ignores the context. Stopping must end that read, so
-// the operation unwinds and removes its decrypted scratch files well
-// within the shutdown timeout (issues #325 and #356).
+// TestInterruptDuringStalledBlobReadCleansTempDir stops a restore and a
+// deep verify while a blob read is stalled on a source that ignores the
+// context. Stopping must end that read, so the operation unwinds and
+// removes its decrypted scratch files well within the shutdown timeout
+// (issue #325).
 //
 // Not parallel: it points TMPDIR at a private directory (via t.Setenv)
 // so it can assert on exactly the scratch files the operation created.
-func TestInterruptDuringStalledReadCleansTempDir(t *testing.T) {
+func TestInterruptDuringStalledBlobReadCleansTempDir(t *testing.T) {
 	log.Initialize(log.Config{})
 
-	stalledReads := map[string]func(key string) bool{
-		"blob": func(key string) bool {
-			return strings.HasPrefix(key, "blobs/")
-		},
-		"snapshot database": func(key string) bool {
-			return strings.HasSuffix(key, "/db.zst.age")
-		},
-	}
-
 	operations := map[string]func(v *Vaultik, snapshotID, targetDir string) error{
 		"restore": func(v *Vaultik, snapshotID, targetDir string) error {
 			return v.Restore(&RestoreOptions{
@@ -289,58 +273,52 @@ func TestInterruptDuringStalledReadCleansTempDir(t *testing.T) {
 		},
 	}
 
-	for readName, stalls := range stalledReads {
-		for opName, op := range operations {
-			t.Run(readName+" read during "+opName, func(t *testing.T) {
-				fs := afero.NewOsFs()
-				root := t.TempDir()
+	for name, op := range operations {
+		t.Run(name, func(t *testing.T) {
+			fs := afero.NewOsFs()
+			root := t.TempDir()
 
-				// Random content, so the blob stays large enough to stall
-				// part-way through its compressed, encrypted body.
-				cfg, storer, snapshotID, _ := backupOneFile(context.Background(),
-					t, fs, root, "a.bin", randomBytes(t, 256*1024), 0o644)
+			// Random content, so the blob stays large enough to stall
+			// part-way through its compressed, encrypted body.
+			cfg, storer, snapshotID, _ := backupOneFile(context.Background(),
+				t, fs, root, "a.bin", randomBytes(t, 256*1024), 0o644)
 
-				scratch := filepath.Join(root, "scratch")
-				require.NoError(t, fs.MkdirAll(scratch, 0o755))
-				t.Setenv("TMPDIR", scratch)
+			scratch := filepath.Join(root, "scratch")
+			require.NoError(t, fs.MkdirAll(scratch, 0o755))
+			t.Setenv("TMPDIR", scratch)
 
-				stalling := newStallingStorer(storer, stalls)
+			stalling := newStallingBlobStorer(storer)
 
-				ctx, cancel := context.WithCancel(context.Background())
-				defer cancel()
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
 
-				v := newRestoreVaultik(ctx, cfg, stalling, fs)
-				v.cancel = cancel
+			v := newRestoreVaultik(ctx, cfg, stalling, fs)
+			v.cancel = cancel
 
-				var opErr error
+			var opErr error
 
-				stop := v.StartOperation(func() {
-					opErr = op(v, snapshotID, filepath.Join(root, "restored"))
-				})
-
-				select {
-				case <-stalling.stalled:
-				case <-time.After(30 * time.Second):
-					t.Fatal("the operation never reached the stalled read")
-				}
-
-				// The snapshot database's temp directory is created just
-				// after its read starts, so it can appear after the stall.
-				require.Eventually(t, func() bool {
-					return len(scratchEntries(t, scratch)) > 0
-				}, 10*time.Second, 10*time.Millisecond,
-					"expected decrypted scratch files to exist mid-read")
-
-				stopCtx, stopCancel := context.WithTimeout(
-					context.Background(), 10*time.Second)
-				defer stopCancel()
-
-				require.True(t, stop(stopCtx),
-					"stop timed out; the stalled read was not interrupted")
-				require.ErrorIs(t, opErr, context.Canceled)
-				require.Empty(t, scratchEntries(t, scratch),
-					"decrypted scratch files remained after the interrupt")
+			stop := v.StartOperation(func() {
+				opErr = op(v, snapshotID, filepath.Join(root, "restored"))
 			})
-		}
+
+			select {
+			case <-stalling.stalled:
+			case <-time.After(30 * time.Second):
+				t.Fatal("the operation never reached a blob read")
+			}
+
+			require.NotEmpty(t, scratchEntries(t, scratch),
+				"expected decrypted scratch files to exist mid-read")
+
+			stopCtx, stopCancel := context.WithTimeout(
+				context.Background(), 10*time.Second)
+			defer stopCancel()
+
+			require.True(t, stop(stopCtx),
+				"stop timed out; the stalled blob read was not interrupted")
+			require.ErrorIs(t, opErr, context.Canceled)
+			require.Empty(t, scratchEntries(t, scratch),
+				"decrypted scratch files remained after the interrupt")
+		})
 	}
 }

internal/vaultik/restore_malformed_db_test.go

@@ -61,8 +61,8 @@ func craftedFile(
 }
 
 // TestRestoreShortChunkHashInNoBlob proves a file whose short chunk hash
-// has no blob_chunks row is left out of the restore plan with an error,
-// and fails the chunk write with an error.
+// has no blob_chunks row fails restore planning and the chunk write with
+// an error.
 func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
 	t.Parallel()
 
@@ -77,11 +77,9 @@ func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
 	chunkToBlobMap, err := v.buildChunkToBlobMap(ctx, repos)
 	require.NoError(t, err)
 
-	plan, unplanned, err := newRestorePlan(ctx, repos, []*database.File{file},
+	_, err = newRestorePlan(ctx, repos, []*database.File{file},
 		chunkToBlobMap, map[string]string{})
-	require.NoError(t, err)
-	require.ErrorIs(t, unplanned[file.ID], errPlanChunkMissing)
-	require.False(t, plan.hasPending())
+	require.ErrorIs(t, err, errPlanChunkMissing)
 
 	fileChunks, err := repos.FileChunks.GetByFileID(ctx, file.ID)
 	require.NoError(t, err)

internal/vaultik/restore_plan.go

@@ -41,25 +41,20 @@ type restorePlan struct {
 
 // newRestorePlan builds the file→blob index for the given files. Files
 // whose chunks reference no blobs (symlinks, directories) start in the
-// ready queue immediately. A file with a chunk that no blob holds cannot
-// be restored: it is left out of the plan and returned, by ID, with the
-// reason.
+// ready queue immediately.
 func newRestorePlan(
 	ctx context.Context,
 	repos *database.Repositories,
 	files []*database.File,
 	chunkToBlobMap map[string]*database.BlobChunk,
 	blobIDToHash map[string]string,
-) (*restorePlan, map[types.FileID]error, error) {
+) (*restorePlan, error) {
 	p := &restorePlan{
 		fileBlobs: make(map[types.FileID]map[string]struct{}, len(files)),
 		blobFiles: make(map[string]map[types.FileID]struct{}),
 		ready:     make([]types.FileID, 0, len(files)),
 		cached:    make(map[string]struct{}),
 	}
-	unplanned := make(map[types.FileID]error)
-
-nextFile:
 	for _, f := range files {
 		if f.IsSymlink() || f.Mode&uint32(os.ModeDir) != 0 {
 			// No chunks to fetch — restore can run immediately.
@@ -71,7 +66,7 @@ nextFile:
 
 		fileChunks, err := repos.FileChunks.GetByFileID(ctx, f.ID)
 		if err != nil {
-			return nil, nil, fmt.Errorf("planning %s: %w", f.Path, err)
+			return nil, fmt.Errorf("planning %s: %w", f.Path, err)
 		}
 
 		blobs := make(map[string]struct{})
@@ -79,18 +74,14 @@ nextFile:
 		for _, fc := range fileChunks {
 			bc, ok := chunkToBlobMap[fc.ChunkHash.String()]
 			if !ok {
-				unplanned[f.ID] = fmt.Errorf("%w: %s",
-					errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
-
-				continue nextFile
+				return nil, fmt.Errorf("planning %s: %w: %s",
+					f.Path, errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
 			}
 
 			hash, ok := blobIDToHash[bc.BlobID.String()]
 			if !ok {
-				unplanned[f.ID] = fmt.Errorf("%w: %s",
-					errPlanBlobIDMissing, bc.BlobID)
-
-				continue nextFile
+				return nil, fmt.Errorf("planning %s: %w: %s",
+					f.Path, errPlanBlobIDMissing, bc.BlobID)
 			}
 
 			blobs[hash] = struct{}{}
@@ -112,7 +103,7 @@ nextFile:
 		}
 	}
 
-	return p, unplanned, nil
+	return p, nil
 }
 
 // markBlobCached records that the named blob is now resident in the

internal/vaultik/restore_skip_errors_test.go

@@ -4,9 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/spf13/afero"
@@ -30,8 +28,7 @@ const (
 )
 
 // missingBlobBackup is a snapshot of single-chunk files spread over two
-// blobs, with one of those blobs lost: deleted from the store, or missing
-// from the snapshot metadata.
+// blobs, with one of those blobs deleted from the store.
 type missingBlobBackup struct {
 	fs         afero.Fs
 	cfg        *config.Config
@@ -40,39 +37,20 @@ type missingBlobBackup struct {
 	restoreDir string
 	// files holds the original content by source path.
 	files map[string][]byte
-	// lost holds the source paths whose chunk is in the lost blob.
+	// lost holds the source paths whose chunk is in the deleted blob.
 	lost map[string]bool
 }
 
 // TestRestoreSkipErrorsSkipsFilesOfMissingBlob restores with SkipErrors
-// after one blob of a two-blob snapshot was deleted from the store.
+// after one blob of a two-blob snapshot was deleted. Every file stored in
+// that blob must be reported as failed and left absent, every other file
+// must be restored intact, and Restore must still return an error.
 func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
 
 	ctx := context.Background()
-	assertRestoreSkipsLostFiles(ctx, t, backupThenDeleteOneBlob(ctx, t))
-}
-
-// TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob restores with
-// SkipErrors a snapshot whose metadata lists chunks that no blob holds, as
-// earlier backup defects left behind.
-func TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	assertRestoreSkipsLostFiles(ctx, t, backupThenDropOneBlobFromMetadata(ctx, t))
-}
-
-// assertRestoreSkipsLostFiles restores backup with SkipErrors. Every file
-// stored in the lost blob must be reported as failed and left absent,
-// every other file must be restored intact, and Restore must still return
-// an error.
-func assertRestoreSkipsLostFiles(
-	ctx context.Context, t *testing.T, backup *missingBlobBackup,
-) {
-	t.Helper()
+	backup := backupThenDeleteOneBlob(ctx, t)
 
 	var out bytes.Buffer
 
@@ -94,142 +72,18 @@ func assertRestoreSkipsLostFiles(
 
 		if backup.lost[path] {
 			assert.Containsf(t, out.String(), path,
-				"%s needs the lost blob and must be reported", path)
+				"%s needs the deleted blob and must be reported", path)
 			assert.NoFileExists(t, restored)
 
 			continue
 		}
 
 		got, err := afero.ReadFile(backup.fs, restored)
-		require.NoErrorf(t, err, "%s does not need the lost blob", path)
+		require.NoErrorf(t, err, "%s does not need the deleted blob", path)
 		assert.Equalf(t, content, got, "%s restored with wrong content", path)
 	}
 }
 
-// TestRestoreSkipErrorsVerifyChecksOnlyRestoredFiles restores with
-// SkipErrors and Verify after one blob of a two-blob snapshot was
-// deleted. Verification must check only the files that were written, so
-// a file of the deleted blob is reported as failed to restore and not
-// also as failing verification, and Restore must still return an error.
-func TestRestoreSkipErrorsVerifyChecksOnlyRestoredFiles(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	backup := backupThenDeleteOneBlob(ctx, t)
-
-	var out bytes.Buffer
-
-	v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, backup.fs)
-	v.UI = ui.NewWithColor(&out, false)
-
-	err := v.Restore(&vaultik.RestoreOptions{
-		SnapshotID: backup.snapshotID,
-		TargetDir:  backup.restoreDir,
-		Verify:     true,
-		SkipErrors: true,
-	})
-
-	require.Error(t, err, "restore must fail when files were skipped")
-	assert.Contains(t, err.Error(),
-		fmt.Sprintf("%d file(s) failed to restore", len(backup.lost)))
-
-	written := len(backup.files) - len(backup.lost)
-
-	assert.Contains(t, out.String(), fmt.Sprintf("Verifying %d files", written))
-	assert.Contains(t, out.String(), fmt.Sprintf("Verified %d files", written))
-	assert.NotContains(t, out.String(), "Verification failed")
-}
-
-// TestRestoreSkipErrorsVerifyReportsFailuresApart restores with
-// SkipErrors and Verify after one blob of a two-blob snapshot was
-// deleted, while one file that does restore is written one byte too long.
-// The restore warning must list only the files of the deleted blob, the
-// verification summary only the long file, each path once, and Restore
-// must return an error.
-func TestRestoreSkipErrorsVerifyReportsFailuresApart(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	backup := backupThenDeleteOneBlob(ctx, t)
-
-	var long string
-
-	for path := range backup.files {
-		if !backup.lost[path] {
-			long = path
-
-			break
-		}
-	}
-
-	var out bytes.Buffer
-
-	fs := trailingByteFs{Fs: backup.fs, suffix: long}
-	v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, fs)
-	v.UI = ui.NewWithColor(&out, false)
-
-	err := v.Restore(&vaultik.RestoreOptions{
-		SnapshotID: backup.snapshotID,
-		TargetDir:  backup.restoreDir,
-		Verify:     true,
-		SkipErrors: true,
-	})
-	require.Error(t, err, "restore must fail when files failed")
-
-	_, afterWarning, found := strings.Cut(out.String(),
-		fmt.Sprintf("%d file(s) failed to restore:", len(backup.lost)))
-	require.Truef(t, found, "no restore warning in:\n%s", out.String())
-
-	warning, summary, found := strings.Cut(afterWarning,
-		"Verification failed: 1 files did not match")
-	require.Truef(t, found, "no verification summary in:\n%s", out.String())
-
-	for path := range backup.lost {
-		assert.Equalf(t, 1, strings.Count(warning, path),
-			"%s must be listed once as failed to restore", path)
-		assert.NotContainsf(t, summary, path,
-			"%s was not restored and must not fail verification", path)
-	}
-
-	assert.NotContainsf(t, warning, long,
-		"%s was restored and must not be listed as failed to restore", long)
-	assert.Equalf(t, 1, strings.Count(summary, long),
-		"%s must be listed once as failing verification", long)
-}
-
-// trailingByteFs appends one byte, on close, to the file it opens whose
-// path ends in suffix, so that file is restored one byte too long and
-// fails verification.
-type trailingByteFs struct {
-	afero.Fs
-
-	suffix string
-}
-
-//nolint:ireturn // afero.Fs.OpenFile is defined to return the interface
-func (fs trailingByteFs) OpenFile(
-	name string, flag int, perm os.FileMode,
-) (afero.File, error) {
-	f, err := fs.Fs.OpenFile(name, flag, perm)
-	if err != nil || !strings.HasSuffix(name, fs.suffix) {
-		return f, err
-	}
-
-	return trailingByteFile{File: f}, nil
-}
-
-type trailingByteFile struct {
-	afero.File
-}
-
-func (f trailingByteFile) Close() error {
-	_, _ = f.Write([]byte{'x'})
-
-	return f.File.Close()
-}
-
 // TestRestoreMissingBlobAbortsWithoutSkipErrors checks that a deleted blob
 // still ends the restore with an error when SkipErrors is not set.
 func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
@@ -249,30 +103,6 @@ func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
 	require.ErrorIs(t, err, storage.ErrNotFound)
 }
 
-// TestRestoreChunkInNoBlobAbortsWithoutSkipErrors checks that a chunk held
-// by no blob still ends the restore, before any file is written, when
-// SkipErrors is not set.
-func TestRestoreChunkInNoBlobAbortsWithoutSkipErrors(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	backup := backupThenDropOneBlobFromMetadata(ctx, t)
-
-	v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, backup.fs)
-
-	err := v.Restore(&vaultik.RestoreOptions{
-		SnapshotID: backup.snapshotID,
-		TargetDir:  backup.restoreDir,
-	})
-
-	require.ErrorContains(t, err, "chunk missing from blob map")
-
-	for path := range backup.files {
-		assert.NoFileExists(t, filepath.Join(backup.restoreDir, path))
-	}
-}
-
 // backupThenDeleteOneBlob backs up missingBlobFileCount single-chunk
 // files, then deletes from the store the blob holding the first of them.
 func backupThenDeleteOneBlob(
@@ -280,54 +110,6 @@ func backupThenDeleteOneBlob(
 ) *missingBlobBackup {
 	t.Helper()
 
-	backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
-	require.NoError(t, db.Close())
-
-	require.NoError(t, backup.storer.Delete(ctx, fmt.Sprintf(
-		"blobs/%s/%s/%s", lostBlob[:2], lostBlob[2:4], lostBlob)))
-
-	return backup
-}
-
-// backupThenDropOneBlobFromMetadata backs up missingBlobFileCount
-// single-chunk files, then removes the blob holding the first of them from
-// the snapshot and exports the snapshot metadata again. The exported
-// metadata then lists chunks that no blob holds.
-func backupThenDropOneBlobFromMetadata(
-	ctx context.Context, t *testing.T,
-) *missingBlobBackup {
-	t.Helper()
-
-	backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
-
-	// The uploads row refers to the blob row, so it would keep the export
-	// from deleting it.
-	for _, query := range []string{
-		"DELETE FROM snapshot_blobs WHERE blob_hash = ?",
-		"DELETE FROM uploads WHERE blob_hash = ?",
-	} {
-		_, err := db.Conn().ExecContext(ctx, query, lostBlob)
-		require.NoError(t, err)
-	}
-
-	sm := newFaultSnapshotManager(
-		backup.fs, backup.storer, backup.cfg, database.NewRepositories(db))
-	require.NoError(t,
-		sm.ExportSnapshotMetadata(ctx, db.Path(), backup.snapshotID))
-	require.NoError(t, db.Close())
-
-	return backup
-}
-
-// backupSingleChunkFiles backs up missingBlobFileCount single-chunk files.
-// It returns the backup, with lost naming the files stored in the same
-// blob as the first of them, the still open local index, and the hash of
-// that blob.
-func backupSingleChunkFiles(
-	ctx context.Context, t *testing.T,
-) (*missingBlobBackup, *database.DB, string) {
-	t.Helper()
-
 	fs := afero.NewOsFs()
 	tempDir := t.TempDir()
 	dataDir := filepath.Join(tempDir, "src")
@@ -361,11 +143,16 @@ func backupSingleChunkFiles(
 		blobOfFile[path] = blobHashOfSingleChunkFile(ctx, t, repos, path)
 	}
 
-	lostBlob := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
+	require.NoError(t, db.Close())
+
+	deleted := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
+	require.NoError(t, storer.Delete(ctx, fmt.Sprintf(
+		"blobs/%s/%s/%s", deleted[:2], deleted[2:4], deleted)))
+
 	lost := make(map[string]bool)
 
 	for path, hash := range blobOfFile {
-		if hash == lostBlob {
+		if hash == deleted {
 			lost[path] = true
 		}
 	}
@@ -381,7 +168,7 @@ func backupSingleChunkFiles(
 		restoreDir: filepath.Join(tempDir, "restored"),
 		files:      files,
 		lost:       lost,
-	}, db, lostBlob
+	}
 }
 
 // blobHashOfSingleChunkFile returns the hash of the blob holding the one

internal/vaultik/reused_blob_verify_test.go

@@ -1,83 +0,0 @@
-package vaultik_test
-
-import (
-	"context"
-	"io"
-	"path/filepath"
-	"testing"
-
-	"filippo.io/age"
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/ui"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// TestVerifyPassesWhenBackupReusesStoredBlob backs up a file, then backs it
-// up again from an empty local index, as after `database delete`, with an
-// extra age recipient. The second backup builds the same blob, finds it
-// already stored and skips the upload, but its own copy is larger, because
-// age writes a header entry for each recipient. The second snapshot must
-// record the size of the stored object, so shallow and deep verify pass.
-func TestVerifyPassesWhenBackupReusesStoredBlob(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-
-	dataDir := filepath.Join(tempDir, "source")
-	storeDir := filepath.Join(tempDir, "remote")
-	secondDBPath := filepath.Join(tempDir, "second.sqlite")
-
-	chunkSize := int64(64 * 1024)
-	maxBlobSize := int64(512 * 1024)
-
-	require.NoError(t, fs.MkdirAll(dataDir, 0o755))
-	require.NoError(t, afero.WriteFile(fs,
-		filepath.Join(dataDir, "data.bin"),
-		bytesPattern("reuse-", int(chunkSize*3)), 0o644))
-
-	ctx := context.Background()
-
-	runFileStorageBackup(ctx, t, fs, dataDir, storeDir,
-		filepath.Join(tempDir, "first.sqlite"), chunkSize, maxBlobSize)
-
-	env := setupDedupBackupEnv(
-		ctx, t, fs, storeDir, secondDBPath, chunkSize, maxBlobSize)
-
-	defer func() { _ = env.db.Close() }()
-
-	extra, err := age.GenerateX25519Identity()
-	require.NoError(t, err)
-
-	env.cfg.AgeRecipients = append(env.cfg.AgeRecipients,
-		extra.Recipient().String())
-
-	snapshotID, result := runDedupSnapshot(ctx, t, env.sm, env.makeScanner(),
-		env.cfg.Hostname, "second", dataDir, secondDBPath)
-	require.Positive(t, result.BlobsCreated)
-	require.Zero(t, result.BlobsUploaded,
-		"the second backup should find every blob already stored")
-
-	require.NoError(t, env.db.Close())
-
-	verifier := &vaultik.Vaultik{
-		Config:  env.cfg,
-		Storage: env.storer,
-		Fs:      fs,
-		Stdout:  io.Discard,
-		Stderr:  io.Discard,
-		UI:      ui.NewWithColor(io.Discard, false),
-	}
-	verifier.SetContext(ctx)
-
-	require.NoError(t,
-		verifier.VerifySnapshotWithOptions(snapshotID, &vaultik.VerifyOptions{}),
-		"shallow verify should pass")
-	require.NoError(t,
-		verifier.VerifySnapshotWithOptions(snapshotID,
-			&vaultik.VerifyOptions{Deep: true}),
-		"deep verify should pass")
-}

internal/vaultik/snapshot.go

@@ -25,7 +25,6 @@ var (
 	errSnapshotVerifyFailed = errors.New("verification failed")
 	errRemoveAllNeedsForce  = errors.New("--all requires --force")
 	errSnapshotNotFound     = errors.New("snapshot not found")
-	errSnapshotRemoveFailed = errors.New("snapshot(s) could not be removed")
 	errInvalidTableName     = errors.New("invalid table name")
 	errNoAgeRecipients      = errors.New(
 		"creating a snapshot needs at least one public key in " +
@@ -243,18 +242,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,
-		UI:                 v.UI,
-		Fs:                 v.Fs,
-		Exclude:            v.Config.GetExcludes(snapName),
-		SkipErrors:         opts.SkipErrors,
-		PreviousSnapshotID: previousSnapshotID,
+		EnableProgress: !opts.Cron,
+		UI:             v.UI,
+		Fs:             v.Fs,
+		Exclude:        v.Config.GetExcludes(snapName),
+		SkipErrors:     opts.SkipErrors,
 	})
 
 	snapshotID, err := v.SnapshotManager.CreateSnapshotWithName(
@@ -289,24 +282,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) {
@@ -1616,8 +1591,8 @@ func (v *Vaultik) handleRemoveAllDryRun(
 
 // executeRemoveAll deletes every local snapshot and, unless LocalOnly
 // is set, every corresponding remote metadata directory plus any
-// orphan remote keys. Blobs are not touched. It returns an error when
-// any snapshot could not be removed.
+// orphan remote keys. Blobs are not touched; the printed prune-command
+// hint is the next step.
 func (v *Vaultik) executeRemoveAll(
 	localSnaps, orphanRemoteKeys []string, opts *RemoveOptions,
 ) (*RemoveResult, error) {
@@ -1631,11 +1606,18 @@ func (v *Vaultik) executeRemoveAll(
 
 	result := &RemoveResult{}
 
-	failures := v.removeAllLocalSnapshots(localSnaps, opts, result)
+	remoteErrors := v.removeAllLocalSnapshots(localSnaps, opts, result)
 
 	if !opts.LocalOnly {
-		failures += v.removeAllOrphanRemotes(orphanRemoteKeys, result)
-		result.RemoteRemoved = failures == 0
+		remoteErrors += v.removeAllOrphanRemotes(orphanRemoteKeys, result)
+
+		if remoteErrors == 0 {
+			result.RemoteRemoved = true
+		} else if v.UI != nil {
+			v.UI.Warningf("Some remote metadata deletions failed. Run '%s' "+
+				"once the remote is healthy to clean up unreferenced blobs.",
+				pruneCommandHint)
+		}
 	}
 
 	if v.SnapshotManager != nil {
@@ -1645,10 +1627,6 @@ func (v *Vaultik) executeRemoveAll(
 		}
 	}
 
-	if failures > 0 {
-		return result, fmt.Errorf("%d %w", failures, errSnapshotRemoveFailed)
-	}
-
 	if opts.JSON {
 		return result, v.outputRemoveJSON(result)
 	}
@@ -1657,6 +1635,9 @@ func (v *Vaultik) executeRemoveAll(
 
 	if !opts.LocalOnly && result.RemoteRemoved {
 		v.printlnStdout("Removed snapshot metadata from remote storage")
+		v.stdoutf("\nNote: Removed snapshots' blobs remain on the remote. "+
+			"Run '%s' to delete any blobs no longer referenced by any "+
+			"remaining remote snapshot.\n", pruneCommandHint)
 	}
 
 	return result, nil
@@ -1699,13 +1680,12 @@ func (v *Vaultik) deleteIncompleteSnapshot(snapshotIDStr string, id any) bool {
 }
 
 // removeAllLocalSnapshots removes each known local snapshot (and, unless
-// --local-only, its remote metadata), returning the number of snapshots
-// it could not remove. A snapshot left in the local index keeps its
-// remote metadata too, since it is not an orphan remote key.
+// --local-only, its remote metadata), returning the number of remote
+// deletion failures.
 func (v *Vaultik) removeAllLocalSnapshots(
 	localSnaps []string, opts *RemoveOptions, result *RemoveResult,
 ) int {
-	failures := 0
+	remoteErrors := 0
 
 	for _, snapshotID := range localSnaps {
 		log.Info("Removing snapshot", "snapshot_id", snapshotID)
@@ -1715,8 +1695,6 @@ func (v *Vaultik) removeAllLocalSnapshots(
 			log.Error("Failed to remove from local database",
 				"snapshot_id", snapshotID, "error", err)
 
-			failures++
-
 			continue
 		}
 
@@ -1727,14 +1705,14 @@ func (v *Vaultik) removeAllLocalSnapshots(
 				log.Warn("Failed to remove snapshot metadata from remote",
 					"snapshot_id", snapshotID, "error", err)
 
-				failures++
+				remoteErrors++
 			}
 		}
 
 		result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID)
 	}
 
-	return failures
+	return remoteErrors
 }
 
 // removeAllOrphanRemotes deletes remote snapshot metadata that has no

internal/vaultik/snapshot_deleted_count_test.go

@@ -1,196 +0,0 @@
-package vaultik_test
-
-import (
-	"bytes"
-	"context"
-	"fmt"
-	"os"
-	"path/filepath"
-	"testing"
-	"time"
-
-	"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 inside a directory that an exclude added after a backup
-// matches is not counted as deleted, though the pattern matches only the
-// directory.
-func TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	env := newDeletedCountEnv(t, map[string][]byte{
-		"top.txt":                         []byte("top"),
-		"user/.local/share/Trash/old.txt": []byte("old"),
-	})
-	env.setSnapshot("home", "")
-
-	assert.Contains(t, env.backUp(t, "home"), filesLine(2, 2, 0))
-
-	env.setSnapshot("home", "", "*/.local/share/Trash")
-	assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
-}
-
-// A backup counts only files that the previous snapshot of its own name
-// had, not one that a later snapshot of another name over the same path
-// had and that is since removed.
-func TestBackupIgnoresSnapshotOfAnotherName(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	env := newDeletedCountEnv(t, map[string][]byte{
-		"kept.txt": []byte("kept"),
-	})
-	env.setSnapshot("home", "")
-	env.setSnapshot("other", "")
-
-	env.backUp(t, "home")
-
-	// Snapshot start times are kept in whole seconds: wait so that other's
-	// snapshot is strictly the most recent.
-	time.Sleep(time.Second)
-
-	extra := filepath.Join(env.src, "extra.txt")
-	require.NoError(t, os.WriteFile(extra, []byte("extra"), 0o600))
-	env.backUp(t, "other")
-	require.NoError(t, os.Remove(extra))
-
-	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))
-}

internal/vaultik/snapshot_list_test.go

@@ -483,14 +483,13 @@ func TestListSnapshots_UnreadableManifestDoesNotHideOthers(t *testing.T) {
 //
 //nolint:tagliatelle // snake_case is the established output format
 type listJSONRow struct {
-	ID               string `json:"id"`
-	RemoteKey        string `json:"remote_key"`
-	Timestamp        string `json:"timestamp"`
-	CompressedSize   int64  `json:"compressed_size"`
-	UncompressedSize int64  `json:"uncompressed_size"`
-	NewChunkSize     int64  `json:"new_chunk_size"`
-	LocallyTracked   bool   `json:"locally_tracked"`
-	RemotePresent    *bool  `json:"remote_present"`
+	ID             string `json:"id"`
+	RemoteKey      string `json:"remote_key"`
+	Timestamp      string `json:"timestamp"`
+	CompressedSize int64  `json:"compressed_size"`
+	NewChunkSize   int64  `json:"new_chunk_size"`
+	LocallyTracked bool   `json:"locally_tracked"`
+	RemotePresent  *bool  `json:"remote_present"`
 }
 
 // decodeListJSON parses the command's stdout, which must contain

internal/vaultik/uncompressed_size_test.go

@@ -1,85 +0,0 @@
-package vaultik_test
-
-import (
-	"bytes"
-	"context"
-	"path/filepath"
-	"testing"
-
-	"github.com/spf13/afero"
-	"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/storage"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// TestUncompressedSizeOfChangedFileStaysFixed covers
-// https://git.eeqj.de/sneak/vaultik/issues/360. A backup that changes a
-// file leaves the earlier snapshot's UNCOMPRESSED SIZE as it was when that
-// snapshot's backup completed. The file's row in the local index is shared
-// by both snapshots and rewritten by the second backup.
-func TestUncompressedSizeOfChangedFileStaysFixed(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-	changedPath := filepath.Join(dataDir, "changed.bin")
-
-	ctx := context.Background()
-	files := writeFaultSourceTree(t, fs, dataDir)
-	cfg := changedFileConfig(dataDir, dbPath)
-
-	appendRandomBytes(t, fs, files, changedPath, appendedFileSize)
-
-	store, err := storage.NewFileStorer(filepath.Join(tempDir, "remote"))
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = db.Close() })
-
-	repos := database.NewRepositories(db)
-	v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
-
-	require.NoError(t, backUp(v, "first"))
-
-	// With nothing to compare against, sizeOfNewChunks totals every
-	// distinct chunk of the files.
-	firstID := localSnapshotID(ctx, t, repos, "first")
-	firstSize := sizeOfNewChunks(t, files, nil)
-	require.Equal(t, firstSize, listedUncompressedSizes(t, v)[firstID],
-		"the first snapshot must count the chunks of every file it holds")
-
-	appendRandomBytes(t, fs, files, changedPath, faultChunkSize)
-	require.NoError(t, backUp(v, "second"))
-
-	secondID := localSnapshotID(ctx, t, repos, "second")
-	sizes := listedUncompressedSizes(t, v)
-	assert.Equal(t, firstSize, sizes[firstID],
-		"a later backup must not change the first snapshot's figure")
-	assert.Equal(t, sizeOfNewChunks(t, files, nil), sizes[secondID],
-		"the second snapshot must count the changed file's chunks")
-}
-
-// listedUncompressedSizes runs `snapshot list --json` and returns each
-// listed snapshot's uncompressed_size by snapshot ID.
-func listedUncompressedSizes(t *testing.T, v *vaultik.Vaultik) map[string]int64 {
-	t.Helper()
-
-	var out bytes.Buffer
-
-	v.Stdout = &out
-	require.NoError(t, v.ListSnapshots(true))
-
-	sizes := make(map[string]int64)
-	for _, row := range decodeListJSON(t, out.String()) {
-		sizes[row.ID] = row.UncompressedSize
-	}
-
-	return sizes
-}

internal/vaultik/verify.go

@@ -260,8 +260,6 @@ func (v *Vaultik) downloadVerifiedSnapshotDB(
 			fmt.Errorf("failed to download database: %w", err))
 	}
 
-	dbReader = newCloseOnCancelReader(v.ctx, dbReader)
-
 	defer func() { _ = dbReader.Close() }()
 
 	tdb, err := v.decryptAndLoadDatabase(dbReader, identities)