From 4c23398243492544ce2ecd4a4715f2bb23b55287 Mon Sep 17 00:00:00 2001 From: user Date: Sun, 15 Feb 2026 21:31:58 -0800 Subject: [PATCH] fix: address PR #24 review concerns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Replace os.Stderr with v.Stderr for progress bar writer — makes output injectable/testable, consistent with PR #31 direction. 2. Fix isTerminal() to accept io.Writer and check v.Stderr fd instead of os.Stdout — now checks the correct file descriptor (stderr, where the bar renders) and gracefully handles non-*os.File writers (returns false in tests). 3. Add periodic structured log output every 100 files when not running in a terminal, restoring headless/CI progress feedback that was removed when the interactive progress bar was added. 4. Apply same os.Stderr -> v.Stderr fix to the verify progress bar for consistency. --- internal/vaultik/restore.go | 38 +++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index 7c0d444..c1c2bfb 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -126,24 +126,26 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error { humanize.Bytes(uint64(totalBytes)), ) - // Create progress bar if output is a terminal + // Create progress bar if stderr is a terminal + isTTY := isTerminal(v.Stderr) var bar *progressbar.ProgressBar - if isTerminal() { + if isTTY { bar = progressbar.NewOptions64( totalBytes, progressbar.OptionSetDescription("Restoring"), - progressbar.OptionSetWriter(os.Stderr), + progressbar.OptionSetWriter(v.Stderr), progressbar.OptionShowBytes(true), progressbar.OptionShowCount(), progressbar.OptionSetWidth(40), progressbar.OptionThrottle(100*time.Millisecond), progressbar.OptionOnCompletion(func() { - fmt.Fprint(os.Stderr, "\n") + fmt.Fprint(v.Stderr, "\n") }), progressbar.OptionSetRenderBlankState(true), ) } + filesProcessed := 0 for _, file := range files { if v.ctx.Err() != nil { return v.ctx.Err() @@ -151,17 +153,33 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error { if err := v.restoreFile(v.ctx, repos, file, opts.TargetDir, identity, chunkToBlobMap, blobCache, result); err != nil { log.Error("Failed to restore file", "path", file.Path, "error", err) + filesProcessed++ // Update progress bar even on failure if bar != nil { _ = bar.Add64(file.Size) } + // Periodic structured log for non-terminal contexts (headless/CI) + if !isTTY && filesProcessed%100 == 0 { + log.Info("Restore progress", + "files", fmt.Sprintf("%d/%d", filesProcessed, len(files)), + "bytes_restored", humanize.Bytes(uint64(result.BytesRestored)), + ) + } continue } + filesProcessed++ // Update progress bar if bar != nil { _ = bar.Add64(file.Size) } + // Periodic structured log for non-terminal contexts (headless/CI) + if !isTTY && (filesProcessed%100 == 0 || filesProcessed == len(files)) { + log.Info("Restore progress", + "files", fmt.Sprintf("%d/%d", filesProcessed, len(files)), + "bytes_restored", humanize.Bytes(uint64(result.BytesRestored)), + ) + } } if bar != nil { @@ -555,7 +573,7 @@ func (v *Vaultik) verifyRestoredFiles( // Create progress bar if output is a terminal var bar *progressbar.ProgressBar - if isTerminal() { + if isTerminal(v.Stderr) { bar = progressbar.NewOptions64( totalBytes, progressbar.OptionSetDescription("Verifying"), @@ -663,7 +681,11 @@ func (v *Vaultik) verifyFile( return bytesVerified, nil } -// isTerminal returns true if stdout is a terminal -func isTerminal() bool { - return term.IsTerminal(int(os.Stdout.Fd())) +// isTerminal returns true if the given writer is connected to a terminal. +// Returns false if the writer does not expose a file descriptor (e.g. in tests). +func isTerminal(w io.Writer) bool { + if f, ok := w.(*os.File); ok { + return term.IsTerminal(int(f.Fd())) + } + return false }