From 39ca0e3111199974a3e53a58f108cd887bec43fc Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 13 Oct 2026 21:43:17 +0000 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 prints the warning prune, snapshot purge and snapshot remove refuse with, including its two ways out (revert the configuration, or run vaultik database delete). It no longer reports the local records as missing or advises vaultik prune, which would refuse. 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. Judgement call: --json output is unchanged; it gives no advice. Model: opus-5-5 --- README.md | 6 ++++- internal/vaultik/snapshot_list.go | 27 +++++++++++++++++--- internal/vaultik/snapshot_list_test.go | 35 ++++++++++++++++++++++++++ internal/vaultik/storage_bind.go | 33 ++++++++++++++++-------- 4 files changed, 86 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index ee375b0..64b250d 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 diff --git a/internal/vaultik/snapshot_list.go b/internal/vaultik/snapshot_list.go index 8ed7b6b..185c753 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 @@ -125,8 +127,22 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error { } } + 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 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 +448,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 +459,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 +469,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..5368801 100644 --- a/internal/vaultik/snapshot_list_test.go +++ b/internal/vaultik/snapshot_list_test.go @@ -421,6 +421,41 @@ 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_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