fix: detect TTY with real isatty check instead of char-device test

[pilat]

Why

devbox run <scenario> and devbox shell fail in any non-interactive environment — CI,
background jobs, nohup, or stdin redirected from /dev/null — with:

cannot attach stdin to a TTY-enabled container because stdin is not a terminal

This happens even for scenarios that rely on TTY auto-detection (no explicit tty: set).

The root cause is a too-loose terminal check. isTTYAvailable treated any character device
as a terminal:

return (stat.Mode() & os.ModeCharDevice) != 0

But /dev/null — what a detached/headless process gets on stdin — is a character device
that is not a terminal. devbox then forced tty=true on the compose exec, and the
docker/cli layer below correctly refused it. devbox's own detection and docker/cli's
enforcement disagreed exactly on /dev/null.

What

• Use a real isatty check. Replace the ModeCharDevice test with golang.org/x/term's
  term.IsTerminal(int(f.Fd())). It performs the same tcget ioctl docker/cli uses
  downstream, so devbox's detection and the compose layer's enforcement now agree.
• Inject stdin as a parameter — isTTYAvailable(f *os.File) — so the detection is unit
  testable instead of reaching for the global os.Stdin.
• Add a regression test covering the non-terminal cases: /dev/null, a regular file,
  and a pipe. The /dev/null case is the one that failed under the old logic.

Notes

• Only the auto-detect path changes. An explicit tty: in a scenario config still takes
  precedence and is untouched.
• golang.org/x/term was already a direct dependency (used by internal/table), so no new
  modules — go.mod/go.sum are unchanged.

Summary by CodeRabbit

• Bug Fixes
  • Improved how the app detects whether an interactive terminal is available.
  • Interactive commands now make better decisions about when to enable terminal-based behavior, especially when input is piped or redirected.
  • This should make shell and scenario execution more reliable across different terminal environments.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 665056e8-88d2-4cd8-a043-37f6d6e71ec6

📥 Commits

Reviewing files that changed from the base of the PR and between 3c382e1 and 4fea925.

⛔ Files ignored due to path filters (1)

• cmd/devbox/tty_test.go is excluded by !**/*_test.go

📒 Files selected for processing (2)

• cmd/devbox/run.go
• cmd/devbox/shell.go

---

Walkthrough

The isTTYAvailable function is changed to accept an *os.File parameter and now determines TTY status via term.IsTerminal(int(f.Fd())) instead of os.Stdin.Stat() with os.ModeCharDevice. Call sites in cmd/devbox/run.go and cmd/devbox/shell.go are updated to pass os.Stdin explicitly.

Changes

TTY detection refactor

Layer / File(s)	Summary
isTTYAvailable helper reimplementation cmd/devbox/shell.go	Adds golang.org/x/term import and replaces the os.Stdin.Stat()/os.ModeCharDevice-based check with isTTYAvailable(f *os.File) returning term.IsTerminal(int(f.Fd())).
Call site updates to pass os.Stdin cmd/devbox/run.go, cmd/devbox/shell.go	Adds os import where needed and updates runShell and the run command's TTY check to call isTTYAvailable(os.Stdin) instead of the no-argument version.

Sequence Diagram(s)

sequenceDiagram
  participant runCmd as run command
  participant runShell
  participant isTTYAvailable
  participant term as golang.org/x/term

  runCmd->>isTTYAvailable: isTTYAvailable(os.Stdin)
  runShell->>isTTYAvailable: isTTYAvailable(os.Stdin)
  isTTYAvailable->>term: term.IsTerminal(int(f.Fd()))
  term-->>isTTYAvailable: bool
  isTTYAvailable-->>runCmd: tty flag
  isTTYAvailable-->>runShell: tty flag

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• pilat/devbox#5: Modifies the same isTTYAvailable logic and its call sites in cmd/devbox/run.go and cmd/devbox/shell.go.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: replacing the char-device TTY check with a real isatty-based terminal detection.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pilat/tty-issue

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}