From 65036bba1b2fd3afb1616a5ee2d86ff10c1f32a2 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 14 Oct 2026 04:29:36 +0200 Subject: [PATCH] Make snapshot list report a changed destination instead of advising prune (closes #317) When the local index is bound to a different destination than the configured one, snapshot list now gives the warning prune, snapshot purge and snapshot remove refuse with: both destinations and the two ways out (revert the configuration, or run vaultik database delete). Below the table it replaces the local-only note and its prune advice. With --json it goes to stderr through the logger, so stdout stays the document, whose local rows show remote_present false. Reading the binding moves out of EnsureStorageBinding into storageBinding, which records nothing, so the read-only list can call it without binding a fresh index. Model: opus-5-5 --- README.md | 14 ++++-- internal/vaultik/snapshot_list.go | 37 ++++++++++++-- internal/vaultik/snapshot_list_test.go | 68 ++++++++++++++++++++++++++ internal/vaultik/storage_bind.go | 33 +++++++++---- 4 files changed, 134 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 17ff3b9..f0cc9ba 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,11 @@ read from the manifest. Snapshots in the local index with no counterpart on the destination store are reported below the table as drift, with the `vaultik prune` -invocation that reconciles them. +invocation that reconciles them. If the local index is bound to a +different destination than the configured one (after a change to +`storage_url`, or `s3.endpoint`, `s3.bucket` or `s3.prefix`), the +command says so instead and gives the same two ways out that `prune` +gives: revert the configuration, or run `vaultik database delete`. If the destination store cannot be listed (unmounted volume, permission denied, network down), the command warns, falls back to the @@ -332,9 +336,11 @@ local index alone, and still exits zero. (whether the snapshot is in the local index), `remote_key` (the full 64-character storage key), and `remote_present` (whether it was seen on the destination store, or `null` if the destination could not be - listed). Warnings about an unlistable destination, unreadable - manifests, and a truncated listing all go to stderr through the - logger, so stdout stays a single parseable document. + listed). Warnings about an unlistable destination, a local index + bound to a different destination (with both destinations and the + same two ways out), unreadable manifests, and a truncated listing all + go to stderr through the logger, so stdout stays a single parseable + document. **`snapshot verify`**: Verify snapshot integrity. * Default (shallow): checks that every blob the manifest lists is present in diff --git a/internal/vaultik/snapshot_list.go b/internal/vaultik/snapshot_list.go index 8ed7b6b..7862e2e 100644 --- a/internal/vaultik/snapshot_list.go +++ b/internal/vaultik/snapshot_list.go @@ -61,7 +61,9 @@ const tabPadding = 3 // attempt is made to recover or fabricate the human ID. // // Snapshots in the local index with no counterpart on the destination -// store are reported as drift below the table. +// store are reported as drift below the table, unless the local index +// is bound to a different destination than the configured one: that is +// reported instead. // // If remote listing fails (unmounted volume, permission denied, // network), we degrade to local-only with a warning. List never fails @@ -103,7 +105,26 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error { return snapshots[i].Timestamp.After(snapshots[j].Timestamp) }) + stored, configured, err := v.storageBinding() + if err != nil { + return err + } + + // A local index bound to another destination describes that one, so + // its records are expected to be missing here, and prune refuses to + // run. The binding is reported instead of advising prune. + boundElsewhere := stored != "" && stored != configured + if jsonOutput { + // stdout holds only the document, so this goes to the logger. + if boundElsewhere { + log.Warn("Local index is bound to a different backup "+ + "destination; to proceed, revert storage_url (or the s3.* "+ + "endpoint, bucket and prefix) to the bound destination, "+ + "or run 'vaultik database delete'", + "bound_to", stored, "configured", configured) + } + if remoteErr == nil { v.reportJSONListingLimits(listing) } @@ -125,8 +146,13 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error { } } + if boundElsewhere { + v.UI.Warningf("%v\n%s", errStorageBindingMismatch, + buildBindingMismatchMessage(stored, configured)) + } + if remoteErr == nil { - v.reportListDrift(snapshots, listing) + v.reportListDrift(snapshots, listing, boundElsewhere) } return nil @@ -432,7 +458,9 @@ func (v *Vaultik) snapshotInfoFromLocal(ls *database.Snapshot) SnapshotInfo { // reportListDrift prints the reconciliation notes the merged table // cannot express on its own: local records with no counterpart on the // destination store, plus counts of remote snapshots that were -// unreadable or omitted. +// unreadable or omitted. Local records are not reported when +// boundElsewhere is set: ListSnapshots has already said why they are +// missing. // // This is what remains of the old reportRemoteDrift, and it no longer // touches the destination store. Its remote-only half collapsed into @@ -441,6 +469,7 @@ func (v *Vaultik) snapshotInfoFromLocal(ls *database.Snapshot) SnapshotInfo { // so `snapshot list` lists the destination exactly once per invocation. func (v *Vaultik) reportListDrift( snapshots []SnapshotInfo, listing *remoteSnapshotListing, + boundElsewhere bool, ) { var localOnly []string @@ -450,7 +479,7 @@ func (v *Vaultik) reportListDrift( } } - if len(localOnly) > 0 { + if len(localOnly) > 0 && !boundElsewhere { v.UI.Warningf("%d local snapshot record(s) not found in backup "+ "destination store:", len(localOnly)) diff --git a/internal/vaultik/snapshot_list_test.go b/internal/vaultik/snapshot_list_test.go index 2dd88e4..ee60b3c 100644 --- a/internal/vaultik/snapshot_list_test.go +++ b/internal/vaultik/snapshot_list_test.go @@ -421,6 +421,74 @@ func TestListSnapshots_LocalOnlyReportedAsDrift(t *testing.T) { assert.NotContains(t, out, "snapshot cleanup") } +// TestListSnapshots_BoundElsewhereDoesNotAdvisePrune covers a changed +// storage_url: the local index is still bound to the old destination, +// so its records are missing from the new one. prune refuses to run in +// that state, so the listing must give the choices prune gives instead +// of advising it. +func TestListSnapshots_BoundElsewhereDoesNotAdvisePrune(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + const ( + oldURL = "file:///mnt/backups/old" + newURL = "file:///mnt/backups/new" + ) + + env := newListEnv(t) + env.v.Config.StorageURL = newURL + require.NoError(t, env.v.Repositories.LocalMeta.Set(context.Background(), + database.LocalMetaKeyStorageURL, oldURL)) + env.addLocal(t, listLocalID, time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC)) + + err := env.v.ListSnapshots(false) + require.NoError(t, err) + + out := env.stdout.String() + + assert.Contains(t, out, listLocalID) + assert.Contains(t, out, + "local index is bound to a different backup destination") + assert.Contains(t, out, oldURL) + assert.Contains(t, out, newURL) + assert.Contains(t, out, "revert storage_url") + assert.Contains(t, out, "vaultik database delete") + assert.NotContains(t, out, "vaultik prune") +} + +// TestListSnapshots_JSONReportsBoundElsewhere is the --json side of the +// test above. Every local row comes out with remote_present false, which +// a script reads as stale local records unless it is told, on stderr, +// that the local index is bound to a different destination. +// +//nolint:paralleltest // captureProcessStderr replaces os.Stderr +func TestListSnapshots_JSONReportsBoundElsewhere(t *testing.T) { + const ( + oldURL = "file:///mnt/backups/old" + newURL = "file:///mnt/backups/new" + ) + + env := newListEnv(t) + env.v.Config.StorageURL = newURL + require.NoError(t, env.v.Repositories.LocalMeta.Set(context.Background(), + database.LocalMetaKeyStorageURL, oldURL)) + env.addLocal(t, listLocalID, time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC)) + + stderr := captureProcessStderr(t, func() { + require.NoError(t, env.v.ListSnapshots(true)) + }) + + rows := decodeListJSON(t, env.stdout.String()) + require.Len(t, rows, 1) + assert.Equal(t, listLocalID, rows[0].ID) + + assert.Contains(t, stderr, "bound to a different backup destination") + assert.Contains(t, stderr, oldURL) + assert.Contains(t, stderr, newURL) + assert.Contains(t, stderr, "revert storage_url") + assert.Contains(t, stderr, "vaultik database delete") +} + // TestListSnapshots_UnreachableRemoteDegrades covers the promise in the // doc comment: an unreachable destination is a warning plus local-only // output, never a failure. diff --git a/internal/vaultik/storage_bind.go b/internal/vaultik/storage_bind.go index d9fe081..d72ae34 100644 --- a/internal/vaultik/storage_bind.go +++ b/internal/vaultik/storage_bind.go @@ -45,17 +45,9 @@ func (v *Vaultik) EnsureStorageBinding() error { return nil } - configured := v.Config.StorageURL - if configured == "" { - // The s3.* form: bind on its endpoint, bucket and prefix, - // written the way storage_url would name the same destination. - s3 := v.Config.S3 - configured = "s3://" + s3.Bucket + "/" + s3.Prefix + "?endpoint=" + s3.Endpoint - } - - stored, err := v.Repositories.LocalMeta.Get(v.ctx, database.LocalMetaKeyStorageURL) + stored, configured, err := v.storageBinding() if err != nil { - return fmt.Errorf("reading local storage binding: %w", err) + return err } if stored == "" { @@ -78,6 +70,27 @@ func (v *Vaultik) EnsureStorageBinding() error { errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured)) } +// storageBinding returns the destination the local index is bound to +// ("" when no binding is recorded yet), then the configured destination +// written the same way. It records nothing, so a read-only command can +// call it. +func (v *Vaultik) storageBinding() (string, string, error) { + configured := v.Config.StorageURL + if configured == "" { + // The s3.* form: bind on its endpoint, bucket and prefix, + // written the way storage_url would name the same destination. + s3 := v.Config.S3 + configured = "s3://" + s3.Bucket + "/" + s3.Prefix + "?endpoint=" + s3.Endpoint + } + + stored, err := v.Repositories.LocalMeta.Get(v.ctx, database.LocalMetaKeyStorageURL) + if err != nil { + return "", "", fmt.Errorf("reading local storage binding: %w", err) + } + + return stored, configured, nil +} + // buildBindingMismatchMessage assembles the multi-line explanation // shown when the local index is bound to a different destination than // the currently-configured one (the first line lives in the