From 017877da4461159afef334d0f1629f86e8a43eaa Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 13 Oct 2026 08:07:08 +0000 Subject: [PATCH] Stop restore --verify re-reporting files that failed to restore (closes #323) With --skip-errors, a file that failed to restore was verified anyway, failed again because it was never written, and was added to the same failure list as the restore failure. restore --verify then listed it twice and called every entry a checksum mismatch. Verification now skips the files that failed to restore and keeps its own failures in a separate list, so failed restores and failed verifications are reported apart, each path once. The restore still exits non-zero. Model: opus-5-5 --- README.md | 4 +- internal/vaultik/restore.go | 39 ++++++++++++++------ internal/vaultik/restore_skip_errors_test.go | 35 ++++++++++++++++++ 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 306b27d..b1ba3c4 100644 --- a/README.md +++ b/README.md @@ -375,7 +375,9 @@ 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 file's chunk hashes match +* `--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. **`prune`**: Tidy up everything that isn't needed. Runs three passes: (1) reconcile the local index against the destination store — any diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index c5843e8..352a608 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -103,11 +103,12 @@ 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 - FilesFailed int - FailedFiles []string // Paths of files that failed verification + FilesVerified int + BytesVerified int64 + VerifyFailedFiles []string // Paths of restored files that failed verification } // Restore restores files from a snapshot to the target directory @@ -647,20 +648,34 @@ func (v *Vaultik) handleRestoreVerification( opts *RestoreOptions, result *RestoreResult, ) error { - err := v.verifyRestoredFiles(v.ctx, repos, files, opts.TargetDir, result) + // 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) if err != nil { return fmt.Errorf("verification failed: %w", err) } - if result.FilesFailed > 0 { + if len(result.VerifyFailedFiles) > 0 { v.UI.Errorf("Verification failed: %s files did not match expected checksums.", - v.UI.Count(result.FilesFailed)) + v.UI.Count(len(result.VerifyFailedFiles))) - for _, path := range result.FailedFiles { + for _, path := range result.VerifyFailedFiles { v.UI.Detailf("%s", v.UI.Path(path)) } - return fmt.Errorf("%d %w", result.FilesFailed, errFilesFailedVerify) + return fmt.Errorf("%d %w", len(result.VerifyFailedFiles), errFilesFailedVerify) } v.UI.Completef("Verified %s files (%s).", @@ -1420,8 +1435,8 @@ func (v *Vaultik) verifyRestoredFiles( if err != nil { log.Error("File verification failed", "path", file.Path, "error", err) - result.FilesFailed++ - result.FailedFiles = append(result.FailedFiles, file.Path.String()) + result.VerifyFailedFiles = append( + result.VerifyFailedFiles, file.Path.String()) } bytesProcessed += file.Size @@ -1437,7 +1452,7 @@ func (v *Vaultik) verifyRestoredFiles( log.Info("Verification complete", "files_verified", result.FilesVerified, "bytes_verified", ubytes(result.BytesVerified), - "files_failed", result.FilesFailed, + "files_failed", len(result.VerifyFailedFiles), ) return nil diff --git a/internal/vaultik/restore_skip_errors_test.go b/internal/vaultik/restore_skip_errors_test.go index 7cbfeff..8ec87b0 100644 --- a/internal/vaultik/restore_skip_errors_test.go +++ b/internal/vaultik/restore_skip_errors_test.go @@ -84,6 +84,41 @@ func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) { } } +// 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") +} + // TestRestoreMissingBlobAbortsWithoutSkipErrors checks that a deleted blob // still ends the restore with an error when SkipErrors is not set. func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {