test(cli): add failing tests for agents init --yes missing option (#536) #566

Merged
brent.edwards merged 2 commits from feature/m3-test-init-yes-flag into master 2026-03-07 02:19:03 +00:00
7 changed files with 449 additions and 0 deletions
+5
View File
@@ -2,6 +2,11 @@
## Unreleased
- Added TDD-style failing Behave BDD tests for the missing `agents init --yes` flag.
Five scenarios: four TDD-failing tests (exit code, prompt suppression, `-y` alias,
output summary) and one regression guard for interactive mode. Includes Robot
Framework smoke tests and ASV benchmarks. Tests are intentionally failing until
the bug fix for #522 is applied. (#536)
- Implemented UKO Layer 1 Domain Ontologies (`uko-doc:`, `uko-data:`, `uko-infra:`)
in the OWL/Turtle ontology file (`docs/ontology/uko.ttl`). Added 17 `uko-doc:` classes
(Document, Section, Paragraph, Citation, etc.), 13 `uko-data:` classes (Table, Column,
+6
View File
@@ -1,4 +1,10 @@
[behave]
paths = features
# Exclude @wip scenarios globally so TDD failing tests do not break CI.
# Any contributor tagging a scenario @wip will have it skipped by default.
# NOTE: --tags=@wip on the CLI will NOT work; Behave ANDs ini and CLI tags.
# To run a @wip scenario locally, target it by file/line number:
# behave features/<file>.feature:<line>
tags = ~@wip
stdout_capture = no
stderr_capture = no
+118
View File
@@ -0,0 +1,118 @@
"""ASV benchmarks for ``agents init --yes`` invocation time.
Measures in-process execution time for the init command with the --yes flag,
which should perform non-interactive initialization using defaults.
These benchmarks target bug #522 and are expected to error until the fix
is applied (the --yes flag does not yet exist).
"""
from __future__ import annotations
import shutil
import sys
import tempfile
from pathlib import Path
from unittest.mock import create_autospec, patch
try:
from cleveragents.cli.main import app
except ModuleNotFoundError:
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
from cleveragents.cli.main import app
from typer.testing import CliRunner
from cleveragents.application.services.project_service import ProjectService
class _MockProject:
"""Lightweight stand-in for the Project model with spec fields.
Outdated
Review

F2 -- Bug (Low): _last_exit_code not defensively initialized in setup()

self._last_exit_code is only assigned inside _invoke_with_mock() (line 58). While track_exit_code() calls _invoke_with_mock() first so under normal execution it works, if the mock setup or runner.invoke() raises an exception before line 58, the subsequent return self._last_exit_code will raise AttributeError rather than a meaningful error.

Consider initializing it here:

def setup(self) -> None:
    self._runner = CliRunner()
    self._tmpdir = tempfile.mkdtemp()
    self._last_exit_code = -1

This follows the defensive pattern seen in other benchmark suites and ensures ASV always gets a numeric value.

**F2 -- Bug (Low): `_last_exit_code` not defensively initialized in `setup()`** `self._last_exit_code` is only assigned inside `_invoke_with_mock()` (line 58). While `track_exit_code()` calls `_invoke_with_mock()` first so under normal execution it works, if the mock setup or `runner.invoke()` raises an exception before line 58, the subsequent `return self._last_exit_code` will raise `AttributeError` rather than a meaningful error. Consider initializing it here: ```python def setup(self) -> None: self._runner = CliRunner() self._tmpdir = tempfile.mkdtemp() self._last_exit_code = -1 ``` This follows the defensive pattern seen in other benchmark suites and ensures ASV always gets a numeric value.
We cannot use ``create_autospec(Project)`` because the spec-required
output fields (``data_dir``, ``config_path``, ``database_status``,
brent.edwards marked this conversation as resolved Outdated
Outdated
Review

Nit: No teardown() method to clean up self._tmpdir. ASV calls setup() before each iteration, so repeated runs will accumulate orphan temp directories. Consider adding:

def teardown(self) -> None:
    import shutil
    shutil.rmtree(self._tmpdir, ignore_errors=True)
Nit: No `teardown()` method to clean up `self._tmpdir`. ASV calls `setup()` before each iteration, so repeated runs will accumulate orphan temp directories. Consider adding: ```python def teardown(self) -> None: import shutil shutil.rmtree(self._tmpdir, ignore_errors=True) ```
``directories``) do not yet exist on the legacy ``Project`` model.
"""
brent.edwards marked this conversation as resolved Outdated
Outdated
Review

Minor: The mock setup block (lines 37-45 / 51-59) is duplicated between the two benchmark methods. Could extract a helper like _invoke_with_mock(self, args: list[str]) to reduce duplication. Not blocking — the two methods are short enough that readability isn't harmed.

Minor: The mock setup block (lines 37-45 / 51-59) is duplicated between the two benchmark methods. Could extract a helper like `_invoke_with_mock(self, args: list[str])` to reduce duplication. Not blocking — the two methods are short enough that readability isn't harmed.
name: str
path: Path
data_dir: Path
config_path: Path
database_status: str
directories: list[str]
class InitYesFlagSuite:
"""Benchmark ``agents init --yes`` invocation."""
timeout = 30.0
Outdated
Review

M2: This benchmark tests --path which is not in the spec (specification.md:1217 defines only agents init [--yes|-y]). TDD benchmarks should drive toward spec behavior. Recommend removing this method.

**M2**: This benchmark tests `--path` which is not in the spec (`specification.md:1217` defines only `agents init [--yes|-y]`). TDD benchmarks should drive toward spec behavior. Recommend removing this method.
def setup(self) -> None:
self._runner = CliRunner()
self._tmpdir = tempfile.mkdtemp()
self._last_exit_code = -1
def teardown(self) -> None:
Outdated
Review

F5 — MEDIUM: Missing -y short-form benchmark variant

The suite benchmarks --yes (long form) and --yes --path but not -y (short form). If alias resolution introduces different overhead, this would go undetected.

Suggested fix — add a -y benchmark:

def time_init_short_y_flag(self) -> None:
    """Measure end-to-end latency of ``agents init -y``."""
    self._invoke_with_mock(["init", "-y"], "bench-project")
**F5 — MEDIUM: Missing `-y` short-form benchmark variant** The suite benchmarks `--yes` (long form) and `--yes --path` but not `-y` (short form). If alias resolution introduces different overhead, this would go undetected. **Suggested fix — add a `-y` benchmark:** ```python def time_init_short_y_flag(self) -> None: """Measure end-to-end latency of ``agents init -y``.""" self._invoke_with_mock(["init", "-y"], "bench-project") ```
shutil.rmtree(self._tmpdir, ignore_errors=True)
def _invoke_with_mock(self, args: list[str], project_name: str) -> None:
Outdated
Review

F4 — MEDIUM: Benchmark stores exit code but never validates it

The docstring correctly notes that benchmark data is only meaningful on exit code 0, but _last_exit_code is stored and never checked. After the --yes flag is implemented, a regression that reintroduces NoSuchOption would silently cause the benchmark to report error-path latency with no signal to the developer.

Suggested fix — consider one of:

  1. Add a track_exit_code method so ASV tracks the value as a metric:
def track_exit_code(self) -> int:
    self._invoke_with_mock(["init", "--yes"], "bench-project")
    return self._last_exit_code
  1. Or add an assertion inside _invoke_with_mock gated by a flag (disabled while TDD-failing, enabled after fix).
**F4 — MEDIUM: Benchmark stores exit code but never validates it** The docstring correctly notes that benchmark data is only meaningful on exit code 0, but `_last_exit_code` is stored and never checked. After the `--yes` flag is implemented, a regression that reintroduces `NoSuchOption` would silently cause the benchmark to report error-path latency with no signal to the developer. **Suggested fix — consider one of:** 1. Add a `track_exit_code` method so ASV tracks the value as a metric: ```python def track_exit_code(self) -> int: self._invoke_with_mock(["init", "--yes"], "bench-project") return self._last_exit_code ``` 2. Or add an assertion inside `_invoke_with_mock` gated by a flag (disabled while TDD-failing, enabled after fix).
"""Invoke the CLI with a mocked container and the given *args*.
Outdated
Review

F3 (MEDIUM): The spec defines agents init [--yes|-y] with no --path option (specification.md:1217). The spec's init is a global environment reset, not a project init. When #522 aligns the implementation with the spec, --path may be removed, making this benchmark invalid.

Suggestion: Add a comment noting --path is an implementation detail subject to change with #522, or remove this benchmark.

**F3 (MEDIUM):** The spec defines `agents init [--yes|-y]` with **no** `--path` option (`specification.md:1217`). The spec's `init` is a global environment reset, not a project init. When #522 aligns the implementation with the spec, `--path` may be removed, making this benchmark invalid. **Suggestion:** Add a comment noting `--path` is an implementation detail subject to change with #522, or remove this benchmark.
Stores the result exit code so benchmark data is only meaningful
when the command succeeds (exit code 0). When ``--yes`` is not yet
implemented the benchmark measures error-path latency; after the
fix it measures real init latency.
"""
with patch(
"cleveragents.application.container.get_container"
) as mock_container:
mock_service = create_autospec(ProjectService, instance=True)
mock_project = _MockProject()
mock_project.name = project_name
mock_project.path = Path(self._tmpdir)
# Pre-populate spec-required fields (specification.md:1381-1386)
# so benchmarks measure real output paths once #522 is fixed.
Outdated
Review

F1 -- Bug (Medium): Missing ASV .unit metadata on track_exit_code()

Every track_* method across all benchmark files in this codebase declares a .unit class attribute for ASV metric labeling. Examples:

  • bench_unit_tests.py:90 -- StepModuleDiscoverySuite.track_step_file_count.unit = "files"
  • bench_unit_tests.py:200 -- TrackTestSuiteMetrics.track_scenario_line_count.unit = "scenarios"
  • bench_coverage_report.py:205 -- CoverageFileSuite.track_source_file_count.unit = "files"
  • bench_subprocess_overhead.py:55 -- SubprocessCountSuite.track_subprocess_count.unit = "subprocesses"

This new track_exit_code() is missing the attribute. Without it, ASV will display the metric with no unit label in its HTML reports and JSON results. Add after the class body:

InitYesFlagSuite.track_exit_code.unit = "exit_code"
**F1 -- Bug (Medium): Missing ASV `.unit` metadata on `track_exit_code()`** Every `track_*` method across all benchmark files in this codebase declares a `.unit` class attribute for ASV metric labeling. Examples: - `bench_unit_tests.py:90` -- `StepModuleDiscoverySuite.track_step_file_count.unit = "files"` - `bench_unit_tests.py:200` -- `TrackTestSuiteMetrics.track_scenario_line_count.unit = "scenarios"` - `bench_coverage_report.py:205` -- `CoverageFileSuite.track_source_file_count.unit = "files"` - `bench_subprocess_overhead.py:55` -- `SubprocessCountSuite.track_subprocess_count.unit = "subprocesses"` This new `track_exit_code()` is missing the attribute. Without it, ASV will display the metric with no unit label in its HTML reports and JSON results. Add after the class body: ```python InitYesFlagSuite.track_exit_code.unit = "exit_code" ```
mock_project.data_dir = Path(self._tmpdir)
mock_project.config_path = Path(self._tmpdir) / "config.toml"
mock_project.database_status = "initialized (schema v3)"
mock_project.directories = ["logs", "cache", "sessions", "contexts"]
mock_service.initialize_project.return_value = mock_project
mock_container.return_value.project_service.return_value = mock_service
result = self._runner.invoke(app, args)
self._last_exit_code = result.exit_code
def time_init_yes_flag(self) -> None:
"""Measure end-to-end latency of ``agents init --yes``."""
self._invoke_with_mock(["init", "--yes"], "bench-project")
Outdated
Review

H1: # type: ignore[attr-defined]pyrightconfig.json sets "include": ["src"], so benchmarks/ is not type-checked by Pyright. This comment is both unnecessary and a CONTRIBUTING.md violation ("Never use # type: ignore"). Remove it.

**H1**: `# type: ignore[attr-defined]` — `pyrightconfig.json` sets `"include": ["src"]`, so `benchmarks/` is not type-checked by Pyright. This comment is both unnecessary and a `CONTRIBUTING.md` violation ("Never use `# type: ignore`"). Remove it.
def time_init_short_y_flag(self) -> None:
"""Measure end-to-end latency of ``agents init -y``."""
self._invoke_with_mock(["init", "-y"], "bench-project")
def time_init_yes_flag_with_path(self) -> None:
"""Measure latency of ``agents init --yes --path <dir>``.
NOTE: ``--path`` is an implementation detail of the current ``init``
command. The spec (``specification.md:1217``) defines only
``agents init [--yes|-y]`` with no ``--path`` option. When #522
aligns the command with the spec this benchmark may need to be
removed or updated.
"""
self._invoke_with_mock(
["init", "--yes", "--path", self._tmpdir], "bench-path-project"
)
def track_exit_code(self) -> int:
"""Track the CLI exit code as an ASV metric.
Returns 0 when ``--yes`` is correctly implemented; non-zero while
the flag is missing. This lets ASV detect regressions that
reintroduce ``NoSuchOption`` without silently reporting error-path
latency.
"""
self._invoke_with_mock(["init", "--yes"], "bench-project")
return self._last_exit_code
InitYesFlagSuite.track_exit_code.unit = "exit_code"
+56
View File
@@ -0,0 +1,56 @@
# These tests target bug #522 and are expected to fail until the fix is applied.
#
# NOTE FOR FIX AUTHOR (#522):
# Scenarios 1-4 will fail for TWO independent reasons:
# (a) The --yes / -y flag is not yet implemented (NoSuchOption).
# (b) The current init_command output format (project.py) does not match
# the spec-defined output at docs/specification.md:1381-1402.
Outdated
Review

F8 — LOW: No @wip or expected-failure tag for CI filtering

All scenarios use @tdd @bug522 but there is no tag that the CI runner / nox session can use to skip or expect-fail these tests. Without a filter, CI will report 4 failures on every run until #522 is fixed.

Consider adding @wip and configuring nox -s unit_tests to pass --tags='not @wip' (or equivalent), or to treat @wip failures as expected.

**F8 — LOW: No `@wip` or expected-failure tag for CI filtering** All scenarios use `@tdd @bug522` but there is no tag that the CI runner / nox session can use to skip or expect-fail these tests. Without a filter, CI will report 4 failures on every run until #522 is fixed. Consider adding `@wip` and configuring `nox -s unit_tests` to pass `--tags='not @wip'` (or equivalent), or to treat `@wip` failures as expected.
# The fix must address both: add the --yes flag AND remodel the output to
# match the spec (Data Dir, Config, Database, Directories fields with the
# "Initialized (non-interactive)" status message).
Feature: CLI init --yes flag for non-interactive initialization
As a developer using CleverAgents in CI or scripts
I want to run "agents init --yes" for non-interactive initialization
So that I can skip interactive prompts and use sensible defaults
@tdd @bug522 @wip
Scenario: agents init --yes completes without error
Given I have a temporary project directory for init
Outdated
Review

The expected output strings ("initialized successfully", "Location:", "Database:", "Project Initialized") are reasonable guesses based on the specification, but since the --yes code path doesn't exist yet, the bug-fix author for #522 will need to ensure their output matches these expectations — or coordinate to update these assertions. Just flagging so this is on the radar for whoever picks up #522.


Resolved — Corrected in commit 429ba8db per F1 feedback from @CoreRasurae. These were not guesses — the spec provides exact expected output at docs/specification.md:1381-1402 and Workflow Example 1 at 34317-34326. Assertions now match the specification: "Initialized (non-interactive)", "Data Dir:", "Database:", "Initialized" (box title).

The expected output strings (`"initialized successfully"`, `"Location:"`, `"Database:"`, `"Project Initialized"`) are reasonable guesses based on the specification, but since the `--yes` code path doesn't exist yet, the bug-fix author for #522 will need to ensure their output matches these expectations — or coordinate to update these assertions. Just flagging so this is on the radar for whoever picks up #522. --- **Resolved** — Corrected in commit `429ba8db` per F1 feedback from @CoreRasurae. These were not guesses — the spec provides exact expected output at `docs/specification.md:1381-1402` and Workflow Example 1 at `34317-34326`. Assertions now match the specification: `"Initialized (non-interactive)"`, `"Data Dir:"`, `"Database:"`, `"Initialized"` (box title).
When I run agents init with the --yes flag
Outdated
Review

F9 — LOW: Prompt-absence check is unreachable until fix is applied

The no interactive prompt should have been presented step (line 20) will never execute in the current TDD state because the preceding exit-code assertion at line 18 fails first (since --yes doesn't exist yet). This means the prompt-suppression logic has zero validation that the step itself works correctly until the fix arrives.

This is an inherent limitation of TDD failing tests — not something that needs fixing now, but worth being aware of when the fix PR is submitted. At that point, ensure the prompt check is manually verified to be exercising real logic.

**F9 — LOW: Prompt-absence check is unreachable until fix is applied** The `no interactive prompt should have been presented` step (line 20) will never execute in the current TDD state because the preceding exit-code assertion at line 18 fails first (since `--yes` doesn't exist yet). This means the prompt-suppression logic has zero validation that the step itself works correctly until the fix arrives. This is an inherent limitation of TDD failing tests — not something that needs fixing now, but worth being aware of when the fix PR is submitted. At that point, ensure the prompt check is manually verified to be exercising real logic.
Then the init command should exit with code 0
And the project service initialize_project should have been called
@tdd @bug522 @wip
Scenario: --yes suppresses interactive prompts
Given I have a temporary project directory for init
When I run agents init with the --yes flag
Then the init command should exit with code 0
Outdated
Review

F2 — HIGH: No mock behavioral verification for -y short-form path

The --yes scenario (line 8) includes And the project service initialize_project should have been called, verifying the mock was actually exercised. This scenario lacks that step.

Without it, the -y code path could produce correct output text through a fallback/error path without ever calling initialize_project, and the test would still pass.

Suggested fix — add mock verification:

    And the project service initialize_project should have been called
**F2 — HIGH: No mock behavioral verification for `-y` short-form path** The `--yes` scenario (line 8) includes `And the project service initialize_project should have been called`, verifying the mock was actually exercised. This scenario lacks that step. Without it, the `-y` code path could produce correct output text through a fallback/error path without ever calling `initialize_project`, and the test would still pass. **Suggested fix — add mock verification:** ```gherkin And the project service initialize_project should have been called ```
And the init output should contain "Initialized (non-interactive)"
And no interactive prompt should have been presented
@tdd @bug522 @wip
Scenario: -y short-form alias completes without error
Given I have a temporary project directory for init
When I run agents init with the -y flag
Outdated
Review

F4 -- Test Coverage (Low): Spec-required output value formats not verified

The spec at docs/specification.md:1381-1388 and the Workflow Example at line 34319-34326 define specific value content for each field:

Field Spec value Tested?
Data Dir: .../.cleveragents (created) Key only
Config: .../.cleveragents/config.toml Key only
Database: initialized (schema v3) Key only
Directories: logs, cache, sessions, contexts Key only

All four assertions check only that the label exists, not the value structure. The tests would pass even if the output printed Database: ERROR or Directories: with no value. Consider adding at least one assertion that spot-checks a value format, e.g.:

And the init output should contain "initialized (schema v3)"
**F4 -- Test Coverage (Low): Spec-required output value formats not verified** The spec at `docs/specification.md:1381-1388` and the Workflow Example at line 34319-34326 define specific value content for each field: | Field | Spec value | Tested? | |:------|:-----------|:--------| | `Data Dir:` | `.../.cleveragents (created)` | Key only | | `Config:` | `.../.cleveragents/config.toml` | Key only | | `Database:` | `initialized (schema v3)` | Key only | | `Directories:` | `logs, cache, sessions, contexts` | Key only | All four assertions check only that the label exists, not the value structure. The tests would pass even if the output printed `Database: ERROR` or `Directories:` with no value. Consider adding at least one assertion that spot-checks a value format, e.g.: ```gherkin And the init output should contain "initialized (schema v3)" ```
Then the init command should exit with code 0
And the init output should contain "Initialized (non-interactive)"
Outdated
Review

F1 — HIGH: Missing Config: and Directories: output assertions against specification

The spec (docs/specification.md:1381-1400) defines four output fields for agents init --yes:

Spec field Tested here?
Data Dir: Yes (line 34)
Config: No
Database: Yes (line 35)
Directories: No

The issue's acceptance criterion #3 explicitly requires: "produces the expected output (data dir, config, database, directories)". Two of the four are missing.

Suggested fix — add two more assertion steps:

    And the init output should contain "Config:"
    And the init output should contain "Directories:"
**F1 — HIGH: Missing `Config:` and `Directories:` output assertions against specification** The spec (`docs/specification.md:1381-1400`) defines four output fields for `agents init --yes`: | Spec field | Tested here? | |:-----------|:-------------| | `Data Dir:` | Yes (line 34) | | `Config:` | **No** | | `Database:` | Yes (line 35) | | `Directories:` | **No** | The issue's acceptance criterion #3 explicitly requires: *"produces the expected output (data dir, config, database, directories)"*. Two of the four are missing. **Suggested fix — add two more assertion steps:** ```gherkin And the init output should contain "Config:" And the init output should contain "Directories:" ```
And the project service initialize_project should have been called
@tdd @bug522 @wip
Scenario: Output includes expected initialization summary
Given I have a temporary project directory for init
When I run agents init with the --yes flag
Then the init command should exit with code 0
And the init output should contain "Data Dir:"
And the init output should contain "Config:"
And the init output should contain "Database:"
Outdated
Review

L1: Gherkin convention uses And for consecutive assertion steps. This should be And the init output should indicate interactive mode.

**L1**: Gherkin convention uses `And` for consecutive assertion steps. This should be `And the init output should indicate interactive mode`.
And the init output should contain "Directories:"
And the init output should contain "logs, cache, sessions, contexts"
And the init output should contain "Initialized"
@tdd @bug522
Scenario: Interactive mode without --yes presents a prompt
Given I have a temporary project directory for init
When I run agents init without the --yes flag
Then the init command should exit with code 0
And the init output should indicate interactive mode
+210
View File
@@ -0,0 +1,210 @@
"""Step definitions for CLI init --yes flag tests (bug #522).
These tests are TDD-style: they assert the CORRECT expected behaviour of
``agents init --yes`` and are expected to FAIL until the bug fix is applied.
"""
from __future__ import annotations
import os
import re
import shutil
import tempfile
from pathlib import Path
from unittest.mock import create_autospec, patch
from behave import given, then, when
from typer.testing import CliRunner
from cleveragents.application.services.project_service import ProjectService
from cleveragents.cli.main import app
Outdated
Review

Minor: context.original_cwd is saved here but never restored (no os.chdir(context.original_cwd) in a cleanup/after step). The existing project_commands_coverage_steps.py has the same pattern, so this is consistent — but if environment.py's after_scenario doesn't handle CWD restoration and the temp dir gets cleaned up first, subsequent scenarios could see a stale CWD.

Worth confirming after_scenario handles this, or adding a restore in a @then cleanup step or after_scenario hook. Low priority since it matches existing convention.


Resolved — Addressed in commit 435093c2. Added context.add_cleanup(_restore_cwd, context) which restores the original CWD and cleans up the temp directory via a Behave cleanup callback, ensuring proper teardown regardless of scenario outcome.

Minor: `context.original_cwd` is saved here but never restored (no `os.chdir(context.original_cwd)` in a cleanup/after step). The existing `project_commands_coverage_steps.py` has the same pattern, so this is consistent — but if `environment.py`'s `after_scenario` doesn't handle CWD restoration and the temp dir gets cleaned up first, subsequent scenarios could see a stale CWD. Worth confirming `after_scenario` handles this, or adding a restore in a `@then` cleanup step or `after_scenario` hook. Low priority since it matches existing convention. --- **Resolved** — Addressed in commit `435093c2`. Added `context.add_cleanup(_restore_cwd, context)` which restores the original CWD and cleans up the temp directory via a Behave cleanup callback, ensuring proper teardown regardless of scenario outcome.
def _restore_cwd(context):
Outdated
Review

F4 (MEDIUM): This overwrites context.original_cwd set by environment.py:222 (os.getcwd()str) with a Path object, creating a type inconsistency. It also silently collides with the framework attribute.

Suggestion: Use a step-private name and match the framework's type:

context._init_original_cwd = os.getcwd()
**F4 (MEDIUM):** This overwrites `context.original_cwd` set by `environment.py:222` (`os.getcwd()` → `str`) with a `Path` object, creating a type inconsistency. It also silently collides with the framework attribute. **Suggestion:** Use a step-private name and match the framework's type: ```python context._init_original_cwd = os.getcwd() ```
"""Restore the original working directory, env var, and clean up."""
os.chdir(context._init_original_cwd)
if context._init_original_home is None:
Outdated
Review

F5 -- Test Flaw (Low): Missing CLEVERAGENTS_HOME environment isolation

The Robot tests properly isolate via CLEVERAGENTS_HOME set in common.resource:23. This Behave step relies on os.chdir(context.temp_dir) but never sets CLEVERAGENTS_HOME.

Currently mitigated because get_container() is mocked at line 40, so the real init path is never exercised. However, when the #522 bug fix is applied and mocks are potentially adjusted, if any code path resolves paths via CLEVERAGENTS_HOME instead of CWD, the test could leak into the developer's real home directory. The before_all() hook in features/environment.py does not set CLEVERAGENTS_HOME either.

Consider adding:

os.environ["CLEVERAGENTS_HOME"] = context.temp_dir

and restoring it during cleanup, matching the Robot test isolation pattern.

**F5 -- Test Flaw (Low): Missing `CLEVERAGENTS_HOME` environment isolation** The Robot tests properly isolate via `CLEVERAGENTS_HOME` set in `common.resource:23`. This Behave step relies on `os.chdir(context.temp_dir)` but never sets `CLEVERAGENTS_HOME`. Currently mitigated because `get_container()` is mocked at line 40, so the real init path is never exercised. However, when the #522 bug fix is applied and mocks are potentially adjusted, if any code path resolves paths via `CLEVERAGENTS_HOME` instead of CWD, the test could leak into the developer's real home directory. The `before_all()` hook in `features/environment.py` does not set `CLEVERAGENTS_HOME` either. Consider adding: ```python os.environ["CLEVERAGENTS_HOME"] = context.temp_dir ``` and restoring it during cleanup, matching the Robot test isolation pattern.
os.environ.pop("CLEVERAGENTS_HOME", None)
else:
os.environ["CLEVERAGENTS_HOME"] = context._init_original_home
shutil.rmtree(context.temp_dir, ignore_errors=True)
def _create_init_mocks(context):
"""Create and configure mocked container, service, and project.
Outdated
Review

F6 -- Informational: Spec vs. implementation alignment risk in mock target

The spec at docs/specification.md:1219-1224 describes agents init as a global environment reset: "Initialize or reset the global CleverAgents environment. This wipes any existing data and re-creates the global config and database."

These steps mock project_service.initialize_project(), which aligns with the current implementation at src/cleveragents/cli/main.py:351-415 where init delegates to project_init_command(). However, the spec semantics (global environment reset) differ from the implementation semantics (project initialization).

If the #522 fix remodels init to match the spec's global-reset semantics (wipe data, re-create config/database, create directories), the mock target would need to change. This is inherent to the TDD approach but worth noting so the fix implementer is aware the mock strategy may need adaptation.

**F6 -- Informational: Spec vs. implementation alignment risk in mock target** The spec at `docs/specification.md:1219-1224` describes `agents init` as a **global environment reset**: *"Initialize or reset the global CleverAgents environment. This wipes any existing data and re-creates the global config and database."* These steps mock `project_service.initialize_project()`, which aligns with the current implementation at `src/cleveragents/cli/main.py:351-415` where `init` delegates to `project_init_command()`. However, the spec semantics (global environment reset) differ from the implementation semantics (project initialization). If the #522 fix remodels `init` to match the spec's global-reset semantics (wipe data, re-create config/database, create directories), the mock target would need to change. This is inherent to the TDD approach but worth noting so the fix implementer is aware the mock strategy may need adaptation.
Returns ``(patcher, mock_service)`` where *patcher* is the started
``patch`` context manager.
Outdated
Review

F2 (MEDIUM): The mock only sets .name and .path, but specification.md:1381-1386 requires the init output to include Data Dir:, Config:, Database:, and Directories: fields. When the #522 fix remodels init_command(), unset MagicMock attributes will stringify as <MagicMock id='...'>, causing output assertions to fail for the wrong reason.

Suggestion: Pre-populate spec-required fields:

mock_project.data_dir = Path(context.temp_dir)
mock_project.config_path = Path(context.temp_dir) / "config.toml"
mock_project.database_status = "initialized (schema v3)"
mock_project.directories = ["logs", "cache", "sessions", "contexts"]
**F2 (MEDIUM):** The mock only sets `.name` and `.path`, but `specification.md:1381-1386` requires the init output to include `Data Dir:`, `Config:`, `Database:`, and `Directories:` fields. When the #522 fix remodels `init_command()`, unset MagicMock attributes will stringify as `<MagicMock id='...'>`, causing output assertions to fail for the wrong reason. **Suggestion:** Pre-populate spec-required fields: ```python mock_project.data_dir = Path(context.temp_dir) mock_project.config_path = Path(context.temp_dir) / "config.toml" mock_project.database_status = "initialized (schema v3)" mock_project.directories = ["logs", "cache", "sessions", "contexts"] ```
The service mock uses ``create_autospec(ProjectService)`` so that
attribute access and method calls are validated against the real
``ProjectService`` interface. The project mock uses a typed
``_MockProject`` class instead of ``MagicMock`` because the
spec-required output fields (``data_dir``, ``config_path``,
``database_status``, ``directories``) do not yet exist on the legacy
``Project`` model — they will be added when the #522 fix aligns the
model with ``docs/specification.md:1381-1386``. Using
``create_autospec(Project)`` would reject those attribute assignments.
"""
mock_service = create_autospec(ProjectService, instance=True)
class _MockProject:
"""Lightweight stand-in for the Project model with spec fields."""
name: str
path: Path
data_dir: Path
config_path: Path
database_status: str
directories: list[str]
mock_project = _MockProject()
mock_project.name = Path(context.temp_dir).name
mock_project.path = Path(context.temp_dir)
# Pre-populate spec-required fields (specification.md:1381-1386) so
# that when the #522 fix lands, output assertions fail for the right
# reason (real bugs) rather than MagicMock stringification artefacts.
mock_project.data_dir = Path(context.temp_dir)
mock_project.config_path = Path(context.temp_dir) / "config.toml"
Outdated
Review

Nit: step_no_interactive_prompt currently only re-asserts exit_code == 0, which the preceding step in the scenario already checks. This makes the step a tautology — it doesn't actually verify that no prompts were presented.

When the bug-fix author implements --yes, consider strengthening this to something like:

# Verify no prompt-like tokens appeared in output
output = context.result["output"]
for prompt_token in ("[Y/n]", "[y/N]", "? ", "Enter "):
    assert prompt_token not in output, (
        f"Unexpected prompt token '{prompt_token}' found in output:\n{output}"
    )

That way the step actually exercises the "no prompts" invariant rather than duplicating the exit-code check. Not blocking since the fix PR will likely revisit this step anyway.


Resolved — Addressed in commit 435093c2. The step now checks for prompt-like tokens ([Y/n], [y/N], Continue? , Proceed? , Enter , Confirm , (yes/no)) instead of duplicating the exit-code assertion. The overly broad "? " token was subsequently refined in commit 429ba8db per F5 feedback from @CoreRasurae.

Nit: `step_no_interactive_prompt` currently only re-asserts `exit_code == 0`, which the preceding step in the scenario already checks. This makes the step a tautology — it doesn't actually verify that no prompts were presented. When the bug-fix author implements `--yes`, consider strengthening this to something like: ```python # Verify no prompt-like tokens appeared in output output = context.result["output"] for prompt_token in ("[Y/n]", "[y/N]", "? ", "Enter "): assert prompt_token not in output, ( f"Unexpected prompt token '{prompt_token}' found in output:\n{output}" ) ``` That way the step actually exercises the "no prompts" invariant rather than duplicating the exit-code check. Not blocking since the fix PR will likely revisit this step anyway. --- **Resolved** — Addressed in commit `435093c2`. The step now checks for prompt-like tokens (`[Y/n]`, `[y/N]`, `Continue? `, `Proceed? `, `Enter `, `Confirm `, `(yes/no)`) instead of duplicating the exit-code assertion. The overly broad `"? "` token was subsequently refined in commit `429ba8db` per F5 feedback from @CoreRasurae.
mock_project.database_status = "initialized (schema v3)"
mock_project.directories = ["logs", "cache", "sessions", "contexts"]
mock_service.initialize_project.return_value = mock_project
patcher = patch("cleveragents.application.container.get_container")
Outdated
Review

M3: The mock patches get_container and accesses .project_service().initialize_project(), which is the current (pre-spec) implementation. The spec defines agents init as a global environment reset (specification.md:1219-1224), not a project-level operation. Add a comment noting this coupling will change when #522 refactors init to match the spec.

**M3**: The mock patches `get_container` and accesses `.project_service().initialize_project()`, which is the current (pre-spec) implementation. The spec defines `agents init` as a global environment reset (`specification.md:1219-1224`), not a project-level operation. Add a comment noting this coupling will change when #522 refactors init to match the spec.
mock_container = patcher.start()
mock_container.return_value.project_service.return_value = mock_service
return patcher, mock_service
@given("I have a temporary project directory for init")
def step_temp_project_directory(context):
"""Create a temporary directory and store it on *context*."""
context.temp_dir = tempfile.mkdtemp()
# Use step-private attribute names (prefixed with _init_) to avoid
# colliding with environment.py's context.original_cwd (str).
# Match the framework's type (str via os.getcwd()) for consistency.
context._init_original_cwd = os.getcwd()
context._init_original_home = os.environ.get("CLEVERAGENTS_HOME")
os.environ["CLEVERAGENTS_HOME"] = context.temp_dir
os.chdir(context.temp_dir)
context.add_cleanup(_restore_cwd, context)
def _run_init_with_flag(context, flag: str) -> None:
"""Invoke ``agents init`` with the given flag via the Typer test runner."""
runner = CliRunner()
patcher, mock_service = _create_init_mocks(context)
try:
result = runner.invoke(app, ["init", flag])
finally:
patcher.stop()
context.init_yes_result = {
"exit_code": result.exit_code,
"output": result.output,
}
Outdated
Review

F6 — MEDIUM: Prompt token "Enter " risks false positives

The substring "Enter " could match legitimate non-prompt output such as "Entered configuration..." or "Enter key configured". The spec's interactive prompt pattern is Continue? [y/N]: — more precise patterns would reduce false-positive risk.

Suggested fix — narrow the token or switch to a pattern:

prompt_tokens = (
    "[Y/n]",
    "[y/N]",
    "Continue? ",
    "Proceed? ",
    "Enter a ",       # more specific than bare "Enter "
    "Confirm ",
    "(yes/no)",
)

Alternatively, a regex like r"(?:Enter|Confirm|Proceed|Continue)\s.*[:?]" would match actual prompt structures more precisely.

**F6 — MEDIUM: Prompt token `"Enter "` risks false positives** The substring `"Enter "` could match legitimate non-prompt output such as `"Entered configuration..."` or `"Enter key configured"`. The spec's interactive prompt pattern is `Continue? [y/N]:` — more precise patterns would reduce false-positive risk. **Suggested fix — narrow the token or switch to a pattern:** ```python prompt_tokens = ( "[Y/n]", "[y/N]", "Continue? ", "Proceed? ", "Enter a ", # more specific than bare "Enter " "Confirm ", "(yes/no)", ) ``` Alternatively, a regex like `r"(?:Enter|Confirm|Proceed|Continue)\s.*[:?]"` would match actual prompt structures more precisely.
Outdated
Review

F3 -- Test Flaw (Low): Prompt token narrowing reduces negative-assertion coverage

Changing from "Enter " to "Enter a " addresses false-positive risk on legitimate output (e.g., "Enterprise"), but it now misses realistic interactive prompt patterns like:

  • "Enter project name: "
  • "Enter path: "
  • "Enter value: "

A more targeted approach -- such as a regex like r"Enter\s+\S+.*[:\?]" or checking for "Enter " only at line boundaries -- would preserve detection coverage without the false-positive risk. As-is, the negative assertion has a blind spot for a common prompt pattern.

**F3 -- Test Flaw (Low): Prompt token narrowing reduces negative-assertion coverage** Changing from `"Enter "` to `"Enter a "` addresses false-positive risk on legitimate output (e.g., "Enterprise"), but it now misses realistic interactive prompt patterns like: - `"Enter project name: "` - `"Enter path: "` - `"Enter value: "` A more targeted approach -- such as a regex like `r"Enter\s+\S+.*[:\?]"` or checking for `"Enter "` only at line boundaries -- would preserve detection coverage without the false-positive risk. As-is, the negative assertion has a blind spot for a common prompt pattern.
context.init_yes_raw_result = result
context.init_yes_mock_service = mock_service
@when("I run agents init with the --yes flag")
def step_run_init_yes(context):
"""Invoke ``agents init --yes`` via the Typer test runner."""
_run_init_with_flag(context, "--yes")
@when("I run agents init with the -y flag")
def step_run_init_short_yes(context):
"""Invoke ``agents init -y`` via the Typer test runner."""
_run_init_with_flag(context, "-y")
Outdated
Review

F1 (MEDIUM) + F5 (LOW): The greedy .* will consume the entire remainder of the line up to the last : or ?, which can over-match if the init output contains colons in non-prompt contexts (e.g., Config: /path). Also, \? is redundant inside [].

Suggestion:

enter_prompt = re.search(r"Enter\s+\S+.*?[:?]", output)

Non-greedy .*? stops at the first : or ?, matching only the actual prompt.

**F1 (MEDIUM) + F5 (LOW):** The greedy `.*` will consume the entire remainder of the line up to the **last** `:` or `?`, which can over-match if the init output contains colons in non-prompt contexts (e.g., `Config: /path`). Also, `\?` is redundant inside `[]`. **Suggestion:** ```python enter_prompt = re.search(r"Enter\s+\S+.*?[:?]", output) ``` Non-greedy `.*?` stops at the first `:` or `?`, matching only the actual prompt.
@then("the init command should exit with code {code:d}")
def step_init_exit_code(context, code):
"""Assert the init command exited with the expected code."""
actual = context.init_yes_result["exit_code"]
assert actual == code, (
f"Expected exit code {code}, got {actual}. "
f"Output: {context.init_yes_result['output']}"
)
@then('the init output should contain "{text}"')
def step_init_output_contains(context, text):
"""Assert that the init command output contains *text*."""
output = context.init_yes_result["output"]
assert text in output, f"Expected '{text}' in output:\n{output}"
@then("the project service initialize_project should have been called")
def step_initialize_project_called(context):
"""Assert the mock project service's initialize_project was invoked."""
context.init_yes_mock_service.initialize_project.assert_called_once()
@when("I run agents init without the --yes flag")
def step_run_init_no_yes(context):
"""Invoke ``agents init`` without --yes (interactive mode)."""
runner = CliRunner()
patcher, _mock_service = _create_init_mocks(context)
try:
result = runner.invoke(app, ["init"])
finally:
patcher.stop()
context.init_yes_result = {
"exit_code": result.exit_code,
"output": result.output,
}
context.init_yes_raw_result = result
@then("the init output should indicate interactive mode")
def step_output_indicates_interactive(context):
"""Assert that without --yes the output does NOT contain the
non-interactive marker, indicating the command ran in interactive mode.
This is the negative complement to the --yes scenarios: when the fix
lands, ``agents init`` (without --yes) should either present a prompt
or omit the ``Initialized (non-interactive)`` marker.
"""
output = context.init_yes_result["output"]
assert "Initialized (non-interactive)" not in output, (
"Expected interactive mode but output contains "
f"'Initialized (non-interactive)':\n{output}"
)
@then("no interactive prompt should have been presented")
def step_no_interactive_prompt(context):
"""Assert that no interactive prompt was presented.
Verifies that the command output does not contain common prompt-like
tokens, which would indicate the ``--yes`` flag failed to suppress
interactive prompts.
"""
output = context.init_yes_result["output"]
prompt_tokens = (
"[Y/n]",
"[y/N]",
"Continue? ",
"Proceed? ",
"Confirm ",
"(yes/no)",
)
for token in prompt_tokens:
assert token not in output, (
f"Unexpected prompt token '{token}' found in output:\n{output}"
)
# Regex catches "Enter project name:", "Enter path:", "Enter value?"
# without false-positiving on "Entered configuration" or "Enterprise".
# Non-greedy .*? stops at the first : or ? to avoid over-matching
# when the line contains colons in non-prompt contexts (e.g. "Config: /path").
enter_prompt = re.search(r"Enter\s+\S+.*?[:?]", output)
assert enter_prompt is None, (
f"Unexpected Enter-style prompt found in output: "
f"'{enter_prompt.group()}'\n{output}"
)
+2
View File
@@ -574,6 +574,8 @@ def integration_tests(session: nox.Session):
"discovery",
"--exclude",
"code_blocks",
"--exclude",
"wip",
*robot_args,
"robot/",
)
+52
View File
@@ -0,0 +1,52 @@
*** Settings ***
Documentation Integration smoke test for agents init --yes (bug #522).
... These tests are TDD-style and expected to FAIL until the
... bug fix is applied.
Resource ${CURDIR}/common.resource
Library Process
Library OperatingSystem
Library String
Suite Setup Setup Test Environment
Suite Teardown Cleanup Test Environment
*** Test Cases ***
Init Yes Flag Exits Without Error
[Documentation] agents init --yes should complete with exit code 0
[Tags] wip
${tmpdir}= Evaluate __import__('tempfile').mkdtemp(prefix='init_yes_')
${result}= Run Process ${PYTHON} -m cleveragents init --yes
... timeout=60s cwd=${tmpdir}
Outdated
Review

F7 — MEDIUM: [Teardown] placed between action and assertion steps

In Robot Framework, [Teardown] is a test setting that always runs after the full test body, regardless of where it appears in the source. Placing it between Run Process and Should Be Equal is functionally correct but visually misleading — it looks like teardown happens before the assertions.

Every Robot Framework style guide places [Teardown] as the last line of the test case. This same pattern applies to all three test cases in this file (lines 18, 27, 38).

Suggested fix: Move [Teardown] to the final line of each test case.

**F7 — MEDIUM: `[Teardown]` placed between action and assertion steps** In Robot Framework, `[Teardown]` is a test setting that always runs after the full test body, regardless of where it appears in the source. Placing it between `Run Process` and `Should Be Equal` is functionally correct but visually misleading — it looks like teardown happens before the assertions. Every Robot Framework style guide places `[Teardown]` as the last line of the test case. This same pattern applies to all three test cases in this file (lines 18, 27, 38). **Suggested fix:** Move `[Teardown]` to the final line of each test case.
Should Be Equal As Integers ${result.rc} 0
... msg=Expected exit code 0 but got ${result.rc}. stderr: ${result.stderr}
[Teardown] Remove Directory ${tmpdir} recursive=True
Init Yes Flag Produces Summary Output
[Documentation] agents init --yes should produce the initialization summary
... with all spec-required output fields.
[Tags] wip
${tmpdir}= Evaluate __import__('tempfile').mkdtemp(prefix='init_yes_out_')
${result}= Run Process ${PYTHON} -m cleveragents init --yes
... timeout=60s cwd=${tmpdir}
Should Be Equal As Integers ${result.rc} 0
Outdated
Review

F3 — HIGH: Robot "Produces Summary Output" only checks status message, not spec fields

This test verifies Initialized (non-interactive) but none of the four spec-required output fields (Data Dir:, Config:, Database:, Directories:). This makes it nearly identical to the exit-code-only test above.

Suggested fix — add output field assertions:

    Should Contain    ${result.stdout}    Data Dir:
    ...    msg=Output should contain Data Dir field per spec
    Should Contain    ${result.stdout}    Config:
    ...    msg=Output should contain Config field per spec
    Should Contain    ${result.stdout}    Database:
    ...    msg=Output should contain Database field per spec
    Should Contain    ${result.stdout}    Directories:
    ...    msg=Output should contain Directories field per spec
**F3 — HIGH: Robot "Produces Summary Output" only checks status message, not spec fields** This test verifies `Initialized (non-interactive)` but none of the four spec-required output fields (`Data Dir:`, `Config:`, `Database:`, `Directories:`). This makes it nearly identical to the exit-code-only test above. **Suggested fix — add output field assertions:** ```robot Should Contain ${result.stdout} Data Dir: ... msg=Output should contain Data Dir field per spec Should Contain ${result.stdout} Config: ... msg=Output should contain Config field per spec Should Contain ${result.stdout} Database: ... msg=Output should contain Database field per spec Should Contain ${result.stdout} Directories: ... msg=Output should contain Directories field per spec ```
... msg=Expected exit code 0 but got ${result.rc}. stderr: ${result.stderr}
Should Contain ${result.stdout} Initialized (non-interactive)
... msg=Output should contain non-interactive initialization message per spec
Should Contain ${result.stdout} Data Dir:
... msg=Output should contain Data Dir field per spec
Should Contain ${result.stdout} Config:
... msg=Output should contain Config field per spec
Should Contain ${result.stdout} Database:
... msg=Output should contain Database field per spec
Should Contain ${result.stdout} Directories:
... msg=Output should contain Directories field per spec
[Teardown] Remove Directory ${tmpdir} recursive=True
Init Short Y Flag Exits Without Error
[Documentation] agents init -y should complete with exit code 0 (short-form alias)
[Tags] wip
${tmpdir}= Evaluate __import__('tempfile').mkdtemp(prefix='init_y_')
${result}= Run Process ${PYTHON} -m cleveragents init -y
... timeout=60s cwd=${tmpdir}
Should Be Equal As Integers ${result.rc} 0
... msg=Expected exit code 0 but got ${result.rc}. stderr: ${result.stderr}
[Teardown] Remove Directory ${tmpdir} recursive=True