Restore the other files when a chunk is in no blob under --skip-errors (closes #326)

Restore planning stopped at the first file needing a chunk that no blob
holds, so --skip-errors never saw that file and nothing was restored.
The planner now leaves such a file out of the plan and returns it with
the reason, and each one goes through handleRestoreFileError like any
other failed file: skipped and counted under --skip-errors, fatal
without it. The fatal error now suggests --skip-errors, as other restore
failures do.

The skip-errors tests now share one backup helper; the new case removes
a blob from the snapshot metadata and exports it again.

Model: opus-5-5

internal/cli/config.go

@@ -251,6 +251,12 @@ func NewConfigCommand() *cobra.Command {
 		Use:   "config",
 		Short: "Manage the configuration file",
 		Long:  "Commands for creating, editing, and querying the vaultik config file.",
+		// cobra checks Args only on a command that has a Run: without
+		// one, a mistyped subcommand printed help and exited 0.
+		Args: cobra.NoArgs,
+		Run: func(cmd *cobra.Command, _ []string) {
+			_ = cmd.Help()
+		},
 	}
 
 	cmd.AddCommand(newConfigInitCommand())

internal/cli/database.go

@@ -15,6 +15,12 @@ func NewDatabaseCommand() *cobra.Command {
 		Use:   "database",
 		Short: "Manage the local state database",
 		Long:  `Commands for managing the local SQLite state database.`,
+		// cobra checks Args only on a command that has a Run: without
+		// one, a mistyped subcommand printed help and exited 0.
+		Args: cobra.NoArgs,
+		Run: func(cmd *cobra.Command, _ []string) {
+			_ = cmd.Help()
+		},
 	}
 
 	cmd.AddCommand(

internal/cli/entry_unknown_subcommand_test.go

@@ -0,0 +1,43 @@
+package cli //nolint:testpackage // shares runEntry and the command-word constants
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestEntryGroupRejectsUnknownSubcommand runs each group command with a
+// mistyped subcommand, as a crontab line might carry one. It must fail
+// with exit 1 and name the word, as `vaultik bogus` does at the top
+// level, instead of printing the group's help and exiting 0 having done
+// nothing. The group name alone still prints its help.
+//
+//nolint:paralleltest // replaces os.Args, os.Stdout and os.Stderr
+func TestEntryGroupRejectsUnknownSubcommand(t *testing.T) {
+	for _, testCase := range []struct {
+		group    string
+		mistyped string
+	}{
+		{group: cmdSnapshot, mistyped: "craete"},
+		{group: "config", mistyped: "inti"},
+		{group: cmdRemote, mistyped: "ifno"},
+		{group: "database", mistyped: "delte"},
+		{group: "completion", mistyped: "nosuchshell"},
+	} {
+		t.Run(testCase.group, func(t *testing.T) {
+			code, stdout, stderr := runEntry(t,
+				testCase.group, testCase.mistyped)
+
+			assert.Equal(t, 1, code)
+			assert.Empty(t, stdout)
+			assert.Contains(t, stderr,
+				`unknown command "`+testCase.mistyped+`"`)
+
+			code, stdout, _ = runEntry(t, testCase.group)
+
+			assert.Equal(t, 0, code)
+			assert.Contains(t, stdout,
+				programName+" "+testCase.group+" [command]")
+		})
+	}
+}

internal/cli/remote.go

@@ -18,6 +18,12 @@ func NewRemoteCommand() *cobra.Command {
 		Use:   "remote",
 		Short: "Remote storage management commands",
 		Long:  "Commands for inspecting and managing remote storage",
+		// cobra checks Args only on a command that has a Run: without
+		// one, a mistyped subcommand printed help and exited 0.
+		Args: cobra.NoArgs,
+		Run: func(cmd *cobra.Command, _ []string) {
+			_ = cmd.Help()
+		},
 	}
 
 	// Add subcommands

internal/cli/root.go

@@ -74,6 +74,21 @@ on the source system.`,
 		NewDatabaseCommand(),
 	)
 
+	// cobra adds the completion command only when Execute runs, and with
+	// no Run, so it skipped its own cobra.NoArgs and a mistyped shell name
+	// printed help and exited 0. Adding it here lets it have a Run. Its
+	// scripts go to os.Stdout as it is now: a later SetOut does not
+	// redirect them.
+	cmd.InitDefaultCompletionCmd()
+
+	for _, sub := range cmd.Commands() {
+		if sub.Name() == "completion" {
+			sub.Run = func(completion *cobra.Command, _ []string) {
+				_ = completion.Help()
+			}
+		}
+	}
+
 	return cmd
 }
 

internal/cli/snapshot.go

@@ -41,6 +41,12 @@ func NewSnapshotCommand() *cobra.Command {
 		Use:   "snapshot",
 		Short: "Snapshot management commands",
 		Long:  "Commands for creating, listing, and managing snapshots",
+		// cobra checks Args only on a command that has a Run: without
+		// one, a mistyped subcommand printed help and exited 0.
+		Args: cobra.NoArgs,
+		Run: func(cmd *cobra.Command, _ []string) {
+			_ = cmd.Help()
+		},
 	}
 
 	// Add subcommands

internal/vaultik/restore.go

@@ -309,11 +309,22 @@ func (v *Vaultik) restoreAllFiles(
 		return nil, err
 	}
 
-	plan, err := newRestorePlan(v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
+	plan, unplanned, err := newRestorePlan(
+		v.ctx, repos, files, chunkToBlobMap, blobIDToHash)
 	if err != nil {
 		return nil, fmt.Errorf("building restore plan: %w", err)
 	}
 
+	// Ranging over files rather than the map reports them in a stable order.
+	for _, file := range files {
+		if planErr, ok := unplanned[file.ID]; ok {
+			err = v.handleRestoreFileError(plan, opts, result, file, file.ID, planErr)
+			if err != nil {
+				return nil, err
+			}
+		}
+	}
+
 	filesByID, totalBytesExpected := indexRestoreFiles(files)
 
 	v.UI.Beginf("Restoring %s files (%s) to %s.",

internal/vaultik/restore_malformed_db_test.go

@@ -61,8 +61,8 @@ func craftedFile(
 }
 
 // TestRestoreShortChunkHashInNoBlob proves a file whose short chunk hash
-// has no blob_chunks row fails restore planning and the chunk write with
-// an error.
+// has no blob_chunks row is left out of the restore plan with an error,
+// and fails the chunk write with an error.
 func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
 	t.Parallel()
 
@@ -77,9 +77,11 @@ func TestRestoreShortChunkHashInNoBlob(t *testing.T) {
 	chunkToBlobMap, err := v.buildChunkToBlobMap(ctx, repos)
 	require.NoError(t, err)
 
-	_, err = newRestorePlan(ctx, repos, []*database.File{file},
+	plan, unplanned, err := newRestorePlan(ctx, repos, []*database.File{file},
 		chunkToBlobMap, map[string]string{})
-	require.ErrorIs(t, err, errPlanChunkMissing)
+	require.NoError(t, err)
+	require.ErrorIs(t, unplanned[file.ID], errPlanChunkMissing)
+	require.False(t, plan.hasPending())
 
 	fileChunks, err := repos.FileChunks.GetByFileID(ctx, file.ID)
 	require.NoError(t, err)

internal/vaultik/restore_plan.go

@@ -41,20 +41,25 @@ type restorePlan struct {
 
 // newRestorePlan builds the file→blob index for the given files. Files
 // whose chunks reference no blobs (symlinks, directories) start in the
-// ready queue immediately.
+// ready queue immediately. A file with a chunk that no blob holds cannot
+// be restored: it is left out of the plan and returned, by ID, with the
+// reason.
 func newRestorePlan(
 	ctx context.Context,
 	repos *database.Repositories,
 	files []*database.File,
 	chunkToBlobMap map[string]*database.BlobChunk,
 	blobIDToHash map[string]string,
-) (*restorePlan, error) {
+) (*restorePlan, map[types.FileID]error, error) {
 	p := &restorePlan{
 		fileBlobs: make(map[types.FileID]map[string]struct{}, len(files)),
 		blobFiles: make(map[string]map[types.FileID]struct{}),
 		ready:     make([]types.FileID, 0, len(files)),
 		cached:    make(map[string]struct{}),
 	}
+	unplanned := make(map[types.FileID]error)
+
+nextFile:
 	for _, f := range files {
 		if f.IsSymlink() || f.Mode&uint32(os.ModeDir) != 0 {
 			// No chunks to fetch — restore can run immediately.
@@ -66,7 +71,7 @@ func newRestorePlan(
 
 		fileChunks, err := repos.FileChunks.GetByFileID(ctx, f.ID)
 		if err != nil {
-			return nil, fmt.Errorf("planning %s: %w", f.Path, err)
+			return nil, nil, fmt.Errorf("planning %s: %w", f.Path, err)
 		}
 
 		blobs := make(map[string]struct{})
@@ -74,14 +79,18 @@ func newRestorePlan(
 		for _, fc := range fileChunks {
 			bc, ok := chunkToBlobMap[fc.ChunkHash.String()]
 			if !ok {
-				return nil, fmt.Errorf("planning %s: %w: %s",
-					f.Path, errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
+				unplanned[f.ID] = fmt.Errorf("%w: %s",
+					errPlanChunkMissing, shortHash(fc.ChunkHash.String()))
+
+				continue nextFile
 			}
 
 			hash, ok := blobIDToHash[bc.BlobID.String()]
 			if !ok {
-				return nil, fmt.Errorf("planning %s: %w: %s",
-					f.Path, errPlanBlobIDMissing, bc.BlobID)
+				unplanned[f.ID] = fmt.Errorf("%w: %s",
+					errPlanBlobIDMissing, bc.BlobID)
+
+				continue nextFile
 			}
 
 			blobs[hash] = struct{}{}
@@ -103,7 +112,7 @@ func newRestorePlan(
 		}
 	}
 
-	return p, nil
+	return p, unplanned, nil
 }
 
 // markBlobCached records that the named blob is now resident in the

internal/vaultik/restore_skip_errors_test.go

@@ -28,7 +28,8 @@ const (
 )
 
 // missingBlobBackup is a snapshot of single-chunk files spread over two
-// blobs, with one of those blobs deleted from the store.
+// blobs, with one of those blobs lost: deleted from the store, or missing
+// from the snapshot metadata.
 type missingBlobBackup struct {
 	fs         afero.Fs
 	cfg        *config.Config
@@ -37,20 +38,39 @@ type missingBlobBackup struct {
 	restoreDir string
 	// files holds the original content by source path.
 	files map[string][]byte
-	// lost holds the source paths whose chunk is in the deleted blob.
+	// lost holds the source paths whose chunk is in the lost blob.
 	lost map[string]bool
 }
 
 // TestRestoreSkipErrorsSkipsFilesOfMissingBlob restores with SkipErrors
-// after one blob of a two-blob snapshot was deleted. Every file stored in
-// that blob must be reported as failed and left absent, every other file
-// must be restored intact, and Restore must still return an error.
+// after one blob of a two-blob snapshot was deleted from the store.
 func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
 
 	ctx := context.Background()
-	backup := backupThenDeleteOneBlob(ctx, t)
+	assertRestoreSkipsLostFiles(ctx, t, backupThenDeleteOneBlob(ctx, t))
+}
+
+// TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob restores with
+// SkipErrors a snapshot whose metadata lists chunks that no blob holds, as
+// earlier backup defects left behind.
+func TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	assertRestoreSkipsLostFiles(ctx, t, backupThenDropOneBlobFromMetadata(ctx, t))
+}
+
+// assertRestoreSkipsLostFiles restores backup with SkipErrors. Every file
+// stored in the lost blob must be reported as failed and left absent,
+// every other file must be restored intact, and Restore must still return
+// an error.
+func assertRestoreSkipsLostFiles(
+	ctx context.Context, t *testing.T, backup *missingBlobBackup,
+) {
+	t.Helper()
 
 	var out bytes.Buffer
 
@@ -72,14 +92,14 @@ func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
 
 		if backup.lost[path] {
 			assert.Containsf(t, out.String(), path,
-				"%s needs the deleted blob and must be reported", path)
+				"%s needs the lost blob and must be reported", path)
 			assert.NoFileExists(t, restored)
 
 			continue
 		}
 
 		got, err := afero.ReadFile(backup.fs, restored)
-		require.NoErrorf(t, err, "%s does not need the deleted blob", path)
+		require.NoErrorf(t, err, "%s does not need the lost blob", path)
 		assert.Equalf(t, content, got, "%s restored with wrong content", path)
 	}
 }
@@ -103,6 +123,30 @@ func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
 	require.ErrorIs(t, err, storage.ErrNotFound)
 }
 
+// TestRestoreChunkInNoBlobAbortsWithoutSkipErrors checks that a chunk held
+// by no blob still ends the restore, before any file is written, when
+// SkipErrors is not set.
+func TestRestoreChunkInNoBlobAbortsWithoutSkipErrors(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	backup := backupThenDropOneBlobFromMetadata(ctx, t)
+
+	v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, backup.fs)
+
+	err := v.Restore(&vaultik.RestoreOptions{
+		SnapshotID: backup.snapshotID,
+		TargetDir:  backup.restoreDir,
+	})
+
+	require.ErrorContains(t, err, "chunk missing from blob map")
+
+	for path := range backup.files {
+		assert.NoFileExists(t, filepath.Join(backup.restoreDir, path))
+	}
+}
+
 // backupThenDeleteOneBlob backs up missingBlobFileCount single-chunk
 // files, then deletes from the store the blob holding the first of them.
 func backupThenDeleteOneBlob(
@@ -110,6 +154,54 @@ func backupThenDeleteOneBlob(
 ) *missingBlobBackup {
 	t.Helper()
 
+	backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
+	require.NoError(t, db.Close())
+
+	require.NoError(t, backup.storer.Delete(ctx, fmt.Sprintf(
+		"blobs/%s/%s/%s", lostBlob[:2], lostBlob[2:4], lostBlob)))
+
+	return backup
+}
+
+// backupThenDropOneBlobFromMetadata backs up missingBlobFileCount
+// single-chunk files, then removes the blob holding the first of them from
+// the snapshot and exports the snapshot metadata again. The exported
+// metadata then lists chunks that no blob holds.
+func backupThenDropOneBlobFromMetadata(
+	ctx context.Context, t *testing.T,
+) *missingBlobBackup {
+	t.Helper()
+
+	backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
+
+	// The uploads row refers to the blob row, so it would keep the export
+	// from deleting it.
+	for _, query := range []string{
+		"DELETE FROM snapshot_blobs WHERE blob_hash = ?",
+		"DELETE FROM uploads WHERE blob_hash = ?",
+	} {
+		_, err := db.Conn().ExecContext(ctx, query, lostBlob)
+		require.NoError(t, err)
+	}
+
+	sm := newFaultSnapshotManager(
+		backup.fs, backup.storer, backup.cfg, database.NewRepositories(db))
+	require.NoError(t,
+		sm.ExportSnapshotMetadata(ctx, db.Path(), backup.snapshotID))
+	require.NoError(t, db.Close())
+
+	return backup
+}
+
+// backupSingleChunkFiles backs up missingBlobFileCount single-chunk files.
+// It returns the backup, with lost naming the files stored in the same
+// blob as the first of them, the still open local index, and the hash of
+// that blob.
+func backupSingleChunkFiles(
+	ctx context.Context, t *testing.T,
+) (*missingBlobBackup, *database.DB, string) {
+	t.Helper()
+
 	fs := afero.NewOsFs()
 	tempDir := t.TempDir()
 	dataDir := filepath.Join(tempDir, "src")
@@ -143,16 +235,11 @@ func backupThenDeleteOneBlob(
 		blobOfFile[path] = blobHashOfSingleChunkFile(ctx, t, repos, path)
 	}
 
-	require.NoError(t, db.Close())
-
-	deleted := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
-	require.NoError(t, storer.Delete(ctx, fmt.Sprintf(
-		"blobs/%s/%s/%s", deleted[:2], deleted[2:4], deleted)))
-
+	lostBlob := blobOfFile[filepath.Join(dataDir, "file-00.bin")]
 	lost := make(map[string]bool)
 
 	for path, hash := range blobOfFile {
-		if hash == deleted {
+		if hash == lostBlob {
 			lost[path] = true
 		}
 	}
@@ -168,7 +255,7 @@ func backupThenDeleteOneBlob(
 		restoreDir: filepath.Join(tempDir, "restored"),
 		files:      files,
 		lost:       lost,
-	}
+	}, db, lostBlob
 }
 
 // blobHashOfSingleChunkFile returns the hash of the blob holding the one