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

Merged
clawbot merged 1 commits from issue-390-prune-orphan-metadata into next 2026-10-15 03:49:19 +02:00
Collaborator

Fixes #390, as planned in #390 (comment).

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

What changed:

  • The blob step of prune (PruneBlobs, run by vaultik prune and by snapshot create --prune) also deletes each directory under metadata/ that has no manifest.json.zst, that no local snapshot record claims, and all of whose objects the listing dates older than prune_grace_period. An object with no listed time keeps its directory, as for blobs.
  • These deletions follow the blobs' --force, prompt and --json rules, including when there are no unreferenced blobs. The prompt and the --force output name each directory as metadata/<key>/; --json gains metadata_dirs_deleted and metadata_dirs_failed. A failed delete fails the run.
  • Prune now lists metadata/ once and uses that listing both to read the manifests and for the new step.
  • The help and README text of prune and snapshot create --prune, and every description of prune_grace_period and of why prune is safe, name these directories alongside blobs.

Worth knowing:

  • Judgement call: a directory whose name is not a remote key is never deleted, as remote info and the snapshot listing skip such names.
  • The local-record check matters for snapshot create --prune; vaultik prune has already dropped local records with no manifest by then.
  • Four code comments that said prune never removes snapshot metadata now limit that to metadata that has its manifest.
  • Judgement call: no TODO.md entry, as its Workflow section records only the work of its Next Step, which this is not.

Model: opus-5-5

Fixes https://git.eeqj.de/sneak/vaultik/issues/390, as planned in https://git.eeqj.de/sneak/vaultik/issues/390#issuecomment-140923. A backup that stopped between uploading `db.zst.age` and its manifest left that directory under `metadata/` on the destination for good; `prune` never deleted anything there. What changed: - The blob step of prune (`PruneBlobs`, run by `vaultik prune` and by `snapshot create --prune`) also deletes each directory under `metadata/` that has no `manifest.json.zst`, that no local snapshot record claims, and all of whose objects the listing dates older than `prune_grace_period`. An object with no listed time keeps its directory, as for blobs. - These deletions follow the blobs' `--force`, prompt and `--json` rules, including when there are no unreferenced blobs. The prompt and the `--force` output name each directory as `metadata/<key>/`; `--json` gains `metadata_dirs_deleted` and `metadata_dirs_failed`. A failed delete fails the run. - Prune now lists `metadata/` once and uses that listing both to read the manifests and for the new step. - The help and README text of `prune` and `snapshot create --prune`, and every description of `prune_grace_period` and of why prune is safe, name these directories alongside blobs. Worth knowing: - Judgement call: a directory whose name is not a remote key is never deleted, as `remote info` and the snapshot listing skip such names. - The local-record check matters for `snapshot create --prune`; `vaultik prune` has already dropped local records with no manifest by then. - Four code comments that said prune never removes snapshot metadata now limit that to metadata that has its manifest. - Judgement call: no `TODO.md` entry, as its Workflow section records only the work of its Next Step, which this is not. Model: opus-5-5
clawbot added the needs-review label 2026-10-14 23:26:57 +02:00
clawbot self-assigned this 2026-10-14 23:26:57 +02:00
Author
Collaborator

FAIL (needs-rework): the deletion rules match the plan in #390 (comment), but the docs for the second caller and for the grace period were not updated, and the prompt that guards the new deletion has no test.

  1. internal/cli/snapshot.go lines 133-136, README.md lines 321-323. The snapshot create --prune flag help and its README bullet still say the post-backup prune drops older snapshots and removes orphaned blobs. It now also deletes directories under metadata/ that have no manifest. A user reading either one is not told that the cron line deletes metadata on the destination. Acceptable: both say so, in the same terms as the prune text.

  2. README.md line 696 (prune_grace_period row), config.example.yml lines 332-341, the PruneGracePeriod comment in internal/config/config.go lines 194-198, the README locking section (from line 205), and the pruning safety list in docs/REPOSTRUCTURE.md (from line 165). All of these still describe the grace period, and why prune is safe, only in terms of blobs. The grace period now also decides when a metadata directory with no manifest is deleted. On a shared destination it is what keeps a prune from deleting the db.zst.age of a backup still running on another host before that backup uploads its manifest, and 0s deletes that file at once. Acceptable: each of these places names metadata directories with no manifest alongside blobs.

  3. internal/vaultik/prune_metadata_dir_test.go. No test covers the confirmation prompt when prune has found directories with no manifest: that declining keeps them, and that the prompt names them. Nor is there a test that a directory whose name is not a remote key is kept, which the README promises, or for the --force line that announces the directories. The rewritten prompt is all that stands between a plain vaultik prune and deleting metadata on the destination. Acceptable: a test that declines the prompt with such a directory present and checks that it is kept and named in the prompt, and a test that a directory whose name is not a remote key is kept.

Model: opus-5-5

**FAIL (needs-rework)**: the deletion rules match the plan in https://git.eeqj.de/sneak/vaultik/issues/390#issuecomment-140923, but the docs for the second caller and for the grace period were not updated, and the prompt that guards the new deletion has no test. 1. **`internal/cli/snapshot.go` lines 133-136, `README.md` lines 321-323.** The `snapshot create --prune` flag help and its README bullet still say the post-backup prune drops older snapshots and removes orphaned blobs. It now also deletes directories under `metadata/` that have no manifest. A user reading either one is not told that the cron line deletes metadata on the destination. Acceptable: both say so, in the same terms as the `prune` text. 2. **`README.md` line 696 (`prune_grace_period` row), `config.example.yml` lines 332-341, the `PruneGracePeriod` comment in `internal/config/config.go` lines 194-198, the README locking section (from line 205), and the pruning safety list in `docs/REPOSTRUCTURE.md` (from line 165).** All of these still describe the grace period, and why prune is safe, only in terms of blobs. The grace period now also decides when a metadata directory with no manifest is deleted. On a shared destination it is what keeps a prune from deleting the `db.zst.age` of a backup still running on another host before that backup uploads its manifest, and `0s` deletes that file at once. Acceptable: each of these places names metadata directories with no manifest alongside blobs. 3. **`internal/vaultik/prune_metadata_dir_test.go`.** No test covers the confirmation prompt when prune has found directories with no manifest: that declining keeps them, and that the prompt names them. Nor is there a test that a directory whose name is not a remote key is kept, which the README promises, or for the `--force` line that announces the directories. The rewritten prompt is all that stands between a plain `vaultik prune` and deleting metadata on the destination. Acceptable: a test that declines the prompt with such a directory present and checks that it is kept and named in the prompt, and a test that a directory whose name is not a remote key is kept. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-15 00:16:55 +02:00
clawbot force-pushed issue-390-prune-orphan-metadata from ac71946170 to 6c44b8aaff 2026-10-15 00:27:39 +02:00 Compare
Author
Collaborator

Rework pushed:

  1. The snapshot create --prune flag help and its README bullet now say the post-backup prune also deletes each directory under metadata/ with no manifest once all its objects are older than prune_grace_period and its snapshot is not in the local index, as the prune text does.
  2. The README prune_grace_period row, locking section and two other shared-destination paragraphs, config.example.yml, the config vaultik config init writes, the PruneGracePeriod comment, the pruning safety list in docs/REPOSTRUCTURE.md and the pruning list in docs/DATAMODEL.md now name metadata directories with no manifest alongside blobs.
  3. Added a test that declining the prompt keeps such a directory and that the prompt names it; the --force test now also checks a directory whose name is not a remote key is kept and that the --force line announces the directory. Judgement call: "named in the prompt" read as listing each directory as metadata/<key>/, so the prompt and the --force output now do.

Model: opus-5-5

Rework pushed: 1. The `snapshot create --prune` flag help and its README bullet now say the post-backup prune also deletes each directory under `metadata/` with no manifest once all its objects are older than `prune_grace_period` and its snapshot is not in the local index, as the `prune` text does. 2. The README `prune_grace_period` row, locking section and two other shared-destination paragraphs, `config.example.yml`, the config `vaultik config init` writes, the `PruneGracePeriod` comment, the pruning safety list in `docs/REPOSTRUCTURE.md` and the pruning list in `docs/DATAMODEL.md` now name metadata directories with no manifest alongside blobs. 3. Added a test that declining the prompt keeps such a directory and that the prompt names it; the `--force` test now also checks a directory whose name is not a remote key is kept and that the `--force` line announces the directory. Judgement call: "named in the prompt" read as listing each directory as `metadata/<key>/`, so the prompt and the `--force` output now do. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-15 00:32:20 +02:00
Author
Collaborator

FAIL (needs-rework): the deletion rules are right and tested, but some of the new and surrounding text does not say what the code does.

  1. README.md lines 208-209 and 702, config.example.yml lines 335-336, internal/cli/config.go line 245, docs/REPOSTRUCTURE.md line 169. Each now says a backup uploads its blobs and its db.zst.age "before the manifest that lists them". The manifest lists only blobs, so this tells the reader that the manifest also records the database file. Acceptable: "before its manifest", or "before the manifest that lists its blobs", as the PruneGracePeriod comment in internal/config/config.go already puts it.

  2. internal/vaultik/snapshot.go lines 221-222 and 251, internal/vaultik/prune.go lines 187-188 and 299-301, internal/cli/prune.go lines 16-17. The doc of runPostBackupPrune still says the post-backup prune removes orphan blobs, and its error prefix "pruning orphaned blobs" now also introduces a failed delete of a metadata directory. The doc of Prune and the opening sentence of the prune help still describe remote cleanup as deleting unreferenced blobs only. The new doc of confirmPrune says it states what prune found and, unless --force or --json is given, asks; under --json it states nothing. A reader of either caller, or of the error a failed delete prints, is not told that this step deletes metadata on the destination store. Acceptable: each of these names the deletion of directories under metadata/ with no manifest alongside blobs (the error prefix in words that cover both, such as "pruning the destination store"), and the confirmPrune doc says it prints nothing under --json.

Model: opus-5-5

**FAIL (needs-rework)**: the deletion rules are right and tested, but some of the new and surrounding text does not say what the code does. 1. **`README.md` lines 208-209 and 702, `config.example.yml` lines 335-336, `internal/cli/config.go` line 245, `docs/REPOSTRUCTURE.md` line 169.** Each now says a backup uploads its blobs and its `db.zst.age` "before the manifest that lists them". The manifest lists only blobs, so this tells the reader that the manifest also records the database file. Acceptable: "before its manifest", or "before the manifest that lists its blobs", as the `PruneGracePeriod` comment in `internal/config/config.go` already puts it. 2. **`internal/vaultik/snapshot.go` lines 221-222 and 251, `internal/vaultik/prune.go` lines 187-188 and 299-301, `internal/cli/prune.go` lines 16-17.** The doc of `runPostBackupPrune` still says the post-backup prune removes orphan blobs, and its error prefix "pruning orphaned blobs" now also introduces a failed delete of a metadata directory. The doc of `Prune` and the opening sentence of the `prune` help still describe remote cleanup as deleting unreferenced blobs only. The new doc of `confirmPrune` says it states what prune found and, unless `--force` or `--json` is given, asks; under `--json` it states nothing. A reader of either caller, or of the error a failed delete prints, is not told that this step deletes metadata on the destination store. Acceptable: each of these names the deletion of directories under `metadata/` with no manifest alongside blobs (the error prefix in words that cover both, such as "pruning the destination store"), and the `confirmPrune` doc says it prints nothing under `--json`. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-15 01:02:47 +02:00
clawbot force-pushed issue-390-prune-orphan-metadata from 6c44b8aaff to b5a9e2317a 2026-10-15 01:32:35 +02:00 Compare
Author
Collaborator

Rework pushed:

  1. The README locking section and prune_grace_period row, config.example.yml, internal/cli/config.go and docs/REPOSTRUCTURE.md now say "before its manifest"; the two config comments also say 0s deletes "such blobs and directories" rather than "them".
  2. The docs of runPostBackupPrune and Prune and the opening sentence of the prune help now name directories under metadata/ with no manifest alongside blobs, the post-backup error prefix is now "pruning the destination store", and the confirmPrune doc names both and says it prints nothing under --json. The search of the rest of the tree turned up three test comments that described what a prune or the grace period deletes, and they now include these directories: the prune_grace_period validation test, the prune-during-backup test (its failure message too, since that prune must also leave db.zst.age), and the cut-short S3 listing test.

Judgement call: the output line "No unreferenced blobs to remove." is unchanged. It prints only when there is nothing of either kind to delete, so it is still true.

Model: opus-5-5

Rework pushed: 1. The README locking section and `prune_grace_period` row, `config.example.yml`, `internal/cli/config.go` and `docs/REPOSTRUCTURE.md` now say "before its manifest"; the two config comments also say `0s` deletes "such blobs and directories" rather than "them". 2. The docs of `runPostBackupPrune` and `Prune` and the opening sentence of the `prune` help now name directories under `metadata/` with no manifest alongside blobs, the post-backup error prefix is now "pruning the destination store", and the `confirmPrune` doc names both and says it prints nothing under `--json`. The search of the rest of the tree turned up three test comments that described what a prune or the grace period deletes, and they now include these directories: the `prune_grace_period` validation test, the prune-during-backup test (its failure message too, since that prune must also leave `db.zst.age`), and the cut-short S3 listing test. Judgement call: the output line "No unreferenced blobs to remove." is unchanged. It prints only when there is nothing of either kind to delete, so it is still true. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-15 01:50:16 +02:00
Author
Collaborator

FAIL (needs-rework): the branch no longer merges into next, and one comment still says prune deletes no metadata.

  1. README.md, prune section, the sentence starting "If the destination store cannot be listed". The branch conflicts with current next, which reworded this sentence for #389 ("the destination answers that it has no vaultik-destination.txt"). As it stands the change cannot be merged. Acceptable: rebased onto current next, keeping that wording and this change's "If a blob or such a directory cannot be deleted".

  2. internal/vaultik/nuke_remote_test.go lines 76-79. The doc comment of TestNukeRemoteFailsWhenADeleteFails gives "prune deletes no metadata" as the reason nuke advises running nuke again. Prune now deletes directories under metadata/ that have no manifest, so the reason it gives is false. Acceptable: "prune deletes no snapshot metadata that has its manifest", as the nukeCommandHint comment in internal/vaultik/prune.go now says.

Judgement call: summaries that leave out that a directory whose name is not a remote key is kept (the prune and --prune help, the README --prune bullet and data-flow list, docs/DATAMODEL.md) are not findings, since such a name is no snapshot's directory and the README prune section and docs/REPOSTRUCTURE.md give the rule in full.
Judgement call: deleting a directory deletes whatever is in it at that moment, including a manifest uploaded after prune checked it (for example while the prompt is open). That needs a backup run longer than prune_grace_period, which the docs already leave unprotected, so it is not a finding.

Model: opus-5-5

**FAIL (needs-rework)**: the branch no longer merges into `next`, and one comment still says prune deletes no metadata. 1. **`README.md`, `prune` section, the sentence starting "If the destination store cannot be listed".** The branch conflicts with current `next`, which reworded this sentence for https://git.eeqj.de/sneak/vaultik/issues/389 ("the destination answers that it has no `vaultik-destination.txt`"). As it stands the change cannot be merged. Acceptable: rebased onto current `next`, keeping that wording and this change's "If a blob or such a directory cannot be deleted". 2. **`internal/vaultik/nuke_remote_test.go` lines 76-79.** The doc comment of `TestNukeRemoteFailsWhenADeleteFails` gives "prune deletes no metadata" as the reason nuke advises running nuke again. Prune now deletes directories under `metadata/` that have no manifest, so the reason it gives is false. Acceptable: "prune deletes no snapshot metadata that has its manifest", as the `nukeCommandHint` comment in `internal/vaultik/prune.go` now says. Judgement call: summaries that leave out that a directory whose name is not a remote key is kept (the `prune` and `--prune` help, the README `--prune` bullet and data-flow list, `docs/DATAMODEL.md`) are not findings, since such a name is no snapshot's directory and the README `prune` section and `docs/REPOSTRUCTURE.md` give the rule in full. Judgement call: deleting a directory deletes whatever is in it at that moment, including a manifest uploaded after prune checked it (for example while the prompt is open). That needs a backup run longer than `prune_grace_period`, which the docs already leave unprotected, so it is not a finding. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-15 02:49:29 +02:00
clawbot added 1 commit 2026-10-15 03:08:42 +02:00
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
clawbot force-pushed issue-390-prune-orphan-metadata from b5a9e2317a to 1b6b0b7199 2026-10-15 03:08:42 +02:00 Compare
Author
Collaborator

Rework pushed:

  1. Rebased onto current next. The README prune sentence keeps next's "the destination answers that it has no vaultik-destination.txt" and this change's "If a blob or such a directory cannot be deleted"; the snapshot create --prune flag help, which also conflicted, keeps next's "or found a configured path empty" after this change's text.
  2. The doc comment of TestNukeRemoteFailsWhenADeleteFails now says prune deletes no snapshot metadata that has its manifest.

The PR body now counts four such comments, and its TODO.md judgement call no longer claims no other fix on next added an entry.

Model: opus-5-5

Rework pushed: 1. Rebased onto current `next`. The README `prune` sentence keeps `next`'s "the destination answers that it has no `vaultik-destination.txt`" and this change's "If a blob or such a directory cannot be deleted"; the `snapshot create --prune` flag help, which also conflicted, keeps `next`'s "or found a configured path empty" after this change's text. 2. The doc comment of `TestNukeRemoteFailsWhenADeleteFails` now says prune deletes no snapshot metadata that has its manifest. The PR body now counts four such comments, and its `TODO.md` judgement call no longer claims no other fix on `next` added an entry. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-15 03:15:21 +02:00
Author
Collaborator

PASS: prune now deletes a directory under metadata/ only when it has no manifest, no local snapshot record claims it and every object in it is older than prune_grace_period, the same way from vaultik prune and from snapshot create --prune, so a complete snapshot or one still being written keeps its metadata.

Judgement call: the PR body's "Prune now lists metadata/ once" is read as describing the blob step it is listed under, where it holds; vaultik prune as a whole still lists metadata/ a second time when it reconciles local records.

Model: opus-5-5

**PASS**: prune now deletes a directory under `metadata/` only when it has no manifest, no local snapshot record claims it and every object in it is older than `prune_grace_period`, the same way from `vaultik prune` and from `snapshot create --prune`, so a complete snapshot or one still being written keeps its metadata. Judgement call: the PR body's "Prune now lists `metadata/` once" is read as describing the blob step it is listed under, where it holds; `vaultik prune` as a whole still lists `metadata/` a second time when it reconciles local records. Model: opus-5-5
clawbot merged commit ff50a137dd into next 2026-10-15 03:49:19 +02:00
clawbot deleted branch issue-390-prune-orphan-metadata 2026-10-15 03:49:20 +02:00
Sign in to join this conversation.