From 4645715f307d0b6b7a5a4b619cad0d0a60e07f78 Mon Sep 17 00:00:00 2001
From: Amolith <amolith@secluded.site>
Date: Fri, 10 Apr 2026 12:03:30 -0600
Subject: [PATCH] Guard subcommand TUI entry against non-interactive

When a wrapped subcommand is invoked with no args, the code now checks
isInteractive() before entering the TUI session. In non-interactive
environments (stdin not a tty or KELD_NONINTERACTIVE set), the command
runs directly with resolved config instead of crashing with "could not
open TTY".

This fixes the systemd timer regression where backup invocations would
fail because root's TraverseChildren consumes --preset, leaving the
subcommand with an empty args slice that incorrectly triggered the TUI.

Task: td-B2KD251
---
 cmd/subcommands.go      |   4 +-
 cmd/subcommands_test.go | 115 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+), 2 deletions(-)
 create mode 100644 cmd/subcommands_test.go

diff --git a/cmd/subcommands.go b/cmd/subcommands.go
index caa6bdf6b64150778191be5a8ed2ff2f7d2d0a87..f7837e9df355b77fa31e98926174dbc465135f40 100644
--- a/cmd/subcommands.go
+++ b/cmd/subcommands.go
@@ -48,8 +48,8 @@ func registerSubcommands() {
 				// When invoked with no flags or arguments and the command
 				// is wrapped (has TUI screens), enter the interactive
 				// session with the command pre-selected, skipping the
-				// menu screen.
-				if len(args) == 0 && isWrappedCommand(commandName) {
+				// menu screen. Only do this when running interactively.
+				if len(args) == 0 && isWrappedCommand(commandName) && isInteractive() {
 					cmdName, preset, overrides, err := runInteractive(commandName)
 					if err != nil {
 						return err
diff --git a/cmd/subcommands_test.go b/cmd/subcommands_test.go
new file mode 100644
index 0000000000000000000000000000000000000000..3c6df4275af94c47b3f3d6fc18035902e52488cd
--- /dev/null
+++ b/cmd/subcommands_test.go
@@ -0,0 +1,115 @@
+package cmd
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestSubcommandNoArgsNonInteractiveRunsDirectly(t *testing.T) {
+	// Not parallel: mutates package-level isStdinTerminal.
+
+	// Override stdin terminal check to simulate non-interactive environment.
+	original := isStdinTerminal
+	t.Cleanup(func() {
+		isStdinTerminal = original
+	})
+	isStdinTerminal = func() bool { return false }
+
+	// Create a config with backup paths defined so the command can run.
+	configFile := setupConfigWithBackupPaths(t)
+	setRootFlagValuesForTest(t, "home@cloud", true, configFile)
+	t.Setenv("KELD_CONFIG_FILE", "")
+	t.Setenv("KELD_DRYRUN", "")
+
+	backup := lookupSubcommand(t, "backup")
+	out, err := captureStdout(t, func() error {
+		return backup.RunE(backup, []string{})
+	})
+
+	// Should NOT get a bubbletea TTY error. Instead, should either:
+	// - Succeed with dry-run output (if config has paths), or
+	// - Fail with a sensible error about missing inputs.
+	if err != nil {
+		errMsg := err.Error()
+		// The old bug would produce "bubbletea: error opening TTY" here.
+		if strings.Contains(errMsg, "bubbletea") || strings.Contains(errMsg, "TTY") {
+			t.Fatalf("non-interactive mode incorrectly tried to open TTY: %v", err)
+		}
+		// A different error is acceptable (e.g., missing paths).
+		// For now we just verify no TTY error occurred.
+		t.Logf("command returned error (expected for missing paths): %v", err)
+		return
+	}
+
+	// If no error, we should see dry-run output.
+	if !strings.Contains(out, `"backup"`) {
+		t.Fatalf("expected dry-run output to contain backup command, got:\n%s", out)
+	}
+}
+
+// setupConfigWithBackupPaths creates a config file with a preset that has
+// backup paths defined, allowing the backup command to run without args.
+func setupConfigWithBackupPaths(t *testing.T) string {
+	t.Helper()
+
+	dir := t.TempDir()
+	cfg := filepath.Join(dir, "config.toml")
+	err := os.WriteFile(cfg, []byte(`
+[global]
+repo = "/repos/default"
+
+["home@"]
+tag = "home"
+
+["home@".backup]
+_arguments = "/src /dst"
+
+["@cloud"]
+repo = "/repos/cloud"
+`), 0o600)
+	if err != nil {
+		t.Fatalf("writing fixture: %v", err)
+	}
+	t.Setenv("HOME", dir)
+
+	return cfg
+}
+
+func TestSubcommandNoArgsInteractiveEntersSession(t *testing.T) {
+	// Not parallel: mutates package-level isStdinTerminal.
+
+	// Override stdin terminal check to simulate interactive terminal.
+	original := isStdinTerminal
+	t.Cleanup(func() {
+		isStdinTerminal = original
+	})
+	isStdinTerminal = func() bool { return true }
+
+	configFile := setupConfigWithBackupPaths(t)
+	setRootFlagValuesForTest(t, "", true, configFile)
+	t.Setenv("KELD_CONFIG_FILE", "")
+	t.Setenv("KELD_DRYRUN", "")
+
+	backup := lookupSubcommand(t, "backup")
+
+	// In interactive mode with no args, runInteractive is called.
+	// Since there's no real TTY attached during test, bubbletea will
+	// try to open /dev/tty and fail. We assert this behavior is happening
+	// (rather than the command running directly).
+	_, err := captureStdout(t, func() error {
+		return backup.RunE(backup, []string{})
+	})
+
+	// The expected behavior: bubbletea tries to open TTY and fails.
+	if err == nil {
+		// If it succeeded, either we're not in interactive mode or
+		// the test environment has a TTY (unlikely in CI).
+		t.Fatal("expected bubbletea TTY error when simulating interactive mode, got success")
+	}
+	errMsg := err.Error()
+	if !strings.Contains(errMsg, "TTY") && !strings.Contains(errMsg, "tty") {
+		t.Fatalf("expected bubbletea TTY error in interactive mode, got: %v", err)
+	}
+}
\ No newline at end of file