Make remote nuke delete leftover .partial uploads (closes #281 )

The file and rclone listings skip an object whose name ends in `.partial`, the temporary name a `file://` or rclone upload writes before moving the object into place. `remote nuke` deletes only what the listings return, so it left the `.partial` objects killed uploads leave behind and still reported the destination store empty. Storer gains DeletePartialUploads. The file and rclone backends remove every `.partial` object under the prefix; S3 has none to remove, since it shows an object only once its upload completes. `remote nuke` calls it for `metadata/` and `blobs/` as its last step. Empty directories under a `file://` destination are still left behind. Model: opus-5-5
2026-10-08 10:32:21 +00:00
4 changed files with 25 additions and 212 deletions
@@ -22,14 +22,6 @@ the tag exists and is exercised; what is left is merging `next` to

 # Completed Steps

- 2026-10-08: Made a backup cancelled under `--skip-errors` stop at once
-  ([issue #286](https://git.eeqj.de/sneak/vaultik/issues/286)). After
-  Ctrl-C or SIGTERM, phase 2 treated the cancellation error from each
-  remaining file like an unreadable file: it opened the file, printed an
-  error line for it, counted it as failed and went on to the next one.
-  The run now stops at the first file after the cancellation, and no
-  file is counted as failed because of it.
-
 - 2026-10-08: Made `remote nuke` delete the `.partial` files that
  uploads cut off part-way leave on the destination store
  ([issue #281](https://git.eeqj.de/sneak/vaultik/issues/281)). The
@@ -37,15 +29,6 @@ the tag exists and is exercised; what is left is merging `next` to
  place and still reported the store empty. It now removes them under
  `metadata/` and `blobs/` as its last step.

- 2026-10-08: Made a local index error while recording a directory or
-  symlink stop a backup under `--skip-errors`
-  ([issue #284](https://git.eeqj.de/sneak/vaultik/issues/284)). Phase 2
-  only records such an entry in the local index, since it has no data
-  to open or read, but an error doing so was skipped like an unreadable
-  file. The snapshot then completed without the entry, and a restore
-  did not recreate it. The error now stops the backup, as it does
-  without the flag.
-
 - 2026-10-08: Counted a file that a backup could not store as failed
  ([issue #280](https://git.eeqj.de/sneak/vaultik/issues/280)). A file
  that phase 1 counted and phase 2 could not open, because it was
@@ -1312,13 +1312,6 @@ func (s *Scanner) processPhase(

 	// Process each file
 	for _, fileToProcess := range filesToProcess {
-		// Check context cancellation
-		select {
-		case <-ctx.Done():
-			return ctx.Err()
-		default:
-		}
-
 		// Update progress
 		if s.progress != nil {
 			s.progress.GetStats().CurrentFile.Store(fileToProcess.Path)
@@ -1355,32 +1348,13 @@ func (s *Scanner) processPhase(
 	return s.finalizeProcessPhase(ctx, result)
 }

-// processFileWithErrorHandling records a directory or symlink, or wraps
-// processFileStreaming for a regular file with error recovery for deleted
-// files and skip-errors mode. Returns (skipped, error).
+// processFileWithErrorHandling wraps processFileStreaming 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() {
-		err := s.recordNonRegularFile(ctx, fileToProcess)
-		if err != nil {
-			return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err)
-		}
-
-		return false, nil
-	}
-
 	err := s.processFileStreaming(ctx, fileToProcess, result)
 	if err != nil {
-		// A cancelled run stops here even under --skip-errors, rather than
-		// counting this file as failed and going on to the next one.
-		if ctx.Err() != nil {
-			return false, fmt.Errorf("processing file %s: %w", fileToProcess.Path, err)
-		}
 		// A packer/database/encryption/upload failure means the chunk's data
 		// may not have been stored. Skipping the file would let the snapshot
 		// record a file whose chunk is in no blob and cannot be restored, so
@@ -1418,7 +1392,12 @@ func (s *Scanner) processFileWithErrorHandling(

 // countFailedFile counts a file that phase 2 could not store as failed
 // and takes its size back out of BytesScanned, where phase 1 put it.
+// Phase 1 counts no directories, so a directory is not counted here.
 func countFailedFile(fileToProcess *FileToProcess, result *ScanResult) {
+	if fileToProcess.FileInfo.IsDir() {
+		return
+	}
+
 	result.FilesFailed++
 	result.BytesScanned -= fileToProcess.FileInfo.Size()
 }
@@ -1791,11 +1770,16 @@ func (e *packerError) Error() string { return e.err.Error() }

 func (e *packerError) Unwrap() error { return e.err }

-// processFileStreaming processes a regular file by streaming chunks directly
-// to the packer
+// processFileStreaming processes a file by streaming chunks directly to the packer
 func (s *Scanner) processFileStreaming(
 	ctx context.Context, fileToProcess *FileToProcess, result *ScanResult,
 ) error {
+	// Symlinks and directories have no data to chunk — just record them in the DB.
+	mode := os.FileMode(fileToProcess.File.Mode)
+	if mode&os.ModeSymlink != 0 || mode.IsDir() {
+		return s.recordNonRegularFile(ctx, fileToProcess)
+	}
+
 	file, err := s.fs.Open(fileToProcess.Path)
 	if err != nil {
 		return fmt.Errorf("opening file: %w", wrapPermissionError(fileToProcess.Path, err))
@@ -82,32 +82,6 @@ func (f *readFailFs) Open(name string) (afero.File, error) {
 	return file, nil
 }

-// cancelOnOpenFs cancels the run when the scanner opens the target file to
-// back it up, as Ctrl-C partway through processing would, and records each
-// file opened after that.
-type cancelOnOpenFs struct {
-	afero.Fs
-
-	target            string
-	cancel            context.CancelFunc
-	cancelled         bool
-	openedAfterCancel []string
-}
-
-//nolint:ireturn // afero.Fs.Open is defined to return the interface.
-func (f *cancelOnOpenFs) Open(name string) (afero.File, error) {
-	if f.cancelled {
-		f.openedAfterCancel = append(f.openedAfterCancel, name)
-	}
-
-	if name == f.target {
-		f.cancel()
-		f.cancelled = true
-	}
-
-	return f.Fs.Open(name)
-}
-
 // linkRemovedAfterLstatFs is the real filesystem, except that the symlink at
 // target is removed right after the walk lstats it, as happens when a link is
 // deleted during a backup. The scanner's readlink of it then fails.
@@ -154,16 +128,15 @@ func writeSkipErrorTestFile(t *testing.T, fs afero.Fs, path, content string) {
 	}
 }

-// runSkipErrorScan scans source on fs under ctx with the given skip-errors
-// setting, printing user-facing messages to uiw (nil discards them), and
-// returns the repositories (for inspection) and the scan error.
+// runSkipErrorScan scans source on fs with the given skip-errors setting,
+// printing user-facing messages to uiw (nil discards them), and returns the
+// repositories (for inspection) and the scan error.
 func runSkipErrorScan(
-	ctx context.Context, t *testing.T, fs afero.Fs, source string,
-	skipErrors bool, uiw *ui.Writer,
+	t *testing.T, fs afero.Fs, source string, skipErrors bool, uiw *ui.Writer,
 ) (*database.Repositories, error) {
 	t.Helper()

-	db, err := database.New(ctx, ":memory:")
+	db, err := database.NewTestDB()
 	if err != nil {
 		t.Fatalf("create test db: %v", err)
 	}
@@ -188,6 +161,7 @@ func runSkipErrorScan(
 		SkipErrors:       skipErrors,
 	})

+	ctx := context.Background()
 	snapshotID := "test-snapshot-skip-errors"
 	createTestSnapshotRecord(ctx, t, repos, snapshotID)

@@ -211,7 +185,7 @@ func TestScannerPackingFailureAbortsUnderSkipErrors(t *testing.T) {
 	writeSkipErrorTestFile(t, fs, "/source/file1.txt", "first file content")
 	writeSkipErrorTestFile(t, fs, "/source/file2.txt", "second file content")

-	repos, err := runSkipErrorScan(context.Background(), t, fs, "/source", true, nil)
+	repos, err := runSkipErrorScan(t, fs, "/source", true, nil)
 	if err == nil {
 		t.Fatal("expected scan to abort on the packer error, got nil")
 	}
@@ -238,7 +212,7 @@ func TestScannerReadErrorAbortsWithoutSkipErrors(t *testing.T) {
 	fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
 	writeSkipErrorTestFile(t, fs, target, "content that cannot be read")

-	_, err := runSkipErrorScan(context.Background(), t, fs, "/source", false, nil)
+	_, err := runSkipErrorScan(t, fs, "/source", false, nil)
 	if err == nil {
 		t.Fatal("expected scan to fail on the read error, got nil")
 	}
@@ -254,7 +228,7 @@ func TestScannerReadErrorSkippedWithSkipErrors(t *testing.T) {
 	fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target}
 	writeSkipErrorTestFile(t, fs, target, "content that cannot be read")

-	repos, err := runSkipErrorScan(context.Background(), t, fs, "/source", true, nil)
+	repos, err := runSkipErrorScan(t, fs, "/source", true, nil)
 	if err != nil {
 		t.Fatalf("expected scan to complete with --skip-errors, got %v", err)
 	}
@@ -293,7 +267,7 @@ func TestScannerUnreadableSymlinkAbortsWithoutSkipErrors(t *testing.T) {
 	sourceDir, linkPath := writeSymlinkSource(t)
 	fs := &linkRemovedAfterLstatFs{t: t, target: linkPath}

-	_, err := runSkipErrorScan(context.Background(), t, fs, sourceDir, false, nil)
+	_, err := runSkipErrorScan(t, fs, sourceDir, false, nil)
 	if !errors.Is(err, os.ErrNotExist) {
 		t.Fatalf("expected scan to fail on the removed symlink, got %v", err)
 	}
@@ -309,7 +283,7 @@ func TestScannerUnreadableSymlinkSkippedWithSkipErrors(t *testing.T) {
 	fs := &linkRemovedAfterLstatFs{t: t, target: linkPath}
 	uiw := ui.NewWithColor(io.Discard, false)

-	repos, err := runSkipErrorScan(context.Background(), t, fs, sourceDir, true, uiw)
+	repos, err := runSkipErrorScan(t, fs, sourceDir, true, uiw)
 	if err != nil {
 		t.Fatalf("expected scan to complete with --skip-errors, got %v", err)
 	}
@@ -328,56 +302,3 @@ func TestScannerUnreadableSymlinkSkippedWithSkipErrors(t *testing.T) {
 		t.Fatalf("expected %s not to be recorded", linkPath)
 	}
 }
-
-// TestScannerCancelStopsSkipErrorsRun checks that a --skip-errors backup
-// cancelled partway through processing returns the cancellation error,
-// opens no further file, and reports no file as failed.
-func TestScannerCancelStopsSkipErrorsRun(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name          string
-		targetContent string
-	}{
-		// The cancellation lands while the target is being read.
-		{name: "while reading a file", targetContent: "first file content"},
-		// An empty target has no chunks, so the cancellation goes unnoticed
-		// until the run moves on to the next file.
-		{name: "between files", targetContent: ""},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			const target = "/source/a.txt"
-
-			ctx, cancel := context.WithCancel(context.Background())
-			defer cancel()
-
-			fs := &cancelOnOpenFs{
-				Fs: afero.NewMemMapFs(), target: target, cancel: cancel,
-			}
-			writeSkipErrorTestFile(t, fs, target, tt.targetContent)
-			writeSkipErrorTestFile(t, fs, "/source/b.txt", "second file content")
-			writeSkipErrorTestFile(t, fs, "/source/c.txt", "third file content")
-
-			uiw := ui.NewWithColor(io.Discard, false)
-
-			_, err := runSkipErrorScan(ctx, t, fs, "/source", true, uiw)
-			if !errors.Is(err, context.Canceled) {
-				t.Fatalf("expected the cancellation error, got %v", err)
-			}
-
-			if len(fs.openedAfterCancel) != 0 {
-				t.Fatalf("expected no file opened after the cancellation, got %v",
-					fs.openedAfterCancel)
-			}
-
-			if uiw.ErrorCount() != 0 {
-				t.Fatalf("expected no file reported as failed, got %d error lines",
-					uiw.ErrorCount())
-			}
-		})
-	}
-}
@@ -1,75 +0,0 @@
-package vaultik_test
-
-import (
-	"context"
-	"fmt"
-	"path/filepath"
-	"testing"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/database"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/storage"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// --skip-errors skips only a file that cannot be opened or read. A local
-// index error while a directory is recorded stops the backup, and the
-// snapshot is not recorded as complete. See
-// https://git.eeqj.de/sneak/vaultik/issues/284.
-func TestSkipErrorsBackupStopsOnIndexErrorRecordingDirectory(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-
-	// The scan walks the source path with symlinks resolved, so dirPath
-	// must be spelled the same way to match the row the scan inserts.
-	tempDir, err := filepath.EvalSymlinks(t.TempDir())
-	require.NoError(t, err)
-
-	srcDir := filepath.Join(tempDir, "src")
-	dirPath := filepath.Join(srcDir, "dir")
-
-	fs := afero.NewOsFs()
-	require.NoError(t, fs.MkdirAll(dirPath, 0o755))
-	require.NoError(t, afero.WriteFile(fs,
-		filepath.Join(dirPath, "file.txt"), []byte("file content"), 0o644))
-
-	cfg := faultTestConfig()
-	cfg.IndexPath = filepath.Join(tempDir, "index.sqlite")
-	cfg.Snapshots = map[string]config.SnapshotConfig{
-		"tree": {Paths: []string{srcDir}},
-	}
-
-	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() })
-
-	// The local index refuses the directory's files row.
-	_, err = db.Conn().ExecContext(ctx, fmt.Sprintf(`
-		CREATE TRIGGER refuse_directory BEFORE INSERT ON files
-		WHEN NEW.path = '%s'
-		BEGIN SELECT RAISE(ABORT, 'simulated index error'); END`, dirPath))
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-	v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
-
-	err = v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
-		SkipErrors: true,
-		Snapshots:  []string{"tree"},
-	})
-	require.ErrorContains(t, err, "simulated index error")
-
-	snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
-	require.NoError(t, err)
-	require.Len(t, snapshots, 1)
-	assert.Nil(t, snapshots[0].CompletedAt)
-}