Exit non-zero when snapshot create --skip-errors skips anything it cannot read (closes #310)

A backup under --skip-errors printed its skip errors, then "Finished
successfully.", and exited 0, so a cron wrapper could not tell it from
a complete backup.

Each skipped entry, including a directory the walk could not list, is
now counted as examined and failed. If --skip-errors skipped anything
it could not read or list, the run still keeps its snapshots and
prunes as before, then fails with "Backup completed, but N file(s)
could not be read". A file removed during the run, even before the
walk reaches it, counts once as failed and leaves the exit status
alone; without the flag it no longer stops the backup. Excluded
entries are never counted.

Model: opus-5-5

ARCHITECTURE.md

@@ -359,7 +359,7 @@ CreateSnapshot(opts)
 
 ## Deduplication Strategy
 
-1. **File-level**: Files unchanged since last backup are skipped (metadata comparison: size, mtime, mode, uid, gid), unless the file lists a chunk that no uploaded blob holds; such a file is re-chunked
+1. **File-level**: Files and symlinks unchanged since last backup are skipped (metadata comparison: size, mtime, mode, uid, gid, and a symlink's target), unless the file lists a chunk that no uploaded blob holds; such a file is re-chunked
 
 2. **Chunk-level**: Chunks are content-addressed by SHA256 hash. If a chunk hash already exists in the database, the chunk data is not re-uploaded.
 
@@ -385,11 +385,12 @@ bucket/
 A destination without `vaultik-destination.txt` is treated as not present
 while the local index holds snapshots stored there (`checkDestinationMarker`
 in `internal/vaultik/destination_marker.go`): `snapshot create` refuses to
-write to it, `prune` and `snapshot purge` fail, and `snapshot list` and
-`snapshot remove` behave as for a destination that cannot be listed. This
-keeps those commands from taking an unplugged volume's empty mount point for
-a destination whose snapshots are all gone. The `file://` backend never
-creates its destination directory, so a write to a missing one fails.
+write to it, `prune`, `snapshot purge` and `remote nuke` fail, and
+`snapshot list` and `snapshot remove` behave as for a destination that cannot
+be listed. This keeps those commands from taking an unplugged volume's empty
+mount point for a destination whose snapshots are all gone. The `file://`
+backend never creates its destination directory, so a write to a missing one
+fails.
 
 The `{remote-key}` directory name is a one-way double SHA-256 hash of the human
 snapshot ID, so the human ID (hostname, snapshot name, timestamp) is never

README.md

@@ -454,9 +454,14 @@ since `prune` never deletes it.
 the backup destination store, whatever its name, and then
 `vaultik-destination.txt`, leaving the bucket prefix empty. Destructive
 and irreversible. This is the single supported way to wipe the entire
-destination store. If a listing afterwards still finds anything under
-`metadata/` or `blobs/`, the command names each object left and exits
-non-zero; running it again deletes what is left.
+destination store. A snapshot is removed from the local index only once
+its metadata is deleted from the destination store. If the destination
+store cannot be listed, or the local index holds snapshots and the
+destination has no `vaultik-destination.txt` (an unplugged volume's
+empty mount point), the command fails before it removes anything. If a
+listing afterwards still finds anything under `metadata/` or `blobs/`,
+the command names each object left and exits non-zero; running it again
+deletes what is left.
 * `--force`: Required to confirm destruction.
 
 **`database delete`**: Delete the local SQLite state database file
@@ -529,10 +534,11 @@ For an implementation-level view of the internals — the data model, the
 * `vaultik-destination.txt` holds one fixed line of text, with no host
   name, path or time. A destination without it is treated as not
   present while the local index holds snapshots stored there:
-  `snapshot create` refuses to write to it, `prune` and `snapshot purge`
-  fail, and `snapshot list` and `snapshot remove` behave as for a
-  destination that cannot be listed. This keeps an unplugged volume's empty mount point
-  from being taken for a destination whose snapshots are all gone. Mount
+  `snapshot create` refuses to write to it, `prune`, `snapshot purge`
+  and `remote nuke` fail, and `snapshot list` and `snapshot remove`
+  behave as for a destination that cannot be listed. This keeps an
+  unplugged volume's empty mount point from being taken for a
+  destination whose snapshots are all gone. Mount
   the volume if it is removable; if the destination was emptied on
   purpose, run `vaultik database delete` and back up again.
 * Blobs are two-level directory sharded using the first 4 hex chars of the blob hash

docs/REPOSTRUCTURE.md

@@ -27,7 +27,7 @@ The metadata subdirectory is named with the **remote key**, a one-way hash of th
 
 ## Marker File (`vaultik-destination.txt`)
 
-Every backup writes this file at the root before it uploads anything. It holds one fixed line of plain text and names no host, path or time. A destination without it is treated as not present while the local index holds snapshots stored there: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove` and `snapshot list` behave as for a destination that cannot be listed. An unplugged volume leaves an empty directory at its mount point, and without the marker that directory would read as a destination whose snapshots are all gone. `remote nuke` deletes the marker last.
+Every backup writes this file at the root before it uploads anything. It holds one fixed line of plain text and names no host, path or time. A destination without it is treated as not present while the local index holds snapshots stored there: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove`, `snapshot list` and `remote nuke` behave as for a destination that cannot be listed. An unplugged volume leaves an empty directory at its mount point, and without the marker that directory would read as a destination whose snapshots are all gone. `remote nuke` deletes the marker last.
 
 ## Blobs Directory (`blobs/`)
 

internal/snapshot/progress_test.go

@@ -78,8 +78,7 @@ func TestProgressPercentWithSecondPathSmaller(t *testing.T) {
 
 	stats := progress.GetStats()
 
-	// Directories count toward the total size but produce no chunks, so
-	// the percentage ends just under 100%.
+	// With both paths counted, the percentage must not exceed 100%.
 	percent := float64(stats.BytesProcessed.Load()) /
 		float64(stats.TotalSize.Load()) * 100
 	if percent > 100 {

internal/snapshot/scanner.go

@@ -291,6 +291,11 @@ func (s *Scanner) Scan(
 		}
 	}
 
+	err = s.recordDirectories(ctx, scanResult.Directories)
+	if err != nil {
+		return nil, err
+	}
+
 	// Summarize scan phase results and update progress
 	s.summarizeScanPhase(result, filesToProcess)
 
@@ -862,14 +867,15 @@ func (s *Scanner) batchInsertFileData(
 // ScanPhaseResult contains the results of the scan phase
 type ScanPhaseResult struct {
 	FilesToProcess   []*FileToProcess
-	UnchangedFileIDs []types.FileID // IDs of unchanged files to associate with snapshot
+	UnchangedFileIDs []types.FileID   // IDs of unchanged files to associate with snapshot
+	Directories      []*FileToProcess // Every directory found, to record
 }
 
 // scanPhase performs the initial directory scan to identify files to process
 // It uses the pre-loaded knownFiles map for fast change detection without DB
 // queries. It also populates existingFiles map for deletion detection.
-// Returns files needing processing and IDs of unchanged files for snapshot
-// association.
+// Returns files needing processing, IDs of unchanged files for snapshot
+// association, and the directories found.
 func (s *Scanner) scanPhase(
 	ctx context.Context,
 	path string,
@@ -877,8 +883,10 @@ func (s *Scanner) scanPhase(
 	existingFiles map[string]struct{},
 	knownFiles map[string]*database.File,
 ) (*ScanPhaseResult, error) {
-	// Use known file count as estimate for progress (accurate for subsequent backups)
-	estimatedTotal := int64(len(knownFiles))
+	// Use known file count as estimate for progress (accurate for
+	// subsequent backups). The walk does not count directories, so the
+	// estimate leaves them out.
+	estimatedTotal := countNonDirectories(knownFiles)
 
 	collector := &scanCollector{}
 
@@ -889,6 +897,18 @@ func (s *Scanner) scanPhase(
 	log.Debug("Starting directory walk", "path", path)
 
 	walkFn := func(filePath string, info os.FileInfo, err error) error {
+		// Check exclude patterns first, so an excluded entry that is gone
+		// or cannot be looked up is neither counted nor reported. For
+		// directories, skip the entire subtree. info is nil when the
+		// walk could not look the entry up.
+		if s.shouldExclude(filePath, path) {
+			if info != nil && info.IsDir() {
+				return filepath.SkipDir
+			}
+
+			return nil
+		}
+
 		// The source path is in no directory listing, so its absence is
 		// an error rather than a removal.
 		if errors.Is(err, os.ErrNotExist) && filePath != path {
@@ -908,18 +928,9 @@ func (s *Scanner) scanPhase(
 		default:
 		}
 
-		// Check exclude patterns - for directories, skip the entire subtree
-		if s.shouldExclude(filePath, path) {
-			if info.IsDir() {
-				return filepath.SkipDir
-			}
-
-			return nil
-		}
-
 		// Handle symlinks and directories
 		handled, err := s.recordSpecialEntry(
-			filePath, info, existingFiles, collector, result)
+			filePath, info, existingFiles, knownFiles, collector, result)
 		if handled {
 			return err
 		}
@@ -933,9 +944,10 @@ func (s *Scanner) scanPhase(
 		existingFiles[filePath] = struct{}{}
 
 		// Check file against in-memory map (no DB query!)
-		file, needsProcessing := s.checkFileInMemory(filePath, info, knownFiles)
+		file := s.buildRegularFileEntry(filePath, info)
+		needsProcessing := s.checkFileInMemory(file, knownFiles)
 
-		filesScanned, changedCount := collector.recordRegularFile(
+		filesScanned, changedCount := collector.recordFile(
 			filePath, info, file, needsProcessing)
 
 		// Update result stats
@@ -959,15 +971,32 @@ func (s *Scanner) scanPhase(
 	return &ScanPhaseResult{
 		FilesToProcess:   collector.filesToProcess,
 		UnchangedFileIDs: collector.unchangedFileIDs,
+		Directories:      collector.directories,
 	}, nil
 }
 
+// countNonDirectories returns how many of files are not directories.
+func countNonDirectories(files map[string]*database.File) int64 {
+	var count int64
+
+	for _, f := range files {
+		if f.Mode&uint32(os.ModeDir) == 0 {
+			count++
+		}
+	}
+
+	return count
+}
+
 // recordSpecialEntry records symlinks and directories (which have no
-// data to chunk) and reports whether it handled the entry. For a symlink
-// whose target cannot be read it returns handleWalkError's result.
+// data to chunk) and reports whether it handled the entry. A symlink is
+// counted and checked for changes as a regular file is; a directory is
+// set aside to be recorded and is not counted. For a symlink whose
+// target cannot be read it returns handleWalkError's result.
 func (s *Scanner) recordSpecialEntry(
 	filePath string, info os.FileInfo,
 	existingFiles map[string]struct{},
+	knownFiles map[string]*database.File,
 	collector *scanCollector, result *ScanResult,
 ) (bool, error) {
 	// Handle symlinks
@@ -978,8 +1007,9 @@ func (s *Scanner) recordSpecialEntry(
 		}
 
 		existingFiles[filePath] = struct{}{}
-		collector.addToProcess(filePath, info, file)
-		s.updateScanEntryStats(result, true, info)
+		needsProcessing := s.checkFileInMemory(file, knownFiles)
+		collector.recordFile(filePath, info, file, needsProcessing)
+		s.updateScanEntryStats(result, needsProcessing, info)
 
 		return true, nil
 	}
@@ -989,7 +1019,7 @@ func (s *Scanner) recordSpecialEntry(
 	if info.IsDir() {
 		file := s.buildDirectoryEntry(filePath, info)
 		existingFiles[filePath] = struct{}{}
-		collector.addToProcess(filePath, info, file)
+		collector.addDirectory(filePath, info, file)
 
 		return true, nil
 	}
@@ -1042,28 +1072,29 @@ type scanCollector struct {
 	mu               sync.Mutex
 	filesToProcess   []*FileToProcess
 	unchangedFileIDs []types.FileID // Just IDs - no new records needed
+	directories      []*FileToProcess
 	filesScanned     int64
 }
 
-// addToProcess queues a file entry for processing.
-func (c *scanCollector) addToProcess(
+// addDirectory sets a directory aside to be recorded. It is not counted
+// as a file.
+func (c *scanCollector) addDirectory(
 	path string, info os.FileInfo, file *database.File,
 ) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	c.filesToProcess = append(c.filesToProcess, &FileToProcess{
+	c.directories = append(c.directories, &FileToProcess{
 		Path:     path,
 		FileInfo: info,
 		File:     file,
 	})
-	c.filesScanned++
 }
 
-// recordRegularFile records a regular file as changed (to process) or
-// unchanged (snapshot association only) and returns the running scanned
-// and changed counts.
-func (c *scanCollector) recordRegularFile(
+// recordFile records a regular file or symlink as changed (to process)
+// or unchanged (snapshot association only) and returns the running
+// scanned and changed counts.
+func (c *scanCollector) recordFile(
 	path string, info os.FileInfo, file *database.File, needsProcessing bool,
 ) (int64, int) {
 	c.mu.Lock()
@@ -1166,7 +1197,28 @@ func (s *Scanner) printScanProgressLine(
 	}
 }
 
-// buildSymlinkEntry creates a File record for a symlink.
+// buildRegularFileEntry creates a File record for a regular file, with
+// no ID: checkFileInMemory sets it.
+func (s *Scanner) buildRegularFileEntry(path string, info os.FileInfo) *database.File {
+	var uid, gid uint32
+	if stat, ok := info.Sys().(*syscall.Stat_t); ok {
+		uid = stat.Uid
+		gid = stat.Gid
+	}
+
+	return &database.File{
+		Path:       types.FilePath(path),
+		SourcePath: types.SourcePath(s.currentSourcePath),
+		MTime:      info.ModTime(),
+		Size:       info.Size(),
+		Mode:       uint32(info.Mode()),
+		UID:        uid,
+		GID:        gid,
+	}
+}
+
+// buildSymlinkEntry creates a File record for a symlink, with no ID:
+// checkFileInMemory sets it.
 func (s *Scanner) buildSymlinkEntry(
 	path string, info os.FileInfo,
 ) (*database.File, error) {
@@ -1182,7 +1234,6 @@ func (s *Scanner) buildSymlinkEntry(
 	}
 
 	return &database.File{
-		ID:         types.NewFileID(),
 		Path:       types.FilePath(path),
 		SourcePath: types.SourcePath(s.currentSourcePath),
 		MTime:      info.ModTime(),
@@ -1214,6 +1265,22 @@ func (s *Scanner) buildDirectoryEntry(path string, info os.FileInfo) *database.F
 	}
 }
 
+// recordDirectories records every directory the walk found. A directory
+// has no data to back up, so it is not among the files counted and
+// backed up.
+func (s *Scanner) recordDirectories(
+	ctx context.Context, directories []*FileToProcess,
+) error {
+	for _, dir := range directories {
+		err := s.recordNonRegularFile(ctx, dir)
+		if err != nil {
+			return fmt.Errorf("recording directory %s: %w", dir.Path, err)
+		}
+	}
+
+	return nil
+}
+
 // recordNonRegularFile writes a symlink or directory entry to the database
 // and associates it with the current snapshot. No chunking is performed.
 func (s *Scanner) recordNonRegularFile(ctx context.Context, ftp *FileToProcess) error {
@@ -1227,62 +1294,36 @@ func (s *Scanner) recordNonRegularFile(ctx context.Context, ftp *FileToProcess)
 	})
 }
 
-// checkFileInMemory checks if a file needs processing using the in-memory map
+// checkFileInMemory reports whether a regular file or symlink needs
+// processing, and gives file the ID of its known record, or a new ID for
+// a new path.
 // No database access is performed - this is purely CPU/memory work
 func (s *Scanner) checkFileInMemory(
-	path string, info os.FileInfo, knownFiles map[string]*database.File,
-) (*database.File, bool) {
-	var uid, gid uint32
-	if stat, ok := info.Sys().(*syscall.Stat_t); ok {
-		uid = stat.Uid
-		gid = stat.Gid
-	}
-
-	// Check against in-memory map first to get existing ID if available
-	existingFile, exists := knownFiles[path]
-
-	// Create file record with ID set upfront
-	// For new files, generate UUID immediately so it's available for chunk associations
-	// For existing files, reuse the existing ID
-	var fileID types.FileID
-	if exists {
-		fileID = existingFile.ID
-	} else {
-		fileID = types.NewFileID()
-	}
-
-	file := &database.File{
-		ID:         fileID,
-		Path:       types.FilePath(path),
-		SourcePath: types.SourcePath(s.currentSourcePath),
-		MTime:      info.ModTime(),
-		Size:       info.Size(),
-		Mode:       uint32(info.Mode()),
-		UID:        uid,
-		GID:        gid,
-	}
+	file *database.File, knownFiles map[string]*database.File,
+) bool {
+	existingFile, exists := knownFiles[file.Path.String()]
 
 	// New file - needs processing
 	if !exists {
-		return file, true
+		file.ID = types.NewFileID()
+
+		return true
 	}
 
+	file.ID = existingFile.ID
+
 	// No uploaded blob holds one of its chunks (see loadFilesToRechunk)
-	if _, rechunk := s.filesToRechunk[fileID]; rechunk {
-		return file, true
+	if _, rechunk := s.filesToRechunk[file.ID]; rechunk {
+		return true
 	}
 
 	// Check if file has changed
-	if existingFile.Size != file.Size ||
+	return existingFile.Size != file.Size ||
 		!existingFile.MTime.Equal(file.MTime) ||
 		existingFile.Mode != file.Mode ||
 		existingFile.UID != file.UID ||
-		existingFile.GID != file.GID {
-		return file, true
-	}
-
-	// File unchanged
-	return file, false
+		existingFile.GID != file.GID ||
+		existingFile.LinkTarget != file.LinkTarget
 }
 
 // batchAddFilesToSnapshot adds existing file IDs to the snapshot association table
@@ -1409,17 +1450,16 @@ func (s *Scanner) processPhase(
 	return s.finalizeProcessPhase(ctx, result)
 }
 
-// processFileWithErrorHandling records a directory or symlink, or wraps
+// processFileWithErrorHandling records a symlink, or wraps
 // processFileStreaming for a regular file with error recovery for deleted
 // files and skip-errors mode. Returns (skipped, error).
 func (s *Scanner) processFileWithErrorHandling(
 	ctx context.Context, fileToProcess *FileToProcess, result *ScanResult,
 ) (bool, error) {
-	// A directory or symlink has no data to open or read; it is only
-	// recorded in the local index. An error recording it stops the run
-	// even under --skip-errors, or the snapshot would complete without it.
-	mode := os.FileMode(fileToProcess.File.Mode)
-	if mode&os.ModeSymlink != 0 || mode.IsDir() {
+	// A symlink has no data to open or read; it is only recorded in the
+	// local index. An error recording it stops the run even under
+	// --skip-errors, or the snapshot would complete without it.
+	if os.FileMode(fileToProcess.File.Mode)&os.ModeSymlink != 0 {
 		err := s.recordNonRegularFile(ctx, fileToProcess)
 		if err != nil {
 			return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err)

internal/snapshot/scanner_estimate_test.go

@@ -0,0 +1,28 @@
+//nolint:testpackage // exercises the unexported countNonDirectories helper
+package snapshot
+
+import (
+	"os"
+	"testing"
+
+	"sneak.berlin/go/vaultik/internal/database"
+)
+
+// The walk counts regular files and symlinks, not directories, so the
+// estimate of its count made from the local index's records leaves the
+// directories out. See https://git.eeqj.de/sneak/vaultik/issues/305.
+func TestCountNonDirectories(t *testing.T) {
+	t.Parallel()
+
+	known := map[string]*database.File{
+		"/src":       {Mode: uint32(os.ModeDir | 0o755)},
+		"/src/sub":   {Mode: uint32(os.ModeDir | 0o755)},
+		"/src/a.txt": {Mode: 0o644},
+		"/src/link":  {Mode: uint32(os.ModeSymlink | 0o777), LinkTarget: "a.txt"},
+	}
+
+	got := countNonDirectories(known)
+	if got != 2 {
+		t.Errorf("countNonDirectories = %d, want 2", got)
+	}
+}

internal/vaultik/destination_marker_test.go

@@ -152,6 +152,25 @@ func TestPurgeRefusesDestinationWithoutMarker(t *testing.T) {
 	assertUntouched(ctx, t, repos, storeDir)
 }
 
+// TestNukeRemoteRefusesDestinationWithoutMarker checks that remote nuke
+// fails, deletes no local snapshot record, and neither reports the
+// destination empty nor advises running nuke again when the destination
+// has no marker.
+func TestNukeRemoteRefusesDestinationWithoutMarker(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	v, repos, out, storeDir := backUpThenUnplugMountPoint(ctx, t)
+
+	err := v.NukeRemote(true)
+	require.ErrorContains(t, err, markerFile)
+	assert.NotContains(t, out.String(), "now empty")
+	assert.NotContains(t, out.String(), "vaultik remote nuke --force")
+
+	assertUntouched(ctx, t, repos, storeDir)
+}
+
 // TestRemoveSnapshotWarnsWhenDestinationHasNoMarker checks that snapshot
 // remove warns that the metadata could not be removed from a destination
 // without a marker, instead of reporting that it was.

internal/vaultik/missing_destination_test.go

@@ -188,3 +188,41 @@ func TestPurgeSaysListingFailedOnceWhenDestinationMissing(t *testing.T) {
 	assert.Equal(t, 1, strings.Count(err.Error(), "listing remote snapshots"),
 		err.Error())
 }
+
+// TestNukeRemoteKeepsLocalIndexWhenDestinationMissing checks that remote
+// nuke fails on a destination it cannot list, leaves the local index
+// unchanged, and neither reports the destination empty nor advises
+// running nuke again.
+func TestNukeRemoteKeepsLocalIndexWhenDestinationMissing(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	v, repos, out := backUpThenUnplug(ctx, t)
+
+	filesBefore, err := repos.Files.ListAll(ctx)
+	require.NoError(t, err)
+	require.NotEmpty(t, filesBefore)
+
+	blobsBefore, err := repos.Blobs.GetAll(ctx)
+	require.NoError(t, err)
+	require.NotEmpty(t, blobsBefore)
+
+	err = v.NukeRemote(true)
+	require.ErrorIs(t, err, fs.ErrNotExist)
+	require.ErrorContains(t, err, "listing remote snapshots")
+	assert.NotContains(t, out.String(), "now empty")
+	assert.NotContains(t, out.String(), "vaultik remote nuke --force")
+
+	snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
+	require.NoError(t, err)
+	assert.Len(t, snapshots, 1, "nuke must delete no local snapshot record")
+
+	files, err := repos.Files.ListAll(ctx)
+	require.NoError(t, err)
+	assert.Len(t, files, len(filesBefore), "nuke must delete no local file record")
+
+	blobs, err := repos.Blobs.GetAll(ctx)
+	require.NoError(t, err)
+	assert.Len(t, blobs, len(blobsBefore), "nuke must delete no local blob record")
+}

internal/vaultik/nuke_remote_test.go

@@ -104,6 +104,32 @@ func TestNukeRemoteFailsWhenADeleteFails(t *testing.T) {
 	}
 }
 
+// TestNukeRemoteKeepsLocalSnapshotWhoseMetadataIsNotDeleted checks that
+// remote nuke keeps a snapshot in the local index while its metadata
+// cannot be deleted from the destination store: dropped from the local
+// index, it would be left there as a remote-only snapshot without its
+// name.
+func TestNukeRemoteKeepsLocalSnapshotWhoseMetadataIsNotDeleted(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	storeDir := filepath.Join(t.TempDir(), "store")
+	v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
+
+	store := faultstore.New(v.Storage)
+	store.OnDelete = func(key string) bool {
+		return strings.HasPrefix(key, "metadata/")
+	}
+	v.Storage = store
+
+	require.Error(t, v.NukeRemote(true))
+
+	snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
+	require.NoError(t, err)
+	assert.Len(t, snapshots, 1)
+}
+
 // TestNukeRemoteNamesWhatIsLeft checks that remote nuke, when an object
 // it does not recognise cannot be deleted, does not report the store
 // empty but names that object and fails.
@@ -154,8 +180,7 @@ func TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice(t *testing.T) {
 
 // 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.
+// run, and that nuke then does not report the destination store empty.
 func TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()

internal/vaultik/prune.go

@@ -55,12 +55,19 @@ func (v *Vaultik) NukeRemote(force bool) error {
 	}
 
 	// Checked before anything is deleted: running nuke again would fail
-	// the same way, so the advice below must not follow this error.
+	// the same way, so the advice below must not follow these errors.
+	// The listing also fails on a destination without the marker while
+	// the local index holds snapshots.
 	err := v.EnsureStorageBinding()
 	if err != nil {
 		return err
 	}
 
+	_, err = v.listAllRemoteSnapshotKeys()
+	if err != nil {
+		return err
+	}
+
 	err = v.emptyDestinationStore()
 	if err != nil {
 		v.UI.Warningf("Run '%s' again to delete what is left on the "+

internal/vaultik/snapshot.go

@@ -1545,7 +1545,9 @@ func (v *Vaultik) removeSnapshotRemote(
 // database from the local index, and (with --remote) every snapshot
 // metadata directory in remote storage. Both sides are processed so a
 // "remove --all" leaves nothing behind, even when the local DB and
-// remote storage have diverged.
+// remote storage have diverged. Unless LocalOnly is set, it fails
+// before it removes anything when the destination store cannot be
+// listed or is not present by checkDestinationMarker.
 func (v *Vaultik) RemoveAllSnapshots(opts *RemoveOptions) (*RemoveResult, error) {
 	err := v.EnsureStorageBinding()
 	if err != nil {
@@ -1558,11 +1560,15 @@ func (v *Vaultik) RemoveAllSnapshots(opts *RemoveOptions) (*RemoveResult, error)
 	}
 
 	// remoteKeys is the set of metadata/<key>/ subdirectories on the
-	// destination store; failures are downgraded to a warning so a
-	// permission-denied or unreachable remote can't block a local-only
-	// remove.
+	// destination store. Only for a local-only remove is a failure
+	// downgraded to a warning, so a permission-denied or unreachable
+	// remote can't block it.
 	remoteKeys, remoteErr := v.listAllRemoteSnapshotKeys()
 	if remoteErr != nil {
+		if !opts.LocalOnly {
+			return nil, remoteErr
+		}
+
 		log.Warn("Could not list remote snapshots", "error", remoteErr)
 		v.UI.Warningf("Could not list remote snapshots: %v.", remoteErr)
 	}
@@ -1624,8 +1630,7 @@ func (v *Vaultik) localSnapshotIDs() ([]string, error) {
 // listAllRemoteSnapshotKeys collects the hashed remote keys
 // (subdirectories under metadata/) currently present in the
 // destination store. Returns (nil, err) when the store cannot be
-// listed, or is not present by checkDestinationMarker; callers must
-// treat that as "no remote info available," not fatal.
+// listed, or is not present by checkDestinationMarker.
 func (v *Vaultik) listAllRemoteSnapshotKeys() ([]string, error) {
 	log.Info("Listing all remote snapshots")
 
@@ -1809,10 +1814,12 @@ func (v *Vaultik) deleteIncompleteSnapshot(snapshotIDStr string, id any) bool {
 	return true
 }
 
-// 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.
+// removeAllLocalSnapshots removes each known local snapshot's remote
+// metadata, unless --local-only, and then its local record, returning
+// the number of snapshots it could not remove. The local record goes
+// only once the metadata is deleted: dropped first, it would leave that
+// metadata on the destination store as a remote-only snapshot whose name
+// is lost.
 func (v *Vaultik) removeAllLocalSnapshots(
 	localSnaps []string, opts *RemoveOptions, result *RemoveResult,
 ) int {
@@ -1821,6 +1828,19 @@ func (v *Vaultik) removeAllLocalSnapshots(
 	for _, snapshotID := range localSnaps {
 		log.Info("Removing snapshot", "snapshot_id", snapshotID)
 
+		if !opts.LocalOnly {
+			_, err := v.deleteRemoteSnapshotByKey(
+				snapshot.RemoteSnapshotKey(snapshotID))
+			if err != nil {
+				log.Warn("Failed to remove snapshot metadata from remote",
+					"snapshot_id", snapshotID, "error", err)
+
+				failures++
+
+				continue
+			}
+		}
+
 		err := v.deleteSnapshotFromLocalDB(snapshotID)
 		if err != nil {
 			log.Error("Failed to remove from local database",
@@ -1831,17 +1851,6 @@ func (v *Vaultik) removeAllLocalSnapshots(
 			continue
 		}
 
-		if !opts.LocalOnly {
-			_, err = v.deleteRemoteSnapshotByKey(
-				snapshot.RemoteSnapshotKey(snapshotID))
-			if err != nil {
-				log.Warn("Failed to remove snapshot metadata from remote",
-					"snapshot_id", snapshotID, "error", err)
-
-				failures++
-			}
-		}
-
 		result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID)
 	}
 

internal/vaultik/snapshot_skip_errors_test.go

@@ -191,3 +191,27 @@ func TestBackupCountsFileRemovedBeforeWalkReachesItAsFailed(t *testing.T) {
 		})
 	}
 }
+
+// A file an exclude pattern matches is not counted when it is removed
+// before the walk reaches it. See
+// https://git.eeqj.de/sneak/vaultik/issues/310.
+func TestBackupIgnoresExcludedFileRemovedBeforeWalkReachesIt(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	env := newDeletedCountEnv(t, map[string][]byte{
+		"stays.txt":   []byte("stays"),
+		"removed.tmp": []byte("removed"),
+	})
+	env.setSnapshot("walk", "", "*.tmp")
+	env.v.Fs = &removeOnLookupFs{
+		path: filepath.Join(env.src, "removed.tmp"),
+	}
+
+	require.NoError(t, env.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
+		SkipErrors: true,
+		Snapshots:  []string{"walk"},
+	}))
+	assert.Contains(t, env.out.String(), filesLine(1, 1, 0))
+	assert.Contains(t, env.out.String(), "Finished successfully.")
+}

internal/vaultik/snapshot_summary_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 	"sneak.berlin/go/vaultik/internal/config"
 	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
@@ -188,6 +189,42 @@ func (e *summaryEnv) dataLine(total, backedUp int64) string {
 		e.v.UI.Size(total), e.v.UI.Size(backedUp))
 }
 
+// enumeratedLine returns the line printed when the walk of a source path
+// ends.
+func (e *summaryEnv) enumeratedLine(
+	files int, total int64, toBackUp int, toBackUpSize int64,
+) string {
+	return fmt.Sprintf(
+		"Enumerated %d snapshot source files (%s total), %d to back up (%s).",
+		files, e.v.UI.Size(total), toBackUp, e.v.UI.Size(toBackUpSize))
+}
+
+// snapshotPaths returns the paths the snapshot lists, sorted.
+func (e *summaryEnv) snapshotPaths(t *testing.T, snapshotID string) []string {
+	t.Helper()
+
+	rows, err := e.db.Conn().QueryContext(context.Background(), `
+		SELECT f.path FROM snapshot_files sf JOIN files f ON f.id = sf.file_id
+		WHERE sf.snapshot_id = ? ORDER BY f.path`, snapshotID)
+	require.NoError(t, err)
+
+	defer func() { _ = rows.Close() }()
+
+	var paths []string
+
+	for rows.Next() {
+		var path string
+
+		require.NoError(t, rows.Scan(&path))
+
+		paths = append(paths, path)
+	}
+
+	require.NoError(t, rows.Err())
+
+	return paths
+}
+
 // A first backup stores copy.bin's chunks while backing up a.bin, so
 // copy.bin's chunks are deduplicated within the run. Each file and byte
 // is still counted once.
@@ -248,6 +285,85 @@ func TestSnapshotSummaryIncrementalRunWithDeduplicatedChunks(t *testing.T) {
 	assert.Equal(t, uncompressed, snap.BlobUncompressedSize)
 }
 
+// A backup of a tree with directories and a symlink, none of which
+// changed since the last backup, backs up nothing, and the snapshot
+// still lists every one of them. Both sides of the line that ends the
+// walk count the same entries, the files and the symlink, and no
+// directory's size. See https://git.eeqj.de/sneak/vaultik/issues/305.
+func TestSnapshotSummaryUnchangedRunWithDirectoriesAndSymlink(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	env := newSummaryEnv(t)
+
+	const linkTarget = "a.bin"
+
+	require.NoError(t, os.Symlink(linkTarget,
+		filepath.Join(filepath.Dir(env.aPath), "link")))
+
+	// A symlink's size is the length of its target.
+	total := env.totalSize + int64(len(linkTarget))
+
+	first := env.backUp(t, "first", false)
+	assert.Contains(t, first, env.enumeratedLine(4, total, 4, total))
+	assert.Contains(t, first, filesLine(4, 4, 0))
+
+	second := env.backUp(t, "second", false)
+	assert.Contains(t, second, env.enumeratedLine(4, total, 0, 0))
+	assert.Contains(t, second, filesLine(4, 0, 4))
+	assert.Contains(t, second, env.dataLine(total, 0))
+
+	assert.Equal(t,
+		env.snapshotPaths(t, env.snapshot(t, "first").ID.String()),
+		env.snapshotPaths(t, env.snapshot(t, "second").ID.String()))
+}
+
+// A directory whose mtime changed and a symlink whose target changed are
+// recorded again by the next backup, which counts only the symlink as
+// backed up.
+func TestSnapshotSummaryChangedDirectoryAndSymlink(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	env := newSummaryEnv(t)
+
+	// The local index keeps paths with symlinks resolved.
+	dir, err := filepath.EvalSymlinks(filepath.Dir(env.aPath))
+	require.NoError(t, err)
+
+	// The new link gets the old one's mtime, and a target, which need not
+	// exist, of the old target's length, a symlink's size: only the
+	// target differs.
+	linkTime := unix.NsecToTimeval(
+		time.Date(2029, time.March, 4, 5, 6, 7, 0, time.UTC).UnixNano())
+	link := filepath.Join(dir, "link")
+	require.NoError(t, os.Symlink("a.bin", link))
+	require.NoError(t, unix.Lutimes(link, []unix.Timeval{linkTime, linkTime}))
+
+	env.backUp(t, "first", false)
+
+	require.NoError(t, os.Remove(link))
+	require.NoError(t, os.Symlink("b.bin", link))
+	require.NoError(t, unix.Lutimes(link, []unix.Timeval{linkTime, linkTime}))
+
+	mtime := time.Date(2030, time.January, 2, 3, 4, 5, 0, time.UTC)
+	require.NoError(t, os.Chtimes(dir, mtime, mtime))
+
+	assert.Contains(t, env.backUp(t, "second", false), filesLine(4, 1, 3))
+
+	ctx := context.Background()
+
+	dirRow, err := env.repos.Files.GetByPath(ctx, dir)
+	require.NoError(t, err)
+	require.NotNil(t, dirRow)
+	assert.Equal(t, mtime.Unix(), dirRow.MTime.Unix())
+
+	linkRow, err := env.repos.Files.GetByPath(ctx, link)
+	require.NoError(t, err)
+	require.NotNil(t, linkRow)
+	assert.Equal(t, "b.bin", linkRow.LinkTarget.String())
+}
+
 // Under --cron the progress reporter is off; the upload figures must
 // still reach the summary and the snapshots row. The snapshot has two
 // paths, each backed up by its own scan.