Add --log level and keep diagnostics off stdout

.golangci.yml

@@ -124,6 +124,18 @@ linters:
     - zerologlint
 
   settings:
+    forbidigo:
+      forbid:
+        # Re-add forbidigo's defaults (defining `forbid` replaces them).
+        - pattern: '^(fmt\.Print(|f|ln)|print|println)$'
+          msg: do not print to stdout directly; use fmt.Fprint with an explicit writer, the output formatter, or slog
+        - pattern: '^spew\.(ConfigState\.)?Dump$'
+          msg: remove debug spew.Dump calls before committing
+        # Only slog.Debug and slog.Warn are sanctioned; other levels map into the
+        # client's two-channel logging model.
+        - pattern: '^slog\.(Info|Error|Log|LogAttrs|DebugContext|InfoContext|WarnContext|ErrorContext)$'
+          msg: use slog.Debug or slog.Warn only
+
     funcorder:
       # Checks if the exported methods of a structure are placed before the non-exported ones.
       # Default: true

CONTRIBUTING.md

@@ -134,6 +134,24 @@ We recommend setting up your editor to run golangci-lint automatically. Most pop
 - **GoLand**: Install the Golangci-lint plugin
 - **Vim/Neovim**: Configure with ALE or similar linting engines
 
+### Logging
+
+The `dutctl` client keeps diagnostic logging separate from command output:
+
+- **stdout** carries results and agent/module output (the `output.Formatter`). Never log to stdout.
+- **stderr** carries client diagnostics via the standard `log/slog` package.
+
+Use only two levels:
+
+- `slog.Debug` — internal trace; hidden unless the user passes `--log debug`.
+- `slog.Warn` — non-fatal anomalies. By default (`--log warn`) warnings are collected and printed as a short summary when the command finishes, so they never interrupt streaming output.
+
+Other slog entry points (`slog.Info`, `slog.Error`, `slog.Log`, the `*Context` variants) are rejected by `forbidigo`. The handler still maps any level by severity (`>= Warn` → warn, else debug), but write `Debug`/`Warn` in code.
+
+Errors that should stop the command are **returned**, not logged — they bubble up to a single exit point and are rendered through the formatter (format-aware, on stderr). There is intentionally no error log level.
+
+The handler and the `--log` flag (`debug|warn|none`, default `warn`) live in [`cmds/dutctl/clilog.go`](cmds/dutctl/clilog.go).
+
 ### Documentation Style Guide
 
 - Use Markdown for documentation

cmds/dutctl/clilog.go

@@ -0,0 +1,222 @@
+// Copyright 2025 Blindspot Software
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"context"
+	"errors"
+	"flag"
+	"fmt"
+	"io"
+	"log/slog"
+	"strings"
+	"sync"
+
+	"github.com/BlindspotSoftware/dutctl/internal/style"
+)
+
+// logMode controls how diagnostic log records are handled. It is derived from
+// the --log flag and implements flag.Value, so an invalid --log value is
+// rejected during flag parsing (flag.ExitOnError prints the error plus usage
+// and exits) rather than being validated separately.
+type logMode int
+
+// Ensure implementing the flag.Value interface.
+var _ flag.Value = (*logMode)(nil)
+
+const (
+	// logModeNone drops all diagnostics.
+	logModeNone logMode = iota
+	// logModeWarn drops debug records and accumulates warnings into a summary
+	// that is flushed on termination (the default).
+	logModeWarn
+	// logModeDebug writes every record live to stderr in temporal order.
+	logModeDebug
+)
+
+// parseLogMode maps a --log flag value to a logMode. Unknown values are
+// rejected. The message is intentionally bare ("must be ...") because the flag
+// package wraps it as `invalid value %q for flag -log: <msg>`.
+func parseLogMode(s string) (logMode, error) {
+	switch s {
+	case "none":
+		return logModeNone, nil
+	case "warn":
+		return logModeWarn, nil
+	case "debug":
+		return logModeDebug, nil
+	default:
+		return 0, errors.New("must be debug, warn, or none")
+	}
+}
+
+// String renders the mode and provides the flag's default display.
+func (m *logMode) String() string {
+	switch *m {
+	case logModeNone:
+		return "none"
+	case logModeDebug:
+		return "debug"
+	default:
+		return "warn"
+	}
+}
+
+// Set parses and stores the flag value, returning an error for unknown values.
+func (m *logMode) Set(s string) error {
+	mode, err := parseLogMode(s)
+	if err != nil {
+		return err
+	}
+
+	*m = mode
+
+	return nil
+}
+
+// cliHandler is a slog.Handler tailored for an interactive CLI. It writes
+// diagnostics to stderr only, and dispatches purely on the record's level so
+// that any slog entry point (Debug/Info/Warn/Error/Log/...) is mapped into a
+// two-channel model: everything < Warn is "debug tier", everything >= Warn is
+// "warn tier".
+//
+// In warn mode, warn-tier records are accumulated and flushed as a summary on
+// termination so they never interrupt command output. In debug mode every
+// record is written live.
+type cliHandler struct {
+	w        io.Writer
+	mode     logMode
+	useColor bool
+	mu       *sync.Mutex // shared across WithAttrs/WithGroup copies
+	buf      *[]string   // accumulated warning lines, shared via pointer
+	attrs    []slog.Attr
+	groups   []string
+}
+
+// Ensure implementing the slog.Handler interface.
+var _ slog.Handler = (*cliHandler)(nil)
+
+// newCLIHandler creates a cliHandler writing to w.
+func newCLIHandler(w io.Writer, mode logMode, useColor bool) *cliHandler {
+	return &cliHandler{
+		w:        w,
+		mode:     mode,
+		useColor: useColor,
+		mu:       &sync.Mutex{},
+		buf:      &[]string{},
+	}
+}
+
+// Enabled reports whether a record at the given level should be handled.
+func (h *cliHandler) Enabled(_ context.Context, level slog.Level) bool {
+	switch h.mode {
+	case logModeNone:
+		return false
+	case logModeWarn:
+		return level >= slog.LevelWarn // drops Debug & Info (everything below Warn)
+	default: // logModeDebug
+		return true
+	}
+}
+
+// Handle writes (debug mode) or buffers (warn mode) a record.
+func (h *cliHandler) Handle(_ context.Context, rec slog.Record) error {
+	line := h.render(rec)
+
+	h.mu.Lock()
+	defer h.mu.Unlock()
+
+	if h.mode == logModeDebug {
+		fmt.Fprintln(h.w, line) // live, temporal order
+
+		return nil
+	}
+
+	// logModeWarn: only warn-tier (>= Warn) records reach here; Enabled gates the rest.
+	*h.buf = append(*h.buf, line)
+
+	return nil
+}
+
+// WithAttrs returns a copy with attrs appended, sharing the buffer and mutex so
+// warning accumulation stays global.
+func (h *cliHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
+	clone := *h
+	clone.attrs = append(append([]slog.Attr{}, h.attrs...), attrs...)
+
+	return &clone
+}
+
+// WithGroup returns a copy that prefixes subsequent attribute keys with name.
+func (h *cliHandler) WithGroup(name string) slog.Handler {
+	if name == "" {
+		return h
+	}
+
+	clone := *h
+	clone.groups = append(append([]string{}, h.groups...), name)
+
+	return &clone
+}
+
+// render formats a record as a single line: "[LEVEL ]msg key=value ...". The
+// level prefix is only added in debug mode, where records of different levels
+// are interleaved; in the warn summary the lines sit under a header so the
+// prefix is redundant.
+func (h *cliHandler) render(rec slog.Record) string {
+	var builder strings.Builder
+
+	if h.mode == logModeDebug {
+		builder.WriteString(rec.Level.String())
+		builder.WriteByte(' ')
+	}
+
+	builder.WriteString(rec.Message)
+
+	prefix := ""
+	if len(h.groups) > 0 {
+		prefix = strings.Join(h.groups, ".") + "."
+	}
+
+	writeAttr := func(a slog.Attr) {
+		builder.WriteByte(' ')
+		builder.WriteString(prefix)
+		builder.WriteString(a.Key)
+		builder.WriteByte('=')
+		builder.WriteString(a.Value.String())
+	}
+
+	for _, a := range h.attrs {
+		writeAttr(a)
+	}
+
+	rec.Attrs(func(a slog.Attr) bool {
+		writeAttr(a)
+
+		return true
+	})
+
+	return builder.String()
+}
+
+// Flush writes the accumulated warning summary (warn mode) and clears the
+// buffer. It is safe to call in any mode and when no warnings were recorded.
+func (h *cliHandler) Flush() {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+
+	if len(*h.buf) == 0 {
+		return
+	}
+
+	header := fmt.Sprintf("%s %d warning(s) during run:", style.MarkerWarning, len(*h.buf))
+	fmt.Fprintln(h.w, style.Colorize(h.useColor, style.Yellow, header))
+
+	for _, line := range *h.buf {
+		fmt.Fprintf(h.w, "  - %s\n", line)
+	}
+
+	*h.buf = (*h.buf)[:0]
+}

cmds/dutctl/clilog_test.go

@@ -0,0 +1,128 @@
+// Copyright 2025 Blindspot Software
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"bytes"
+	"log/slog"
+	"strings"
+	"testing"
+)
+
+func TestParseLogMode(t *testing.T) {
+	tests := []struct {
+		in      string
+		want    logMode
+		wantErr bool
+	}{
+		{"debug", logModeDebug, false},
+		{"warn", logModeWarn, false},
+		{"none", logModeNone, false},
+		{"", 0, true},
+		{"info", 0, true},
+		{"bogus", 0, true},
+	}
+
+	for _, tt := range tests {
+		got, err := parseLogMode(tt.in)
+		if tt.wantErr {
+			if err == nil {
+				t.Errorf("parseLogMode(%q): want error, got nil", tt.in)
+			}
+
+			continue
+		}
+
+		if err != nil {
+			t.Errorf("parseLogMode(%q): unexpected error: %v", tt.in, err)
+		}
+
+		if got != tt.want {
+			t.Errorf("parseLogMode(%q) = %v, want %v", tt.in, got, tt.want)
+		}
+	}
+}
+
+// newTestLogger returns a logger backed by a cliHandler in the given mode,
+// writing (colourless) to the returned buffer.
+func newTestLogger(mode logMode) (*slog.Logger, *cliHandler, *bytes.Buffer) {
+	var buf bytes.Buffer
+
+	h := newCLIHandler(&buf, mode, false)
+
+	return slog.New(h), h, &buf
+}
+
+func TestCLIHandler_WarnMode_BuffersUntilFlush(t *testing.T) {
+	logger, handler, buf := newTestLogger(logModeWarn)
+
+	logger.Debug("d1")          // dropped (debug tier)
+	logger.Info("i1")           // dropped (Info < Warn)
+	logger.Warn("w1", "k", "v") // buffered (warn tier)
+	logger.Error("e1")          // buffered (Error >= Warn → warn tier)
+
+	if buf.Len() != 0 {
+		t.Fatalf("warn mode wrote before flush:\n%s", buf.String())
+	}
+
+	handler.Flush()
+
+	out := buf.String()
+	if !strings.Contains(out, "2 warning(s) during run:") {
+		t.Errorf("missing summary header, got:\n%s", out)
+	}
+
+	for _, want := range []string{"- w1 k=v", "- e1"} {
+		if !strings.Contains(out, want) {
+			t.Errorf("summary missing %q, got:\n%s", want, out)
+		}
+	}
+
+	if strings.Contains(out, "d1") || strings.Contains(out, "i1") {
+		t.Errorf("debug/info leaked into warn summary:\n%s", out)
+	}
+}
+
+func TestCLIHandler_DebugMode_WritesLive(t *testing.T) {
+	logger, handler, buf := newTestLogger(logModeDebug)
+
+	logger.Debug("d1")
+	logger.Warn("w1")
+
+	out := buf.String()
+	if !strings.Contains(out, "DEBUG d1") || !strings.Contains(out, "WARN w1") {
+		t.Errorf("debug mode did not write live, got:\n%s", out)
+	}
+
+	before := buf.Len()
+	handler.Flush() // nothing buffered in debug mode
+
+	if buf.Len() != before {
+		t.Errorf("Flush wrote in debug mode: %q", buf.String()[before:])
+	}
+}
+
+func TestCLIHandler_NoneMode_Silent(t *testing.T) {
+	logger, handler, buf := newTestLogger(logModeNone)
+
+	logger.Debug("d1")
+	logger.Warn("w1")
+	logger.Error("e1")
+	handler.Flush()
+
+	if buf.Len() != 0 {
+		t.Errorf("none mode produced output:\n%s", buf.String())
+	}
+}
+
+func TestCLIHandler_FlushNoWarnings_NoOutput(t *testing.T) {
+	_, handler, buf := newTestLogger(logModeWarn)
+
+	handler.Flush()
+
+	if buf.Len() != 0 {
+		t.Errorf("Flush with no warnings produced output:\n%s", buf.String())
+	}
+}