diff --git a/README.md b/README.md index 70f5eb4..72097c3 100644 --- a/README.md +++ b/README.md @@ -352,8 +352,9 @@ may hold snapshots this host doesn't know about), which is what prune` invocation to run as a follow-up. Local row cleanup (files, chunks, blobs the snapshot was the last referrer for) runs automatically. If the destination store is unreachable, the local-DB -removal still completes and a warning is emitted; rerun `vaultik prune` -once the store is reachable to finish remote cleanup. To wipe everything +removal still completes and a warning is emitted; run `vaultik snapshot +remove ` again once the store is reachable to remove the +snapshot's metadata from it (`vaultik prune` does not). To wipe everything on the destination in one go, use `vaultik remote nuke --force`. * `--local-only`: Skip remote cleanup; only touch the local index * `--dry-run`: Show what would be deleted without deleting diff --git a/TODO.md b/TODO.md index ec17c26..f280f3d 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,17 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-06: Made `snapshot remove --json` write only its document to + stdout when the destination store cannot be reached + ([issue #251](https://git.eeqj.de/sneak/vaultik/issues/251)). Its + warning that the snapshot's metadata was left on the destination store + went to stdout ahead of the document, breaking `| jq` on a command that + exited 0. Under `--json` the warning now reaches stderr only, through + the logger. The warning, the README and the command's help said + `vaultik prune` would finish the cleanup, but `prune` never removes + snapshot metadata; they now say to run `vaultik snapshot remove` for the + snapshot again once the destination store is reachable. + - 2026-10-06: Made the backup summary and the `snapshots` row count each file, byte and upload once ([issue #225](https://git.eeqj.de/sneak/vaultik/issues/225)). The diff --git a/internal/cli/entry_stdout_stderr_test.go b/internal/cli/entry_stdout_stderr_test.go index 43a6e42..0a8a0e9 100644 --- a/internal/cli/entry_stdout_stderr_test.go +++ b/internal/cli/entry_stdout_stderr_test.go @@ -87,12 +87,33 @@ func TestEntryJSONFailureIsReportedOnStderr(t *testing.T) { } } -// writeUnusableDestinationConfig builds a config whose destination -// directory does not exist, which fails `remote info`, and whose local -// index is bound to another destination, which fails `prune` and -// `snapshot remove` (a missing destination alone only makes `snapshot -// remove` warn). Returns the config path. -func writeUnusableDestinationConfig(t *testing.T) string { +// TestEntrySnapshotRemoveJSONWarningIsOnStderr runs `snapshot remove +// --json` on a snapshot in the local index, against a destination +// directory that does not exist. The command removes the snapshot from +// the local index and still exits 0. Its stdout must hold the document +// alone, with the warning about the destination store, and the command +// to run again once it is reachable, on stderr. +// +//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals +func TestEntrySnapshotRemoveJSONWarningIsOnStderr(t *testing.T) { + configPath, indexPath := writeMissingDestinationConfig(t) + seedStaleSnapshotRecord(t, indexPath) + + code, stdout, stderr := runEntry(t, flagConfig, configPath, + cmdSnapshot, cmdRemove, stalePruneSnapshotID, flagJSON) + + require.Equal(t, 0, code) + requireExactlyOneJSONDocument(t, stdout) + assert.Contains(t, stderr, + "Could not remove snapshot metadata from remote storage") + assert.Contains(t, stderr, + "run 'vaultik snapshot remove "+stalePruneSnapshotID+"' again") +} + +// writeMissingDestinationConfig builds a config whose destination +// directory does not exist. Returns the config path and the path of +// its local index, which is not created here. +func writeMissingDestinationConfig(t *testing.T) (string, string) { t.Helper() dir := t.TempDir() @@ -114,6 +135,19 @@ func writeUnusableDestinationConfig(t *testing.T) string { xdg.Reload() t.Cleanup(xdg.Reload) + return configPath, indexPath +} + +// writeUnusableDestinationConfig builds a config whose destination +// directory does not exist, which fails `remote info`, and whose local +// index is bound to another destination, which fails `prune` and +// `snapshot remove` (a missing destination alone only makes `snapshot +// remove` warn). Returns the config path. +func writeUnusableDestinationConfig(t *testing.T) string { + t.Helper() + + configPath, indexPath := writeMissingDestinationConfig(t) + ctx := context.Background() db, err := database.New(ctx, indexPath) @@ -122,7 +156,7 @@ func writeUnusableDestinationConfig(t *testing.T) string { defer func() { require.NoError(t, db.Close()) }() require.NoError(t, database.NewRepositories(db).LocalMeta.Set(ctx, - database.LocalMetaKeyStorageURL, "file://"+filepath.Join(dir, "other"))) + database.LocalMetaKeyStorageURL, "file://"+t.TempDir())) return configPath } diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index 4a0e095..fb2dc4d 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -258,8 +258,9 @@ Use --local-only to skip the remote half (e.g. when you want to forget a snapshot locally without touching the destination store). If the remote is unreachable, the local-database removal still completes -and a warning is emitted; rerun 'vaultik prune' once the destination store -is reachable to finish remote cleanup. +and a warning is emitted; run 'vaultik snapshot remove ' again +once the destination store is reachable to remove the snapshot's metadata +from it ('vaultik prune' does not). To wipe the entire destination store and start over, use 'vaultik remote nuke --force' — it is the single supported entry point for that.`, diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 82a116e..f7b26c6 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -1146,7 +1146,7 @@ func (v *Vaultik) RemoveSnapshot( } if !opts.LocalOnly { - result.RemoteRemoved = v.removeSnapshotRemote(snapshotID) + result.RemoteRemoved = v.removeSnapshotRemote(snapshotID, opts) } if v.SnapshotManager != nil { @@ -1229,9 +1229,11 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions) // removeSnapshotRemote strips the snapshot's metadata from the // destination store, warning and proceeding on failure: the local-DB // removal has already happened, so the user is told the remote half -// didn't finish and can retry with `vaultik prune` once the destination -// store is reachable. Returns true when the remote removal succeeded. -func (v *Vaultik) removeSnapshotRemote(snapshotID string) bool { +// didn't finish and to run `vaultik snapshot remove` for the snapshot +// again once the destination store is reachable (`vaultik prune` never +// removes snapshot metadata). Returns true when the remote removal +// succeeded. +func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) bool { log.Info("Removing snapshot metadata from remote storage", "snapshot_id", snapshotID) @@ -1239,13 +1241,18 @@ func (v *Vaultik) removeSnapshotRemote(snapshotID string) bool { err := v.deleteRemoteSnapshotByKey(remoteKey) if err != nil { - log.Warn("Could not remove snapshot metadata from remote storage", + removeCommand := "vaultik snapshot remove " + snapshotID + + log.Warn("Could not remove snapshot metadata from remote storage; "+ + "run '"+removeCommand+"' again once the remote is reachable", "error", err) - if v.UI != nil { + // The UI writes to stdout, which under --json holds only the + // document; the log record above is the warning on stderr. + if v.UI != nil && !opts.JSON { v.UI.Warningf("Could not remove snapshot metadata from remote: "+ - "%v. Run '%s' once the remote is reachable to finish cleanup.", - err, pruneCommandHint) + "%v. Run '%s' again once the remote is reachable.", + err, removeCommand) } return false