Skip to content

Read stdin through the command, with shared read helpers - #187

Open
nathanjcochran wants to merge 3 commits into
mainfrom
nathan/improve-stdin
Open

Read stdin through the command, with shared read helpers#187
nathanjcochran wants to merge 3 commits into
mainfrom
nathan/improve-stdin

Conversation

@nathanjcochran

@nathanjcochran nathanjcochran commented Aug 6, 2026

Copy link
Copy Markdown
Member

Follow-up to #186, applying the same idea to input. Commands read through cmd.InOrStdin() rather than os.Stdin, and the ad-hoc terminal helpers in internal/cmd are replaced by util.ReadLine / util.ReadPassword / util.ReadAll, ported from Ghost.

The benefit is one place that knows how to read a line, a password, or piped input, instead of each command hand-rolling bufio and term calls. All three helpers run the blocking read on a goroutine and select on the context, so Ctrl-C unblocks a waiting prompt instead of hanging until the user hits enter. Tests drive the interactive paths by stubbing util.IsTerminal and util.ReadPassword centrally, rather than every command carrying its own override hooks.

The interactive menus needed the same treatment. BubbleTea silently defaults to os.Stdin when WithInput is omitted — and there's no compile error to catch it — so all four pickers (project, read replica, password recovery, MCP client) were reading real stdin regardless of what the command was given. They now get both streams explicitly, which is why oauthLogin carries the *cobra.Command instead of a writer.

Finally, every prompt that blocks on a real-time answer is now gated on whether stdin is a terminal, and names the flag to use when it isn't. Three were missing that check and would fail on an opaque EOF, or hang, when run in CI.

Bug fixes and behavior changes:

  • The password prompt never saved or restored terminal state, so cancelling one could leave the shell in raw mode with echo off. Affects db save-password, db connect's password recovery, service update-password, and auth login.
  • tiger mcp install with no argument now tells you to name a client, instead of failing somewhere inside BubbleTea.
  • tiger service delete without --confirm now points at that flag rather than dying on failed to read confirmation: EOF. Note this also stops a piped confirmation (echo svc-x | tiger service delete svc-x) from working — --confirm is the automation path for a destructive command.
  • tiger auth login with multiple projects and no TTY points at --public-key / --secret-key instead of erroring out of the picker.

The conventions are written up in CLAUDE.md.

@nathanjcochran nathanjcochran self-assigned this Aug 6, 2026
Comment thread internal/util/read.go
})
}

func ReadAll(ctx context.Context, stdin io.Reader) (string, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems to have no callers, I guess it's still fine to add since you ported it over. Just calling that out.

Comment thread internal/util/read.go

val, err := readFn()
if err != nil {
resultCh <- result{err: err}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This breaks for piped input apparently if it doesn't end with a \n, idk if that's a real use case though so probably fine.

Base automatically changed from nathan/improve-output to main August 11, 2026 16:16
BubbleTea defaults to os.Stdin when WithInput is omitted, so the four
interactive pickers read real stdin regardless of what the command was
given. Pass both streams explicitly, and gate the two ungated pickers on
a TTY so they fail with a useful message instead of inside BubbleTea.

oauthLogin now holds the *cobra.Command rather than an io.Writer, since
the project picker needs stdin as well as stderr.
Without the gate the prompt fails on an unhelpful EOF in CI, instead of
pointing at --confirm.
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