cli: maturity polish — color, error translation, tabwriter consistency

Adds three small but high-leverage presentation tweaks for v0.1:

1. internal/cli/style is a new ~70 LOC package with Pass/Fail/Warn/
   Dim/Bold helpers. Each is TTY-gated and obeys NO_COLOR. No
   external dep. Wired into the doctor PASS/FAIL/WARN status, the
   "banger:" error prefix on stderr, and the dim 'ready in <elapsed>'
   line.
2. internal/cli/errors translates rpc.ErrorResponse into user-facing
   text. operation_failed becomes invisible (the message wins);
   not_found, already_exists, bad_request, bad_version, unauthorized,
   unknown_method get short labels; unknown codes pass through. The
   daemon-attached op_id lands in dim parens — paste into
   journalctl --grep to find the daemon log line that produced the
   failure.
3. Tabwriter config converges on (0, 8, 2, ' ', 0) across every
   list/table command. The vm prune confirmation table picked up the
   right config; system install + system status switched from bare
   "key: value\n" lines to tabular form. printVMSpecLine drops its
   Unicode middle dot for an ASCII '|' so terminals without UTF-8
   render cleanly.

Tests cover translateRPCError for every code, style helpers no-op
on non-TTY and under NO_COLOR. Smoke status greps switch from
"key: value" to "key   value" to match the new format.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cmd/banger/main.go

@@ -9,6 +9,7 @@ import (
 	"syscall"
 
 	"banger/internal/cli"
+	"banger/internal/cli/style"
 )
 
 func main() {
@@ -21,7 +22,12 @@ func main() {
 		if errors.As(err, &exitErr) {
 			os.Exit(exitErr.Code)
 		}
-		fmt.Fprintf(os.Stderr, "banger: %v\n", err)
+		// Render the failure through the CLI's translator so RPC
+		// codes become friendly text, op_ids land in parens for
+		// journalctl grepping, and the "banger:" prefix turns red
+		// on a TTY.
+		prefix := style.Fail(os.Stderr, "banger:")
+		fmt.Fprintf(os.Stderr, "%s %s\n", prefix, cli.TranslateError(os.Stderr, err))
 		os.Exit(1)
 	}
 }

internal/cli/cli_test.go

@@ -1790,10 +1790,14 @@ func TestDaemonStatusIncludesLogPathWhenStopped(t *testing.T) {
 	}
 
 	output := stdout.String()
+	// Output is tabwriter-formatted (key TAB value, padded). Assert
+	// the key and value land on the same line rather than pinning a
+	// specific separator.
 	for _, want := range []string{
-		"service: bangerd.service",
+		"service",
-		"socket: /run/banger/bangerd.sock",
+		"bangerd.service",
-		"log: journalctl -u bangerd.service",
+		"/run/banger/bangerd.sock",
+		"journalctl -u bangerd.service",
 	} {
 		if !strings.Contains(output, want) {
 			t.Fatalf("output = %q, want %q", output, want)
@@ -1825,13 +1829,14 @@ func TestDaemonStatusIncludesDaemonBuildInfoWhenRunning(t *testing.T) {
 
 	output := stdout.String()
 	for _, want := range []string{
-		"service: bangerd.service",
+		"service",
-		"socket: /run/banger/bangerd.sock",
+		"bangerd.service",
-		"log: journalctl -u bangerd.service",
+		"/run/banger/bangerd.sock",
-		"pid: 42",
+		"journalctl -u bangerd.service",
-		"version: v1.2.3",
+		"42",
-		"commit: abc123",
+		"v1.2.3",
-		"built_at: 2026-03-22T12:00:00Z",
+		"abc123",
+		"2026-03-22T12:00:00Z",
 	} {
 		if !strings.Contains(output, want) {
 			t.Fatalf("output = %q, want %q", output, want)

internal/cli/commands_system.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"text/tabwriter"
 
 	"banger/internal/buildinfo"
 	"banger/internal/installmeta"
@@ -190,8 +191,16 @@ func (d *deps) runSystemInstall(ctx context.Context, out io.Writer, ownerFlag st
 	if err := d.waitForDaemonReady(ctx, installmeta.DefaultSocketPath); err != nil {
 		return err
 	}
-	_, err = fmt.Fprintf(out, "installed\nowner: %s\nsocket: %s\nhelper_socket: %s\nservice: %s\nhelper_service: %s\n", meta.OwnerUser, installmeta.DefaultSocketPath, installmeta.DefaultRootHelperSocketPath, installmeta.DefaultService, installmeta.DefaultRootHelperService)
+	if _, err := fmt.Fprintln(out, "installed"); err != nil {
-	return err
+		return err
+	}
+	w := tabwriter.NewWriter(out, 0, 8, 2, ' ', 0)
+	fmt.Fprintf(w, "owner\t%s\n", meta.OwnerUser)
+	fmt.Fprintf(w, "socket\t%s\n", installmeta.DefaultSocketPath)
+	fmt.Fprintf(w, "helper_socket\t%s\n", installmeta.DefaultRootHelperSocketPath)
+	fmt.Fprintf(w, "service\t%s\n", installmeta.DefaultService)
+	fmt.Fprintf(w, "helper_service\t%s\n", installmeta.DefaultRootHelperService)
+	return w.Flush()
 }
 
 func (d *deps) runSystemStatus(ctx context.Context, out io.Writer) error {
@@ -212,17 +221,28 @@ func (d *deps) runSystemStatus(ctx context.Context, out io.Writer) error {
 	if helperEnabled == "" {
 		helperEnabled = "unknown"
 	}
-	fmt.Fprintf(out, "service: %s\nenabled: %s\nactive: %s\nhelper_service: %s\nhelper_enabled: %s\nhelper_active: %s\nsocket: %s\nhelper_socket: %s\nlog: journalctl -u %s -u %s\n",
+	w := tabwriter.NewWriter(out, 0, 8, 2, ' ', 0)
-		installmeta.DefaultService, enabled, active,
+	fmt.Fprintf(w, "service\t%s\n", installmeta.DefaultService)
-		installmeta.DefaultRootHelperService, helperEnabled, helperActive,
+	fmt.Fprintf(w, "enabled\t%s\n", enabled)
-		layout.SocketPath, installmeta.DefaultRootHelperSocketPath,
+	fmt.Fprintf(w, "active\t%s\n", active)
-		installmeta.DefaultService, installmeta.DefaultRootHelperService)
+	fmt.Fprintf(w, "helper_service\t%s\n", installmeta.DefaultRootHelperService)
+	fmt.Fprintf(w, "helper_enabled\t%s\n", helperEnabled)
+	fmt.Fprintf(w, "helper_active\t%s\n", helperActive)
+	fmt.Fprintf(w, "socket\t%s\n", layout.SocketPath)
+	fmt.Fprintf(w, "helper_socket\t%s\n", installmeta.DefaultRootHelperSocketPath)
+	fmt.Fprintf(w, "log\tjournalctl -u %s -u %s\n", installmeta.DefaultService, installmeta.DefaultRootHelperService)
 	if ping, err := d.daemonPing(ctx, layout.SocketPath); err == nil {
 		info := buildinfo.Normalize(ping.Version, ping.Commit, ping.BuiltAt)
-		_, err = fmt.Fprintf(out, "pid: %d\n%s", ping.PID, formatBuildInfoBlock(info))
+		fmt.Fprintf(w, "pid\t%d\n", ping.PID)
-		return err
+		fmt.Fprintf(w, "version\t%s\n", info.Version)
+		if info.Commit != "" {
+			fmt.Fprintf(w, "commit\t%s\n", info.Commit)
+		}
+		if info.BuiltAt != "" {
+			fmt.Fprintf(w, "built_at\t%s\n", info.BuiltAt)
+		}
 	}
-	return nil
+	return w.Flush()
 }
 
 func (d *deps) runSystemUninstall(ctx context.Context, out io.Writer, purge bool) error {

internal/cli/commands_vm.go

@@ -281,7 +281,7 @@ func (d *deps) runVMPrune(cmd *cobra.Command, socketPath string, force bool) err
 	}
 
 	fmt.Fprintf(stdout, "The following %d VM(s) will be deleted:\n", len(victims))
-	w := tabwriter.NewWriter(stdout, 0, 0, 2, ' ', 0)
+	w := tabwriter.NewWriter(stdout, 0, 8, 2, ' ', 0)
 	fmt.Fprintln(w, "  ID\tNAME\tSTATE")
 	for _, vm := range victims {
 		fmt.Fprintf(w, "  %s\t%s\t%s\n", shortID(vm.ID), vm.Name, vm.State)

internal/cli/errors.go

@@ -0,0 +1,90 @@
+package cli
+
+import (
+	"errors"
+	"strings"
+
+	"banger/internal/cli/style"
+	"banger/internal/rpc"
+	"io"
+)
+
+// TranslateError is the public entry point used by cmd/banger/main.go
+// to render any error reaching the top of the cobra tree. Forwards
+// to the package-internal helper so tests can reach it directly.
+func TranslateError(w io.Writer, err error) string {
+	return translateRPCError(w, err)
+}
+
+// translateRPCError turns an error returned by rpc.Call into a
+// user-facing string. Known codes get short, friendly prefixes;
+// unknown codes pass through verbatim so debuggability is preserved.
+// When the daemon attached an op_id the helper appends it in parens
+// so an operator can paste it into journalctl --grep.
+//
+// Color is applied only when w is a TTY (and NO_COLOR is unset).
+// The returned string never includes a trailing newline — caller
+// chooses where it goes.
+func translateRPCError(w io.Writer, err error) string {
+	if err == nil {
+		return ""
+	}
+	var rpcErr *rpc.ErrorResponse
+	if !errors.As(err, &rpcErr) || rpcErr == nil {
+		// Non-RPC failures (dialing the socket, decode errors,
+		// context cancellation, ...) come through as plain Go
+		// errors. Surface them verbatim — they already mention
+		// the underlying cause clearly enough.
+		return err.Error()
+	}
+	prefix := errorCodePrefix(rpcErr.Code)
+	body := rpcErr.Message
+	if prefix != "" {
+		body = prefix + ": " + rpcErr.Message
+	} else if rpcErr.Message == "" {
+		// Defensive: a server that returned a code with no
+		// message still has SOMETHING to report; default to the
+		// raw code so we never print an empty error.
+		body = rpcErr.Code
+	}
+	if rpcErr.OpID != "" {
+		body = body + " (" + style.Dim(w, rpcErr.OpID) + ")"
+	}
+	return body
+}
+
+// errorCodePrefix maps the small set of codes the daemon emits to
+// short user-facing labels. Unknown codes return "" so the message
+// alone is shown — keeps the door open for future codes the CLI
+// hasn't been updated to recognise.
+//
+// "operation_failed" is the catch-all the generic dispatcher uses
+// when a service returned an error; the message is already self-
+// explanatory, so we strip the code entirely. Specialised codes
+// (not_found, already_exists, ...) keep a label because the
+// message body alone may not say what kind of failure it is.
+func errorCodePrefix(code string) string {
+	switch strings.TrimSpace(code) {
+	case "", "operation_failed":
+		return ""
+	case "not_found":
+		return "not found"
+	case "not_running":
+		return "not running"
+	case "already_exists":
+		return "already exists"
+	case "bad_request", "bad_params":
+		return "bad request"
+	case "bad_version":
+		return "version mismatch"
+	case "unauthorized":
+		return "unauthorized"
+	case "unknown_method":
+		return "unknown method"
+	default:
+		// Surface the raw code so an operator filing a bug has
+		// something concrete to grep for. Strips the boilerplate
+		// "operation_failed" but keeps anything novel.
+		return code
+	}
+}

internal/cli/errors_test.go

@@ -0,0 +1,60 @@
+package cli
+
+import (
+	"bytes"
+	"errors"
+	"strings"
+	"testing"
+
+	"banger/internal/rpc"
+)
+
+// TestTranslateRPCError pins the user-facing error rendering for
+// every code the daemon emits today plus the catch-all unknown-code
+// path. Buffer is non-TTY so style helpers no-op and assertions
+// stay readable.
+func TestTranslateRPCError(t *testing.T) {
+	var buf bytes.Buffer
+	cases := []struct {
+		name   string
+		code   string
+		msg    string
+		opID   string
+		expect string
+	}{
+		{"operation_failed strips code", "operation_failed", "vm running", "", "vm running"},
+		{"empty code drops prefix", "", "raw boom", "", "raw boom"},
+		{"not_found", "not_found", `vm "x" not found`, "", `not found: vm "x" not found`},
+		{"not_running", "not_running", "vm is not running", "", "not running: vm is not running"},
+		{"already_exists", "already_exists", "image foo", "", "already exists: image foo"},
+		{"bad_request", "bad_request", "missing rootfs", "", "bad request: missing rootfs"},
+		{"bad_params", "bad_params", "invalid tap name", "", "bad request: invalid tap name"},
+		{"bad_version", "bad_version", "unsupported version 99", "", "version mismatch: unsupported version 99"},
+		{"unauthorized", "unauthorized", "uid 1000 not allowed", "", "unauthorized: uid 1000 not allowed"},
+		{"unknown_method", "unknown_method", "no.such.method", "", "unknown method: no.such.method"},
+		{"unknown code falls through", "weird_new_code", "boom", "", "weird_new_code: boom"},
+		{"op_id appended in parens", "operation_failed", "boom", "op-deadbeef00ff", "boom (op-deadbeef00ff)"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := &rpc.ErrorResponse{Code: tc.code, Message: tc.msg, OpID: tc.opID}
+			got := translateRPCError(&buf, err)
+			if got != tc.expect {
+				t.Errorf("got %q, want %q", got, tc.expect)
+			}
+		})
+	}
+}
+
+// TestTranslateRPCErrorPassesThroughNonRPCErrors covers the dial
+// failure / decode failure paths where rpc.Call returns a plain Go
+// error rather than *rpc.ErrorResponse. The translator must not
+// hide the original message — that's the only signal an operator
+// has when the daemon is down.
+func TestTranslateRPCErrorPassesThroughNonRPCErrors(t *testing.T) {
+	var buf bytes.Buffer
+	got := translateRPCError(&buf, errors.New("dial unix /run/banger/bangerd.sock: connect: no such file or directory"))
+	if !strings.Contains(got, "no such file or directory") {
+		t.Fatalf("plain error lost: got %q", got)
+	}
+}

internal/cli/printers.go

@@ -3,12 +3,14 @@ package cli
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"sort"
 	"strings"
 	"text/tabwriter"
 
 	"banger/internal/api"
+	"banger/internal/cli/style"
 	"banger/internal/model"
 	"banger/internal/system"
 )
@@ -278,8 +280,19 @@ func printKernelCatalogTable(out anyWriter, entries []api.KernelCatalogEntry) er
 // -- doctor printer -------------------------------------------------
 
 func printDoctorReport(out anyWriter, report system.Report) error {
+	colorWriter, _ := out.(io.Writer)
 	for _, check := range report.Checks {
 		status := strings.ToUpper(string(check.Status))
+		if colorWriter != nil {
+			switch check.Status {
+			case system.CheckStatusPass:
+				status = style.Pass(colorWriter, status)
+			case system.CheckStatusFail:
+				status = style.Fail(colorWriter, status)
+			case system.CheckStatusWarn:
+				status = style.Warn(colorWriter, status)
+			}
+		}
 		if _, err := fmt.Fprintf(out, "%s\t%s\n", status, check.Name); err != nil {
 			return err
 		}

internal/cli/style/style.go

@@ -0,0 +1,70 @@
+// Package style provides a tiny, conservative ANSI-color helper for
+// banger's CLI. The contract:
+//
+//   - Each helper takes the writer the styled string is going to and
+//     returns either the wrapped string or the plain one.
+//   - "Wrapped" only happens when the writer is a TTY AND the
+//     NO_COLOR environment variable is unset.
+//   - No 256-color or truecolor; no theme system; no external dep.
+//
+// Banger's CLI uses these for status (pass/fail/warn), error
+// prefixes, and dim secondary text. Anything richer belongs in a
+// dedicated TUI layer that this package isn't.
+package style
+
+import (
+	"io"
+	"os"
+	"strings"
+)
+
+// ANSI escape sequences. Kept private — callers compose meaning via
+// the named helpers (Pass/Fail/Warn/...), not raw codes.
+const (
+	ansiReset = "\x1b[0m"
+	ansiBold  = "\x1b[1m"
+	ansiDim   = "\x1b[2m"
+	ansiRed   = "\x1b[31m"
+	ansiGreen = "\x1b[32m"
+	ansiYel   = "\x1b[33m"
+)
+
+// Pass wraps s in green when w is a TTY and NO_COLOR is unset.
+func Pass(w io.Writer, s string) string { return wrap(w, ansiGreen, s) }
+
+// Fail wraps s in red.
+func Fail(w io.Writer, s string) string { return wrap(w, ansiRed, s) }
+
+// Warn wraps s in yellow.
+func Warn(w io.Writer, s string) string { return wrap(w, ansiYel, s) }
+
+// Dim wraps s in dim.
+func Dim(w io.Writer, s string) string { return wrap(w, ansiDim, s) }
+
+// Bold wraps s in bold.
+func Bold(w io.Writer, s string) string { return wrap(w, ansiBold, s) }
+
+// SupportsColor reports whether colored output should be emitted to
+// w. Exposed so callers that build multi-segment strings can avoid
+// duplicating the gate per call.
+func SupportsColor(w io.Writer) bool {
+	if strings.TrimSpace(os.Getenv("NO_COLOR")) != "" {
+		return false
+	}
+	file, ok := w.(*os.File)
+	if !ok {
+		return false
+	}
+	info, err := file.Stat()
+	if err != nil {
+		return false
+	}
+	return info.Mode()&os.ModeCharDevice != 0
+}
+
+func wrap(w io.Writer, code, s string) string {
+	if !SupportsColor(w) {
+		return s
+	}
+	return code + s + ansiReset
+}

internal/cli/style/style_test.go

@@ -0,0 +1,64 @@
+package style
+
+import (
+	"bytes"
+	"os"
+	"strings"
+	"testing"
+)
+
+// TestStyleNoOpsForNonTTYWriter pins that styled helpers don't emit
+// ANSI escapes when the destination isn't a terminal. Buffers stand
+// in for any non-TTY writer (CI, redirected stdout, log files).
+func TestStyleNoOpsForNonTTYWriter(t *testing.T) {
+	var buf bytes.Buffer
+	cases := map[string]string{
+		"pass": Pass(&buf, "ok"),
+		"fail": Fail(&buf, "boom"),
+		"warn": Warn(&buf, "huh"),
+		"dim":  Dim(&buf, "sub"),
+		"bold": Bold(&buf, "bold"),
+	}
+	for label, got := range cases {
+		if strings.Contains(got, "\x1b[") {
+			t.Errorf("%s: contains ANSI escape on non-TTY writer: %q", label, got)
+		}
+	}
+}
+
+// TestStyleSuppressedByNoColor pins https://no-color.org compliance:
+// even on a "real" TTY, NO_COLOR forces plain output.
+func TestStyleSuppressedByNoColor(t *testing.T) {
+	t.Setenv("NO_COLOR", "1")
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("Pipe: %v", err)
+	}
+	defer r.Close()
+	defer w.Close()
+	// w is a pipe end, not a char device — NO_COLOR is the dominant
+	// gate but verifying the helper still suppresses guards against
+	// a future TTY-detection regression that would otherwise need a
+	// pty harness to surface.
+	if got := Pass(w, "ok"); strings.Contains(got, "\x1b[") {
+		t.Errorf("NO_COLOR set but Pass() emitted ANSI: %q", got)
+	}
+	if got := Fail(w, "boom"); strings.Contains(got, "\x1b[") {
+		t.Errorf("NO_COLOR set but Fail() emitted ANSI: %q", got)
+	}
+}
+
+// TestSupportsColorRespectsNoColor confirms the gate function used
+// by the helpers. Required for callers that compose multi-segment
+// strings and want to ask once.
+func TestSupportsColorRespectsNoColor(t *testing.T) {
+	t.Setenv("NO_COLOR", "1")
+	tmp, err := os.CreateTemp(t.TempDir(), "style-*")
+	if err != nil {
+		t.Fatalf("CreateTemp: %v", err)
+	}
+	defer tmp.Close()
+	if SupportsColor(tmp) {
+		t.Fatal("SupportsColor returned true with NO_COLOR set")
+	}
+}

internal/cli/vm_create.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"banger/internal/api"
+	"banger/internal/cli/style"
 	"banger/internal/config"
 	"banger/internal/model"
 	"banger/internal/paths"
@@ -52,7 +53,7 @@ func printVMSpecLine(out io.Writer, params api.VMCreateParams) {
 			diskBytes = parsed
 		}
 	}
-	_, _ = fmt.Fprintf(out, "spec: %d vcpu · %d MiB · %s disk\n",
+	_, _ = fmt.Fprintf(out, "spec: %d vcpu | %d MiB | %s disk\n",
 		vcpu, memory, model.FormatSizeBytes(diskBytes))
 }
 
@@ -75,7 +76,8 @@ func (d *deps) runVMCreate(ctx context.Context, socketPath string, stderr io.Wri
 		if op.Done {
 			renderer.render(op)
 			if op.Success && op.VM != nil {
-				_, _ = fmt.Fprintf(stderr, "[vm create] ready in %s\n", formatVMCreateElapsed(time.Since(start)))
+				elapsed := formatVMCreateElapsed(time.Since(start))
+				_, _ = fmt.Fprintf(stderr, "[vm create] ready in %s\n", style.Dim(stderr, elapsed))
 				return *op.VM, nil
 			}
 			if strings.TrimSpace(op.Error) == "" {

scripts/smoke.sh

@@ -134,8 +134,8 @@ sudo env \
 sudo touch "$smoke_marker"
 
 status_out="$("$BANGER" system status)" || die 'system status failed after install'
-grep -q 'active: active' <<<"$status_out" || die "owner daemon not active after install: $status_out"
+grep -qE '^active +active' <<<"$status_out" || die "owner daemon not active after install: $status_out"
-grep -q 'helper_active: active' <<<"$status_out" || die "root helper not active after install: $status_out"
+grep -qE '^helper_active +active' <<<"$status_out" || die "root helper not active after install: $status_out"
 
 log 'doctor: checking host readiness'
 if ! "$BANGER" doctor; then
@@ -145,8 +145,8 @@ fi
 log 'system restart: services should come back cleanly'
 sudo_banger "$BANGER" system restart >/dev/null || die 'system restart failed'
 status_out="$("$BANGER" system status)" || die 'system status failed after restart'
-grep -q 'active: active' <<<"$status_out" || die "owner daemon not active after restart: $status_out"
+grep -qE '^active +active' <<<"$status_out" || die "owner daemon not active after restart: $status_out"
-grep -q 'helper_active: active' <<<"$status_out" || die "root helper not active after restart: $status_out"
+grep -qE '^helper_active +active' <<<"$status_out" || die "root helper not active after restart: $status_out"
 
 # --- bare vm run ------------------------------------------------------
 log "bare vm run: create + start + ssh + exec 'echo smoke-bare-ok' + --rm"