Refuse a backup destination an unplugged volume left empty (closes #239)

Every backup now writes vaultik-destination.txt at the destination
root. While the local index holds snapshots, a destination without it
is not present: snapshot create refuses before writing, prune and
snapshot purge fail, and snapshot list and snapshot remove warn.
remote nuke deletes the marker last. The file:// backend no longer
creates its destination directory and checks for it on every write;
that protects a directory on a removable volume, not the mount point
itself.

Tests back up into existing directories; fixtures standing for a
written destination carry the marker.

Judgement call: the marker is rewritten on every backup.
Deviation: for the complexity limit, snapshot name selection left CreateSnapshot and the --force/--json test moved into confirmRemoveSnapshot, both unchanged.

Model: opus-5-5

README.md

@@ -302,8 +302,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
+were not stored before, recorded when its backup ran).
-snapshot's backup runs.
 
 Listing the destination store does **not** require the age secret key,
 so it works in vaultik's intended configuration, where the backed-up
@@ -389,9 +388,7 @@ 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
+* `--verify`: After restoring, verify every file's chunk hashes match
-  match. A file skipped under `--skip-errors` is reported as failed to
-  restore and is not verified.
 
 **`prune`**: Tidy up everything that isn't needed. Runs three passes:
 (1) reconcile the local index against the destination store — any

docs/DATAMODEL.md

@@ -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

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
+// UpdateExtendedStats updates extended statistics for a snapshot
-// 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.
 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 = ?,
+		    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
-		    )
 		WHERE id = ?
 	`
 
@@ -569,22 +555,30 @@ func (r *SnapshotRepository) GetSnapshotBlobSizes(
 	return compressed, uncompressed, nil
 }
 
-// GetSnapshotUncompressedChunkSize returns the total plaintext size of the
+// GetSnapshotUncompressedChunkSize returns the sum of plaintext sizes of all unique
-// distinct chunks the snapshot's files reference, as the snapshot's backup
+// chunks referenced by a snapshot (via snapshot_files → file_chunks → chunks).
-// recorded it (see UpdateExtendedStats).
 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

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
+	FilesFailed   int
+	FailedFiles   []string // Paths of files that failed verification
 }
 
 // Restore restores files from a snapshot to the target directory
@@ -659,34 +658,20 @@ func (v *Vaultik) handleRestoreVerification(
 	opts *RestoreOptions,
 	result *RestoreResult,
 ) error {
-	// A file that failed to restore was not written and is already
+	err := v.verifyRestoredFiles(v.ctx, repos, files, opts.TargetDir, result)
-	// 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)
 	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).",
@@ -1448,8 +1433,8 @@ func (v *Vaultik) verifyRestoredFiles(
 		if err != nil {
 			log.Error("File verification failed", "path", file.Path, "error", err)
 
-			result.VerifyFailedFiles = append(
+			result.FilesFailed++
-				result.VerifyFailedFiles, file.Path.String())
+			result.FailedFiles = append(result.FailedFiles, file.Path.String())
 		}
 
 		bytesProcessed += file.Size
@@ -1465,7 +1450,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_skip_errors_test.go

@@ -4,9 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/spf13/afero"
@@ -106,130 +104,6 @@ func assertRestoreSkipsLostFiles(
 	}
 }
 
-// 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) {

internal/vaultik/snapshot_list_test.go

@@ -490,7 +490,6 @@ type listJSONRow struct {
 	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"`

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(t.TempDir())
-	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
-}