Skip to content

Comments

fix: tty error#108

Open
yjp20 wants to merge 1 commit intonextfrom
youngjin/fix-tty
Open

fix: tty error#108
yjp20 wants to merge 1 commit intonextfrom
youngjin/fix-tty

Conversation

@yjp20
Copy link
Member

@yjp20 yjp20 commented Feb 20, 2026

I confirmed this works with claude code now, though still not super well. Hoping to improve that soon.

@yjp20 yjp20 changed the title Youngjin/fix tty fix: tty error Feb 20, 2026
@yjp20 yjp20 requested a review from bruce-hill February 20, 2026 11:31
Comment on lines +20 to +29
// Always output to stderr, in case we want to also output JSON so that the json is redirectable e.g. to jq.
opts = append(opts, tea.WithOutput(os.Stderr))

// If not a TTY, use stdin and disable renderer
if !term.IsTerminal(int(os.Stderr.Fd())) {
opts = append(opts,
tea.WithInput(os.Stdin),
tea.WithoutRenderer(),
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is a bit suspicious to me. I think it would be helpful to expand on the comments here and/or rework the logic a bit.

Scenario 1: using the tool with no pipes -> output to stderr?
Scenario 2: using the tool and piping stdout somewhere -> output charm stuff to stderr?
Scenario 3: using the tool and piping stderr somewhere -> read from stdin, output charm stuff to stderr, and no charm rendering?

I would expect the logic to look more like:

if !isatty(stdout) { // stdout is being piped somewhere
  if isatty(stderr) { // stderr is still a tty, we can use that for TUI stuff
    // output = stderr
  } else { // no tty on either stdout/stderr, this would be the agent case
    // disable output, force stdin as input instead of tty input
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants