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

Closed
opened 2026-03-13 21:15:43 +00:00 by freemo · 4 comments
Owner

Background and Context

Bug #783 (agents init --yes should not require user input) identifies a defect where running agents init --yes still prompts the user to approve database migration application. The --yes flag should auto-approve all prompts, making the command fully non-interactive. Per the mandatory TDD bug-fix workflow (CONTRIBUTING.md §Bug Fix Workflow), a failing test must be written before the fix is implemented.

This issue tracks the TDD test-writing phase for bug #783.

Current Behavior

No test exists that verifies agents init --yes completes without any user input prompts.

Expected Behavior

A Behave BDD scenario and/or Robot Framework test tagged with @tdd_expected_fail, @tdd_bug, and @tdd_bug_783 should exist that:

  1. Runs agents init --yes in a fresh environment (no existing database)
  2. Provides no stdin input
  3. Asserts the command completes successfully without blocking for input
  4. Asserts no "Apply migrations now?" prompt appears in output
  5. Currently fails (proving the bug exists), but passes CI via the @tdd_expected_fail inversion

Acceptance Criteria

  • At least one Behave scenario in features/tdd_init_yes_no_input.feature
  • Scenario tagged with @tdd_expected_fail @tdd_bug @tdd_bug_783
  • At least one Robot Framework test in robot/tdd_init_yes_no_input.robot
  • Test tagged with tdd_expected_fail, tdd_bug, tdd_bug_783
  • Full test suite passes (nox -s unit_tests integration_tests)
  • ruff check and ruff format pass

Metadata

  • Commit message: test(cli): TDD failing tests for init --yes non-interactive (bug #783)
  • Branch name: tdd/m3-init-yes-no-input
  • Type: Testing
  • Priority: Critical
  • MoSCoW: Must have
  • Points: 2
  • Milestone: v3.2.0

Subtasks

  • Write Behave scenario exercising agents init --yes with no stdin
  • Write Robot Framework test exercising same scenario with closed stdin
  • Apply @tdd_expected_fail tags
  • Verify tests fail (bug present) but CI passes (inversion working)

Definition of Done

  • TDD tests exist and are tagged correctly
  • Tests demonstrate the bug (fail without @tdd_expected_fail)
  • Full CI suite passes with the inversion
  • PR merged to master
## Background and Context Bug #783 (`agents init --yes` should not require user input) identifies a defect where running `agents init --yes` still prompts the user to approve database migration application. The `--yes` flag should auto-approve all prompts, making the command fully non-interactive. Per the mandatory TDD bug-fix workflow (CONTRIBUTING.md §Bug Fix Workflow), a failing test must be written **before** the fix is implemented. This issue tracks the TDD test-writing phase for bug #783. ## Current Behavior No test exists that verifies `agents init --yes` completes without any user input prompts. ## Expected Behavior A Behave BDD scenario and/or Robot Framework test tagged with `@tdd_expected_fail`, `@tdd_bug`, and `@tdd_bug_783` should exist that: 1. Runs `agents init --yes` in a fresh environment (no existing database) 2. Provides no stdin input 3. Asserts the command completes successfully without blocking for input 4. Asserts no "Apply migrations now?" prompt appears in output 5. Currently **fails** (proving the bug exists), but passes CI via the `@tdd_expected_fail` inversion ## Acceptance Criteria - [x] At least one Behave scenario in `features/tdd_init_yes_no_input.feature` - [x] Scenario tagged with `@tdd_expected_fail @tdd_bug @tdd_bug_783` - [x] At least one Robot Framework test in `robot/tdd_init_yes_no_input.robot` - [x] Test tagged with `tdd_expected_fail`, `tdd_bug`, `tdd_bug_783` - [x] Full test suite passes (`nox -s unit_tests integration_tests`) - [x] `ruff check` and `ruff format` pass ## Metadata - **Commit message**: `test(cli): TDD failing tests for init --yes non-interactive (bug #783)` - **Branch name**: `tdd/m3-init-yes-no-input` - **Type**: Testing - **Priority**: Critical - **MoSCoW**: Must have - **Points**: 2 - **Milestone**: v3.2.0 ## Subtasks - [x] Write Behave scenario exercising `agents init --yes` with no stdin - [x] Write Robot Framework test exercising same scenario with closed stdin - [x] Apply `@tdd_expected_fail` tags - [x] Verify tests fail (bug present) but CI passes (inversion working) ## Definition of Done - TDD tests exist and are tagged correctly - Tests demonstrate the bug (fail without `@tdd_expected_fail`) - Full CI suite passes with the inversion - PR merged to `master`
freemo added this to the v3.2.0 milestone 2026-03-13 21:15:59 +00:00
Author
Owner

Dependency (TDD workflow): This TDD test issue blocks bug fix #783. This issue must be completed and merged first so the @tdd_expected_fail test exists before the fix is implemented.

Blocks: #783

**Dependency (TDD workflow):** This TDD test issue blocks bug fix #783. This issue must be completed and merged first so the `@tdd_expected_fail` test exists before the fix is implemented. **Blocks:** #783
Author
Owner

PM Status — Day 37

Status: Not started. This TDD issue blocks bug #783 and has had no activity since the dependency note on Day 34.

@brent.edwards — Per the Week 6 schedule, TDD issues must be prioritized. Please provide an ETA or flag blockers by Day 38 EOD.


PM status — Day 37

## PM Status — Day 37 **Status**: Not started. This TDD issue blocks bug #783 and has had no activity since the dependency note on Day 34. @brent.edwards — Per the Week 6 schedule, TDD issues must be prioritized. Please provide an ETA or flag blockers by Day 38 EOD. --- *PM status — Day 37*
Member

Implementation Notes — TDD Tests for Bug #783

Design Decisions

Bug reproduction strategy: The core challenge in writing TDD tests for this bug is that CliRunner from Typer provides a non-TTY stdin, which silently bypasses the migration prompt code path in _default_prompt_for_migration(). In a real terminal, sys.stdin.isatty() returns True and the migration runner calls typer.confirm(), which blocks for user input — this is where the bug manifests.

To reproduce the bug in tests, both the Behave scenarios and Robot helper scripts patch sys.stdin.isatty() to return True inside the cleveragents.infrastructure.database.migration_runner module. This forces the interactive prompt code path. With empty stdin (input=""), typer.confirm(default=False) returns False, declining migration — causing MigrationNotApprovedError and a non-zero exit code.

Root cause analysis: init_command() in cleveragents.cli.commands.project receives the --yes flag but only uses it to control the output format (Rich Panel vs. plain output). It does not pass yes through to the migration runner or the DI container. The migration runner's init_or_upgrade() is called via unit_of_work._ensure_database_initialized() with require_confirmation=True hardcoded.

Test Structure

  1. Behave feature (features/tdd_init_yes_no_input.feature):

    • Tagged @tdd_bug @tdd_bug_783 @tdd_expected_fail
    • Two scenarios: one verifying exit code 0, one verifying no prompt text in output
    • Steps in features/steps/tdd_init_yes_no_input_steps.py
  2. Robot Framework test (robot/tdd_init_yes_no_input.robot):

    • Tagged tdd_bug, tdd_bug_783, tdd_expected_fail
    • Two test cases matching the Behave scenarios
    • Helper script in robot/helper_tdd_init_yes_no_input.py

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 coverage_report passed (97% coverage)
## Implementation Notes — TDD Tests for Bug #783 ### Design Decisions **Bug reproduction strategy:** The core challenge in writing TDD tests for this bug is that `CliRunner` from Typer provides a non-TTY stdin, which silently bypasses the migration prompt code path in `_default_prompt_for_migration()`. In a real terminal, `sys.stdin.isatty()` returns `True` and the migration runner calls `typer.confirm()`, which blocks for user input — this is where the bug manifests. To reproduce the bug in tests, both the Behave scenarios and Robot helper scripts patch `sys.stdin.isatty()` to return `True` inside the `cleveragents.infrastructure.database.migration_runner` module. This forces the interactive prompt code path. With empty stdin (`input=""`), `typer.confirm(default=False)` returns `False`, declining migration — causing `MigrationNotApprovedError` and a non-zero exit code. **Root cause analysis:** `init_command()` in `cleveragents.cli.commands.project` receives the `--yes` flag but only uses it to control the output format (Rich Panel vs. plain output). It does not pass `yes` through to the migration runner or the DI container. The migration runner's `init_or_upgrade()` is called via `unit_of_work._ensure_database_initialized()` with `require_confirmation=True` hardcoded. ### Test Structure 1. **Behave feature** (`features/tdd_init_yes_no_input.feature`): - Tagged `@tdd_bug @tdd_bug_783 @tdd_expected_fail` - Two scenarios: one verifying exit code 0, one verifying no prompt text in output - Steps in `features/steps/tdd_init_yes_no_input_steps.py` 2. **Robot Framework test** (`robot/tdd_init_yes_no_input.robot`): - Tagged `tdd_bug`, `tdd_bug_783`, `tdd_expected_fail` - Two test cases matching the Behave scenarios - Helper script in `robot/helper_tdd_init_yes_no_input.py` ### 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 coverage_report` — ✅ passed (97% coverage)
hurui200320 added reference tdd/m3-init-yes-no-input 2026-03-18 08:29:05 +00:00
Member

Self-QA Implementation Notes (Cycles 1–2)

Cycle 1

Review findings (10 issues):

# Severity Issue
1 Critical Mock target ineffective — patch("...migration_runner.sys") does not reach the local import sys inside _default_prompt_for_migration(), so tests never exercise the TTY code path where bug #783 manifests
2 Major Behave steps never clean up temporary directory — tempfile.mkdtemp() without context.add_cleanup() leaks dirs with SQLite databases
3 Major import shutil inside finally blocks in Robot helper violates CONTRIBUTING.md §Import Guidelines
4 Minor mock_sys.modules = sys.modules is dead/ineffective code
5 Minor context: Any instead of context: Context — inconsistent with project pattern
6 Minor Missing docstrings on Then step functions
7 Minor Environment variable CI not restored on edge-case failure (Given step pops, restoration only in When step's finally)
8 Minor Scenario 2's differentiating assertion never evaluated while bug exists
9 Nit Feature-level tag ordering inconsistent with existing TDD features
10 Nit CLEVERAGENTS_HOME not saved/restored in Robot helper

Fixes applied (all 10 addressed):

  1. Critical mock fix: Reworked mock strategy to use two-pronged approach: patch.object(sys, "stdin", mock_stdin) (correct target per project convention) + patch.object(MigrationRunner, "_default_prompt_for_migration", staticmethod(_tty_prompt_declines)) to simulate TTY prompt decline. Also set custom CLEVERAGENTS_DATABASE_URL with non-template filename to bypass before_scenario template-DB fast-path.
  2. Temp dir cleanup: Added context.add_cleanup(_cleanup) in Given step that removes tmpdir via shutil.rmtree() and restores env vars.
  3. Import placement: Moved import shutil to top-level imports in Robot helper; removed both inline imports from finally blocks.
  4. Dead code removed: Eliminated all mock_sys.modules = sys.modules lines.
  5. Type annotation: Changed context: Any to context: Context with proper import from behave.runner.
  6. Docstrings added: All three @then step functions now have one-line docstrings.
  7. Env var safety: Moved all env var save/restore into context.add_cleanup() in Given step — fires regardless of step execution.
  8. Scenario 2 documented: Added comment block in feature file explaining the TDD-phase limitation.
  9. Tag ordering fixed: Reordered to @tdd_expected_fail @tdd_bug @tdd_bug_783 in Behave feature.
  10. Robot env save expanded: Added CLEVERAGENTS_HOME, CLEVERAGENTS_DATABASE_URL, and CLEVERAGENTS_TEST_DATABASE_URL to env_save dict.

Key discoveries during fix:

  • Click's CliRunner replaces sys.stdin during invoke(), making direct sys.stdin patching alone insufficient — the _default_prompt_for_migration patch is needed to actually exercise the bug path.
  • The Behave before_scenario hook installs a template-DB fast-path that copies pre-migrated databases for filenames matching cleveragents_*, test_*, or db.* — bypassing migration and the prompt entirely. Original tests were passing for the wrong reason.

Cycle 2

Review findings: All 10 previously identified issues verified as fixed. No critical or major issues remaining. 3 minor items noted (Robot env var restoration not in finally — mitigated by subprocess isolation; Robot tests lack positive output assertion matching Behave Scenario 2; code duplication between Robot helper functions) and 2 nits (Robot tag ordering; redundant env var pops in Behave When step). None affect correctness.

Verdict: Approved

Remaining Issues (non-blocking)

Severity Issue Reason
Minor Robot helper env var restore not exception-safe Mitigated by subprocess isolation — env vars discarded on process exit
Minor Robot tests lack positive "Initialized" output assertion Asymmetry with Behave Scenario 2; functional coverage still adequate
Minor Code duplication between Robot helper functions DRY concern for future maintainability, not a correctness issue
Nit Robot .robot file tag ordering No functional impact (Robot tags are unordered)
Nit Redundant env var pops in Behave When step Defensive/no-op; no harm

All quality gates passed: lint | typecheck | unit tests (387 features, 11121 scenarios) | integration tests (1561 tests) | e2e tests (16 tests) | coverage (97%)

## Self-QA Implementation Notes (Cycles 1–2) ### Cycle 1 **Review findings (10 issues):** | # | Severity | Issue | |---|----------|-------| | 1 | Critical | Mock target ineffective — `patch("...migration_runner.sys")` does not reach the local `import sys` inside `_default_prompt_for_migration()`, so tests never exercise the TTY code path where bug #783 manifests | | 2 | Major | Behave steps never clean up temporary directory — `tempfile.mkdtemp()` without `context.add_cleanup()` leaks dirs with SQLite databases | | 3 | Major | `import shutil` inside `finally` blocks in Robot helper violates CONTRIBUTING.md §Import Guidelines | | 4 | Minor | `mock_sys.modules = sys.modules` is dead/ineffective code | | 5 | Minor | `context: Any` instead of `context: Context` — inconsistent with project pattern | | 6 | Minor | Missing docstrings on `Then` step functions | | 7 | Minor | Environment variable `CI` not restored on edge-case failure (Given step pops, restoration only in When step's finally) | | 8 | Minor | Scenario 2's differentiating assertion never evaluated while bug exists | | 9 | Nit | Feature-level tag ordering inconsistent with existing TDD features | | 10 | Nit | `CLEVERAGENTS_HOME` not saved/restored in Robot helper | **Fixes applied (all 10 addressed):** 1. **Critical mock fix:** Reworked mock strategy to use two-pronged approach: `patch.object(sys, "stdin", mock_stdin)` (correct target per project convention) + `patch.object(MigrationRunner, "_default_prompt_for_migration", staticmethod(_tty_prompt_declines))` to simulate TTY prompt decline. Also set custom `CLEVERAGENTS_DATABASE_URL` with non-template filename to bypass `before_scenario` template-DB fast-path. 2. **Temp dir cleanup:** Added `context.add_cleanup(_cleanup)` in Given step that removes tmpdir via `shutil.rmtree()` and restores env vars. 3. **Import placement:** Moved `import shutil` to top-level imports in Robot helper; removed both inline imports from `finally` blocks. 4. **Dead code removed:** Eliminated all `mock_sys.modules = sys.modules` lines. 5. **Type annotation:** Changed `context: Any` to `context: Context` with proper import from `behave.runner`. 6. **Docstrings added:** All three `@then` step functions now have one-line docstrings. 7. **Env var safety:** Moved all env var save/restore into `context.add_cleanup()` in Given step — fires regardless of step execution. 8. **Scenario 2 documented:** Added comment block in feature file explaining the TDD-phase limitation. 9. **Tag ordering fixed:** Reordered to `@tdd_expected_fail @tdd_bug @tdd_bug_783` in Behave feature. 10. **Robot env save expanded:** Added `CLEVERAGENTS_HOME`, `CLEVERAGENTS_DATABASE_URL`, and `CLEVERAGENTS_TEST_DATABASE_URL` to `env_save` dict. **Key discoveries during fix:** - Click's `CliRunner` replaces `sys.stdin` during `invoke()`, making direct `sys.stdin` patching alone insufficient — the `_default_prompt_for_migration` patch is needed to actually exercise the bug path. - The Behave `before_scenario` hook installs a template-DB fast-path that copies pre-migrated databases for filenames matching `cleveragents_*`, `test_*`, or `db.*` — bypassing migration and the prompt entirely. Original tests were passing for the wrong reason. ### Cycle 2 **Review findings:** All 10 previously identified issues verified as fixed. No critical or major issues remaining. 3 minor items noted (Robot env var restoration not in `finally` — mitigated by subprocess isolation; Robot tests lack positive output assertion matching Behave Scenario 2; code duplication between Robot helper functions) and 2 nits (Robot tag ordering; redundant env var pops in Behave When step). None affect correctness. **Verdict: ✅ Approved** ### Remaining Issues (non-blocking) | Severity | Issue | Reason | |----------|-------|--------| | Minor | Robot helper env var restore not exception-safe | Mitigated by subprocess isolation — env vars discarded on process exit | | Minor | Robot tests lack positive "Initialized" output assertion | Asymmetry with Behave Scenario 2; functional coverage still adequate | | Minor | Code duplication between Robot helper functions | DRY concern for future maintainability, not a correctness issue | | Nit | Robot `.robot` file tag ordering | No functional impact (Robot tags are unordered) | | Nit | Redundant env var pops in Behave When step | Defensive/no-op; no harm | All quality gates passed: lint ✅ | typecheck ✅ | unit tests ✅ (387 features, 11121 scenarios) | integration tests ✅ (1561 tests) | e2e tests ✅ (16 tests) | coverage ✅ (97%)
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core#842
No description provided.