From a63c729fbc1f7e7b043fd7631242cfd7e48559f0 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 17 Jun 2026 06:56:34 +0200 Subject: [PATCH] Print banner before cobra parsing; route arg errors through ui.Error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two output-style fixes plus a quiet-mode correction. Banner: a manual scan of os.Args in CLIEntry decides whether to suppress the banner (--quiet/-q/--cron), then prints it before cobra parses any arguments. This makes the banner appear even when cobra rejects bad args ("requires at least 2 arg(s)") and on --help — paths that previously skipped PersistentPreRun entirely. The cobra-side hook plumbing (sync.Once, PersistentPreRun, custom HelpFunc) is removed. Errors: rootCmd.SilenceErrors = true so cobra no longer prints its own "Error: " line. Any error returned from Execute() goes through ui.New(os.Stderr).Error(...), giving the documented "🛑 ERROR: " format. A new helper cli.ReportError() formats errors from goroutine paths that can't return through cobra's normal return chain; every CLI command's fx-goroutine error path now calls it alongside the existing structured log.Error so both channels record the failure. Quiet mode: previously --quiet/--cron swapped Vaultik.UI to io.Discard, which silenced Warning and Error messages too — contradicting the documented "suppresses non-error output" semantics. ui.Writer now has a SetQuiet flag that drops Begin/Complete/Info/Notice/Detail/Progress/ Banner only; Warning and Error always emit. Also folds in restore.go cleanups the audit flagged: the hardcoded "WARNING:" prefix on the failed-files block now uses ui.Warning + ui.Detail, the post-restore "Restored N files" line uses ui.Complete, and the "No files found to restore" branch emits both log.Warn and ui.Warning so structured logs continue to capture it under --verbose. --- internal/cli/app.go | 11 +++--- internal/cli/entry.go | 57 +++++++++++++++++++++++++++- internal/cli/info.go | 1 + internal/cli/prune.go | 1 + internal/cli/remote.go | 2 + internal/cli/root.go | 46 ++-------------------- internal/cli/snapshot.go | 7 ++++ internal/cli/snapshot_restore.go | 1 + internal/ui/ui.go | 34 +++++++++++++++++ internal/vaultik/integration_test.go | 4 ++ internal/vaultik/restore.go | 14 ++++--- 11 files changed, 123 insertions(+), 55 deletions(-) diff --git a/internal/cli/app.go b/internal/cli/app.go index 44d1221..c1da377 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "io" "os" "os/signal" "path/filepath" @@ -36,15 +35,17 @@ type AppOptions struct { } // setupGlobals records the startup time and, when an output-suppression -// flag is active, replaces the UI writer with a discarding one so no -// user-facing output is emitted. The startup banner itself is printed -// by the root command's PersistentPreRun (see maybePrintBanner). +// flag is active, marks the UI writer quiet so that Begin/Complete/ +// Info/Notice/Detail/Progress are silenced. Warning and Error are NOT +// silenced — per the documented convention that --quiet suppresses +// non-error output only. The startup banner is printed by CLIEntry +// before cobra parses arguments, gated by the same arg-level check. func setupGlobals(lc fx.Lifecycle, g *globals.Globals, v *vaultik.Vaultik, opts log.LogOptions) { lc.Append(fx.Hook{ OnStart: func(ctx context.Context) error { g.StartTime = time.Now().UTC() if opts.Cron || opts.Quiet { - v.UI = ui.NewWithColor(io.Discard, false) + v.UI.SetQuiet(true) } return nil }, diff --git a/internal/cli/entry.go b/internal/cli/entry.go index 577452e..4b9fed8 100644 --- a/internal/cli/entry.go +++ b/internal/cli/entry.go @@ -2,14 +2,67 @@ package cli import ( "os" + "strings" + "time" + + "sneak.berlin/go/vaultik/internal/globals" + "sneak.berlin/go/vaultik/internal/ui" ) // CLIEntry is the main entry point for the CLI application. -// It creates the root command, executes it, and exits with status 1 -// if an error occurs. This function should be called from main(). +// It prints the startup banner (unless a quiet flag is present in os.Args), +// executes the root cobra command, and routes any returned error through +// the ui.Writer so the user sees a properly formatted "🛑 ERROR:" line. func CLIEntry() { + if !bannerSuppressedInArgs(os.Args[1:]) { + short := globals.Commit + if len(short) > 12 { + short = short[:12] + } + writeStartupBanner(ui.New(os.Stdout), time.Now().UTC(), short) + } + rootCmd := NewRootCommand() + rootCmd.SilenceErrors = true + if err := rootCmd.Execute(); err != nil { + ReportError("%s", err.Error()) os.Exit(1) } } + +// ReportError emits a user-facing error to stderr in the standard +// 🛑 ERROR: format. Use it from goroutine error paths (where returning +// an error to cobra isn't an option) and anywhere else a CLI command +// must surface a failure outside the normal RunE return path. +func ReportError(format string, args ...any) { + ui.New(os.Stderr).Error(format, args...) +} + +// bannerSuppressedInArgs reports whether any of args is a flag that +// should suppress the startup banner (--quiet/-q/--cron). Stops at the +// "--" argument terminator. Recognizes both long forms and short -q, +// including combined short flags like "-qv". +func bannerSuppressedInArgs(args []string) bool { + for _, a := range args { + if a == "--" { + return false + } + switch a { + case "--quiet", "-q", "--cron": + return true + } + if strings.HasPrefix(a, "--quiet=") || strings.HasPrefix(a, "--cron=") { + return true + } + // Combined short flags like -qv or -vq. + if len(a) > 1 && a[0] == '-' && a[1] != '-' { + for _, c := range a[1:] { + if c == 'q' { + return true + } + } + } + } + return false +} diff --git a/internal/cli/info.go b/internal/cli/info.go index 9bc8914..e4aca7e 100644 --- a/internal/cli/info.go +++ b/internal/cli/info.go @@ -47,6 +47,7 @@ func NewInfoCommand() *cobra.Command { if err := v.ShowInfo(); err != nil { if err != context.Canceled { log.Error("Failed to show info", "error", err) + ReportError("Failed to show info: %v", err) os.Exit(1) } } diff --git a/internal/cli/prune.go b/internal/cli/prune.go index 35e6c62..f4d76b6 100644 --- a/internal/cli/prune.go +++ b/internal/cli/prune.go @@ -53,6 +53,7 @@ storage space.`, if err != context.Canceled { if !opts.JSON { log.Error("Prune operation failed", "error", err) + ReportError("Prune failed: %v", err) } os.Exit(1) } diff --git a/internal/cli/remote.go b/internal/cli/remote.go index 9f29819..cde873e 100644 --- a/internal/cli/remote.go +++ b/internal/cli/remote.go @@ -66,6 +66,7 @@ This is destructive and irreversible. Requires --force.`, if err := v.NukeRemote(true); err != nil { if err != context.Canceled { log.Error("Remote nuke failed", "error", err) + ReportError("Remote nuke failed: %v", err) os.Exit(1) } } @@ -129,6 +130,7 @@ func newRemoteInfoCommand() *cobra.Command { if err != context.Canceled { if !jsonOutput { log.Error("Failed to get remote info", "error", err) + ReportError("Failed to get remote info: %v", err) } os.Exit(1) } diff --git a/internal/cli/root.go b/internal/cli/root.go index 3818ffc..ecaee21 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -5,39 +5,11 @@ import ( "os" "path/filepath" "strings" - "sync" - "time" "github.com/adrg/xdg" "github.com/spf13/cobra" - "sneak.berlin/go/vaultik/internal/globals" - "sneak.berlin/go/vaultik/internal/ui" ) -// bannerOnce ensures the banner is printed at most once per process, -// even if multiple cobra hooks (PersistentPreRun, Help, Run) would -// otherwise each call maybePrintBanner. -var bannerOnce sync.Once - -// maybePrintBanner prints the application banner unless an output- -// suppression flag is active. Safe to call multiple times — it prints -// at most once per process. -func maybePrintBanner(cmd *cobra.Command) { - if rootFlags.Quiet { - return - } - if cronFlag := cmd.Flags().Lookup("cron"); cronFlag != nil && cronFlag.Value.String() == "true" { - return - } - bannerOnce.Do(func() { - short := globals.Commit - if len(short) > 12 { - short = short[:12] - } - writeStartupBanner(ui.New(os.Stdout), time.Now().UTC(), short) - }) -} - // RootFlags holds global flags that apply to all commands. // These flags are defined on the root command and inherited by all subcommands. type RootFlags struct { @@ -61,25 +33,15 @@ func NewRootCommand() *cobra.Command { public keys and uploads to S3-compatible storage. No private keys are needed on the source system.`, SilenceUsage: true, - // Banner before every subcommand invocation that doesn't - // suppress output. fx setupGlobals will not print it again. - PersistentPreRun: func(cmd *cobra.Command, args []string) { - maybePrintBanner(cmd) - }, - // Bare 'vaultik' (no subcommand): banner + help. + // Bare 'vaultik' (no subcommand): print help. The banner is + // printed once at process startup by CLIEntry, before cobra + // parses arguments, so it appears even when cobra rejects + // args (e.g. "requires at least 2 arg(s)") and on --help. Run: func(cmd *cobra.Command, args []string) { - maybePrintBanner(cmd) _ = cmd.Help() }, } - // Help output (--help and group-level cmds) also gets the banner. - defaultHelp := cmd.HelpFunc() - cmd.SetHelpFunc(func(c *cobra.Command, args []string) { - maybePrintBanner(c) - defaultHelp(c, args) - }) - // Add global flags cmd.PersistentFlags().StringVar(&rootFlags.ConfigPath, "config", "", "Path to config file (default: $VAULTIK_CONFIG or platform config dir)") cmd.PersistentFlags().BoolVarP(&rootFlags.Verbose, "verbose", "v", false, "Enable verbose output") diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index cbba394..376f4c2 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -79,6 +79,7 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`, if err := v.CreateSnapshot(opts); err != nil { if err != context.Canceled { log.Error("Snapshot creation failed", "error", err) + ReportError("Snapshot creation failed: %v", err) os.Exit(1) } } @@ -144,6 +145,7 @@ func newSnapshotListCommand() *cobra.Command { if err := v.ListSnapshots(jsonOutput); err != nil { if err != context.Canceled { log.Error("Failed to list snapshots", "error", err) + ReportError("Failed to list snapshots: %v", err) os.Exit(1) } } @@ -214,6 +216,7 @@ restrict the operation to specific snapshot names.`, if err := v.PurgeSnapshotsWithOptions(opts); err != nil { if err != context.Canceled { log.Error("Failed to purge snapshots", "error", err) + ReportError("Failed to purge snapshots: %v", err) os.Exit(1) } } @@ -287,6 +290,7 @@ func newSnapshotVerifyCommand() *cobra.Command { if err != context.Canceled { if !opts.JSON { log.Error("Verification failed", "error", err) + ReportError("Verification failed: %v", err) } os.Exit(1) } @@ -380,6 +384,7 @@ Use --all --force to remove all snapshots.`, if err != context.Canceled { if !opts.JSON { log.Error("Failed to remove snapshot", "error", err) + ReportError("Failed to remove snapshot: %v", err) } os.Exit(1) } @@ -444,6 +449,7 @@ accumulate from incomplete backups or deleted snapshots.`, if _, err := v.PruneDatabase(); err != nil { if err != context.Canceled { log.Error("Failed to prune database", "error", err) + ReportError("Failed to prune database: %v", err) os.Exit(1) } } @@ -501,6 +507,7 @@ This command does not delete anything from remote storage.`, if err := v.CleanupLocalSnapshots(); err != nil { if err != context.Canceled { log.Error("Cleanup failed", "error", err) + ReportError("Cleanup failed: %v", err) os.Exit(1) } } diff --git a/internal/cli/snapshot_restore.go b/internal/cli/snapshot_restore.go index d4c78d9..9f60b4a 100644 --- a/internal/cli/snapshot_restore.go +++ b/internal/cli/snapshot_restore.go @@ -132,6 +132,7 @@ func buildRestoreInvokes(snapshotID string, opts *RestoreOptions) []fx.Option { if err := app.Vaultik.Restore(restoreOpts); err != nil { if err != context.Canceled { log.Error("Restore operation failed", "error", err) + ReportError("Restore failed: %v", err) os.Exit(1) } } diff --git a/internal/ui/ui.go b/internal/ui/ui.go index 19dbe44..6855b41 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -49,9 +49,15 @@ const Marker = "》" // It also counts warnings and errors emitted so the caller can summarize at // the end of an operation ("Finished successfully." vs "Finished with // warnings."). +// +// When Quiet is set, Begin/Complete/Info/Notice/Detail/Progress/Banner +// are silently dropped, but Warning and Error always emit. This honors +// the convention that --quiet "Suppresses non-error output" — warnings +// and errors are by definition not suppressible. type Writer struct { out io.Writer color bool + quiet bool warnings int errors int } @@ -70,6 +76,13 @@ func NewWithColor(out io.Writer, color bool) *Writer { return &Writer{out: out, color: color} } +// SetQuiet toggles the writer's quiet mode. In quiet mode all message +// classes are silenced except Warning and Error. +func (w *Writer) SetQuiet(quiet bool) { w.quiet = quiet } + +// Quiet reports whether the writer is in quiet mode. +func (w *Writer) Quiet() bool { return w.quiet } + // Out returns the underlying writer. func (w *Writer) Out() io.Writer { return w.out } @@ -100,21 +113,33 @@ func (w *Writer) paint(color, s string) string { // Begin prints an operation-start line, left-aligned with a white marker. func (w *Writer) Begin(format string, args ...any) { + if w.quiet { + return + } w.emit(ansiWhite, Marker, "", format, args) } // Complete prints an operation-completion line in green, left-aligned. func (w *Writer) Complete(format string, args ...any) { + if w.quiet { + return + } w.emit(ansiGreen, Marker, ansiGreen, format, args) } // Info prints a neutral status line, left-aligned with a white marker. func (w *Writer) Info(format string, args ...any) { + if w.quiet { + return + } w.emit(ansiWhite, Marker, "", format, args) } // Notice prints an attention-worthy informational line, marker in cyan. func (w *Writer) Notice(format string, args ...any) { + if w.quiet { + return + } w.emit(ansiCyan, Marker, "", format, args) } @@ -139,6 +164,9 @@ func (w *Writer) Error(format string, args ...any) { // Distinct from Progress (semantically a "heartbeat") in usage but // visually identical. func (w *Writer) Detail(format string, args ...any) { + if w.quiet { + return + } w.emit(ansiWhite, " "+Marker, "", format, args) } @@ -150,12 +178,18 @@ func (w *Writer) ErrorCount() int { return w.errors } // Progress prints an indented heartbeat / per-item update, marker in white. func (w *Writer) Progress(format string, args ...any) { + if w.quiet { + return + } w.emit(ansiWhite, " "+Marker, "", format, args) } // Banner prints a line with no marker, left-aligned. Bold when color // is enabled. Used for the application startup banner only. func (w *Writer) Banner(format string, args ...any) { + if w.quiet { + return + } body := fmt.Sprintf(format, args...) if w.color { body = ansiBold + body + ansiReset diff --git a/internal/vaultik/integration_test.go b/internal/vaultik/integration_test.go index d977533..5fd4b2c 100644 --- a/internal/vaultik/integration_test.go +++ b/internal/vaultik/integration_test.go @@ -20,6 +20,7 @@ import ( "sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/storage" "sneak.berlin/go/vaultik/internal/types" + "sneak.berlin/go/vaultik/internal/ui" "sneak.berlin/go/vaultik/internal/vaultik" ) @@ -520,6 +521,7 @@ func TestBackupAndRestore(t *testing.T) { Fs: fs, Stdout: io.Discard, Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), } vaultikApp.SetContext(ctx) @@ -666,6 +668,7 @@ func TestEndToEndFileStorage(t *testing.T) { Fs: fs, Stdout: io.Discard, Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), } restoreVaultik.SetContext(ctx) @@ -803,6 +806,7 @@ func TestDedupOnlySnapshotRestores(t *testing.T) { Fs: fs, Stdout: io.Discard, Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), } restoreVaultik.SetContext(ctx) diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index 76f5e03..8233f20 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -93,10 +93,12 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error { if len(files) == 0 { log.Warn("No files found to restore") + v.UI.Warning("No files found to restore.") return nil } log.Info("Found files to restore", "count", len(files)) + v.UI.Info("Found %s files to restore.", v.UI.Count(len(files))) // Step 3: Create target directory if err := v.Fs.MkdirAll(opts.TargetDir, 0755); err != nil { @@ -125,16 +127,16 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error { "duration", result.Duration, ) - v.printfStdout("Restored %d files (%s) in %s\n", - result.FilesRestored, - humanize.Bytes(uint64(result.BytesRestored)), - result.Duration.Round(time.Second), + v.UI.Complete("Restored %s files (%s) in %s.", + v.UI.Count(result.FilesRestored), + v.UI.Size(result.BytesRestored), + v.UI.Duration(result.Duration), ) if result.FilesFailed > 0 { - _, _ = fmt.Fprintf(v.Stdout, "\nWARNING: %d file(s) failed to restore:\n", result.FilesFailed) + v.UI.Warning("%d file(s) failed to restore:", result.FilesFailed) for _, path := range result.FailedFiles { - _, _ = fmt.Fprintf(v.Stdout, " - %s\n", path) + v.UI.Detail("%s", v.UI.Path(path)) } }