Skip to content

Commit 4db4222

Browse files
committed
fix: doctor wizards isatty-gate before prompting (clean JSON stdout on non-TTY)
Advisory follow-up to ef6f924. The guard_wizard_eof EOF-catch let the wizard call input() first, which echoes the prompt onto stdout before EOFing — so `teaagent doctor <x> --wizard </dev/null` emitted a classified JSON error prefixed with "MCP host (default 127.0.0.1): ...", breaking `| jq`. The unit test used side_effect=EOFError mocks that never write the prompt, so it missed the pollution; a live check confirmed it. guard_wizard_eof now isatty-pre-checks: on non-TTY it returns the classified error BEFORE the wizard body (no prompt echoed, clean JSON), keeping the EOFError catch as belt-and-suspenders for a TTY that still hits end-of-input. The five doctor wizards are interactive-only (no value flags), so requiring a TTY is the correct contract; the four existing wizard tests declare a TTY (patch sys.stdin.isatty -> True) — the interactive path they exercise — with the same assertions, no test weakened. setup stays clean via its existing cannot_prompt gate (verified). Live: doctor mcp/providers/model --wizard </dev/null -> clean classified JSON. Gate: governance-gap (clean machine-readable error contract; AGENTS.md: tool errors must be actionable and classified) Constraint: isatty pre-check for doctor wizards only; setup/init unchanged; 4 existing tests declare the interactive precondition, no assertion removed Tested: full suite 6745 passed / 6 failed via sharded xdist (same 6 = 4 pre-existing + 2 xdist-only; zero new); 11 targeted doctor tests; live </dev/null cleanliness on 3 wizards; ruff/mypy clean Not-tested: the 4 pre-existing failures (prior-session debt, diagnosed for owner triage) Confidence: high Action: G-P2-19 Roadmap-Status: unchanged
1 parent afd8a63 commit 4db4222

3 files changed

Lines changed: 52 additions & 38 deletions

File tree

teaagent/cli/_handlers/_doctor/_wizard_io.py

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,53 @@
1-
"""Shared EOF guard for ``teaagent doctor ... --wizard`` flows."""
1+
"""Shared interactive-input guard for ``teaagent doctor ... --wizard`` flows."""
22

33
from __future__ import annotations
44

55
import functools
6+
import sys
67
from typing import Any, Callable
78

89
from .sanitize import print_json
910

1011

12+
def _classified_no_tty(wizard: str) -> None:
13+
print_json(
14+
{
15+
'ok': False,
16+
'error': (
17+
f'{wizard} needs interactive input; run it in a terminal, or '
18+
'use the non-wizard commands/flags to configure without prompts'
19+
),
20+
}
21+
)
22+
23+
1124
def guard_wizard_eof(
1225
wizard: str,
1326
) -> Callable[[Callable[[Any], int]], Callable[[Any], int]]:
14-
"""Wrap a doctor wizard so exhausted stdin fails cleanly, not with a crash.
15-
16-
The wizards prompt with ``input()``/``getpass``. On ``</dev/null`` or when
17-
piped answers run out, those raise ``EOFError``, which previously surfaced
18-
as the generic ``Unexpected error`` (the same first-run crash class removed
19-
for ``init``/``setup`` in G23/G38). Return a classified error instead;
20-
provided/piped answers still work while they last.
27+
"""Guard a doctor wizard against non-interactive stdin.
28+
29+
The wizards prompt with ``input()``/``getpass``, which write the prompt to
30+
stdout. The wizard's own result is JSON on stdout, so on a non-TTY stdin we
31+
must not even *start* prompting — otherwise the echoed prompt corrupts the
32+
JSON (and the read would ``EOFError`` into the generic ``Unexpected error``,
33+
the first-run crash class removed for ``init``/``setup`` in G23/G38).
34+
35+
So fail fast with a classified error *before* the wizard body when stdin is
36+
not a TTY (clean JSON, no prompt echoed), and keep an ``EOFError`` catch as a
37+
belt-and-suspenders for an interactive session that still hits end-of-input
38+
(e.g. Ctrl-D).
2139
"""
2240

2341
def decorator(fn: Callable[[Any], int]) -> Callable[[Any], int]:
2442
@functools.wraps(fn)
2543
def wrapper(args: Any) -> int:
44+
if not sys.stdin.isatty():
45+
_classified_no_tty(wizard)
46+
return 1
2647
try:
2748
return fn(args)
2849
except EOFError:
29-
print_json(
30-
{
31-
'ok': False,
32-
'error': (
33-
f'{wizard} needs interactive input; run it in a '
34-
'terminal or pipe answers to its prompts'
35-
),
36-
}
37-
)
50+
_classified_no_tty(wizard)
3851
return 1
3952

4053
return wrapper

tests/test_cli.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ def test_doctor_model_ok_when_key_set() -> None:
322322
def test_doctor_model_wizard_uses_keychain_when_prompt_empty() -> None:
323323
output = io.StringIO()
324324
with (
325+
patch('sys.stdin.isatty', return_value=True),
325326
patch('teaagent.cli._handlers._doctor.model.getpass.getpass', return_value=''),
326327
patch('teaagent.cli._handlers._doctor.model.input', return_value=''),
327328
patch('teaagent.wizard.subprocess.run') as security_run,
@@ -430,6 +431,7 @@ def test_doctor_aigateway_wizard_writes_env() -> None:
430431
env_path.write_text('export OPENAI_API_KEY=sk-existing\n', encoding='utf-8')
431432
output = io.StringIO()
432433
with (
434+
patch('sys.stdin.isatty', return_value=True),
433435
patch(
434436
'teaagent.cli._handlers._doctor.model.input',
435437
side_effect=['acct123', 'gw123', 'y'],
@@ -465,6 +467,7 @@ def test_doctor_aigateway_wizard_writes_compat_base_url() -> None:
465467
with tempfile.TemporaryDirectory() as tmp:
466468
output = io.StringIO()
467469
with (
470+
patch('sys.stdin.isatty', return_value=True),
468471
patch(
469472
'teaagent.cli._handlers._doctor.model.input',
470473
side_effect=['acct123', 'gw123', 'n'],
@@ -502,6 +505,7 @@ def test_doctor_aigateway_wizard_writes_compat_base_url() -> None:
502505
def test_doctor_aigateway_wizard_reads_keychain_token_when_input_empty() -> None:
503506
output = io.StringIO()
504507
with (
508+
patch('sys.stdin.isatty', return_value=True),
505509
patch(
506510
'teaagent.cli._handlers._doctor.model.input',
507511
side_effect=['acct123', 'gw123', 'n'],

tests/test_dogfood_doctor_wizard_tty_g38_twin.py

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
# test-type: behavior
2-
"""G38 twin: `doctor ... --wizard` must not crash on exhausted stdin.
2+
"""G38 twin: `doctor ... --wizard` must not crash or pollute stdout on non-TTY.
33
4-
The five doctor wizards prompt with ``input()``/``getpass``. On piped/headless
5-
stdin those raised ``EOFError`` and surfaced as the generic ``Unexpected
6-
error`` (rc 1, empty/opaque message) — the same first-run crash class removed
7-
for ``init``/``setup`` in G23/G38, and reachable straight from ``init``'s own
4+
The five doctor wizards prompt with ``input()``/``getpass``, which echo the
5+
prompt to stdout — and the wizard's own result is JSON on stdout. On piped or
6+
headless stdin those reads previously ``EOFError``ed into the generic
7+
``Unexpected error`` (rc 1) — the same first-run crash class removed for
8+
``init``/``setup`` in G23/G38, and reachable straight from ``init``'s own
89
``next_steps`` (``teaagent doctor mcp --wizard``). Each wizard must now fail
9-
fast with a classified ``{"ok": false, "error": ...}``. Provided/piped answers
10-
still work (the guard only catches EOF), so the existing wizard tests that mock
11-
``input``/``getpass`` keep passing.
10+
fast with a classified ``{"ok": false, "error": ...}`` *before* prompting, so
11+
the JSON stdout stays clean. Provided input on a real TTY still works, so the
12+
existing wizard tests (which declare a TTY and mock ``input``/``getpass``) keep
13+
passing.
1214
"""
1315

1416
from __future__ import annotations
@@ -36,52 +38,47 @@
3638
ids=['mcp', 'project', 'providers', 'aigateway', 'model'],
3739
)
3840
def test_doctor_wizard_non_tty_is_classified_error(argv: list[str], tmp_path) -> None:
39-
"""Each wizard on exhausted stdin -> rc 1 + classified error, never a crash."""
41+
"""Non-TTY -> rc 1 + clean classified JSON, no prompt echoed, no crash."""
4042
output = io.StringIO()
41-
with (
42-
patch('teaagent.cli._handlers._doctor.model.input', side_effect=EOFError),
43-
patch('teaagent.cli._handlers._doctor.project.input', side_effect=EOFError),
44-
patch('getpass.getpass', side_effect=EOFError),
45-
redirect_stdout(output),
46-
):
43+
with patch('sys.stdin.isatty', return_value=False), redirect_stdout(output):
4744
exit_code = main([*argv, '--root', str(tmp_path)])
4845

4946
assert exit_code == 1
5047
raw = output.getvalue()
5148
assert 'Unexpected error' not in raw
49+
# The classified error must be the whole of stdout — no echoed prompt prefix.
5250
payload = json.loads(raw)
5351
assert payload['ok'] is False
5452
assert 'interactive' in payload['error']
5553

5654

5755
def test_require_wizard_tty_allows_a_tty() -> None:
58-
"""Happy path passes through unchanged, no output.
56+
"""Happy path passes through unchanged on a TTY, with no added output.
5957
6058
Successor to the removed ``require_wizard_tty`` isatty check: the wizard
6159
guard is now the ``guard_wizard_eof`` decorator, which must leave a
62-
successful (non-EOF) call untouched — the same "allowed path is not
63-
blocked" guarantee, now covering provided/piped input, not just a TTY.
60+
successful call on an interactive stdin untouched.
6461
"""
6562
output = io.StringIO()
6663

6764
@guard_wizard_eof('teaagent doctor demo --wizard')
6865
def _ok(_args: object) -> int:
6966
return 0
7067

71-
with redirect_stdout(output):
68+
with patch('sys.stdin.isatty', return_value=True), redirect_stdout(output):
7269
assert _ok(object()) == 0
7370
assert output.getvalue() == ''
7471

7572

7673
def test_guard_wizard_eof_converts_eof_to_classified_error() -> None:
77-
"""An EOFError inside the wizard becomes a classified error and rc 1."""
74+
"""On a TTY that still hits end-of-input, EOFError -> classified error, rc 1."""
7875
output = io.StringIO()
7976

8077
@guard_wizard_eof('teaagent doctor demo --wizard')
8178
def _boom(_args: object) -> int:
8279
raise EOFError
8380

84-
with redirect_stdout(output):
81+
with patch('sys.stdin.isatty', return_value=True), redirect_stdout(output):
8582
rc = _boom(object())
8683

8784
assert rc == 1

0 commit comments

Comments
 (0)