Correct the Vaultik.UI doc comment about --cron (closes #84) #86

Merged
clawbot merged 1 commits from fix-ui-cron-comment into main 2026-08-09 07:43:46 +02:00
Collaborator

Fixes the false Vaultik.UI doc comment described in #84. Comments and
documentation only — the Go diff contains no non-comment lines.

What the code actually does

I verified the description against the code rather than taking the issue on
faith, and the issue is correct:

  • internal/vaultik/vaultik.go:109 constructs the field once as
    ui.New(os.Stdout). Nothing reassigns v.UI anywhere outside New, so no
    writer is ever swapped in.
  • internal/cli/app.go:58-60 (setupGlobals) calls v.UI.SetQuiet(true)
    when opts.Cron || opts.Quiet.
  • In internal/ui/ui.go, the quiet flag is checked by exactly seven
    methods — Beginf, Completef, Infof, Noticef, Detailf, Progressf,
    Bannerf — which return early. Warningf (line 158) and Errorf (line 167)
    have no quiet check at all and always write to w.out.

So --cron (and --quiet, which takes the same branch) suppresses
informational output only; Warning and Error still reach the user. The one
io.Discard UI writer in the tree is internal/snapshot/scanner.go:176, a
nil-UI fallback for the scanner, unrelated to --cron — plausibly where the
"discarding writer" wording came from.

No behavior change was needed: the code already matches a coherent
description, it was only the comment that lied.

Before

// UI is the writer for user-facing status, progress, warnings, errors.
// See package internal/ui for formatting conventions. Defaults to a
// writer wrapping Stdout; the cli layer replaces it with a discarding
// writer in --cron mode.

After

// UI is the writer for user-facing status, progress, warnings, errors.
// See package internal/ui for formatting conventions. It always wraps
// Stdout and is never swapped out; under --cron (and --quiet) the cli
// layer instead calls UI.SetQuiet(true), which drops Begin, Complete,
// Info, Notice, Detail, Progress, and Banner messages. Warning and
// Error are still emitted in that mode, so callers must not assume
// that --cron makes this writer silent.

Other locations (DoD item 2)

I grepped every --cron / quiet mention in the tree.

Corrected:

  • README.md line 171 described --cron as "Silent unless error", which
    understates what survives — warnings are emitted too, and the codebase
    deliberately relies on that (internal/vaultik/snapshot.go:116-124 routes
    the end-of-run summary through UI.Warningf precisely so cron delivers
    something). Now reads "Silent on total success; warnings and errors are
    still printed (for crontab)".

Audited and already accurate, left untouched:

  • internal/log/log.go:53-55 — states the cron/quiet log level keeps warnings
    and errors visible, which matches level = slog.LevelWarn.
  • internal/cli/snapshot.go:104 — "--cron suppression is wired through v.UI
    by setupGlobals"; true, and it does not claim total silence.
  • internal/vaultik/snapshot.go:116-120 — correctly states Complete is dropped
    while the warning summary still fires.

Reported, not changed

The --cron cobra flag help string at internal/cli/snapshot.go:138 reads
"Run in cron mode (silent unless error)", carrying the same imprecision the
README line had. That string is program output rather than a comment, so
changing it under a docs-only issue would be a behavior change; leaving it for
separate triage.

Also noted for separate triage: make lint now prints
The linter 'gomodguard' is deprecated (since v2.12.0) ... Replaced by gomodguard_v2 on every run. It does not fail the gate, and .golangci.yml is
hash-pinned, so it is out of scope here.

Verification

script/cibuild — literal EXIT=0, captured immediately after the run.

The run was forced uncached rather than trusted: the new commit invalidates the
Docker source-copy layer, so #19 [lint 8/8] RUN make lint executed in 72.0s
and printed 0 issues., and #31 [builder 8/9] RUN make test executed in
100.0s. Neither was CACHED — the only CACHED layers in the build were
apk add, COPY go.mod go.sum, and go mod download. Every package line in
the test stage shows a real duration with no (cached) marker.

Separately, GOFLAGS=-count=1 make check was run in the worktree to defeat the
Go test cache independently of Docker: 0 issues. from lint, and all 14 test
packages ok with real timings and no (cached) entries.

Diff scope confirmed mechanically: filtering the Go portion of the diff for
added/removed lines that are not comment lines yields zero lines.
.golangci.yml is untouched and still hashes to
021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.

Fixes the false `Vaultik.UI` doc comment described in #84. Comments and documentation only — the Go diff contains no non-comment lines. ## What the code actually does I verified the description against the code rather than taking the issue on faith, and the issue is correct: - `internal/vaultik/vaultik.go:109` constructs the field once as `ui.New(os.Stdout)`. Nothing reassigns `v.UI` anywhere outside `New`, so no writer is ever swapped in. - `internal/cli/app.go:58-60` (`setupGlobals`) calls `v.UI.SetQuiet(true)` when `opts.Cron || opts.Quiet`. - In `internal/ui/ui.go`, the `quiet` flag is checked by exactly seven methods — `Beginf`, `Completef`, `Infof`, `Noticef`, `Detailf`, `Progressf`, `Bannerf` — which return early. `Warningf` (line 158) and `Errorf` (line 167) have no quiet check at all and always write to `w.out`. So `--cron` (and `--quiet`, which takes the same branch) suppresses informational output only; Warning and Error still reach the user. The one `io.Discard` UI writer in the tree is `internal/snapshot/scanner.go:176`, a nil-UI fallback for the scanner, unrelated to `--cron` — plausibly where the "discarding writer" wording came from. No behavior change was needed: the code already matches a coherent description, it was only the comment that lied. ## Before ``` // UI is the writer for user-facing status, progress, warnings, errors. // See package internal/ui for formatting conventions. Defaults to a // writer wrapping Stdout; the cli layer replaces it with a discarding // writer in --cron mode. ``` ## After ``` // UI is the writer for user-facing status, progress, warnings, errors. // See package internal/ui for formatting conventions. It always wraps // Stdout and is never swapped out; under --cron (and --quiet) the cli // layer instead calls UI.SetQuiet(true), which drops Begin, Complete, // Info, Notice, Detail, Progress, and Banner messages. Warning and // Error are still emitted in that mode, so callers must not assume // that --cron makes this writer silent. ``` ## Other locations (DoD item 2) I grepped every `--cron` / quiet mention in the tree. Corrected: - `README.md` line 171 described `--cron` as "Silent unless error", which understates what survives — warnings are emitted too, and the codebase deliberately relies on that (`internal/vaultik/snapshot.go:116-124` routes the end-of-run summary through `UI.Warningf` precisely so cron delivers something). Now reads "Silent on total success; warnings and errors are still printed (for crontab)". Audited and already accurate, left untouched: - `internal/log/log.go:53-55` — states the cron/quiet log level keeps warnings and errors visible, which matches `level = slog.LevelWarn`. - `internal/cli/snapshot.go:104` — "--cron suppression is wired through v.UI by setupGlobals"; true, and it does not claim total silence. - `internal/vaultik/snapshot.go:116-120` — correctly states Complete is dropped while the warning summary still fires. ## Reported, not changed The `--cron` cobra flag help string at `internal/cli/snapshot.go:138` reads "Run in cron mode (silent unless error)", carrying the same imprecision the README line had. That string is program output rather than a comment, so changing it under a docs-only issue would be a behavior change; leaving it for separate triage. Also noted for separate triage: `make lint` now prints `The linter 'gomodguard' is deprecated (since v2.12.0) ... Replaced by gomodguard_v2` on every run. It does not fail the gate, and `.golangci.yml` is hash-pinned, so it is out of scope here. ## Verification `script/cibuild` — literal `EXIT=0`, captured immediately after the run. The run was forced uncached rather than trusted: the new commit invalidates the Docker source-copy layer, so `#19 [lint 8/8] RUN make lint` executed in 72.0s and printed `0 issues.`, and `#31 [builder 8/9] RUN make test` executed in 100.0s. Neither was `CACHED` — the only `CACHED` layers in the build were `apk add`, `COPY go.mod go.sum`, and `go mod download`. Every package line in the test stage shows a real duration with no `(cached)` marker. Separately, `GOFLAGS=-count=1 make check` was run in the worktree to defeat the Go test cache independently of Docker: `0 issues.` from lint, and all 14 test packages `ok` with real timings and no `(cached)` entries. Diff scope confirmed mechanically: filtering the Go portion of the diff for added/removed lines that are not comment lines yields zero lines. `.golangci.yml` is untouched and still hashes to `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`.
clawbot added 1 commit 2026-08-09 07:42:36 +02:00
Correct the Vaultik.UI doc comment about --cron (closes #84)
All checks were successful
check / check (pull_request) Successful in 2m59s
dcf3dd3756
The field comment claimed the cli layer replaces the UI writer with a
discarding writer in --cron mode. It does not: the writer is created
once as ui.New(os.Stdout) and never reassigned, and setupGlobals calls
UI.SetQuiet(true) instead. Quiet mode drops Begin, Complete, Info,
Notice, Detail, Progress, and Banner, but Warningf and Errorf have no
quiet check and always emit, so --cron does not make the writer silent.

The README's --cron bullet had the adjacent understatement ("Silent
unless error"), and now names warnings as well.

Comments and documentation only; no behavior change.
Author
Collaborator

Summary of what landed here, in commit dcf3dd3:

  • internal/vaultik/vaultik.go — the Vaultik.UI field comment now names UI.SetQuiet(true) as the actual mechanism, lists the seven message classes it drops, and states that Warning and Error are still emitted under --cron. It also notes that --quiet takes the same branch and that the writer is never swapped out.
  • README.md — the --cron bullet no longer says "Silent unless error"; it now says silent on total success, with warnings and errors still printed.
  • TODO.md — Completed Steps entry in the same commit as the work.

Verification, forced uncached because a cached gate proves nothing here:

  • script/cibuild returned literal EXIT=0. The new commit invalidated the Docker source layer, so #19 [lint 8/8] RUN make lint ran for 72.0s and printed 0 issues., and #31 [builder 8/9] RUN make test ran for 100.0s. Neither stage was CACHED; the only CACHED layers were apk add, COPY go.mod go.sum, and go mod download. No test package line carried a (cached) marker.
  • Independently, GOFLAGS=-count=1 make check in the worktree: 0 issues. from lint and all 14 test packages ok with real durations, none cached.

Scope check: filtering the Go half of the diff for added/removed lines that are not comment lines returns zero lines, so there is no behavior change. .golangci.yml untouched and still at the pinned sha256.

Summary of what landed here, in commit `dcf3dd3`: - `internal/vaultik/vaultik.go` — the `Vaultik.UI` field comment now names `UI.SetQuiet(true)` as the actual mechanism, lists the seven message classes it drops, and states that Warning and Error are still emitted under `--cron`. It also notes that `--quiet` takes the same branch and that the writer is never swapped out. - `README.md` — the `--cron` bullet no longer says "Silent unless error"; it now says silent on total success, with warnings and errors still printed. - `TODO.md` — Completed Steps entry in the same commit as the work. Verification, forced uncached because a cached gate proves nothing here: - `script/cibuild` returned literal `EXIT=0`. The new commit invalidated the Docker source layer, so `#19 [lint 8/8] RUN make lint` ran for 72.0s and printed `0 issues.`, and `#31 [builder 8/9] RUN make test` ran for 100.0s. Neither stage was `CACHED`; the only `CACHED` layers were `apk add`, `COPY go.mod go.sum`, and `go mod download`. No test package line carried a `(cached)` marker. - Independently, `GOFLAGS=-count=1 make check` in the worktree: `0 issues.` from lint and all 14 test packages `ok` with real durations, none cached. Scope check: filtering the Go half of the diff for added/removed lines that are not comment lines returns zero lines, so there is no behavior change. `.golangci.yml` untouched and still at the pinned sha256.
clawbot added the merge-ready label 2026-08-09 07:43:37 +02:00
clawbot merged commit 3bcdbcfd83 into main 2026-08-09 07:43:46 +02:00
clawbot deleted branch fix-ui-cron-comment 2026-08-09 07:43:46 +02:00
Sign in to join this conversation.