feat(diary): stdin append, date headers in read, CI/CD stack

[birdmanmandbir]

Summary

• stdin append: `diary append` now reads from stdin when no positional arg is given (pipe detection via `golang.org/x/term`). Positional arg still wins if both provided, with a stderr warning.
• date headers in read: plain-text `diary read` output now prefixed with `Today:` and `Entry:` date lines for temporal context. `-t` interactive viewer unchanged.
• CI/CD: lefthook pre-commit hooks (fmt/vet/lint), GitHub Actions pr/ci/release workflows, goreleaser config with homebrew tap
• golangci-lint: `.golangci.yml` configured with govet, staticcheck, unused, ineffassign
• tests: subprocess-based CLI tests covering all new behaviors

Test plan

• `echo "content" | diary append` appends via stdin
• `diary append` with no stdin returns error
• `diary append "arg"` with piped stdin warns and uses arg
• `diary read` shows `Today:` and `Entry:` header before content
• `diary read -t` shows no header (TUI mode unchanged)
• `make ci` passes (fmt + vet + lint + test + build)
• Pre-commit hooks fire on `*.go` changes

[birdmanmandbir]

PR Review: diary-cli stdin append + date headers + CI/CD

Overall the feature work is solid and well-tested. Several issues were found across the CI/CD, core logic, and test suite.

---

🔴 Critical Issues

[ci.yaml:32, pr.yaml:23] Format gate is completely inert — always passes

run: make fmt && git status --porcelain | grep -q '\.go$' && exit 1 || true

The trailing || true means this step exits 0 unconditionally. Unformatted code will never be caught by CI. Ironically, lefthook.yml uses the correct idiom — replace both occurrences with:

make fmt && git diff --exit-code -- '*.go'

---

🟠 Important Issues

[main.go] "Today:" header label is wrong for historical entries

fmt.Printf("Today: %s\n", now.Format("2006-01-02 (Mon) 15:04 -07:00"))
fmt.Printf("Entry: %s\n\n", date)

When reading diary neil read 2025-01-15, the header says Today: but prints the current wall-clock time — not the entry date. For historical reads this is misleading. Change the label to "Now:" or "Read at:".

[main.go:247-249, :256-258] Duplicated error block in handleAppend

The "Error: text required" + usage message + os.Exit(1) block appears twice. Extract to after the if/else:

// after stdin vs arg branching:
if text == "" {
    fmt.Fprintln(os.Stderr, "Error: text required for append command")
    fmt.Fprintln(os.Stderr, "Usage: diary <user> append \"text\"")
    os.Exit(1)
}

[internal/tui/search.go] nolint:unused on selectedStyle has no effect

unused is a whole-program linter, not line-level — this directive is silently ignored. The variable is dead code. Either use it or delete it.

[internal/crypto/age_test.go:20,81] nolint:ineffassign,staticcheck suppresses real dead code

recipient := identity.Recipient().String() //nolint:ineffassign,staticcheck

The linter is correctly flagging this. recipient is assigned but never read before being overwritten from loadedIdentity. The fix is to remove the first assignment entirely, not silence the linter.

[go.mod] termenv incorrectly promoted to direct dependency

github.com/muesli/termenv is only used in internal/tui, not main.go. Running go mod tidy (which the goreleaser before.hooks does) will revert it back to indirect and produce a diff. Only golang.org/x/term (used in main.go via term.IsTerminal) should be direct.

[pr.yaml:14-15] Redundant go mod download step

actions/setup-go@v5 with cache: true already handles module caching. The explicit download step runs twice and is absent from ci.yaml, creating an inconsistency.

---

🟡 Test Gaps

[main_test.go] No test for sequential appends (most common usage)

The newline-joining logic ("\n" separator between existing content and new content) is never exercised. A test should call diary append twice and verify the read output contains both pieces joined by exactly one newline.

[main_test.go:192] cmd.Output() error discarded in TestReadTUISkipsHeader

out, _ := cmd.Output()

A TUI panic or key error would silently pass the test. Capture the error and at minimum log unexpected stderr:

out, err := cmd.Output()
if err != nil {
    var exitErr *exec.ExitError
    if errors.As(err, &exitErr) && len(exitErr.Stderr) > 0 {
        t.Logf("unexpected TUI stderr: %s", exitErr.Stderr)
    }
}

[main_test.go] Each test rebuilds the binary independently

binaryPath(t) runs go build into a per-test t.TempDir(), so 5 tests = 5 builds. A TestMain or sync.Once wrapping the build would make the suite significantly faster.

---

✅ Strengths

• Stdin feature implementation is clean and the term.IsTerminal guard is exactly right
• Black-box integration test approach (real binary + temp HOME) is the correct choice for a CLI tool
• TestAppendArgWinsOverStdin is a good precedence test catching a non-obvious interaction
• lefthook.yml hook design is correct (aside from parallel mutation race noted below)
• Goreleaser config covers all major platforms and correctly disables CGO

---

VERDICT: Request Changes — CI format gate is broken (critical), "Today:" label is misleading, and a handful of medium issues should be cleaned up before merge.

[birdmanmandbir]

Triage Update

Fixed

• CI format gate inert — replaced || true with git diff --exit-code -- '*.go' in both pr.yaml and ci.yaml
• Redundant go mod download — removed from pr.yaml (setup-go@v5 with cache handles it)
• "Today:" label misleading for historical entries — renamed to "Now:" in plain-text read header
• Duplicated error block in handleAppend — deduplicated into single guard after the arg/stdin branching
• selectedStyle dead code in search.go — removed
• Dead recipient first-assignment in age_test.go — removed both occurrences; tests now use the loaded identity directly (also removed now-unused filippo.io/age import)
• Test binary rebuilt per test — added TestMain + sync.Once; all 6 tests share one build
• No sequential append test — added TestAppendSequential verifying newline-joining logic
• TUI test silently discards error — logs unexpected stderr via errors.As(*exec.ExitError)

Not Fixed

• termenv as direct dep — reviewer flagged this but go mod tidy confirms it's correct: internal/tui/reader.go and internal/tui/search.go both import github.com/muesli/termenv directly. Goreleaser's go mod tidy hook will not revert it.

[birdmanmandbir]

Re-review: fix(diary) address PR review findings

Fixed ✅

• CI format gate — || true replaced with git diff --exit-code -- '*.go' in both ci.yaml and pr.yaml
• Redundant go mod download — removed from pr.yaml
• "Today:" label — renamed to "Now:" in plain-text read header; test assertion updated to match
• Duplicated error block — collapsed into a single if text == "" guard after arg/stdin branching; clean
• selectedStyle dead variable — removed from search.go
• Dead recipient first-assignment — removed from age_test.go along with the now-unused filippo.io/age import
• golangci.yml exclude-rules — removed (no longer needed now that the underlying code is fixed)
• Shared binary build — TestMain + sync.Once in place; all 6 tests share one go build
• Sequential append not tested — TestAppendSequential added with correct single-newline boundary assertion
• TUI test silently discarded error — errors.As(*exec.ExitError) + t.Logf added
• writeEntry hardcoded date — helper now takes a date parameter; TestReadPlaintextDateHeader passes time.Now().Format("2006-01-02")

Reviewer's false positive (acknowledged) ✅

• termenv as direct dep — my review was wrong. internal/tui/reader.go and internal/tui/search.go both import termenv directly; it is correctly listed as a direct dependency and go mod tidy will not revert it.

New minor observation

[main_test.go:253] Custom min helper shadows built-in

func min(a, b int) int {
    if a < b { return a }
    return b
}

Go 1.21 introduced a built-in min. Since this module targets Go 1.25.5, this custom function is redundant and shadows the built-in. Harmless, but can be deleted.

---

VERDICT: LGTM 🟢

All reported issues resolved. The one new observation is non-blocking. Ready to merge.

[birdmanmandbir]

Triage Update

Fixed

• Custom min helper — removed; Go 1.21+ built-in used instead

All other issues were already resolved in the previous round. Reviewer verdict: LGTM 🟢

[birdmanmandbir]

Re-review: chore(test) remove redundant min helper

Fixed ✅

• Custom min helper removed — func min(a, b int) int deleted from main_test.go; Go 1.21+ built-in used implicitly

---

VERDICT: LGTM 🟢

All issues fully resolved across three rounds. Ready to merge.