1248 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Luis Mendes	d196e90779	test(integration): workflow example 14 — server mode team collaboration (supervised profile) ... Implemented Robot Framework integration test suite validating the server mode team collaboration workflow (Specification Example 14) with mocked LLM providers and in-memory domain services. Tests cover server mode configuration, config-registry diagnostics, namespace management, action publishing to team namespaces with namespaced actor references, shared action consumption via use_action(), and namespace-filtered plan monitoring across namespaces. Uses standard integration test infrastructure with mock AI. Post-review fixes applied: - F1: plan_namespace() now creates actions, calls use_action() to instantiate plans, and verifies namespace-filtered listing with non-empty results - F2: Renamed actor_namespace() to action_actor_refs() with accurate docstring (tests action actor field references, not ActorService) - F3: Added use_shared_action() subcommand covering spec Step 3 (use a shared action to create a plan and verify namespace inheritance) - F5: Updated diagnostics() docstring to accurately describe scope (config registry verification, not full diagnostic operations) - F7: Fixed _COMMANDS type from dict[str, object] to dict[str, Callable] removing type: ignore suppression - F9: Extracted _make_config_service() factory to remove duplicated tempdir creation pattern - F10: Added [Tags] to all Robot test cases for filtering support - F11: Added automation_profile="supervised" to all create_action() calls for spec consistency - M1: Fixed CHANGELOG wording from "mocked server backend" to "in-memory domain services" to accurately describe the test approach - M5: Added symmetric phase and action_name assertions for beta plan in plan_namespace() to match alpha plan verification depth - M6: Added strategy_actor and review_actor assertions after get_action() in action_actor_refs() for complete actor field persistence verification - M8: Added core.namespace default value verification ("local") in diagnostics() subcommand - L2: Added total unfiltered action count verification in shared_actions() - L3: Added distinguishing tags (publish/shared) to disambiguate Robot test cases with previously identical tag sets - L4: Added ActionState.AVAILABLE assertion for all actions in shared_actions() for consistency with action_namespace() ISSUES CLOSED: #778	2026-03-26 22:33:19 +00:00
CoreRasurae	80f1664a0d	test(integration): workflow example 7 — CI/CD integration, automated PR review and fix (ci profile) ... Implemented Robot Framework integration test suite validating the CI/CD workflow (Specification Example 7). Tests cover ci-profile configuration (automation-profile, format, log level), idempotent resource and project registration with duplicate-detection assertions, three validation tools (ci-lint, ci-typecheck, ci-tests) registration and resource attachment via ToolRegistryService, action creation with typed arguments and invariants per spec Step 2, plan lifecycle with phase-by-phase completion through all phases (strategize, execute, apply) until terminal applied state, and JSON output structure verification including plan_id, phase, state, action, projects, and arguments fields. Post-review fixes applied (round 1): - H1: Fix truncated invariant #2 text to match spec line 39051 - H2: Fix definition_of_done to match spec's 5-bullet-point format - M1: Register all 3 spec validations (ci-lint, ci-typecheck, ci-tests) - M3: Add branch property to resource registration per spec Step 3 - M4: Replace phantom resource_id with real registered resource - M5: Add projects and arguments field assertions to JSON output test - M7: Replace 'polling-based' with 'phase-by-phase' in docs/comments - L1: Use timezone-aware datetime (timezone.utc) - L2: Use tempfile for resource path instead of hardcoded /tmp - L3: Clarify json_output() docstring to reflect as_cli_dict() scope - L4: Tighten attachment count assertion from >= 1 to == 1 (now == 3) Post-review fixes applied (round 2): - M2: Use Setup Test Environment With Database Isolation for pabot safety - L1: Replace remaining hardcoded /tmp paths with tempfile (validation_attach, resource_idempotent duplicate registration) - L2: Fix stale 'Polling-based' comment missed in round 1 - L5: Align action description with spec line 39027 - M1/L3/L6: Document automation_profile propagation gap in use_action() with TODO for when production code wires the field onto Plan - L4: Add comment explaining local actor name deviation from spec Includes CHANGELOG update describing the integration test scope. ISSUES CLOSED: #771	2026-03-26 21:42:37 +00:00
CoreRasurae	8ad156f5e1	build(nox): Expose ENV var to build pre-migrated database template	2026-03-26 19:35:54 +00:00
CoreRasurae	d2c70bd489	test(e2e): workflow example 14 — server mode team collaboration (supervised profile) ... Implemented Robot Framework E2E test suite validating Specification Workflow Example 14 (Server Mode Team Collaboration). Tests exercise the real CleverAgents CLI with zero mocking, covering server mode configuration (server.url, server.token, core.namespace), diagnostics, action publishing to team namespace with namespace-scoped listing, actor registration, plan list graceful handling, and supervised automation profile verification. Review fixes applied: - Added WF14 Suite Setup with agents init and uuid4 suffix generation - Added WF14 Suite Teardown to reset config state (server.url, server.token, core.namespace) - Added server.token config round-trip per spec Example 14 Step 1 - Changed action name to myteam/ namespace prefix per spec Step 2 - Added action list --namespace myteam test per issue AC - Replaced epoch-based name uniqueness with uuid4 for CI safety - Updated diagnostics test documentation (no server connectivity check) - Changed to Force Tags E2E at suite level for consistency - Added description field to actor YAML configuration - Updated plan list test documentation to reflect actual behavior - Reverted unrelated cls->klass rename from strategy_registry.py (CONTRIBUTING.md: do not mix cosmetic changes with functional changes) - Updated CHANGELOG to accurately reflect test coverage ISSUES CLOSED: #760	2026-03-26 18:56:16 +00:00
CoreRasurae	7cf03f03f4	fix(action): query persisted actions from database in list_actions ... list_actions() previously merged in-memory cache with the database but used the cache as the primary source. Actions created by previous CLI invocations were only stored in the database and were invisible to the current process. When persistence is enabled and a Unit of Work is available, the method now queries the database first — using get_by_namespace() when a namespace filter is given, or the new list_all() otherwise — and refreshes the in-memory cache with any newly-discovered entries. Falls back to the in-memory cache on DatabaseError or when no UoW is wired. Added ActionRepository.list_all() to retrieve all persisted actions ordered by namespace and name. Refs: #760	2026-03-26 18:51:43 +00:00
CoreRasurae	cefbca73c2	test(e2e): workflow example 7 — CI/CD integration, automated PR review and fix (ci profile) ... Implemented Robot Framework E2E test suite validating Specification Workflow Example 7 (CI/CD Integration). Tests exercise the real CleverAgents CLI with zero mocking, covering: - ci-profile configuration with JSON output format and WARN log level (spec Step 1) - Idempotent resource and project registration with resource linking, verified via strict occurrence-count assertions - Three-validation registration (lint, typecheck, tests) with project attachment and tool-list verification - CI plan launch with action args (pr_branch, base_branch), explicit plan execute for lifecycle progression, and terminal-state assertion - JSON output verification via json.loads() with raw_decode fallback for mixed log/JSON CLI output - Graceful degradation via Skip If No LLM Keys when API keys are unavailable Resource/project naming follows spec convention (local/ci-workspace project, local/ci-main resource). Entity creation commands tolerate "already exists" for CI re-runnability. Config assertions use stdout-only matching and exact equality. Validation naming aligned with spec (local/ci-lint per §Example 7, line 39130). Added robot/common_vars.py module placeholder for shared Robot Framework variables. ISSUES CLOSED: #753	2026-03-26 18:10:14 +00:00
Luis Mendes	01efa2922c	fix(test): align behave BDD scenarios with updated lifecycle-apply behavior ... Updated automation_levels.feature to expect "applied" processing state instead of "queued" after completing execute on a full_automation plan, matching the new auto_progress() behavior that drives Apply to terminal state. Updated plan_cli_coverage_boost_steps.py to properly mock the full apply lifecycle (get_plan → apply_plan → start_apply → complete_apply) instead of returning a single plan from apply_plan, matching the updated lifecycle_apply_plan command handler that now drives plans through to the terminal applied state. Refs: #753	2026-03-26 17:35:34 +00:00
Luis Mendes	59be111e1d	fix(plan): complete apply phase inline in auto_progress and lifecycle-apply CLI ... Modified auto_progress() to complete the Apply phase immediately after transitioning from Execute to Apply, since Apply is a metadata transition with no LLM processing. This ensures `plan execute` drives the plan to the terminal `applied` state when the automation profile permits (ci, full-auto profiles with auto_apply < 1.0). Extracted `_complete_apply_if_queued()` helper that consolidates the Apply-completion pattern (start_apply + complete_apply) into a single method with error recovery (calls `fail_apply` on failure) and async-job guard (skips inline completion when async execution is enabled to avoid orphaning enqueued jobs). Used by `auto_progress()`, `lifecycle_apply_plan()`, and `try_auto_run()`. Added PlanLifecycleService.try_auto_run() that drives plans through all lifecycle phases (strategize → execute → apply) when automation-profile thresholds allow automatic progression. Each phase checks the profile's auto_* threshold before proceeding; a threshold of 1.0 stops the plan at that phase boundary for human approval. Fixed `lifecycle-apply` CLI leaving plans stuck in `apply/queued` without completing. The command now calls `_complete_apply_if_queued()` when the plan is in Apply/queued, driving it to the terminal `applied` state. Fixed stale RICH output in `lifecycle_apply_plan` that printed "Plan is now in Apply phase (queued)" after the plan had already reached terminal `applied` state; now branches on `plan.is_terminal`. Additional fixes: - SQLite UNIQUE constraint violation in LifecyclePlanRepository.update(): added session.flush() after clear() on child collections (project_links, arguments, invariants) before re-inserting rows - Added 'state' alias in _plan_spec_dict() JSON output for spec §Example 7 jq compatibility - Updated plan execute and lifecycle-apply reference documentation Refs: #753	2026-03-26 17:35:24 +00:00
CoreRasurae	e4c01492d5	refactor(cli): align actor run signature with spec positional args ... Aligned the `agents actor run` command signature with the specification by introducing positional NAME and PROMPT arguments. The --config/-c option is preserved as an optional fallback for direct YAML invocation. When NAME is provided without --config, the actor is resolved from the Actor Registry. Updated both actor_run.py and actor.py run commands. Added backward compatibility: if --config is provided, it takes precedence over name-based resolution. Review fixes applied (code review round 1): - P1-1: Narrowed bare `except Exception` to `except NotFoundError` in _resolve_config_files to avoid masking infrastructure errors. - P1-2: Moved _resolve_config_files call inside the try block in run() so container/registry init errors get user-friendly messages. Added `except click.exceptions.Exit: raise` to let typer.Exit propagate through the broadened try scope. - P1-3: Added atexit.register cleanup for temp files created by _resolve_config_files (resource leak fix). - P1-4: Added CHANGELOG.md entry for the breaking CLI change. - P2-1: Extracted duplicated _resolve_config_files to shared module `_resolve_actor.py`; both actor.py and actor_run.py now import it. - P2-2: Added guard for actors with no configuration data (config_blob=None) to produce a clear error instead of invalid YAML. - P2-3/P2-4: Added 5 BDD scenarios exercising the real resolve_config_files function (registry path, yaml_text path, config_blob fallback, no-config-data error, not-found error). - P2-5: Added @coverage tags to all new BDD scenarios. - P3-1: Added timeout=120s and on_timeout=kill to Robot tests. - Fixed rxpy_route_validation.robot tests that used the removed --prompt/-p option (replaced with positional NAME + PROMPT args). Review fixes applied (code review round 2): - P2-1: Aligned actor_run.py exception handler from `CleverAgentsException` to `CleverAgentsError`, matching actor.py so infrastructure errors from registry resolution get user-friendly messages instead of falling through to the generic handler. - P2-2: Changed `yaml.dump` to `yaml.safe_dump` in _resolve_actor.py for fail-fast behavior on unexpected types, consistent with the codebase's dominant pattern. - P3-6: Replaced defensive `getattr(actor, ...)` calls with direct Pydantic model attribute access (`actor.yaml_text`, `actor.config_blob`) for type-checker coverage. - P3-1: Switched BDD temp file cleanup from post-assertion `unlink()` to `context.add_cleanup()` for leak-proof teardown. - P2-3/P3-2/P3-3/P3-4: Added 3 BDD edge-case scenarios (empty config_blob dict, infrastructure error propagation, empty string name) and 1 Robot test case (actor_app registry resolution). Review fixes applied (code review round 3): - P1-1: Migrated 48 remaining `-p` invocations across 9 Robot test files to the new positional `NAME PROMPT` pattern (context_delete_all_yes, load_context_test, scientific_paper_e2e_test, routing_prefix_stripping, scientific_paper_basic, scientific_paper_writer_test, context_management_test, initial_next_command_test, system_prompt_template_rendering). - P2-1: Documented `--config/-c` as a spec deviation in `_resolve_actor.py` module docstring (spec lines 4562-4566 define `actor run` with no --config option; issue #901 AC accepts keeping it as optional). - P2-2: Corrected `--config` help text from "fallback" to "overrides registry-based name resolution" — the option takes precedence, not the other way around. - P3-1: Added `.strip()` to `yaml_text` emptiness check in `_resolve_actor.py` to handle whitespace-only values that would otherwise bypass the `config_blob` fallback. - P3-2: Added `from None` to the no-configuration-data `typer.Exit(code=2)` for consistency with the not-found path. - P3-3: Added BDD scenario testing `--config` precedence for `actor_run_app` (was only tested for `actor_app`). - P3-4: Strengthened config_blob BDD scenario to verify generated YAML is parseable via `yaml.safe_load` round-trip. - P3-7: Replaced hardcoded `/tmp/dummy.yaml` with `tempfile.gettempdir()` for portability. - P3-8: Moved 5 inline imports to module level per CONTRIBUTING.md §1292-1294 (3x `import click`, 1x InfrastructureError in steps; 1x `import typer` in robot helper). - P3-9: Added `encoding="utf-8"` to `_write_yaml` in Robot helper for consistency with production code. Review fixes applied (code review round 4): - P2-1: Wrapped `yaml.safe_dump` in `_resolve_actor.py` with `try/except yaml.YAMLError` so non-serialisable config_blob values produce a user-friendly error message instead of a raw traceback. - P2-2: Moved remaining inline `import yaml` to module level in `actor_run_signature_steps.py` per CONTRIBUTING.md §1292-1294. - P2-3: Replaced 3 bare `assert` statements in Robot helper `helper_actor_run_signature.py` with diagnostic `if/print/sys.exit` pattern matching the rest of the file, improving failure diagnostics. Review fixes applied (code review round 5): - P3-4: Replaced per-call `atexit.register(lambda)` in `_resolve_actor.py` with a module-level `_temp_files` set and a single `atexit` handler (`_cleanup_temp_files`) to prevent unbounded handler accumulation in same-process usage (e.g. test suites running multiple CliRunner invocations). - P3-1/P3-2/P3-3: Added 4 BDD scenarios: whitespace-only `yaml_text` fallback to config_blob, config-precedence registry-not-consulted assertion for both `actor_app` and `actor_run_app`, multiple `--config` files with positional NAME. - P4-1: Strengthened error-path BDD assertions to verify error message content (not-found, no-config-data, serialisation-error) alongside exit codes via captured stderr. - P4-2: Added Robot test case for `actor_app` unknown-name error path and corresponding helper function. ISSUES CLOSED: #901	2026-03-26 13:41:31 +00:00
CoreRasurae	00881a3e5f	feat(observability): implement Event System Domain Event Taxonomy (full EventType enum + DomainEvent model) ... Verified and completed the Event System Domain Event Taxonomy implementation. Gap analysis: - EventType enum (38 types across 12 domains): ALREADY EXISTED, verified complete - DomainEvent Pydantic model (all 9 fields): ALREADY EXISTED, verified complete - EventBus Protocol (emit + subscribe): ALREADY EXISTED, verified complete - ReactiveEventBus (RxPY Subject, stream property): ALREADY EXISTED, verified - LoggingEventBus (structlog-based): ALREADY EXISTED, verified complete Added: - In-memory audit_log on ReactiveEventBus (list[DomainEvent] with defensive copy) - Emit ordering aligned with specification: RxPY stream push, then audit append, then handler dispatch (§Event System) - Clarified audit_log docstring: volatile in-memory log, not durable SQLite persistence (durable persistence wired separately via audit service layer) - Behave feature: features/observability/event_system_taxonomy.feature (36 scenarios) - Step definitions: features/steps/event_system_taxonomy_steps.py - Robot integration test: robot/event_system_taxonomy_integration.robot (13 test cases) - Robot helper: robot/helper_event_system_taxonomy.py (13 subcommands) - ASV benchmark: benchmarks/bench_event_bus.py (5 benchmark suites) - vulture_whitelist.py: added audit_log property Code review fixes applied: - Reordered emit() to match spec: on_next → audit_log → handlers (was: audit → on_next → handlers) - Clarified audit_log docstring to distinguish volatile in-memory log from spec SQLite table - Fixed benchmark TaxonomyAuditLogSuite: parameterized setup instead of inline construction - Added teardown to TaxonomyLoggingEmitSuite to restore logging.disable(NOTSET) - Clear _received list between benchmark iterations to reduce noise - Catch specific pydantic.ValidationError in frozen model mutation tests - Consolidated duplicate singular/plural step definitions - Tightened EventType count threshold from >=30 to >=38 Quality gates: - lint: PASSED - typecheck: PASSED (0 errors) - unit_tests: 36/36 new scenarios PASSED (pre-existing 12 flaky failures unchanged) - integration_tests: 1483/1483 PASSED - security_scan: PASSED - dead_code: PASSED ISSUES CLOSED: #587	2026-03-26 11:52:52 +00:00
aditya	f6b8c0ffa9	Merge pull request 'feat(acms): implement builtin/context skill for CRP' (#1149) from feature/m5-crp-context-skill into master ... Reviewed-on: #1149 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>	2026-03-26 08:07:54 +00:00
aditya	1e4b6d5be3	fix(acms): address context skill review feedback ... Use public ContextTierService fragment accessors in builtin/context handlers and remove the unused query_history scope input so the tool contract matches runtime behavior.	2026-03-26 07:52:09 +00:00
aditya	37b6d27d0b	feat(acms): implement builtin/context skill for CRP ... Wire the three CRP tool handlers in context_ops.py to the ACMS pipeline and ContextTierService, replacing NotImplementedError stubs with functional implementations: - request_context: Sources fragments from ContextTierService, filters by query/focus keywords, and delegates to ACMSPipeline.assemble() for budget-constrained context assembly. Accepts optional plan_id for actor-context invocation. - query_history: Searches across hot/warm/cold tiers for fragments matching the query string via case-insensitive substring matching. Returns results sorted by last-accessed timestamp. - get_context_budget: Returns current token budget state (max, reserved, available, used) computed from ContextBudget defaults and hot-tier fragment token counts. Additionally: - Register ACMSPipeline as a Singleton in the DI container - Create build_context_skill_definition() factory for SkillRegistry auto-registration of the builtin/context skill - Replace 3 obsolete NotImplementedError test scenarios with 8 new functional BDD scenarios covering all handler paths Lint, typecheck, and coverage (98%) all pass. ISSUES CLOSED: #873	2026-03-26 07:52:09 +00:00
hurui200320	414abb1396	fix(cli): add missing --yes flag to plan apply command (#1127) ... ## Summary Adds the `--yes`/`-y` flag to the `lifecycle-apply` CLI command as required by the specification (`agents plan apply [--yes|-y] <PLAN_ID>`). Without `--yes`, a confirmation prompt now displays before proceeding with the destructive Apply phase. With `--yes`, the apply proceeds immediately without prompting. Closes #932 ## Changes ### Source Code - **`src/cleveragents/cli/commands/plan.py`**: Added `yes: Annotated[bool, typer.Option("--yes", "-y", help="Skip confirmation prompt")] = False` parameter to `lifecycle_apply_plan`. Added `typer.confirm()` prompt before the apply operation, consistent with the pattern used by `rollback_plan`, `correct_plan`, and other destructive commands. - Confirmation prompt text matches spec exactly: `"Apply changes for plan {plan_id}?"` producing `Apply changes for plan <ID>? [y/N]:`. - Fixed redundant plan ID display when `pre_plan` is `None` — now shows `"Apply changes for plan X?"` instead of `"Apply plan X (X)?"`. - Added `except ValueError` handler consistent with sibling commands `lifecycle_execute_plan` and `_lifecycle_apply_with_id`. - Added `except Exception` catch-all handler with `isinstance(e, (typer.Abort, typer.Exit))` re-raise guard, consistent with `lifecycle_execute_plan`. - Moved `PlanPhase` and `ProcessingState` imports to module level per CONTRIBUTING.md §Import Guidelines. ### TDD Tag Removal (Bug Fix Workflow) - **`features/tdd_plan_apply_yes_flag.feature`**: Removed `@tdd_expected_fail` tag (leaving `@tdd_bug` and `@tdd_bug_932` as permanent regression guards). - **`robot/tdd_plan_apply_yes_flag.robot`**: Removed `tdd_expected_fail` tag (leaving `tdd_bug` and `tdd_bug_932`). ### Test Updates Updated all existing `lifecycle-apply` invocations across 17 test/benchmark files to pass `--yes`, since the new confirmation prompt would otherwise abort in non-interactive test environments: - 9 Behave step definition files - 3 Robot Framework helper scripts - 2 Robot Framework e2e acceptance tests - 3 ASV benchmark files (4 invocations: `cli_robot_flow_bench.py` ×2, `m1_sourcecode_smoke_bench.py` ×1, `plan_cli_smoke_bench.py` ×1) ### Confirmation Prompt Tests (New + Strengthened) - **`features/tdd_plan_apply_yes_flag.feature`**: 5 scenarios total: - `lifecycle-apply recognises the --yes long flag` — verifies flag acceptance, prompt suppression, exit code 0, and `apply_plan` was called - `lifecycle-apply recognises the -y short flag` — same as above for short flag - `lifecycle-apply without --yes prompts for confirmation and user declines` — verifies `"Apply cancelled."` message, `exit_code == 0`, and `apply_plan` was NOT called - `lifecycle-apply without --yes prompts for confirmation and user accepts` — verifies prompt appears, `exit_code == 0`, and `apply_plan` was called - `lifecycle-apply catches unexpected exceptions cleanly` — verifies `"Unexpected error"` output, no traceback leak, non-zero exit code (exercises the `except Exception` catch-all) - **`features/steps/tdd_plan_apply_yes_flag_steps.py`**: Refactored step definitions: - `_make_mock_plan` uses `PlanPhase` and `ProcessingState` enum types instead of raw strings - `_make_mock_plan` uses `datetime.now(tz=UTC)` instead of timezone-naive `datetime.now()` - Unified prompt suppression step handles both `--yes` and `-y` via parameterised step pattern - Added `When` step for unexpected error scenario with `RuntimeError` side_effect - Added `Then` step for non-zero exit code assertion - **Feature/Robot documentation**: Updated stale descriptions that said "implementation does not accept --yes" to reflect the flag is now implemented. ### Documentation - **`docs/reference/plan_cli.md`**: Updated `lifecycle-apply` section with: - `### Synopsis` heading with code block - `### Options` table listing `--yes/-y` and `--format/-f` flags - `### Arguments` table listing `PLAN_ID` - Matches the style used by other command sections in the same file ## Review Fixes (Cycle 3 — Luis's review) | ID | Severity | Issue | Resolution | |----|----------|-------|------------| | M1 | Medium | `typer.Abort()` on user decline produces exit code 1 and redundant "Aborted." | Changed to `raise typer.Exit(0)` — consistent with `correct_decision` and legacy `apply` | | M2 | Medium | Missing exit code assertion on decline scenario | Added `And the lifecycle-apply exit code should be 0` to the decline scenario | | M3 | Medium | Spec compliance: "summary of pending changes" not implemented | Deferred — spec example shows summary *after* confirmation, not before; implementation matches spec. Ticket-vs-spec ambiguity noted. | | L1 | Low | Missing `except Exception` catch-all handler | Added catch-all matching `lifecycle_execute_plan` pattern; re-raises `typer.Abort`/`typer.Exit` | | L2 | Low | Documentation description not updated | Expanded description in `plan_cli.md` to explain confirmation prompt and `--yes` | | L3 | Low | Dead code `is not None` guards | Removed both guards — `get_plan()` raises `NotFoundError`, never returns `None` | | I1 | Info | Duplicate `PlanPhase` import | Hoisted import to top of `try` block, eliminating duplicate at old line 2087 | | I2 | Info | `typer.confirm` without explicit `default=False` | Added `default=False` for consistency with sibling commands | | L4 | Low | No test for `--yes` after positional arg | Not addressed — Typer/Click handles both orderings; low risk | | L5 | Low | No test for auto-select + interactive prompt | Not addressed — separate concern outside ticket scope | | I3 | Info | Robot helper only tests flag recognition | By design — noted as informational | ## Review Fixes (Cycle 4 — Self-QA) | ID | Severity | Issue | Resolution | |----|----------|-------|------------| | Major-1 | Major | No test for `except Exception` catch-all handler | Added new scenario `"lifecycle-apply catches unexpected exceptions cleanly"` with `RuntimeError` side_effect; asserts `"Unexpected error"` output, no traceback, non-zero exit | | Minor-2 | Minor | Missing `ValueError` handler inconsistent with siblings | Added `except ValueError as e:` with `"[red]Execution Error:[/red]"` before catch-all, matching `lifecycle_execute_plan` and `_lifecycle_apply_with_id` | | Minor-3 | Minor | Flag scenarios don't verify `apply_plan` called | Added `And the lifecycle-apply should have called apply` to both `--yes` and `-y` scenarios | | Minor-4 | Minor | Stale docstring in Robot helper references `tdd_expected_fail` inversion | Updated to reflect bug is fixed and tests serve as regression guards | | Minor-5 | Minor | `plan_cli.md` lacks Options table for `lifecycle-apply` | Added Synopsis, Options, and Arguments sections matching sibling command style | | Nit-6 | Nit | Duplicated step defs for `--yes` vs `-y` prompt suppression | Unified into single parameterised step `"the lifecycle-apply {flag} output should not contain the confirmation prompt"` | | Nit-7 | Nit | `datetime.now()` timezone-naive | Changed to `datetime.now(tz=UTC)` | | Nit-8 | Nit | `_make_mock_plan` params use `str` instead of enum types | Changed to `PlanPhase` and `ProcessingState` enum types | ## Review Fixes (Cycle 5 — Jeff's approval note) | ID | Severity | Issue | Resolution | |----|----------|-------|------------| | Import-1 | Minor | `PlanPhase`/`ProcessingState` imports inside function body instead of module level | Moved to module-level import per CONTRIBUTING.md §Import Guidelines | ## Known Limitations / Deferred Items - **M3: Ticket AC mentions "summary of pending changes"** but the spec example only shows `"Apply changes for plan <ID>? [y/N]: y"` without a change summary. The implementation shows plan ID only (matching the spec), not a change summary. This is a ticket-vs-spec ambiguity; recommend discussing with ticket author. - **Legacy `apply` command** accepts `--yes` but does not pass it to `_lifecycle_apply_with_id()`. This is a pre-existing issue outside the scope of this ticket. - **`pre_plan is None` branch** has no explicit test. Pre-existing architectural issue; no action taken. ## Quality Gates | Gate | Result | |------|--------| | `nox -s lint` | ✅ passed | | `nox -s typecheck` | ✅ passed (0 errors) | | `nox -s unit_tests` | ✅ passed (471 features, 12,424 scenarios, 0 failures) | | `nox -s integration_tests` | ✅ passed (1,727 tests, 0 failures) | | `nox -s e2e_tests` | ✅ passed (41 tests, 0 failures) | | `nox -s coverage_report` | ✅ passed (≥97% coverage) | Reviewed-on: #1127 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>	2026-03-26 07:50:09 +00:00
aditya	4aeeaabf34	Merge pull request 'feat(acms): budget enforcement with max_file_size and max_total_size constraints' (#1137) from feature/m5-budget-enforcement into master ... Reviewed-on: #1137 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>	2026-03-26 07:33:06 +00:00
aditya	d8d08facde	feat(acms): budget enforcement with max_file_size and max_total_size constraints ... Implements byte-size budget enforcement for the ACMS context assembly pipeline. enforce_size_budget() filters context fragments against max_file_size (per-fragment) and max_total_size (cumulative) limits defined in a ContextView. New domain models BudgetViolation and BudgetEnforcementResult provide structured violation reporting. Pipeline integration in ACMSPipeline.assemble() applies enforcement as a pre-filter when a context_view is provided. Review feedback addressed: - Fixed PR milestone to v3.4.0 (matching ticket #847) - Rebased branch onto latest master - Added CHANGELOG.md entry - Extracted duplicated enforcement logic into _apply_budget_enforcement() shared method on ACMSPipeline, called by both parent and subclass - Fixed _make_fragment return type from object to ContextFragment - Moved all imports to top of files (acms_pipeline.py, steps file) - Added errors="replace" to encode("utf-8") for surrogate safety - Documented thread-safety caveat on last_enforcement_result property - Added short-circuit early return when both limits are None - Added multi-byte Unicode content test scenario - Added total_size assertions to key scenarios - Added violation type verification to mixed limits scenario - Merged duplicate singular/plural step definitions - Added edge case scenarios (empty list, all exceed, exact boundary) - Fixed misleading docstring in ContextAssemblyPipeline.assemble() - Re-exported VALID_PHASES through core __init__.py public API ISSUES CLOSED: #847	2026-03-26 07:14:43 +00:00
aditya	336a79b681	Merge pull request 'test(cli): TDD failing tests for actor list database update (bug #797)' (#1148) from tdd/m3-actor-list-no-db-update into master ... Reviewed-on: #1148 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>	2026-03-26 07:00:40 +00:00
aditya	968f33c17d	test(cli): TDD failing tests for actor list database update (bug #797) ... Add TDD tests that prove bug #797 exists: the read-only `agents actor list` command triggers database writes via `ActorRegistry.list_actors()` calling `ensure_built_in_actors()`, which invokes `upsert_actor()` and potentially `set_default_actor()` on every invocation. Behave scenarios (features/tdd_actor_list_no_db_update.feature): - Assert upsert_actor is not called during actor list (fails: bug present) - Assert set_default_actor is not called during actor list (fails: bug present) Robot Framework tests (robot/tdd_actor_list_no_db_update.robot): - Integration smoke tests via helper script with same assertions - Both test cases fail as expected, inverted by tdd_expected_fail_listener All tests tagged @tdd_expected_fail @tdd_bug @tdd_bug_797 per the mandatory TDD bug-fix workflow. The inversion mechanism converts the expected failures to passes in CI. When bug #797 is fixed, the @tdd_expected_fail tag will be removed and the tests will run normally. Quality gates: lint pass, typecheck pass, coverage 98.22% (>97%). ISSUES CLOSED: #841	2026-03-26 06:45:11 +00:00
aditya	02d19c9672	Merge pull request 'fix(acms): align budget allocation formula and protocol signatures with spec' (#1140) from fix/acms-budget-allocation-spec-alignment into master ... Reviewed-on: #1140 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>	2026-03-26 06:34:44 +00:00
aditya	42a32c2709	fix(acms): align budget allocation formula and protocol signatures with spec ... Align ACMS pipeline protocol signatures and allocation formula with the specification (docs/specification.md §42630-42937): - BudgetAllocator.allocate() now accepts optional request: ContextRequest parameter per BudgetAllocatorProtocol spec (§44754-44766), enabling future request-aware allocation strategies. - Allocation formula changed from proportional to confidence alone to proportional to confidence * quality_score per spec §45003. Both DefaultBudgetAllocator and ProportionalBudgetAllocator use the new weighted formula. With default quality_score=1.0, behavior is backward- compatible. - DetailDepthResolver.resolve() now accepts budget: int parameter per DetailDepthResolverProtocol spec (§44803-44814), enabling future budget-aware depth resolution decisions. - Default packer changed from no-op DefaultBudgetPacker to production GreedyKnapsackPacker per spec §45013. ACMSPipeline uses lazy import to avoid circular dependency; ContextAssemblyPipeline imports directly. - Added quality_score field to service-layer StrategyCapabilities (already present in domain model). - Updated 5 feature scenarios and 1 Robot helper where GreedyKnapsackPacker relevance-based ordering changed fragment output order vs. the old no-op packer. Fixed by adjusting test fragment relevance scores to preserve expected ordering. ISSUES CLOSED: #924	2026-03-26 06:19:14 +00:00
brent.edwards	ee01e2b826	Merge pull request 'test: add TDD bug-capture test for #993 — server_connect non-atomic writes' (#1128) from tdd/m6-server-connect-non-atomic into master ... Reviewed-on: #1128 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>	2026-03-26 05:41:46 +00:00
brent.edwards	52511533b5	Merge remote-tracking branch 'origin/master' into merge-master-tmp	2026-03-26 02:58:55 +00:00
brent.edwards	3e704ff9c5	test: add TDD bug-capture test for #1076 — use_action automation_profile propagation (#1116) ... ## Summary Add Behave BDD scenarios that capture the bug described in #1076 where `PlanLifecycleService.use_action()` does not resolve or propagate the `automation_profile` from the Action (or any other source in the spec's precedence chain) to the created Plan. This is the TDD counterpart to bug #1076, following the project's [Bug Fix Workflow](CONTRIBUTING.md#bug-fix-workflow). The test proves the bug exists and will serve as a regression guard once the fix is merged. ### Changes - **`features/tdd_use_action_automation_profile.feature`** — Three Behave scenarios tagged `@tdd_expected_fail @tdd_bug @tdd_bug_1076`: 1. Action with `automation_profile="full-auto"` — Plan's `automation_profile` should be `AutomationProfileRef(profile_name="full-auto", provenance=ACTION)` but is `None`. 2. Action without `automation_profile` but with project-scoped config `"trusted"` — Plan's `automation_profile` should be `AutomationProfileRef(profile_name="trusted", provenance=PROJECT)` but is `None`. 3. Action without `automation_profile` — Plan's `automation_profile` should resolve to the global default `"supervised"` with `provenance=GLOBAL` but is `None`. - **`features/steps/tdd_use_action_automation_profile_steps.py`** — Step definitions exercising `PlanLifecycleService.use_action()` and asserting the expected behavior per the specification (docs/specification.md lines 18919, 18967). Shared `_use_action_on_project()` helper eliminates duplicate When step bodies. Guard assertion on `Action.automation_profile` after `create_action()` ensures the Action itself stores the profile correctly. - **`CHANGELOG.md`** — Entry added under `## Unreleased` describing the TDD test addition. ### How It Works All three scenarios fail at the assertion level (confirming the bug exists), but the `@tdd_expected_fail` tag inverts the result so the test suite passes CI. When the bug is fixed in #1076, the `@tdd_expected_fail` tag will be removed and the tests will run normally. ### Quality Gates | Gate | Result | |------|--------| | `nox -s lint` | PASS | | `nox -s typecheck` | PASS | | `nox -s unit_tests` | PASS (463 features, 12236 scenarios, 0 failures) | | `nox -s integration_tests` | Pre-existing Pabot infrastructure failure (identical on master) | | `nox -s e2e_tests` | PASS (37/37) | | `nox -s coverage_report` | PASS (98%, threshold 97%) | Closes #1098 Reviewed-on: #1116 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>	2026-03-26 02:56:45 +00:00
brent.edwards	8ccda647a8	Merge remote-tracking branch 'origin/master' into merge-master-tmp	2026-03-26 02:09:54 +00:00
brent.edwards	0e407f7f19	test: add TDD bug-capture test for #1022 — InvariantService persistence (#1109) ... ## Summary Add Behave BDD and Robot Framework integration tests that capture bug #1022 — `InvariantService` stores invariants in an in-memory dict only, losing them across CLI process invocations. ### Motivation Per the project's TDD Bug Fix Workflow (CONTRIBUTING.md), every bug must first have a test written that captures the buggy behavior before the fix is implemented. This PR fulfills the TDD counterpart issue #1032 for bug #1022. ### What was done **Behave tests** (`features/tdd_invariant_persistence.feature`): Four scenarios tagged `@tdd_expected_fail @tdd_bug @tdd_bug_1022 @mock_only` that simulate separate CLI invocations via fresh `InvariantService` instances and assert cross-instance data visibility: 1. Project invariant persistence across service instances 2. Global invariant persistence across service instances 3. CLI add/list across separate invocations 4. Cross-instance soft-delete by invariant ID Step definitions in `features/steps/tdd_invariant_persistence_steps.py`. **Robot integration tests** (`robot/tdd_invariant_persistence.robot`): Three integration test cases with a helper script (`robot/helper_tdd_invariant_persistence.py`) that exercises add-then-list and add-then-remove across fresh service instances. ### How it passes CI All tests carry `@tdd_expected_fail`, which inverts the test result via the environment hooks (Behave) and listener (Robot). The underlying assertions fail (proving the bug exists), but are reported as passed to CI. The `@tdd_expected_fail` tag will be removed when bug #1022 is fixed, at which point the tests will run normally and must pass. ### Review fixes applied - **M1**: Moved `InvariantScope` import from function body in `robot/helper_tdd_invariant_persistence.py` to module-level top imports. - **m1**: Added `@mock_only` tag to `features/tdd_invariant_persistence.feature` — these tests use purely in-memory services and never touch the database. - **m2**: Added `exit_code == 0` assertion to `step_invoke_list_cli` in `features/steps/tdd_invariant_persistence_steps.py`, matching the pattern used in `step_invoke_add_cli`. - **m3**: Changed all `context: Any` parameter types to `context: Context` (from `behave.runner`), following the project-wide convention used in 335+ step files. - **m4**: Narrowed `except Exception` to `except NotFoundError` in `robot/helper_tdd_invariant_persistence.py`, matching the specific error being tested. - **n3**: Branch rebased onto latest `master`. ### Quality Gates - **lint**: ✅ passed - **typecheck**: ✅ passed (Pyright, 0 errors) - **unit_tests**: ✅ 1 feature, 4 scenarios, 16 steps — all passed via `@tdd_expected_fail` - **coverage**: ≥ 97% (test-only changes, no source modifications) Closes #1032 Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Reviewed-on: #1109 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>	2026-03-26 02:06:28 +00:00
brent.edwards	822f999ebb	fix(test): increase container resolve regression timeouts to 120s	2026-03-26 01:44:00 +00:00
brent.edwards	498dca8137	Merge remote-tracking branch 'origin/master' into merge-master-tmp	2026-03-26 01:06:58 +00:00
brent.edwards	3d961fa0aa	test: add TDD bug-capture test for #988 — ReactiveEventBus.emit swallows exceptions (#1106) ... ## Summary Add a TDD bug-capture Behave scenario that proves bug #988 exists: `ReactiveEventBus.emit()` exception handler logs only `type(exc).__name__` (e.g., "ValueError") without the exception message (`str(exc)`) or traceback (`exc_info=True`). When a subscriber fails, the log contains zero diagnostic detail, making production debugging impossible. ## What was done - **Feature file**: `features/tdd_event_bus_exception_swallow.feature` — tagged `@tdd_expected_fail @tdd_bug @tdd_bug_988` - **Step definitions**: `features/steps/tdd_event_bus_exception_swallow_steps.py` — subscribes a handler that raises `ValueError("detailed error message for debugging")`, emits an event, captures the structlog warning via `structlog.testing.capture_logs()`, and asserts: 1. The exception message text appears in the log entry (scenario 1) 2. The `exc_info` key is present and truthy, confirming traceback logging (scenario 2) - **Changelog**: Updated `CHANGELOG.md` with the new entry ## How the test works 1. A `ReactiveEventBus` is created with a subscriber that raises `ValueError` with a distinctive message 2. An event is emitted, triggering the failing handler 3. The `emit()` exception handler catches the error and logs a warning via structlog 4. **Scenario 1** asserts the exception **message** (not just the type name) appears in the log entry 5. **Scenario 2** asserts the log entry includes **`exc_info`** (traceback), per bug #988's acceptance criteria requiring `exc_info=True` 6. Both assertions **FAIL** because the current code only logs `type(exc).__name__` — confirming the bug 7. The `@tdd_expected_fail` tag inverts these failures to CI passes ## Test verification - `nox -s unit_tests` ✅ passes (462 features, 12,232 scenarios passed, 0 failed) - Both underlying assertions correctly fail, proving the bug exists - Tag validation rules pass: `@tdd_bug_988` has corresponding `@tdd_bug`, and `@tdd_expected_fail` has both - `nox -s lint` ✅ passes - `nox -s typecheck` ✅ passes (0 errors on changed files) ## Review fixes applied - **C1 (Critical)**: Rebased onto latest `master` (`5f5ef891`) to eliminate unrelated `docs/timeline.md` regression that was overwriting Day 42 data with stale Day 39 content - **m1**: Added docstrings to all four step functions - **m2**: Renamed parameter `ctx` → `context` across all step functions to match project convention (97%+ of codebase uses `context`) - **m3**: Added second scenario "Bug #988 — emit() logs traceback via exc_info when handler raises" with new `step_then_log_contains_traceback` step that verifies the `exc_info=True` requirement from bug #988 acceptance criteria - **n1 (Informational)**: Feature-level tags are valid Gherkin; no change needed - **n2 (Informational)**: `# type: ignore[import-untyped]` on behave imports is the established project convention (106+ files); no change needed ## Robot test N/A — this is a purely unit-level concern (testing a single class's internal error handling, no external services or IPC involved). Closes #1093 Reviewed-on: #1106 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com> Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>	2026-03-26 01:02:10 +00:00
brent.edwards	2802af13d0	Merge branch 'master' into tdd/m6-server-connect-non-atomic	2026-03-26 00:13:28 +00:00
brent.edwards	2abbfff443	Merge pull request 'test: add TDD bug-capture test for #986 — CorrectionService checkpoint_service wiring' (#1108) from tdd/m4-correction-checkpoint-wiring into master ... Reviewed-on: #1108 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>	2026-03-26 00:11:10 +00:00
brent.edwards	3dee9441a1	Merge remote-tracking branch 'origin/master' into tdd/m6-server-connect-non-atomic	2026-03-25 20:45:08 +00:00
brent.edwards	3ec5114e01	Merge remote-tracking branch 'origin/master' into tdd/m4-correction-checkpoint-wiring	2026-03-25 20:44:56 +00:00
Luis Mendes	34c2acc354	fix(acms): implement context tier runtime promotion/demotion/eviction ... Implement the missing runtime logic for the ACMS context tier service. Previously only data models and manual promote()/demote()/evict_lru() methods existed. This commit adds: - Auto-promotion on access: get() now promotes fragments one tier up when access_count reaches the configurable promotion_threshold (default: 5 accesses). The access counter resets after each successful promotion so fragments must accumulate fresh accesses before the next tier transition. - Staleness enforcement: new enforce_staleness() method demotes hot fragments older than hot_ttl (default: 24h) to warm, and warm fragments older than warm_ttl to cold. A snapshot of existing warm-tier IDs prevents double-demotion in a single pass. - Budget enforcement on store and promote: store() and promote() now enforce TierBudget.max_tokens_hot by evicting LRU hot-tier fragments until the token budget is met. The eviction loop uses incremental token tracking to avoid recomputing the sum. - Event emission: Added TIER_PROMOTED, TIER_DEMOTED, TIER_EVICTED event types to EventType enum. All tier transitions emit DomainEvent instances through the optional EventBus. - Configuration: Added context_tier_promotion_threshold, context_tier_hot_ttl_hours, context_tier_warm_ttl_hours settings. The warm TTL setting also accepts the spec-defined env var CLEVERAGENTS_CTX_WARM_HOURS as an alias. - DI wiring: container.py now injects event_bus into context_tier_service. Review fixes applied (code review on PR #1150): - C1: Reset access_count to 0 after each auto-promotion to prevent chain promotion that bypassed the warm tier. - C2: Call _enforce_hot_budget() inside promote() warm-to-hot path so auto-promoted fragments respect the token budget. - H1: Corrected _enforce_hot_budget() docstring: actual complexity is O(n + n*k) not O(n), since min() scans remaining entries on each eviction. - M1: Added CLEVERAGENTS_CTX_WARM_HOURS as an additional env var alias for context_tier_warm_ttl_hours per specification line 30555. Review fixes applied (second code review on PR #1150): - B-CRIT-1: Fixed data loss in promote() warm-to-hot: emit TIER_PROMOTED before _enforce_hot_budget(), and if the promoted fragment is evicted by budget, restore it to the warm tier instead of silently losing it. - B-HIGH-1: Fixed self-eviction on store(): fragments whose token_count exceeds the entire hot-tier budget are now redirected to the warm tier with a warning log. - B-MED-1: Wrapped _emit_tier_event() in try/except so a failing event bus does not break tier operations (best-effort emission). - B-MED-2: Fixed event ordering so TIER_PROMOTED fires before any budget-triggered TIER_EVICTED events. - D-LOW-1: Fixed type hint in Robot helper (dict[str, Callable]). - D-LOW-2: Added __all__ export to context_tiers.py. - S-LOW-1: Added thread-safety docstring note to ContextTierService. Review fixes applied (third code review on PR #1150): - B-MED-1: Added TIER_DEMOTED event emission for oversized fragment redirect in store(), closing the observability gap where the only tier transition without event emission was the hot-to-warm redirect for fragments exceeding the entire hot-tier budget. - S-LOW-1: Added CLEVERAGENTS_CTX_HOT_HOURS as an additional env var alias for context_tier_hot_ttl_hours, for consistency with the warm-tier alias CLEVERAGENTS_CTX_WARM_HOURS. - S-LOW-2: Added docstring note to enforce_staleness() reconciling the hot-tier TTL with the specification statement that hot-tier retention is "Until resource removed" (TTL controls tier placement, not data retention). Review fixes applied (fourth code review on PR #1150): - B-HIGH-1: Reset access_count to 0 on demotion so that demoted fragments must accumulate fresh accesses before re-promotion. Without this reset, a previously popular fragment whose access_count already exceeded the promotion threshold would be re-promoted on the very next get() call, making staleness enforcement ineffective. Review fixes applied (freemo APPROVED review on PR #1150): - #1: Removed all # type: ignore annotations from test files. Fixed _EventCollector, _FailingBus, and _NullBus subscribe() signatures to use Callable[[DomainEvent], None] matching the EventBus protocol. Replaced dict-spread TieredFragment construction with explicit keyword arguments and post-construction assignment. - #2: Extracted runtime policy logic (enforce_staleness, _maybe_auto_promote, _re_fetch_after_promotion, _enforce_hot_budget, _emit_tier_event) into TierRuntimeMixin in tier_runtime.py to reduce context_tiers.py toward the 500-line guideline. - #3: Added fragment_id non-empty validation guard to promote() and demote() per CONTRIBUTING.md argument validation policy. - #8: Renamed _resolve to _re_fetch_after_promotion for clarity. Removed @tdd_expected_fail from TDD tests (Behave + Robot) as the bug is now fixed. All 3 TDD scenarios pass normally. Tests: 27 Behave scenarios (24 feature + 3 TDD), 4 Robot integration tests, 4 ASV benchmark suites. ISSUES CLOSED: #821	2026-03-25 17:49:06 +00:00
freemo	06130212ed	test(e2e): workflow example 5 — database schema migration with safety nets (review profile) (#816) ... ## Summary E2E test for Workflow Example 5 — database schema migration with safety nets using the **review** automation profile. Exercises the full spec-aligned workflow: - **Custom resource type registration** via `resource type add --config` (postgres-db type with `transaction_rollback` sandbox strategy, `--host`, `--port`, `--database`, `--schema` CLI args with flat `type`/`default` fields per `ResourceTypeArgument` schema) - **Custom resource instantiation** — attempts `resource add` with the custom type to exercise mixed resource types, followed by `project link-resource` to link DB resource to the project - **Custom skill creation** with spec-aligned database tools: `local/query_db` (read-only), `local/execute_migration` (writes, checkpointable), `local/backfill_column` (writes, checkpointable) — registered via `skill add --config`, with namespaced tool reference names per `SkillToolRefSchema` validation - **Action creation** with `automation_profile: review`, `reusable: true`, `state: available`, spec invariants, and typed `arguments` section (`table_name`, `column_name`, `column_type`, `backfill_source` — all required per spec, using `arguments` field per `ActionConfigSchema`) - **Plan use** with `--arg` flags exercising parameterized action invocation including `backfill_source=audit_log`, plus **explicit `--automation-profile review`** flag (action-to-plan profile propagation is not yet wired in `PlanLifecycleService.use_action`) - **Phased child plan verification** via `plan tree --format json` with `decision_count >= 2` hard assertion on framework decisions plus WARN tiers for LLM decomposition quality (`< 3`, `< 5`) - **Plan phase assertion** — hard assertion that phase is populated after execute - **Checkpoint-based rollback** with hard assertions: `rc=0` on rollback success, `rc!=0` on fake checkpoint, None guard for JSON null checkpoint IDs, re-execute with Traceback/INTERNAL checks on success and explanatory comment on failure path - **Plan diff** with hard `rc=0` assertion and content-signal verification - **Migration content verification** — baseline SHA saved before apply, diff against baseline (not `HEAD~1`), WARN-level check on migration keywords (`last_login`, `schema`, `migration`, `column`, `alter`) — flexible per LLM non-determinism - **Commit count** assertion `>= 2` (fixture baseline: Create Temp Git Repo + DB fixture commit), WARN if no additional commits from lifecycle-apply - **Backfill evidence** WARN-level check in plan tree/execution output (`backfill`, `batch`, `populate`, `last_login`) with explanatory comment noting tree covers decomposition plan - **Combined AC #6 gate** — if *both* migration content *and* backfill evidence are absent, explicit WARN visibility for CI debugging - **Terminal state assertion** after `lifecycle-apply` — `plan status` call verifies phase/processing_state reflects terminal or apply-progress outcome - **Automation profile fallback verification** — if `plan use` output omits `automation_profile`, falls back to `plan status` for secondary verification (hard assertion always runs) - **Traceback and INTERNAL checks** on all CLI commands (resource add, project create, resource type add, skill add, action create, plan use, strategize, execute, plan tree, plan status, plan diff, plan rollback, re-execute after rollback, lifecycle-apply) including custom resource error paths - **Dynamic actor selection** — detects available API keys (Anthropic/OpenAI) at suite setup - **Skip If No LLM Keys** guard for graceful CI degradation - **Test-level teardown** with diagnostic logging for both plan status and plan tree on failure - **30-minute timeout** covering worst-case rollback+re-execute path - **Force Tags** for consistency with `m6_acceptance.robot` - **Timeout parameters** (`timeout=60s on_timeout=kill`) on all local `Run Process` git commands - **Sequential section numbering** (1 through 15) for readability Closes #751 ISSUES CLOSED: #751 ## Approach Follows the patterns established by `m6_acceptance.robot` and `m2_acceptance.robot`: - `WF05 Suite Setup` initialises the workspace, generates a unique run suffix, and detects available LLM API keys - `Safe Parse Json Field` from `common_e2e.resource` for JSON field extraction with None guards for JSON null values - All CLI commands use `--format json` for predictable, parseable output - `expected_rc=None` with explicit `Should Be Equal As Integers` for detailed failure messages - Hard assertions on infrastructure/framework behavior (CLI commands, phase transitions, tool registration) - WARN-level assertions on LLM-dependent output (decision decomposition, migration content, backfill evidence, commit count) — per ticket requirement "output validation is flexible" - Traceback and INTERNAL checks on all CLI commands following `m2_acceptance.robot` pattern - Baseline SHA approach for post-apply diff verification eliminates false positives from fixture commits ## Bug Fix: LifecyclePlanRepository.update() UNIQUE Constraint Violation **Root cause**: `LifecyclePlanRepository.update()` called `clear()` on child relationship collections (project_links, arguments, invariants) followed by `append()` with new items, but only flushed at the end. SQLAlchemy's default operation ordering can emit INSERTs before DELETEs within the same flush, causing `UNIQUE constraint failed: plan_arguments.plan_id, plan_arguments.name` when plans have arguments. **Fix**: Group all three `clear()` calls together and flush them before appending new rows. This ensures the DELETEs are committed before any INSERTs, preventing the UNIQUE constraint violation. **Impact**: This was a latent bug affecting ALL plans with arguments when `update()` is called. Previously undetected because existing E2E tests (M1, M2, M5, M6) create plans without `--arg` flags. ## Review Fixes (addressing medium findings from @CoreRasurae review) | # | Finding | Fix | |---|---------|-----| | **BUG-1** | No regression test for UNIQUE constraint fix | Added targeted BDD scenario in `repositories_coverage_boost.feature` — creates plan with argument `x=v1`, updates to `x=v2`, asserts no `IntegrityError` | | **TEST-1** | AC #4 weakened — fragile string counting | Replaced raw `count('"decision_id"')` with proper JSON parsing via `json.loads()`, recursive tree walking for decision counting, structural `children_key_count` and `child_link_count` verification | | **TEST-2** | AC #5 conditionally tested | Added explicit WARN log when no checkpoint_id is present ("AC #5 visibility"); fake checkpoint test now runs unconditionally (moved outside IF/ELSE) with Traceback/INTERNAL checks | | **TEST-3** | No terminal state assertion after lifecycle-apply | Added `plan status` call after apply with phase/processing_state extraction; hard assertion on terminal state or apply-phase progress | | **TEST-4** | AC #6 migration/backfill WARN-only | Added combined gate (`has_ac6_evidence`): if *both* migration and backfill evidence are absent, explicit WARN for CI visibility. WARN-only is intentional per ticket AC "output validation is flexible" | | **TEST-5** | Automation profile silently skipped | Added fallback to `plan status --format json` when `plan use` output omits `automation_profile`; hard assertion (`Should Be Equal As Strings review`) now always executes | | **TEST-8** | Missing Traceback/INTERNAL on custom resource error paths | Added Traceback/INTERNAL checks inside both `resource add` and `project link-resource` ELSE branches with `NoSuchOption` guard | ## Quality Gates - `nox -e lint` ✅ - `nox -e typecheck` ✅ (0 errors) - `nox -e unit_tests` ✅ (471 features, 12,422 scenarios, 0 failures) - `nox -e integration_tests` ✅ (1,727 tests, 0 failures) - `nox -e e2e_tests` ✅ (42 tests, 42 passed, 0 failed) - `nox -e coverage_report` ✅ (98%, meets threshold) ## Manual Verification ### Prerequisites - `OPENAI_API_KEY` or `ANTHROPIC_API_KEY` environment variable set ### Commands ```bash nox -e e2e_tests # Or run just this suite: python -m robot --outputdir build/reports/robot --include E2E robot/e2e/wf05_db_migration.robot ``` Reviewed-on: #816 Reviewed-by: Luis Mendes <luis.mendes@cleverthis.com> Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>	2026-03-25 13:05:04 +00:00
aditya	37c1329257	Merge pull request 'feat(acms): context assembly CLI functional (context list/add/show/clear)' (#1138) from feature/m5-context-cli into master ... Reviewed-on: #1138 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>	2026-03-25 12:39:49 +00:00
aditya	2aa2bb6a43	fix(e2e): disable git commit signing in temp test repos	2026-03-25 12:05:40 +00:00
aditya	37d55f4321	feat(acms): context assembly CLI functional (context list/add/show/clear) ... Verified the context assembly CLI commands (context list, add, show, clear) are fully functional and integrated with ContextService and the DI container. Both Robot E2E acceptance tests (ACMS Scoped Context Output Per Phase and Context Policy Clear And Inheritance Fallback) pass, confirming phase-scoped view resolution, size limit narrowing, inheritance fallback on clear, and persistence round-trip correctness. Added 5 new Behave scenarios to m5_acms_smoke.feature covering the required unit test patterns: multiple resources management (list, add, show with multiple files), clear-then-re-add flow, and clear on empty project. All 12,235 unit test scenarios pass. Coverage is 98.38% (threshold >97%). ISSUES CLOSED: #848	2026-03-25 12:05:40 +00:00
aditya	af5e331b06	Merge pull request 'fix(cli): bypass migration prompt entirely when --yes flag is passed' (#1139) from bugfix/m3-init-yes-no-input into master ... Reviewed-on: #1139 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>	2026-03-25 12:05:02 +00:00
aditya	c082c8f022	fix(cli): bypass migration prompt entirely when --yes flag is passed ... Make require_confirmation configurable on UnitOfWork so that when the --yes flag is passed to agents init, the migration runner skips the confirmation prompt entirely rather than calling a prompt that always returns True. The previous approach (injecting a prompt callback that auto-approves) was a workaround that left the require_confirmation=True hardcoded in _ensure_database_initialized, meaning the prompt code path was still exercised unnecessarily. Changes: - Add require_confirmation parameter to UnitOfWork constructor (default True for backward compatibility) - Pass self._require_confirmation to MigrationRunner.init_or_upgrade instead of hardcoded True - Update init_command to pass require_confirmation=False when --yes is set, with the prompt callback retained as belt-and-suspenders The TDD bug-capture tests from #842 (features/tdd_init_yes_no_input.feature, robot/tdd_init_yes_no_input.robot) now run as normal regression tests with @tdd_expected_fail already removed. All nox quality gates pass: - lint, typecheck: clean - unit_tests: 12230 scenarios passed - integration_tests: all passed - coverage_report: 98.38% (threshold >97%) ISSUES CLOSED: #783	2026-03-25 11:28:06 +00:00
hurui200320	83c22b8391	test(e2e): TDD behavioral test proving ACMS indexing pipeline is not wired into CLI (bug #1028) (#1124) ... ## Summary This PR adds a Robot Framework E2E test suite (`robot/e2e/tdd_acms_behavioral_validation.robot`) that proves bug #1028 exists — the ACMS indexing pipeline is not wired into the CLI, so `ContextTierService` starts empty on every invocation. ### Changes - **New file**: `robot/e2e/tdd_acms_behavioral_validation.robot` — 4 E2E test cases tagged `tdd_expected_fail`, `tdd_bug`, `tdd_bug_1028`, `E2E` - **Modified**: `robot/e2e/common_e2e.resource` — Extracted shared keywords (`Run CLI`, `Extract JSON From Stdout`, `Link Resource To Project`, `Create Synthetic Codebase`) from both `m5_acceptance.robot` and `tdd_acms_behavioral_validation.robot` to eliminate ~97 lines of duplication. `Create Synthetic Codebase` is parameterized with `project_label`. - **Modified**: `robot/e2e/m5_acceptance.robot` — Removed duplicated keywords now provided by `common_e2e.resource`. - **CHANGELOG.md**: Added entry under `## Unreleased` documenting the TDD tests for #1029. ### Test Cases - **Test 1**: Context Simulate Returns Non-Empty Tier Data — asserts `fragment_count > 0` (fails, proving bug) - **Test 2**: Context Inspect Shows Indexed Resources — asserts tier metrics total > 0 (fails, proving bug) - **Test 3**: Budget Enforcement Excludes Oversized Files — asserts `fragment_count > 0` with `max_file_size` policy (fails, proving bug). Includes TODO comment for post-fix exclusion assertion. - **Test 4**: Large Project Indexes Without Timeout — generates 10K+ files, asserts `fragment_count > 0` (fails, proving bug). Includes explicit developer responsibility note about git-tracking of generated files. All tests pass CI through result inversion by the `tdd_expected_fail_listener.py` — failing assertions (bug confirmed) are inverted to PASS. ### Documentation & Robustness Improvements - Suite-level documentation expanded with **known limitation** section explaining `tdd_expected_fail` result inversion scope. - Suite setup error messages include `(rc=${var.rc}). Check DEBUG logs above.` for debugging consistency with `m5_acceptance.robot`. - Redundant exit code assertions after `Run CLI` calls removed (Run CLI already validates rc internally). - `Run CLI` keyword documentation includes API key security notes. - Budget enforcement test (Test 3) includes `TODO(bugfix/...)` comment for the bug-fix developer. - Large project test (Test 4) includes explicit NOTE assigning responsibility to the bug-fix developer to evaluate filesystem vs. git-tracked content indexing. ### Review Fix Round Addressed all findings from Luis's review (review #2691): - **C1 (CRITICAL)**: Restored the #845 `CorrectionService` changelog entry (50 lines) accidentally deleted during merge conflict resolution. - **L1 (LOW)**: Extracted ~97 lines of duplicated keywords into `common_e2e.resource` (parameterized `Create Synthetic Codebase`, shared `Run CLI`, `Extract JSON From Stdout`, `Link Resource To Project`). - **M1 (MEDIUM)**: Strengthened NOTE comment about 10K files not being git-committed — explicit developer MUST responsibility. - **L3 (LOW)**: Made suite setup error messages verbose with `(rc=...). Check DEBUG logs above.` - **L2 (LOW)**: Removed redundant exit code assertions after `Run CLI` calls. - **M2 (MEDIUM)**: Acknowledged — partial assertion is acceptable for TDD capture phase (TODO documents the gap). - **I1 (INFO)**: Acknowledged — `tdd_expected_fail` masking is documented and mitigated. ### Motivation Per the Bug Fix Workflow in CONTRIBUTING.md, this TDD issue (#1029) is the prerequisite for bug fix #1028. The tests capture the buggy behavior so that when the fix is implemented, removing the `tdd_expected_fail` tag will cause the tests to pass normally. ### Quality Gates | Gate | Result | |------|--------| | lint | PASS | | typecheck | PASS | | unit_tests | PASS | | integration_tests | PASS | | e2e_tests | PASS (41 tests, 4 TDD) | | coverage_report | PASS (>=97%) | Closes #1029 Reviewed-on: #1124 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>	2026-03-25 11:13:37 +00:00
Luis Mendes	3cf3f1f69e	feat(validation): implement Fix-then-Revalidate orchestration loop for required validations ... Implemented FixThenRevalidateOrchestrator with full diagnosis → self-fix → re-validation → retry limit → strategy revision → user escalation → terminal failure flow. Added retry counting per validation per plan, configurable retry limit (default 3), auto_strategy_revision flag support, and validation_fix_history recording in plan execution metadata. Review fixes applied (round 1): - Added try/except around fix_callback and revalidate_callback (spec: validation errors treated as required failure regardless of mode) - Wired optional EventBus for VALIDATION_FIX_ATTEMPTED/SUCCEEDED/EXHAUSTED events - Fixed total_attempts derivation from fix_history length instead of cumulative retry counts - Updated module docstring to reflect implemented vs caller-responsible steps - Added exhausted-retry logging on re-invocation - Replaced bare assert in Robot helper with explicit _check() function - Added 10 new Behave scenarios covering exception paths, multi-failure, reset, field validation, boundary values, event_bus property, and exhausted re-invocation Review fixes applied (round 2 — PR #711): - B1+B7: Fixed TOCTOU race in _fix_single_validation; atomic claim-per-iteration under RLock; eliminated defaultdict auto-vivification with .get() reads - B2: Added validation_name mismatch check on revalidate_callback return - B3: Added fix_description truncation to 2000 chars before FixAttemptRecord - T1: Switched threading.Lock to threading.RLock for defensive reentrancy - A1: Added model_validator preventing escalated+terminal_failure both True - R1: Added max_length=255 to FixAttemptRecord.validation_name - S7: Fixed misleading docstring about automation_profile integration - D4: Fixed timestamp field description to specify UTC - D1: Renamed misleading benchmark time_fix_after_two_retries - Added 5 new Behave scenarios for truncation, model constraints, name mismatch ISSUES CLOSED: #583	2026-03-24 22:01:48 +00:00
freemo	e52c79e958	test: add TDD bug-capture test for #1024 — SQLite DB URL CWD resolution ... Add Behave BDD scenarios (tagged @tdd_bug @tdd_bug_1024 @tdd_expected_fail) that verify the default database_url resolves inside CLEVERAGENTS_HOME rather than the current working directory. Two scenarios exercise both the container get_database_url() helper and the Settings model default. Add Robot Framework integration tests with a helper script exercising the same resolution paths via subprocess, verifying database URL resolution and CLI database file placement relative to CLEVERAGENTS_HOME. The tests currently fail as expected because the bug in #1024 is still present: the relative SQLite path sqlite:///cleveragents.db resolves against CWD. The @tdd_expected_fail tag inverts the result so CI passes. ISSUES CLOSED: #1034	2026-03-24 20:29:49 +00:00
freemo	2e094ef938	feat(acms): add text, vector, and graph backend protocol implementations ... Define TextBackend, VectorBackend, and GraphBackend protocols with corresponding result dataclasses. Implement in-memory stub backends for pipeline integration testing. Register backends in DI container with configurable provider selection. ISSUES CLOSED: #498	2026-03-24 18:12:16 +00:00
freemo	93da31e80f	fix(skill): resolve skill add persistence regression after PR #640 ... Root cause: _build_skill_service in container.py created a SkillRepository pointing at the database but did not ensure the skills/skill_items tables existed. When the tables were missing, SkillRepository.list_all() and create() failed silently (caught by SkillService._load_from_db and _persist_skill exception handlers), causing the service to operate in in-memory-only mode. Skills added in one CLI process were lost when a new process created a fresh SkillService. Additionally, SkillRepository lacked auto_commit support. Each call to session_factory() returned a new session, so the flush in create/update/ delete operated on a different session than the commit in SkillService._commit(), meaning data was never actually persisted even when the tables existed. Fix: 1. Add targeted table creation in _build_skill_service (following the pattern in _build_session_service) — checks for missing skills and skill_items tables and creates them via Base.metadata.create_all. 2. Add auto_commit parameter to SkillRepository (following the pattern in SessionRepository) so each mutating method commits and closes its own session. 3. Pass auto_commit=True from the container builder. 4. Remove @tdd_expected_fail from TDD test (leaving @tdd_bug and @tdd_bug_980 as permanent regression guards). ISSUES CLOSED: #980	2026-03-24 17:08:22 +00:00
freemo	a6e2bc7842	test: add TDD bug-capture test for #980 — skill add cross-process persistence ... Write cross-process Behave and Robot tests capturing skill add persistence regression. Tests use subprocess invocations to verify skills persist across CLI process boundaries. The existing persistence tests (skill_add_persist.feature) verify round-trip within the same Python process by creating two SkillService instances sharing the same in-memory database. This approach cannot detect the cross-process regression where _build_skill_service falls back to in-memory storage because the skills table does not exist in the database created by agents init. Behave: features/tdd_skill_add_regression.feature Robot: robot/tdd_skill_add_regression.robot Tags: @tdd_bug, @tdd_bug_980, @tdd_expected_fail ISSUES CLOSED: #981	2026-03-24 17:08:22 +00:00
hamza.khyari	4a5f826f77	Merge pull request 'feat(resource): implement ResourceHandler CRUD and discovery methods' (#1054) from feature/resource-handler-crud into master ... Reviewed-on: #1054	2026-03-24 14:40:34 +00:00
hamza.khyari	90e5bbb99c	feat(resource): implement ResourceHandler CRUD and discovery methods ... Extend the ResourceHandler protocol with six content operations (read, write, delete, list_children, diff, discover_children) and four frozen dataclass result types (Content, WriteResult, DeleteResult, DiffResult). Handler implementations: - GitCheckoutHandler: read via git show (binary-safe), write/delete via filesystem ops, list via git ls-tree, diff via git diff --no-index with locale-safe shortstat parsing, discover via git ls-tree -d - FsDirectoryHandler: full CRUD via pathlib/os/difflib/shutil - DevcontainerHandler: read/write/discover via devcontainer exec - CloudResourceHandler: NotImplementedError stubs for protocol compliance - DatabaseResourceHandler: inherits base NotImplementedError stubs Security: - Path traversal guard (_safe_resolve) on all read/write/delete ops using os.sep-suffixed startswith check to prevent prefix collisions - Empty-path deletion rejected with PermissionError Tests: - 22 Behave scenarios (115 steps): CRUD for FsDirectory and GitCheckout, path traversal rejection (3 scenarios), NotImplementedError defaults - 2 Robot integration tests: read -> write -> diff cycle on real temp directories and git repos ISSUES CLOSED: #827	2026-03-24 13:53:41 +00:00
hamza.khyari	188a9edd49	Merge pull request 'feat(resource): add LSP resource types' (#1046) from feature/lsp-resource-types into master ... Reviewed-on: #1046	2026-03-24 13:36:30 +00:00
hamza.khyari	65a2e4db76	feat(resource): add LSP resource types ... Add 4 LSP-related built-in resource types to the resource registry: - executable: system binary/interpreter/LSP server binary with auto-discovery from container-exec-env and fs-directory (lazy) - lsp-server: LSP server definition with command, language-ids, transport, args, port, initialization-options config; children: lsp-workspace - lsp-workspace: workspace root tracked by LSP server, auto-discovered from lsp-server; children: lsp-document; not user-addable - lsp-document: text document tracked by LSP server, auto-discovered from lsp-workspace; read+write capabilities; not user-addable Type definitions extracted to _resource_registry_lsp.py for consistency with existing type modules. Parent/child hierarchy: lsp-server -> lsp-workspace -> lsp-document. YAML configs with ADR references (ADR-039, ADR-040). All 7 lsp-server CLI args per ADR-040. Behave tests (21 scenarios): YAML loading, user-addable flags, capabilities, parent/child hierarchy, auto-discovery for all 3 discoverable types, BUILTIN_NAMES, DB bootstrap roundtrip, negative tests for manual registration rejection. Robot tests (6 tests): import, BUILTIN_NAMES, DB roundtrip, hierarchy, auto-discovery, user-addable guard. ISSUES CLOSED: #832	2026-03-24 13:21:12 +00:00
hamza.khyari	e07b68221a	Merge pull request 'feat(lsp): implement functional LSP runtime' (#1044) from feature/lsp-functional-runtime into master ... Reviewed-on: #1044	2026-03-24 13:17:44 +00:00