fix(cli): keep dirctl logs off stdout

cli/cli.go

@@ -10,9 +10,17 @@ import (
 	"syscall"
 
 	"github.com/agntcy/dir/cli/cmd"
+	"github.com/agntcy/dir/utils/logging"
 )
 
 func main() {
+	// dirctl's result output is machine-readable (stdout is often piped into
+	// jq, parsed as JSON, etc.), so diagnostic logs default to stderr instead
+	// of the package-wide stdout default used by server/reconciler binaries.
+	// An explicit DIRECTORY_LOGGER_LOG_FILE or DIRECTORY_LOGGER_LOG_STREAM
+	// still wins over this (see utils/logging.SetDefaultOutput).
+	logging.SetDefaultOutput(os.Stderr)
+
 	ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGHUP, syscall.SIGTERM)
 
 	if err := cmd.Run(ctx); err != nil {

cli/cmd/daemon/start.go

@@ -41,6 +41,11 @@ The daemon blocks until SIGINT or SIGTERM is received.`,
 
 //nolint:cyclop
 func runStart(cmd *cobra.Command, _ []string) error {
+	// `daemon start` bundles the apiserver and reconciler in-process, so it
+	// keeps their stdout logging behavior rather than the stderr default
+	// cli.go sets for normal dirctl commands.
+	logging.SetDefaultOutput(os.Stdout)
+
 	running, pid, err := readPID()
 	if err != nil {
 		return err

utils/logging/config.go

@@ -21,6 +21,10 @@ type Config struct {
 	LogFile   string `json:"log_file,omitempty"   mapstructure:"log_file"`
 	LogLevel  string `json:"log_level,omitempty"  mapstructure:"log_level"`
 	LogFormat string `json:"log_format,omitempty" mapstructure:"log_format"`
+	// LogStream explicitly selects "stdout" or "stderr". Left empty, it means
+	// "use the binary's default" - deliberately has no Viper default, since
+	// that default differs per binary (see SetDefaultOutput).
+	LogStream string `json:"log_stream,omitempty" mapstructure:"log_stream"`
 }
 
 func LoadConfig() (*Config, error) {
@@ -41,6 +45,10 @@ func LoadConfig() (*Config, error) {
 	_ = v.BindEnv("log_format")
 	v.SetDefault("log_format", DefaultLogFormat)
 
+	// No default: an unset log_stream means "use the binary's default",
+	// which InitLogger/SetDefaultOutput resolve per binary.
+	_ = v.BindEnv("log_stream")
+
 	// Load configuration into struct
 	decodeHooks := mapstructure.ComposeDecodeHookFunc(
 		mapstructure.TextUnmarshallerHookFunc(),

utils/logging/config_test.go

@@ -14,6 +14,7 @@ func TestLoadConfigWithDefaults(t *testing.T) {
 	os.Unsetenv("DIRECTORY_LOGGER_LOG_FILE")
 	os.Unsetenv("DIRECTORY_LOGGER_LOG_LEVEL")
 	os.Unsetenv("DIRECTORY_LOGGER_LOG_FORMAT")
+	os.Unsetenv("DIRECTORY_LOGGER_LOG_STREAM")
 
 	cfg, err := LoadConfig()
 	if err != nil {
@@ -32,6 +33,30 @@ func TestLoadConfigWithDefaults(t *testing.T) {
 	if cfg.LogFile != "" {
 		t.Errorf("Expected LogFile='', got: %s", cfg.LogFile)
 	}
+
+	if cfg.LogStream != "" {
+		t.Errorf("Expected LogStream='' (use binary default), got: %s", cfg.LogStream)
+	}
+}
+
+// TestLoadConfigLogStreamValues verifies DIRECTORY_LOGGER_LOG_STREAM loads through
+// unchanged for each supported value (unset stays empty - see TestLoadConfigWithDefaults
+// for why that matters).
+func TestLoadConfigLogStreamValues(t *testing.T) {
+	for _, stream := range []string{"stdout", "stderr"} {
+		t.Run(stream, func(t *testing.T) {
+			t.Setenv("DIRECTORY_LOGGER_LOG_STREAM", stream)
+
+			cfg, err := LoadConfig()
+			if err != nil {
+				t.Fatalf("LoadConfig() failed: %v", err)
+			}
+
+			if cfg.LogStream != stream {
+				t.Errorf("Expected LogStream=%s, got: %s", stream, cfg.LogStream)
+			}
+		})
+	}
 }
 
 // TestLoadConfigWithEnvVars verifies environment variable configuration.
@@ -40,6 +65,7 @@ func TestLoadConfigWithEnvVars(t *testing.T) {
 	t.Setenv("DIRECTORY_LOGGER_LOG_FILE", "/tmp/test.log")
 	t.Setenv("DIRECTORY_LOGGER_LOG_LEVEL", "DEBUG")
 	t.Setenv("DIRECTORY_LOGGER_LOG_FORMAT", "json")
+	t.Setenv("DIRECTORY_LOGGER_LOG_STREAM", "stderr")
 
 	cfg, err := LoadConfig()
 	if err != nil {
@@ -58,6 +84,10 @@ func TestLoadConfigWithEnvVars(t *testing.T) {
 	if cfg.LogFormat != "json" {
 		t.Errorf("Expected LogFormat='json', got: %s", cfg.LogFormat)
 	}
+
+	if cfg.LogStream != "stderr" {
+		t.Errorf("Expected LogStream='stderr', got: %s", cfg.LogStream)
+	}
 }
 
 // TestLoadConfigWithPartialEnvVars verifies partial environment variable configuration.
@@ -157,6 +187,7 @@ func TestConfigJSONMarshaling(t *testing.T) {
 		LogFile:   "/var/log/app.log",
 		LogLevel:  "INFO",
 		LogFormat: "json",
+		LogStream: "stdout",
 	}
 
 	// Just verify the struct is valid and fields are accessible
@@ -171,6 +202,10 @@ func TestConfigJSONMarshaling(t *testing.T) {
 	if cfg.LogFormat != "json" {
 		t.Errorf("LogFormat mismatch")
 	}
+
+	if cfg.LogStream != "stdout" {
+		t.Errorf("LogStream mismatch")
+	}
 }
 
 // TestConfigConstants verifies all constants are correctly defined.

utils/logging/logging.go

@@ -4,6 +4,7 @@
 package logging
 
 import (
+	"io"
 	"log/slog"
 	"os"
 	"strings"
@@ -16,23 +17,102 @@ const (
 	// Log format types.
 	formatJSON = "json"
 	formatText = "text"
+
+	// Log stream types.
+	streamStdout = "stdout"
+	streamStderr = "stderr"
 )
 
 var once sync.Once
 
-// getLogOutput determines where logs should be written.
-func getLogOutput(logFilePath string) *os.File {
-	if logFilePath != "" {
-		// Try to open or create the log file.
-		file, err := os.OpenFile(logFilePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, filePermission)
-		if err == nil {
-			return file
-		}
+// switchableWriter is an io.Writer whose target can be swapped after
+// construction. slog handlers bind to a writer once, at construction time -
+// including the ~60 `var logger = logging.Logger(component)` package-level
+// loggers in this repo, all constructed before any binary's main() runs.
+// Routing writes through a switchableWriter lets SetDefaultOutput redirect
+// those already-constructed loggers later, without a custom slog.Handler.
+type switchableWriter struct {
+	mu sync.RWMutex
+	w  io.Writer
+}
+
+func newSwitchableWriter(w io.Writer) *switchableWriter {
+	return &switchableWriter{w: w}
+}
+
+func (s *switchableWriter) Write(p []byte) (int, error) {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	//nolint:wrapcheck // io.Writer implementations pass through the underlying error unwrapped.
+	return s.w.Write(p)
+}
+
+func (s *switchableWriter) Set(w io.Writer) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s.w = w
+}
+
+// defaultWriter is the process-wide fallback output target, used only when
+// InitLogger did not bind the handler to an explicit log file or an explicit
+// DIRECTORY_LOGGER_LOG_STREAM. Binaries pick their own default via
+// SetDefaultOutput.
+var defaultWriter = newSwitchableWriter(os.Stdout)
+
+// getFileOutput attempts to open the configured log file. It returns nil if
+// no file is configured, or if opening it fails - the failure is logged so
+// the caller can fall back to the next output in the precedence order.
+func getFileOutput(logFilePath string) *os.File {
+	if logFilePath == "" {
+		return nil
+	}
+
+	file, err := os.OpenFile(logFilePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, filePermission)
+	if err != nil {
+		slog.Error("Failed to open log file, falling back", "error", err)
+
+		return nil
+	}
+
+	return file
+}
+
+// resolveExplicitStream returns the writer for an explicit
+// DIRECTORY_LOGGER_LOG_STREAM value. ok is false when the stream is unset or
+// invalid, meaning the caller should fall back to the binary-default writer.
+func resolveExplicitStream(logStream string) (io.Writer, bool) {
+	switch strings.ToLower(strings.TrimSpace(logStream)) {
+	case "":
+		return nil, false
+	case streamStdout:
+		return os.Stdout, true
+	case streamStderr:
+		return os.Stderr, true
+	default:
+		slog.Warn("Invalid log stream, using default output", "log_stream", logStream)
+
+		return nil, false
+	}
+}
+
+// resolveOutput implements the output-selection precedence required by
+// #2009:
+//  1. An explicit log file that opens successfully always wins.
+//  2. Otherwise an explicit LogStream (stdout/stderr) wins.
+//  3. Otherwise logs flow through defaultWriter, which the binary can
+//     redirect later via SetDefaultOutput.
+func resolveOutput(cfg *Config) io.Writer {
+	if file := getFileOutput(cfg.LogFile); file != nil {
+		return file
+	}
 
-		slog.Error("Failed to open log file, defaulting to stdout", "error", err)
+	if w, ok := resolveExplicitStream(cfg.LogStream); ok {
+		return w
 	}
 
-	return os.Stdout
+	return defaultWriter
 }
 
 // InitLogger initializes the global logger with the provided configuration.
@@ -42,36 +122,43 @@ func InitLogger(cfg *Config) {
 	once.Do(func() {
 		var logLevel slog.Level
 
-		logOutput := getLogOutput(cfg.LogFile)
-
 		// Parse log level; default to INFO if invalid.
 		if err := logLevel.UnmarshalText([]byte(strings.ToLower(cfg.LogLevel))); err != nil {
 			slog.Warn("Invalid log level, defaulting to INFO", "error", err)
 
 			logLevel = slog.LevelInfo
 		}
 
+		output := resolveOutput(cfg)
+
 		// Create handler based on format
 		var handler slog.Handler
 
 		opts := &slog.HandlerOptions{Level: logLevel}
 
 		switch strings.ToLower(cfg.LogFormat) {
 		case formatJSON:
-			handler = slog.NewJSONHandler(logOutput, opts)
+			handler = slog.NewJSONHandler(output, opts)
 		case formatText:
-			handler = slog.NewTextHandler(logOutput, opts)
+			handler = slog.NewTextHandler(output, opts)
 		default:
 			slog.Warn("Invalid log format, defaulting to text", "format", cfg.LogFormat)
 
-			handler = slog.NewTextHandler(logOutput, opts)
+			handler = slog.NewTextHandler(output, opts)
 		}
 
 		// Set global logger before other packages initialize.
 		slog.SetDefault(slog.New(handler))
 	})
 }
 
+// SetDefaultOutput redirects defaultWriter (see above). It has no effect on
+// loggers bound to an explicit log file or DIRECTORY_LOGGER_LOG_STREAM, since
+// those write directly to their configured target instead.
+func SetDefaultOutput(w io.Writer) {
+	defaultWriter.Set(w)
+}
+
 func Logger(component string) *slog.Logger {
 	return slog.Default().With("component", component)
 }