diff --git a/features/steps/tdd_init_yes_no_input_steps.py b/features/steps/tdd_init_yes_no_input_steps.py new file mode 100644 index 000000000..bdcca6e76 --- /dev/null +++ b/features/steps/tdd_init_yes_no_input_steps.py @@ -0,0 +1,226 @@ +"""Step definitions for TDD Bug #783 — init --yes should not require user input. + +These steps exercise the *real* ``agents init --yes`` code path while +simulating a TTY environment by replacing the migration runner's +``_default_prompt_for_migration`` with a function that declines +migration (simulating a user on a real terminal providing no input). +The ``@tdd_expected_fail`` tag on the scenarios inverts the result: +these tests **pass** CI while the bug is present and will **fail** once +the bug is fixed (signalling that the tag should be removed). + +Root cause +~~~~~~~~~~ +``init_command()`` in ``project.py`` receives the ``--yes`` flag but +does not forward it to the migration runner. The migration runner's +``_default_prompt_for_migration()`` calls ``typer.confirm()`` when a +TTY is detected, blocking the command for user input even though +``--yes`` was specified. In a non-TTY environment (like CliRunner), +the prompt is silently skipped — the bug only surfaces in real terminal +sessions. We replace the prompt function to reproduce the TTY code +path. + +Mock strategy +~~~~~~~~~~~~~ +CliRunner replaces ``sys.stdin`` during ``invoke()``, so patching +``sys.stdin.isatty()`` alone is ineffective. Instead we: + +1. Patch ``sys.stdin`` on the real ``sys`` module (documents intent + and is the correct target per the project's established pattern in + ``features/steps/migration_runner_steps.py``). +2. Patch ``MigrationRunner._default_prompt_for_migration`` with a + replacement that returns ``False`` — simulating a TTY prompt where + the user declines migration. This ensures the prompt code path is + exercised deterministically regardless of CliRunner's stdin + replacement. +3. Set ``CLEVERAGENTS_DATABASE_URL`` to a path inside our tmp directory + whose filename does **not** match the ``before_scenario`` template-DB + prefixes (``cleveragents_*``, ``test_*``, ``db.*``). This forces the + migration runner's ``init_or_upgrade`` through the real Alembic path + instead of the test fast-path that copies a pre-migrated template. + +With bug #783 present, ``require_confirmation=True`` is hardcoded in +``UnitOfWork._ensure_database_initialized()``, so the prompt fires and +returns ``False``, causing ``MigrationNotApprovedError``. After the +fix, ``--yes`` should bypass the prompt entirely. +""" + +from __future__ import annotations + +import os +import shutil +import sys +import tempfile +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.application.container import reset_container +from cleveragents.cli.commands.project import app as project_app +from cleveragents.infrastructure.database.migration_runner import MigrationRunner + +runner = CliRunner() + + +def _tty_prompt_declines(_message: str) -> bool: + """Simulate a TTY migration prompt that declines (no user input). + + Replaces ``_default_prompt_for_migration`` to reproduce the TTY + code path without depending on ``sys.stdin.isatty()``, which + CliRunner overrides during ``invoke()``. + """ + return False + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given("a fresh environment for tdd-init-yes-no-input") +def step_fresh_environment(context: Context) -> None: + """Set up a fresh temporary environment with no existing database. + + Creates a temporary directory to serve as the working directory so + that ``agents init`` creates a fresh ``.cleveragents`` directory. + Disables auto-apply environment variables so the real prompt + behaviour is exercised. + """ + context.tdd_init_tmpdir = tempfile.mkdtemp(prefix="tdd_init_yes_") + + # Save env vars that would bypass the prompt or redirect the + # database, and remove them. ``CLEVERAGENTS_DATABASE_URL`` and + # ``CLEVERAGENTS_TEST_DATABASE_URL`` are set by ``before_scenario`` + # to pre-migrated scenario databases — removing them forces the + # container to derive a fresh DB path from ``CLEVERAGENTS_HOME``. + context.tdd_init_saved_env = { + k: os.environ.pop(k, None) + for k in ( + "CLEVERAGENTS_AUTO_APPLY_MIGRATIONS", + "CI", + "BEHAVE_TESTING", + "CLEVERAGENTS_DATABASE_URL", + "CLEVERAGENTS_TEST_DATABASE_URL", + ) + } + + # Register cleanup to restore env vars and remove tmpdir even if + # subsequent steps are never reached (e.g. hook errors). + saved_env = context.tdd_init_saved_env + tmpdir = context.tdd_init_tmpdir + + def _cleanup() -> None: + for key, val in saved_env.items(): + if val is not None: + os.environ[key] = val + else: + os.environ.pop(key, None) + shutil.rmtree(tmpdir, ignore_errors=True) + reset_container() + + context.add_cleanup(_cleanup) + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when("I invoke agents init --yes with no stdin") +def step_invoke_init_yes_no_stdin(context: Context) -> None: + """Invoke ``agents init --yes`` simulating a TTY with no real input. + + Uses ``CliRunner`` with ``input=""`` to provide no input. Patches + ``sys.stdin`` on the real ``sys`` module for intent documentation + and replaces ``MigrationRunner._default_prompt_for_migration`` with + a function that returns ``False`` (simulating a TTY prompt where the + user declines). See module docstring for the full mock strategy. + """ + # Create a mock stdin that reports as a TTY but has no data + mock_stdin = MagicMock(spec=sys.stdin) + mock_stdin.isatty.return_value = True + + # Reset the global container to ensure a fresh UoW is created + # using our env var overrides (no stale CLEVERAGENTS_DATABASE_URL). + reset_container() + + # Use a DB path whose filename (``init_yes_test.sqlite``) does + # NOT start with the template-fast-path prefixes + # (``cleveragents_*``, ``test_*``, ``db.*``) so the real Alembic + # migration path runs instead of copying a pre-migrated template. + # Do NOT pre-create directories — ``init_command`` expects a + # fresh path without an existing ``.cleveragents`` directory. + tdd_db_path = os.path.join( + context.tdd_init_tmpdir, ".cleveragents", "init_yes_test.sqlite" + ) + tdd_db_url = f"sqlite:///{tdd_db_path}" + + with patch.dict( + os.environ, + { + "CLEVERAGENTS_HOME": context.tdd_init_tmpdir, + "CLEVERAGENTS_DATABASE_URL": tdd_db_url, + }, + ): + # Remove auto-apply env vars inside the patched environment + for key in ( + "CLEVERAGENTS_AUTO_APPLY_MIGRATIONS", + "CI", + "BEHAVE_TESTING", + ): + os.environ.pop(key, None) + + # Patch sys.stdin on the real sys module (correct target per + # project convention) and replace the prompt function to + # simulate a TTY decline. See module docstring for why + # both patches are needed. + with ( + patch.object(sys, "stdin", mock_stdin), + patch.object( + MigrationRunner, + "_default_prompt_for_migration", + staticmethod(_tty_prompt_declines), + ), + ): + context.tdd_init_result = runner.invoke( + project_app, + ["init", "--yes", "--path", context.tdd_init_tmpdir], + input="", + ) + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then("the tdd-init-yes-no-input command should exit successfully") +def step_exit_success(context: Context) -> None: + """Assert the init command exited with code 0.""" + result = context.tdd_init_result + assert result is not None, "init --yes was not invoked" + actual = result.exit_code + assert actual == 0, f"Expected exit code 0, got {actual}.\nOutput:\n{result.output}" + + +@then('the tdd-init-yes-no-input output should not contain "{text}"') +def step_output_not_contains(context: Context, text: str) -> None: + """Assert the given text does NOT appear in the command output.""" + result = context.tdd_init_result + assert result is not None, "init --yes was not invoked" + output = result.output + assert text.lower() not in output.lower(), ( + f"Did NOT expect '{text}' in output but found it:\n{output}" + ) + + +@then('the tdd-init-yes-no-input output should contain "{text}"') +def step_output_contains(context: Context, text: str) -> None: + """Assert the given text appears in the command output.""" + result = context.tdd_init_result + assert result is not None, "init --yes was not invoked" + output = result.output + assert text.lower() in output.lower(), ( + f"Expected '{text}' in output but did not find it:\n{output}" + ) diff --git a/features/tdd_init_yes_no_input.feature b/features/tdd_init_yes_no_input.feature new file mode 100644 index 000000000..b67384eff --- /dev/null +++ b/features/tdd_init_yes_no_input.feature @@ -0,0 +1,28 @@ +@tdd_expected_fail @tdd_bug @tdd_bug_783 +Feature: TDD Bug #783 — agents init --yes should not require user input + As a developer + I want to verify that `agents init --yes` completes without any user + input prompts + So that the bug is captured and will be caught by a regression test + + The root cause is that `init_command()` in project.py receives the + `--yes` flag but does not pass it through to the migration runner. + The migration runner's `_default_prompt_for_migration` uses + `typer.confirm()` when a TTY is detected, causing the command to block + for user input even when `--yes` was specified. + + Scenario: Init with --yes completes without stdin + Given a fresh environment for tdd-init-yes-no-input + When I invoke agents init --yes with no stdin + Then the tdd-init-yes-no-input command should exit successfully + And the tdd-init-yes-no-input output should not contain "Apply migrations now" + + # Note: While the bug is present, the first assertion (exit code 0) will + # fail and Behave will skip subsequent steps. This means the "contains + # Initialized" assertion is only evaluated after the bug is fixed, at + # which point this scenario provides distinct post-fix regression value. + Scenario: Init with --yes does not prompt for migration approval + Given a fresh environment for tdd-init-yes-no-input + When I invoke agents init --yes with no stdin + Then the tdd-init-yes-no-input command should exit successfully + And the tdd-init-yes-no-input output should contain "Initialized" diff --git a/robot/helper_tdd_init_yes_no_input.py b/robot/helper_tdd_init_yes_no_input.py new file mode 100644 index 000000000..9eb759f53 --- /dev/null +++ b/robot/helper_tdd_init_yes_no_input.py @@ -0,0 +1,242 @@ +"""Helper script for tdd_init_yes_no_input.robot smoke tests. + +Each subcommand exercises the real ``agents init --yes`` code path +simulating a TTY environment with no real stdin input to reproduce +bug #783. The helper reports the **real** outcome: it exits 0 and +prints the sentinel when the operation succeeds (bug is fixed), and +exits 1 when the bug is still present. The +``tdd_expected_fail_listener`` on the Robot side handles pass/fail +inversion while the bug remains open. + +Mock strategy +~~~~~~~~~~~~~ +CliRunner replaces ``sys.stdin`` during ``invoke()``, so patching +``sys.stdin.isatty()`` alone is ineffective. Instead we: + +1. Patch ``sys.stdin`` on the real ``sys`` module (documents intent + and is the correct mock target per the project convention). +2. Patch ``MigrationRunner._default_prompt_for_migration`` with a + replacement that returns ``False`` — simulating a TTY prompt where + the user declines migration. +3. Set ``CLEVERAGENTS_DATABASE_URL`` to a path whose filename does + **not** match the ``before_scenario`` template-DB prefixes so the + real Alembic migration path runs. + +With bug #783, ``require_confirmation=True`` is hardcoded, the prompt +fires and returns ``False``, causing ``MigrationNotApprovedError``. +After the fix, ``--yes`` should bypass the prompt entirely. +""" + +from __future__ import annotations + +import os +import shutil +import sys +import tempfile +from collections.abc import Callable +from pathlib import Path +from unittest.mock import MagicMock, patch + +# Ensure local source tree is importable +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from typer.testing import CliRunner # noqa: E402 + +from cleveragents.cli.commands.project import app as project_app # noqa: E402 +from cleveragents.infrastructure.database.migration_runner import ( # noqa: E402 + MigrationRunner, +) + +runner = CliRunner() + + +def _tty_prompt_declines(_message: str) -> bool: + """Simulate a TTY migration prompt that declines (no user input). + + Replaces ``_default_prompt_for_migration`` to reproduce the TTY + code path without depending on ``sys.stdin.isatty()``, which + CliRunner overrides during ``invoke()``. + """ + return False + + +# --------------------------------------------------------------------------- +# Subcommands +# --------------------------------------------------------------------------- + + +def init_no_stdin() -> None: + """Invoke ``agents init --yes`` simulating a TTY with no real input. + + Patches ``sys.stdin`` on the real ``sys`` module for intent + documentation and replaces ``_default_prompt_for_migration`` with + a function that returns ``False`` (simulating a TTY prompt that + declines). See module docstring for the full mock strategy. + + Exits 0 with sentinel when the command completes successfully + (bug is fixed). Exits 1 when the command fails (bug still + present). + """ + tmpdir = tempfile.mkdtemp(prefix="tdd_init_yes_robot_") + try: + # Remove env vars that would bypass the prompt + env_save: dict[str, str | None] = {} + for key in ( + "CLEVERAGENTS_AUTO_APPLY_MIGRATIONS", + "CI", + "BEHAVE_TESTING", + "CLEVERAGENTS_HOME", + "CLEVERAGENTS_DATABASE_URL", + "CLEVERAGENTS_TEST_DATABASE_URL", + ): + env_save[key] = os.environ.pop(key, None) + + os.environ["CLEVERAGENTS_HOME"] = tmpdir + + # Use a DB path whose filename does NOT start with the + # template-fast-path prefixes (``cleveragents_*``, ``test_*``, + # ``db.*``) so the real Alembic migration path runs. + tdd_db_path = os.path.join(tmpdir, ".cleveragents", "init_yes_test.sqlite") + os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{tdd_db_path}" + + # Create a mock stdin that reports as a TTY + mock_stdin = MagicMock(spec=sys.stdin) + mock_stdin.isatty.return_value = True + + # Patch sys.stdin on the real sys module and replace the + # prompt function to simulate a TTY decline. + with ( + patch.object(sys, "stdin", mock_stdin), + patch.object( + MigrationRunner, + "_default_prompt_for_migration", + staticmethod(_tty_prompt_declines), + ), + ): + result = runner.invoke( + project_app, + ["init", "--yes", "--path", tmpdir], + input="", + ) + + # Restore env vars + for key, val in env_save.items(): + if val is not None: + os.environ[key] = val + else: + os.environ.pop(key, None) + + if result.exit_code != 0: + print( + f"init --yes failed with exit code {result.exit_code}", + file=sys.stderr, + ) + print(f"Output: {result.output}", file=sys.stderr) + sys.exit(1) + + # Bug fixed — command succeeded without input. + print("tdd-init-yes-no-input-ok") + finally: + # Clean up tmpdir + shutil.rmtree(tmpdir, ignore_errors=True) + + +def init_no_prompt() -> None: + """Invoke ``agents init --yes`` and verify no migration prompt appears. + + Uses the same mock strategy as ``init_no_stdin``. See module + docstring for details. + + Exits 0 with sentinel when the output does not contain the + migration prompt text (bug is fixed). Exits 1 when the prompt + text appears or the command fails (bug still present). + """ + tmpdir = tempfile.mkdtemp(prefix="tdd_init_yes_robot_") + try: + # Remove env vars that would bypass the prompt + env_save: dict[str, str | None] = {} + for key in ( + "CLEVERAGENTS_AUTO_APPLY_MIGRATIONS", + "CI", + "BEHAVE_TESTING", + "CLEVERAGENTS_HOME", + "CLEVERAGENTS_DATABASE_URL", + "CLEVERAGENTS_TEST_DATABASE_URL", + ): + env_save[key] = os.environ.pop(key, None) + + os.environ["CLEVERAGENTS_HOME"] = tmpdir + + # Use a DB path whose filename does NOT start with the + # template-fast-path prefixes. + tdd_db_path = os.path.join(tmpdir, ".cleveragents", "init_yes_test.sqlite") + os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{tdd_db_path}" + + # Create a mock stdin that reports as a TTY + mock_stdin = MagicMock(spec=sys.stdin) + mock_stdin.isatty.return_value = True + + # Patch sys.stdin on the real sys module and replace the + # prompt function to simulate a TTY decline. + with ( + patch.object(sys, "stdin", mock_stdin), + patch.object( + MigrationRunner, + "_default_prompt_for_migration", + staticmethod(_tty_prompt_declines), + ), + ): + result = runner.invoke( + project_app, + ["init", "--yes", "--path", tmpdir], + input="", + ) + + # Restore env vars + for key, val in env_save.items(): + if val is not None: + os.environ[key] = val + else: + os.environ.pop(key, None) + + if result.exit_code != 0: + print( + f"init --yes failed with exit code {result.exit_code}", + file=sys.stderr, + ) + print(f"Output: {result.output}", file=sys.stderr) + sys.exit(1) + + # Check that no migration prompt text appears + output = result.output.lower() + if "apply migrations now" in output: + print( + "Bug present: migration prompt appeared in output", + file=sys.stderr, + ) + sys.exit(1) + + # Bug fixed — no prompt appeared. + print("tdd-init-yes-no-prompt-ok") + finally: + # Clean up tmpdir + shutil.rmtree(tmpdir, ignore_errors=True) + + +# --------------------------------------------------------------------------- +# Dispatcher +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Callable[[], None]] = { + "init-no-stdin": init_no_stdin, + "init-no-prompt": init_no_prompt, +} + +if __name__ == "__main__": + if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS: + print(f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>", file=sys.stderr) + sys.exit(1) + cmd = _COMMANDS[sys.argv[1]] + cmd() diff --git a/robot/tdd_init_yes_no_input.robot b/robot/tdd_init_yes_no_input.robot new file mode 100644 index 000000000..e2761ba8f --- /dev/null +++ b/robot/tdd_init_yes_no_input.robot @@ -0,0 +1,33 @@ +*** Settings *** +Documentation TDD Bug #783 — agents init --yes should not require user input +... Integration smoke tests verifying that the ``init --yes`` command +... completes without blocking for user input. Bug #783 reports that +... the migration runner still prompts for approval even with ``--yes``. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment auto_apply_migrations=${FALSE} +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_tdd_init_yes_no_input.py + +*** Test Cases *** +TDD Init Yes No Input Completes Without Stdin + [Documentation] Verify that ``init --yes`` exits successfully without any + ... stdin input. The helper runs the real CLI init command in + ... a fresh directory with stdin closed. + [Tags] tdd_bug tdd_bug_783 tdd_expected_fail + ${result}= Run Process ${PYTHON} ${HELPER} init-no-stdin cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-init-yes-no-input-ok + +TDD Init Yes No Migration Prompt + [Documentation] Verify that ``init --yes`` output does not contain the + ... migration approval prompt text. + [Tags] tdd_bug tdd_bug_783 tdd_expected_fail + ${result}= Run Process ${PYTHON} ${HELPER} init-no-prompt cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-init-yes-no-prompt-ok