diff --git a/features/correction_service_new_coverage.feature b/features/correction_service_new_coverage.feature new file mode 100644 index 000000000..36c5ad16e --- /dev/null +++ b/features/correction_service_new_coverage.feature @@ -0,0 +1,244 @@ +@phase2 @correction @coverage +Feature: CorrectionService comprehensive coverage + As a developer + I want thorough unit tests for the CorrectionService + So that every method and branch has coverage + + # ── request_correction ─────────────────────────────────────── + + Scenario: Create a revert correction with valid parameters + Given a fresh correction service instance + When I create a correction for plan "PX1" targeting "DX1" in revert mode + Then the new correction should be stored in the service + And the new correction plan_id should equal "PX1" + And the new correction target_decision_id should equal "DX1" + And the new correction mode should equal "revert" + And the new correction status should equal "pending" + And the new correction dry_run should be false + + Scenario: Create correction with empty plan_id raises ValidationError + Given a fresh correction service instance + When I attempt to create a correction with an empty plan_id + Then a service ValidationError should have been raised + + Scenario: Create correction with empty target_decision_id raises ValidationError + Given a fresh correction service instance + When I attempt to create a correction with an empty target_decision_id + Then a service ValidationError should have been raised + + Scenario: Create correction with guidance and dry_run flag + Given a fresh correction service instance + When I create a correction for plan "PX1" targeting "DX1" in append mode with guidance "Refactor auth" and dry_run true + Then the new correction guidance should equal "Refactor auth" + And the new correction dry_run should be true + And the new correction mode should equal "append" + + # ── analyze_impact ─────────────────────────────────────────── + + Scenario: Analyze impact with no children yields single node and low risk + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + When I analyze impact with an empty decision tree + Then the impact affected decisions should be exactly "DX1" + And the impact risk level should equal "low" + And the impact rollback tier should equal "full" + And the impact estimated cost should equal 1.5 + And the impact artifacts to archive should be exactly "DX1.artifact" + And the impact affected files should be exactly "DX1.py" + + Scenario: Analyze impact with deep BFS tree traversal + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "A" + When I analyze impact with tree "A->B,C;B->D,E;C->F" + Then the impact affected decisions should be exactly "A,B,C,D,E,F" + And the impact risk level should equal "medium" + + Scenario: Analyze impact with medium risk subtree of 4 nodes + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "R" + When I analyze impact with tree "R->C1,C2,C3,C4" + Then the impact risk level should equal "medium" + And the impact affected decisions count should be 5 + + Scenario: Analyze impact with high risk subtree of 11 or more nodes + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "R" + When I analyze impact with a chain tree of 12 nodes rooted at "R" + Then the impact risk level should equal "high" + And the impact affected decisions count should be 12 + + Scenario: Analyze impact with non-existent correction raises ResourceNotFoundError + Given a fresh correction service instance + When I attempt to analyze impact for correction "bogus-id-999" + Then a service ResourceNotFoundError should have been raised + + Scenario: Analyze impact sets status to analyzing + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + When I analyze impact with an empty decision tree + Then the stored correction status should be "analyzing" + + Scenario: Analyze impact for append mode yields append_only rollback tier + Given a fresh correction service instance + And a stored append correction for plan "PX1" targeting "DX1" + When I analyze impact with an empty decision tree + Then the impact rollback tier should equal "append_only" + + # ── generate_dry_run_report ────────────────────────────────── + + Scenario: Dry-run report for revert with high risk includes high-risk warning + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "R" + When I generate a dry-run report with a chain tree of 15 nodes rooted at "R" + Then the dry-run report mode should equal "revert" + And the dry-run report warnings should include "High risk" + And the dry-run report warnings should include "Revert will invalidate" + And the dry-run report decisions to invalidate should not be empty + And the dry-run report estimated recompute time should be 30.0 + + Scenario: Dry-run report for revert with medium risk includes medium-risk warning + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "R" + When I generate a dry-run report with a chain tree of 7 nodes rooted at "R" + Then the dry-run report warnings should include "Medium risk" + And the dry-run report warnings should include "Revert will invalidate" + + Scenario: Dry-run report for append has empty decisions to invalidate + Given a fresh correction service instance + And a stored append correction for plan "PX1" targeting "DX1" + When I generate a dry-run report with an empty decision tree + Then the dry-run report decisions to invalidate should be empty + And the dry-run report mode should equal "append" + + # ── execute_revert ─────────────────────────────────────────── + + Scenario: Execute revert with simple tree returns applied result + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + When I execute revert with tree "DX1->DX2" + Then the revert result status should equal "applied" + And the revert result reverted decisions should include "DX1" + And the revert result reverted decisions should include "DX2" + And the revert result archived artifacts should include "DX1.artifact" + And the revert result archived artifacts should include "DX2.artifact" + + Scenario: Execute revert records a successful attempt with completed_at + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + When I execute revert with an empty decision tree + Then the service attempts for the correction should have 1 entry + And the first attempt should have success true + And the first attempt completed_at should not be none + + # ── execute_append ─────────────────────────────────────────── + + Scenario: Execute append creates a child plan and new decision + Given a fresh correction service instance + And a stored append correction for plan "PX1" targeting "DX1" + When I execute append for the stored correction + Then the append result status should equal "applied" + And the append result spawned_child_plan_id should not be none + And the append result new decisions should not be empty + + # ── execute_correction dispatch ────────────────────────────── + + Scenario: Execute correction dispatches to revert for REVERT mode + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + When I execute correction via the dispatch method with tree "DX1->DX2" + Then the dispatched result status should equal "applied" + And the dispatched result reverted decisions should include "DX1" + + Scenario: Execute correction dispatches to append for APPEND mode + Given a fresh correction service instance + And a stored append correction for plan "PX1" targeting "DX1" + When I execute correction via the dispatch method with no tree + Then the dispatched result status should equal "applied" + And the dispatched result spawned_child_plan_id should not be none + + # ── get_correction ─────────────────────────────────────────── + + Scenario: Get correction with valid id returns the request + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + When I get the stored correction by its id + Then the retrieved correction plan_id should equal "PX1" + And the retrieved correction target_decision_id should equal "DX1" + + Scenario: Get correction with invalid id raises ResourceNotFoundError + Given a fresh correction service instance + When I attempt to get correction with id "nonexistent-xyz" + Then a service ResourceNotFoundError should have been raised + + # ── list_corrections ───────────────────────────────────────── + + Scenario: List corrections without filter returns all corrections + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + And a stored append correction for plan "PX2" targeting "DX2" + When I list all service corrections without filter + Then the service corrections list should have 2 items + + Scenario: List corrections with plan_id filter returns only matching + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + And a stored append correction for plan "PX1" targeting "DX2" + And a stored revert correction for plan "PX2" targeting "DX3" + When I list service corrections for plan "PX1" + Then the service corrections list should have 2 items + When I list service corrections for plan "PX2" + Then the service corrections list should have 1 items + + # ── list_attempts ──────────────────────────────────────────── + + Scenario: List attempts for a valid correction returns attempts + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + And the stored correction has been reverted with an empty tree + When I list attempts for the stored correction + Then the attempts list should have 1 entry + + # ── cancel_correction ──────────────────────────────────────── + + Scenario: Cancel correction in pending status succeeds + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + When I cancel the stored correction + Then the stored correction status should be "cancelled" + + Scenario: Cancel correction in applied status raises ValidationError + Given a fresh correction service instance + And a stored revert correction for plan "PX1" targeting "DX1" + And the stored correction has been reverted with an empty tree + When I attempt to cancel the stored correction + Then a service ValidationError should have been raised + + # ── _classify_risk boundary tests ──────────────────────────── + + Scenario: Classify risk at exact boundary values + Given a fresh correction service instance + Then classifying risk for 0 affected nodes should return "low" + And classifying risk for 1 affected nodes should return "low" + And classifying risk for 3 affected nodes should return "low" + And classifying risk for 4 affected nodes should return "medium" + And classifying risk for 10 affected nodes should return "medium" + And classifying risk for 11 affected nodes should return "high" + And classifying risk for 100 affected nodes should return "high" + + # ── _compute_affected_subtree BFS order ────────────────────── + + Scenario: Compute affected subtree follows BFS order + Given a fresh correction service instance + When I compute affected subtree for "A" with tree "A->B,C;B->D;C->E" + Then the affected subtree should be exactly "A,B,C,D,E" in order + + Scenario: Compute affected subtree with no children returns only root + Given a fresh correction service instance + When I compute affected subtree for "LEAF" with an empty tree + Then the affected subtree should be exactly "LEAF" in order + + Scenario: Compute affected subtree with wide tree + Given a fresh correction service instance + When I compute affected subtree for "R" with tree "R->A,B,C,D,E" + Then the affected subtree should be exactly "R,A,B,C,D,E" in order + And the affected subtree count should be 6 diff --git a/features/database_models_new_coverage.feature b/features/database_models_new_coverage.feature new file mode 100644 index 000000000..2faae5a23 --- /dev/null +++ b/features/database_models_new_coverage.feature @@ -0,0 +1,85 @@ +@unit @database @models @new_coverage +Feature: Database models new coverage for missed lines and branches + As a developer + I want comprehensive coverage for database model conversions + So that all code paths including edge cases are tested + + # ------------------------------------------------------------------- + # ToolModel.from_domain: object (non-dict) access via getattr + # ------------------------------------------------------------------- + + @tool @from_domain @object_access + Scenario: ToolModel from_domain with object attribute access pattern + Given a domain tool object with attribute access + When I create a ToolModel from the domain object + Then the ToolModel should have the correct name + And the ToolModel should have the correct description + And the ToolModel should have the correct tool_type + And the ToolModel should have the correct source + + @tool @from_domain @object_access_namespaced + Scenario: ToolModel from_domain with namespaced object splits name correctly + Given a domain tool object with a namespaced name attribute + When I create a ToolModel from the namespaced domain object + Then the ToolModel namespace should be extracted correctly + And the ToolModel short_name should be extracted correctly + + @tool @from_domain @object_defaults + Scenario: ToolModel from_domain with object falling back to defaults + Given a domain tool object with minimal attributes + When I create a ToolModel from the minimal domain object + Then the ToolModel should use default values for missing attributes + + # ------------------------------------------------------------------- + # SessionModel.from_domain: non-empty messages list + # ------------------------------------------------------------------- + + @session @from_domain @messages + Scenario: SessionModel from_domain with non-empty messages list + Given a domain session object with two messages + When I create a SessionModel from the domain session + Then the SessionModel should have two message child models + And the first message model should have the correct role and content + And the second message model should have the correct sequence + + @session @from_domain @single_message + Scenario: SessionModel from_domain with a single message + Given a domain session object with one message + When I create a SessionModel from the single-message domain session + Then the SessionModel should have exactly one message child model + And the message model should preserve the message_id + + # ------------------------------------------------------------------- + # SkillModel: instantiation exercises short_name and description columns + # ------------------------------------------------------------------- + + @skill @instantiation + Scenario: SkillModel instantiation covers short_name and description columns + Given a SkillModel is created with short_name and description values + Then the SkillModel short_name should match the provided value + And the SkillModel description should match the provided value + And the SkillModel namespace should be set correctly + + @skill @instantiation @with_items + Scenario: SkillModel instantiation with child SkillItemModel records + Given a SkillModel is created with a child SkillItemModel + Then the SkillModel should have one item in items_rel + And the SkillItemModel should have the correct item_type and item_name + + # ------------------------------------------------------------------- + # LifecyclePlanModel.from_domain: arguments_order with missing key + # ------------------------------------------------------------------- + + @plan @from_domain @arguments_order_missing_key + Scenario: Plan from_domain skips arguments_order entries not in arguments_dict + Given a plan domain object with arguments_order containing a missing key + When I convert the plan domain object to a database model + Then the database plan model should only have arguments for keys present in arguments_dict + And the missing key should not appear in the argument child records + + @plan @from_domain @arguments_order_mixed + Scenario: Plan from_domain handles mix of present and missing argument keys + Given a plan domain object with three ordered keys but only two in arguments_dict + When I convert the mixed-arguments plan to a database model + Then the database plan model should have exactly two argument child records + And the argument positions should reflect their order in arguments_order diff --git a/features/invariant_cli_new_coverage.feature b/features/invariant_cli_new_coverage.feature new file mode 100644 index 000000000..1b5191ab4 --- /dev/null +++ b/features/invariant_cli_new_coverage.feature @@ -0,0 +1,161 @@ +Feature: Invariant CLI commands coverage + As a developer + I want full test coverage for the invariant CLI commands module + So that all commands and helper functions are verified + + # === _resolve_scope helper === + + Scenario: Resolve scope with --global flag returns GLOBAL scope + When I resolve invariant scope with global flag set + Then the resolved invariant scope should be "global" + And the resolved invariant source name should be "system" + + Scenario: Resolve scope with --project flag returns PROJECT scope + When I resolve invariant scope with project "myapp" + Then the resolved invariant scope should be "project" + And the resolved invariant source name should be "myapp" + + Scenario: Resolve scope with --plan flag returns PLAN scope + When I resolve invariant scope with plan "PLAN_01HX" + Then the resolved invariant scope should be "plan" + And the resolved invariant source name should be "PLAN_01HX" + + Scenario: Resolve scope with --action flag returns ACTION scope + When I resolve invariant scope with action "deploy-service" + Then the resolved invariant scope should be "action" + And the resolved invariant source name should be "deploy-service" + + Scenario: Resolve scope with no flags defaults to GLOBAL + When I resolve invariant scope with no flags + Then the resolved invariant scope should be "global" + And the resolved invariant source name should be "system" + + Scenario: Resolve scope with conflicting flags raises BadParameter + When I resolve invariant scope with global and project flags + Then a BadParameter error should be raised for invariant scope + + # === _invariant_dict helper === + + Scenario: Invariant dict serializes an invariant correctly + Given a sample invariant with known fields for dict test + When I call invariant_dict on the sample invariant + Then the invariant dict should contain the correct id and text + And the invariant dict should contain scope and source_name + And the invariant dict should contain active and created_at ISO string + + # === _get_service singleton === + + Scenario: Get service creates InvariantService lazily + When I call get_service with invariant module service reset to None + Then an InvariantService instance should be returned from get_service + And calling get_service again returns the same InvariantService instance + + # === invariant add command === + + Scenario: Add invariant with --global flag via CLI + Given a mocked InvariantService for invariant CLI add + When I invoke invariant add with "--global" and text "Never delete prod data" + Then the invariant CLI exit code should be 0 + And the invariant CLI output should contain "Invariant added" + And the invariant CLI output should contain "Never delete prod data" + + Scenario: Add invariant with --project flag via CLI + Given a mocked InvariantService for invariant CLI add + When I invoke invariant add with "--project myapp" and text "All APIs need tests" + Then the invariant CLI exit code should be 0 + And the invariant CLI output should contain "Invariant added" + + Scenario: Add invariant with --format json via CLI + Given a mocked InvariantService for invariant CLI add + When I invoke invariant add with "--global --format json" and text "JSON constraint" + Then the invariant CLI exit code should be 0 + And the invariant CLI output should contain "JSON constraint" + + Scenario: Add invariant handles CleverAgentsError + Given a mocked InvariantService that raises CleverAgentsError on add + When I invoke invariant add with "--global" and text "will fail" + Then the invariant CLI exit code should be non-zero + And the invariant CLI output should contain "Error" + + # === invariant list command === + + Scenario: List invariants with no results + Given a mocked InvariantService that returns empty invariant list + When I invoke invariant list with no filters + Then the invariant CLI exit code should be 0 + And the invariant CLI output should contain "No invariants found" + + Scenario: List invariants with results in rich format + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with no filters + Then the invariant CLI exit code should be 0 + And the invariant CLI output should contain "Invariants" + And the invariant CLI output should contain "Never delete prod" + + Scenario: List invariants with --global filter + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with flags "--global" + Then the invariant CLI exit code should be 0 + And the invariant service list was called with global scope + + Scenario: List invariants with --project filter + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with flags "--project myapp" + Then the invariant CLI exit code should be 0 + And the invariant service list was called with project scope and source "myapp" + + Scenario: List invariants with --plan filter + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with flags "--plan PLAN_01HX" + Then the invariant CLI exit code should be 0 + And the invariant service list was called with plan scope and source "PLAN_01HX" + + Scenario: List invariants with --action filter + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with flags "--action deploy" + Then the invariant CLI exit code should be 0 + And the invariant service list was called with action scope and source "deploy" + + Scenario: List invariants with --effective flag + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with flags "--effective" + Then the invariant CLI exit code should be 0 + And the invariant service list was called with effective true + + Scenario: List invariants with regex filter matching + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with regex "prod" + Then the invariant CLI exit code should be 0 + And the invariant CLI output should contain "Never delete prod" + + Scenario: List invariants with invalid regex + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with regex "[invalid" + Then the invariant CLI exit code should be non-zero + And the invariant CLI output should contain "Invalid regex" + + Scenario: List invariants with --format json + Given a mocked InvariantService that returns two invariants for list + When I invoke invariant list with flags "--format json" + Then the invariant CLI exit code should be 0 + And the invariant CLI output should contain "Never delete prod" + + # === invariant remove command === + + Scenario: Remove invariant with --yes flag + Given a mocked InvariantService for invariant CLI remove + When I invoke invariant remove with "--yes" for id "INV_01HX" + Then the invariant CLI exit code should be 0 + And the invariant CLI output should contain "Invariant removed" + + Scenario: Remove invariant without --yes cancelled by user + Given a mocked InvariantService for invariant CLI remove + When I invoke invariant remove without --yes for id "INV_01HX" and answer no + Then the invariant CLI exit code should be non-zero + And the invariant CLI output should contain "Cancelled" + + Scenario: Remove invariant raises NotFoundError + Given a mocked InvariantService that raises NotFoundError on remove + When I invoke invariant remove with "--yes" for id "MISSING_ID" + Then the invariant CLI exit code should be non-zero + And the invariant CLI output should contain "not found" diff --git a/features/lsp_cli_new_coverage.feature b/features/lsp_cli_new_coverage.feature new file mode 100644 index 000000000..82510b8d9 --- /dev/null +++ b/features/lsp_cli_new_coverage.feature @@ -0,0 +1,173 @@ +@phase2 @domain @lsp @lsp_cli @lsp_cli_new_coverage +Feature: LSP CLI commands coverage + As a developer managing LSP server registrations + I want the LSP CLI commands (add, remove, list, show) to work correctly + So that I can register, inspect, and manage language servers from the command line + + Background: + Given a clean LSP CLI test environment + + # --------------------------------------------------------------------------- + # lsp add + # --------------------------------------------------------------------------- + + @lsp_cli_add + Scenario: Add LSP server with valid YAML config + Given a valid LSP server YAML config file + When I run lsp CLI add with --config pointing to the YAML file + Then the lsp CLI command should succeed + And the lsp CLI output should contain "LSP Server Registered" + + @lsp_cli_add @error_handling + Scenario: Add LSP server with non-existent config file + When I run lsp CLI add with --config pointing to a non-existent file + Then the lsp CLI command should abort + And the lsp CLI output should contain "not found" + + @lsp_cli_add @error_handling + Scenario: Add LSP server with invalid YAML that is not a mapping + Given a YAML config file containing a list instead of a mapping + When I run lsp CLI add with --config pointing to the invalid YAML file + Then the lsp CLI command should abort + And the lsp CLI output should contain "Schema validation error" + + @lsp_cli_add + Scenario: Add LSP server with --update flag overwrites existing + Given a valid LSP server YAML config file + And the LSP server has been registered via CLI + When I run lsp CLI add with --config and --update + Then the lsp CLI command should succeed + And the lsp CLI output should contain "LSP Server Registered" + + @lsp_cli_add + Scenario: Add LSP server with --format json + Given a valid LSP server YAML config file + When I run lsp CLI add with --config and --format json + Then the lsp CLI command should succeed + And the lsp CLI output should contain "registered" + + @lsp_cli_add @error_handling + Scenario: Add duplicate LSP server without --update is rejected + Given a valid LSP server YAML config file + And the LSP server has been registered via CLI + When I run lsp CLI add with --config without --update + Then the lsp CLI command should abort + And the lsp CLI output should contain "already registered" + + @lsp_cli_add @error_handling + Scenario: Add LSP server with schema validation error from Pydantic + Given a YAML config file with invalid schema fields + When I run lsp CLI add with --config pointing to the bad schema YAML file + Then the lsp CLI command should abort + And the lsp CLI output should contain "Schema validation error" + + # --------------------------------------------------------------------------- + # lsp remove + # --------------------------------------------------------------------------- + + @lsp_cli_remove + Scenario: Remove a registered LSP server with --yes + Given a valid LSP server YAML config file + And the LSP server has been registered via CLI + When I run lsp CLI remove with name "local/test-lsp" and --yes + Then the lsp CLI command should succeed + And the lsp CLI output should contain "LSP Server Removed" + + @lsp_cli_remove @error_handling + Scenario: Remove an unknown LSP server returns not found error + When I run lsp CLI remove with name "local/nonexistent" and --yes + Then the lsp CLI command should abort + And the lsp CLI output should contain "not found" + + @lsp_cli_remove + Scenario: Remove a registered LSP server with --format json + Given a valid LSP server YAML config file + And the LSP server has been registered via CLI + When I run lsp CLI remove with name "local/test-lsp" and --yes and --format json + Then the lsp CLI command should succeed + And the lsp CLI output should contain "removed" + + # --------------------------------------------------------------------------- + # lsp list + # --------------------------------------------------------------------------- + + @lsp_cli_list + Scenario: List LSP servers when servers are registered + Given multiple LSP servers have been registered via CLI + When I run lsp CLI list + Then the lsp CLI command should succeed + And the lsp CLI output should contain "LSP Servers" + And the lsp CLI output should contain "listed" + + @lsp_cli_list + Scenario: List LSP servers when no servers are registered + When I run lsp CLI list + Then the lsp CLI command should succeed + And the lsp CLI output should contain "No LSP servers found" + + @lsp_cli_list + Scenario: List LSP servers filtered by namespace + Given multiple LSP servers have been registered via CLI + When I run lsp CLI list with --namespace "local" + Then the lsp CLI command should succeed + And the lsp CLI output should contain "local/test-lsp" + And the lsp CLI output should not contain "devops/clangd" + + @lsp_cli_list + Scenario: List LSP servers with --format json + Given multiple LSP servers have been registered via CLI + When I run lsp CLI list with --format json + Then the lsp CLI command should succeed + And the lsp CLI output should be valid JSON + + @lsp_cli_list + Scenario: List LSP servers filtered by language + Given multiple LSP servers have been registered via CLI + When I run lsp CLI list with --language "python" + Then the lsp CLI command should succeed + And the lsp CLI output should contain "local/test-lsp" + + # --------------------------------------------------------------------------- + # lsp show + # --------------------------------------------------------------------------- + + @lsp_cli_show + Scenario: Show details for a registered LSP server + Given a valid LSP server YAML config file + And the LSP server has been registered via CLI + When I run lsp CLI show with name "local/test-lsp" + Then the lsp CLI command should succeed + And the lsp CLI output should contain "LSP Server Details" + And the lsp CLI output should contain "local/test-lsp" + + @lsp_cli_show @error_handling + Scenario: Show details for an unknown LSP server returns error + When I run lsp CLI show with name "local/nonexistent" + Then the lsp CLI command should abort + And the lsp CLI output should contain "not found" + + @lsp_cli_show + Scenario: Show LSP server with --format json + Given a valid LSP server YAML config file + And the LSP server has been registered via CLI + When I run lsp CLI show with name "local/test-lsp" and --format json + Then the lsp CLI command should succeed + And the lsp CLI output should contain "local/test-lsp" + + @lsp_cli_show + Scenario: Show LSP server displays environment variables when present + Given a valid LSP server YAML config file with env vars + And the LSP server with env has been registered via CLI + When I run lsp CLI show with name "local/test-lsp-env" + Then the lsp CLI command should succeed + And the lsp CLI output should contain "Environment" + + # --------------------------------------------------------------------------- + # Helper functions + # --------------------------------------------------------------------------- + + @lsp_cli_helpers + Scenario: Reset and get registry helpers work correctly + When I call _reset_registry and then _get_registry + Then a new LspRegistry instance should be returned + And the new registry should be empty diff --git a/features/plan_commands_new_coverage.feature b/features/plan_commands_new_coverage.feature new file mode 100644 index 000000000..7f9c97117 --- /dev/null +++ b/features/plan_commands_new_coverage.feature @@ -0,0 +1,82 @@ +Feature: Plan CLI commands new coverage + As a developer + I want to cover the remaining uncovered lines in plan.py + So that plan diff (with/without correction), plan artifacts, and _get_apply_service are fully tested + + # ---------- plan diff with --correction flag (lines 1856-1867) ---------- + + Scenario: Plan diff with correction flag shows correction panel + Given new_cov no service mocks are needed + When new_cov I run plan diff for "plan-100" with correction "corr-42" + Then new_cov the exit code should be 0 + And new_cov the output should contain "Correction Diff" + And new_cov the output should contain "corr-42" + And new_cov the output should contain "plan-100" + + # ---------- plan diff without correction (lines 1869-1871) ---------- + + Scenario: Plan diff without correction calls service diff + Given new_cov the apply service diff returns "--- a/app.py\n+++ b/app.py\n@@ -1 +1 @@" + When new_cov I run plan diff for "plan-200" + Then new_cov the exit code should be 0 + And new_cov the output should contain "app.py" + + Scenario: Plan diff without correction with json format + Given new_cov the apply service diff returns '{"entries": [], "summary": {}}' + When new_cov I run plan diff for "plan-201" with format "json" + Then new_cov the exit code should be 0 + And new_cov the output should contain "entries" + + # ---------- plan diff error paths ---------- + + Scenario: Plan diff raises PlanError + Given new_cov the apply service diff raises PlanError "changeset missing" + When new_cov I run plan diff for "plan-bad" + Then new_cov the exit code should be nonzero + And new_cov the output should contain "Diff Error" + And new_cov the output should contain "changeset missing" + + Scenario: Plan diff raises CleverAgentsError + Given new_cov the apply service diff raises CleverAgentsError "backend down" + When new_cov I run plan diff for "plan-err" + Then new_cov the exit code should be nonzero + And new_cov the output should contain "Error" + And new_cov the output should contain "backend down" + + # ---------- plan artifacts (lines 1901-1904) ---------- + + Scenario: Plan artifacts calls service artifacts + Given new_cov the apply service artifacts returns "changeset_id: cs-999\nfiles: 3" + When new_cov I run plan artifacts for "plan-300" + Then new_cov the exit code should be 0 + And new_cov the output should contain "cs-999" + + Scenario: Plan artifacts with json format + Given new_cov the apply service artifacts returns '{"changeset_id": "cs-abc", "files": 5}' + When new_cov I run plan artifacts for "plan-301" with format "json" + Then new_cov the exit code should be 0 + And new_cov the output should contain "cs-abc" + + # ---------- plan artifacts error paths ---------- + + Scenario: Plan artifacts raises PlanError + Given new_cov the apply service artifacts raises PlanError "plan not found" + When new_cov I run plan artifacts for "plan-missing" + Then new_cov the exit code should be nonzero + And new_cov the output should contain "Artifacts Error" + And new_cov the output should contain "plan not found" + + Scenario: Plan artifacts raises CleverAgentsError + Given new_cov the apply service artifacts raises CleverAgentsError "timeout" + When new_cov I run plan artifacts for "plan-timeout" + Then new_cov the exit code should be nonzero + And new_cov the output should contain "Error" + And new_cov the output should contain "timeout" + + # ---------- _get_apply_service real body (lines 1817-1822) ---------- + + Scenario: _get_apply_service imports PlanApplyService and creates it + Given new_cov a mocked lifecycle service for apply service creation + When new_cov I call _get_apply_service directly + Then new_cov the returned object should be a PlanApplyService instance + And new_cov PlanApplyService was constructed with the lifecycle service diff --git a/features/plan_executor_new_coverage.feature b/features/plan_executor_new_coverage.feature new file mode 100644 index 000000000..8f9fd3042 --- /dev/null +++ b/features/plan_executor_new_coverage.feature @@ -0,0 +1,209 @@ +@plan @executor @coverage +Feature: PlanExecutor coverage gaps + As a developer + I want comprehensive tests for PlanExecutor, StrategizeStubActor, and ExecuteStubActor + So that uncovered branches and lines are fully exercised + + # -- PlanExecutor __init__ validation -- + + @init @error + Scenario: PlanExecutor raises ValidationError when lifecycle_service is None + When I construct a PlanExecutor with None lifecycle_service + Then a PE ValidationError should be raised containing "lifecycle_service" + + @init + Scenario: PlanExecutor initialises successfully with valid lifecycle_service + Given a mock plan lifecycle service + When I construct a PlanExecutor with the mock lifecycle service + Then the PlanExecutor should be created without error + + # -- has_runtime property -- + + @property + Scenario: has_runtime is True when execution_context is provided + Given a mock plan lifecycle service + And a mock execution context + When I construct a PlanExecutor with the mock lifecycle service and execution context + Then the executor has_runtime property should be True + + @property + Scenario: has_runtime is False when no execution_context is provided + Given a mock plan lifecycle service + When I construct a PlanExecutor with the mock lifecycle service + Then the executor has_runtime property should be False + + # -- changeset_store property -- + + @property + Scenario: changeset_store returns store from execution_context + Given a mock plan lifecycle service + And a mock execution context with a changeset store + When I construct a PlanExecutor with the mock lifecycle service and execution context + Then the executor changeset_store should be the mock store + + @property + Scenario: changeset_store returns None when no execution_context + Given a mock plan lifecycle service + When I construct a PlanExecutor with the mock lifecycle service + Then the executor changeset_store should be None + + # -- run_strategize guard: wrong phase -- + + @strategize @error + Scenario: run_strategize raises PlanError when plan is not in STRATEGIZE phase + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service + And the lifecycle returns a plan in EXECUTE phase + When I attempt to run strategize on the executor + Then a PE PlanError should be raised containing "not in Strategize phase" + + # -- run_strategize guard: empty plan_id -- + + @strategize @error + Scenario: run_strategize raises ValidationError for empty plan_id + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service + When I attempt to run strategize with an empty plan_id + Then a PE ValidationError should be raised containing "plan_id" + + # -- run_strategize exception handling (except branch) -- + + @strategize @error + Scenario: run_strategize calls fail_strategize when an exception occurs during execution + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service + And the lifecycle returns a plan in STRATEGIZE phase + And the lifecycle start_strategize will raise a RuntimeError + When I attempt to run strategize and expect an exception + Then fail_strategize should have been called with the plan_id + And the raised exception should be a RuntimeError + + @strategize @error + Scenario: run_strategize records error_details on plan when exception occurs + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service + And the lifecycle returns a plan in STRATEGIZE phase that fails during actor execute + When I attempt to run strategize and expect an exception + Then the plan error_details should contain the exception_type + And fail_strategize should have been called with the plan_id + + # -- _guard_execute: wrong phase -- + + @execute @error + Scenario: run_execute raises PlanError when plan is not in EXECUTE phase + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service + And the lifecycle returns a plan in STRATEGIZE phase for execute guard + When I attempt to run execute on the executor + Then a PE PlanError should be raised containing "not in Execute phase" + + # -- _guard_execute: wrong state -- + + @execute @error + Scenario: run_execute raises PlanError when plan state is not QUEUED + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service + And the lifecycle returns a plan in EXECUTE phase with PROCESSING state + When I attempt to run execute on the executor + Then a PE PlanError should be raised containing "not queued for execution" + + # -- _guard_execute: None decision_root_id -- + + @execute @error + Scenario: run_execute raises PlanError when decision_root_id is None + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service + And the lifecycle returns a plan in EXECUTE phase with QUEUED state but no decision root + When I attempt to run execute on the executor + Then a PE PlanError should be raised containing "no decision tree" + + # -- run_execute routing to stub -- + + @execute @stub + Scenario: run_execute routes to stub actor when no execution_context + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service and no execution context + And the lifecycle returns a fully valid plan for stub execution + When I run execute on the executor + Then the plan executor result should be an ExecuteResult + And complete_execute should have been called + + # -- run_execute routing to runtime -- + + @execute @runtime + Scenario: run_execute routes to runtime actor when execution_context is set + Given a mock plan lifecycle service + And a mock execution context + And a PlanExecutor with the mock lifecycle service and execution context and tool runner + And the lifecycle returns a fully valid plan for runtime execution + When I run execute on the executor + Then the plan executor result should be a RuntimeExecuteResult + And complete_execute should have been called + + # -- StrategizeStubActor with stream_callback -- + + @actor @strategize + Scenario: StrategizeStubActor emits stream events when callback is provided + Given a stream event collector + When I execute the StrategizeStubActor with a plan_id and stream callback + Then the collector should contain a "strategize_started" event + And the collector should contain a "strategize_decisions" event + And the collector should contain a "strategize_complete" event + + @actor @strategize + Scenario: StrategizeStubActor works without stream_callback + When I execute the StrategizeStubActor with a plan_id and no callback + Then the strategize result should have a valid decision_root_id + And the strategize result should have at least one decision + + # -- ExecuteStubActor with stream_callback -- + + @actor @execute + Scenario: ExecuteStubActor emits stream events when callback is provided + Given a stream event collector + And a list of two strategy decisions + When I execute the ExecuteStubActor with the decisions and stream callback + Then the collector should contain an "execute_started" event + And the collector should contain an "execute_step" event + And the collector should contain an "execute_complete" event + + @actor @execute + Scenario: ExecuteStubActor works without stream_callback + Given a list of two strategy decisions + When I execute the ExecuteStubActor with the decisions and no callback + Then the execute result should have a valid changeset_id + And the execute result tool_calls_count should be zero + + # -- run_strategize happy path with stream_callback (lines 138-143) -- + + @strategize + Scenario: run_strategize succeeds with stream_callback on a valid plan + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service + And the lifecycle returns a plan in STRATEGIZE phase + And a stream event collector + When I run strategize with the stream callback + Then the strategize result should have decisions + And complete_strategize should have been called + + # -- run_strategize happy path sets execution_context.decision_root_id -- + + @strategize + Scenario: run_strategize sets decision_root_id on execution_context when present + Given a mock plan lifecycle service + And a mock execution context + And a PlanExecutor with the mock lifecycle and execution context + And the lifecycle returns a plan in STRATEGIZE phase + When I run strategize successfully + Then the execution context decision_root_id should be set + + # -- run_execute with stub and exception -- + + @execute @error + Scenario: run_execute calls fail_execute when stub execution raises an error + Given a mock plan lifecycle service + And a PlanExecutor with the mock lifecycle service and no execution context + And the lifecycle returns a fully valid plan for stub execution + And the stub execute actor is patched to raise an error + When I attempt to run execute and expect an exception + Then fail_execute should have been called with the plan_id diff --git a/features/steps/correction_service_new_coverage_steps.py b/features/steps/correction_service_new_coverage_steps.py new file mode 100644 index 000000000..a45ee1287 --- /dev/null +++ b/features/steps/correction_service_new_coverage_steps.py @@ -0,0 +1,575 @@ +"""Step definitions for correction_service_new_coverage.feature. + +Provides comprehensive coverage for every method in CorrectionService, +including boundary conditions, BFS ordering, risk classification, and +error paths. Step text is intentionally distinct from the existing +correction_flows_steps.py and correction_model_steps.py to avoid clashes. +""" + +from __future__ import annotations + +from behave import given, then, when + +from cleveragents.application.services.correction_service import CorrectionService +from cleveragents.core.exceptions import ResourceNotFoundError, ValidationError +from cleveragents.domain.models.core.correction import CorrectionMode + + +# ------------------------------------------------------------------- +# Helpers +# ------------------------------------------------------------------- + + +def _parse_adjacency(spec: str) -> dict[str, list[str]]: + """Parse a compact adjacency-list notation. + + Format: ``"A->B,C;B->D,E;C->F"`` + Returns: ``{"A": ["B","C"], "B": ["D","E"], "C": ["F"]}`` + """ + tree: dict[str, list[str]] = {} + for segment in spec.split(";"): + segment = segment.strip() + if not segment: + continue + parent, children_str = segment.split("->") + tree[parent.strip()] = [c.strip() for c in children_str.split(",")] + return tree + + +def _build_chain(root: str, count: int) -> dict[str, list[str]]: + """Build a linear chain of *count* nodes starting at *root*.""" + tree: dict[str, list[str]] = {} + current = root + for i in range(1, count): + child = f"{root}_n{i}" + tree[current] = [child] + current = child + return tree + + +# ------------------------------------------------------------------- +# Given steps +# ------------------------------------------------------------------- + + +@given("a fresh correction service instance") +def step_fresh_service(context): + context.svc = CorrectionService() + context.cid = None # current correction_id + context.new_req = None + context.impact = None + context.revert_result = None + context.append_result = None + context.dispatch_result = None + context.dry_run_report = None + context.retrieved = None + context.corrections_list = None + context.attempts_list = None + context.subtree = None + context.svc_error = None + + +@given('a stored revert correction for plan "{plan}" targeting "{target}"') +def step_stored_revert(context, plan, target): + req = context.svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.REVERT, + ) + context.cid = req.correction_id + context.new_req = req + + +@given('a stored append correction for plan "{plan}" targeting "{target}"') +def step_stored_append(context, plan, target): + req = context.svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.APPEND, + ) + context.cid = req.correction_id + context.new_req = req + + +@given("the stored correction has been reverted with an empty tree") +def step_execute_revert_given(context): + context.svc.execute_revert(context.cid, {}) + + +# ------------------------------------------------------------------- +# When steps +# ------------------------------------------------------------------- + + +@when('I create a correction for plan "{plan}" targeting "{target}" in revert mode') +def step_create_revert(context, plan, target): + context.new_req = context.svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.REVERT, + ) + context.cid = context.new_req.correction_id + + +@when( + 'I create a correction for plan "{plan}" targeting "{target}" in append mode' + ' with guidance "{guidance}" and dry_run true' +) +def step_create_append_guidance_dryrun(context, plan, target, guidance): + context.new_req = context.svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.APPEND, + guidance=guidance, + dry_run=True, + ) + context.cid = context.new_req.correction_id + + +@when('I attempt to create a correction with plan_id "{plan}" and target "{target}"') +def step_attempt_create_bad(context, plan, target): + try: + context.svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.REVERT, + ) + context.svc_error = None + except (ValidationError, Exception) as exc: + context.svc_error = exc + + +@when("I analyze impact with an empty decision tree") +def step_analyze_empty(context): + context.impact = context.svc.analyze_impact(context.cid, {}) + + +@when('I analyze impact with tree "{spec}"') +def step_analyze_with_tree(context, spec): + tree = _parse_adjacency(spec) + context.impact = context.svc.analyze_impact(context.cid, tree) + + +@when('I analyze impact with a chain tree of {n:d} nodes rooted at "{root}"') +def step_analyze_chain(context, n, root): + tree = _build_chain(root, n) + context.impact = context.svc.analyze_impact(context.cid, tree) + + +@when('I attempt to analyze impact for correction "{cid}"') +def step_attempt_analyze_bad(context, cid): + try: + context.svc.analyze_impact(cid, {}) + context.svc_error = None + except ResourceNotFoundError as exc: + context.svc_error = exc + + +@when('I generate a dry-run report with a chain tree of {n:d} nodes rooted at "{root}"') +def step_dryrun_chain(context, n, root): + tree = _build_chain(root, n) + context.dry_run_report = context.svc.generate_dry_run_report(context.cid, tree) + + +@when("I generate a dry-run report with an empty decision tree") +def step_dryrun_empty(context): + context.dry_run_report = context.svc.generate_dry_run_report(context.cid, {}) + + +@when('I execute revert with tree "{spec}"') +def step_execute_revert_tree(context, spec): + tree = _parse_adjacency(spec) + context.revert_result = context.svc.execute_revert(context.cid, tree) + + +@when("I execute revert with an empty decision tree") +def step_execute_revert_empty(context): + context.revert_result = context.svc.execute_revert(context.cid, {}) + + +@when("I execute append for the stored correction") +def step_execute_append(context): + context.append_result = context.svc.execute_append(context.cid) + + +@when('I execute correction via the dispatch method with tree "{spec}"') +def step_dispatch_with_tree(context, spec): + tree = _parse_adjacency(spec) + context.dispatch_result = context.svc.execute_correction(context.cid, tree) + + +@when("I execute correction via the dispatch method with no tree") +def step_dispatch_no_tree(context): + context.dispatch_result = context.svc.execute_correction(context.cid) + + +@when("I get the stored correction by its id") +def step_get_by_id(context): + context.retrieved = context.svc.get_correction(context.cid) + + +@when('I attempt to get correction with id "{cid}"') +def step_attempt_get_bad(context, cid): + try: + context.svc.get_correction(cid) + context.svc_error = None + except ResourceNotFoundError as exc: + context.svc_error = exc + + +@when("I list all service corrections without filter") +def step_list_all(context): + context.corrections_list = context.svc.list_corrections() + + +@when('I list service corrections for plan "{plan}"') +def step_list_by_plan(context, plan): + context.corrections_list = context.svc.list_corrections(plan_id=plan) + + +@when("I list attempts for the stored correction") +def step_list_attempts(context): + context.attempts_list = context.svc.list_attempts(context.cid) + + +@when("I cancel the stored correction") +def step_cancel(context): + context.svc.cancel_correction(context.cid) + + +@when("I attempt to cancel the stored correction") +def step_attempt_cancel(context): + try: + context.svc.cancel_correction(context.cid) + context.svc_error = None + except ValidationError as exc: + context.svc_error = exc + + +@when('I compute affected subtree for "{root}" with tree "{spec}"') +def step_compute_subtree(context, root, spec): + tree = _parse_adjacency(spec) + context.subtree = CorrectionService._compute_affected_subtree(root, tree) + + +@when('I compute affected subtree for "{root}" with an empty tree') +def step_compute_subtree_empty(context, root): + context.subtree = CorrectionService._compute_affected_subtree(root, {}) + + +# ------------------------------------------------------------------- +# Then steps +# ------------------------------------------------------------------- + + +@then("the new correction should be stored in the service") +def step_stored(context): + assert context.new_req is not None, "No correction was created" + fetched = context.svc.get_correction(context.new_req.correction_id) + assert fetched.correction_id == context.new_req.correction_id + + +@then('the new correction plan_id should equal "{val}"') +def step_new_plan(context, val): + assert context.new_req.plan_id == val, ( + f"Expected plan_id '{val}', got '{context.new_req.plan_id}'" + ) + + +@then('the new correction target_decision_id should equal "{val}"') +def step_new_target(context, val): + assert context.new_req.target_decision_id == val + + +@then('the new correction mode should equal "{val}"') +def step_new_mode(context, val): + assert context.new_req.mode.value == val, ( + f"Expected mode '{val}', got '{context.new_req.mode.value}'" + ) + + +@then('the new correction status should equal "{val}"') +def step_new_status(context, val): + assert context.new_req.status.value == val + + +@then("the new correction dry_run should be false") +def step_dryrun_false(context): + assert context.new_req.dry_run is False + + +@then("the new correction dry_run should be true") +def step_dryrun_true(context): + assert context.new_req.dry_run is True + + +@then('the new correction guidance should equal "{val}"') +def step_new_guidance(context, val): + assert context.new_req.guidance == val + + +@then("a service ValidationError should have been raised") +def step_svc_validation_error(context): + assert context.svc_error is not None, "Expected ValidationError but none was raised" + assert isinstance(context.svc_error, ValidationError), ( + f"Expected ValidationError, got {type(context.svc_error).__name__}" + ) + + +@then("a service ResourceNotFoundError should have been raised") +def step_svc_not_found_error(context): + assert context.svc_error is not None, ( + "Expected ResourceNotFoundError but none was raised" + ) + assert isinstance(context.svc_error, ResourceNotFoundError), ( + f"Expected ResourceNotFoundError, got {type(context.svc_error).__name__}" + ) + + +# ── impact assertions ────────────────────────────────────────── + + +@then('the impact affected decisions should be exactly "{expected}"') +def step_impact_decisions(context, expected): + expected_list = [d.strip() for d in expected.split(",")] + assert context.impact.affected_decisions == expected_list, ( + f"Expected {expected_list}, got {context.impact.affected_decisions}" + ) + + +@then('the impact risk level should equal "{val}"') +def step_impact_risk(context, val): + assert context.impact.risk_level == val, ( + f"Expected risk '{val}', got '{context.impact.risk_level}'" + ) + + +@then('the impact rollback tier should equal "{val}"') +def step_impact_rollback(context, val): + assert context.impact.rollback_tier == val + + +@then("the impact estimated cost should equal {cost:g}") +def step_impact_cost(context, cost): + assert context.impact.estimated_cost == cost, ( + f"Expected cost {cost}, got {context.impact.estimated_cost}" + ) + + +@then('the impact artifacts to archive should be exactly "{expected}"') +def step_impact_artifacts(context, expected): + expected_list = [a.strip() for a in expected.split(",")] + assert context.impact.artifacts_to_archive == expected_list + + +@then('the impact affected files should be exactly "{expected}"') +def step_impact_files(context, expected): + expected_list = [f.strip() for f in expected.split(",")] + assert context.impact.affected_files == expected_list + + +@then("the impact affected decisions count should be {n:d}") +def step_impact_count(context, n): + actual = len(context.impact.affected_decisions) + assert actual == n, f"Expected {n} affected decisions, got {actual}" + + +@then('the stored correction status should be "{val}"') +def step_stored_status(context, val): + req = context.svc.get_correction(context.cid) + assert req.status.value == val, f"Expected status '{val}', got '{req.status.value}'" + + +# ── dry-run report assertions ───────────────────────────────── + + +@then('the dry-run report mode should equal "{val}"') +def step_dr_mode(context, val): + assert context.dry_run_report.mode.value == val + + +@then('the dry-run report warnings should include "{fragment}"') +def step_dr_warning(context, fragment): + joined = " | ".join(context.dry_run_report.warnings) + assert fragment in joined, ( + f"'{fragment}' not in warnings: {context.dry_run_report.warnings}" + ) + + +@then("the dry-run report decisions to invalidate should not be empty") +def step_dr_invalidate_not_empty(context): + assert len(context.dry_run_report.decisions_to_invalidate) > 0 + + +@then("the dry-run report decisions to invalidate should be empty") +def step_dr_invalidate_empty(context): + assert len(context.dry_run_report.decisions_to_invalidate) == 0 + + +@then("the dry-run report estimated recompute time should be {t:g}") +def step_dr_recompute(context, t): + assert context.dry_run_report.estimated_recompute_time_seconds == t, ( + f"Expected {t}, got {context.dry_run_report.estimated_recompute_time_seconds}" + ) + + +# ── revert result assertions ────────────────────────────────── + + +@then('the revert result status should equal "{val}"') +def step_revert_status(context, val): + assert context.revert_result.status.value == val + + +@then('the revert result reverted decisions should include "{d}"') +def step_revert_decisions(context, d): + assert d in context.revert_result.reverted_decisions, ( + f"'{d}' not in {context.revert_result.reverted_decisions}" + ) + + +@then('the revert result archived artifacts should include "{a}"') +def step_revert_artifacts(context, a): + assert a in context.revert_result.archived_artifacts, ( + f"'{a}' not in {context.revert_result.archived_artifacts}" + ) + + +# ── attempt assertions ──────────────────────────────────────── + + +@then("the service attempts for the correction should have {n:d} entry") +def step_attempts_count(context, n): + attempts = context.svc.list_attempts(context.cid) + assert len(attempts) == n, f"Expected {n} attempts, got {len(attempts)}" + + +@then("the first attempt should have success true") +def step_first_attempt_ok(context): + attempts = context.svc.list_attempts(context.cid) + assert attempts[0].success is True + + +@then("the first attempt completed_at should not be none") +def step_first_attempt_completed(context): + attempts = context.svc.list_attempts(context.cid) + assert attempts[0].completed_at is not None + + +@then("the attempts list should have {n:d} entry") +def step_attempts_list_count(context, n): + assert len(context.attempts_list) == n, ( + f"Expected {n}, got {len(context.attempts_list)}" + ) + + +# ── append result assertions ────────────────────────────────── + + +@then('the append result status should equal "{val}"') +def step_append_status(context, val): + assert context.append_result.status.value == val + + +@then("the append result spawned_child_plan_id should not be none") +def step_append_child(context): + assert context.append_result.spawned_child_plan_id is not None + + +@then("the append result new decisions should not be empty") +def step_append_new_decisions(context): + assert len(context.append_result.new_decisions) > 0 + + +# ── dispatch result assertions ──────────────────────────────── + + +@then('the dispatched result status should equal "{val}"') +def step_dispatch_status(context, val): + assert context.dispatch_result.status.value == val + + +@then('the dispatched result reverted decisions should include "{d}"') +def step_dispatch_reverted(context, d): + assert d in context.dispatch_result.reverted_decisions + + +@then("the dispatched result spawned_child_plan_id should not be none") +def step_dispatch_child(context): + assert context.dispatch_result.spawned_child_plan_id is not None + + +# ── get_correction assertions ───────────────────────────────── + + +@then('the retrieved correction plan_id should equal "{val}"') +def step_retrieved_plan(context, val): + assert context.retrieved.plan_id == val + + +@then('the retrieved correction target_decision_id should equal "{val}"') +def step_retrieved_target(context, val): + assert context.retrieved.target_decision_id == val + + +# ── list_corrections assertions ─────────────────────────────── + + +@then("the service corrections list should have {n:d} items") +def step_corrections_count(context, n): + assert len(context.corrections_list) == n, ( + f"Expected {n}, got {len(context.corrections_list)}" + ) + + +# ── classify_risk assertions ────────────────────────────────── + + +@then('classifying risk for {n:d} affected nodes should return "{expected}"') +def step_classify_risk(context, n, expected): + result = CorrectionService._classify_risk(n) + assert result == expected, ( + f"_classify_risk({n}) = '{result}', expected '{expected}'" + ) + + +# ── subtree assertions ──────────────────────────────────────── + + +@then('the affected subtree should be exactly "{expected}" in order') +def step_subtree_order(context, expected): + expected_list = [n.strip() for n in expected.split(",")] + assert context.subtree == expected_list, ( + f"Expected {expected_list}, got {context.subtree}" + ) + + +@then("the affected subtree count should be {n:d}") +def step_subtree_count(context, n): + assert len(context.subtree) == n, f"Expected {n} nodes, got {len(context.subtree)}" + + +@when("I attempt to create a correction with an empty plan_id") +def step_attempt_create_empty_plan(context): + try: + context.svc.request_correction( + plan_id="", + target_decision_id="DX1", + mode=CorrectionMode.REVERT, + ) + context.svc_error = None + except (ValidationError, Exception) as exc: + context.svc_error = exc + + +@when("I attempt to create a correction with an empty target_decision_id") +def step_attempt_create_empty_target(context): + try: + context.svc.request_correction( + plan_id="PX1", + target_decision_id="", + mode=CorrectionMode.REVERT, + ) + context.svc_error = None + except (ValidationError, Exception) as exc: + context.svc_error = exc diff --git a/features/steps/database_models_new_coverage_steps.py b/features/steps/database_models_new_coverage_steps.py new file mode 100644 index 000000000..fb48acdcd --- /dev/null +++ b/features/steps/database_models_new_coverage_steps.py @@ -0,0 +1,541 @@ +"""Step definitions for database_models_new_coverage.feature. + +Covers missed lines and branches in database models: +- ToolModel.from_domain() with object (non-dict) access via getattr (line 1666-1667) +- SessionModel.from_domain() with non-empty messages list (line 1920-1921) +- SkillModel instantiation exercising short_name and description columns (lines 2131-2132) +- LifecyclePlanModel.from_domain() arguments_order with missing keys (line 948 false branch) +""" + +from __future__ import annotations + +import json +from datetime import datetime +from types import SimpleNamespace + +from behave import given, then, when +from ulid import ULID + +from cleveragents.infrastructure.database.models import ( + Base, + LifecycleActionModel, + LifecyclePlanModel, + PlanArgumentModel, + SessionMessageModel, + SessionModel, + SkillItemModel, + SkillModel, + ToolModel, +) + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +NOW = datetime.now() +NOW_ISO = NOW.isoformat() +LATER_ISO = datetime(2025, 12, 31, 23, 59, 59).isoformat() + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_tool_object(**overrides): + """Create a SimpleNamespace mimicking a domain tool object (non-dict).""" + defaults = { + "name": "local/my-tool", + "description": "A test tool", + "tool_type": "tool", + "source": "builtin", + "input_schema_json": None, + "output_schema_json": None, + "capability_json": None, + "lifecycle_json": None, + "code": None, + "mcp_server": None, + "mcp_tool_name": None, + "agent_skill_path": None, + "timeout": 300, + "wraps": None, + "transform": None, + "mode": None, + "argument_mapping_json": None, + "created_at": NOW_ISO, + "updated_at": NOW_ISO, + "resource_bindings": [], + } + defaults.update(overrides) + return SimpleNamespace(**defaults) + + +def _make_session_message( + *, + message_id=None, + role="user", + content="hello", + sequence=0, + metadata=None, + tool_call_id=None, +): + """Create a SimpleNamespace mimicking a SessionMessage domain object.""" + return SimpleNamespace( + message_id=message_id or str(ULID()), + session_id=None, + role=SimpleNamespace(value=role), + content=content, + sequence=sequence, + timestamp=NOW, + metadata=metadata or {}, + tool_call_id=tool_call_id, + ) + + +def _make_token_usage(input_tokens=10, output_tokens=20, estimated_cost=0.001): + """Create a SimpleNamespace mimicking SessionTokenUsage.""" + return SimpleNamespace( + input_tokens=input_tokens, + output_tokens=output_tokens, + estimated_cost=estimated_cost, + ) + + +def _make_session_object(*, messages=None, session_id=None): + """Create a SimpleNamespace mimicking a Session domain object.""" + return SimpleNamespace( + session_id=session_id or str(ULID()), + actor_name="test-actor", + namespace="local", + linked_plan_ids=[], + token_usage=_make_token_usage(), + metadata={"test": True}, + messages=messages or [], + created_at=NOW, + updated_at=NOW, + ) + + +def _make_plan_identity(): + """Create a SimpleNamespace mimicking PlanIdentity.""" + return SimpleNamespace( + plan_id=str(ULID()), + parent_plan_id=None, + root_plan_id=None, + attempt=1, + ) + + +def _make_plan_timestamps(): + """Create a SimpleNamespace mimicking PlanTimestamps.""" + return SimpleNamespace( + created_at=NOW, + updated_at=NOW, + strategize_started_at=None, + strategize_completed_at=None, + execute_started_at=None, + execute_completed_at=None, + apply_started_at=None, + applied_at=None, + ) + + +def _make_namespaced_name(full="local/test-plan"): + """Create a SimpleNamespace mimicking NamespacedName.""" + parts = full.split("/", 1) + return SimpleNamespace( + namespace=parts[0] if len(parts) > 1 else "local", + __str__=lambda self=None, _f=full: _f, + ) + + +class _FakeNamespacedName: + """Fake NamespacedName that has namespace and supports str().""" + + def __init__(self, full: str): + parts = full.split("/", 1) + self.namespace = parts[0] if len(parts) > 1 else "local" + self._full = full + + def __str__(self) -> str: + return self._full + + +def _make_plan_object( + *, arguments=None, arguments_order=None, action_name="local/cov-action" +): + """Create a SimpleNamespace mimicking a Plan domain object.""" + return SimpleNamespace( + identity=_make_plan_identity(), + namespaced_name=_FakeNamespacedName("local/test-plan"), + action_name=action_name, + description="Test plan for coverage", + definition_of_done="All tests pass", + phase=SimpleNamespace(value="action"), + processing_state=SimpleNamespace(value="queued"), + automation_profile=None, + strategy_actor="local/strategy", + execution_actor="local/executor", + review_actor=None, + apply_actor=None, + estimation_actor=None, + invariant_actor=None, + project_links=[], + invariants=[], + arguments=arguments if arguments is not None else {}, + arguments_order=arguments_order if arguments_order is not None else [], + changeset_id=None, + timestamps=_make_plan_timestamps(), + error_message=None, + error_details=None, + created_by="tester", + tags=[], + reusable=True, + read_only=False, + ) + + +# =========================================================================== +# ToolModel.from_domain with object (non-dict) access +# =========================================================================== + + +@given("a domain tool object with attribute access") +def step_tool_object_attr_access(context): + """Create a non-dict domain tool object for ToolModel.from_domain().""" + context.tool_obj = _make_tool_object( + name="local/attr-tool", + description="Tool via attribute access", + tool_type="tool", + source="inline", + ) + + +@when("I create a ToolModel from the domain object") +def step_create_tool_model(context): + """Call ToolModel.from_domain() with the non-dict object.""" + context.tool_model = ToolModel.from_domain(context.tool_obj) + + +@then("the ToolModel should have the correct name") +def step_tool_model_name(context): + assert context.tool_model.name == "local/attr-tool", ( + f"Expected 'local/attr-tool', got '{context.tool_model.name}'" + ) + + +@then("the ToolModel should have the correct description") +def step_tool_model_description(context): + assert context.tool_model.description == "Tool via attribute access" + + +@then("the ToolModel should have the correct tool_type") +def step_tool_model_tool_type(context): + assert context.tool_model.tool_type == "tool" + + +@then("the ToolModel should have the correct source") +def step_tool_model_source(context): + assert context.tool_model.source == "inline" + + +# -- Namespaced tool object -- + + +@given("a domain tool object with a namespaced name attribute") +def step_tool_namespaced_object(context): + """Create a non-dict domain tool object with a namespace/short_name style name.""" + context.ns_tool_obj = _make_tool_object( + name="acme/code-lint", + description="Namespaced tool", + ) + + +@when("I create a ToolModel from the namespaced domain object") +def step_create_ns_tool_model(context): + context.ns_tool_model = ToolModel.from_domain(context.ns_tool_obj) + + +@then("the ToolModel namespace should be extracted correctly") +def step_ns_tool_namespace(context): + assert context.ns_tool_model.namespace == "acme", ( + f"Expected 'acme', got '{context.ns_tool_model.namespace}'" + ) + + +@then("the ToolModel short_name should be extracted correctly") +def step_ns_tool_short_name(context): + assert context.ns_tool_model.short_name == "code-lint", ( + f"Expected 'code-lint', got '{context.ns_tool_model.short_name}'" + ) + + +# -- Minimal tool object (defaults) -- + + +@given("a domain tool object with minimal attributes") +def step_tool_minimal_object(context): + """Create a non-dict object with only name — other fields use getattr defaults.""" + context.min_tool_obj = SimpleNamespace(name="simple-tool") + + +@when("I create a ToolModel from the minimal domain object") +def step_create_min_tool_model(context): + context.min_tool_model = ToolModel.from_domain(context.min_tool_obj) + + +@then("the ToolModel should use default values for missing attributes") +def step_min_tool_defaults(context): + m = context.min_tool_model + assert m.name == "simple-tool" + assert m.description == "" + assert m.tool_type == "tool" + assert m.source == "builtin" + assert m.timeout == 300 + # No namespace for non-namespaced name + assert m.namespace == "" + assert m.short_name == "simple-tool" + + +# =========================================================================== +# SessionModel.from_domain with non-empty messages +# =========================================================================== + + +@given("a domain session object with two messages") +def step_session_two_messages(context): + """Create a session domain object with two messages in the list.""" + msg1 = _make_session_message( + role="user", + content="Hello world", + sequence=0, + ) + msg2 = _make_session_message( + role="assistant", + content="Hi there", + sequence=1, + ) + context.session_obj = _make_session_object(messages=[msg1, msg2]) + context.msg1_id = msg1.message_id + context.msg2_id = msg2.message_id + + +@when("I create a SessionModel from the domain session") +def step_create_session_model(context): + context.session_model = SessionModel.from_domain(context.session_obj) + + +@then("the SessionModel should have two message child models") +def step_session_two_messages_count(context): + assert len(context.session_model.messages_rel) == 2, ( + f"Expected 2 messages, got {len(context.session_model.messages_rel)}" + ) + + +@then("the first message model should have the correct role and content") +def step_first_message_role_content(context): + msg = context.session_model.messages_rel[0] + assert msg.role == "user" + assert msg.content == "Hello world" + + +@then("the second message model should have the correct sequence") +def step_second_message_sequence(context): + msg = context.session_model.messages_rel[1] + assert msg.sequence == 1 + + +# -- Single message -- + + +@given("a domain session object with one message") +def step_session_one_message(context): + """Create a session domain object with a single message.""" + msg = _make_session_message( + role="user", + content="Single message", + sequence=0, + ) + context.single_msg_session = _make_session_object(messages=[msg]) + context.single_msg_id = msg.message_id + + +@when("I create a SessionModel from the single-message domain session") +def step_create_single_msg_session_model(context): + context.single_session_model = SessionModel.from_domain(context.single_msg_session) + + +@then("the SessionModel should have exactly one message child model") +def step_single_msg_count(context): + assert len(context.single_session_model.messages_rel) == 1 + + +@then("the message model should preserve the message_id") +def step_single_msg_id_preserved(context): + msg = context.single_session_model.messages_rel[0] + assert msg.message_id == context.single_msg_id + + +# =========================================================================== +# SkillModel instantiation (short_name and description columns) +# =========================================================================== + + +@given("a SkillModel is created with short_name and description values") +def step_create_skill_model(context): + """Instantiate a SkillModel to exercise the short_name and description columns.""" + context.skill_model = SkillModel( + name="local/code-tools", + namespace="local", + short_name="code-tools", + description="A collection of code-related tools", + version="1.0.0", + metadata_json=json.dumps({"overrides": {}}), + created_at=NOW_ISO, + updated_at=NOW_ISO, + ) + + +@then("the SkillModel short_name should match the provided value") +def step_skill_short_name(context): + assert context.skill_model.short_name == "code-tools", ( + f"Expected 'code-tools', got '{context.skill_model.short_name}'" + ) + + +@then("the SkillModel description should match the provided value") +def step_skill_description(context): + assert context.skill_model.description == "A collection of code-related tools" + + +@then("the SkillModel namespace should be set correctly") +def step_skill_namespace(context): + assert context.skill_model.namespace == "local" + + +# -- SkillModel with child SkillItemModel -- + + +@given("a SkillModel is created with a child SkillItemModel") +def step_create_skill_with_item(context): + """Instantiate a SkillModel with a SkillItemModel child record.""" + context.skill_with_item = SkillModel( + name="local/data-tools", + namespace="local", + short_name="data-tools", + description="Data processing tools", + version="2.0.0", + metadata_json=None, + created_at=NOW_ISO, + updated_at=NOW_ISO, + ) + item = SkillItemModel( + skill_name="local/data-tools", + item_type="tool_ref", + item_name="local/csv-parser", + item_config=None, + item_order=0, + created_at=NOW_ISO, + ) + context.skill_with_item.items_rel.append(item) + + +@then("the SkillModel should have one item in items_rel") +def step_skill_item_count(context): + assert len(context.skill_with_item.items_rel) == 1 + + +@then("the SkillItemModel should have the correct item_type and item_name") +def step_skill_item_fields(context): + item = context.skill_with_item.items_rel[0] + assert item.item_type == "tool_ref" + assert item.item_name == "local/csv-parser" + + +# =========================================================================== +# LifecyclePlanModel.from_domain: arguments_order with key NOT in arguments_dict +# =========================================================================== + + +@given("a plan domain object with arguments_order containing a missing key") +def step_plan_with_missing_arg_key(context): + """Create a plan where arguments_order has a key absent from arguments_dict. + + arguments_dict has "file_path" but arguments_order lists both + "file_path" and "nonexistent_key". The false branch of + `if arg_name in arguments_dict` at line 949 should be hit for + "nonexistent_key". + """ + context.plan_obj = _make_plan_object( + arguments={"file_path": "/tmp/test.txt"}, + arguments_order=["file_path", "nonexistent_key"], + ) + + +@when("I convert the plan domain object to a database model") +def step_convert_plan_to_model(context): + context.plan_model = LifecyclePlanModel.from_domain( + context.plan_obj, action_name="local/cov-action" + ) + + +@then( + "the database plan model should only have arguments for keys present in arguments_dict" +) +def step_plan_args_only_present(context): + args = context.plan_model.arguments_rel + arg_names = [a.name for a in args] + assert "file_path" in arg_names, ( + f"Expected 'file_path' in arguments, got {arg_names}" + ) + assert len(args) == 1, f"Expected 1 argument, got {len(args)}" + + +@then("the missing key should not appear in the argument child records") +def step_plan_no_missing_key(context): + arg_names = [a.name for a in context.plan_model.arguments_rel] + assert "nonexistent_key" not in arg_names, ( + f"'nonexistent_key' should not be in arguments: {arg_names}" + ) + + +# -- Mixed present/missing keys -- + + +@given("a plan domain object with three ordered keys but only two in arguments_dict") +def step_plan_mixed_keys(context): + """Create a plan with 3 keys in arguments_order but only 2 in arguments_dict. + + This exercises the false branch multiple times and confirms position tracking. + """ + context.mixed_plan_obj = _make_plan_object( + arguments={ + "input_file": "data.csv", + "output_dir": "/tmp/out", + }, + arguments_order=["input_file", "ghost_key", "output_dir"], + ) + + +@when("I convert the mixed-arguments plan to a database model") +def step_convert_mixed_plan(context): + context.mixed_plan_model = LifecyclePlanModel.from_domain( + context.mixed_plan_obj, action_name="local/cov-action" + ) + + +@then("the database plan model should have exactly two argument child records") +def step_mixed_plan_two_args(context): + args = context.mixed_plan_model.arguments_rel + assert len(args) == 2, f"Expected 2 arguments, got {len(args)}" + + +@then("the argument positions should reflect their order in arguments_order") +def step_mixed_plan_positions(context): + args = sorted(context.mixed_plan_model.arguments_rel, key=lambda a: a.position) + # "input_file" is at index 0 in arguments_order + assert args[0].name == "input_file" + assert args[0].position == 0 + # "output_dir" is at index 2 in arguments_order (ghost_key at index 1 is skipped) + assert args[1].name == "output_dir" + assert args[1].position == 2 diff --git a/features/steps/invariant_cli_new_coverage_steps.py b/features/steps/invariant_cli_new_coverage_steps.py new file mode 100644 index 000000000..7cf11707b --- /dev/null +++ b/features/steps/invariant_cli_new_coverage_steps.py @@ -0,0 +1,361 @@ +"""Step definitions for invariant_cli_new_coverage.feature. + +Tests all CLI commands and helper functions in +cleveragents.cli.commands.invariant to bring coverage from 0% to high. +""" + +from __future__ import annotations + +from datetime import datetime +from unittest.mock import MagicMock, patch + +import typer +from behave import given, then, when +from typer.testing import CliRunner + +from cleveragents.cli.commands.invariant import ( + _get_service, + _invariant_dict, + _resolve_scope, + app, +) +from cleveragents.core.exceptions import CleverAgentsError, NotFoundError +from cleveragents.domain.models.core.invariant import Invariant, InvariantScope + +_runner = CliRunner() + +_ULID = "01JTEST0000000000000000001" +_ULID2 = "01JTEST0000000000000000002" +_NOW = datetime(2025, 7, 1, 12, 0, 0) + + +def _make_invariant( + *, + inv_id: str = _ULID, + text: str = "Never delete prod data", + scope: InvariantScope = InvariantScope.GLOBAL, + source_name: str = "system", + active: bool = True, + created_at: datetime = _NOW, +) -> Invariant: + """Build a test Invariant with sensible defaults.""" + return Invariant( + id=inv_id, + text=text, + scope=scope, + source_name=source_name, + active=active, + created_at=created_at, + ) + + +def _patch_svc(context, svc): + """Patch the module-level _service and register cleanup.""" + patcher = patch("cleveragents.cli.commands.invariant._service", svc) + patcher.start() + context.add_cleanup(patcher.stop) + + +# ================================================================ +# _resolve_scope helper steps +# ================================================================ + + +@when("I resolve invariant scope with global flag set") +def step_resolve_scope_global(context): + context.resolved_scope, context.resolved_source = _resolve_scope( + is_global=True, project=None, plan=None, action=None + ) + + +@when('I resolve invariant scope with project "{project}"') +def step_resolve_scope_project(context, project): + context.resolved_scope, context.resolved_source = _resolve_scope( + is_global=False, project=project, plan=None, action=None + ) + + +@when('I resolve invariant scope with plan "{plan}"') +def step_resolve_scope_plan(context, plan): + context.resolved_scope, context.resolved_source = _resolve_scope( + is_global=False, project=None, plan=plan, action=None + ) + + +@when('I resolve invariant scope with action "{action}"') +def step_resolve_scope_action(context, action): + context.resolved_scope, context.resolved_source = _resolve_scope( + is_global=False, project=None, plan=None, action=action + ) + + +@when("I resolve invariant scope with no flags") +def step_resolve_scope_default(context): + context.resolved_scope, context.resolved_source = _resolve_scope( + is_global=False, project=None, plan=None, action=None + ) + + +@when("I resolve invariant scope with global and project flags") +def step_resolve_scope_conflicting(context): + context.inv_bad_parameter_raised = False + try: + _resolve_scope(is_global=True, project="myapp", plan=None, action=None) + except typer.BadParameter: + context.inv_bad_parameter_raised = True + + +@then('the resolved invariant scope should be "{scope}"') +def step_check_resolved_scope(context, scope): + assert context.resolved_scope.value == scope, ( + f"Expected scope '{scope}', got '{context.resolved_scope.value}'" + ) + + +@then('the resolved invariant source name should be "{source}"') +def step_check_resolved_source(context, source): + assert context.resolved_source == source, ( + f"Expected source '{source}', got '{context.resolved_source}'" + ) + + +@then("a BadParameter error should be raised for invariant scope") +def step_check_bad_parameter(context): + assert context.inv_bad_parameter_raised, "Expected typer.BadParameter to be raised" + + +# ================================================================ +# _invariant_dict helper steps +# ================================================================ + + +@given("a sample invariant with known fields for dict test") +def step_create_sample_invariant(context): + context.sample_inv = _make_invariant() + + +@when("I call invariant_dict on the sample invariant") +def step_call_invariant_dict(context): + context.inv_dict = _invariant_dict(context.sample_inv) + + +@then("the invariant dict should contain the correct id and text") +def step_check_dict_id_text(context): + d = context.inv_dict + assert d["id"] == _ULID + assert d["text"] == "Never delete prod data" + + +@then("the invariant dict should contain scope and source_name") +def step_check_dict_scope_source(context): + d = context.inv_dict + assert d["scope"] == "global" + assert d["source_name"] == "system" + + +@then("the invariant dict should contain active and created_at ISO string") +def step_check_dict_active_created(context): + d = context.inv_dict + assert d["active"] is True + assert d["created_at"] == _NOW.isoformat() + + +# ================================================================ +# _get_service singleton steps +# ================================================================ + + +@when("I call get_service with invariant module service reset to None") +def step_get_service_reset(context): + import cleveragents.cli.commands.invariant as mod + + # Save original and reset + context._orig_inv_service = mod._service + mod._service = None + context.add_cleanup(lambda: setattr(mod, "_service", context._orig_inv_service)) + + context.first_inv_service = _get_service() + + +@then("an InvariantService instance should be returned from get_service") +def step_check_service_instance(context): + from cleveragents.application.services.invariant_service import InvariantService + + assert isinstance(context.first_inv_service, InvariantService) + + +@then("calling get_service again returns the same InvariantService instance") +def step_check_service_singleton(context): + second = _get_service() + assert second is context.first_inv_service + + +# ================================================================ +# invariant add command steps +# ================================================================ + + +@given("a mocked InvariantService for invariant CLI add") +def step_mock_service_for_add(context): + svc = MagicMock() + + def _fake_add(text, scope, source_name): + return _make_invariant(text=text, scope=scope, source_name=source_name) + + svc.add_invariant.side_effect = _fake_add + svc.list_invariants.return_value = [] + context.inv_mock_svc = svc + _patch_svc(context, svc) + + +@when('I invoke invariant add with "{flags}" and text "{text}"') +def step_run_add(context, flags, text): + args = ["add"] + flags.split() + [text] + context.inv_result = _runner.invoke(app, args) + + +@then("the invariant CLI exit code should be 0") +def step_check_exit_zero(context): + assert context.inv_result.exit_code == 0, ( + f"Expected exit code 0, got {context.inv_result.exit_code}.\n" + f"Output: {context.inv_result.output}" + ) + + +@then("the invariant CLI exit code should be non-zero") +def step_check_exit_nonzero(context): + assert context.inv_result.exit_code != 0, ( + f"Expected non-zero exit code, got {context.inv_result.exit_code}.\n" + f"Output: {context.inv_result.output}" + ) + + +@then('the invariant CLI output should contain "{text}"') +def step_check_output_contains(context, text): + assert text in context.inv_result.output, ( + f"Expected '{text}' in output, got:\n{context.inv_result.output}" + ) + + +@given("a mocked InvariantService that raises CleverAgentsError on add") +def step_mock_service_error_add(context): + svc = MagicMock() + svc.add_invariant.side_effect = CleverAgentsError("Service failure") + _patch_svc(context, svc) + + +# ================================================================ +# invariant list command steps +# ================================================================ + + +@given("a mocked InvariantService that returns empty invariant list") +def step_mock_service_empty_list(context): + svc = MagicMock() + svc.list_invariants.return_value = [] + context.inv_mock_svc = svc + _patch_svc(context, svc) + + +@given("a mocked InvariantService that returns two invariants for list") +def step_mock_service_two_invariants(context): + svc = MagicMock() + inv1 = _make_invariant(inv_id=_ULID, text="Never delete prod data") + inv2 = _make_invariant( + inv_id=_ULID2, + text="All changes need review", + scope=InvariantScope.PROJECT, + source_name="myapp", + ) + svc.list_invariants.return_value = [inv1, inv2] + context.inv_mock_svc = svc + _patch_svc(context, svc) + + +@when("I invoke invariant list with no filters") +def step_run_list_no_filters(context): + context.inv_result = _runner.invoke(app, ["list"]) + + +@when('I invoke invariant list with flags "{flags}"') +def step_run_list_with_flags(context, flags): + args = ["list"] + flags.split() + context.inv_result = _runner.invoke(app, args) + + +@when('I invoke invariant list with regex "{pattern}"') +def step_run_list_with_regex(context, pattern): + context.inv_result = _runner.invoke(app, ["list", pattern]) + + +@then("the invariant service list was called with global scope") +def step_check_list_global(context): + call_kwargs = context.inv_mock_svc.list_invariants.call_args + assert call_kwargs is not None + kwargs = call_kwargs[1] if call_kwargs[1] else {} + assert kwargs.get("scope") == InvariantScope.GLOBAL + + +@then('the invariant service list was called with project scope and source "{source}"') +def step_check_list_project(context, source): + call_kwargs = context.inv_mock_svc.list_invariants.call_args + kwargs = call_kwargs[1] if call_kwargs[1] else {} + assert kwargs.get("scope") == InvariantScope.PROJECT + assert kwargs.get("source_name") == source + + +@then('the invariant service list was called with plan scope and source "{source}"') +def step_check_list_plan(context, source): + call_kwargs = context.inv_mock_svc.list_invariants.call_args + kwargs = call_kwargs[1] if call_kwargs[1] else {} + assert kwargs.get("scope") == InvariantScope.PLAN + assert kwargs.get("source_name") == source + + +@then('the invariant service list was called with action scope and source "{source}"') +def step_check_list_action(context, source): + call_kwargs = context.inv_mock_svc.list_invariants.call_args + kwargs = call_kwargs[1] if call_kwargs[1] else {} + assert kwargs.get("scope") == InvariantScope.ACTION + assert kwargs.get("source_name") == source + + +@then("the invariant service list was called with effective true") +def step_check_list_effective(context): + call_kwargs = context.inv_mock_svc.list_invariants.call_args + kwargs = call_kwargs[1] if call_kwargs[1] else {} + assert kwargs.get("effective") is True + + +# ================================================================ +# invariant remove command steps +# ================================================================ + + +@given("a mocked InvariantService for invariant CLI remove") +def step_mock_service_for_remove(context): + svc = MagicMock() + mock_inv = _make_invariant(active=False) + svc.remove_invariant.return_value = mock_inv + context.inv_mock_svc = svc + _patch_svc(context, svc) + + +@when('I invoke invariant remove with "{flags}" for id "{inv_id}"') +def step_run_remove_with_flags(context, flags, inv_id): + args = ["remove"] + flags.split() + [inv_id] + context.inv_result = _runner.invoke(app, args) + + +@when('I invoke invariant remove without --yes for id "{inv_id}" and answer no') +def step_run_remove_no_confirm(context, inv_id): + context.inv_result = _runner.invoke(app, ["remove", inv_id], input="n\n") + + +@given("a mocked InvariantService that raises NotFoundError on remove") +def step_mock_service_not_found_remove(context): + svc = MagicMock() + svc.remove_invariant.side_effect = NotFoundError( + resource_type="invariant", resource_id="MISSING_ID" + ) + _patch_svc(context, svc) diff --git a/features/steps/lsp_cli_new_coverage_steps.py b/features/steps/lsp_cli_new_coverage_steps.py new file mode 100644 index 000000000..d0a0f6635 --- /dev/null +++ b/features/steps/lsp_cli_new_coverage_steps.py @@ -0,0 +1,365 @@ +"""Step definitions for LSP CLI commands coverage tests. + +Covers all four CLI commands in cleveragents.cli.commands.lsp: + - add (register from YAML config) + - remove (unregister a server) + - list (list registered servers) + - show (show server details) + +Also covers the module-level helpers _reset_registry and _get_registry. +""" + +from __future__ import annotations + +import json +import os +import tempfile + +from behave import given, then, when +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.cli.commands.lsp import app, _get_registry, _reset_registry +from cleveragents.lsp.registry import LspRegistry + +_runner = CliRunner() + +# --------------------------------------------------------------------------- +# YAML fixtures +# --------------------------------------------------------------------------- + +_VALID_YAML = """\ +name: local/test-lsp +command: test-langserver +args: + - "--stdio" +languages: + - python +capabilities: + - diagnostics + - completions + - type_info +""" + +_VALID_YAML_2 = """\ +name: devops/clangd +command: clangd +args: + - "--background-index" +languages: + - c + - cpp +capabilities: + - diagnostics + - symbols +""" + +_VALID_YAML_ENV = """\ +name: local/test-lsp-env +command: test-langserver-env +args: + - "--stdio" +languages: + - python +capabilities: + - diagnostics +env: + VIRTUAL_ENV: /tmp/venv + PATH: /tmp/venv/bin +""" + +_LIST_YAML = """\ +- name: invalid + command: invalid +""" + +_BAD_SCHEMA_YAML = """\ +name: missing-slash-in-name +command: some-command +languages: + - python +capabilities: + - diagnostics +""" + + +# --------------------------------------------------------------------------- +# Helper to write temp YAML +# --------------------------------------------------------------------------- + + +def _write_temp_yaml(content: str) -> str: + """Write content to a temporary YAML file and return its path.""" + fd, path = tempfile.mkstemp(suffix=".yaml") + with os.fdopen(fd, "w") as fh: + fh.write(content) + return path + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("a clean LSP CLI test environment") +def step_clean_lsp_cli_env(context: Context) -> None: + _reset_registry() + context.lsp_cli_result = None + context.lsp_yaml_path = None + context.lsp_invalid_yaml_path = None + context.lsp_bad_schema_yaml_path = None + context.lsp_env_yaml_path = None + context._cleanup_paths: list[str] = [] + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given("a valid LSP server YAML config file") +def step_valid_lsp_yaml(context: Context) -> None: + path = _write_temp_yaml(_VALID_YAML) + context.lsp_yaml_path = path + context._cleanup_paths.append(path) + + +@given("a YAML config file containing a list instead of a mapping") +def step_invalid_yaml_list(context: Context) -> None: + path = _write_temp_yaml(_LIST_YAML) + context.lsp_invalid_yaml_path = path + context._cleanup_paths.append(path) + + +@given("a YAML config file with invalid schema fields") +def step_bad_schema_yaml(context: Context) -> None: + path = _write_temp_yaml(_BAD_SCHEMA_YAML) + context.lsp_bad_schema_yaml_path = path + context._cleanup_paths.append(path) + + +@given("a valid LSP server YAML config file with env vars") +def step_valid_lsp_yaml_env(context: Context) -> None: + path = _write_temp_yaml(_VALID_YAML_ENV) + context.lsp_env_yaml_path = path + context._cleanup_paths.append(path) + + +@given("the LSP server has been registered via CLI") +def step_lsp_server_registered(context: Context) -> None: + result = _runner.invoke(app, ["add", "--config", context.lsp_yaml_path]) + assert result.exit_code == 0, ( + f"Pre-registration failed (exit {result.exit_code}): {result.output}" + ) + + +@given("the LSP server with env has been registered via CLI") +def step_lsp_server_env_registered(context: Context) -> None: + result = _runner.invoke(app, ["add", "--config", context.lsp_env_yaml_path]) + assert result.exit_code == 0, ( + f"Pre-registration failed (exit {result.exit_code}): {result.output}" + ) + + +@given("multiple LSP servers have been registered via CLI") +def step_multiple_lsp_servers_registered(context: Context) -> None: + path1 = _write_temp_yaml(_VALID_YAML) + path2 = _write_temp_yaml(_VALID_YAML_2) + context._cleanup_paths.extend([path1, path2]) + + result1 = _runner.invoke(app, ["add", "--config", path1]) + assert result1.exit_code == 0, ( + f"Registration 1 failed (exit {result1.exit_code}): {result1.output}" + ) + + result2 = _runner.invoke(app, ["add", "--config", path2]) + assert result2.exit_code == 0, ( + f"Registration 2 failed (exit {result2.exit_code}): {result2.output}" + ) + + +# --------------------------------------------------------------------------- +# When steps — lsp add +# --------------------------------------------------------------------------- + + +@when("I run lsp CLI add with --config pointing to the YAML file") +def step_run_lsp_add(context: Context) -> None: + context.lsp_cli_result = _runner.invoke( + app, ["add", "--config", context.lsp_yaml_path] + ) + + +@when("I run lsp CLI add with --config pointing to a non-existent file") +def step_run_lsp_add_missing(context: Context) -> None: + context.lsp_cli_result = _runner.invoke( + app, ["add", "--config", "/tmp/nonexistent_lsp_config_abcdef.yaml"] + ) + + +@when("I run lsp CLI add with --config pointing to the invalid YAML file") +def step_run_lsp_add_invalid_yaml(context: Context) -> None: + context.lsp_cli_result = _runner.invoke( + app, ["add", "--config", context.lsp_invalid_yaml_path] + ) + + +@when("I run lsp CLI add with --config pointing to the bad schema YAML file") +def step_run_lsp_add_bad_schema(context: Context) -> None: + context.lsp_cli_result = _runner.invoke( + app, ["add", "--config", context.lsp_bad_schema_yaml_path] + ) + + +@when("I run lsp CLI add with --config and --update") +def step_run_lsp_add_update(context: Context) -> None: + context.lsp_cli_result = _runner.invoke( + app, ["add", "--config", context.lsp_yaml_path, "--update"] + ) + + +@when("I run lsp CLI add with --config and --format json") +def step_run_lsp_add_json(context: Context) -> None: + context.lsp_cli_result = _runner.invoke( + app, ["add", "--config", context.lsp_yaml_path, "--format", "json"] + ) + + +@when("I run lsp CLI add with --config without --update") +def step_run_lsp_add_no_update(context: Context) -> None: + context.lsp_cli_result = _runner.invoke( + app, ["add", "--config", context.lsp_yaml_path] + ) + + +# --------------------------------------------------------------------------- +# When steps — lsp remove +# --------------------------------------------------------------------------- + + +@when('I run lsp CLI remove with name "{name}" and --yes') +def step_run_lsp_remove(context: Context, name: str) -> None: + context.lsp_cli_result = _runner.invoke(app, ["remove", name, "--yes"]) + + +@when('I run lsp CLI remove with name "{name}" and --yes and --format json') +def step_run_lsp_remove_json(context: Context, name: str) -> None: + context.lsp_cli_result = _runner.invoke( + app, ["remove", name, "--yes", "--format", "json"] + ) + + +# --------------------------------------------------------------------------- +# When steps — lsp list +# --------------------------------------------------------------------------- + + +@when("I run lsp CLI list") +def step_run_lsp_list(context: Context) -> None: + context.lsp_cli_result = _runner.invoke(app, ["list"]) + + +@when('I run lsp CLI list with --namespace "{ns}"') +def step_run_lsp_list_namespace(context: Context, ns: str) -> None: + context.lsp_cli_result = _runner.invoke(app, ["list", "--namespace", ns]) + + +@when("I run lsp CLI list with --format json") +def step_run_lsp_list_json(context: Context) -> None: + context.lsp_cli_result = _runner.invoke(app, ["list", "--format", "json"]) + + +@when('I run lsp CLI list with --language "{lang}"') +def step_run_lsp_list_language(context: Context, lang: str) -> None: + context.lsp_cli_result = _runner.invoke(app, ["list", "--language", lang]) + + +# --------------------------------------------------------------------------- +# When steps — lsp show +# --------------------------------------------------------------------------- + + +@when('I run lsp CLI show with name "{name}"') +def step_run_lsp_show(context: Context, name: str) -> None: + context.lsp_cli_result = _runner.invoke(app, ["show", name]) + + +@when('I run lsp CLI show with name "{name}" and --format json') +def step_run_lsp_show_json(context: Context, name: str) -> None: + context.lsp_cli_result = _runner.invoke(app, ["show", name, "--format", "json"]) + + +# --------------------------------------------------------------------------- +# When steps — helpers +# --------------------------------------------------------------------------- + + +@when("I call _reset_registry and then _get_registry") +def step_reset_and_get_registry(context: Context) -> None: + _reset_registry() + context.lsp_new_registry = _get_registry() + + +# --------------------------------------------------------------------------- +# Then assertions +# --------------------------------------------------------------------------- + + +@then("the lsp CLI command should succeed") +def step_lsp_cli_success(context: Context) -> None: + result = context.lsp_cli_result + assert result is not None, "No CLI result captured" + assert result.exit_code == 0, ( + f"Expected exit code 0, got {result.exit_code}. Output:\n{result.output}" + ) + + +@then("the lsp CLI command should abort") +def step_lsp_cli_abort(context: Context) -> None: + result = context.lsp_cli_result + assert result is not None, "No CLI result captured" + assert result.exit_code != 0, ( + f"Expected non-zero exit code, got {result.exit_code}. Output:\n{result.output}" + ) + + +@then('the lsp CLI output should contain "{text}"') +def step_lsp_cli_output_contains(context: Context, text: str) -> None: + result = context.lsp_cli_result + assert result is not None, "No CLI result captured" + assert text in result.output, f"Expected '{text}' in output. Got:\n{result.output}" + + +@then('the lsp CLI output should not contain "{text}"') +def step_lsp_cli_output_not_contains(context: Context, text: str) -> None: + result = context.lsp_cli_result + assert result is not None, "No CLI result captured" + assert text not in result.output, ( + f"Did not expect '{text}' in output. Got:\n{result.output}" + ) + + +@then("the lsp CLI output should be valid JSON") +def step_lsp_cli_output_valid_json(context: Context) -> None: + result = context.lsp_cli_result + assert result is not None, "No CLI result captured" + try: + parsed = json.loads(result.output) + assert parsed is not None + except json.JSONDecodeError as exc: + raise AssertionError( + f"Output is not valid JSON: {exc}\nOutput:\n{result.output}" + ) from exc + + +@then("a new LspRegistry instance should be returned") +def step_new_registry_instance(context: Context) -> None: + assert context.lsp_new_registry is not None + assert isinstance(context.lsp_new_registry, LspRegistry) + + +@then("the new registry should be empty") +def step_new_registry_empty(context: Context) -> None: + assert len(context.lsp_new_registry) == 0, ( + f"Expected empty registry, got {len(context.lsp_new_registry)} servers" + ) diff --git a/features/steps/plan_commands_new_coverage_steps.py b/features/steps/plan_commands_new_coverage_steps.py new file mode 100644 index 000000000..55f96228e --- /dev/null +++ b/features/steps/plan_commands_new_coverage_steps.py @@ -0,0 +1,228 @@ +"""Step definitions for plan_commands_new_coverage.feature. + +Covers the remaining uncovered lines in plan.py: +- Lines 1817, 1821, 1822: _get_apply_service() function body +- Lines 1845-1855: plan_diff command body (correction branch + normal path) +- Lines 1878-1888: plan_artifacts command body +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +from behave import given, then, use_step_matcher, when +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.cli.commands.plan import app as plan_app +from cleveragents.core.exceptions import CleverAgentsError, PlanError + +runner = CliRunner() + +_PATCH_GET_APPLY = "cleveragents.cli.commands.plan._get_apply_service" +_PATCH_LIFECYCLE = "cleveragents.cli.commands.plan._get_lifecycle_service" +_PATCH_PAS_CLASS = ( + "cleveragents.application.services.plan_apply_service.PlanApplyService" +) + + +def _build_mock_service( + diff_return=None, + diff_side_effect=None, + artifacts_return=None, + artifacts_side_effect=None, +): + """Build a mock PlanApplyService with configurable diff/artifacts behaviour.""" + svc = MagicMock() + if diff_side_effect: + svc.diff.side_effect = diff_side_effect + else: + svc.diff.return_value = diff_return or "" + if artifacts_side_effect: + svc.artifacts.side_effect = artifacts_side_effect + else: + svc.artifacts.return_value = artifacts_return or "" + return svc + + +# --------------------------------------------------------------------------- +# GIVEN steps +# --------------------------------------------------------------------------- + +use_step_matcher("parse") + + +@given("new_cov no service mocks are needed") +def step_new_cov_no_mocks(context: Context) -> None: + """The correction branch returns early before calling any service.""" + context.new_cov_mock_service = None + + +@given('new_cov the apply service diff returns "{output}"') +def step_new_cov_diff_returns_dq(context: Context, output: str) -> None: + context.new_cov_mock_service = _build_mock_service(diff_return=output) + + +@given("new_cov the apply service diff returns '{output}'") +def step_new_cov_diff_returns_sq(context: Context, output: str) -> None: + context.new_cov_mock_service = _build_mock_service(diff_return=output) + + +@given('new_cov the apply service diff raises PlanError "{msg}"') +def step_new_cov_diff_plan_error(context: Context, msg: str) -> None: + context.new_cov_mock_service = _build_mock_service( + diff_side_effect=PlanError(msg), + ) + + +@given('new_cov the apply service diff raises CleverAgentsError "{msg}"') +def step_new_cov_diff_ca_error(context: Context, msg: str) -> None: + context.new_cov_mock_service = _build_mock_service( + diff_side_effect=CleverAgentsError(msg), + ) + + +@given('new_cov the apply service artifacts returns "{output}"') +def step_new_cov_artifacts_returns_dq(context: Context, output: str) -> None: + context.new_cov_mock_service = _build_mock_service(artifacts_return=output) + + +@given("new_cov the apply service artifacts returns '{output}'") +def step_new_cov_artifacts_returns_sq(context: Context, output: str) -> None: + context.new_cov_mock_service = _build_mock_service(artifacts_return=output) + + +@given('new_cov the apply service artifacts raises PlanError "{msg}"') +def step_new_cov_artifacts_plan_error(context: Context, msg: str) -> None: + context.new_cov_mock_service = _build_mock_service( + artifacts_side_effect=PlanError(msg), + ) + + +@given('new_cov the apply service artifacts raises CleverAgentsError "{msg}"') +def step_new_cov_artifacts_ca_error(context: Context, msg: str) -> None: + context.new_cov_mock_service = _build_mock_service( + artifacts_side_effect=CleverAgentsError(msg), + ) + + +@given("new_cov a mocked lifecycle service for apply service creation") +def step_new_cov_mock_lifecycle(context: Context) -> None: + context.new_cov_mock_lifecycle = MagicMock() + + +# --------------------------------------------------------------------------- +# WHEN steps +# --------------------------------------------------------------------------- + +use_step_matcher("re") + + +@when( + r'new_cov I run plan diff for "(?P[^"]+)" with correction "(?P[^"]+)"' +) +def step_new_cov_diff_with_correction( + context: Context, plan_id: str, corr: str +) -> None: + """Invoke ``plan diff --correction `` via the CLI runner. + + The correction branch returns before calling _get_apply_service, so + we do NOT need to mock it. + """ + context.new_cov_result = runner.invoke( + plan_app, ["diff", plan_id, "--correction", corr] + ) + + +@when(r'new_cov I run plan diff for "(?P[^"]+)" with format "(?P[^"]+)"') +def step_new_cov_diff_with_fmt(context: Context, plan_id: str, fmt: str) -> None: + with patch(_PATCH_GET_APPLY, return_value=context.new_cov_mock_service): + context.new_cov_result = runner.invoke( + plan_app, ["diff", plan_id, "--format", fmt] + ) + + +@when(r'new_cov I run plan diff for "(?P[^"]+)"') +def step_new_cov_diff(context: Context, plan_id: str) -> None: + with patch(_PATCH_GET_APPLY, return_value=context.new_cov_mock_service): + context.new_cov_result = runner.invoke(plan_app, ["diff", plan_id]) + + +@when( + r'new_cov I run plan artifacts for "(?P[^"]+)" with format "(?P[^"]+)"' +) +def step_new_cov_artifacts_with_fmt(context: Context, plan_id: str, fmt: str) -> None: + with patch(_PATCH_GET_APPLY, return_value=context.new_cov_mock_service): + context.new_cov_result = runner.invoke( + plan_app, ["artifacts", plan_id, "--format", fmt] + ) + + +@when(r'new_cov I run plan artifacts for "(?P[^"]+)"') +def step_new_cov_artifacts(context: Context, plan_id: str) -> None: + with patch(_PATCH_GET_APPLY, return_value=context.new_cov_mock_service): + context.new_cov_result = runner.invoke(plan_app, ["artifacts", plan_id]) + + +# Switch back to parse for the remaining non-parameterized WHEN step +use_step_matcher("parse") + + +@when("new_cov I call _get_apply_service directly") +def step_new_cov_call_get_apply(context: Context) -> None: + """Call the real _get_apply_service, mocking only its dependencies. + + This exercises the actual function body (import, lifecycle lookup, + PlanApplyService construction) to cover lines 1817, 1821, 1822. + """ + from cleveragents.cli.commands.plan import _get_apply_service + + mock_pas_instance = MagicMock() + mock_pas_class = MagicMock(return_value=mock_pas_instance) + + with ( + patch(_PATCH_LIFECYCLE, return_value=context.new_cov_mock_lifecycle), + patch(_PATCH_PAS_CLASS, mock_pas_class), + ): + context.new_cov_apply_result = _get_apply_service() + context.new_cov_pas_class = mock_pas_class + context.new_cov_pas_instance = mock_pas_instance + + +# --------------------------------------------------------------------------- +# THEN steps +# --------------------------------------------------------------------------- + + +@then("new_cov the exit code should be {code:d}") +def step_new_cov_exit_code(context: Context, code: int) -> None: + assert context.new_cov_result.exit_code == code, ( + f"Expected exit code {code}, got {context.new_cov_result.exit_code}. " + f"Output: {context.new_cov_result.output}" + ) + + +@then("new_cov the exit code should be nonzero") +def step_new_cov_exit_nonzero(context: Context) -> None: + assert context.new_cov_result.exit_code != 0, ( + f"Expected non-zero exit code, got {context.new_cov_result.exit_code}. " + f"Output: {context.new_cov_result.output}" + ) + + +@then('new_cov the output should contain "{text}"') +def step_new_cov_output_contains(context: Context, text: str) -> None: + output = context.new_cov_result.output + assert text in output, f"Expected '{text}' in output. Got:\n{output}" + + +@then("new_cov the returned object should be a PlanApplyService instance") +def step_new_cov_result_is_pas(context: Context) -> None: + assert context.new_cov_apply_result is context.new_cov_pas_instance + + +@then("new_cov PlanApplyService was constructed with the lifecycle service") +def step_new_cov_pas_called_with_lifecycle(context: Context) -> None: + context.new_cov_pas_class.assert_called_once_with( + lifecycle_service=context.new_cov_mock_lifecycle, + ) diff --git a/features/steps/plan_executor_new_coverage_steps.py b/features/steps/plan_executor_new_coverage_steps.py new file mode 100644 index 000000000..a34ec886c --- /dev/null +++ b/features/steps/plan_executor_new_coverage_steps.py @@ -0,0 +1,640 @@ +"""Step definitions for plan_executor_new_coverage.feature. + +Covers uncovered branches and lines in plan_executor.py: +- PlanExecutor.__init__ with None lifecycle_service +- has_runtime / changeset_store properties +- run_strategize guard branches (wrong phase, empty plan_id, exception path) +- _guard_execute branches (wrong phase, wrong state, None decision_root_id) +- run_execute routing (stub vs runtime) +- StrategizeStubActor / ExecuteStubActor with and without stream_callback +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock + +from behave import given, then, when +from ulid import ULID + +from cleveragents.application.services.plan_execution_context import ( + PlanExecutionContext, + RuntimeExecuteResult, +) +from cleveragents.application.services.plan_executor import ( + ExecuteResult, + ExecuteStubActor, + PlanExecutor, + StrategizeResult, + StrategizeStubActor, + StrategyDecision, +) +from cleveragents.core.exceptions import PlanError, ValidationError +from cleveragents.domain.models.core.plan import PlanPhase, ProcessingState +from cleveragents.tool.registry import ToolRegistry +from cleveragents.tool.runner import ToolRunner + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_plan_mock( + phase: PlanPhase = PlanPhase.STRATEGIZE, + state: ProcessingState = ProcessingState.QUEUED, + decision_root_id: str | None = None, + definition_of_done: str = "Step one\nStep two", + invariants: list | None = None, +) -> MagicMock: + """Create a mock Plan object with the given attributes.""" + plan = MagicMock() + plan.phase = phase + plan.state = state + plan.decision_root_id = decision_root_id + plan.definition_of_done = definition_of_done + plan.invariants = invariants or [] + plan.timestamps = MagicMock() + plan.error_details = {} + plan.changeset_id = None + plan.sandbox_refs = [] + plan.namespaced_name = "test/plan" + plan.identity = MagicMock() + return plan + + +def _make_lifecycle(**overrides: Any) -> MagicMock: + """Create a mock lifecycle service with default methods.""" + lc = MagicMock() + lc.start_strategize = MagicMock() + lc.complete_strategize = MagicMock() + lc.fail_strategize = MagicMock() + lc.start_execute = MagicMock() + lc.complete_execute = MagicMock() + lc.fail_execute = MagicMock() + lc._commit_plan = MagicMock() + for key, value in overrides.items(): + setattr(lc, key, value) + return lc + + +def _make_decisions(count: int = 2) -> list[StrategyDecision]: + root_id = str(ULID()) + decisions = [] + for i in range(count): + decisions.append( + StrategyDecision( + decision_id=root_id if i == 0 else str(ULID()), + step_text=f"Step {i + 1}", + sequence=i, + parent_id=root_id if i > 0 else None, + ) + ) + return decisions + + +# --------------------------------------------------------------------------- +# PlanExecutor __init__ validation +# --------------------------------------------------------------------------- + +@when("I construct a PlanExecutor with None lifecycle_service") +def step_construct_pe_none_lc(ctx: Any) -> None: + ctx.pe_error = None + try: + PlanExecutor(lifecycle_service=None) + except ValidationError as exc: + ctx.pe_error = exc + + +@then('a PE ValidationError should be raised containing "{text}"') +def step_assert_pe_validation_error_contains(ctx: Any, text: str) -> None: + assert ctx.pe_error is not None, "Expected a ValidationError but none was raised" + assert isinstance(ctx.pe_error, ValidationError), ( + f"Expected ValidationError, got {type(ctx.pe_error).__name__}" + ) + assert text in str(ctx.pe_error), ( + f"Expected '{text}' in error message, got: {ctx.pe_error}" + ) + + +@given("a mock plan lifecycle service") +def step_given_mock_lifecycle(ctx: Any) -> None: + ctx.lifecycle = _make_lifecycle() + ctx.plan_id = str(ULID()) + + +@when("I construct a PlanExecutor with the mock lifecycle service") +def step_construct_pe_with_lc(ctx: Any) -> None: + ctx.pe_error = None + try: + ctx.executor = PlanExecutor(lifecycle_service=ctx.lifecycle) + except Exception as exc: + ctx.pe_error = exc + + +@then("the PlanExecutor should be created without error") +def step_assert_no_error(ctx: Any) -> None: + assert ctx.pe_error is None, f"Unexpected error: {ctx.pe_error}" + assert ctx.executor is not None + + +# --------------------------------------------------------------------------- +# has_runtime property +# --------------------------------------------------------------------------- + +@given("a mock execution context") +def step_given_mock_exec_ctx(ctx: Any) -> None: + ctx.exec_ctx = PlanExecutionContext(plan_id=ctx.plan_id) + + +@when("I construct a PlanExecutor with the mock lifecycle service and execution context") +def step_construct_pe_with_ctx(ctx: Any) -> None: + ctx.executor = PlanExecutor( + lifecycle_service=ctx.lifecycle, + execution_context=ctx.exec_ctx, + ) + + +@then("the executor has_runtime property should be True") +def step_assert_has_runtime_true(ctx: Any) -> None: + assert ctx.executor.has_runtime is True + + +@then("the executor has_runtime property should be False") +def step_assert_has_runtime_false(ctx: Any) -> None: + assert ctx.executor.has_runtime is False + + +# --------------------------------------------------------------------------- +# changeset_store property +# --------------------------------------------------------------------------- + +@given("a mock execution context with a changeset store") +def step_given_exec_ctx_with_store(ctx: Any) -> None: + ctx.mock_store = MagicMock() + ctx.exec_ctx = MagicMock() + ctx.exec_ctx.changeset_store = ctx.mock_store + ctx.exec_ctx.decision_root_id = None + + +@then("the executor changeset_store should be the mock store") +def step_assert_changeset_store_is_mock(ctx: Any) -> None: + assert ctx.executor.changeset_store is ctx.mock_store + + +@then("the executor changeset_store should be None") +def step_assert_changeset_store_none(ctx: Any) -> None: + assert ctx.executor.changeset_store is None + + +# --------------------------------------------------------------------------- +# run_strategize guard: wrong phase +# --------------------------------------------------------------------------- + +@given("a PlanExecutor with the mock lifecycle service") +def step_given_pe_with_lc(ctx: Any) -> None: + ctx.executor = PlanExecutor(lifecycle_service=ctx.lifecycle) + + +@given("the lifecycle returns a plan in EXECUTE phase") +def step_lifecycle_returns_execute_phase(ctx: Any) -> None: + plan = _make_plan_mock(phase=PlanPhase.EXECUTE) + ctx.lifecycle.get_plan = MagicMock(return_value=plan) + + +@when("I attempt to run strategize on the executor") +def step_attempt_run_strategize(ctx: Any) -> None: + ctx.pe_error = None + try: + ctx.executor.run_strategize(ctx.plan_id) + except (PlanError, ValidationError) as exc: + ctx.pe_error = exc + + +@then('a PE PlanError should be raised containing "{text}"') +def step_assert_pe_plan_error_contains(ctx: Any, text: str) -> None: + assert ctx.pe_error is not None, "Expected a PlanError but none was raised" + assert isinstance(ctx.pe_error, PlanError), ( + f"Expected PlanError, got {type(ctx.pe_error).__name__}" + ) + assert text in str(ctx.pe_error), ( + f"Expected '{text}' in error message, got: {ctx.pe_error}" + ) + + +# --------------------------------------------------------------------------- +# run_strategize guard: empty plan_id +# --------------------------------------------------------------------------- + +@when("I attempt to run strategize with an empty plan_id") +def step_attempt_strategize_empty_plan_id(ctx: Any) -> None: + ctx.pe_error = None + try: + ctx.executor.run_strategize("") + except (ValidationError, PlanError) as exc: + ctx.pe_error = exc + + +# --------------------------------------------------------------------------- +# run_strategize exception handling +# --------------------------------------------------------------------------- + +@given("the lifecycle returns a plan in STRATEGIZE phase") +def step_lifecycle_returns_strategize_phase(ctx: Any) -> None: + plan = _make_plan_mock( + phase=PlanPhase.STRATEGIZE, + definition_of_done="Step one\nStep two", + ) + ctx.lifecycle.get_plan = MagicMock(return_value=plan) + ctx.mock_plan = plan + + +@given("the lifecycle start_strategize will raise a RuntimeError") +def step_lifecycle_start_strategize_raises(ctx: Any) -> None: + """Make the strategize actor itself raise inside the try block. + + start_strategize is called *before* the try, so we need to make + the actor.execute raise instead to hit the except branch (line 314). + """ + ctx._patch_strategize_actor_to_fail = True + + +@when("I attempt to run strategize and expect an exception") +def step_attempt_strategize_expect_exception(ctx: Any) -> None: + # If the actor should fail, patch it before running + if getattr(ctx, "_patch_strategize_actor_to_fail", False): + ctx.executor._strategize_actor.execute = MagicMock( + side_effect=RuntimeError("simulated actor failure") + ) + if getattr(ctx, "_patch_actor_to_fail", False): + ctx.executor._strategize_actor.execute = MagicMock( + side_effect=RuntimeError("simulated actor failure") + ) + ctx.pe_error = None + ctx.raised_exception = None + try: + ctx.executor.run_strategize(ctx.plan_id) + except Exception as exc: + ctx.pe_error = exc + ctx.raised_exception = exc + + +@then("fail_strategize should have been called with the plan_id") +def step_assert_fail_strategize_called(ctx: Any) -> None: + ctx.lifecycle.fail_strategize.assert_called() + call_args = ctx.lifecycle.fail_strategize.call_args + assert ctx.plan_id == call_args[0][0], ( + f"Expected plan_id={ctx.plan_id}, got {call_args[0][0]}" + ) + + +@then("the raised exception should be a RuntimeError") +def step_assert_raised_runtime_error(ctx: Any) -> None: + assert ctx.raised_exception is not None, "No exception was raised" + assert isinstance(ctx.raised_exception, RuntimeError), ( + f"Expected RuntimeError, got {type(ctx.raised_exception).__name__}" + ) + + +# --------------------------------------------------------------------------- +# run_strategize exception during actor execute (except branch line 314) +# --------------------------------------------------------------------------- + +@given("the lifecycle returns a plan in STRATEGIZE phase that fails during actor execute") +def step_lifecycle_strategize_fails_in_actor(ctx: Any) -> None: + """Set up lifecycle so start_strategize succeeds but actor.execute raises.""" + plan = _make_plan_mock( + phase=PlanPhase.STRATEGIZE, + definition_of_done="Step one\nStep two", + ) + ctx.lifecycle.get_plan = MagicMock(return_value=plan) + ctx.lifecycle.start_strategize = MagicMock() # succeeds + ctx.mock_plan = plan + ctx._patch_actor_to_fail = True + + +@then("the plan error_details should contain the exception_type") +def step_assert_error_details_exception_type(ctx: Any) -> None: + plan = ctx.lifecycle.get_plan.return_value + assert plan.error_details is not None + if isinstance(plan.error_details, dict): + assert "exception_type" in plan.error_details + + +# --------------------------------------------------------------------------- +# _guard_execute: wrong phase +# --------------------------------------------------------------------------- + +@given("the lifecycle returns a plan in STRATEGIZE phase for execute guard") +def step_lifecycle_returns_strategize_for_execute(ctx: Any) -> None: + plan = _make_plan_mock(phase=PlanPhase.STRATEGIZE) + ctx.lifecycle.get_plan = MagicMock(return_value=plan) + + +@when("I attempt to run execute on the executor") +def step_attempt_run_execute(ctx: Any) -> None: + ctx.pe_error = None + try: + ctx.executor.run_execute(ctx.plan_id) + except (PlanError, ValidationError) as exc: + ctx.pe_error = exc + + +# --------------------------------------------------------------------------- +# _guard_execute: wrong state +# --------------------------------------------------------------------------- + +@given("the lifecycle returns a plan in EXECUTE phase with PROCESSING state") +def step_lifecycle_returns_execute_processing(ctx: Any) -> None: + plan = _make_plan_mock( + phase=PlanPhase.EXECUTE, + state=ProcessingState.PROCESSING, + ) + ctx.lifecycle.get_plan = MagicMock(return_value=plan) + + +# --------------------------------------------------------------------------- +# _guard_execute: None decision_root_id +# --------------------------------------------------------------------------- + +@given("the lifecycle returns a plan in EXECUTE phase with QUEUED state but no decision root") +def step_lifecycle_returns_execute_queued_no_root(ctx: Any) -> None: + plan = _make_plan_mock( + phase=PlanPhase.EXECUTE, + state=ProcessingState.QUEUED, + decision_root_id=None, + ) + ctx.lifecycle.get_plan = MagicMock(return_value=plan) + + +# --------------------------------------------------------------------------- +# run_execute routing to stub +# --------------------------------------------------------------------------- + +@given("a PlanExecutor with the mock lifecycle service and no execution context") +def step_given_pe_no_ctx(ctx: Any) -> None: + ctx.executor = PlanExecutor(lifecycle_service=ctx.lifecycle) + + +@given("the lifecycle returns a fully valid plan for stub execution") +def step_lifecycle_returns_valid_plan_for_stub(ctx: Any) -> None: + root_id = str(ULID()) + plan = _make_plan_mock( + phase=PlanPhase.EXECUTE, + state=ProcessingState.QUEUED, + decision_root_id=root_id, + definition_of_done="Do thing one\nDo thing two", + ) + ctx.lifecycle.get_plan = MagicMock(return_value=plan) + ctx.mock_plan = plan + + +@when("I run execute on the executor") +def step_run_execute(ctx: Any) -> None: + ctx.pe_error = None + try: + ctx.result = ctx.executor.run_execute(ctx.plan_id) + except Exception as exc: + ctx.pe_error = exc + + +@then("the plan executor result should be an ExecuteResult") +def step_assert_execute_result_type(ctx: Any) -> None: + assert ctx.pe_error is None, f"Unexpected error: {ctx.pe_error}" + assert isinstance(ctx.result, ExecuteResult), ( + f"Expected ExecuteResult, got {type(ctx.result).__name__}" + ) + + +@then("complete_execute should have been called") +def step_assert_complete_execute_called(ctx: Any) -> None: + ctx.lifecycle.complete_execute.assert_called_once() + + +# --------------------------------------------------------------------------- +# run_execute routing to runtime +# --------------------------------------------------------------------------- + +@given("a PlanExecutor with the mock lifecycle service and execution context and tool runner") +def step_given_pe_with_ctx_and_runner(ctx: Any) -> None: + ctx.tool_runner = ToolRunner(registry=ToolRegistry()) + ctx.executor = PlanExecutor( + lifecycle_service=ctx.lifecycle, + tool_runner=ctx.tool_runner, + execution_context=ctx.exec_ctx, + ) + + +@given("the lifecycle returns a fully valid plan for runtime execution") +def step_lifecycle_returns_valid_plan_for_runtime(ctx: Any) -> None: + root_id = str(ULID()) + plan = _make_plan_mock( + phase=PlanPhase.EXECUTE, + state=ProcessingState.QUEUED, + decision_root_id=root_id, + definition_of_done="Do thing one\nDo thing two", + ) + ctx.lifecycle.get_plan = MagicMock(return_value=plan) + ctx.mock_plan = plan + + +@then("the plan executor result should be a RuntimeExecuteResult") +def step_assert_runtime_execute_result_type(ctx: Any) -> None: + assert ctx.pe_error is None, f"Unexpected error: {ctx.pe_error}" + assert isinstance(ctx.result, RuntimeExecuteResult), ( + f"Expected RuntimeExecuteResult, got {type(ctx.result).__name__}" + ) + + +# --------------------------------------------------------------------------- +# StrategizeStubActor with stream_callback +# --------------------------------------------------------------------------- + +@given("a stream event collector") +def step_given_stream_collector(ctx: Any) -> None: + ctx.stream_events = [] + ctx.stream_callback = lambda event_type, data: ctx.stream_events.append( + (event_type, data) + ) + + +@when("I execute the StrategizeStubActor with a plan_id and stream callback") +def step_exec_strategize_actor_with_cb(ctx: Any) -> None: + actor = StrategizeStubActor() + plan_id = str(ULID()) + ctx.actor_plan_id = plan_id + ctx.strategize_result = actor.execute( + plan_id=plan_id, + definition_of_done="Step A\nStep B", + stream_callback=ctx.stream_callback, + ) + + +@then('the collector should contain a "{event_type}" event') +def step_assert_collector_contains_event(ctx: Any, event_type: str) -> None: + event_types = [e[0] for e in ctx.stream_events] + assert event_type in event_types, ( + f"Expected '{event_type}' in events, got: {event_types}" + ) + + +@when("I execute the StrategizeStubActor with a plan_id and no callback") +def step_exec_strategize_actor_no_cb(ctx: Any) -> None: + actor = StrategizeStubActor() + plan_id = str(ULID()) + ctx.strategize_result = actor.execute( + plan_id=plan_id, + definition_of_done="Step A\nStep B", + stream_callback=None, + ) + + +@then("the strategize result should have a valid decision_root_id") +def step_assert_strategize_result_root_id(ctx: Any) -> None: + assert ctx.strategize_result.decision_root_id + assert len(ctx.strategize_result.decision_root_id) == 26 + + +@then("the strategize result should have at least one decision") +def step_assert_strategize_result_has_decisions(ctx: Any) -> None: + assert len(ctx.strategize_result.decisions) >= 1 + + +# --------------------------------------------------------------------------- +# ExecuteStubActor with stream_callback +# --------------------------------------------------------------------------- + +@given("a list of two strategy decisions") +def step_given_two_decisions(ctx: Any) -> None: + ctx.decisions = _make_decisions(2) + + +@when("I execute the ExecuteStubActor with the decisions and stream callback") +def step_exec_execute_actor_with_cb(ctx: Any) -> None: + actor = ExecuteStubActor() + plan_id = str(ULID()) + ctx.actor_plan_id = plan_id + ctx.execute_result = actor.execute( + plan_id=plan_id, + decisions=ctx.decisions, + stream_callback=ctx.stream_callback, + ) + + +@then('the collector should contain an "{event_type}" event') +def step_assert_collector_contains_event_an(ctx: Any, event_type: str) -> None: + event_types = [e[0] for e in ctx.stream_events] + assert event_type in event_types, ( + f"Expected '{event_type}' in events, got: {event_types}" + ) + + +@when("I execute the ExecuteStubActor with the decisions and no callback") +def step_exec_execute_actor_no_cb(ctx: Any) -> None: + actor = ExecuteStubActor() + plan_id = str(ULID()) + ctx.execute_result = actor.execute( + plan_id=plan_id, + decisions=ctx.decisions, + stream_callback=None, + ) + + +@then("the execute result should have a valid changeset_id") +def step_assert_execute_result_changeset_id(ctx: Any) -> None: + assert ctx.execute_result.changeset_id + assert len(ctx.execute_result.changeset_id) == 26 + + +@then("the execute result tool_calls_count should be zero") +def step_assert_execute_result_tool_calls_zero(ctx: Any) -> None: + assert ctx.execute_result.tool_calls_count == 0 + + +# --------------------------------------------------------------------------- +# run_strategize happy path with stream_callback +# --------------------------------------------------------------------------- + +@when("I run strategize with the stream callback") +def step_run_strategize_with_callback(ctx: Any) -> None: + ctx.pe_error = None + try: + ctx.strategize_result = ctx.executor.run_strategize( + ctx.plan_id, + stream_callback=ctx.stream_callback, + ) + except Exception as exc: + ctx.pe_error = exc + + +@then("the strategize result should have decisions") +def step_assert_result_has_decisions(ctx: Any) -> None: + assert ctx.pe_error is None, f"Unexpected error: {ctx.pe_error}" + assert len(ctx.strategize_result.decisions) >= 1 + + +@then("complete_strategize should have been called") +def step_assert_complete_strategize_called(ctx: Any) -> None: + ctx.lifecycle.complete_strategize.assert_called_once() + + +# --------------------------------------------------------------------------- +# PlanExecutor with execution context (for run_strategize + context scenario) +# --------------------------------------------------------------------------- + +@given("a PlanExecutor with the mock lifecycle and execution context") +def step_given_pe_with_lc_and_ctx(ctx: Any) -> None: + ctx.executor = PlanExecutor( + lifecycle_service=ctx.lifecycle, + execution_context=ctx.exec_ctx, + ) + + +# --------------------------------------------------------------------------- +# run_strategize sets execution_context.decision_root_id +# --------------------------------------------------------------------------- + +@when("I run strategize successfully") +def step_run_strategize_successfully(ctx: Any) -> None: + ctx.pe_error = None + try: + ctx.strategize_result = ctx.executor.run_strategize(ctx.plan_id) + except Exception as exc: + ctx.pe_error = exc + + +@then("the execution context decision_root_id should be set") +def step_assert_exec_ctx_decision_root_id_set(ctx: Any) -> None: + assert ctx.pe_error is None, f"Unexpected error: {ctx.pe_error}" + assert ctx.exec_ctx.decision_root_id is not None + assert ctx.exec_ctx.decision_root_id == ctx.strategize_result.decision_root_id + + +# --------------------------------------------------------------------------- +# run_execute stub exception path +# --------------------------------------------------------------------------- + +@given("the stub execute actor is patched to raise an error") +def step_patch_stub_execute_to_fail(ctx: Any) -> None: + ctx.executor._execute_actor.execute = MagicMock( + side_effect=RuntimeError("simulated stub execution failure") + ) + + +@when("I attempt to run execute and expect an exception") +def step_attempt_run_execute_expect_exception(ctx: Any) -> None: + ctx.pe_error = None + ctx.raised_exception = None + try: + ctx.result = ctx.executor.run_execute(ctx.plan_id) + except Exception as exc: + ctx.pe_error = exc + ctx.raised_exception = exc + + +@then("fail_execute should have been called with the plan_id") +def step_assert_fail_execute_called(ctx: Any) -> None: + ctx.lifecycle.fail_execute.assert_called() + call_args = ctx.lifecycle.fail_execute.call_args + assert ctx.plan_id == call_args[0][0], ( + f"Expected plan_id={ctx.plan_id}, got {call_args[0][0]}" + )