refactor(converge): render the goldens through the production printer and probe the terminal once

[mlorentedev]

Summary

Two Minor findings from CLI-114's independent review (specs/archive/CLI-114-converge-console-ux/review.md).

1. The goldens tested a renderer production no longer called. printConvergeReport had no production caller once runConverge moved to newConvergePrinter + RunObserved + finish, and it took its column width from the report rather than the registry. It is deleted. The tests render through newConvergePrinter, with a registry built from the report's names, Done for each entry, then finish. The golden files are unchanged byte for byte: production prints what the old fixture printed. From now on, a change to the production printer reaches them.

2. Two terminal probes.

• console.IsTerminalBy(w, isTerminal) is the descriptor seam the converge tests need. console.IsTerminal is now that function with term.IsTerminal, so both share one *os.File check.
• convergeTerminal is gone.
• runConverge probes once, on the raw writer, and passes the answer to convergePainter(terminal bool). A caller can no longer hand the tee to one probe and the raw writer to the other.

Closes #2487

Verification

$ grep -rn 'printConvergeReport(\|convergeTerminal' cli/internal/cmd/*.go   # exit 1, no output
$ go test ./internal/cmd/ ./internal/console/ -count=1
ok  	github.com/mlorentedev/dotfiles/cli/internal/cmd
ok  	github.com/mlorentedev/dotfiles/cli/internal/console

• Mutation 1: production's default name column 16 -> 10. TestConvergeConsole_Colour and TestConvergeConsole_FailureSummary fail. Before this change they passed, because the deleted fixture had its own 16.
• Mutation 2: runConverge ignores its probe (convergePainter(false)). TestConvergeConsole_ARunOnATerminalIsColoured and TestConvergeConsole_AnAppliedRunColoursTheTerminalNotTheTranscript fail.
• The tests the ticket names still pass: TestIsTerminal_NeverForANonFile, TestConvergeConsole_ColourOnATerminal, and the transcript test.
• go build ./..., go vet ./..., GOOS=windows go vet ./... and go test ./... all pass. golangci-lint run reports 0 issues.

SDD skip rationale

A refactor with 37 production LOC across converge.go and console.go. Output is unchanged (the goldens are byte-identical), and it adds no CLI flag, contract or dependency.

Knowledge

• Lesson: none: the lesson (a golden through a fixture renderer proves nothing about production) is the review finding itself, recorded in CLI-114's review.md and verification.md
• ADR: none: no decision changes
• Runbook: none: no procedure changes

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 6 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: mlorentedev/dotfiles/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6c494f8a-0432-4074-9f48-5a65d28502bc

📥 Commits

Reviewing files that changed from the base of the PR and between f3413d8 and 3029996.

📒 Files selected for processing: 3

• cli/internal/cmd/converge.go
• cli/internal/cmd/converge_console_test.go
• cli/internal/console/console.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 2487 - Partially compliant Compliant requirements: printConvergeReport is deleted from converge.go; the PR verification shows the grep exiting 1 with no output. Goldens now render via renderConvergeReport, which drives newConvergePrinter with a registry built from the report entries, header, Done per entry, and finish. Golden files are unchanged (no golden hunks in the diff), so the regeneration rule is moot. console.IsTerminalBy(w, isTerminal) added; console.IsTerminal delegates to it with term.IsTerminal, preserving the *os.File guard. convergeTerminal removed; convergePainter now takes terminal bool. runConverge probes once on the raw writer (console.IsTerminalBy(w, isTerminal)) before beginRunLog wraps it, and passes the result to convergePainter(terminal). go test ./internal/cmd/ ./internal/console/ -count=1 reported ok for both packages, covering the three named existing tests. Non-compliant requirements: (empty) Requires further human verification: Commit messages were not included in the review material, so the English-only, no-AI-attribution and no-phase-reference rules over the commits (and the "reason in the commit" clause for any regenerated golden) cannot be checked here.

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Harness compliance Definition of Done §2 (Knowledge) fails as submitted: the PR body has a ## Summary and a ## Verification section but no ## Knowledge section naming a Lesson/ADR/Runbook path the PR adds or changes, or none: <reason>. Note: the body I was given is truncated mid-Verification, so if a ## Knowledge section exists below the cut, this is a pass — it must be confirmed on the actual PR. Classification: REAL as submitted, with that truncation caveat. Harness compliance Spec-Driven Development rule: a change of this size must touch a specs/<id>/ folder or carry the skip-sdd label with a written rationale. The diff touches no specs/ folder, and no skip-sdd label is visible in the supplied PR metadata (only the ticket's chore, debt labels were provided). If the PR itself carries skip-sdd with a rationale, this passes; otherwise the gate is unmet. Classification: REAL as submitted, label presence unverified from the material given.

---

⚙️ Agent run details

• Model: openai/mimo-v2.6-flash
• Tokens: 18,093 in / 13,065 out / 31,158 total
• Time cost: 250.1s
• AI calls: 1

[mlorentedev]

Review triage

Reviewers: PR-Agent (openai/mimo-v2.6-flash) reviewed e2178214. CodeRabbit posted only a rate-limit notice, so it did not review.

Finding	Disposition
No ## Knowledge section	Declined: the body has one (Lesson, ADR, Runbook each none: with a reason). The reviewer saw a truncated body, as it says.
No skip-sdd label	Declined: the PR carries skip-sdd, and the body has ## SDD skip rationale.

CI: from-zero (macos-latest) failed on run 38643121196. That is BUG-138 (#2481): uv built ansible-core's venv on Python 3.15. Unrelated to this diff; the fix is #2495. This PR lands after it, with the branch updated, so from-zero runs again on the fix.