fix: GAP-020 - bound the first-run confirm prompt to 3 attempts

amplifier_app_cli/commands/init.py

@@ -5,6 +5,7 @@
 import click
 from rich.console import Console
 from rich.prompt import Confirm
+from rich.prompt import InvalidResponse
 from rich.prompt import Prompt
 from rich.table import Table
 
@@ -220,6 +221,59 @@ def check_first_run() -> bool:
     return False
 
 
+def _bounded_confirm(
+    console_arg: Console,
+    prompt: str,
+    default: bool = True,
+    max_attempts: int = 3,
+) -> bool:
+    """Attempt-bounded replacement for ``rich.prompt.Confirm.ask()``.
+
+    GAP-020: ``Confirm.ask()`` re-prompts on invalid (non-y/n) input with
+    **no bound** -- forever, if the user never types a recognized response.
+    Reproduced live: two non-y/n inputs in a row just re-prompt a third
+    time, with nothing indicating there's any limit at all. That's a real
+    gap at a first-run gate a brand-new user hits before anything else
+    works: anyone who doesn't type exactly "y" or "n" (a stray keypress, a
+    pasted line, an automation sending the wrong thing) is stuck with no
+    signal that it will ever end.
+
+    EOF and Ctrl-C were investigated too and are deliberately left alone:
+    Click's own ``BaseCommand.main()`` already catches ``(EOFError,
+    KeyboardInterrupt)`` globally and converts them to a clean "Aborted!"
+    exit (verified from Click's source and confirmed live on native
+    Windows for both Ctrl-C and Ctrl-Z -- Windows' real console EOF key;
+    Ctrl-D, POSIX's EOF key, has no special meaning to the Windows console
+    subsystem at all and is simply inert there, not a bug). Catching EOF
+    here too would just relitigate something Click already gets right, and
+    would silently change "the user asked to stop" into "skip setup and
+    keep going" -- exactly the kind of quiet fallback the far more
+    predictable existing "Aborted!" exit avoids.
+
+    This bounds only the retry count, falling through to the same "setup
+    skipped" messaging an explicit "n" answer already produces once
+    ``max_attempts`` is exhausted, so a run that can't get a valid answer
+    still terminates the prompt loudly and predictably instead of hanging.
+    """
+    prompt_obj = Confirm(prompt, console=console_arg)
+    for _ in range(max_attempts):
+        value = prompt_obj.get_input(
+            console_arg, prompt_obj.make_prompt(default), False
+        )
+        if value == "":
+            return default
+        try:
+            return prompt_obj.process_response(value)
+        except InvalidResponse as error:
+            prompt_obj.on_validate_error(value, error)
+
+    console_arg.print(
+        f"[yellow]No valid y/n response after {max_attempts} attempts. "
+        "Skipping setup.[/yellow]"
+    )
+    return False
+
+
 def prompt_first_run_init(console_arg: Console) -> bool:
     """Prompt user to run init on first run. Returns True if provider was added.
 
@@ -238,7 +292,7 @@ def prompt_first_run_init(console_arg: Console) -> bool:
     )
     console_arg.print()
 
-    if Confirm.ask("Run setup now?", default=True):
+    if _bounded_confirm(console_arg, "Run setup now?", default=True):
         from .provider import provider_manage_loop
 
         settings = _get_settings()

tests/test_gap020_bounded_confirm.py

@@ -0,0 +1,127 @@
+"""Regression tests for GAP-020: the first-run confirm prompt must terminate.
+
+This fix had **zero** test coverage. It was found by auditing every GAP claimed
+in source comments against the GAPs referenced in tests -- the same audit that
+would have caught GAP-021, whose untested fix turned out to silently corrupt
+user input on every platform.
+
+`rich.prompt.Confirm.ask()` is a bare `while True:` with no bound. On invalid
+(non-y/n) input it re-prompts forever, with nothing indicating a limit exists.
+That sits at a first-run gate a brand-new user hits before anything else works,
+so anyone who doesn't type exactly "y" or "n" -- a stray keypress, a pasted
+line, an automation sending the wrong thing -- is stuck with no signal that it
+will ever end.
+
+The contract these tests pin is exactly the one the fix exists to add: after
+`max_attempts` invalid responses it **stops**, loudly, and falls through to the
+same "setup skipped" state an explicit "n" produces. Every test here is bounded
+by `pytest-timeout`-free construction -- a hang shows up as a failed assertion
+on call count, not as a wedged suite.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import patch
+
+from amplifier_app_cli.commands.init import _bounded_confirm
+from rich.console import Console
+
+
+class _ScriptedInput:
+    """Feeds a fixed script of responses, then refuses to be asked again.
+
+    If the loop is unbounded it will ask past the end of the script; raising
+    there converts an infinite hang into an immediate, legible failure rather
+    than a suite that never finishes.
+    """
+
+    def __init__(self, responses: list[str], hard_limit: int = 50) -> None:
+        self.responses = responses
+        self.calls = 0
+        self.hard_limit = hard_limit
+
+    def __call__(self, *_args: object, **_kwargs: object) -> str:
+        self.calls += 1
+        if self.calls > self.hard_limit:
+            raise AssertionError(
+                f"_bounded_confirm asked for input {self.calls} times -- the "
+                "loop is unbounded. This is the GAP-020 hang."
+            )
+        idx = min(self.calls - 1, len(self.responses) - 1)
+        return self.responses[idx]
+
+
+def test_invalid_input_terminates_after_max_attempts() -> None:
+    """Three invalid answers must end the prompt, not re-ask forever."""
+    console = Console(quiet=True)
+    scripted = _ScriptedInput(["banana", "banana", "banana"])
+
+    with patch("rich.prompt.PromptBase.get_input", scripted):
+        result = _bounded_confirm(console, "Proceed?", default=True, max_attempts=3)
+
+    assert scripted.calls == 3, (
+        f"expected exactly 3 prompts, got {scripted.calls}. Fewer means the "
+        "bound is too tight; more means it is not being honoured."
+    )
+    assert result is False, (
+        "after exhausting attempts the result must be the conservative "
+        "'skip setup' answer, matching an explicit 'n'"
+    )
+
+
+def test_max_attempts_is_actually_honoured() -> None:
+    """The bound must track max_attempts, not be hardcoded."""
+    console = Console(quiet=True)
+    for limit in (1, 2, 5):
+        scripted = _ScriptedInput(["nonsense"])
+        with patch("rich.prompt.PromptBase.get_input", scripted):
+            _bounded_confirm(console, "Proceed?", default=True, max_attempts=limit)
+        assert scripted.calls == limit, (
+            f"max_attempts={limit} produced {scripted.calls} prompts"
+        )
+
+
+def test_valid_answer_short_circuits_immediately() -> None:
+    """A good answer must not consume the retry budget.
+
+    Guards against a "fix" that bounds the loop by always running it to
+    exhaustion.
+    """
+    console = Console(quiet=True)
+
+    for answer, expected in (("y", True), ("n", False)):
+        scripted = _ScriptedInput([answer])
+        with patch("rich.prompt.PromptBase.get_input", scripted):
+            result = _bounded_confirm(
+                console, "Proceed?", default=False, max_attempts=3
+            )
+        assert result is expected, f"answer {answer!r} produced {result!r}"
+        assert scripted.calls == 1, (
+            f"a valid answer took {scripted.calls} prompts; should take 1"
+        )
+
+
+def test_recovery_after_invalid_input() -> None:
+    """An invalid answer followed by a valid one must accept the valid one."""
+    console = Console(quiet=True)
+    scripted = _ScriptedInput(["what", "y"])
+
+    with patch("rich.prompt.PromptBase.get_input", scripted):
+        result = _bounded_confirm(console, "Proceed?", default=False, max_attempts=3)
+
+    assert result is True
+    assert scripted.calls == 2
+
+
+def test_empty_input_returns_the_default() -> None:
+    """Bare Enter means "accept the default", not "invalid"."""
+    console = Console(quiet=True)
+
+    for default in (True, False):
+        scripted = _ScriptedInput([""])
+        with patch("rich.prompt.PromptBase.get_input", scripted):
+            result = _bounded_confirm(
+                console, "Proceed?", default=default, max_attempts=3
+            )
+        assert result is default
+        assert scripted.calls == 1