Skip to content

Commit d58c173

Browse files
alexkromanclaude
andauthored
Centralize stdio TTY checks into dedicated functions (#185)
## Summary Introduces dedicated functions in the `stdio` module as single chokepoints for checking whether stdin, stdout, and stderr are interactive terminals. This centralizes TTY detection logic and makes it easier to test and mock terminal behavior across the codebase. ## Changes - **New stdio functions**: Added `stdin_is_tty()`, `stdout_is_tty()`, and `stderr_is_tty()` as the canonical places to call `isatty()` on each stream - **Updated `interactive_stdio()`**: Now composes `stdin_is_tty()` and `stdout_is_tty()` instead of calling `sys.stdin.isatty()` and `sys.stdout.isatty()` directly - **Updated callers**: Replaced direct `sys.stdin.isatty()` and `sys.stderr.isatty()` calls in: - `aai_cli/app/context.py` (`_interactive_session()`) - `aai_cli/ui/output.py` (`_stdout_is_tty()`) - `aai_cli/commands/login.py` (`_read_stdin_key()`) - **Improved login key reading**: Refactored `_read_stdin_key()` to use `stdio.piped_stdin_text()` instead of raw `sys.stdin.read()`, providing clearer semantics and better error handling - **Comprehensive test coverage**: Added tests for all three new TTY functions and `interactive_stdio()` to verify correct behavior with both TTY and piped inputs ## Implementation Details The new functions serve as mutation-resistant seams that: - Provide a single point of control for TTY detection across the codebase - Make it easier to test code paths that depend on terminal interactivity - Establish clear contracts for what "interactive" means in different contexts (stdin+stdout for prompting, stdin+stderr for browser login) https://claude.ai/code/session_01UcQmBRzu1UL12DbFTGdiRf Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6684d19 commit d58c173

6 files changed

Lines changed: 80 additions & 13 deletions

File tree

aai_cli/app/context.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
from __future__ import annotations
22

3-
import sys
43
from collections.abc import Callable
54
from dataclasses import dataclass
65
from typing import NoReturn, Protocol
76

87
import keyring.errors
98
import typer
109

11-
from aai_cli.core import config, debuglog, env, environments, telemetry
10+
from aai_cli.core import config, debuglog, env, environments, stdio, telemetry
1211
from aai_cli.core.environments import Environment
1312
from aai_cli.core.errors import APIError, CLIError, NotAuthenticated
1413
from aai_cli.ui import output, update_check
@@ -143,7 +142,7 @@ def _fail(err: CLIError, *, json_mode: bool) -> NoReturn:
143142
def _interactive_session() -> bool:
144143
"""True only when a human can complete a browser login: stdin and stderr are both
145144
real TTYs and no agent/CI context is detected (`output.is_agentic`)."""
146-
return sys.stdin.isatty() and sys.stderr.isatty() and not output.is_agentic()
145+
return stdio.stdin_is_tty() and stdio.stderr_is_tty() and not output.is_agentic()
147146

148147

149148
def _should_auto_login(err: NotAuthenticated) -> bool:

aai_cli/commands/login.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
from __future__ import annotations
22

3-
import sys
4-
53
import typer
64
from rich.markup import escape
75
from rich.table import Table
86

97
from aai_cli import command_registry, help_panels, options
108
from aai_cli.app.context import AppState, persist_browser_login, run_command
11-
from aai_cli.core import client, config, environments
9+
from aai_cli.core import client, config, environments, stdio
1210
from aai_cli.core.errors import STDIN_KEY_RECIPE, APIError, CLIError, UsageError, mutually_exclusive
1311
from aai_cli.ui import output
1412
from aai_cli.ui.help_text import examples_epilog
@@ -28,21 +26,23 @@ def _read_stdin_key() -> str:
2826
Stdin-only on purpose (the Codex-CLI pattern): a key passed as an argv value
2927
lands in shell history and ``ps`` output; a piped key does not.
3028
"""
31-
if sys.stdin.isatty():
29+
if not stdio.stdin_is_piped():
3230
raise UsageError(
3331
"--with-api-key reads the key from stdin, but stdin is a terminal.",
3432
suggestion=f"Pipe the key in: {STDIN_KEY_RECIPE}",
3533
)
36-
key = sys.stdin.read().strip()
37-
if not key:
34+
# stdin is piped, so a None here means an empty/blank pipe (not a terminal) —
35+
# distinct from the terminal case above, so the recipe hint stays accurate.
36+
key = stdio.piped_stdin_text()
37+
if key is None:
3838
raise UsageError(
3939
"--with-api-key found no key on stdin.",
4040
suggestion=(
4141
f"Pipe a non-empty key: {STDIN_KEY_RECIPE} "
4242
"(check that the variable you piped is set)."
4343
),
4444
)
45-
return key
45+
return key.strip()
4646

4747

4848
@app.command(

aai_cli/core/stdio.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,31 @@ def silence_stdout() -> None:
2424
os.close(devnull_fd)
2525

2626

27+
def stdin_is_tty() -> bool:
28+
"""True when stdin is an interactive terminal. The single raw-``isatty`` chokepoint
29+
for stdin, so higher layers compose this rather than re-reaching for ``sys.stdin``."""
30+
return sys.stdin.isatty()
31+
32+
33+
def stdout_is_tty() -> bool:
34+
"""True when stdout is an interactive terminal. The single raw-``isatty`` chokepoint
35+
for stdout (e.g. `output.is_agentic`/`print_code` compose it)."""
36+
return sys.stdout.isatty()
37+
38+
39+
def stderr_is_tty() -> bool:
40+
"""True when stderr is an interactive terminal. The single raw-``isatty`` chokepoint
41+
for stderr (the browser-login interactivity probe composes it)."""
42+
return sys.stderr.isatty()
43+
44+
2745
def interactive_stdio() -> bool:
2846
"""True only when stdin and stdout are both real TTYs — i.e. a human can answer
2947
a prompt and see it. The shared "may we prompt here?" predicate for the bare-`assembly`
3048
setup offer, the onboarding prompter, and the `assembly init` template picker, so the
3149
three can't drift on what counts as interactive.
3250
"""
33-
return sys.stdin.isatty() and sys.stdout.isatty()
51+
return stdin_is_tty() and stdout_is_tty()
3452

3553

3654
def stdin_is_piped() -> bool:

aai_cli/ui/output.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from rich.text import Text
1313

1414
from aai_cli import __version__
15-
from aai_cli.core import choices, env, jsonshape
15+
from aai_cli.core import choices, env, jsonshape, stdio
1616
from aai_cli.ui import theme
1717

1818
if TYPE_CHECKING:
@@ -27,7 +27,7 @@
2727

2828

2929
def _stdout_is_tty() -> bool:
30-
return sys.stdout.isatty()
30+
return stdio.stdout_is_tty()
3131

3232

3333
def is_agentic() -> bool:

tests/test_output.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,23 @@
55
from aai_cli.ui import output
66

77

8+
class _StdoutProbe:
9+
def __init__(self, tty):
10+
self._tty = tty
11+
12+
def isatty(self):
13+
return self._tty
14+
15+
16+
def test_stdout_is_tty_seam_reflects_real_stdout(monkeypatch):
17+
# The seam every other test patches delegates to the stdio chokepoint; exercise
18+
# the real body so the delegation is covered and a negated isatty mutant dies.
19+
monkeypatch.setattr("sys.stdout", _StdoutProbe(True))
20+
assert output._stdout_is_tty() is True
21+
monkeypatch.setattr("sys.stdout", _StdoutProbe(False))
22+
assert output._stdout_is_tty() is False
23+
24+
825
def test_resolve_json_true_only_when_explicit():
926
# JSON is opt-in: the flag is the single source of truth.
1027
assert output.resolve_json(explicit=True) is True

tests/test_stdio.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,39 @@ def test_stdin_is_piped(monkeypatch):
3030
assert stdio.stdin_is_piped() is False
3131

3232

33+
def test_stdin_is_tty(monkeypatch):
34+
monkeypatch.setattr("sys.stdin", _Tty(""))
35+
assert stdio.stdin_is_tty() is True
36+
monkeypatch.setattr("sys.stdin", _Pipe(""))
37+
assert stdio.stdin_is_tty() is False
38+
39+
40+
def test_stdout_is_tty(monkeypatch):
41+
monkeypatch.setattr("sys.stdout", _Tty(""))
42+
assert stdio.stdout_is_tty() is True
43+
monkeypatch.setattr("sys.stdout", _Pipe(""))
44+
assert stdio.stdout_is_tty() is False
45+
46+
47+
def test_stderr_is_tty(monkeypatch):
48+
monkeypatch.setattr("sys.stderr", _Tty(""))
49+
assert stdio.stderr_is_tty() is True
50+
monkeypatch.setattr("sys.stderr", _Pipe(""))
51+
assert stdio.stderr_is_tty() is False
52+
53+
54+
def test_interactive_stdio_requires_both_stdin_and_stdout_tty(monkeypatch):
55+
# Both must be terminals; either one piped flips it false (kills the and->or mutant).
56+
monkeypatch.setattr("sys.stdin", _Tty(""))
57+
monkeypatch.setattr("sys.stdout", _Tty(""))
58+
assert stdio.interactive_stdio() is True
59+
monkeypatch.setattr("sys.stdout", _Pipe(""))
60+
assert stdio.interactive_stdio() is False
61+
monkeypatch.setattr("sys.stdin", _Pipe(""))
62+
monkeypatch.setattr("sys.stdout", _Tty(""))
63+
assert stdio.interactive_stdio() is False
64+
65+
3366
def test_piped_stdin_text_returns_none_on_tty(monkeypatch):
3467
monkeypatch.setattr("sys.stdin", _Tty("ignored\n"))
3568
assert stdio.piped_stdin_text() is None

0 commit comments

Comments
 (0)