Delete metadata directories a backup left without a manifest (closes #390)

A backup that stopped between uploading db.zst.age and its manifest
left that directory under metadata/ on the destination for good:
prune deleted nothing there.

The blob step of prune, which snapshot create --prune also runs, now
deletes each directory under metadata/ that has no manifest.json.zst,
that no local snapshot record claims, and all of whose objects are
older than prune_grace_period; an object listed with no time keeps its
directory, and only names that are remote keys are deleted. The prompt
and the --force output name each such directory, the --json result
counts them (metadata_dirs_deleted, metadata_dirs_failed), and a failed
delete fails the run. Every description of the grace period now
covers these directories.

Model: opus-5-5

README.md

@@ -205,9 +205,9 @@ corrupt the local index.
 
 The lock covers one user on one machine. It does not stop another user,
 or another host, from writing to the same destination store at the same
-time. A backup uploads its blobs, and then its `db.zst.age`, before its
+time. A backup uploads its blobs, and then its `db.zst.age`, before the
-manifest, so a prune from elsewhere at that moment finds blobs no snapshot
+manifest that lists them, so a prune from elsewhere at that moment finds
-lists and a directory under `metadata/` with no manifest. It
+blobs no snapshot lists and a directory under `metadata/` with no manifest. It
 deletes such a blob only once it is older than `prune_grace_period` (default
 `24h`), and such a directory only once every object in it is, so the grace
 period must be longer than the longest backup run of any host or user sharing
@@ -699,7 +699,7 @@ complete annotated example also lives in
 | `chunk_size` | `10MB` | Average chunk size for content-defined chunking |
 | `blob_size_limit` | `10GB` | Maximum blob size before splitting. Must be at least four times `chunk_size` (the largest chunk the chunker can emit), otherwise a single-chunk blob could exceed the limit. A backup needs free temporary space, because each blob is written whole to a temporary file before it is uploaded (up to about `blob_size_limit`; an rclone destination that cannot stream uploads needs about twice that) and the metadata export writes copies of the local index. Temporary files go to `$TMPDIR` (default `/tmp`); with `TMPDIR` unset, SQLite writes one of those copies to `/var/tmp` |
 | `compression_level` | `3` | zstd compression level (1-19) |
-| `prune_grace_period` | `24h` | How old a blob that no snapshot lists must be before `prune` deletes it, and how old every object in a directory under `metadata/` with no manifest must be before `prune` deletes that directory. A backup uploads its blobs and its `db.zst.age` before its manifest, so on a destination shared between hosts or users this must be longer than the longest backup run (see [locking](#locking)). It does not protect a blob a backup reuses from an earlier run that stopped before its manifest: that blob keeps the time of its first upload, so a prune can delete it. A duration such as `36h` or `90m`; `0s` deletes such blobs and directories at once |
+| `prune_grace_period` | `24h` | How old a blob that no snapshot lists must be before `prune` deletes it, and how old every object in a directory under `metadata/` with no manifest must be before `prune` deletes that directory. A backup uploads its blobs and its `db.zst.age` before the manifest that lists them, so on a destination shared between hosts or users this must be longer than the longest backup run (see [locking](#locking)). It does not protect a blob a backup reuses from an earlier run that stopped before its manifest: that blob keeps the time of its first upload, so a prune can delete it. A duration such as `36h` or `90m`; `0s` deletes such blobs and directories at once |
 | `hostname` | system hostname | Hostname used in snapshot IDs |
 | `index_path` | platform data dir | Local SQLite index path |
 

config.example.yml

@@ -332,13 +332,13 @@ compression_level: 5
 # How old a blob that no snapshot lists must be before prune deletes it, and
 # how old every object in a directory under metadata/ with no manifest must
 # be before prune deletes that directory
-# A backup uploads its blobs and its db.zst.age before its manifest, so on a
+# A backup uploads its blobs and its db.zst.age before the manifest that
-# destination shared between hosts or users this must be longer than the
+# lists them, so on a destination shared between hosts or users this must be
-# longest backup run
+# longer than the longest backup run
 # It does not protect a blob a backup reuses from an earlier run that stopped
 # before its manifest: that blob keeps the time of its first upload, so a
 # prune can delete it
-# Supports: 36h, 90m, etc.; 0s deletes such blobs and directories at once
+# Supports: 36h, 90m, etc.; 0s deletes them at once
 # Default: 24h
 #prune_grace_period: 24h
 

docs/REPOSTRUCTURE.md

@@ -166,7 +166,7 @@ The prune operation is safe because:
 1. It keeps every blob listed in any snapshot's manifest and deletes only blobs that no manifest references. Under `metadata/` it deletes only a directory that has no `manifest.json.zst`, whose name is a remote key, and that no local snapshot record claims
 2. Manifests are unencrypted and can be read without keys
 3. If any manifest cannot be downloaded or decoded, prune deletes nothing and exits with an error, rather than treating that snapshot's blobs as unreferenced
-4. It deletes an unreferenced blob only once the blob is older than `prune_grace_period` (default `24h`), and a directory under `metadata/` with no manifest only once every object in it is. A backup uploads its blobs and its `db.zst.age` before its manifest, so a prune running meanwhile, on this host or another sharing the destination, leaves them as long as no backup run takes longer than the grace period. The grace period does not protect a blob a backup reuses from an earlier run that stopped before its manifest: the backup skips the upload of a blob already on the destination, so that blob keeps the time of its first upload, and a prune before the backup's manifest is written can delete it; the new snapshot then cannot be restored. `remote nuke` has no such check
+4. It deletes an unreferenced blob only once the blob is older than `prune_grace_period` (default `24h`), and a directory under `metadata/` with no manifest only once every object in it is. A backup uploads its blobs and its `db.zst.age` before the manifest that lists them, so a prune running meanwhile, on this host or another sharing the destination, leaves them as long as no backup run takes longer than the grace period. The grace period does not protect a blob a backup reuses from an earlier run that stopped before its manifest: the backup skips the upload of a blob already on the destination, so that blob keeps the time of its first upload, and a prune before the backup's manifest is written can delete it; the new snapshot then cannot be restored. `remote nuke` has no such check
 
 ## Restoration Requirements
 

internal/cli/config.go

@@ -242,12 +242,12 @@ storage_url: ""
 # How old a blob that no snapshot lists must be before prune deletes it,
 # and how old every object in a directory under metadata/ with no manifest
 # must be before prune deletes that directory. A backup uploads its blobs
-# and its db.zst.age before its manifest, so on a destination shared
+# and its db.zst.age before the manifest that lists them, so on a
-# between hosts or users this must be longer than the longest backup run.
+# destination shared between hosts or users this must be longer than the
-# It does not protect a blob a backup reuses from an earlier run that
+# longest backup run. It does not protect a blob a backup reuses from an
-# stopped before its manifest: that blob keeps the time of its first
+# earlier run that stopped before its manifest: that blob keeps the time of
-# upload, so a prune can delete it.
+# its first upload, so a prune can delete it.
-# Accepts: 36h, 90m, etc. 0s deletes such blobs and directories at once.
+# Accepts: 36h, 90m, etc. 0s deletes them at once.
 # Default: 24h
 # prune_grace_period: 24h
 

internal/cli/prune.go

@@ -13,9 +13,8 @@ func NewPruneCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "prune",
 		Short: "Tidy local database and remote storage",
-		Long: `Removes orphaned data from the local index database, and
+		Long: `Removes orphaned data from both the local index database and
-unreferenced blobs and directories under metadata/ with no manifest
+unreferenced blobs from the backup destination store.
-from the backup destination store.
 
 Local cleanup drops incomplete snapshots and any files, chunks, or
 blobs no longer referenced by a completed snapshot. Remote cleanup

internal/config/config_test.go

@@ -415,8 +415,7 @@ func TestDocumentedSizeExamples(t *testing.T) {
 
 // TestValidatePruneGracePeriod checks that a negative prune_grace_period
 // is refused rather than acting like 0s, which deletes unreferenced blobs
-// and directories under metadata/ with no manifest at once, and that 0s
+// at once, and that 0s and the documented examples are accepted.
-// and the documented examples are accepted.
 func TestValidatePruneGracePeriod(t *testing.T) {
 	t.Parallel()
 

internal/storage/s3_test.go

@@ -100,9 +100,8 @@ func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
 // TestS3ListingCutShortIsAnError checks that List and ListStream fail on a
 // listing page marked truncated that carries no continuation token, as a
 // server answering V2 listings V1-style sends it. Taken as the last page,
-// it would hide every later key: prune would treat the blobs of the
+// it would hide every later key, and prune would treat the blobs of the
-// snapshots listed there as unreferenced, and a directory under metadata/
+// snapshots listed there as unreferenced.
-// whose manifest is listed there as one with no manifest.
 func TestS3ListingCutShortIsAnError(t *testing.T) {
 	t.Parallel()
 

internal/vaultik/prune.go

@@ -184,13 +184,12 @@ type PruneBlobsResult struct {
 	MetadataDirsFailed  int `json:"metadata_dirs_failed,omitempty"`
 }
 
-// Prune removes orphaned data from the local index database AND, from the
+// Prune removes orphaned data from the local index database AND
-// backup destination store, unreferenced blobs and the directories under
+// unreferenced blobs from the backup destination store. This is the
-// metadata/ with no manifest. This is the single user-facing prune entry
+// single user-facing prune entry point — the split between local and
-// point — the split between local and remote cleanup is an
+// remote cleanup is an implementation detail. Calling code should
-// implementation detail. Calling code should prefer this method over
+// prefer this method over PruneDatabase or PruneBlobs individually
-// PruneDatabase or PruneBlobs individually unless it specifically wants
+// unless it specifically wants one half.
-// one half.
 func (v *Vaultik) Prune(opts *PruneOptions) error {
 	err := v.EnsureStorageBinding()
 	if err != nil {
@@ -297,11 +296,9 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
 	return errors.Join(failures...)
 }
 
-// confirmPrune states what prune found to delete, unreferenced blobs and
+// confirmPrune states what prune found to delete, naming each directory
-// directories under metadata/ with no manifest, naming each directory by
+// under metadata/ by its remote key, and, unless --force or --json is
-// its remote key, and, unless --force is given, asks the operator to
+// given, asks the operator to confirm. It reports whether to go ahead.
-// confirm. Under --json it prints nothing and does not ask. It reports
-// whether to go ahead.
 func (v *Vaultik) confirmPrune(
 	opts *PruneOptions, blobCount int, blobsSize int64, dirs []string,
 ) bool {

internal/vaultik/prune_grace_period_test.go

@@ -24,9 +24,8 @@ import (
 
 // TestPruneDuringBackupKeepsItsBlobs runs a prune from another host
 // sharing the destination store at the moment a backup has uploaded its
-// blobs and its db.zst.age but not yet its manifest, so no manifest lists
+// blobs but not yet its manifest, so no manifest lists those blobs. The
-// those blobs. The prune must leave the blobs and the db.zst.age, and the
+// prune must leave them, and the backup must then restore.
-// backup must then restore.
 func TestPruneDuringBackupKeepsItsBlobs(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
@@ -80,8 +79,7 @@ func TestPruneDuringBackupKeepsItsBlobs(t *testing.T) {
 		SnapshotID: id,
 		TargetDir:  restoreDir,
 		Verify:     true,
-	}), "a prune during the backup must not delete the backup's blobs "+
+	}), "a prune during the backup must not delete the backup's blobs")
-		"or its db.zst.age")
 
 	assertRestoredTree(t, fs, restoreDir, testFiles)
 }

internal/vaultik/snapshot.go

@@ -219,10 +219,9 @@ func (v *Vaultik) snapshotNamesToCreate(requested []string) ([]string, error) {
 }
 
 // runPostBackupPrune drops older snapshots of the given names and removes
-// from remote storage the orphan blobs and the directories under
+// orphan blobs from remote storage. If keepNewerThan is set (e.g. "4w"),
-// metadata/ with no manifest, as PruneBlobs does. If keepNewerThan is set
+// snapshots newer than that duration are kept. Otherwise only the latest
-// (e.g. "4w"), snapshots newer than that duration are kept. Otherwise only
+// snapshot of each name is kept.
-// the latest snapshot of each name is kept.
 func (v *Vaultik) runPostBackupPrune(
 	snapshotNames []string, keepNewerThan string,
 ) error {
@@ -249,7 +248,7 @@ func (v *Vaultik) runPostBackupPrune(
 
 	err = v.PruneBlobs(&PruneOptions{Force: true})
 	if err != nil {
-		return fmt.Errorf("pruning the destination store: %w", err)
+		return fmt.Errorf("pruning orphaned blobs: %w", err)
 	}
 
 	return nil