cleveragents/cleveragents-core

Fork 3

test(cli): TDD failing tests for init --yes non-interactive (bug #783) #1049

Merged

hurui200320 merged 2 commits from tdd/m3-init-yes-no-input into master

2026-03-19 06:31:20 +00:00

hurui200320 commented

2026-03-18 08:26:13 +00:00

Member

Summary

Adds TDD bug-capture tests proving that agents init --yes fails to bypass the
migration approval prompt in a TTY environment (bug #783). These tests follow the
mandatory TDD bug-fix workflow defined in CONTRIBUTING.md §Bug Fix Workflow.

Changes

Behave feature (features/tdd_init_yes_no_input.feature): Two scenarios
tagged @tdd_expected_fail @tdd_bug @tdd_bug_783 that invoke agents init --yes
in a fresh environment with the migration prompt simulated as declining (TTY with
no input). The tests verify the command exits successfully and does not display the
"Apply migrations now?" prompt.
Behave steps (features/steps/tdd_init_yes_no_input_steps.py): Step definitions
that create an isolated temp directory, clear auto-apply and database-URL environment
variables, patch sys.stdin on the real sys module (correct mock target per project
convention), and replace MigrationRunner._default_prompt_for_migration with a
function that returns False (simulating a TTY prompt where the user declines).
Temp directory and env var cleanup is registered via context.add_cleanup().
Robot Framework test (robot/tdd_init_yes_no_input.robot): Two integration test
cases tagged tdd_bug, tdd_bug_783, tdd_expected_fail matching the Behave scenarios.
Robot helper (robot/helper_tdd_init_yes_no_input.py): Helper script that exercises
the same code path using the same mock strategy as the Behave steps.

Bug Reproduction Mechanism

The core challenge is that CliRunner replaces sys.stdin during invoke(), making
direct isatty() patching ineffective for controlling the prompt path. The mock strategy
addresses this with a two-pronged approach:

patch.object(sys, "stdin", mock_stdin) — Patches sys.stdin directly on the
real sys module (correct mock target per the project's established pattern in
features/steps/migration_runner_steps.py). Documents the intent of simulating a TTY.
patch.object(MigrationRunner, "_default_prompt_for_migration", ...) — Replaces
the prompt function with one that returns False (simulating a TTY user declining
migration). This is the mechanism that actually exercises the bug path, since
CliRunner's stdin replacement bypasses the isatty() check.
CLEVERAGENTS_DATABASE_URL with non-template filename — Sets the database URL
to a path whose filename does not match the before_scenario template-DB prefixes,
forcing the real Alembic migration path instead of the test fast-path.

With bug #783 present, require_confirmation=True is hardcoded in
UnitOfWork._ensure_database_initialized(), so the prompt fires, returns False,
causing MigrationNotApprovedError and a non-zero exit code. After the fix, --yes
should bypass the prompt entirely.

Root Cause (for the bug fix developer)

init_command() in cleveragents.cli.commands.project receives the --yes flag but
only uses it to control output format. It does not forward yes to the migration runner.
The migration runner's init_or_upgrade() is called via
unit_of_work._ensure_database_initialized() with require_confirmation=True hardcoded.

Scenario 2 Limitation

While the bug is present, Scenario 2's first assertion (exit code 0) fails and Behave
skips subsequent steps. The "contains Initialized" assertion is only evaluated after the
bug is fixed, providing distinct post-fix regression value.

Quality Gate Results

nox -s lint — ✅ passed
nox -s typecheck — ✅ passed (0 errors)
nox -s unit_tests — ✅ passed (387 features, 11121 scenarios, 0 failed)
nox -s integration_tests — ✅ passed (1561 tests, 0 failed)
nox -s e2e_tests — ✅ passed (16 tests, 0 failed)
nox -s coverage_report — ✅ passed (97% coverage)

Closes #842

## Summary Adds TDD bug-capture tests proving that `agents init --yes` fails to bypass the migration approval prompt in a TTY environment (bug #783). These tests follow the mandatory TDD bug-fix workflow defined in CONTRIBUTING.md §Bug Fix Workflow. ### Changes - **Behave feature** (`features/tdd_init_yes_no_input.feature`): Two scenarios tagged `@tdd_expected_fail @tdd_bug @tdd_bug_783` that invoke `agents init --yes` in a fresh environment with the migration prompt simulated as declining (TTY with no input). The tests verify the command exits successfully and does not display the "Apply migrations now?" prompt. - **Behave steps** (`features/steps/tdd_init_yes_no_input_steps.py`): Step definitions that create an isolated temp directory, clear auto-apply and database-URL environment variables, patch `sys.stdin` on the real `sys` module (correct mock target per project convention), and replace `MigrationRunner._default_prompt_for_migration` with a function that returns `False` (simulating a TTY prompt where the user declines). Temp directory and env var cleanup is registered via `context.add_cleanup()`. - **Robot Framework test** (`robot/tdd_init_yes_no_input.robot`): Two integration test cases tagged `tdd_bug`, `tdd_bug_783`, `tdd_expected_fail` matching the Behave scenarios. - **Robot helper** (`robot/helper_tdd_init_yes_no_input.py`): Helper script that exercises the same code path using the same mock strategy as the Behave steps. ### Bug Reproduction Mechanism The core challenge is that `CliRunner` replaces `sys.stdin` during `invoke()`, making direct `isatty()` patching ineffective for controlling the prompt path. The mock strategy addresses this with a two-pronged approach: 1. **`patch.object(sys, "stdin", mock_stdin)`** — Patches `sys.stdin` directly on the real `sys` module (correct mock target per the project's established pattern in `features/steps/migration_runner_steps.py`). Documents the intent of simulating a TTY. 2. **`patch.object(MigrationRunner, "_default_prompt_for_migration", ...)`** — Replaces the prompt function with one that returns `False` (simulating a TTY user declining migration). This is the mechanism that actually exercises the bug path, since CliRunner's stdin replacement bypasses the `isatty()` check. 3. **`CLEVERAGENTS_DATABASE_URL` with non-template filename** — Sets the database URL to a path whose filename does not match the `before_scenario` template-DB prefixes, forcing the real Alembic migration path instead of the test fast-path. With bug #783 present, `require_confirmation=True` is hardcoded in `UnitOfWork._ensure_database_initialized()`, so the prompt fires, returns `False`, causing `MigrationNotApprovedError` and a non-zero exit code. After the fix, `--yes` should bypass the prompt entirely. ### Root Cause (for the bug fix developer) `init_command()` in `cleveragents.cli.commands.project` receives the `--yes` flag but only uses it to control output format. It does not forward `yes` to the migration runner. The migration runner's `init_or_upgrade()` is called via `unit_of_work._ensure_database_initialized()` with `require_confirmation=True` hardcoded. ### Scenario 2 Limitation While the bug is present, Scenario 2's first assertion (exit code 0) fails and Behave skips subsequent steps. The "contains Initialized" assertion is only evaluated after the bug is fixed, providing distinct post-fix regression value. ### Quality Gate Results - `nox -s lint` — ✅ passed - `nox -s typecheck` — ✅ passed (0 errors) - `nox -s unit_tests` — ✅ passed (387 features, 11121 scenarios, 0 failed) - `nox -s integration_tests` — ✅ passed (1561 tests, 0 failed) - `nox -s e2e_tests` — ✅ passed (16 tests, 0 failed) - `nox -s coverage_report` — ✅ passed (97% coverage) Closes #842

hurui200320 added this to the v3.2.0 milestone

2026-03-18 08:26:24 +00:00

hurui200320 added the

Type

Testing

label

2026-03-18 08:26:25 +00:00

hurui200320 added a new dependency

2026-03-18 08:28:50 +00:00

#842 TDD: agents init --yes should not require user input (bug #783)

hurui200320 force-pushed tdd/m3-init-yes-no-input from 5f3158cf39

CI / benchmark-publish (pull_request) Has been skipped

Details

CI / lint (pull_request) Successful in 27s

Details

CI / build (pull_request) Successful in 24s

Details

CI / quality (pull_request) Successful in 42s

Details

CI / typecheck (pull_request) Successful in 50s

Details

CI / security (pull_request) Successful in 1m3s

Details

CI / integration_tests (pull_request) Successful in 5m7s

Details

CI / unit_tests (pull_request) Successful in 5m28s

Details

CI / e2e_tests (pull_request) Successful in 5m37s

Details

CI / docker (pull_request) Successful in 59s

Details

CI / coverage (pull_request) Successful in 7m10s

Details

CI / benchmark-regression (pull_request) Successful in 40m28s

Details

to c35e372a9c

CI / benchmark-publish (pull_request) Has been skipped

Details

CI / lint (pull_request) Successful in 18s

Details

CI / build (pull_request) Successful in 18s

Details

CI / quality (pull_request) Successful in 29s

Details

CI / typecheck (pull_request) Successful in 43s

Details

CI / security (pull_request) Successful in 45s

Details

CI / unit_tests (pull_request) Successful in 2m58s

Details

CI / integration_tests (pull_request) Successful in 3m33s

Details

CI / e2e_tests (pull_request) Successful in 3m53s

Details

CI / docker (pull_request) Successful in 55s

Details

CI / coverage (pull_request) Successful in 7m24s

Details

CI / benchmark-regression (pull_request) Successful in 38m4s

Details

2026-03-18 10:17:19 +00:00

Compare

freemo approved these changes

2026-03-19 04:54:38 +00:00

freemo left a comment

Code Review — PR #1049 `test(cli): TDD failing tests for init --yes non-interactive (bug #783)`

Clean TDD test PR. Well-structured with 2 Behave scenarios + 2 Robot tests. The mock strategy is well-documented (patching MigrationRunner._default_prompt_for_migration since CliRunner replaces sys.stdin). TDD tags @tdd_expected_fail @tdd_bug @tdd_bug_783 are correctly applied. Cleanup properly registered via context.add_cleanup().

Approved. One very minor note: sibling PRs #1050-#1052 include CHANGELOG entries while this one does not. This is a minor inconsistency but not a blocker.

## Code Review — PR #1049 `test(cli): TDD failing tests for init --yes non-interactive (bug #783)` Clean TDD test PR. Well-structured with 2 Behave scenarios + 2 Robot tests. The mock strategy is well-documented (patching `MigrationRunner._default_prompt_for_migration` since CliRunner replaces `sys.stdin`). TDD tags `@tdd_expected_fail @tdd_bug @tdd_bug_783` are correctly applied. Cleanup properly registered via `context.add_cleanup()`. **Approved.** One very minor note: sibling PRs #1050-#1052 include CHANGELOG entries while this one does not. This is a minor inconsistency but not a blocker.

freemo requested reviews from brent.edwards, freemo

2026-03-19 05:16:38 +00:00

hurui200320 added 1 commit

2026-03-19 06:23:28 +00:00

Merge branch 'master' into tdd/m3-init-yes-no-input

CI / benchmark-publish (pull_request) Has been skipped

Details

CI / lint (pull_request) Successful in 17s

Details

CI / build (pull_request) Successful in 17s

Details

CI / quality (pull_request) Successful in 29s