Extract _get_api_key() and _create_provider_instance() as a single internal
factory that handles all provider types, including MOCK. Both create_llm() and
create_ai_provider() now delegate to _create_provider_instance(), eliminating
the duplicated if/elif provider-switching chains and the divergence between the
two code paths. Adding a new provider now requires touching exactly one place.
Key changes:
- New _get_api_key(provider_type) consolidates API key lookup and validation
that was previously duplicated across create_llm(), _create_provider_llm(),
and every branch of create_ai_provider().
- New _create_provider_instance() is the unified raw-LLM factory, replacing
_create_provider_llm() and the specialised adapter constructors in
create_ai_provider(). MOCK is now handled here via FakeListLLM.
- create_ai_provider() wraps all providers uniformly in LangChainChatProvider
instead of using specialised adapters (OpenAIChatProvider, AnthropicChatProvider,
GoogleChatProvider, OpenRouterChatProvider) for the four primary providers.
- create_llm() drops its own API key validation block; validation now happens
inside _create_provider_instance() via _get_api_key().
- All BDD scenarios updated: _create_provider_llm references become
_create_provider_instance, and provider-type assertions on create_ai_provider
results are updated to LangChainChatProvider.
Review fixes (Cycle 2):
- Add _coerce_env_bool() for consistent boolean env-var coercion in
CLEVERAGENTS_ALLOW_MOCK_PROVIDER checks.
- Introduce _ApiKeyMissing sentinel to distinguish 'not provided' from None
in provider configuration.
- Set MOCK supports_streaming=False in DEFAULT_CAPABILITIES to align with
FakeListLLM semantics.
- Make MOCK is_configured conditional on CLEVERAGENTS_ALLOW_MOCK_PROVIDER.
- Use FakeListLLM(responses=['mock response'] * 10) to prevent IndexError
in multi-step workflows.
- Strip max_retries from kwargs before passing to LangChain constructors.
- Extract _resolve_provider_type() helper shared by create_llm() and
create_ai_provider() to reduce duplication and improve readability.
- Ensure __api_key_sentinel flows through factory_kwargs even when api_key
is None, avoiding double _get_api_key() calls.
- Add CHANGELOG env-var documentation and providers.md env-var table.
- Resolve template DB lock issues by persisting in-memory SQLite via
VACUUM INTO instead of direct file creation on tmpfs.
ISSUES CLOSED: #10949
The root cause was a race condition in parallel Behave workers caused by
mutating a module-level singleton (_service) in
cleveragents.cli.commands.session. Concurrent cleanup in one worker could
reset _service to None while another worker was still using it, leading to
intermittent tell command test failures and exit code 1.
The fix patches the _get_session_service function directly in the affected
test steps (session_cli_coverage_boost_steps.py and
session_cli_uncovered_branches_steps.py) to avoid mutating the module-level
_service, and adds a reset call in features/environment.py's after_scenario
to ensure the singleton is cleared between scenarios, preventing stale
service instances from leaking across tests.
Closes#9121
Surface the non-AssertionError guard warning in standard Behave output by emitting to stderr in addition to the structured logger, and add infrastructure coverage that asserts this guard path is visible during test runs. Document the @tdd_expected_fail expectation that bug-signaling failures must use AssertionError so infrastructure exceptions are not accidentally treated as expected bug failures.
ISSUES CLOSED: #8294
Add a process-global `_INITIALIZED_DBS: set[str]` to `features/environment.py` that caches database URLs after `_fast_init_or_upgrade` has processed them. On subsequent calls with the same URL, the function returns immediately — skipping all URL parsing, path extraction, prefix matching, and `stat()` syscalls that previously executed on every new `UnitOfWork` instance.
The cache is cleared at the start of each scenario in `before_scenario` to prevent cross-scenario state leaks, since each scenario receives fresh temp-DB paths via `tempfile.mktemp()`.
Four new Behave scenarios validate cache hits on repeated calls, cache clearing between scenarios, and non-caching of non-matching-prefix URLs.
Estimated savings: ~65,000 unnecessary function-body executions eliminated per full test run (~7 UnitOfWork instantiations × ~10,700 scenarios).
ISSUES CLOSED: #735
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Rename the TDD tag system from tdd_bug/tdd_bug_<N> to tdd_issue/tdd_issue_<N>
across the entire codebase. The tdd_expected_fail tag is unchanged.
The TDD expected-failure workflow is not limited to bug fixes — it applies
equally to any issue type (features, tasks, refactors). The _bug suffix was
misleading and narrowed the perceived scope. The new _issue suffix accurately
reflects that the TDD tagging system applies to any Forgejo issue.
Changes span 92 files:
- features/environment.py: validate_tdd_tags(), should_invert_result(), and
apply_tdd_inversion() updated — regex, variables, error messages
- robot/tdd_expected_fail_listener.py: _validate_tdd_tags(), _should_invert_result(),
start_test(), end_test() updated consistently
- 33 Behave .feature files: all @tdd_bug/@tdd_bug_<N> tags renamed
- 29 Robot .robot files: all tdd_bug/tdd_bug_<N> tags renamed
- 3 Robot fixture files renamed (tdd_bug_alone, tdd_missing_tdd_bug,
tdd_expected_fail_missing_bug_n) with content and references updated
- Tag validation tests and helpers updated (function names, command dispatch
keys, output strings, fixture references)
- CONTRIBUTING.md: section renamed from 'TDD Bug Test Tags' to
'TDD Issue Test Tags', all tag references and examples updated
- noxfile.py: comment references updated
- Step definition files, mock helpers, and benchmark files: docstring
references updated
ISSUES CLOSED: #965
The _notify_facade and _facade_dispatch functions call get_container()
during lazy facade construction. This can trigger structlog output that
corrupts CLI stdout captured by CliRunner in tests.
Wrap facade construction in redirect_stdout/redirect_stderr to
suppress any side-effect output. Also reset the facade singleton
in after_scenario for test isolation.
Implemented the three-tag TDD bug-capture system in Behave environment hooks:
- Added tag validation in before_scenario: @tdd_bug_<N> requires @tdd_bug,
@tdd_expected_fail requires both @tdd_bug and @tdd_bug_<N>
- Added result inversion via Scenario.run() wrapper installed in before_all:
scenarios tagged @tdd_expected_fail that fail are reported as passed
(expected failure), and scenarios that unexpectedly pass are reported as
failed with guidance to remove the tag
- Added helper functions validate_tdd_tags() and should_invert_result()
- Added inline documentation referencing CONTRIBUTING.md > TDD Bug Test Tags
- Added Behave test scenarios for tag validation and inversion behavior
- Extract apply_tdd_inversion() as a public, testable function that
encapsulates all inversion logic with proper guards
- Refactored handle_tdd_expected_fail() to delegate to apply_tdd_inversion()
after tag validation, eliminating ~55 lines of duplicated inversion logic
- Tag validation errors in handle_tdd_expected_fail() are now logged at
WARNING level instead of being silently swallowed
- Add hook_failed guard: never invert infrastructure/hook errors
- Add was_dry_run guard: skip inversion when no test actually ran
- Add non-AssertionError guard: warn and skip inversion for exceptions
that likely indicate infrastructure problems, not the captured bug
- Log exception details at DEBUG level before clearing (previously
discarded silently)
- Attach synthetic AssertionError to last step on unexpected pass so
the failure reason appears in standard Behave formatter output
ISSUES CLOSED: #627
When a test database already exists and is non-empty (created by a prior
step like `I have initialized a project`), _fast_init_or_upgrade in
features/environment.py was falling through to _original_init_or_upgrade,
running a full Alembic migration version check every time. This created
redundant SQLAlchemy engines, caused SQLite lock contention under
parallel execution, and triggered cumulative overhead from @database_retry
decorators (wait_fixed(0.5) × 3 attempts on ~100 repository methods),
leading to intermittent hangs and CI timeouts.
The fix changes the existing-non-empty-database branch to return
immediately instead of delegating to the original migration runner.
This is safe because test databases are either template-copied (already
at Alembic HEAD) or created by a prior step that already ran the full
migration. The _SCENARIO_DB_PREFIXES guard ensures non-scenario
databases (migration-runner unit tests) remain unaffected.
Verified: 15/15 scenarios pass with both --processes 1 and --processes 16.
ISSUES CLOSED: #726
Implement TDD bug-capture tests for bug #570 where `agents session create`
fails because `_get_session_service()` calls `container.db()` which does
not exist on the DI Container class (AttributeError). Same root cause as
bug #554.
Behave BDD scenarios tagged @tdd_bug @tdd_bug_570 @tdd_expected_fail
exercise the real DI path (no mocks). Includes Robot Framework integration
smoke tests with self-inverting helper and ASV benchmark baseline.
ISSUES CLOSED: #631
Implement TDD bug-capture tests for bug #554 where `agents session list`
fails because `_get_session_service()` calls `container.db()` which does
not exist on the DI Container class (AttributeError).
Behave BDD scenarios tagged @tdd_bug @tdd_bug_554 @tdd_expected_fail
exercise the real DI path (no mocks) and assert correct behavior. The
@tdd_expected_fail handler in environment.py inverts failed→passed while
the bug is present, keeping CI green.
Also adds:
- @tdd_expected_fail infrastructure in features/environment.py
(tag validation + status inversion in after_scenario hook)
- behave-parallel exit logic fix to use summary-based failure
detection (compatible with TDD status inversion)
- Robot Framework integration smoke tests with self-inverting helper
- ASV benchmark baseline for session list command throughput
ISSUES CLOSED: #630
Implemented lazy container activation for devcontainer-instance resources
with ContainerLifecycleState enum tracking six states (inactive, starting,
active, stopping, stopped, error) with validated transitions. Extended
DevcontainerHandler with devcontainer up CLI integration and JSON output
parsing for container start. Added periodic health checking via
devcontainer exec ping with configurable interval. Added agents resource
stop and agents resource rebuild CLI commands for manual lifecycle
control. Wired session close and plan completion hooks to automatic
container cleanup. Includes lifecycle state persistence in resource
registry with timestamped transitions. Added Behave BDD tests, Robot
integration tests, and ASV activation latency benchmarks.
- Added remoteWorkspaceFolder absolute-path validation
- Aligned spec: handler name, rebuild types, --yes flag on stop/rebuild
- Added registry re-read in stop_container success path for consistency
- Added session_id field to ContainerLifecycleTracker for scoped cleanup
- Scoped stop_all_active_containers to session_id when provided
- Wired _cleanup_devcontainers into fail_apply and fail_execute
- Wired start_health_check into activate_container success path
- Restructured facade session close to always run container cleanup
even without session service (F4)
- Re-read tracker from registry in activate_container success path
- Added evict_terminal_trackers to cap registry growth
- Updated devcontainer_resources.md: health check auto-start, scoped
cleanup hooks, known limitations for eviction and sandbox_strategy
- Wired evict_terminal_trackers into stop_all_active_containers so
terminal-state trackers are actually evicted in production
- Made stop_container idempotent: returns early when container is
already in a terminal state instead of raising ValueError
- Fixed benchmark health check thread leak in TimeActivationLatency
by clearing registry after each timing loop
- Added rebuild pass-through (--reset-container flag to devcontainer up)
- Added host_workspace_path field on ContainerLifecycleTracker so
health probes use the host-side path for devcontainer exec
- Wired lazy activation into DevcontainerHandler.resolve() for
devcontainer-instance resources in non-running states
- Changed _default_strategy from SNAPSHOT to NONE (container
itself provides isolation; SandboxFactory raises NotImplementedError
for snapshot)
- Restricted _STOPPABLE_TYPES to devcontainer-instance only
(container-instance is not directly stoppable via CLI)
ISSUES CLOSED: #514
Cap time.sleep and asyncio.sleep globally at 10ms in before_all to
eliminate retry/backoff waits (tenacity wait_fixed, retry_auto_debug
exponential backoff). Save originals as time._original_sleep and
asyncio._original_sleep for tests that need real wall-clock delays
(CircuitBreaker recovery, debounce timers, validation timeouts).
Enhance template-DB patch: add "db." prefix to _SCENARIO_DB_PREFIXES
and check db_path.stat().st_size > 0 so 0-byte auto-created SQLite
files receive the template copy instead of falling through to real
migrations.
Replace subprocess.run CLI invocations with typer.testing.CliRunner
in module_coverage, main_coverage_complete, and coverage_extras step
files, eliminating ~6s Python cold-start overhead per call.
Switch plan_persistence and action_persistence to in-memory SQLite by
default; only cross-restart scenarios use file-based via an explicit
Given step.
Replace MigrationRunner.run_migrations with Base.metadata.create_all
in plan_service_steps for freshly created in-memory engines.
Removed leftover debug comment in environment.py.
Total tier runtime: 565s -> 21s (96% reduction), 20 features all
under 5s behave-internal time.
ISSUES CLOSED: #480
Optimized the 8 features accounting for 64% of total BDD test runtime
(1,505s of 2,352s):
- features/environment.py: Added _ensure_template_db() for direct behave
invocations, expanded _SCENARIO_DB_PREFIXES to include "test_" for
step-file-created DBs, added @mock_only tag support to skip
unnecessary DB setup.
- features/plan_commands_coverage.feature: Added @mock_only tag (fully
mocked, no DB needed).
- features/plan_service.feature: 14 actor-resolution scenarios now use
lightweight in-memory plan service instead of heavyweight file-based
DB + project init.
- features/steps/plan_service_steps.py: New
step_create_lightweight_plan_service for actor-resolution tests.
- features/steps/services_coverage_steps.py: Extracted 3 helper functions
to consolidate 8 near-identical Given steps (~200 lines of duplicated
boilerplate removed).
Per-feature results: services_coverage 245s->2.8s (99%), context_service
215s->2.1s (99%), project_service 140s->0.7s (99%), plan_service
215s->4.6s (98%), core_cli_commands 113s->4.2s (96%), cli_streaming
213s->6.4s (97%), plan_commands_coverage 116s->20s (83%),
cli_plan_context_commands 248s->31.6s (87%).
ISSUES CLOSED: #479
Created scripts/create_template_db.py that builds a pre-migrated SQLite
template database using Base.metadata.create_all() + alembic stamp
(~5ms for 34 tables, vs ~0.5-3s x 25 Alembic migrations per scenario).
Nox unit_tests and coverage_report sessions generate the template before
test execution and propagate CLEVERAGENTS_TEMPLATE_DB env var to all
workers.
features/environment.py before_all() installs a monkey-patch on
MigrationRunner.init_or_upgrade that copies the template for fresh
scenario temp DBs, falling through to real migrations for :memory:,
existing files, and migration-runner unit tests.
Quick wins: sleep(0.5) -> sleep(0.05) in cli_streaming wait step;
removed redundant Background re-declaration in cli_streaming.feature
scenario 7.
ISSUES CLOSED: #483
Implemented namespace/project/plan/skill permission model with role
bindings (owner/admin/editor/viewer) and default deny policy. Added
enforcement hooks at CLI/service boundaries that are server-only; local
mode returns permissive defaults. Includes role enums, permission check
service, role matrix documentation.
Includes Behave BDD scenarios, Robot integration tests, ASV benchmarks,
and reference documentation.
ISSUES CLOSED: #344
P0: reject register() after close_all() with RuntimeError.
P1: catch CancelledError in close_all(), use WeakKeyDictionary for
cancellation_reasons to prevent memory leak, guard StateManager
update_state/reset/load_checkpoint/time_travel after close().
P2: contextlib.suppress in __del__ for partial construction, re-cancel
pending tasks in cleanup_tasks_async, handle late tasks added during
await window, guard AcpEventQueue.publish() after close with _is_closed
flag and is_closed property, fix ASV TimeRegisterBatch crash.
Tests: 5 new Behave scenarios (T1-T4 + is_closed), log handler and
event loop cleanup in after_scenario (T5-T6).
Docs: async_safety.md updated for register-after-close and state
mutation guards.
ISSUES CLOSED: #321
Each behave-parallel worker previously resolved to the same file-based
SQLite database (cleveragents.db or .cleveragents/db.sqlite) because
before_scenario removed the CLEVERAGENTS_DATABASE_URL env var and the
fallback paths are shared across processes. Under parallel execution
this caused intermittent 'database is locked' and duplicate-project
errors—most visibly in the coverage_report nox session.
Replace the env-var removal in before_scenario with unique temp database
paths via tempfile.mktemp(), giving every scenario its own isolated
database file. Temp files are cleaned up in after_scenario.
Also fix cli_streaming_steps.py where a Background + duplicate Given
sequence triggered a duplicate project name collision, and update
coverage_boost_steps.py so Settings-default assertions explicitly clear
the env vars before testing pydantic defaults.
hurui200320
HAL9000
brent.edwards
freemo
Luis Mendes
khyari hamza
CleverAgents Dev