Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 193 additions & 0 deletions e2e/setup.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
#!/usr/bin/env bats
# setup.bats - `basecamp setup` refuses to prompt when nothing can answer it.
#
# The wizard is prompts end to end, and huh runs them as a bubbletea program.
# Redirecting stdin does not make that program fail: bubbletea sees a
# non-terminal stdin and opens /dev/tty instead, so the prompt waits on the real
# terminal — `basecamp setup --json < /dev/null` hung forever.
#
# Every case runs under a timeout, and the timeout is the assertion: exit 124 is
# the bug reproducing. A unit test with a fake reader cannot catch this, because
# the hang lives in what the real os.Stdin makes bubbletea do — which is also
# why the PTY case at the bottom exists. Under bats alone, stdout is captured,
# so stdout is what fails the interactivity check and stdin is never the
# deciding factor.

load test_helper

# timeout_bin names GNU timeout, which stock macOS does not ship. Tests that
# need it skip rather than silently drop their only real assertion.
timeout_bin() {
if command -v timeout >/dev/null 2>&1; then
echo timeout
elif command -v gtimeout >/dev/null 2>&1; then
echo gtimeout
fi
}

# run_guarded runs a shell snippet under a timeout.
#
# The timeout wraps the whole shell, not the snippet's first word. Prefixing it
# (`timeout 10 printf '' | basecamp setup`) times `printf` and leaves `basecamp`
# to hang the suite forever — the exact failure these tests exist to catch.
# pipefail so a refusal upstream of a pipe is not masked by the last stage.
run_guarded() {
local to
to="$(timeout_bin)"
if [[ -z "$to" ]]; then
skip "no GNU timeout available (install coreutils)"
fi
run "$to" 10 bash -c "set -o pipefail; $1"
}

assert_not_timed_out() {
if [[ "$status" -eq 124 ]]; then
echo "Command hit the timeout (exit 124) — it hung instead of refusing"
echo "Output: $output"
return 1
fi
}

# assert_refused is the whole contract: exit 1, a usage envelope, and a hint
# naming something that works without a terminal. "Non-zero and not 124" is too
# weak — the pre-fix binary also exits non-zero where no controlling terminal
# exists, on huh's own TTY error, several steps in.
assert_refused() {
assert_not_timed_out
assert_exit_code 1
assert_json_value '.code' 'usage'
assert_json_value '.hint | contains("basecamp setup agents")' 'true'
}


@test "setup --json with stdin closed refuses instead of hanging" {
create_credentials
create_global_config '{"account_id": 99999}'

run_guarded "basecamp setup --json < /dev/null"
assert_refused
}

@test "setup without --json and stdin closed refuses instead of hanging" {
create_credentials
create_global_config '{"account_id": 99999}'

run_guarded "basecamp setup < /dev/null"
assert_refused
}

@test "setup with piped stdin refuses instead of hanging" {
create_credentials
create_global_config '{"account_id": 99999}'

run_guarded "printf '' | basecamp setup --json"
assert_refused
}

@test "setup with piped stdout refuses instead of hanging" {
create_credentials
create_global_config '{"account_id": 99999}'

run_guarded "basecamp setup --json < /dev/null | cat"
assert_refused
}


# The reported bug: a real terminal on stdout, nothing on stdin. bats cannot
# produce that on its own — it captures stdout — so borrow a pty from script(1).
# The exit code travels in a sentinel rather than through script, whose status
# propagation differs between the util-linux and BSD versions.

# run_in_pty runs a shell snippet with stdout attached to a pseudo-terminal.
run_in_pty() {
local snippet="$1"
if ! command -v script >/dev/null 2>&1; then
skip "script(1) not available to allocate a pty"
fi
if script --version 2>&1 | grep -qi util-linux; then
run bash -c "script -qec '$snippet' /dev/null | tr -d '\r'"
else
run bash -c "script -q /dev/null /bin/sh -c '$snippet' | tr -d '\r'"
fi
}

@test "setup --json on a terminal with stdin closed refuses instead of hanging" {
create_credentials
create_global_config '{"account_id": 99999}'

local to
to="$(timeout_bin)"
if [[ -z "$to" ]]; then
skip "no GNU timeout available (install coreutils)"
fi

# stdout is a pty here, so /dev/null on stdin is the only disqualifier — the
# case a character-device check called interactive and bubbletea did not.
run_in_pty "$to 10 basecamp setup --json < /dev/null; echo EXIT:\$?"

assert_output_not_contains "EXIT:124"
assert_output_contains "EXIT:1"
assert_output_contains "basecamp setup agents"
}

@test "setup on a terminal with stdin closed refuses instead of hanging" {
create_credentials
create_global_config '{"account_id": 99999}'

local to
to="$(timeout_bin)"
if [[ -z "$to" ]]; then
skip "no GNU timeout available (install coreutils)"
fi

run_in_pty "$to 10 basecamp setup < /dev/null; echo EXIT:\$?"

assert_output_not_contains "EXIT:124"
assert_output_contains "EXIT:1"
assert_output_contains "basecamp setup agents"
}


# The gate is on the parent's RunE only. These three are the supported
# non-interactive paths and have to keep working — a persistent hook would have
# taken all of them out, which is the easiest thing to get wrong here.

# hide_agent_binaries drops the developer's real claude/codex from PATH. Without
# it these tests shell out to whichever agent CLI happens to be installed, and
# those have prompts of their own — a hang in somebody else's tool, unrelated to
# the gate under test. What we are asserting is that the parent's gate does not
# reach the subcommands, and that holds with or without an agent installed.
hide_agent_binaries() {
export PATH="$BASECAMP_ROOT/bin:/usr/bin:/bin"
}

@test "setup agents still runs without a terminal" {
create_credentials
create_global_config '{"account_id": 99999}'
hide_agent_binaries
export BASECAMP_SETUP_AGENT=none

run_guarded "basecamp setup agents --json < /dev/null"
assert_not_timed_out
assert_success
}

@test "setup claude still runs without a terminal" {
create_credentials
create_global_config '{"account_id": 99999}'
hide_agent_binaries

run_guarded "basecamp setup claude --json < /dev/null"
assert_not_timed_out
assert_success
}

@test "setup codex still runs without a terminal" {
create_credentials
create_global_config '{"account_id": 99999}'
hide_agent_binaries

run_guarded "basecamp setup codex --json < /dev/null"
assert_not_timed_out
assert_success
}
9 changes: 5 additions & 4 deletions internal/appctx/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,11 @@ func (a *App) IsInteractive() bool {
return false
}

// Both stdout and stdin must be character devices: a TUI draws to stdout
// and reads keystrokes from stdin, so a pipe on either end can never
// drive one — and when the command is consuming piped content (a "-"
// stdin input), a TUI would eat that content as key events.
// Both stdout and stdin must be terminals: a TUI draws to stdout and reads
// keystrokes from stdin, so a pipe on either end can never drive one — and
// when the command is consuming piped content (a "-" stdin input), a TUI
// would eat that content as key events. A character device is not enough;
// /dev/null is one and delivers nothing. See stdinarg.InteractiveStdio.
return stdinarg.InteractiveStdio()
}

Expand Down
33 changes: 19 additions & 14 deletions internal/appctx/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net/http"
"net/http/httptest"
"os"
"runtime"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -205,30 +206,34 @@ func TestIsInteractiveWithCountMode(t *testing.T) {
}

func TestIsInteractiveWithNonInteractiveEnv(t *testing.T) {
// Swap os.Stdout to the null device — a char device that passes the
// ModeCharDevice guard — so IsInteractive() would otherwise return true.
// Without this, go test's piped stdout makes IsInteractive() false regardless
// of the env var, and the assertion would pass even if the short-circuit were
// removed.
devNull, err := os.Open(os.DevNull)
// Point both ends at a pseudo-terminal so IsInteractive() would otherwise
// return true. Without this, go test's piped stdout and /dev/null stdin make
// it false regardless of the env var, and the assertion would pass even if
// the short-circuit were removed. /dev/null will not stand in for a terminal
// here: it is a character device but not a terminal, which is precisely the
// distinction IsInteractive() now draws.
if runtime.GOOS == "windows" {
t.Skip("no /dev/ptmx on Windows")
}
pty, err := os.OpenFile("/dev/ptmx", os.O_RDWR, 0)
if err != nil {
t.Skip(os.DevNull + " not available")
t.Skipf("open /dev/ptmx: %v", err)
}
origStdout := os.Stdout
os.Stdout = devNull
origStdout, origStdin := os.Stdout, os.Stdin
os.Stdout, os.Stdin = pty, pty
t.Cleanup(func() {
os.Stdout = origStdout
devNull.Close()
os.Stdout, os.Stdin = origStdout, origStdin
pty.Close()
})

cfg := &config.Config{}
app := NewApp(cfg)

// Baseline: char-device stdout, no env/flags → interactive.
// Baseline: terminal stdio, no env/flags → interactive.
t.Setenv("BASECAMP_NONINTERACTIVE", "")
require.True(t, app.IsInteractive(), "char-device stdout should be interactive without the escape hatch")
require.True(t, app.IsInteractive(), "terminal stdio should be interactive without the escape hatch")

// The env escape hatch forces non-interactive even with an interactive stdout.
// The env escape hatch forces non-interactive even with interactive stdio.
t.Setenv("BASECAMP_NONINTERACTIVE", "1")
assert.False(t, app.IsInteractive(), "BASECAMP_NONINTERACTIVE should force non-interactive")
// Output format is untouched — the escape hatch only disables prompts.
Expand Down
8 changes: 5 additions & 3 deletions internal/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,13 +578,15 @@ func profileNames(cfg *config.Config) string {
}

// isInteractiveTTY reports whether the profile picker may run: no
// noninteractive mode set, and both ends of stdio are character devices.
// noninteractive mode set, and both ends of stdio are terminals.
//
// Stdin counts because the picker is a TUI reading key events, and this runs
// from PersistentPreRunE — before any command touches its own input. Gating on
// stdout alone let "printf body | basecamp todos create -" open the picker on
// a terminal stdout and consume the piped body as keystrokes. Same predicate
// as App.IsInteractive and resolve.Resolver.IsInteractive.
// a terminal stdout and consume the piped body as keystrokes. A character
// device is not enough either: /dev/null is one, delivers no keystrokes, and
// Bubble Tea answers it by waiting on /dev/tty. Same predicate as
// App.IsInteractive and resolve.Resolver.IsInteractive.
func isInteractiveTTY(flags appctx.GlobalFlags) bool {
if config.NonInteractiveEnv() {
return false
Expand Down
29 changes: 18 additions & 11 deletions internal/cli/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cli
import (
"bytes"
"os"
"runtime"
"testing"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -166,7 +167,7 @@ func isolateRootTest(t *testing.T) {
}

func TestIsInteractiveTTYWithNonInteractiveEnv(t *testing.T) {
stubCharDeviceStdio(t)
stubTerminalStdio(t)

t.Setenv("BASECAMP_NONINTERACTIVE", "")
require.True(t, isInteractiveTTY(appctx.GlobalFlags{}))
Expand Down Expand Up @@ -283,10 +284,10 @@ func TestVersionWithJQReturnsUsageError(t *testing.T) {
// feeding a "-" content input the picker would eat that body as keystrokes —
// so a terminal stdout is not on its own enough to open one.
func TestIsInteractiveTTYRequiresUnpipedStdin(t *testing.T) {
stubCharDeviceStdio(t)
stubTerminalStdio(t)
t.Setenv("BASECAMP_NONINTERACTIVE", "")

require.True(t, isInteractiveTTY(appctx.GlobalFlags{}), "char-device stdio is interactive")
require.True(t, isInteractiveTTY(appctx.GlobalFlags{}), "terminal stdio is interactive")

reader, writer, err := os.Pipe()
require.NoError(t, err)
Expand All @@ -306,13 +307,12 @@ func TestIsInteractiveTTYRequiresUnpipedStdin(t *testing.T) {
// must stay nil for cobra's unknown-command handling), so quick-start's own
// interactive paths sit behind it. The e2e suite always has a piped stdout,
// which takes the machine-output branch and never reaches them — this covers
// the other side: a character-device stdout, the terminal stand-in, with a
// piped stdin. The root carries a subcommand so InstallDashGuard takes the
// the other side: a terminal stdout with a piped stdin. The root carries a subcommand so InstallDashGuard takes the
// pre-run branch production uses, not the Args branch for a childless root.
func TestRootDashGuardWithTerminalStdout(t *testing.T) {
isolateRootTest(t)

stubCharDeviceStdio(t)
stubTerminalStdio(t)
t.Setenv("BASECAMP_NONINTERACTIVE", "")

reader, writer, err := os.Pipe()
Expand Down Expand Up @@ -350,16 +350,23 @@ func TestRootDashGuardWithTerminalStdout(t *testing.T) {
// device, so interactivity assertions do not depend on how `go test` itself was
// invoked — a piped stdin on the test runner would otherwise fail the
// interactive baseline now that both streams are checked.
func stubCharDeviceStdio(t *testing.T) {
// stubTerminalStdio points stdio at a pseudo-terminal, the only stand-in
// isInteractiveTTY accepts. /dev/null used to serve here — it is a character
// device — but a character device is not a terminal, and treating it as one is
// how `cmd < /dev/null` ended up launching prompts that wait on /dev/tty.
func stubTerminalStdio(t *testing.T) {
t.Helper()
devNull, err := os.Open(os.DevNull)
if runtime.GOOS == "windows" {
t.Skip("no /dev/ptmx on Windows")
}
pty, err := os.OpenFile("/dev/ptmx", os.O_RDWR, 0)
if err != nil {
t.Skip(os.DevNull + " not available")
t.Skipf("open /dev/ptmx: %v", err)
}
origStdout, origStdin := os.Stdout, os.Stdin
os.Stdout, os.Stdin = devNull, devNull
os.Stdout, os.Stdin = pty, pty
t.Cleanup(func() {
os.Stdout, os.Stdin = origStdout, origStdin
devNull.Close()
pty.Close()
})
}
Loading