Count a file as deleted only if the previous snapshot of its name had it (closes #304)

A backup counted as deleted every file in the local index under the
source path that the walk did not find. That took in files the
snapshot excludes but another snapshot backs up, and files removed long
ago, which every later backup, and the first backup of another name
over the same path, reported as deleted again.

The backup now counts only the files that the most recent completed
snapshot of the same name lists, the walk did not find, and are gone
from disk. A file that is excluded, or inside an excluded directory, is
not counted, nor is one the walk could not reach under --skip-errors.

The snapshots row stores no deleted figure; only the backup's output
changes.

Model: opus-5-5

README.md

@@ -360,20 +360,16 @@ snapshot from the local index and strips the snapshot's metadata from
 the backup destination store. Blobs are NOT touched — deleting blobs
 requires reading every remaining remote manifest (the destination store
 may hold snapshots this host doesn't know about), which is what
-`vaultik prune` does. The command prints the `vaultik prune` invocation
+`vaultik prune` does. On success the command prints the exact `vaultik
-to run as a follow-up only when it removed the snapshot's metadata from
+prune` invocation to run as a follow-up. An ID that names no snapshot in
-the destination store, and never under `--json`. An ID that names no
+the local index or on the destination store is an error; with
-snapshot in the local index or on the destination store is an error;
+`--local-only`, the snapshot must be in the local index. Local row
-with `--local-only`, the snapshot must be in the local index. Local row
 cleanup (files, chunks, blobs the snapshot was the last referrer for)
-runs automatically. If removing the snapshot's metadata from the
+runs automatically. If the destination store is unreachable, the local-DB
-destination store fails (for example because the store is unreachable),
+removal still completes and a warning is emitted; run `vaultik snapshot
-the local-DB removal still completes and a warning is emitted; run
+remove <snapshot-id>` again once the store is reachable to remove the
-`vaultik snapshot remove <snapshot-id>` again once the store is reachable
+snapshot's metadata from it (`vaultik prune` does not). To wipe everything
-to remove the snapshot's metadata from it (`vaultik prune` does not). For
+on the destination in one go, use `vaultik remote nuke --force`.
-a snapshot that is only on the destination store, that failure is an
-error. To wipe everything on the destination in one go, use `vaultik
-remote nuke --force`.
 * `--local-only`: Skip remote cleanup; only touch the local index
 * `--dry-run`: Show what would be deleted without deleting
 * `--force`: Skip confirmation prompt

internal/cli/config.go

@@ -333,15 +333,8 @@ func newConfigEditCommand() *cobra.Command {
 				editor = "vi"
 			}
 
-			// Run $EDITOR through the shell, as git does, so a value
-			// with arguments such as "code --wait" works. The path
-			// goes in as an argument ("$@"), not into the command
-			// text, so a path with spaces or quotes arrives whole.
-			// sh -c takes the word after the command as $0, which
-			// is why editor appears twice.
 			//nolint:gosec // G204: launching the operator's own $EDITOR is the point
-			ed := exec.CommandContext(cmd.Context(),
+			ed := exec.CommandContext(cmd.Context(), editor, path)
-				"sh", "-c", editor+` "$@"`, editor, path)
 			ed.Stdin = os.Stdin
 			ed.Stdout = os.Stdout
 			ed.Stderr = os.Stderr

internal/cli/config_test.go

@@ -647,30 +647,6 @@ func TestWriteConfigSetTightensMode(t *testing.T) {
 	}
 }
 
-// TestConfigEditEditorWithArgument checks that an $EDITOR carrying an
-// argument works and that a config path with a space reaches it whole.
-// The "editor" is cp with the replacement file as its argument, so the
-// config ends up with the replacement's content only if both arrived.
-func TestConfigEditEditorWithArgument(t *testing.T) {
-	dir := t.TempDir()
-	path := seedFile(t, dir, "my config.yml", "compression_level: 3\n")
-	edited := seedFile(t, dir, "edited.yml", "compression_level: 9\n")
-
-	setRootFlags(t, RootFlags{ConfigPath: path})
-	t.Setenv("EDITOR", "cp '"+edited+"'")
-
-	mustExecute(t, newConfigEditCommand())
-
-	data, err := os.ReadFile(path) //nolint:gosec // G304: test-controlled path
-	if err != nil {
-		t.Fatalf("reading config back: %v", err)
-	}
-
-	if string(data) != "compression_level: 9\n" {
-		t.Errorf("config after edit = %q, want the edited content", data)
-	}
-}
-
 func splitPath(s string) []string {
 	return strings.Split(s, ".")
 }

internal/cli/snapshot.go

@@ -270,9 +270,8 @@ By default, this removes the snapshot from the local index database and
 strips the snapshot's metadata from the backup destination store. Blobs
 are NOT touched: deleting them requires reading every remaining remote
 manifest (the destination store may hold snapshots this host doesn't
-know about), which is what 'vaultik prune' does. The command prints the
+know about), which is what 'vaultik prune' does. On success the command
-'vaultik prune' invocation to run as a follow-up only when it removed the
+prints the exact 'vaultik prune' invocation to run as a follow-up.
-snapshot's metadata from the destination store, and never under --json.
 
 An ID that names no snapshot in the local index or on the destination
 store is an error.
@@ -281,12 +280,10 @@ Use --local-only to skip the remote half (e.g. when you want to forget a
 snapshot locally without touching the destination store); the snapshot
 must then be in the local index.
 
-If removing the snapshot's metadata from the destination store fails (for
+If the remote is unreachable, the local-database removal still completes
-example because the remote is unreachable), the local-database removal
+and a warning is emitted; run 'vaultik snapshot remove <snapshot-id>' again
-still completes and a warning is emitted; run 'vaultik snapshot remove
+once the destination store is reachable to remove the snapshot's metadata
-<snapshot-id>' again once the destination store is reachable to remove
+from it ('vaultik prune' does not).
-the snapshot's metadata from it ('vaultik prune' does not). For a
-snapshot that is only on the destination store, that failure is an error.
 
 To wipe the entire destination store and start over, use 'vaultik remote
 nuke --force' — it is the single supported entry point for that.`,

internal/snapshot/scanner.go

@@ -2034,9 +2034,7 @@ func (s *Scanner) excludedOrInExcludedDir(filePath, rootPath string) bool {
 
 // goneFromDisk reports whether looking filePath up on disk finds that
 // it does not exist. Like the walk, it does not follow a symlink, so a
-// symlink whose target is gone is still on disk. A file whose directory
+// symlink whose target is gone is still on disk.
-// was replaced by a regular file is gone too, though the lookup reports
-// "not a directory".
 func (s *Scanner) goneFromDisk(filePath string) bool {
 	var err error
 	if lstater, ok := s.fs.(afero.Lstater); ok {
@@ -2045,7 +2043,7 @@ func (s *Scanner) goneFromDisk(filePath string) bool {
 		_, err = s.fs.Stat(filePath)
 	}
 
-	return errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR)
+	return errors.Is(err, os.ErrNotExist)
 }
 
 // wrapPermissionError augments permission errors with platform-specific

internal/vaultik/prune_count_test.go

@@ -1,17 +1,13 @@
 package vaultik //nolint:testpackage // exercises unexported count helpers
 
 import (
-	"bytes"
 	"context"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/snapshot"
-	"sneak.berlin/go/vaultik/internal/ui"
 )
 
 // TestTableCountForReportSurfacesReadFailure is the regression guard for
@@ -80,59 +76,3 @@ func TestCountDiffUnknownWhenEitherSideUnknown(t *testing.T) {
 	assert.Nil(t, countDiff(&before, nil), "unknown after yields unknown delta")
 	assert.Nil(t, countDiff(nil, nil))
 }
-
-// TestPruneDatabaseSummaryLabelsTotals removes the only incomplete
-// snapshot from an index that also holds a complete one. The figure in
-// brackets is every snapshot left in the local index, so the line must
-// say so rather than read as incomplete snapshots that remain.
-func TestPruneDatabaseSummaryLabelsTotals(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-
-	db, err := database.New(ctx, ":memory:")
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = db.Close() })
-
-	repos := database.NewRepositories(db)
-
-	startedAt := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC)
-	completedAt := startedAt.Add(time.Minute)
-
-	require.NoError(t, repos.Snapshots.Create(ctx, nil, &database.Snapshot{
-		ID:          "host_complete_2026-01-02T03:04:05Z",
-		Hostname:    "host",
-		StartedAt:   startedAt,
-		CompletedAt: &completedAt,
-	}))
-	require.NoError(t, repos.Snapshots.Create(ctx, nil, &database.Snapshot{
-		ID:        "host_incomplete_2026-01-02T03:04:05Z",
-		Hostname:  "host",
-		StartedAt: startedAt,
-	}))
-
-	var out bytes.Buffer
-
-	v := &Vaultik{
-		DB:           db,
-		Repositories: repos,
-		SnapshotManager: snapshot.NewSnapshotManager(
-			snapshot.SnapshotManagerParams{Repos: repos}),
-		UI: ui.NewWithColor(&out, false),
-	}
-	v.SetContext(ctx)
-
-	result, err := v.PruneDatabase()
-	require.NoError(t, err)
-	require.Equal(t, int64(1), result.SnapshotsDeleted)
-
-	assert.Contains(t, out.String(), "Incomplete snapshots: 1 removed "+
-		"(total snapshots left in the local index: 1).")
-	assert.Contains(t, out.String(), "Orphaned files: 0 removed "+
-		"(total files left in the local index: 0).")
-	assert.Contains(t, out.String(), "Orphaned chunks: 0 removed "+
-		"(total chunks left in the local index: 0).")
-	assert.Contains(t, out.String(), "Orphaned blobs: 0 removed "+
-		"(total blobs left in the local index: 0).")
-}

internal/vaultik/remove_snapshot_test.go

@@ -17,7 +17,6 @@ import (
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/snapshot"
 	"sneak.berlin/go/vaultik/internal/storage"
-	"sneak.berlin/go/vaultik/internal/storage/faultstore"
 	"sneak.berlin/go/vaultik/internal/types"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
@@ -432,40 +431,6 @@ func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
 	assert.Contains(t, out, "Removed snapshot metadata from remote storage")
 }
 
-// TestRemoveSnapshot_NotInLocalIndexDeleteFails checks that removing a
-// snapshot only the destination store holds fails when its metadata
-// cannot be deleted there, under --force and under --json: nothing was
-// removed, so the command must not succeed.
-func TestRemoveSnapshot_NotInLocalIndexDeleteFails(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	for name, opts := range map[string]vaultik.RemoveOptions{
-		"force": {Force: true},
-		"json":  {JSON: true},
-	} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			store := newTestStorer()
-			addManifest(t, store, "snapshot-001", nil)
-
-			faulty := faultstore.New(store)
-			faulty.OnDelete = func(string) bool { return true }
-
-			tv := vaultik.NewForTesting(faulty)
-			tv.Repositories = newLocalIndex(t)
-
-			result, err := tv.RemoveSnapshot("snapshot-001", &opts)
-
-			require.ErrorIs(t, err, faultstore.ErrInjectedDelete)
-			assert.False(t, result.RemoteRemoved)
-			assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
-			assert.Empty(t, tv.Stdout.String())
-		})
-	}
-}
-
 func TestRemoveAllSnapshots_RequiresForce(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()

internal/vaultik/snapshot.go

@@ -1252,11 +1252,9 @@ const snapshotRemoveCommandHint = "vaultik snapshot remove"
 // destination store. Blobs are NOT touched: removing a snapshot's
 // blobs requires reading every remaining remote manifest (the remote
 // may hold snapshots this host doesn't know about), which is what
-// `vaultik prune` is for — when metadata was removed from the
+// `vaultik prune` is for — the command prints the prune invocation to
-// destination store, the text output names the prune invocation to run
+// run as a follow-up. When the remote is unreachable the command still
-// as a follow-up. When removing the metadata fails, a snapshot in the
+// completes the local-DB removal and warns.
-// local index is still removed from it, with a warning; a snapshot
-// only on the destination store is an error.
 func (v *Vaultik) RemoveSnapshot(
 	snapshotID string, opts *RemoveOptions,
 ) (*RemoveResult, error) {
@@ -1291,10 +1289,8 @@ func (v *Vaultik) RemoveSnapshot(
 		}
 	}
 
-	result.RemoteRemoved, err = v.removeSnapshotRemote(
+	if !opts.LocalOnly {
-		snapshotID, inLocalIndex, opts)
+		result.RemoteRemoved = v.removeSnapshotRemote(snapshotID, opts)
-	if err != nil {
-		return result, err
 	}
 
 	if v.SnapshotManager != nil {
@@ -1419,33 +1415,19 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
 }
 
 // removeSnapshotRemote strips the snapshot's metadata from the
-// destination store. It returns true only when the delete finished
+// destination store, warning and proceeding on failure: the local-DB
-// without an error and removed at least one object; under LocalOnly it
+// removal has already happened, so the user is told the remote half
-// does nothing. When the snapshot is only on the destination store, a
+// didn't finish and to run `vaultik snapshot remove` for the snapshot
-// failure is returned as an error, because that removal is all the
+// again once the destination store is reachable (`vaultik prune` never
-// command does. When the snapshot was in the local index, its removal
+// removes snapshot metadata). Returns true when the remote removal
-// there has already happened, so a failure is only a warning telling the
+// succeeded and deleted something.
-// user to run `vaultik snapshot remove` for the snapshot again once the
+func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) bool {
-// destination store is reachable (`vaultik prune` never removes snapshot
-// metadata).
-func (v *Vaultik) removeSnapshotRemote(
-	snapshotID string, inLocalIndex bool, opts *RemoveOptions,
-) (bool, error) {
-	if opts.LocalOnly {
-		return false, nil
-	}
-
 	log.Info("Removing snapshot metadata from remote storage",
 		"snapshot_id", snapshotID)
 
 	remoteKey := snapshot.RemoteSnapshotKey(snapshotID)
 
 	deleted, err := v.deleteRemoteSnapshotByKey(remoteKey)
-	if err != nil && !inLocalIndex {
-		return false, fmt.Errorf(
-			"removing snapshot metadata from destination store: %w", err)
-	}
-
 	if err != nil {
 		log.Warn("Could not remove snapshot metadata from remote storage; "+
 			"run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+
@@ -1460,10 +1442,10 @@ func (v *Vaultik) removeSnapshotRemote(
 				err, snapshotRemoveCommandHint, snapshotID)
 		}
 
-		return false, nil
+		return false
 	}
 
-	return deleted > 0, nil
+	return deleted > 0
 }
 
 // RemoveAllSnapshots removes every snapshot known to the local
@@ -1944,17 +1926,13 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
 	snapshotsRemain := countDiff(snapshotCountBefore, &result.SnapshotsDeleted)
 
 	v.UI.Completef("Pruned local index database.")
-	v.UI.Detailf("Incomplete snapshots: %s removed "+
+	v.UI.Detailf("Incomplete snapshots: %s removed (%s remain).",
-		"(total snapshots left in the local index: %s).",
 		countText(&result.SnapshotsDeleted), countText(snapshotsRemain))
-	v.UI.Detailf("Orphaned files: %s removed "+
+	v.UI.Detailf("Orphaned files: %s removed (%s remain).",
-		"(total files left in the local index: %s).",
 		countText(result.FilesDeleted), countText(fileCountAfter))
-	v.UI.Detailf("Orphaned chunks: %s removed "+
+	v.UI.Detailf("Orphaned chunks: %s removed (%s remain).",
-		"(total chunks left in the local index: %s).",
 		countText(result.ChunksDeleted), countText(chunkCountAfter))
-	v.UI.Detailf("Orphaned blobs: %s removed "+
+	v.UI.Detailf("Orphaned blobs: %s removed (%s remain).",
-		"(total blobs left in the local index: %s).",
 		countText(result.BlobsDeleted), countText(blobCountAfter))
 
 	return result, nil

internal/vaultik/snapshot_deleted_count_test.go

@@ -115,7 +115,7 @@ func TestBackupDoesNotCountExcludedFileAsDeleted(t *testing.T) {
 
 // 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 and the directory is gone from disk.
+// directory.
 func TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
@@ -128,34 +128,10 @@ func TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted(t *testing.T) {
 
 	assert.Contains(t, env.backUp(t, "home"), filesLine(2, 2, 0))
 
-	require.NoError(t, os.RemoveAll(
-		filepath.Join(env.src, "user/.local/share/Trash")))
-
 	env.setSnapshot("home", "", "*/.local/share/Trash")
 	assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
 }
 
-// A file whose directory was replaced by a regular file of the same name
-// is counted as deleted.
-func TestBackupCountsFileInDirectoryReplacedByFileAsDeleted(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	env := newDeletedCountEnv(t, map[string][]byte{
-		"dir/inside.txt": []byte("inside"),
-	})
-	env.setSnapshot("home", "")
-
-	assert.Contains(t, env.backUp(t, "home"), filesLine(1, 1, 0))
-
-	dir := filepath.Join(env.src, "dir")
-	require.NoError(t, os.RemoveAll(dir))
-	require.NoError(t, os.WriteFile(dir, []byte("now a file"), 0o600))
-
-	assert.Contains(t, env.backUp(t, "home"),
-		"Files: 1 examined, 1 backed up, 0 unchanged, 1 deleted.")
-}
-
 // Under --skip-errors a file inside a directory that the walk cannot
 // open is not counted as deleted: it is still on disk.
 func TestBackupDoesNotCountFileInUnopenableDirectoryAsDeleted(t *testing.T) {