From 55aee7cf226b5b7aa2e984addddfb92c7dfef335 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Tue, 24 Feb 2026 11:15:09 -0500 Subject: [PATCH] fix(test): commit after each add_skill to prevent session GC rollback, and improved coverage. The step_register_skills_table step called add_skill in a loop but only committed once at the end. Because SkillRepository.create() obtains a new session per call and only flushes (never commits), the intermediate sessions could be garbage-collected before the final commit, rolling back their transactions on the shared SQLite :memory: connection. Moving _commit_pending inside the loop ensures each skill is durably committed before the next session is created. ISSUES CLOSED: #418 --- features/config_service_coverage.feature | 277 ++++++ features/correction_service_coverage.feature | 342 ++++++++ .../database_models_missing_coverage.feature | 77 ++ .../plan_cli_cancel_revert_coverage.feature | 91 ++ features/plan_executor_coverage.feature | 158 ++++ ...cycle_service_persistence_coverage.feature | 24 + features/repl_coverage.feature | 309 +++++++ .../resource_handler_service_coverage.feature | 151 ++++ features/skill_cli_coverage.feature | 33 + .../steps/config_service_coverage_steps.py | 803 ++++++++++++++++++ .../correction_service_coverage_steps.py | 769 +++++++++++++++++ .../database_models_missing_coverage_steps.py | 408 +++++++++ .../plan_cli_cancel_revert_coverage_steps.py | 359 ++++++++ .../steps/plan_executor_coverage_steps.py | 612 +++++++++++++ ...ycle_service_persistence_coverage_steps.py | 155 ++++ features/steps/repl_coverage_steps.py | 736 ++++++++++++++++ ...resource_handler_service_coverage_steps.py | 514 +++++++++++ features/steps/skill_cli_coverage_steps.py | 201 +++++ features/steps/skill_registry_steps.py | 2 +- 19 files changed, 6020 insertions(+), 1 deletion(-) create mode 100644 features/config_service_coverage.feature create mode 100644 features/correction_service_coverage.feature create mode 100644 features/database_models_missing_coverage.feature create mode 100644 features/plan_cli_cancel_revert_coverage.feature create mode 100644 features/plan_executor_coverage.feature create mode 100644 features/plan_lifecycle_service_persistence_coverage.feature create mode 100644 features/repl_coverage.feature create mode 100644 features/resource_handler_service_coverage.feature create mode 100644 features/skill_cli_coverage.feature create mode 100644 features/steps/config_service_coverage_steps.py create mode 100644 features/steps/correction_service_coverage_steps.py create mode 100644 features/steps/database_models_missing_coverage_steps.py create mode 100644 features/steps/plan_cli_cancel_revert_coverage_steps.py create mode 100644 features/steps/plan_executor_coverage_steps.py create mode 100644 features/steps/plan_lifecycle_service_persistence_coverage_steps.py create mode 100644 features/steps/repl_coverage_steps.py create mode 100644 features/steps/resource_handler_service_coverage_steps.py create mode 100644 features/steps/skill_cli_coverage_steps.py diff --git a/features/config_service_coverage.feature b/features/config_service_coverage.feature new file mode 100644 index 000000000..4daf78249 --- /dev/null +++ b/features/config_service_coverage.feature @@ -0,0 +1,277 @@ +Feature: ConfigService full coverage + As a developer + I want every line of config_service.py tested + So that the module has complete test coverage + + # ---------- ConfigLevel enum ---------- + + Scenario: ConfigLevel enum has five precedence levels + Then ConfigLevel should have values CLI_FLAG ENV_VAR PROJECT GLOBAL DEFAULT + + # ---------- ConfigEntry and ResolvedValue dataclasses ---------- + + Scenario: ConfigEntry dataclass stores all metadata fields + Given a ConfigEntry with key "test.key" and type str + Then the ConfigEntry fields should match the provided values + + Scenario: ResolvedValue dataclass stores resolution result + Given a ResolvedValue with key "test.key" and source DEFAULT + Then the ResolvedValue fields should match including an empty chain by default + + # ---------- _env_name helper ---------- + + Scenario: _env_name builds uppercase CLEVERAGENTS env var name + When I call _env_name with section "core" and key "log_level" + Then the env name should be "CLEVERAGENTS_CORE_LOG_LEVEL" + + # ---------- _register and _build_catalog ---------- + + Scenario: _register adds an entry to the global registry + When I register a custom key "custom.test_key" with type int and default 42 + Then the registry should contain "custom.test_key" with default 42 + + Scenario: _register uses explicit env_var when provided + When I register a key "custom.explicit_env" with explicit env_var "MY_CUSTOM_VAR" + Then the entry "custom.explicit_env" should have env_var "MY_CUSTOM_VAR" + + Scenario: _build_catalog populates all expected config sections + Then the registry should contain keys from sections core plan provider sandbox context index + + Scenario: _build_catalog registers the expected number of keys + Then the registry should contain at least 20 keys + + # ---------- ConfigService instantiation ---------- + + Scenario: ConfigService uses custom config_dir and config_path + Given a temporary directory for config service + When I create a ConfigService with custom paths + Then the service should use the custom config directory and path + + Scenario: ConfigService uses defaults when no paths provided + When I create a ConfigService with default paths + Then the service config path should end with ".cleveragents/config.toml" + + # ---------- registry(), get_entry(), registered_keys() ---------- + + Scenario: registry returns a copy of all registered entries + When I call ConfigService.registry + Then it should return a dict with the same keys as the internal registry + + Scenario: get_entry returns the entry for a known key + When I call ConfigService.get_entry with "core.log_level" + Then it should return a ConfigEntry with key "core.log_level" + + Scenario: get_entry returns None for an unknown key + When I call ConfigService.get_entry with "nonexistent.key" + Then it should return None + + Scenario: registered_keys returns a sorted list of all keys + When I call ConfigService.registered_keys + Then it should return a sorted list containing "core.log_level" + + # ---------- read_config ---------- + + Scenario: read_config returns empty dict when file does not exist + Given a temporary directory for config service + When I create a ConfigService pointing to a missing config file + Then read_config should return an empty dict + + Scenario: read_config parses an existing TOML file + Given a temporary directory for config service + And a TOML config file with key "core.log_level" set to "DEBUG" + When I create a ConfigService pointing to that TOML file + Then read_config should return a dict with "core.log_level" equal to "DEBUG" + + # ---------- write_config ---------- + + Scenario: write_config creates directories and writes a new TOML file + Given a temporary directory for config service + When I create a ConfigService with a nested non-existent config directory + And I call write_config with key "greeting" value "hello" + Then the TOML file should exist and contain key "greeting" with value "hello" + + Scenario: write_config merges data into an existing TOML file + Given a temporary directory for config service + And a TOML config file with key "existing" set to "value1" + When I create a ConfigService pointing to that TOML file + And I call write_config with key "new_key" value "value2" + Then read_config should return a dict containing both "existing" and "new_key" + + # ---------- set_value ---------- + + Scenario: set_value persists a single key-value pair + Given a temporary directory for config service + When I create a ConfigService with custom paths + And I call set_value with key "mykey" and value "myval" + Then read_config should return a dict with "mykey" equal to "myval" + + # ---------- validate_key ---------- + + Scenario: validate_key returns entry for a known key + When I call validate_key with "core.log_level" + Then it should return the ConfigEntry for "core.log_level" without error + + Scenario: validate_key raises ValueError for an unknown key + When I call validate_key with "totally.unknown" + Then a ValueError should be raised mentioning "Unknown configuration key" + + # ---------- validate_type ---------- + + Scenario: validate_type returns value unchanged when type already matches + When I call validate_type with key "core.log_level" and value "INFO" + Then it should return "INFO" unchanged + + Scenario: validate_type coerces a string to int + When I call validate_type with key "core.server_port" and string value "9090" + Then it should return the integer 9090 + + Scenario: validate_type coerces a string to float + When I call validate_type with key "provider.temperature" and string value "0.5" + Then it should return the float 0.5 + + Scenario: validate_type coerces non-string to str + When I call validate_type with key "core.log_level" and integer value 123 + Then it should return the string "123" + + Scenario: validate_type coerces "true" string to bool True + When I call validate_type with key "core.debug_enabled" and string value "true" + Then it should return boolean True + + Scenario: validate_type coerces "1" string to bool True + When I call validate_type with key "core.debug_enabled" and string value "1" + Then it should return boolean True + + Scenario: validate_type coerces "yes" string to bool True + When I call validate_type with key "core.debug_enabled" and string value "yes" + Then it should return boolean True + + Scenario: validate_type coerces "false" string to bool False + When I call validate_type with key "core.debug_enabled" and string value "false" + Then it should return boolean False + + Scenario: validate_type coerces "0" string to bool False + When I call validate_type with key "core.debug_enabled" and string value "0" + Then it should return boolean False + + Scenario: validate_type coerces "no" string to bool False + When I call validate_type with key "core.debug_enabled" and string value "no" + Then it should return boolean False + + Scenario: validate_type raises TypeError for invalid bool string + When I call validate_type with key "core.debug_enabled" and string value "maybe" + Then a TypeError should be raised mentioning "Cannot convert" + + Scenario: validate_type raises TypeError for non-coercible int value + When I call validate_type with key "core.server_port" and string value "not_a_number" + Then a TypeError should be raised mentioning "Type mismatch" + + Scenario: validate_type raises TypeError for non-bool non-string to bool key + When I call validate_type with key "core.debug_enabled" and a list value + Then a TypeError should be raised mentioning "Type mismatch" + + Scenario: validate_type raises ValueError for completely unknown key + When I call validate_type with unknown key "fake.unknown" and value "x" + Then a ValueError should be raised mentioning "Cannot validate type for unknown key" + + # ---------- resolve: five-level chain ---------- + + Scenario: resolve returns default value when no overrides exist + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + When I resolve key "core.log_level" with no overrides + Then the resolved value should be "INFO" from source DEFAULT + + Scenario: resolve returns CLI flag value when provided + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + When I resolve key "core.log_level" with cli_value "TRACE" + Then the resolved value should be "TRACE" from source CLI_FLAG + + Scenario: resolve returns environment variable when no CLI flag + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + And the env var "CLEVERAGENTS_CORE_LOG_LEVEL" is injected with "WARNING" + When I resolve key "core.log_level" with no overrides + Then the resolved value should be "WARNING" from source ENV_VAR + + Scenario: CLI flag takes precedence over environment variable + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + And the env var "CLEVERAGENTS_CORE_LOG_LEVEL" is injected with "WARNING" + When I resolve key "core.log_level" with cli_value "TRACE" + Then the resolved value should be "TRACE" from source CLI_FLAG + + Scenario: resolve returns project-scoped value when set + Given a temporary directory for config service + And a ConfigService with project-scoped config for key "core.log_level" value "PROJECT_DEBUG" under project "myproj" + When I resolve key "core.log_level" with project_name "myproj" + Then the resolved value should be "PROJECT_DEBUG" from source PROJECT + + Scenario: resolve returns global config value when no higher overrides + Given a temporary directory for config service + And a ConfigService with global config key "core.log_level" set to "GLOBAL_WARN" + When I resolve key "core.log_level" with no overrides + Then the resolved value should be "GLOBAL_WARN" from source GLOBAL + + Scenario: resolve populates chain when verbose is True + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + When I resolve key "core.log_level" with verbose True and no overrides + Then the resolved chain should have 5 entries covering all levels + + Scenario: resolve verbose chain includes env_name for ENV_VAR level + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + When I resolve key "core.log_level" with verbose True and no overrides + Then the verbose chain ENV_VAR entry should include env_name "CLEVERAGENTS_CORE_LOG_LEVEL" + + Scenario: resolve verbose chain includes path for GLOBAL level + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + When I resolve key "core.log_level" with verbose True and no overrides + Then the verbose chain GLOBAL entry should include the config file path + + Scenario: resolve verbose chain shows project value when project-scoped wins + Given a temporary directory for config service + And a ConfigService with project-scoped config for key "core.log_level" value "PROJECT_DEBUG" under project "myproj" + When I resolve key "core.log_level" with project_name "myproj" and verbose True + Then the resolved value should be "PROJECT_DEBUG" from source PROJECT + And the verbose chain PROJECT entry should have value "PROJECT_DEBUG" + + Scenario: resolve verbose chain shows global value when global config wins + Given a temporary directory for config service + And a ConfigService with global config key "core.log_level" set to "GLOBAL_WARN" + When I resolve key "core.log_level" with verbose True and no overrides + Then the resolved value should be "GLOBAL_WARN" from source GLOBAL + And the verbose chain GLOBAL entry should have value "GLOBAL_WARN" + + Scenario: resolve skips project scope for non-scopable keys + Given a temporary directory for config service + And a ConfigService with project-scoped config for key "core.database_url" value "pg://proj" under project "myproj" + When I resolve key "core.database_url" with project_name "myproj" + Then the resolved value should not be "pg://proj" + And the resolved source should be DEFAULT or GLOBAL + + # ---------- env_var_for_key ---------- + + Scenario: env_var_for_key returns the env var name for a valid key + When I call env_var_for_key with "core.log_level" + Then it should return "CLEVERAGENTS_CORE_LOG_LEVEL" + + Scenario: env_var_for_key raises ValueError for an unknown key + When I call env_var_for_key with "nonexistent.key" + Then a ValueError should be raised mentioning "Unknown configuration key" + + # ---------- resolve_all ---------- + + Scenario: resolve_all returns a resolved value for every registered key + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + When I call resolve_all with no overrides + Then the result should contain an entry for every registered key + + Scenario: resolve_all applies CLI overrides to matching keys + Given a temporary directory for config service + And a ConfigService with empty config for resolve tests + When I call resolve_all with cli_override "core.log_level" set to "FATAL" + Then the resolve_all result for "core.log_level" should have value "FATAL" and source CLI_FLAG diff --git a/features/correction_service_coverage.feature b/features/correction_service_coverage.feature new file mode 100644 index 000000000..664499188 --- /dev/null +++ b/features/correction_service_coverage.feature @@ -0,0 +1,342 @@ +Feature: CorrectionService full coverage + As a developer maintaining CorrectionService + I want every line and branch fully tested + So that correction_service.py achieves 100% coverage + + Background: + Given a coverage test correction service + + # ------------------------------------------------------------------- + # __init__ + # ------------------------------------------------------------------- + + Scenario: Service initializes with empty internal dictionaries + Then the coverage service corrections dict should be empty + And the coverage service impacts dict should be empty + And the coverage service attempts dict should be empty + And the coverage service results dict should be empty + + # ------------------------------------------------------------------- + # request_correction – happy paths + # ------------------------------------------------------------------- + + Scenario: Create a revert correction with defaults + When I cov-request a correction for plan "P1" targeting "D1" in revert mode + Then the cov-request should be stored in the service + And the cov-request plan_id should be "P1" + And the cov-request target_decision_id should be "D1" + And the cov-request mode should be "revert" + And the cov-request status should be "pending" + And the cov-request dry_run should be false + And the cov-request guidance should be empty + + Scenario: Create an append correction with guidance and dry_run + When I cov-request a correction for plan "P2" targeting "D2" in append mode with guidance "fix things" and dry_run + Then the cov-request should be stored in the service + And the cov-request plan_id should be "P2" + And the cov-request target_decision_id should be "D2" + And the cov-request mode should be "append" + And the cov-request dry_run should be true + And the cov-request guidance should be "fix things" + + # ------------------------------------------------------------------- + # request_correction – validation errors + # ------------------------------------------------------------------- + + Scenario: Empty plan_id raises ValidationError + When I cov-attempt to create a correction with empty plan_id and target "D1" + Then a cov ValidationError should have been raised + + Scenario: Whitespace-only plan_id raises ValidationError + When I cov-attempt to create a correction with plan_id " " and target "D1" + Then a cov ValidationError should have been raised + + Scenario: Empty target_decision_id raises ValidationError + When I cov-attempt to create a correction with plan_id "P1" and empty target + Then a cov ValidationError should have been raised + + Scenario: Whitespace-only target_decision_id raises ValidationError + When I cov-attempt to create a correction with plan_id "P1" and target " " + Then a cov ValidationError should have been raised + + # ------------------------------------------------------------------- + # analyze_impact – BFS traversal + # ------------------------------------------------------------------- + + Scenario: Analyze impact with no decision tree yields single-node result + Given a cov-stored revert correction for plan "P1" targeting "ROOT" + When I cov-analyze impact with an empty tree + Then the cov-impact affected decisions should be "ROOT" + And the cov-impact risk level should be "low" + And the cov-impact rollback tier should be "full" + And the cov-impact estimated cost should be 1.5 + And the cov-impact artifacts to archive should be "ROOT.artifact" + And the cov-impact affected files should be "ROOT.py" + + Scenario: Analyze impact with tree having children + Given a cov-stored revert correction for plan "P1" targeting "A" + When I cov-analyze impact with adjacency "A->B,C;B->D" + Then the cov-impact affected decisions should be "A,B,C,D" + And the cov-impact risk level should be "medium" + And the cov-impact estimated cost should be 6.0 + And the cov-impact affected decisions count should be 4 + + Scenario: Analyze impact for append mode sets rollback tier to append_only + Given a cov-stored append correction for plan "P1" targeting "X" + When I cov-analyze impact with an empty tree + Then the cov-impact rollback tier should be "append_only" + + Scenario: Analyze impact for nonexistent correction raises ResourceNotFoundError + When I cov-attempt to analyze impact for correction id "BOGUS_ID" + Then a cov ResourceNotFoundError should have been raised + + Scenario: Analyze impact sets status to analyzing + Given a cov-stored revert correction for plan "P1" targeting "N1" + When I cov-analyze impact with an empty tree + Then the cov-stored correction status should be "analyzing" + + # ------------------------------------------------------------------- + # generate_dry_run_report + # ------------------------------------------------------------------- + + Scenario: Dry-run report for low-risk revert with single decision + Given a cov-stored revert correction for plan "P1" targeting "SOLO" + When I cov-generate dry-run report with an empty tree + Then the cov dry-run mode should be "revert" + And the cov dry-run warnings should be empty + And the cov dry-run decisions to invalidate should be "SOLO" + And the cov dry-run estimated recompute time should be 2.0 + + Scenario: Dry-run report for medium-risk revert with multiple decisions + Given a cov-stored revert correction for plan "P1" targeting "R" + When I cov-generate dry-run report with adjacency "R->C1,C2,C3,C4" + Then the cov dry-run warnings should contain "Medium risk" + And the cov dry-run warnings should contain "Revert will invalidate 5 decisions" + And the cov dry-run decisions to invalidate count should be 5 + And the cov dry-run estimated recompute time should be 10.0 + + Scenario: Dry-run report for high-risk revert + Given a cov-stored revert correction for plan "P1" targeting "H" + When I cov-generate dry-run report with a chain of 12 nodes rooted at "H" + Then the cov dry-run warnings should contain "High risk" + And the cov dry-run warnings should contain "Revert will invalidate 12 decisions" + And the cov dry-run decisions to invalidate count should be 12 + + Scenario: Dry-run report for append mode has empty decisions_to_invalidate + Given a cov-stored append correction for plan "P1" targeting "APP" + When I cov-generate dry-run report with an empty tree + Then the cov dry-run mode should be "append" + And the cov dry-run decisions to invalidate should be empty list + And the cov dry-run warnings should be empty + + Scenario: Dry-run report for nonexistent correction raises error + When I cov-attempt to generate dry-run report for correction id "NO_SUCH" + Then a cov ResourceNotFoundError should have been raised + + # ------------------------------------------------------------------- + # execute_revert + # ------------------------------------------------------------------- + + Scenario: Execute revert on a pending correction succeeds + Given a cov-stored revert correction for plan "P1" targeting "T1" + When I cov-execute revert with adjacency "T1->T2" + Then the cov revert result status should be "applied" + And the cov revert result reverted decisions should include "T1" + And the cov revert result reverted decisions should include "T2" + And the cov revert result archived artifacts should include "T1.artifact" + And the cov revert result archived artifacts should include "T2.artifact" + And the cov-stored correction status should be "applied" + And the cov attempts for the stored correction should have 1 entry + And the cov first attempt should be successful + And the cov first attempt completed_at should be set + + Scenario: Execute revert with empty tree marks only the target + Given a cov-stored revert correction for plan "P1" targeting "ONLY" + When I cov-execute revert with empty tree + Then the cov revert result status should be "applied" + And the cov revert result reverted decisions should include "ONLY" + + Scenario: Execute revert on already-applied correction raises ValidationError + Given a cov-stored revert correction for plan "P1" targeting "R1" + And the cov-stored correction has been executed via revert with empty tree + When I cov-attempt to execute revert on the stored correction + Then a cov ValidationError should have been raised + + # ------------------------------------------------------------------- + # execute_append + # ------------------------------------------------------------------- + + Scenario: Execute append on a pending correction succeeds + Given a cov-stored append correction for plan "P1" targeting "A1" + When I cov-execute append for the stored correction + Then the cov append result status should be "applied" + And the cov append result spawned_child_plan_id should not be none + And the cov append result new_decisions should not be empty + And the cov-stored correction status should be "applied" + And the cov attempts for the stored correction should have 1 entry + And the cov first attempt should be successful + And the cov first attempt details should contain spawned_child_plan_id + + Scenario: Execute append on already-applied correction raises ValidationError + Given a cov-stored append correction for plan "P1" targeting "A2" + And the cov-stored correction has been executed via append + When I cov-attempt to execute append on the stored correction + Then a cov ValidationError should have been raised + + # ------------------------------------------------------------------- + # execute_correction (dispatch) + # ------------------------------------------------------------------- + + Scenario: Dispatch routes revert mode to execute_revert + Given a cov-stored revert correction for plan "P1" targeting "DR" + When I cov-dispatch execute correction with adjacency "DR->DR2" + Then the cov dispatch result status should be "applied" + And the cov dispatch result reverted decisions should include "DR" + And the cov dispatch result reverted decisions should include "DR2" + + Scenario: Dispatch routes append mode to execute_append + Given a cov-stored append correction for plan "P1" targeting "DA" + When I cov-dispatch execute correction with no tree + Then the cov dispatch result status should be "applied" + And the cov dispatch result spawned_child_plan_id should not be none + + # ------------------------------------------------------------------- + # get_correction + # ------------------------------------------------------------------- + + Scenario: Get correction by id returns correct request + Given a cov-stored revert correction for plan "PG" targeting "DG" + When I cov-get the stored correction by id + Then the cov-retrieved correction plan_id should be "PG" + And the cov-retrieved correction target_decision_id should be "DG" + + Scenario: Get correction for nonexistent id raises ResourceNotFoundError + When I cov-attempt to get correction with id "MISSING" + Then a cov ResourceNotFoundError should have been raised + + # ------------------------------------------------------------------- + # list_corrections + # ------------------------------------------------------------------- + + Scenario: List all corrections without filter + Given a cov-stored revert correction for plan "PA" targeting "DA" + And a cov-stored revert correction for plan "PB" targeting "DB" + When I cov-list all corrections without filter + Then the cov corrections list should have at least 2 items + + Scenario: List corrections filtered by plan_id + Given a cov-stored revert correction for plan "PX" targeting "DX" + Given a cov-stored append correction for plan "PY" targeting "DY" + When I cov-list corrections for plan "PX" + Then the cov corrections list should have 1 item + + Scenario: List corrections filtered by plan_id with no matches + When I cov-list corrections for plan "NONEXISTENT_PLAN" + Then the cov corrections list should have 0 items + + # ------------------------------------------------------------------- + # list_attempts + # ------------------------------------------------------------------- + + Scenario: List attempts before any execution returns empty + Given a cov-stored revert correction for plan "P1" targeting "D1" + When I cov-list attempts for the stored correction + Then the cov attempts list should have 0 items + + Scenario: List attempts after execution returns the attempt + Given a cov-stored revert correction for plan "P1" targeting "D1" + And the cov-stored correction has been executed via revert with empty tree + When I cov-list attempts for the stored correction + Then the cov attempts list should have 1 item + + Scenario: List attempts for nonexistent correction raises error + When I cov-attempt to list attempts for correction id "NOPE" + Then a cov ResourceNotFoundError should have been raised + + # ------------------------------------------------------------------- + # cancel_correction + # ------------------------------------------------------------------- + + Scenario: Cancel a pending correction succeeds + Given a cov-stored revert correction for plan "P1" targeting "D1" + When I cov-cancel the stored correction + Then the cov-stored correction status should be "cancelled" + + Scenario: Cancel an analyzing correction succeeds + Given a cov-stored revert correction for plan "P1" targeting "D1" + And the cov-stored correction has been analyzed with empty tree + When I cov-cancel the stored correction + Then the cov-stored correction status should be "cancelled" + + Scenario: Cancel an already-applied correction raises ValidationError + Given a cov-stored revert correction for plan "P1" targeting "D1" + And the cov-stored correction has been executed via revert with empty tree + When I cov-attempt to cancel the stored correction + Then a cov ValidationError should have been raised + + Scenario: Cancel an already-cancelled correction raises ValidationError + Given a cov-stored revert correction for plan "P1" targeting "D1" + When I cov-cancel the stored correction + And I cov-attempt to cancel the stored correction + Then a cov ValidationError should have been raised + + # ------------------------------------------------------------------- + # _classify_risk (static) + # ------------------------------------------------------------------- + + Scenario Outline: Risk classification boundary values + Then cov classifying risk for affected returns "" + + Examples: + | count | expected | + | 0 | low | + | 1 | low | + | 3 | low | + | 4 | medium | + | 10 | medium | + | 11 | high | + | 50 | high | + + # ------------------------------------------------------------------- + # _compute_affected_subtree (static) + # ------------------------------------------------------------------- + + Scenario: Compute affected subtree with empty tree returns only root + When I cov-compute subtree for "ROOT" with empty tree + Then the cov subtree should be "ROOT" + + Scenario: Compute affected subtree with linear chain + When I cov-compute subtree for "A" with adjacency "A->B;B->C;C->D" + Then the cov subtree should be "A,B,C,D" + And the cov subtree count should be 4 + + Scenario: Compute affected subtree with branching tree + When I cov-compute subtree for "R" with adjacency "R->L,M;L->LL,LR" + Then the cov subtree should be "R,L,M,LL,LR" + + Scenario: Compute affected subtree starting at leaf node + When I cov-compute subtree for "LEAF" with adjacency "ROOT->LEAF" + Then the cov subtree should be "LEAF" + And the cov subtree count should be 1 + + # ------------------------------------------------------------------- + # _assert_executable internal helper (via execute_revert / execute_append) + # ------------------------------------------------------------------- + + Scenario: Execute revert on FAILED correction raises ValidationError + Given a cov-stored revert correction for plan "P1" targeting "F1" + And the cov-stored correction status is forced to "failed" + When I cov-attempt to execute revert on the stored correction + Then a cov ValidationError should have been raised + + Scenario: Execute append on EXECUTING correction raises ValidationError + Given a cov-stored append correction for plan "P1" targeting "E1" + And the cov-stored correction status is forced to "executing" + When I cov-attempt to execute append on the stored correction + Then a cov ValidationError should have been raised + + Scenario: Execute append on CANCELLED correction raises ValidationError + Given a cov-stored append correction for plan "P1" targeting "C1" + And the cov-stored correction status is forced to "cancelled" + When I cov-attempt to execute append on the stored correction + Then a cov ValidationError should have been raised diff --git a/features/database_models_missing_coverage.feature b/features/database_models_missing_coverage.feature new file mode 100644 index 000000000..caaf7e582 --- /dev/null +++ b/features/database_models_missing_coverage.feature @@ -0,0 +1,77 @@ +@phase1 @database @models @missing_coverage +Feature: Database models missing coverage for lines 1666, 1921, 2131-2132 + As a developer + I want to exercise the remaining uncovered lines in database models + So that code coverage reaches 100% + + Background: + Given the missing coverage database is ready + And a missing coverage database session is open + + # ------------------------------------------------------------------- + # ToolModel.from_domain() with a dict (covers line 1666) + # ------------------------------------------------------------------- + + @tool @from_domain @dict_input + Scenario: ToolModel.from_domain creates model from a plain dict + Given a tool definition provided as a plain dict + When the tool dict is converted to a ToolModel via from_domain + Then the ToolModel should have the correct name from the dict + And the ToolModel should have the correct description from the dict + And the ToolModel should have the correct tool_type from the dict + And the ToolModel should have the correct source from the dict + + @tool @from_domain @dict_input @resource_bindings + Scenario: ToolModel.from_domain populates resource bindings from dict input + Given a tool definition provided as a plain dict with resource bindings + When the tool dict with bindings is converted to a ToolModel via from_domain + Then the ToolModel should have resource binding child records + And each resource binding should have the correct slot_name + And each resource binding should have the correct resource_type + + @tool @from_domain @dict_input @namespace + Scenario: ToolModel.from_domain correctly splits namespaced name from dict + Given a tool definition dict with a namespaced name + When the namespaced tool dict is converted to a ToolModel via from_domain + Then the ToolModel namespace should be extracted from the dict name + And the ToolModel short_name should be extracted from the dict name + + # ------------------------------------------------------------------- + # SessionModel.from_domain() with messages (covers line 1921) + # ------------------------------------------------------------------- + + @session @from_domain @messages + Scenario: SessionModel.from_domain populates message child models + Given a Session domain object with two messages + When the Session domain object is converted to a SessionModel via from_domain + Then the SessionModel should have two message child records + And the first message record should have role "user" + And the second message record should have role "assistant" + And each message record should have the correct content + + @session @from_domain @messages @single + Scenario: SessionModel.from_domain handles a session with a single message + Given a Session domain object with one message + When the single-message Session is converted to a SessionModel via from_domain + Then the SessionModel should have one message child record + And the single message record should have the correct message_id + + # ------------------------------------------------------------------- + # SkillModel.from_domain() exercises column definitions (covers 2131-2132) + # ------------------------------------------------------------------- + + @skill @from_domain @columns + Scenario: SkillModel.from_domain creates model with description and version + Given a skill definition dict with description and version + When the skill dict is converted to a SkillModel via from_domain + Then the SkillModel should have the correct description + And the SkillModel should have the correct version + And the SkillModel should have the correct namespace + And the SkillModel should have the correct short_name + + @skill @from_domain @no_version + Scenario: SkillModel.from_domain handles skill without version + Given a skill definition dict without a version field + When the versionless skill dict is converted to a SkillModel via from_domain + Then the SkillModel version should be None + And the SkillModel description should still be set diff --git a/features/plan_cli_cancel_revert_coverage.feature b/features/plan_cli_cancel_revert_coverage.feature new file mode 100644 index 000000000..866220c67 --- /dev/null +++ b/features/plan_cli_cancel_revert_coverage.feature @@ -0,0 +1,91 @@ +Feature: Plan CLI cancel, revert, and lifecycle-list rich coverage + As a developer + I want to exercise the uncovered cancel_plan, revert_plan, + and lifecycle_list_plans rich table rendering paths + So that coverage for plan.py lines 1817, 1821-1822, 1845-1855, 1878-1888 is achieved + + Background: + Given a CLI runner for cancel-revert coverage + And a mocked lifecycle service for cancel-revert coverage + + # ---------- lifecycle-list rich table rendering (lines 1817, 1820-1822) ---------- + + Scenario: lifecycle-list in rich format renders created_at timestamp + Given the service returns plans with project links and timestamps for rich list + When I invoke lifecycle-list in rich format for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "Lifecycle Plans" + + Scenario: lifecycle-list in rich format renders automation profile column + Given the service returns plans with an automation profile for rich list + When I invoke lifecycle-list in rich format for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "Lifecycle Plans" + + Scenario: lifecycle-list in rich format renders multiple project links + Given the service returns a plan with three project links for rich list + When I invoke lifecycle-list in rich format for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "more" + + Scenario: lifecycle-list in rich format renders invariant count + Given the service returns plans with invariants for rich list + When I invoke lifecycle-list in rich format for cancel-revert coverage + Then the cancel-revert command should succeed + + # ---------- cancel_plan (lines 1845-1855) ---------- + + Scenario: cancel_plan succeeds in rich format without reason + Given the service can cancel a plan for cancel-revert coverage + When I invoke cancel in rich format without reason for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "Plan cancelled" + + Scenario: cancel_plan succeeds in rich format with reason + Given the service can cancel a plan for cancel-revert coverage + When I invoke cancel in rich format with reason "no longer needed" for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "Plan cancelled" + And the cancel-revert output should contain "no longer needed" + + Scenario: cancel_plan succeeds in JSON format without reason + Given the service can cancel a plan for cancel-revert coverage + When I invoke cancel with format "json" and no reason for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "plan_id" + And the cancel-revert output should not contain "cancel_reason" + + Scenario: cancel_plan succeeds in JSON format with reason + Given the service can cancel a plan for cancel-revert coverage + When I invoke cancel with format "json" and reason "obsolete" for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "cancel_reason" + And the cancel-revert output should contain "obsolete" + + # ---------- revert_plan (lines 1878-1888) ---------- + + Scenario: revert_plan succeeds in rich format with default phase + Given the service can revert a plan for cancel-revert coverage + When I invoke revert in rich format with default phase for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "Plan Reverted" + And the cancel-revert output should contain "Reversion count" + + Scenario: revert_plan succeeds in rich format with reason + Given the service can revert a plan for cancel-revert coverage + When I invoke revert in rich format with reason "constraints too strict" for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "Plan Reverted" + + Scenario: revert_plan succeeds in JSON format + Given the service can revert a plan for cancel-revert coverage + When I invoke revert with format "json" for cancel-revert coverage + Then the cancel-revert command should succeed + And the cancel-revert output should contain "plan_id" + And the cancel-revert output should contain "reversion_reason" + + Scenario: revert_plan rejects invalid phase + Given the service can revert a plan for cancel-revert coverage + When I invoke revert with invalid phase "bogus" for cancel-revert coverage + Then the cancel-revert command should abort + And the cancel-revert output should contain "Invalid phase" diff --git a/features/plan_executor_coverage.feature b/features/plan_executor_coverage.feature new file mode 100644 index 000000000..62a1be00d --- /dev/null +++ b/features/plan_executor_coverage.feature @@ -0,0 +1,158 @@ +Feature: PlanExecutor coverage for uncovered lines + As a developer + I want to test uncovered paths in plan_executor.py + So that lines 144, 233-235, 273, 483, 490 are covered + + # --- StrategizeStubActor._parse_steps empty input (line 144) --- + + Scenario: Parse steps returns default when input is empty string + Given a StrategizeStubActor instance for coverage + When I call _parse_steps with an empty string + Then the parsed steps should be the default fallback + + Scenario: Parse steps returns default when input is whitespace only + Given a StrategizeStubActor instance for coverage + When I call _parse_steps with whitespace only + Then the parsed steps should be the default fallback + + # --- StrategizeStubActor.execute happy path --- + + Scenario: Strategize stub actor produces decisions from definition of done + Given a StrategizeStubActor instance for coverage + When I execute the strategize stub with a valid plan and definition of done + Then the strategize result should contain decisions matching the steps + + Scenario: Strategize stub actor handles None definition of done + Given a StrategizeStubActor instance for coverage + When I execute the strategize stub with a None definition of done + Then the strategize result should contain the default decision + + Scenario: Strategize stub actor rejects empty plan_id + Given a StrategizeStubActor instance for coverage + When I execute the strategize stub with an empty plan_id + Then a plan executor ValidationError should be raised containing "plan_id" + + Scenario: Strategize stub actor processes invariants + Given a StrategizeStubActor instance for coverage + When I execute the strategize stub with invariants + Then the strategize result should contain invariant records + + Scenario: Strategize stub actor invokes stream callback + Given a StrategizeStubActor instance for coverage + When I execute the strategize stub with a stream callback + Then the stream callback should have been called with strategize events + + # --- ExecuteStubActor happy path --- + + Scenario: Execute stub actor produces a changeset + Given an ExecuteStubActor instance for coverage + When I execute the execute stub with decisions + Then the execute result should contain a changeset id + + Scenario: Execute stub actor rejects empty plan_id + Given an ExecuteStubActor instance for coverage + When I execute the execute stub with an empty plan_id + Then a plan executor ValidationError should be raised containing "plan_id" + + # --- PlanExecutor construction --- + + Scenario: PlanExecutor rejects None lifecycle service + When I construct a PlanExecutor with None lifecycle service + Then a plan executor ValidationError should be raised containing "lifecycle_service" + + Scenario: PlanExecutor has_runtime is False without execution context + Given a fresh mock lifecycle for plan executor coverage + When I construct a PlanExecutor without execution context + Then plan executor has_runtime should be False + + Scenario: PlanExecutor changeset_store is None without execution context + Given a fresh mock lifecycle for plan executor coverage + When I construct a PlanExecutor without execution context + Then plan executor changeset_store should be None + + # --- PlanExecutor.run_strategize happy path --- + + Scenario: PlanExecutor run_strategize succeeds for a plan in Strategize phase + Given a mock lifecycle with a plan in Strategize phase for executor coverage + And a PlanExecutor using that lifecycle for coverage + When I call run_strategize on the PlanExecutor for coverage + Then the strategize result should be returned successfully + And the lifecycle should have called start_strategize for coverage + And the lifecycle should have called complete_strategize for coverage + + # --- PlanExecutor.run_strategize exception path (line 273) --- + + Scenario: PlanExecutor run_strategize handles exception and calls fail_strategize + Given a mock lifecycle with a plan in Strategize phase that will fail during execute + And a PlanExecutor using that failing lifecycle for coverage + When I call run_strategize expecting an exception for coverage + Then the lifecycle should have called fail_strategize for coverage + + # --- PlanExecutor.run_strategize wrong phase --- + + Scenario: PlanExecutor run_strategize rejects plan not in Strategize phase + Given a mock lifecycle with a plan in Execute phase for executor coverage + And a PlanExecutor using that lifecycle for coverage + When I call run_strategize expecting a PlanError for coverage + Then a PlanError should be raised about wrong phase for coverage + + # --- PlanExecutor._guard_execute: decision_root_id is None (lines 233-235) --- + + Scenario: Guard execute rejects plan with no decision root id + Given a mock lifecycle with an execute-phase plan missing decision_root_id + And a PlanExecutor using that lifecycle for coverage + When I call guard_execute for the plan for coverage + Then a PlanError should be raised about missing decision tree + + Scenario: Guard execute rejects plan not in Execute phase + Given a mock lifecycle with a plan in Strategize phase for guard coverage + And a PlanExecutor using that lifecycle for coverage + When I call guard_execute expecting wrong phase for coverage + Then a PlanError should be raised about not in Execute phase + + Scenario: Guard execute rejects plan not in queued state + Given a mock lifecycle with an execute-phase plan in processing state for coverage + And a PlanExecutor using that lifecycle for coverage + When I call guard_execute expecting wrong state for coverage + Then a PlanError should be raised about not queued + + # --- PlanExecutor._run_execute_with_stub happy path --- + + Scenario: Run execute with stub succeeds for a properly configured plan + Given a mock lifecycle with a fully configured execute-phase plan for coverage + And a PlanExecutor using that lifecycle without runtime for coverage + When I call run_execute on the PlanExecutor for coverage + Then the execute result should be returned successfully + And the lifecycle should have called start_execute for coverage + And the lifecycle should have called complete_execute for coverage + + # --- PlanExecutor._run_execute_with_stub exception path (lines 483, 490) --- + + Scenario: Run execute with stub handles exception and calls fail_execute + Given a mock lifecycle with an execute-phase plan that will fail during stub execute + And a PlanExecutor using that failing stub lifecycle for coverage + When I call run_execute expecting an exception from stub for coverage + Then the lifecycle should have called fail_execute for coverage + + # --- PlanExecutor.run_execute validation --- + + Scenario: PlanExecutor run_execute rejects empty plan_id + Given a fresh mock lifecycle for plan executor coverage + And a PlanExecutor using that lifecycle for coverage + When I call run_execute with an empty plan_id for coverage + Then a plan executor ValidationError should be raised containing "plan_id" + + # --- PlanExecutor._build_decisions --- + + Scenario: Build decisions creates decisions from plan definition of done + Given a mock lifecycle with a fully configured execute-phase plan for coverage + And a PlanExecutor using that lifecycle without runtime for coverage + When I call build_decisions for the plan for coverage + Then the decisions should match the plan definition of done steps + + # --- PlanExecutor.execution_context property --- + + Scenario: PlanExecutor execution_context returns None without context + Given a fresh mock lifecycle for plan executor coverage + When I construct a PlanExecutor without execution context + Then plan executor execution_context should be None diff --git a/features/plan_lifecycle_service_persistence_coverage.feature b/features/plan_lifecycle_service_persistence_coverage.feature new file mode 100644 index 000000000..8a1e7c902 --- /dev/null +++ b/features/plan_lifecycle_service_persistence_coverage.feature @@ -0,0 +1,24 @@ +Feature: PlanLifecycleService persistence fallback coverage + Cover the uncovered persistence-layer fallback paths in + PlanLifecycleService.get_action() (lines 340-341) and + PlanLifecycleService.get_action_by_name() (lines 376-380). + + These branches execute when: + - The service has a UnitOfWork (persisted mode) + - The requested action is NOT in the in-memory _actions cache + - The action IS found in the persistence layer via ctx.actions.get_by_name() + + Background: + Given a plan lifecycle service with a mock unit of work + + Scenario: get_action falls back to persistence layer when not in memory + Given an action "local/db-only-action" exists only in the persistence layer + When I call get_action with "local/db-only-action" + Then the returned action should have namespaced name "local/db-only-action" + And the action "local/db-only-action" should now be cached in memory + + Scenario: get_action_by_name falls back to persistence layer when not in memory + Given an action "local/db-only-named" exists only in the persistence layer for name lookup + When I call get_action_by_name with "local/db-only-named" + Then the returned action from name lookup should have namespaced name "local/db-only-named" + And the action "local/db-only-named" should now be cached in memory after name lookup diff --git a/features/repl_coverage.feature b/features/repl_coverage.feature new file mode 100644 index 000000000..9ac505015 --- /dev/null +++ b/features/repl_coverage.feature @@ -0,0 +1,309 @@ +Feature: REPL module full coverage + Comprehensive unit-level tests that exercise every function and branch + inside ``src/cleveragents/cli/commands/repl.py`` to bring its coverage + from 0% to near-100%. + + # ----------------------------------------------------------------- + # Constants + # ----------------------------------------------------------------- + + Scenario: Module constants are defined correctly + Given the repl module is imported + Then the constant _MULTILINE_CONTINUATION equals backslash + And the constant _LAST_COMMAND_TOKEN equals "!!" + And the _REPL_COMMANDS list is non-empty + And the _BUILTIN_COMMANDS list contains help exit and quit + + # ----------------------------------------------------------------- + # _get_prompt_context + # ----------------------------------------------------------------- + + Scenario: _get_prompt_context returns bare prompt when no env vars set + Given neither CLEVERAGENTS_PROJECT nor CLEVERAGENTS_PLAN is set + When I call _get_prompt_context + Then the prompt result is "agents> " + + Scenario: _get_prompt_context returns project-only prompt + Given CLEVERAGENTS_PROJECT is set to "proj1" and CLEVERAGENTS_PLAN is unset + When I call _get_prompt_context + Then the prompt result is "agents (proj1)> " + + Scenario: _get_prompt_context returns project+plan prompt + Given CLEVERAGENTS_PROJECT is set to "proj1" and CLEVERAGENTS_PLAN is set to "plan1" + When I call _get_prompt_context + Then the prompt result is "agents (proj1/plan1)> " + + # ----------------------------------------------------------------- + # _setup_history + # ----------------------------------------------------------------- + + Scenario: _setup_history loads existing history file + Given a temporary history file that exists + When I call _setup_history with that path + Then readline.read_history_file is called with the path + And readline.set_history_length is called with 10000 + + Scenario: _setup_history skips read when file does not exist + Given a temporary history path that does not exist + When I call _setup_history with that path + Then readline.read_history_file is not called + And readline.set_history_length is called with 10000 + + # ----------------------------------------------------------------- + # _save_history + # ----------------------------------------------------------------- + + Scenario: _save_history writes the history file + Given a temporary history path for saving + When I call _save_history with that path + Then readline.write_history_file is called with the path + + Scenario: _save_history suppresses OSError + Given readline.write_history_file will raise OSError + When I call _save_history with a dummy path + Then no exception is raised + + # ----------------------------------------------------------------- + # _setup_completer + # ----------------------------------------------------------------- + + Scenario: _setup_completer installs readline completer + When I call _setup_completer + Then readline.set_completer is called with a callable + And readline.set_completer_delims is called + And readline.parse_and_bind is called with tab complete + + Scenario: The installed completer returns matching tokens + Given the completer is installed via _setup_completer + When I query the completer with text "ver" and state 0 + Then the completer returns "version" + + Scenario: The installed completer returns None for out-of-range state + Given the completer is installed via _setup_completer + When I query the completer with text "ver" and state 99 + Then the completer returns None + + # ----------------------------------------------------------------- + # _read_multiline + # ----------------------------------------------------------------- + + Scenario: _read_multiline reads continuation lines ending normally + Given builtins.input will return "continued line" + When I call _read_multiline with initial line ending in backslash + Then the multiline result is "first continued line" + + Scenario: _read_multiline handles chained continuations + Given builtins.input will return chained continuations + When I call _read_multiline with initial line ending in backslash + Then the multiline result is "first second third" + + Scenario: _read_multiline handles EOFError in continuation + Given builtins.input will raise EOFError on continuation + When I call _read_multiline with initial line ending in backslash + Then the multiline result is "only" + + Scenario: _read_multiline handles KeyboardInterrupt in continuation + Given builtins.input will raise KeyboardInterrupt on continuation + When I call _read_multiline with initial line ending in backslash + Then the multiline result is "only" + + # ----------------------------------------------------------------- + # _print_repl_help + # ----------------------------------------------------------------- + + Scenario: _print_repl_help runs without error + When I call _print_repl_help + Then no exception is raised from _print_repl_help + + # ----------------------------------------------------------------- + # dispatch_command + # ----------------------------------------------------------------- + + Scenario: dispatch_command returns 0 on success + Given the root Typer app is mocked to succeed + When I call dispatch_command with args "version" + Then the dispatch return code is 0 + + Scenario: dispatch_command returns code from SystemExit + Given the root Typer app raises SystemExit with code 42 + When I call dispatch_command with args "bad" + Then the dispatch return code is 42 + + Scenario: dispatch_command returns 0 for SystemExit with None code + Given the root Typer app raises SystemExit with None code + When I call dispatch_command with args "something" + Then the dispatch return code is 0 + + Scenario: dispatch_command returns 130 on KeyboardInterrupt + Given the root Typer app raises KeyboardInterrupt + When I call dispatch_command with args "slow" + Then the dispatch return code is 130 + + Scenario: dispatch_command returns 1 and prints error on generic Exception + Given the root Typer app raises RuntimeError boom + When I call dispatch_command with args "fail" + Then the dispatch return code is 1 + And the dispatch error output contains "boom" + + # ----------------------------------------------------------------- + # run_repl — exit commands + # ----------------------------------------------------------------- + + Scenario: run_repl exits on :exit command + Given the REPL input sequence is + | line | + | :exit | + When I run the REPL loop + Then the REPL loop exit code is 0 + + Scenario: run_repl exits on :quit command + Given the REPL input sequence is + | line | + | :quit | + When I run the REPL loop + Then the REPL loop exit code is 0 + + # ----------------------------------------------------------------- + # run_repl — help + # ----------------------------------------------------------------- + + Scenario: run_repl handles :help and continues + Given the REPL input sequence is + | line | + | :help | + | :exit | + When I run the REPL loop + Then the REPL loop output contains "Built-in commands" + + # ----------------------------------------------------------------- + # run_repl — !! repeat + # ----------------------------------------------------------------- + + Scenario: run_repl repeats last command with !! + Given the REPL input sequence is + | line | + | version | + | !! | + | :exit | + And dispatch_command is mocked for run_repl + When I run the REPL loop + Then dispatch_command was called at least 2 times with "version" + + Scenario: run_repl warns when !! has no previous command + Given the REPL input sequence is + | line | + | !! | + | :exit | + When I run the REPL loop + Then the REPL loop output contains "No previous command" + + # ----------------------------------------------------------------- + # run_repl — empty and whitespace input + # ----------------------------------------------------------------- + + Scenario: run_repl skips empty lines + Given the REPL input sequence is + | line | + | | + | :exit | + When I run the REPL loop + Then the REPL loop exit code is 0 + + # ----------------------------------------------------------------- + # run_repl — multiline + # ----------------------------------------------------------------- + + Scenario: run_repl handles multiline input ending with backslash + Given the REPL input sequence for multiline is "version \" then "" then ":exit" + And dispatch_command is mocked for run_repl + When I run the REPL loop + Then the REPL loop exit code is 0 + + # ----------------------------------------------------------------- + # run_repl — KeyboardInterrupt / EOFError + # ----------------------------------------------------------------- + + Scenario: run_repl handles KeyboardInterrupt and continues + Given the REPL input sequence for interrupt then exit + When I run the REPL loop + Then the REPL loop exit code is 0 + + Scenario: run_repl handles EOFError for clean exit + Given the REPL input sequence is empty + When I run the REPL loop + Then the REPL loop exit code is 0 + + # ----------------------------------------------------------------- + # run_repl — shlex parse error + # ----------------------------------------------------------------- + + Scenario: run_repl handles shlex parse error gracefully + Given the REPL input sequence has an unclosed quote then exit + When I run the REPL loop + Then the REPL loop output contains "Parse error" + + # ----------------------------------------------------------------- + # run_repl — history on/off + # ----------------------------------------------------------------- + + Scenario: run_repl with history enabled calls setup and save + Given the REPL input sequence is + | line | + | :exit | + When I run the REPL loop with history enabled + Then _setup_history was called + And _save_history was called + + Scenario: run_repl with no_history skips setup and save + Given the REPL input sequence is + | line | + | :exit | + When I run the REPL loop with no_history + Then _setup_history was not called + And _save_history was not called + + # ----------------------------------------------------------------- + # run_repl — dispatch a real-ish command + # ----------------------------------------------------------------- + + Scenario: run_repl dispatches tokenised command + Given the REPL input sequence is + | line | + | info | + | :exit | + And dispatch_command is mocked for run_repl + When I run the REPL loop + Then dispatch_command was called with args "info" + + # ----------------------------------------------------------------- + # repl_callback + # ----------------------------------------------------------------- + + Scenario: repl_callback starts REPL when stdout is a tty + Given sys.stdout.isatty returns True + And run_repl is mocked to return 0 + When I call repl_callback + Then run_repl was invoked + And repl_callback raises SystemExit with code 0 + + Scenario: repl_callback starts REPL when CLEVERAGENTS_FORCE_REPL is set + Given sys.stdout.isatty returns False + And CLEVERAGENTS_FORCE_REPL is set in the environment + And run_repl is mocked to return 0 + When I call repl_callback + Then run_repl was invoked + + Scenario: repl_callback does nothing when not a tty and no force env + Given sys.stdout.isatty returns False + And CLEVERAGENTS_FORCE_REPL is not set + When I call repl_callback + Then run_repl was not invoked + + # ----------------------------------------------------------------- + # Completer edge case: empty text matches all tokens + # ----------------------------------------------------------------- + + Scenario: The installed completer returns first token for empty text + Given the completer is installed via _setup_completer + When I query the completer with empty text and state 0 + Then the completer returns a non-None value diff --git a/features/resource_handler_service_coverage.feature b/features/resource_handler_service_coverage.feature new file mode 100644 index 000000000..7561e5743 --- /dev/null +++ b/features/resource_handler_service_coverage.feature @@ -0,0 +1,151 @@ +Feature: ResourceHandlerService full coverage + As a plan execution engine + I want the ResourceHandlerService to resolve bindings into BoundResources + So that tools receive sandbox-backed resource paths during execution + + Background: + Given a mock sandbox manager for rhs + And a mock resource lookup for rhs + And a mock type lookup for rhs + And a ResourceHandlerService under test + + # --- resolve_binding: deferred / missing resource_id --- + + Scenario: Resolving a deferred binding raises ValueError + Given an rhs binding that is deferred with slot name "repo" + When I resolve the rhs single binding + Then an rhs ValueError is raised with message containing "Cannot resolve deferred binding for slot 'repo'" + + Scenario: Resolving a binding with no resource_id raises ValueError + Given an rhs binding with no resource_id and slot name "data" + When I resolve the rhs single binding + Then an rhs ValueError is raised with message containing "Cannot resolve deferred binding for slot 'data'" + + # --- resolve_binding: successful resolution --- + + Scenario: Resolving a valid binding returns a BoundResource + Given an rhs non-deferred binding with slot "repo" and resource_id "RES001" + And rhs resource "RES001" typed "git-checkout" located at "/tmp/repo" with strategy "none" + And an rhs type spec "git-checkout" with no handler and sandbox strategy "git_worktree" + And the rhs sandbox manager returns a sandbox with path "/sandbox/abc" + When I resolve the rhs single binding with plan_id "plan-1" and access "read_write" + Then the rhs result is a BoundResource with slot "repo" and sandbox_path "/sandbox/abc" + And the rhs BoundResource has resource_id "RES001" and access "read_write" + + # --- _resolve_handler_for_type: type_spec has a handler --- + + Scenario: Handler is resolved from type_spec handler reference + Given an rhs non-deferred binding with slot "code" and resource_id "RES002" + And rhs resource "RES002" typed "custom/py" located at "/src" with strategy "none" + And an rhs type spec "custom/py" with handler "some.module:Handler" and sandbox strategy "copy_on_write" + And the rhs resolve_handler function returns a mock handler + And the rhs mock handler resolves to a BoundResource with sandbox_path "/sandbox/custom" + When I resolve the rhs single binding with plan_id "plan-2" and access "read_only" + Then the rhs result is a BoundResource with slot "code" and sandbox_path "/sandbox/custom" + + # --- _resolve_handler_for_type: handler resolution fails, falls back --- + + Scenario: Handler resolution failure falls back to DefaultHandler + Given an rhs non-deferred binding with slot "files" and resource_id "RES003" + And rhs resource "RES003" typed "custom/broken" located at "/data" with strategy "copy_on_write" + And an rhs type spec "custom/broken" with handler "bad.module:Missing" and sandbox strategy "copy_on_write" + And the rhs resolve_handler function raises HandlerResolutionError + And the rhs sandbox manager returns a sandbox with path "/sandbox/fallback" + When I resolve the rhs single binding with plan_id "plan-3" and access "read_only" + Then the rhs result is a BoundResource with slot "files" and sandbox_path "/sandbox/fallback" + + # --- _resolve_handler_for_type: no handler on type_spec --- + + Scenario: No handler on type_spec uses DefaultHandler directly + Given an rhs non-deferred binding with slot "dir" and resource_id "RES004" + And rhs resource "RES004" typed "fs-directory" located at "/home/user" with strategy "copy_on_write" + And an rhs type spec "fs-directory" with no handler and sandbox strategy "copy_on_write" + And the rhs sandbox manager returns a sandbox with path "/sandbox/dir" + When I resolve the rhs single binding with plan_id "plan-4" and access "read_only" + Then the rhs result is a BoundResource with slot "dir" and sandbox_path "/sandbox/dir" + + # --- resolve_bindings: mixed deferred and non-deferred --- + + Scenario: resolve_bindings skips deferred bindings and resolves the rest + Given an rhs binding list with: + | slot_name | resource_id | deferred | + | repo | RES010 | false | + | param | | true | + | config | RES011 | false | + And rhs resource "RES010" typed "git-checkout" located at "/repo" with strategy "none" + And rhs resource "RES011" typed "fs-directory" located at "/config" with strategy "none" + And an rhs type spec "git-checkout" with no handler and sandbox strategy "git_worktree" + And an rhs type spec "fs-directory" with no handler and sandbox strategy "copy_on_write" + And the rhs sandbox manager returns a sandbox with path "/sandbox/multi" + When I resolve all rhs bindings with plan_id "plan-5" and access "read_only" + Then the rhs result dict has keys "repo" and "config" + And the rhs result dict does not contain key "param" + + # --- resolve_bindings: failure propagation --- + + Scenario: resolve_bindings raises when a non-deferred binding fails + Given an rhs binding list with: + | slot_name | resource_id | deferred | + | broken | RES_MISSING | false | + And the rhs resource lookup raises NotFoundError for "RES_MISSING" + When I resolve all rhs bindings expecting an error + Then an rhs NotFoundError is raised + + # --- resolve_bindings: all deferred returns empty dict --- + + Scenario: resolve_bindings with all deferred bindings returns empty dict + Given an rhs binding list with: + | slot_name | resource_id | deferred | + | a | | true | + | b | | true | + When I resolve all rhs bindings with plan_id "plan-6" and access "read_only" + Then the rhs result dict is empty + + # --- resolve_resource: direct resolution --- + + Scenario: resolve_resource resolves a resource directly without a binding + Given rhs resource "RES020" typed "git-checkout" located at "/direct/repo" with strategy "none" + And an rhs type spec "git-checkout" with no handler and sandbox strategy "git_worktree" + And the rhs sandbox manager returns a sandbox with path "/sandbox/direct" + When I resolve the rhs resource directly with plan_id "plan-7" slot "src" and access "read_write" + Then the rhs result is a BoundResource with slot "src" and sandbox_path "/sandbox/direct" + And the rhs BoundResource has resource_id "RES020" and access "read_write" + + # --- _DefaultHandler: resource sandbox_strategy overrides type --- + + Scenario: DefaultHandler uses resource sandbox_strategy when present + Given an rhs non-deferred binding with slot "db" and resource_id "RES030" + And rhs resource "RES030" typed "custom/db" located at "/data/db" with strategy "snapshot" + And an rhs type spec "custom/db" with no handler and sandbox strategy "copy_on_write" + And the rhs sandbox manager returns a sandbox with path "/sandbox/snap" + When I resolve the rhs single binding with plan_id "plan-8" and access "read_only" + Then the rhs sandbox manager was called with strategy "snapshot" + + # --- _DefaultHandler: falls back to type_spec sandbox_strategy --- + + Scenario: DefaultHandler falls back to type_spec sandbox_strategy when resource has none + Given an rhs non-deferred binding with slot "fs" and resource_id "RES031" + And rhs resource "RES031" typed "fs-directory" located at "/mnt/share" with strategy "none" + And an rhs type spec "fs-directory" with no handler and sandbox strategy "copy_on_write" + And the rhs sandbox manager returns a sandbox with path "/sandbox/cow" + When I resolve the rhs single binding with plan_id "plan-9" and access "read_only" + Then the rhs sandbox manager was called with strategy "copy_on_write" + + # --- _DefaultHandler: no location error --- + + Scenario: DefaultHandler raises ValueError when resource has no location + Given an rhs non-deferred binding with slot "empty" and resource_id "RES040" + And rhs resource "RES040" typed "fs-directory" with no location + And an rhs type spec "fs-directory" with no handler and sandbox strategy "copy_on_write" + When I resolve the rhs single binding + Then an rhs ValueError is raised with message containing "has no location" + + # --- _DefaultHandler: sandbox context is None --- + + Scenario: DefaultHandler raises RuntimeError when sandbox has no context + Given an rhs non-deferred binding with slot "ctx" and resource_id "RES050" + And rhs resource "RES050" typed "fs-directory" located at "/tmp/ctx" with strategy "none" + And an rhs type spec "fs-directory" with no handler and sandbox strategy "copy_on_write" + And the rhs sandbox manager returns a sandbox with no context + When I resolve the rhs single binding + Then an rhs RuntimeError is raised with message containing "has no context" diff --git a/features/skill_cli_coverage.feature b/features/skill_cli_coverage.feature new file mode 100644 index 000000000..18f299e67 --- /dev/null +++ b/features/skill_cli_coverage.feature @@ -0,0 +1,33 @@ +Feature: Skill CLI uncovered lines + Cover uncovered lines in src/cleveragents/cli/commands/skill.py: + 273, 343-344, 353, 393, 589-590, 593-594. + + Scenario: Show skill with unregistered include + Given a clean skill service + And a skill with an include pointing to a non-registered skill + When I run skill show for the skill with unregistered include + Then the show output contains the not registered marker + + Scenario: Show skill when capability summary raises ValueError + Given a clean skill service + And a registered skill for capability error testing + When I run skill show with capability summary raising ValueError + Then the show output contains OK and no capability panel + + Scenario: Show skill with dependent actors + Given a clean skill service + And a registered skill for dependent actors testing + When I run skill show with dependents returning actors + Then the show output contains the actors reference line + + Scenario: Tools command with agent_skill entries + Given a clean skill service + And a registered skill with agent_skill sources + When I run skill tools for the agent_skill skill + Then the tools output contains agent_skill source type + + Scenario: Remove skill with dependent actors + Given a clean skill service + And a registered skill for removal with actor dependents + When I run skill remove with dependents returning actors + Then the remove output contains the actor dependency warning diff --git a/features/steps/config_service_coverage_steps.py b/features/steps/config_service_coverage_steps.py new file mode 100644 index 000000000..254d972fa --- /dev/null +++ b/features/steps/config_service_coverage_steps.py @@ -0,0 +1,803 @@ +"""Behave steps for complete coverage of config_service.py.""" + +from __future__ import annotations + +import tempfile +from pathlib import Path +from typing import Any +from unittest.mock import patch + +import tomlkit +from behave import given, then, when + +from cleveragents.application.services.config_service import ( + _REGISTRY, + ConfigEntry, + ConfigLevel, + ConfigService, + ResolvedValue, + _env_name, + _register, +) + +# --------------------------------------------------------------------------- +# ConfigLevel enum +# --------------------------------------------------------------------------- + + +@then("ConfigLevel should have values CLI_FLAG ENV_VAR PROJECT GLOBAL DEFAULT") +def step_config_level_enum(context: Any) -> None: + assert ConfigLevel.CLI_FLAG.value == "cli_flag" + assert ConfigLevel.ENV_VAR.value == "env_var" + assert ConfigLevel.PROJECT.value == "project" + assert ConfigLevel.GLOBAL.value == "global" + assert ConfigLevel.DEFAULT.value == "default" + assert len(ConfigLevel) == 5 + + +# --------------------------------------------------------------------------- +# ConfigEntry dataclass +# --------------------------------------------------------------------------- + + +@given('a ConfigEntry with key "test.key" and type str') +def step_create_config_entry(context: Any) -> None: + context.entry = ConfigEntry( + key="test.key", + python_type=str, + default="default_val", + env_var="CLEVERAGENTS_TEST_KEY", + project_scopable=True, + description="A test key.", + section="test", + ) + + +@then("the ConfigEntry fields should match the provided values") +def step_verify_config_entry(context: Any) -> None: + e = context.entry + assert e.key == "test.key" + assert e.python_type is str + assert e.default == "default_val" + assert e.env_var == "CLEVERAGENTS_TEST_KEY" + assert e.project_scopable is True + assert e.description == "A test key." + assert e.section == "test" + + +# --------------------------------------------------------------------------- +# ResolvedValue dataclass +# --------------------------------------------------------------------------- + + +@given('a ResolvedValue with key "test.key" and source DEFAULT') +def step_create_resolved_value(context: Any) -> None: + context.resolved = ResolvedValue( + key="test.key", + value="default_val", + source=ConfigLevel.DEFAULT, + ) + + +@then("the ResolvedValue fields should match including an empty chain by default") +def step_verify_resolved_value(context: Any) -> None: + rv = context.resolved + assert rv.key == "test.key" + assert rv.value == "default_val" + assert rv.source == ConfigLevel.DEFAULT + assert rv.chain == [] + + +# --------------------------------------------------------------------------- +# _env_name helper +# --------------------------------------------------------------------------- + + +@when('I call _env_name with section "core" and key "log_level"') +def step_call_env_name(context: Any) -> None: + context.env_name_result = _env_name("core", "log_level") + + +@then('the env name should be "CLEVERAGENTS_CORE_LOG_LEVEL"') +def step_verify_env_name(context: Any) -> None: + assert context.env_name_result == "CLEVERAGENTS_CORE_LOG_LEVEL" + + +# --------------------------------------------------------------------------- +# _register +# --------------------------------------------------------------------------- + + +@when('I register a custom key "custom.test_key" with type int and default 42') +def step_register_custom_key(context: Any) -> None: + _register("custom", "test_key", int, 42, description="Custom test key.") + + +@then('the registry should contain "custom.test_key" with default 42') +def step_verify_custom_key(context: Any) -> None: + entry = _REGISTRY.get("custom.test_key") + assert entry is not None + assert entry.default == 42 + assert entry.python_type is int + assert entry.env_var == "CLEVERAGENTS_CUSTOM_TEST_KEY" + + +@when('I register a key "custom.explicit_env" with explicit env_var "MY_CUSTOM_VAR"') +def step_register_explicit_env(context: Any) -> None: + _register( + "custom", + "explicit_env", + str, + "", + env_var="MY_CUSTOM_VAR", + description="Explicit env var.", + ) + + +@then('the entry "custom.explicit_env" should have env_var "MY_CUSTOM_VAR"') +def step_verify_explicit_env(context: Any) -> None: + entry = _REGISTRY.get("custom.explicit_env") + assert entry is not None + assert entry.env_var == "MY_CUSTOM_VAR" + + +# --------------------------------------------------------------------------- +# _build_catalog +# --------------------------------------------------------------------------- + + +@then( + "the registry should contain keys from sections core plan provider sandbox context index" +) +def step_verify_catalog_sections(context: Any) -> None: + sections_found: set[str] = set() + for entry in _REGISTRY.values(): + sections_found.add(entry.section) + for expected in ("core", "plan", "provider", "sandbox", "context", "index"): + assert expected in sections_found, f"Section '{expected}' not found in registry" + + +@then("the registry should contain at least 20 keys") +def step_verify_catalog_count(context: Any) -> None: + # _build_catalog registers ~24 keys from the source + assert len(_REGISTRY) >= 20, f"Expected >=20 keys, got {len(_REGISTRY)}" + + +# --------------------------------------------------------------------------- +# ConfigService instantiation +# --------------------------------------------------------------------------- + + +@given("a temporary directory for config service") +def step_temp_dir(context: Any) -> None: + context.temp_dir = Path(tempfile.mkdtemp()) + context.config_dir = context.temp_dir / "config" + context.config_path = context.config_dir / "config.toml" + context.svc = None + context.result = None + context.error = None + + +@when("I create a ConfigService with custom paths") +def step_create_service_custom(context: Any) -> None: + context.config_dir.mkdir(parents=True, exist_ok=True) + context.svc = ConfigService( + config_dir=context.config_dir, + config_path=context.config_path, + ) + + +@then("the service should use the custom config directory and path") +def step_verify_custom_paths(context: Any) -> None: + assert context.svc._config_dir == context.config_dir + assert context.svc._config_path == context.config_path + + +@when("I create a ConfigService with default paths") +def step_create_service_default(context: Any) -> None: + context.svc = ConfigService() + + +@then('the service config path should end with ".cleveragents/config.toml"') +def step_verify_default_path(context: Any) -> None: + path_str = str(context.svc._config_path) + assert path_str.endswith(".cleveragents/config.toml"), f"Got: {path_str}" + + +# --------------------------------------------------------------------------- +# registry(), get_entry(), registered_keys() +# --------------------------------------------------------------------------- + + +@when("I call ConfigService.registry") +def step_call_registry(context: Any) -> None: + context.result = ConfigService.registry() + + +@then("it should return a dict with the same keys as the internal registry") +def step_verify_registry_copy(context: Any) -> None: + assert isinstance(context.result, dict) + assert set(context.result.keys()) == set(_REGISTRY.keys()) + # Verify it's a copy, not the same object + assert context.result is not _REGISTRY + + +@when('I call ConfigService.get_entry with "core.log_level"') +def step_call_get_entry_known(context: Any) -> None: + context.result = ConfigService.get_entry("core.log_level") + + +@then('it should return a ConfigEntry with key "core.log_level"') +def step_verify_get_entry_known(context: Any) -> None: + assert isinstance(context.result, ConfigEntry) + assert context.result.key == "core.log_level" + + +@when('I call ConfigService.get_entry with "nonexistent.key"') +def step_call_get_entry_unknown(context: Any) -> None: + context.result = ConfigService.get_entry("nonexistent.key") + + +@then("it should return None") +def step_verify_get_entry_none(context: Any) -> None: + assert context.result is None + + +@when("I call ConfigService.registered_keys") +def step_call_registered_keys(context: Any) -> None: + context.result = ConfigService.registered_keys() + + +@then('it should return a sorted list containing "core.log_level"') +def step_verify_registered_keys(context: Any) -> None: + assert isinstance(context.result, list) + assert context.result == sorted(context.result) + assert "core.log_level" in context.result + + +# --------------------------------------------------------------------------- +# read_config +# --------------------------------------------------------------------------- + + +@when("I create a ConfigService pointing to a missing config file") +def step_service_missing_file(context: Any) -> None: + context.svc = ConfigService( + config_dir=context.config_dir, + config_path=context.config_dir / "nonexistent.toml", + ) + + +@then("read_config should return an empty dict") +def step_verify_read_empty(context: Any) -> None: + data = context.svc.read_config() + assert data == {} + + +@given('a TOML config file with key "core.log_level" set to "DEBUG"') +def step_write_toml_log_level(context: Any) -> None: + context.config_dir.mkdir(parents=True, exist_ok=True) + doc = tomlkit.document() + doc["core.log_level"] = "DEBUG" + with open(context.config_path, "w") as fh: + tomlkit.dump(doc, fh) + + +@when("I create a ConfigService pointing to that TOML file") +def step_service_existing_file(context: Any) -> None: + context.svc = ConfigService( + config_dir=context.config_dir, + config_path=context.config_path, + ) + + +@then('read_config should return a dict with "core.log_level" equal to "DEBUG"') +def step_verify_read_debug(context: Any) -> None: + data = context.svc.read_config() + assert data.get("core.log_level") == "DEBUG" + + +# --------------------------------------------------------------------------- +# write_config +# --------------------------------------------------------------------------- + + +@when("I create a ConfigService with a nested non-existent config directory") +def step_service_nested_dir(context: Any) -> None: + nested = context.temp_dir / "a" / "b" / "c" + context.config_dir = nested + context.config_path = nested / "config.toml" + context.svc = ConfigService( + config_dir=nested, + config_path=context.config_path, + ) + + +@when('I call write_config with key "greeting" value "hello"') +def step_call_write_config(context: Any) -> None: + context.svc.write_config({"greeting": "hello"}) + + +@then('the TOML file should exist and contain key "greeting" with value "hello"') +def step_verify_written_toml(context: Any) -> None: + assert context.config_path.exists() + data = context.svc.read_config() + assert data["greeting"] == "hello" + + +@given('a TOML config file with key "existing" set to "value1"') +def step_write_toml_existing(context: Any) -> None: + context.config_dir.mkdir(parents=True, exist_ok=True) + doc = tomlkit.document() + doc["existing"] = "value1" + with open(context.config_path, "w") as fh: + tomlkit.dump(doc, fh) + + +@when('I call write_config with key "new_key" value "value2"') +def step_call_write_config_merge(context: Any) -> None: + context.svc.write_config({"new_key": "value2"}) + + +@then('read_config should return a dict containing both "existing" and "new_key"') +def step_verify_merged_toml(context: Any) -> None: + data = context.svc.read_config() + assert data["existing"] == "value1" + assert data["new_key"] == "value2" + + +# --------------------------------------------------------------------------- +# set_value +# --------------------------------------------------------------------------- + + +@when('I call set_value with key "mykey" and value "myval"') +def step_call_set_value(context: Any) -> None: + context.svc.set_value("mykey", "myval") + + +@then('read_config should return a dict with "mykey" equal to "myval"') +def step_verify_set_value(context: Any) -> None: + data = context.svc.read_config() + assert data["mykey"] == "myval" + + +# --------------------------------------------------------------------------- +# validate_key +# --------------------------------------------------------------------------- + + +@when('I call validate_key with "core.log_level"') +def step_call_validate_key_known(context: Any) -> None: + try: + context.result = ConfigService.validate_key("core.log_level") + context.error = None + except Exception as exc: + context.error = exc + + +@then('it should return the ConfigEntry for "core.log_level" without error') +def step_verify_validate_key_ok(context: Any) -> None: + assert context.error is None + assert isinstance(context.result, ConfigEntry) + assert context.result.key == "core.log_level" + + +@when('I call validate_key with "totally.unknown"') +def step_call_validate_key_unknown(context: Any) -> None: + try: + context.result = ConfigService.validate_key("totally.unknown") + context.error = None + except ValueError as exc: + context.error = exc + + +@then('a ValueError should be raised mentioning "Unknown configuration key"') +def step_verify_validate_key_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, ValueError) + assert "Unknown configuration key" in str(context.error) + + +# --------------------------------------------------------------------------- +# validate_type +# --------------------------------------------------------------------------- + + +@when('I call validate_type with key "core.log_level" and value "INFO"') +def step_validate_type_same(context: Any) -> None: + context.result = ConfigService.validate_type("core.log_level", "INFO") + + +@then('it should return "INFO" unchanged') +def step_verify_type_same(context: Any) -> None: + assert context.result == "INFO" + assert isinstance(context.result, str) + + +@when('I call validate_type with key "core.server_port" and string value "9090"') +def step_validate_type_str_to_int(context: Any) -> None: + context.result = ConfigService.validate_type("core.server_port", "9090") + + +@then("it should return the integer 9090") +def step_verify_int_coercion(context: Any) -> None: + assert context.result == 9090 + assert isinstance(context.result, int) + + +@when('I call validate_type with key "provider.temperature" and string value "0.5"') +def step_validate_type_str_to_float(context: Any) -> None: + context.result = ConfigService.validate_type("provider.temperature", "0.5") + + +@then("it should return the float 0.5") +def step_verify_float_coercion(context: Any) -> None: + assert context.result == 0.5 + assert isinstance(context.result, float) + + +@when('I call validate_type with key "core.log_level" and integer value 123') +def step_validate_type_int_to_str(context: Any) -> None: + context.result = ConfigService.validate_type("core.log_level", 123) + + +@then('it should return the string "123"') +def step_verify_str_coercion(context: Any) -> None: + assert context.result == "123" + assert isinstance(context.result, str) + + +@when('I call validate_type with key "core.debug_enabled" and string value "true"') +def step_validate_type_bool_true(context: Any) -> None: + context.result = ConfigService.validate_type("core.debug_enabled", "true") + + +@then("it should return boolean True") +def step_verify_bool_true(context: Any) -> None: + assert context.result is True + + +@when('I call validate_type with key "core.debug_enabled" and string value "1"') +def step_validate_type_bool_one(context: Any) -> None: + context.result = ConfigService.validate_type("core.debug_enabled", "1") + + +@when('I call validate_type with key "core.debug_enabled" and string value "yes"') +def step_validate_type_bool_yes(context: Any) -> None: + context.result = ConfigService.validate_type("core.debug_enabled", "yes") + + +@when('I call validate_type with key "core.debug_enabled" and string value "false"') +def step_validate_type_bool_false(context: Any) -> None: + context.result = ConfigService.validate_type("core.debug_enabled", "false") + + +@then("it should return boolean False") +def step_verify_bool_false(context: Any) -> None: + assert context.result is False + + +@when('I call validate_type with key "core.debug_enabled" and string value "0"') +def step_validate_type_bool_zero(context: Any) -> None: + context.result = ConfigService.validate_type("core.debug_enabled", "0") + + +@when('I call validate_type with key "core.debug_enabled" and string value "no"') +def step_validate_type_bool_no(context: Any) -> None: + context.result = ConfigService.validate_type("core.debug_enabled", "no") + + +@when('I call validate_type with key "core.debug_enabled" and string value "maybe"') +def step_validate_type_bool_invalid(context: Any) -> None: + try: + context.result = ConfigService.validate_type("core.debug_enabled", "maybe") + context.error = None + except TypeError as exc: + context.error = exc + + +@then('a TypeError should be raised mentioning "Cannot convert"') +def step_verify_bool_invalid_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, TypeError) + assert "Cannot convert" in str(context.error) + + +@when( + 'I call validate_type with key "core.server_port" and string value "not_a_number"' +) +def step_validate_type_int_invalid(context: Any) -> None: + try: + context.result = ConfigService.validate_type("core.server_port", "not_a_number") + context.error = None + except TypeError as exc: + context.error = exc + + +@then('a TypeError should be raised mentioning "Type mismatch"') +def step_verify_type_mismatch_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, TypeError) + assert "Type mismatch" in str(context.error) + + +@when('I call validate_type with key "core.debug_enabled" and a list value') +def step_validate_type_bool_list(context: Any) -> None: + try: + context.result = ConfigService.validate_type("core.debug_enabled", [1, 2, 3]) + context.error = None + except TypeError as exc: + context.error = exc + + +@when('I call validate_type with unknown key "fake.unknown" and value "x"') +def step_validate_type_unknown_key(context: Any) -> None: + try: + context.result = ConfigService.validate_type("fake.unknown", "x") + context.error = None + except ValueError as exc: + context.error = exc + + +@then('a ValueError should be raised mentioning "Cannot validate type for unknown key"') +def step_verify_validate_type_unknown_key_error(context: Any) -> None: + assert context.error is not None + assert isinstance(context.error, ValueError) + assert "Cannot validate type for unknown key" in str(context.error) + + +# --------------------------------------------------------------------------- +# resolve - five-level chain +# --------------------------------------------------------------------------- + + +@given("a ConfigService with empty config for resolve tests") +def step_service_empty_for_resolve(context: Any) -> None: + context.config_dir.mkdir(parents=True, exist_ok=True) + context.svc = ConfigService( + config_dir=context.config_dir, + config_path=context.config_path, + ) + + +@when('I resolve key "core.log_level" with no overrides') +def step_resolve_no_overrides(context: Any) -> None: + context.result = context.svc.resolve("core.log_level") + + +@then('the resolved value should be "INFO" from source DEFAULT') +def step_verify_resolve_default(context: Any) -> None: + assert context.result.value == "INFO" + assert context.result.source == ConfigLevel.DEFAULT + + +@when('I resolve key "core.log_level" with cli_value "TRACE"') +def step_resolve_cli(context: Any) -> None: + context.result = context.svc.resolve("core.log_level", cli_value="TRACE") + + +@then('the resolved value should be "TRACE" from source CLI_FLAG') +def step_verify_resolve_cli(context: Any) -> None: + assert context.result.value == "TRACE" + assert context.result.source == ConfigLevel.CLI_FLAG + # Clean up env patch if present (from CLI-over-env scenario) + if hasattr(context, "_env_patch"): + context._env_patch.stop() + + +@given('the env var "{name}" is injected with "{value}"') +def step_inject_env_var(context: Any, name: str, value: str) -> None: + context._env_patch = patch.dict("os.environ", {name: value}) + context._env_patch.start() + + +@then('the resolved value should be "WARNING" from source ENV_VAR') +def step_verify_resolve_env(context: Any) -> None: + assert context.result.value == "WARNING" + assert context.result.source == ConfigLevel.ENV_VAR + # Clean up the patch + if hasattr(context, "_env_patch"): + context._env_patch.stop() + + +@given( + 'a ConfigService with project-scoped config for key "core.log_level" value "PROJECT_DEBUG" under project "myproj"' +) +def step_service_project_scoped(context: Any) -> None: + context.config_dir.mkdir(parents=True, exist_ok=True) + doc = tomlkit.document() + project_table = tomlkit.table() + myproj_table = tomlkit.table() + myproj_table["core.log_level"] = "PROJECT_DEBUG" + project_table["myproj"] = myproj_table + doc["project"] = project_table + with open(context.config_path, "w") as fh: + tomlkit.dump(doc, fh) + context.svc = ConfigService( + config_dir=context.config_dir, + config_path=context.config_path, + ) + + +@when('I resolve key "core.log_level" with project_name "myproj"') +def step_resolve_project(context: Any) -> None: + context.result = context.svc.resolve("core.log_level", project_name="myproj") + + +@then('the resolved value should be "PROJECT_DEBUG" from source PROJECT') +def step_verify_resolve_project(context: Any) -> None: + assert context.result.value == "PROJECT_DEBUG" + assert context.result.source == ConfigLevel.PROJECT + + +@given('a ConfigService with global config key "core.log_level" set to "GLOBAL_WARN"') +def step_service_global_config(context: Any) -> None: + context.config_dir.mkdir(parents=True, exist_ok=True) + doc = tomlkit.document() + doc["core.log_level"] = "GLOBAL_WARN" + with open(context.config_path, "w") as fh: + tomlkit.dump(doc, fh) + context.svc = ConfigService( + config_dir=context.config_dir, + config_path=context.config_path, + ) + + +@then('the resolved value should be "GLOBAL_WARN" from source GLOBAL') +def step_verify_resolve_global(context: Any) -> None: + assert context.result.value == "GLOBAL_WARN" + assert context.result.source == ConfigLevel.GLOBAL + + +@when('I resolve key "core.log_level" with verbose True and no overrides') +def step_resolve_verbose(context: Any) -> None: + context.result = context.svc.resolve("core.log_level", verbose=True) + + +@then("the resolved chain should have 5 entries covering all levels") +def step_verify_verbose_chain_length(context: Any) -> None: + chain = context.result.chain + assert len(chain) == 5, f"Expected 5 chain entries, got {len(chain)}" + sources = [entry["source"] for entry in chain] + assert sources == [ + ConfigLevel.CLI_FLAG.value, + ConfigLevel.ENV_VAR.value, + ConfigLevel.PROJECT.value, + ConfigLevel.GLOBAL.value, + ConfigLevel.DEFAULT.value, + ] + + +@then( + 'the verbose chain ENV_VAR entry should include env_name "CLEVERAGENTS_CORE_LOG_LEVEL"' +) +def step_verify_verbose_env_name(context: Any) -> None: + chain = context.result.chain + env_entry = next(e for e in chain if e["source"] == ConfigLevel.ENV_VAR.value) + assert env_entry["env_name"] == "CLEVERAGENTS_CORE_LOG_LEVEL" + + +@then("the verbose chain GLOBAL entry should include the config file path") +def step_verify_verbose_global_path(context: Any) -> None: + chain = context.result.chain + global_entry = next(e for e in chain if e["source"] == ConfigLevel.GLOBAL.value) + assert "path" in global_entry + assert str(context.config_path) in global_entry["path"] + + +# -- project scope skip for non-scopable keys -- + + +@given( + 'a ConfigService with project-scoped config for key "core.database_url" value "pg://proj" under project "myproj"' +) +def step_service_project_scoped_nonscopable(context: Any) -> None: + context.config_dir.mkdir(parents=True, exist_ok=True) + doc = tomlkit.document() + project_table = tomlkit.table() + myproj_table = tomlkit.table() + myproj_table["core.database_url"] = "pg://proj" + project_table["myproj"] = myproj_table + doc["project"] = project_table + with open(context.config_path, "w") as fh: + tomlkit.dump(doc, fh) + context.svc = ConfigService( + config_dir=context.config_dir, + config_path=context.config_path, + ) + + +@when('I resolve key "core.log_level" with project_name "myproj" and verbose True') +def step_resolve_project_verbose(context: Any) -> None: + context.result = context.svc.resolve( + "core.log_level", project_name="myproj", verbose=True + ) + + +@then('the verbose chain PROJECT entry should have value "PROJECT_DEBUG"') +def step_verify_verbose_project_value(context: Any) -> None: + chain = context.result.chain + proj_entry = next(e for e in chain if e["source"] == ConfigLevel.PROJECT.value) + assert proj_entry["value"] == "PROJECT_DEBUG" + + +@then('the verbose chain GLOBAL entry should have value "GLOBAL_WARN"') +def step_verify_verbose_global_value(context: Any) -> None: + chain = context.result.chain + global_entry = next(e for e in chain if e["source"] == ConfigLevel.GLOBAL.value) + assert global_entry["value"] == "GLOBAL_WARN" + + +@when('I resolve key "core.database_url" with project_name "myproj"') +def step_resolve_nonscopable(context: Any) -> None: + context.result = context.svc.resolve("core.database_url", project_name="myproj") + + +@then('the resolved value should not be "pg://proj"') +def step_verify_not_project_value(context: Any) -> None: + assert context.result.value != "pg://proj" + + +@then("the resolved source should be DEFAULT or GLOBAL") +def step_verify_source_not_project(context: Any) -> None: + assert context.result.source in (ConfigLevel.DEFAULT, ConfigLevel.GLOBAL) + + +# --------------------------------------------------------------------------- +# env_var_for_key +# --------------------------------------------------------------------------- + + +@when('I call env_var_for_key with "core.log_level"') +def step_call_env_var_for_key_valid(context: Any) -> None: + context.result = ConfigService.env_var_for_key("core.log_level") + + +@then('it should return "CLEVERAGENTS_CORE_LOG_LEVEL"') +def step_verify_env_var_for_key(context: Any) -> None: + assert context.result == "CLEVERAGENTS_CORE_LOG_LEVEL" + + +@when('I call env_var_for_key with "nonexistent.key"') +def step_call_env_var_for_key_unknown(context: Any) -> None: + try: + context.result = ConfigService.env_var_for_key("nonexistent.key") + context.error = None + except ValueError as exc: + context.error = exc + + +# The "a ValueError should be raised mentioning 'Unknown configuration key'" +# step is already defined above for validate_key and is reused here. + + +# --------------------------------------------------------------------------- +# resolve_all +# --------------------------------------------------------------------------- + + +@when("I call resolve_all with no overrides") +def step_call_resolve_all(context: Any) -> None: + context.result = context.svc.resolve_all() + + +@then("the result should contain an entry for every registered key") +def step_verify_resolve_all_keys(context: Any) -> None: + for key in _REGISTRY: + assert key in context.result, f"Missing resolved key: {key}" + assert isinstance(context.result[key], ResolvedValue) + + +@when('I call resolve_all with cli_override "core.log_level" set to "FATAL"') +def step_call_resolve_all_with_overrides(context: Any) -> None: + context.result = context.svc.resolve_all(cli_overrides={"core.log_level": "FATAL"}) + + +@then( + 'the resolve_all result for "core.log_level" should have value "FATAL" and source CLI_FLAG' +) +def step_verify_resolve_all_override(context: Any) -> None: + rv = context.result["core.log_level"] + assert rv.value == "FATAL" + assert rv.source == ConfigLevel.CLI_FLAG diff --git a/features/steps/correction_service_coverage_steps.py b/features/steps/correction_service_coverage_steps.py new file mode 100644 index 000000000..2b5a21a24 --- /dev/null +++ b/features/steps/correction_service_coverage_steps.py @@ -0,0 +1,769 @@ +"""Step definitions for correction_service_coverage.feature. + +Provides full line-and-branch coverage for +``cleveragents.application.services.correction_service.CorrectionService``. + +All step-text uses a ``cov-`` prefix to avoid collisions with existing +correction_flows_steps.py and correction_service_new_coverage_steps.py. +""" + +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, + CorrectionStatus, +) + +# ------------------------------------------------------------------- +# Helpers +# ------------------------------------------------------------------- + + +def _parse_adj(spec: str) -> dict[str, list[str]]: + """Parse ``"A->B,C;B->D"`` into an adjacency dict.""" + tree: dict[str, list[str]] = {} + for seg in spec.split(";"): + seg = seg.strip() + if not seg: + continue + parent, children_str = seg.split("->") + tree[parent.strip()] = [c.strip() for c in children_str.split(",")] + return tree + + +def _build_chain_cov(root: str, count: int) -> dict[str, list[str]]: + """Build a linear chain of *count* total nodes starting at *root*.""" + tree: dict[str, list[str]] = {} + current = root + for i in range(1, count): + child = f"{root}_ch{i}" + tree[current] = [child] + current = child + return tree + + +# ------------------------------------------------------------------- +# Background / Given +# ------------------------------------------------------------------- + + +@given("a coverage test correction service") +def step_bg_service(context): + context.cov_svc = CorrectionService() + context.cov_cid = None + context.cov_req = None + context.cov_impact = None + context.cov_revert_result = None + context.cov_append_result = None + context.cov_dispatch_result = None + context.cov_dry_run = None + context.cov_retrieved = None + context.cov_list = None + context.cov_attempts = None + context.cov_subtree = None + context.cov_error = None + + +@given('a cov-stored revert correction for plan "{plan}" targeting "{target}"') +def step_stored_revert_cov(context, plan, target): + req = context.cov_svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.REVERT, + ) + context.cov_cid = req.correction_id + context.cov_req = req + + +@given('a cov-stored append correction for plan "{plan}" targeting "{target}"') +def step_stored_append_cov(context, plan, target): + req = context.cov_svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.APPEND, + ) + context.cov_cid = req.correction_id + context.cov_req = req + + +@given("the cov-stored correction has been executed via revert with empty tree") +def step_pre_execute_revert(context): + context.cov_svc.execute_revert(context.cov_cid, {}) + + +@given("the cov-stored correction has been executed via append") +def step_pre_execute_append(context): + context.cov_svc.execute_append(context.cov_cid) + + +@given("the cov-stored correction has been analyzed with empty tree") +def step_pre_analyze(context): + context.cov_svc.analyze_impact(context.cov_cid, {}) + + +@given('the cov-stored correction status is forced to "{status}"') +def step_force_status(context, status): + req = context.cov_svc.get_correction(context.cov_cid) + req.status = CorrectionStatus(status) + + +# ------------------------------------------------------------------- +# When steps - request_correction +# ------------------------------------------------------------------- + + +@when( + 'I cov-request a correction for plan "{plan}" targeting "{target}" in revert mode' +) +def step_request_revert(context, plan, target): + context.cov_req = context.cov_svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.REVERT, + ) + context.cov_cid = context.cov_req.correction_id + + +@when( + 'I cov-request a correction for plan "{plan}" targeting "{target}"' + ' in append mode with guidance "{guidance}" and dry_run' +) +def step_request_append_guidance_dryrun(context, plan, target, guidance): + context.cov_req = context.cov_svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.APPEND, + guidance=guidance, + dry_run=True, + ) + context.cov_cid = context.cov_req.correction_id + + +@when( + 'I cov-attempt to create a correction with plan_id "{plan}" and target "{target}"' +) +def step_attempt_create(context, plan, target): + try: + context.cov_svc.request_correction( + plan_id=plan, + target_decision_id=target, + mode=CorrectionMode.REVERT, + ) + context.cov_error = None + except (ValidationError, Exception) as exc: + context.cov_error = exc + + +@when('I cov-attempt to create a correction with empty plan_id and target "{target}"') +def step_attempt_create_empty_plan(context, target): + try: + context.cov_svc.request_correction( + plan_id="", + target_decision_id=target, + mode=CorrectionMode.REVERT, + ) + context.cov_error = None + except (ValidationError, Exception) as exc: + context.cov_error = exc + + +@when('I cov-attempt to create a correction with plan_id "{plan}" and empty target') +def step_attempt_create_empty_target(context, plan): + try: + context.cov_svc.request_correction( + plan_id=plan, + target_decision_id="", + mode=CorrectionMode.REVERT, + ) + context.cov_error = None + except (ValidationError, Exception) as exc: + context.cov_error = exc + + +# ------------------------------------------------------------------- +# When steps - analyze_impact +# ------------------------------------------------------------------- + + +@when("I cov-analyze impact with an empty tree") +def step_analyze_empty(context): + context.cov_impact = context.cov_svc.analyze_impact(context.cov_cid, {}) + + +@when('I cov-analyze impact with adjacency "{spec}"') +def step_analyze_adj(context, spec): + tree = _parse_adj(spec) + context.cov_impact = context.cov_svc.analyze_impact(context.cov_cid, tree) + + +@when('I cov-attempt to analyze impact for correction id "{cid}"') +def step_attempt_analyze(context, cid): + try: + context.cov_svc.analyze_impact(cid, {}) + context.cov_error = None + except ResourceNotFoundError as exc: + context.cov_error = exc + + +# ------------------------------------------------------------------- +# When steps - generate_dry_run_report +# ------------------------------------------------------------------- + + +@when("I cov-generate dry-run report with an empty tree") +def step_dryrun_empty(context): + context.cov_dry_run = context.cov_svc.generate_dry_run_report(context.cov_cid, {}) + + +@when('I cov-generate dry-run report with adjacency "{spec}"') +def step_dryrun_adj(context, spec): + tree = _parse_adj(spec) + context.cov_dry_run = context.cov_svc.generate_dry_run_report(context.cov_cid, tree) + + +@when('I cov-generate dry-run report with a chain of {n:d} nodes rooted at "{root}"') +def step_dryrun_chain(context, n, root): + tree = _build_chain_cov(root, n) + context.cov_dry_run = context.cov_svc.generate_dry_run_report(context.cov_cid, tree) + + +@when('I cov-attempt to generate dry-run report for correction id "{cid}"') +def step_attempt_dryrun(context, cid): + try: + context.cov_svc.generate_dry_run_report(cid, {}) + context.cov_error = None + except ResourceNotFoundError as exc: + context.cov_error = exc + + +# ------------------------------------------------------------------- +# When steps - execute_revert +# ------------------------------------------------------------------- + + +@when('I cov-execute revert with adjacency "{spec}"') +def step_exec_revert_adj(context, spec): + tree = _parse_adj(spec) + context.cov_revert_result = context.cov_svc.execute_revert(context.cov_cid, tree) + + +@when("I cov-execute revert with empty tree") +def step_exec_revert_empty(context): + context.cov_revert_result = context.cov_svc.execute_revert(context.cov_cid, {}) + + +@when("I cov-attempt to execute revert on the stored correction") +def step_attempt_exec_revert(context): + try: + context.cov_svc.execute_revert(context.cov_cid, {}) + context.cov_error = None + except (ValidationError, ResourceNotFoundError) as exc: + context.cov_error = exc + + +# ------------------------------------------------------------------- +# When steps - execute_append +# ------------------------------------------------------------------- + + +@when("I cov-execute append for the stored correction") +def step_exec_append(context): + context.cov_append_result = context.cov_svc.execute_append(context.cov_cid) + + +@when("I cov-attempt to execute append on the stored correction") +def step_attempt_exec_append(context): + try: + context.cov_svc.execute_append(context.cov_cid) + context.cov_error = None + except (ValidationError, ResourceNotFoundError) as exc: + context.cov_error = exc + + +# ------------------------------------------------------------------- +# When steps - execute_correction (dispatch) +# ------------------------------------------------------------------- + + +@when('I cov-dispatch execute correction with adjacency "{spec}"') +def step_dispatch_adj(context, spec): + tree = _parse_adj(spec) + context.cov_dispatch_result = context.cov_svc.execute_correction( + context.cov_cid, tree + ) + + +@when("I cov-dispatch execute correction with no tree") +def step_dispatch_no_tree(context): + context.cov_dispatch_result = context.cov_svc.execute_correction(context.cov_cid) + + +# ------------------------------------------------------------------- +# When steps - get_correction +# ------------------------------------------------------------------- + + +@when("I cov-get the stored correction by id") +def step_get_by_id(context): + context.cov_retrieved = context.cov_svc.get_correction(context.cov_cid) + + +@when('I cov-attempt to get correction with id "{cid}"') +def step_attempt_get(context, cid): + try: + context.cov_svc.get_correction(cid) + context.cov_error = None + except ResourceNotFoundError as exc: + context.cov_error = exc + + +# ------------------------------------------------------------------- +# When steps - list_corrections +# ------------------------------------------------------------------- + + +@when("I cov-list all corrections without filter") +def step_list_all(context): + context.cov_list = context.cov_svc.list_corrections() + + +@when('I cov-list corrections for plan "{plan}"') +def step_list_by_plan(context, plan): + context.cov_list = context.cov_svc.list_corrections(plan_id=plan) + + +# ------------------------------------------------------------------- +# When steps - list_attempts +# ------------------------------------------------------------------- + + +@when("I cov-list attempts for the stored correction") +def step_list_attempts(context): + context.cov_attempts = context.cov_svc.list_attempts(context.cov_cid) + + +@when('I cov-attempt to list attempts for correction id "{cid}"') +def step_attempt_list_attempts(context, cid): + try: + context.cov_svc.list_attempts(cid) + context.cov_error = None + except ResourceNotFoundError as exc: + context.cov_error = exc + + +# ------------------------------------------------------------------- +# When steps - cancel_correction +# ------------------------------------------------------------------- + + +@when("I cov-cancel the stored correction") +def step_cancel(context): + context.cov_svc.cancel_correction(context.cov_cid) + + +@when("I cov-attempt to cancel the stored correction") +def step_attempt_cancel(context): + try: + context.cov_svc.cancel_correction(context.cov_cid) + context.cov_error = None + except ValidationError as exc: + context.cov_error = exc + + +# ------------------------------------------------------------------- +# When steps - _compute_affected_subtree +# ------------------------------------------------------------------- + + +@when('I cov-compute subtree for "{root}" with empty tree') +def step_subtree_empty(context, root): + context.cov_subtree = CorrectionService._compute_affected_subtree(root, {}) + + +@when('I cov-compute subtree for "{root}" with adjacency "{spec}"') +def step_subtree_adj(context, root, spec): + tree = _parse_adj(spec) + context.cov_subtree = CorrectionService._compute_affected_subtree(root, tree) + + +# =================================================================== +# Then steps +# =================================================================== + +# ------------------------------------------------------------------- +# __init__ assertions +# ------------------------------------------------------------------- + + +@then("the coverage service corrections dict should be empty") +def step_init_corrections(context): + assert len(context.cov_svc._corrections) == 0 + + +@then("the coverage service impacts dict should be empty") +def step_init_impacts(context): + assert len(context.cov_svc._impacts) == 0 + + +@then("the coverage service attempts dict should be empty") +def step_init_attempts(context): + assert len(context.cov_svc._attempts) == 0 + + +@then("the coverage service results dict should be empty") +def step_init_results(context): + assert len(context.cov_svc._results) == 0 + + +# ------------------------------------------------------------------- +# request_correction assertions +# ------------------------------------------------------------------- + + +@then("the cov-request should be stored in the service") +def step_req_stored(context): + assert context.cov_req is not None + fetched = context.cov_svc.get_correction(context.cov_req.correction_id) + assert fetched.correction_id == context.cov_req.correction_id + + +@then('the cov-request plan_id should be "{val}"') +def step_req_plan(context, val): + assert context.cov_req.plan_id == val + + +@then('the cov-request target_decision_id should be "{val}"') +def step_req_target(context, val): + assert context.cov_req.target_decision_id == val + + +@then('the cov-request mode should be "{val}"') +def step_req_mode(context, val): + assert context.cov_req.mode.value == val + + +@then('the cov-request status should be "{val}"') +def step_req_status(context, val): + assert context.cov_req.status.value == val + + +@then("the cov-request dry_run should be false") +def step_req_dryrun_false(context): + assert context.cov_req.dry_run is False + + +@then("the cov-request dry_run should be true") +def step_req_dryrun_true(context): + assert context.cov_req.dry_run is True + + +@then('the cov-request guidance should be "{val}"') +def step_req_guidance(context, val): + assert context.cov_req.guidance == val + + +@then("the cov-request guidance should be empty") +def step_req_guidance_empty(context): + assert context.cov_req.guidance == "" + + +# ------------------------------------------------------------------- +# Error assertions +# ------------------------------------------------------------------- + + +@then("a cov ValidationError should have been raised") +def step_cov_val_error(context): + assert context.cov_error is not None, "Expected ValidationError but none was raised" + assert isinstance(context.cov_error, ValidationError), ( + f"Expected ValidationError, got {type(context.cov_error).__name__}" + ) + + +@then("a cov ResourceNotFoundError should have been raised") +def step_cov_rnf_error(context): + assert context.cov_error is not None, ( + "Expected ResourceNotFoundError but none was raised" + ) + assert isinstance(context.cov_error, ResourceNotFoundError), ( + f"Expected ResourceNotFoundError, got {type(context.cov_error).__name__}" + ) + + +# ------------------------------------------------------------------- +# Impact assertions +# ------------------------------------------------------------------- + + +@then('the cov-impact affected decisions should be "{expected}"') +def step_impact_decisions(context, expected): + expected_list = [d.strip() for d in expected.split(",")] + assert context.cov_impact.affected_decisions == expected_list, ( + f"Expected {expected_list}, got {context.cov_impact.affected_decisions}" + ) + + +@then('the cov-impact risk level should be "{val}"') +def step_impact_risk(context, val): + assert context.cov_impact.risk_level == val + + +@then('the cov-impact rollback tier should be "{val}"') +def step_impact_rollback(context, val): + assert context.cov_impact.rollback_tier == val + + +@then("the cov-impact estimated cost should be {cost:g}") +def step_impact_cost(context, cost): + assert context.cov_impact.estimated_cost == cost + + +@then('the cov-impact artifacts to archive should be "{expected}"') +def step_impact_artifacts(context, expected): + expected_list = [a.strip() for a in expected.split(",")] + assert context.cov_impact.artifacts_to_archive == expected_list + + +@then('the cov-impact affected files should be "{expected}"') +def step_impact_files(context, expected): + expected_list = [f.strip() for f in expected.split(",")] + assert context.cov_impact.affected_files == expected_list + + +@then("the cov-impact affected decisions count should be {n:d}") +def step_impact_count(context, n): + actual = len(context.cov_impact.affected_decisions) + assert actual == n, f"Expected {n}, got {actual}" + + +@then('the cov-stored correction status should be "{val}"') +def step_stored_status(context, val): + req = context.cov_svc.get_correction(context.cov_cid) + assert req.status.value == val, f"Expected status '{val}', got '{req.status.value}'" + + +# ------------------------------------------------------------------- +# Dry-run report assertions +# ------------------------------------------------------------------- + + +@then('the cov dry-run mode should be "{val}"') +def step_dr_mode(context, val): + assert context.cov_dry_run.mode.value == val + + +@then("the cov dry-run warnings should be empty") +def step_dr_warnings_empty(context): + assert len(context.cov_dry_run.warnings) == 0, ( + f"Expected no warnings, got {context.cov_dry_run.warnings}" + ) + + +@then('the cov dry-run warnings should contain "{fragment}"') +def step_dr_warning_contains(context, fragment): + joined = " | ".join(context.cov_dry_run.warnings) + assert fragment in joined, ( + f"'{fragment}' not found in warnings: {context.cov_dry_run.warnings}" + ) + + +@then('the cov dry-run decisions to invalidate should be "{expected}"') +def step_dr_invalidate(context, expected): + expected_list = [d.strip() for d in expected.split(",")] + assert context.cov_dry_run.decisions_to_invalidate == expected_list + + +@then("the cov dry-run decisions to invalidate should be empty list") +def step_dr_invalidate_empty(context): + assert context.cov_dry_run.decisions_to_invalidate == [] + + +@then("the cov dry-run decisions to invalidate count should be {n:d}") +def step_dr_invalidate_count(context, n): + actual = len(context.cov_dry_run.decisions_to_invalidate) + assert actual == n, f"Expected {n}, got {actual}" + + +@then("the cov dry-run estimated recompute time should be {t:g}") +def step_dr_recompute(context, t): + assert context.cov_dry_run.estimated_recompute_time_seconds == t + + +# ------------------------------------------------------------------- +# Revert result assertions +# ------------------------------------------------------------------- + + +@then('the cov revert result status should be "{val}"') +def step_revert_status(context, val): + assert context.cov_revert_result.status.value == val + + +@then('the cov revert result reverted decisions should include "{d}"') +def step_revert_incl(context, d): + assert d in context.cov_revert_result.reverted_decisions + + +@then('the cov revert result archived artifacts should include "{a}"') +def step_revert_artifacts(context, a): + assert a in context.cov_revert_result.archived_artifacts + + +# ------------------------------------------------------------------- +# Append result assertions +# ------------------------------------------------------------------- + + +@then('the cov append result status should be "{val}"') +def step_append_status(context, val): + assert context.cov_append_result.status.value == val + + +@then("the cov append result spawned_child_plan_id should not be none") +def step_append_child(context): + assert context.cov_append_result.spawned_child_plan_id is not None + + +@then("the cov append result new_decisions should not be empty") +def step_append_new_decisions(context): + assert len(context.cov_append_result.new_decisions) > 0 + + +@then("the cov first attempt details should contain spawned_child_plan_id") +def step_append_attempt_details(context): + attempts = context.cov_svc.list_attempts(context.cov_cid) + assert "spawned_child_plan_id" in attempts[0].details + + +# ------------------------------------------------------------------- +# Dispatch result assertions +# ------------------------------------------------------------------- + + +@then('the cov dispatch result status should be "{val}"') +def step_dispatch_status(context, val): + assert context.cov_dispatch_result.status.value == val + + +@then('the cov dispatch result reverted decisions should include "{d}"') +def step_dispatch_reverted(context, d): + assert d in context.cov_dispatch_result.reverted_decisions + + +@then("the cov dispatch result spawned_child_plan_id should not be none") +def step_dispatch_child(context): + assert context.cov_dispatch_result.spawned_child_plan_id is not None + + +# ------------------------------------------------------------------- +# get_correction assertions +# ------------------------------------------------------------------- + + +@then('the cov-retrieved correction plan_id should be "{val}"') +def step_get_plan(context, val): + assert context.cov_retrieved.plan_id == val + + +@then('the cov-retrieved correction target_decision_id should be "{val}"') +def step_get_target(context, val): + assert context.cov_retrieved.target_decision_id == val + + +# ------------------------------------------------------------------- +# list_corrections assertions +# ------------------------------------------------------------------- + + +@then("the cov corrections list should have at least {n:d} items") +def step_list_min(context, n): + actual = len(context.cov_list) + assert actual >= n, f"Expected at least {n}, got {actual}" + + +@then("the cov corrections list should have {n:d} item") +def step_list_exact_one(context, n): + actual = len(context.cov_list) + assert actual == n, f"Expected {n}, got {actual}" + + +@then("the cov corrections list should have {n:d} items") +def step_list_exact(context, n): + actual = len(context.cov_list) + assert actual == n, f"Expected {n}, got {actual}" + + +# ------------------------------------------------------------------- +# list_attempts assertions +# ------------------------------------------------------------------- + + +@then("the cov attempts list should have {n:d} items") +def step_attempts_count_plural(context, n): + actual = len(context.cov_attempts) + assert actual == n, f"Expected {n}, got {actual}" + + +@then("the cov attempts list should have {n:d} item") +def step_attempts_count_single(context, n): + actual = len(context.cov_attempts) + assert actual == n, f"Expected {n}, got {actual}" + + +# ------------------------------------------------------------------- +# Attempt detail assertions +# ------------------------------------------------------------------- + + +@then("the cov attempts for the stored correction should have {n:d} entry") +def step_attempts_for_stored(context, n): + attempts = context.cov_svc.list_attempts(context.cov_cid) + assert len(attempts) == n, f"Expected {n}, got {len(attempts)}" + + +@then("the cov first attempt should be successful") +def step_first_attempt_ok(context): + attempts = context.cov_svc.list_attempts(context.cov_cid) + assert attempts[0].success is True + + +@then("the cov first attempt completed_at should be set") +def step_first_attempt_completed(context): + attempts = context.cov_svc.list_attempts(context.cov_cid) + assert attempts[0].completed_at is not None + + +# ------------------------------------------------------------------- +# _classify_risk assertions +# ------------------------------------------------------------------- + + +@then('cov classifying risk for {n:d} affected returns "{expected}"') +def step_classify_risk(context, n, expected): + result = CorrectionService._classify_risk(n) + assert result == expected, ( + f"_classify_risk({n}) = '{result}', expected '{expected}'" + ) + + +# ------------------------------------------------------------------- +# _compute_affected_subtree assertions +# ------------------------------------------------------------------- + + +@then('the cov subtree should be "{expected}"') +def step_subtree_result(context, expected): + expected_list = [n.strip() for n in expected.split(",")] + assert context.cov_subtree == expected_list, ( + f"Expected {expected_list}, got {context.cov_subtree}" + ) + + +@then("the cov subtree count should be {n:d}") +def step_subtree_count(context, n): + actual = len(context.cov_subtree) + assert actual == n, f"Expected {n}, got {actual}" diff --git a/features/steps/database_models_missing_coverage_steps.py b/features/steps/database_models_missing_coverage_steps.py new file mode 100644 index 000000000..d18fb215e --- /dev/null +++ b/features/steps/database_models_missing_coverage_steps.py @@ -0,0 +1,408 @@ +"""Step definitions for database models missing coverage tests. + +Targets uncovered lines: +- Line 1666: ToolModel.from_domain() dict branch of _get helper +- Line 1921: SessionModel.from_domain() iterating over session.messages +- Lines 2131-2132: SkillModel column definitions (description, version) +""" + +import json +from datetime import UTC, datetime + +from behave import given, then, when +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from ulid import ULID + +from cleveragents.domain.models.core.session import ( + MessageRole, + Session, + SessionMessage, + SessionTokenUsage, +) +from cleveragents.infrastructure.database.models import ( + Base, + SessionModel, + SkillModel, + ToolModel, +) + +# --------------------------------------------------------------------------- +# Reusable ULIDs +# --------------------------------------------------------------------------- + +ULID_SESSION = str(ULID()) +ULID_MSG_1 = str(ULID()) +ULID_MSG_2 = str(ULID()) + +NOW = datetime.now(tz=UTC) +NOW_ISO = NOW.isoformat() + +# --------------------------------------------------------------------------- +# Background steps +# --------------------------------------------------------------------------- + + +@given("the missing coverage database is ready") +def step_missing_coverage_db_ready(context): + """Set up an in-memory database with all tables.""" + context.mc_engine = create_engine("sqlite:///:memory:") + context.McSessionLocal = sessionmaker( + bind=context.mc_engine, autoflush=False, autocommit=False + ) + Base.metadata.create_all(context.mc_engine) + + +@given("a missing coverage database session is open") +def step_missing_coverage_session_open(context): + """Open a database session for missing coverage tests.""" + context.mc_session = context.McSessionLocal() + + +# =================================================================== +# ToolModel.from_domain() with dict input — line 1666 +# =================================================================== + + +@given("a tool definition provided as a plain dict") +def step_tool_as_plain_dict(context): + """Create a plain dict with tool fields.""" + context.tool_dict = { + "name": "myns/my-tool", + "description": "A test tool from a dict", + "tool_type": "function", + "source": "custom", + "input_schema_json": json.dumps({"type": "object"}), + "output_schema_json": json.dumps({"type": "string"}), + "capability_json": None, + "lifecycle_json": None, + "code": "print('hello')", + "mcp_server": None, + "mcp_tool_name": None, + "agent_skill_path": None, + "timeout": 60, + "wraps": None, + "transform": None, + "mode": None, + "argument_mapping_json": None, + "resource_bindings": [], + } + + +@when("the tool dict is converted to a ToolModel via from_domain") +def step_convert_tool_dict(context): + """Call ToolModel.from_domain with a plain dict.""" + context.tool_model = ToolModel.from_domain(context.tool_dict) + + +@then("the ToolModel should have the correct name from the dict") +def step_tool_model_name(context): + assert context.tool_model.name == "myns/my-tool" + + +@then("the ToolModel should have the correct description from the dict") +def step_tool_model_description(context): + assert context.tool_model.description == "A test tool from a dict" + + +@then("the ToolModel should have the correct tool_type from the dict") +def step_tool_model_tool_type(context): + assert context.tool_model.tool_type == "function" + + +@then("the ToolModel should have the correct source from the dict") +def step_tool_model_source(context): + assert context.tool_model.source == "custom" + + +# ------------------------------------------------------------------- +# ToolModel.from_domain() with resource bindings as dicts +# ------------------------------------------------------------------- + + +@given("a tool definition provided as a plain dict with resource bindings") +def step_tool_dict_with_bindings(context): + """Create a dict with resource_bindings also as dicts.""" + context.tool_dict_bindings = { + "name": "myns/binding-tool", + "description": "Tool with bindings", + "tool_type": "function", + "source": "custom", + "resource_bindings": [ + { + "slot_name": "db_conn", + "resource_type": "database/postgres", + "access_mode": "read_write", + "binding_mode": "static", + "static_resource": "main-db", + "required": True, + "description": "Primary database connection", + }, + { + "slot_name": "cache", + "resource_type": "cache/redis", + "access_mode": "read_only", + "binding_mode": "contextual", + "static_resource": None, + "required": False, + "description": "Optional cache layer", + }, + ], + } + + +@when("the tool dict with bindings is converted to a ToolModel via from_domain") +def step_convert_tool_dict_bindings(context): + """Call ToolModel.from_domain with dict resource bindings.""" + context.tool_model_bindings = ToolModel.from_domain(context.tool_dict_bindings) + + +@then("the ToolModel should have resource binding child records") +def step_tool_model_has_bindings(context): + assert len(context.tool_model_bindings.resource_bindings_rel) == 2 + + +@then("each resource binding should have the correct slot_name") +def step_binding_slot_names(context): + bindings = context.tool_model_bindings.resource_bindings_rel + assert bindings[0].slot_name == "db_conn" + assert bindings[1].slot_name == "cache" + + +@then("each resource binding should have the correct resource_type") +def step_binding_resource_types(context): + bindings = context.tool_model_bindings.resource_bindings_rel + assert bindings[0].resource_type == "database/postgres" + assert bindings[1].resource_type == "cache/redis" + + +# ------------------------------------------------------------------- +# ToolModel.from_domain() with namespaced name from dict +# ------------------------------------------------------------------- + + +@given("a tool definition dict with a namespaced name") +def step_tool_dict_namespaced(context): + """Create a dict whose name includes a namespace.""" + context.tool_dict_ns = { + "name": "acme/super-tool", + "description": "Namespaced tool", + "tool_type": "tool", + "source": "builtin", + } + + +@when("the namespaced tool dict is converted to a ToolModel via from_domain") +def step_convert_ns_tool_dict(context): + """Call ToolModel.from_domain with a namespaced dict.""" + context.tool_model_ns = ToolModel.from_domain(context.tool_dict_ns) + + +@then("the ToolModel namespace should be extracted from the dict name") +def step_tool_model_ns_namespace(context): + assert context.tool_model_ns.namespace == "acme" + + +@then("the ToolModel short_name should be extracted from the dict name") +def step_tool_model_ns_short_name(context): + assert context.tool_model_ns.short_name == "super-tool" + + +# =================================================================== +# SessionModel.from_domain() with messages — line 1921 +# =================================================================== + + +def _make_session_message(message_id, role, content, sequence, tool_call_id=None): + """Create a SessionMessage domain object.""" + return SessionMessage( + message_id=message_id, + role=role, + content=content, + sequence=sequence, + timestamp=NOW, + metadata={"source": "test"}, + tool_call_id=tool_call_id, + ) + + +@given("a Session domain object with two messages") +def step_session_with_two_messages(context): + """Create a Session domain object containing two messages.""" + msg1 = _make_session_message(ULID_MSG_1, MessageRole.USER, "Hello agent", 0) + msg2 = _make_session_message(ULID_MSG_2, MessageRole.ASSISTANT, "Hello human", 1) + + context.session_domain = Session( + session_id=ULID_SESSION, + actor_name="local/test-actor", + namespace="local", + linked_plan_ids=[], + token_usage=SessionTokenUsage( + input_tokens=100, + output_tokens=50, + estimated_cost=0.002, + ), + metadata={"env": "test"}, + messages=[msg1, msg2], + created_at=NOW, + updated_at=NOW, + ) + + +@when("the Session domain object is converted to a SessionModel via from_domain") +def step_convert_session_domain(context): + """Call SessionModel.from_domain with a Session that has messages.""" + context.session_model = SessionModel.from_domain(context.session_domain) + + +@then("the SessionModel should have two message child records") +def step_session_model_two_messages(context): + assert len(context.session_model.messages_rel) == 2 + + +@then('the first message record should have role "{role}"') +def step_first_msg_role(context, role): + assert context.session_model.messages_rel[0].role == role + + +@then('the second message record should have role "{role}"') +def step_second_msg_role(context, role): + assert context.session_model.messages_rel[1].role == role + + +@then("each message record should have the correct content") +def step_msg_contents(context): + assert context.session_model.messages_rel[0].content == "Hello agent" + assert context.session_model.messages_rel[1].content == "Hello human" + + +# ------------------------------------------------------------------- +# SessionModel.from_domain() with a single message +# ------------------------------------------------------------------- + + +@given("a Session domain object with one message") +def step_session_with_one_message(context): + """Create a Session domain object containing one message.""" + single_msg_id = str(ULID()) + msg = _make_session_message(single_msg_id, MessageRole.USER, "Single message", 0) + + context.single_msg_id = single_msg_id + context.session_domain_single = Session( + session_id=str(ULID()), + actor_name="local/solo-actor", + namespace="local", + linked_plan_ids=[], + token_usage=SessionTokenUsage( + input_tokens=10, + output_tokens=5, + estimated_cost=0.0001, + ), + metadata={}, + messages=[msg], + created_at=NOW, + updated_at=NOW, + ) + + +@when("the single-message Session is converted to a SessionModel via from_domain") +def step_convert_single_msg_session(context): + """Call SessionModel.from_domain with a single-message Session.""" + context.session_model_single = SessionModel.from_domain( + context.session_domain_single + ) + + +@then("the SessionModel should have one message child record") +def step_session_model_one_message(context): + assert len(context.session_model_single.messages_rel) == 1 + + +@then("the single message record should have the correct message_id") +def step_single_msg_id(context): + assert ( + context.session_model_single.messages_rel[0].message_id == context.single_msg_id + ) + + +# =================================================================== +# SkillModel.from_domain() — lines 2131-2132 (description, version) +# =================================================================== + + +@given("a skill definition dict with description and version") +def step_skill_dict_with_version(context): + """Create a dict representing a skill with description and version.""" + context.skill_dict = { + "name": "local/code-tools", + "description": "A collection of code analysis tools", + "version": "1.2.3", + "tool_refs": ["local/lint", "local/format"], + "includes": [], + "anonymous_tools": [], + "mcp_servers": [], + "agent_skills": [], + "overrides": {}, + } + + +@when("the skill dict is converted to a SkillModel via from_domain") +def step_convert_skill_dict(context): + """Call SkillModel.from_domain with a dict.""" + context.skill_model = SkillModel.from_domain(context.skill_dict) + + +@then("the SkillModel should have the correct description") +def step_skill_model_description(context): + assert context.skill_model.description == "A collection of code analysis tools" + + +@then("the SkillModel should have the correct version") +def step_skill_model_version(context): + assert context.skill_model.version == "1.2.3" + + +@then("the SkillModel should have the correct namespace") +def step_skill_model_namespace(context): + assert context.skill_model.namespace == "local" + + +@then("the SkillModel should have the correct short_name") +def step_skill_model_short_name(context): + assert context.skill_model.short_name == "code-tools" + + +# ------------------------------------------------------------------- +# SkillModel.from_domain() without version +# ------------------------------------------------------------------- + + +@given("a skill definition dict without a version field") +def step_skill_dict_no_version(context): + """Create a skill dict without the version key.""" + context.skill_dict_noversion = { + "name": "local/basic-skill", + "description": "A basic skill without version", + "tool_refs": [], + "includes": [], + "anonymous_tools": [], + "mcp_servers": [], + "agent_skills": [], + "overrides": {}, + } + + +@when("the versionless skill dict is converted to a SkillModel via from_domain") +def step_convert_versionless_skill_dict(context): + """Call SkillModel.from_domain with a dict missing version.""" + context.skill_model_noversion = SkillModel.from_domain(context.skill_dict_noversion) + + +@then("the SkillModel version should be None") +def step_skill_model_version_none(context): + assert context.skill_model_noversion.version is None + + +@then("the SkillModel description should still be set") +def step_skill_model_description_still_set(context): + assert context.skill_model_noversion.description == "A basic skill without version" diff --git a/features/steps/plan_cli_cancel_revert_coverage_steps.py b/features/steps/plan_cli_cancel_revert_coverage_steps.py new file mode 100644 index 000000000..a17fba6e2 --- /dev/null +++ b/features/steps/plan_cli_cancel_revert_coverage_steps.py @@ -0,0 +1,359 @@ +"""Step definitions for plan_cli_cancel_revert_coverage.feature. + +Covers uncovered lines in cleveragents/cli/commands/plan.py: +- Line 1817: timestamps.created_at.strftime in lifecycle_list_plans rich table +- Lines 1821-1822: console.print(table) and CleverAgentsError catch +- Lines 1845-1855: cancel_plan command body +- Lines 1878-1888: revert_plan command body +""" + +from __future__ import annotations + +from datetime import datetime +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from typer.testing import CliRunner + +from cleveragents.cli.commands.plan import app as plan_app +from cleveragents.domain.models.core.plan import ( + AutomationProfileProvenance, + AutomationProfileRef, + InvariantSource, + NamespacedName, + Plan, + PlanIdentity, + PlanInvariant, + PlanPhase, + PlanTimestamps, + ProcessingState, + ProjectLink, +) + +# Valid ULIDs for test plans +_ULID_CANCEL = "01ARZ3NDEKTSV4RRFFQ69G5FB1" +_ULID_REVERT = "01ARZ3NDEKTSV4RRFFQ69G5FB2" +_ULID_LIST_1 = "01ARZ3NDEKTSV4RRFFQ69G5FB3" +_ULID_LIST_2 = "01ARZ3NDEKTSV4RRFFQ69G5FB4" + + +def _make_plan( + *, + plan_id: str = _ULID_CANCEL, + name: str = "local/test-plan", + description: str = "Test plan for cancel-revert coverage", + phase: PlanPhase = PlanPhase.STRATEGIZE, + processing_state: ProcessingState = ProcessingState.QUEUED, + project_links: list[ProjectLink] | None = None, + automation_profile: AutomationProfileRef | None = None, + invariants: list[PlanInvariant] | None = None, + timestamps: PlanTimestamps | None = None, + reversion_count: int = 0, + action_name: str = "local/test-action", +) -> Plan: + """Build a real Plan object for testing.""" + if timestamps is None: + timestamps = PlanTimestamps( + created_at=datetime(2025, 6, 15, 10, 30, 0), + updated_at=datetime(2025, 6, 15, 11, 0, 0), + ) + return Plan( + identity=PlanIdentity(plan_id=plan_id), + namespaced_name=NamespacedName.parse(name), + action_name=action_name, + description=description, + phase=phase, + processing_state=processing_state, + project_links=project_links or [], + automation_profile=automation_profile, + invariants=invariants or [], + timestamps=timestamps, + reversion_count=reversion_count, + reusable=True, + read_only=False, + ) + + +# --------------------------------------------------------------------------- +# Background steps +# --------------------------------------------------------------------------- + + +@given("a CLI runner for cancel-revert coverage") +def step_cli_runner(context): + context.runner = CliRunner() + + +@given("a mocked lifecycle service for cancel-revert coverage") +def step_mocked_lifecycle_service(context): + context.mock_service = MagicMock() + patcher = patch( + "cleveragents.cli.commands.plan._get_lifecycle_service", + return_value=context.mock_service, + ) + patcher.start() + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(patcher.stop) + + +# --------------------------------------------------------------------------- +# Given steps — lifecycle-list rich format +# --------------------------------------------------------------------------- + + +@given("the service returns plans with project links and timestamps for rich list") +def step_service_plans_with_links_and_timestamps(context): + plans = [ + _make_plan( + plan_id=_ULID_LIST_1, + name="local/plan-with-links", + phase=PlanPhase.STRATEGIZE, + processing_state=ProcessingState.QUEUED, + project_links=[ + ProjectLink(project_name="proj-alpha"), + ProjectLink(project_name="proj-beta"), + ], + timestamps=PlanTimestamps( + created_at=datetime(2025, 3, 10, 14, 25, 0), + updated_at=datetime(2025, 3, 10, 15, 0, 0), + ), + ), + ] + context.mock_service.list_plans.return_value = plans + + +@given("the service returns plans with an automation profile for rich list") +def step_service_plans_with_profile(context): + plans = [ + _make_plan( + plan_id=_ULID_LIST_1, + name="local/profiled-plan", + phase=PlanPhase.EXECUTE, + processing_state=ProcessingState.PROCESSING, + project_links=[ProjectLink(project_name="proj-one")], + automation_profile=AutomationProfileRef( + profile_name="trusted", + provenance=AutomationProfileProvenance.ACTION, + ), + timestamps=PlanTimestamps( + created_at=datetime(2025, 4, 1, 9, 0, 0), + updated_at=datetime(2025, 4, 1, 10, 0, 0), + ), + ), + ] + context.mock_service.list_plans.return_value = plans + + +@given("the service returns a plan with three project links for rich list") +def step_service_plan_three_links(context): + plans = [ + _make_plan( + plan_id=_ULID_LIST_1, + name="local/multi-project", + phase=PlanPhase.STRATEGIZE, + processing_state=ProcessingState.QUEUED, + project_links=[ + ProjectLink(project_name="proj-a"), + ProjectLink(project_name="proj-b"), + ProjectLink(project_name="proj-c"), + ], + timestamps=PlanTimestamps( + created_at=datetime(2025, 5, 20, 8, 15, 0), + updated_at=datetime(2025, 5, 20, 9, 0, 0), + ), + ), + ] + context.mock_service.list_plans.return_value = plans + + +@given("the service returns plans with invariants for rich list") +def step_service_plans_with_invariants(context): + plans = [ + _make_plan( + plan_id=_ULID_LIST_1, + name="local/invariant-plan", + phase=PlanPhase.STRATEGIZE, + processing_state=ProcessingState.QUEUED, + project_links=[ProjectLink(project_name="proj-one")], + invariants=[ + PlanInvariant( + text="No breaking changes", source=InvariantSource.ACTION + ), + PlanInvariant( + text="Keep backward compat", source=InvariantSource.PROJECT + ), + ], + timestamps=PlanTimestamps( + created_at=datetime(2025, 6, 1, 12, 0, 0), + updated_at=datetime(2025, 6, 1, 13, 0, 0), + ), + ), + ] + context.mock_service.list_plans.return_value = plans + + +# --------------------------------------------------------------------------- +# Given steps — cancel_plan +# --------------------------------------------------------------------------- + + +@given("the service can cancel a plan for cancel-revert coverage") +def step_service_can_cancel(context): + plan = _make_plan( + plan_id=_ULID_CANCEL, + name="local/cancel-target", + phase=PlanPhase.STRATEGIZE, + processing_state=ProcessingState.CANCELLED, + project_links=[ProjectLink(project_name="proj-cancel")], + ) + context.mock_service.cancel_plan.return_value = plan + context._cancel_plan_id = _ULID_CANCEL + + +# --------------------------------------------------------------------------- +# Given steps — revert_plan +# --------------------------------------------------------------------------- + + +@given("the service can revert a plan for cancel-revert coverage") +def step_service_can_revert(context): + plan = _make_plan( + plan_id=_ULID_REVERT, + name="local/revert-target", + phase=PlanPhase.STRATEGIZE, + processing_state=ProcessingState.QUEUED, + project_links=[ProjectLink(project_name="proj-revert")], + reversion_count=1, + ) + context.mock_service.revert_plan.return_value = plan + context._revert_plan_id = _ULID_REVERT + + +# --------------------------------------------------------------------------- +# When steps — lifecycle-list +# --------------------------------------------------------------------------- + + +@when("I invoke lifecycle-list in rich format for cancel-revert coverage") +def step_invoke_lifecycle_list_rich(context): + context.result = context.runner.invoke( + plan_app, + ["lifecycle-list", "--format", "rich"], + ) + + +# --------------------------------------------------------------------------- +# When steps — cancel_plan +# --------------------------------------------------------------------------- + + +@when("I invoke cancel in rich format without reason for cancel-revert coverage") +def step_invoke_cancel_rich_no_reason(context): + context.result = context.runner.invoke( + plan_app, + ["cancel", context._cancel_plan_id], + ) + + +@when( + 'I invoke cancel in rich format with reason "{reason}" for cancel-revert coverage' +) +def step_invoke_cancel_rich_with_reason(context, reason): + context.result = context.runner.invoke( + plan_app, + ["cancel", context._cancel_plan_id, "--reason", reason], + ) + + +@when('I invoke cancel with format "{fmt}" and no reason for cancel-revert coverage') +def step_invoke_cancel_fmt_no_reason(context, fmt): + context.result = context.runner.invoke( + plan_app, + ["cancel", context._cancel_plan_id, "--format", fmt], + ) + + +@when( + 'I invoke cancel with format "{fmt}" and reason "{reason}" for cancel-revert coverage' +) +def step_invoke_cancel_fmt_with_reason(context, fmt, reason): + context.result = context.runner.invoke( + plan_app, + ["cancel", context._cancel_plan_id, "--format", fmt, "--reason", reason], + ) + + +# --------------------------------------------------------------------------- +# When steps — revert_plan +# --------------------------------------------------------------------------- + + +@when("I invoke revert in rich format with default phase for cancel-revert coverage") +def step_invoke_revert_rich_default(context): + context.result = context.runner.invoke( + plan_app, + ["revert", context._revert_plan_id], + ) + + +@when( + 'I invoke revert in rich format with reason "{reason}" for cancel-revert coverage' +) +def step_invoke_revert_rich_with_reason(context, reason): + context.result = context.runner.invoke( + plan_app, + ["revert", context._revert_plan_id, "--reason", reason], + ) + + +@when('I invoke revert with format "{fmt}" for cancel-revert coverage') +def step_invoke_revert_fmt(context, fmt): + context.result = context.runner.invoke( + plan_app, + ["revert", context._revert_plan_id, "--format", fmt], + ) + + +@when('I invoke revert with invalid phase "{phase}" for cancel-revert coverage') +def step_invoke_revert_invalid_phase(context, phase): + context.result = context.runner.invoke( + plan_app, + ["revert", context._revert_plan_id, "--to-phase", phase], + ) + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +def _output(context) -> str: + return getattr(context.result, "output", "") if hasattr(context, "result") else "" + + +@then("the cancel-revert command should succeed") +def step_command_succeeds(context): + assert context.result.exit_code == 0, ( + f"Expected exit code 0 but got {context.result.exit_code}.\n" + f"Output: {_output(context)}" + ) + + +@then("the cancel-revert command should abort") +def step_command_aborts(context): + assert context.result.exit_code != 0, ( + f"Expected non-zero exit code but got 0.\nOutput: {_output(context)}" + ) + + +@then('the cancel-revert output should contain "{text}"') +def step_output_contains(context, text): + output = _output(context) + assert text in output, f"Expected '{text}' in output:\n{output}" + + +@then('the cancel-revert output should not contain "{text}"') +def step_output_not_contains(context, text): + output = _output(context) + assert text not in output, f"Did not expect '{text}' in output:\n{output}" diff --git a/features/steps/plan_executor_coverage_steps.py b/features/steps/plan_executor_coverage_steps.py new file mode 100644 index 000000000..43d998a76 --- /dev/null +++ b/features/steps/plan_executor_coverage_steps.py @@ -0,0 +1,612 @@ +"""Step definitions for plan_executor.py coverage tests. + +Targets uncovered lines: 144, 233-235, 273, 483, 490. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock + +from behave import given, then, when +from behave.runner import Context + +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 ( + InvariantSource, + PlanInvariant, + PlanPhase, + PlanTimestamps, + ProcessingState, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +PLAN_ID = "01HABCDE12345678901234PLAN" + + +def _make_mock_plan( + *, + phase: PlanPhase = PlanPhase.STRATEGIZE, + state: ProcessingState = ProcessingState.QUEUED, + definition_of_done: str | None = "Step one\nStep two", + decision_root_id: str | None = "01HROOT000000000000000ROOT", + invariants: list[PlanInvariant] | None = None, +) -> MagicMock: + """Build a mock plan object with sensible defaults.""" + plan = MagicMock() + plan.phase = phase + plan.state = state + plan.processing_state = state + plan.definition_of_done = definition_of_done + plan.decision_root_id = decision_root_id + plan.invariants = invariants or [] + plan.timestamps = PlanTimestamps() + plan.changeset_id = None + plan.sandbox_refs = [] + plan.error_details = None + return plan + + +def _make_lifecycle(plan: Any | None = None) -> MagicMock: + """Build a mock lifecycle service returning *plan* on get_plan.""" + lcs = MagicMock() + if plan is not None: + lcs.get_plan.return_value = plan + lcs.start_strategize = MagicMock() + lcs.complete_strategize = MagicMock() + lcs.fail_strategize = MagicMock() + lcs.start_execute = MagicMock() + lcs.complete_execute = MagicMock() + lcs.fail_execute = MagicMock() + lcs._commit_plan = MagicMock() + return lcs + + +# --------------------------------------------------------------------------- +# StrategizeStubActor._parse_steps (line 144) +# --------------------------------------------------------------------------- + + +@given("a StrategizeStubActor instance for coverage") +def step_given_strategize_stub(context: Context) -> None: + context.strategize_actor = StrategizeStubActor() + + +@when("I call _parse_steps with an empty string") +def step_parse_steps_empty(context: Context) -> None: + context.parsed_steps = context.strategize_actor._parse_steps("") + + +@when("I call _parse_steps with whitespace only") +def step_parse_steps_whitespace(context: Context) -> None: + context.parsed_steps = context.strategize_actor._parse_steps(" \n\t ") + + +@then("the parsed steps should be the default fallback") +def step_check_default_steps(context: Context) -> None: + assert context.parsed_steps == ["Complete the plan objectives"], ( + f"Expected default fallback, got {context.parsed_steps}" + ) + + +# --------------------------------------------------------------------------- +# StrategizeStubActor.execute happy paths +# --------------------------------------------------------------------------- + + +@when("I execute the strategize stub with a valid plan and definition of done") +def step_exec_strategize_valid(context: Context) -> None: + context.strategize_result = context.strategize_actor.execute( + plan_id=PLAN_ID, + definition_of_done="First step\nSecond step", + ) + + +@then("the strategize result should contain decisions matching the steps") +def step_check_strategize_decisions(context: Context) -> None: + result: StrategizeResult = context.strategize_result + assert len(result.decisions) == 2 + assert result.decisions[0].step_text == "First step" + assert result.decisions[1].step_text == "Second step" + assert result.decision_root_id == result.decisions[0].decision_id + + +@when("I execute the strategize stub with a None definition of done") +def step_exec_strategize_none_dod(context: Context) -> None: + context.strategize_result = context.strategize_actor.execute( + plan_id=PLAN_ID, + definition_of_done=None, + ) + + +@then("the strategize result should contain the default decision") +def step_check_default_decision(context: Context) -> None: + result: StrategizeResult = context.strategize_result + assert len(result.decisions) == 1 + assert result.decisions[0].step_text == "Complete the plan objectives" + + +@when("I execute the strategize stub with an empty plan_id") +def step_exec_strategize_empty_plan_id(context: Context) -> None: + try: + context.strategize_actor.execute(plan_id="", definition_of_done="x") + context.raised_exception = None + except Exception as exc: + context.raised_exception = exc + + +@then('a plan executor ValidationError should be raised containing "{text}"') +def step_check_validation_error(context: Context, text: str) -> None: + assert context.raised_exception is not None, ( + "Expected an exception but none was raised" + ) + assert isinstance(context.raised_exception, ValidationError), ( + f"Expected ValidationError, got {type(context.raised_exception).__name__}" + ) + assert text in str(context.raised_exception), ( + f"Expected '{text}' in '{context.raised_exception}'" + ) + + +@when("I execute the strategize stub with invariants") +def step_exec_strategize_with_invariants(context: Context) -> None: + invariants = [ + PlanInvariant(text="Must be safe", source=InvariantSource.PLAN), + PlanInvariant(text="Must be fast", source=InvariantSource.PROJECT), + ] + context.strategize_result = context.strategize_actor.execute( + plan_id=PLAN_ID, + definition_of_done="Do it", + invariants=invariants, + ) + + +@then("the strategize result should contain invariant records") +def step_check_invariant_records(context: Context) -> None: + result: StrategizeResult = context.strategize_result + assert len(result.invariant_records) == 2 + assert result.invariant_records[0]["text"] == "Must be safe" + assert result.invariant_records[1]["source"] == "project" + + +@when("I execute the strategize stub with a stream callback") +def step_exec_strategize_with_callback(context: Context) -> None: + context.stream_events = [] + + def _cb(event: str, data: dict) -> None: + context.stream_events.append((event, data)) + + context.strategize_result = context.strategize_actor.execute( + plan_id=PLAN_ID, + definition_of_done="A step", + stream_callback=_cb, + ) + + +@then("the stream callback should have been called with strategize events") +def step_check_stream_events(context: Context) -> None: + event_names = [e[0] for e in context.stream_events] + assert "strategize_started" in event_names + assert "strategize_decisions" in event_names + assert "strategize_complete" in event_names + + +# --------------------------------------------------------------------------- +# ExecuteStubActor +# --------------------------------------------------------------------------- + + +@given("an ExecuteStubActor instance for coverage") +def step_given_execute_stub(context: Context) -> None: + context.execute_actor = ExecuteStubActor() + + +@when("I execute the execute stub with decisions") +def step_exec_execute_stub(context: Context) -> None: + decisions = [ + StrategyDecision( + decision_id="01HDEC0000000000000000DEC0", + step_text="Do the thing", + sequence=0, + ), + ] + context.execute_result = context.execute_actor.execute( + plan_id=PLAN_ID, + decisions=decisions, + ) + + +@then("the execute result should contain a changeset id") +def step_check_execute_changeset(context: Context) -> None: + result: ExecuteResult = context.execute_result + assert result.changeset_id is not None + assert len(result.changeset_id) > 0 + + +@when("I execute the execute stub with an empty plan_id") +def step_exec_execute_stub_empty_plan_id(context: Context) -> None: + try: + context.execute_actor.execute(plan_id="", decisions=[]) + context.raised_exception = None + except Exception as exc: + context.raised_exception = exc + + +# --------------------------------------------------------------------------- +# PlanExecutor construction +# --------------------------------------------------------------------------- + + +@when("I construct a PlanExecutor with None lifecycle service") +def step_construct_none_lifecycle(context: Context) -> None: + try: + PlanExecutor(lifecycle_service=None) + context.raised_exception = None + except Exception as exc: + context.raised_exception = exc + + +@given("a fresh mock lifecycle for plan executor coverage") +def step_given_fresh_mock_lifecycle(context: Context) -> None: + context.lifecycle = _make_lifecycle() + + +@when("I construct a PlanExecutor without execution context") +def step_construct_no_ctx(context: Context) -> None: + context.executor = PlanExecutor( + lifecycle_service=context.lifecycle, + execution_context=None, + ) + + +@then("plan executor has_runtime should be False") +def step_check_no_runtime(context: Context) -> None: + assert context.executor.has_runtime is False + + +@then("plan executor changeset_store should be None") +def step_check_no_changeset_store(context: Context) -> None: + assert context.executor.changeset_store is None + + +@then("plan executor execution_context should be None") +def step_check_no_execution_context(context: Context) -> None: + assert context.executor.execution_context is None + + +# --------------------------------------------------------------------------- +# PlanExecutor.run_strategize happy path +# --------------------------------------------------------------------------- + + +@given("a mock lifecycle with a plan in Strategize phase for executor coverage") +def step_given_lifecycle_strategize(context: Context) -> None: + plan = _make_mock_plan( + phase=PlanPhase.STRATEGIZE, + state=ProcessingState.QUEUED, + definition_of_done="Build the widget", + ) + context.lifecycle = _make_lifecycle(plan) + context.plan_id = PLAN_ID + + +@given("a PlanExecutor using that lifecycle for coverage") +def step_given_executor(context: Context) -> None: + context.executor = PlanExecutor(lifecycle_service=context.lifecycle) + + +@when("I call run_strategize on the PlanExecutor for coverage") +def step_run_strategize(context: Context) -> None: + context.strategize_result = context.executor.run_strategize(context.plan_id) + + +@then("the strategize result should be returned successfully") +def step_check_strategize_ok(context: Context) -> None: + assert isinstance(context.strategize_result, StrategizeResult) + assert len(context.strategize_result.decisions) > 0 + + +@then("the lifecycle should have called start_strategize for coverage") +def step_check_start_strategize(context: Context) -> None: + context.lifecycle.start_strategize.assert_called_once_with(context.plan_id) + + +@then("the lifecycle should have called complete_strategize for coverage") +def step_check_complete_strategize(context: Context) -> None: + context.lifecycle.complete_strategize.assert_called_once_with(context.plan_id) + + +# --------------------------------------------------------------------------- +# PlanExecutor.run_strategize exception path (line 273) +# --------------------------------------------------------------------------- + + +@given("a mock lifecycle with a plan in Strategize phase that will fail during execute") +def step_given_lifecycle_strategize_fail(context: Context) -> None: + plan = _make_mock_plan( + phase=PlanPhase.STRATEGIZE, + state=ProcessingState.QUEUED, + definition_of_done="Build the widget", + ) + context.lifecycle = _make_lifecycle(plan) + context.plan_id = PLAN_ID + + +@given("a PlanExecutor using that failing lifecycle for coverage") +def step_given_executor_failing(context: Context) -> None: + context.executor = PlanExecutor(lifecycle_service=context.lifecycle) + # Patch the internal strategize actor to raise + context.executor._strategize_actor = MagicMock() + context.executor._strategize_actor.execute.side_effect = RuntimeError( + "Boom in strategize" + ) + + +@when("I call run_strategize expecting an exception for coverage") +def step_run_strategize_fail(context: Context) -> None: + try: + context.executor.run_strategize(context.plan_id) + context.raised_exception = None + except RuntimeError as exc: + context.raised_exception = exc + + +@then("the lifecycle should have called fail_strategize for coverage") +def step_check_fail_strategize(context: Context) -> None: + assert context.raised_exception is not None, "Expected an exception" + context.lifecycle.fail_strategize.assert_called_once() + call_args = context.lifecycle.fail_strategize.call_args + assert context.plan_id in call_args[0] + assert "RuntimeError" in call_args[0][1] + + +# --------------------------------------------------------------------------- +# PlanExecutor.run_strategize wrong phase +# --------------------------------------------------------------------------- + + +@given("a mock lifecycle with a plan in Execute phase for executor coverage") +def step_given_lifecycle_execute_phase(context: Context) -> None: + plan = _make_mock_plan(phase=PlanPhase.EXECUTE) + context.lifecycle = _make_lifecycle(plan) + context.plan_id = PLAN_ID + + +@when("I call run_strategize expecting a PlanError for coverage") +def step_run_strategize_wrong_phase(context: Context) -> None: + try: + context.executor.run_strategize(context.plan_id) + context.raised_exception = None + except PlanError as exc: + context.raised_exception = exc + + +@then("a PlanError should be raised about wrong phase for coverage") +def step_check_wrong_phase_error(context: Context) -> None: + assert context.raised_exception is not None, "Expected a PlanError" + assert isinstance(context.raised_exception, PlanError) + assert "not in Strategize phase" in str(context.raised_exception) + + +# --------------------------------------------------------------------------- +# PlanExecutor._guard_execute: decision_root_id is None (lines 233-235) +# --------------------------------------------------------------------------- + + +@given("a mock lifecycle with an execute-phase plan missing decision_root_id") +def step_given_lifecycle_no_root(context: Context) -> None: + plan = _make_mock_plan( + phase=PlanPhase.EXECUTE, + state=ProcessingState.QUEUED, + decision_root_id=None, + ) + context.lifecycle = _make_lifecycle(plan) + context.plan_id = PLAN_ID + + +@when("I call guard_execute for the plan for coverage") +def step_call_guard_execute(context: Context) -> None: + try: + context.executor._guard_execute(context.plan_id) + context.raised_exception = None + except PlanError as exc: + context.raised_exception = exc + + +@then("a PlanError should be raised about missing decision tree") +def step_check_no_decision_tree(context: Context) -> None: + assert context.raised_exception is not None, "Expected a PlanError" + assert isinstance(context.raised_exception, PlanError) + assert "no decision tree" in str(context.raised_exception) + + +@given("a mock lifecycle with a plan in Strategize phase for guard coverage") +def step_given_lifecycle_strategize_for_guard(context: Context) -> None: + plan = _make_mock_plan( + phase=PlanPhase.STRATEGIZE, + state=ProcessingState.QUEUED, + ) + context.lifecycle = _make_lifecycle(plan) + context.plan_id = PLAN_ID + + +@when("I call guard_execute expecting wrong phase for coverage") +def step_call_guard_execute_wrong_phase(context: Context) -> None: + try: + context.executor._guard_execute(context.plan_id) + context.raised_exception = None + except PlanError as exc: + context.raised_exception = exc + + +@then("a PlanError should be raised about not in Execute phase") +def step_check_not_execute_phase(context: Context) -> None: + assert context.raised_exception is not None, "Expected a PlanError" + assert isinstance(context.raised_exception, PlanError) + assert "not in Execute phase" in str(context.raised_exception) + + +@given("a mock lifecycle with an execute-phase plan in processing state for coverage") +def step_given_lifecycle_processing(context: Context) -> None: + plan = _make_mock_plan( + phase=PlanPhase.EXECUTE, + state=ProcessingState.PROCESSING, + ) + context.lifecycle = _make_lifecycle(plan) + context.plan_id = PLAN_ID + + +@when("I call guard_execute expecting wrong state for coverage") +def step_call_guard_execute_wrong_state(context: Context) -> None: + try: + context.executor._guard_execute(context.plan_id) + context.raised_exception = None + except PlanError as exc: + context.raised_exception = exc + + +@then("a PlanError should be raised about not queued") +def step_check_not_queued(context: Context) -> None: + assert context.raised_exception is not None, "Expected a PlanError" + assert isinstance(context.raised_exception, PlanError) + assert "not queued" in str(context.raised_exception) + + +# --------------------------------------------------------------------------- +# PlanExecutor._run_execute_with_stub happy path +# --------------------------------------------------------------------------- + + +@given("a mock lifecycle with a fully configured execute-phase plan for coverage") +def step_given_lifecycle_full_execute(context: Context) -> None: + plan = _make_mock_plan( + phase=PlanPhase.EXECUTE, + state=ProcessingState.QUEUED, + definition_of_done="Implement feature\nWrite tests", + decision_root_id="01HROOT000000000000000ROOT", + ) + context.lifecycle = _make_lifecycle(plan) + context.plan_id = PLAN_ID + + +@given("a PlanExecutor using that lifecycle without runtime for coverage") +def step_given_executor_no_runtime(context: Context) -> None: + context.executor = PlanExecutor( + lifecycle_service=context.lifecycle, + execution_context=None, + ) + + +@when("I call run_execute on the PlanExecutor for coverage") +def step_run_execute(context: Context) -> None: + context.execute_result = context.executor.run_execute(context.plan_id) + + +@then("the execute result should be returned successfully") +def step_check_execute_ok(context: Context) -> None: + assert isinstance(context.execute_result, ExecuteResult) + assert context.execute_result.changeset_id is not None + + +@then("the lifecycle should have called start_execute for coverage") +def step_check_start_execute(context: Context) -> None: + context.lifecycle.start_execute.assert_called_once_with(context.plan_id) + + +@then("the lifecycle should have called complete_execute for coverage") +def step_check_complete_execute(context: Context) -> None: + context.lifecycle.complete_execute.assert_called_once_with(context.plan_id) + + +# --------------------------------------------------------------------------- +# PlanExecutor._run_execute_with_stub exception path (lines 483, 490) +# --------------------------------------------------------------------------- + + +@given("a mock lifecycle with an execute-phase plan that will fail during stub execute") +def step_given_lifecycle_stub_fail(context: Context) -> None: + plan = _make_mock_plan( + phase=PlanPhase.EXECUTE, + state=ProcessingState.QUEUED, + definition_of_done="Something", + decision_root_id="01HROOT000000000000000ROOT", + ) + context.lifecycle = _make_lifecycle(plan) + context.plan_id = PLAN_ID + + +@given("a PlanExecutor using that failing stub lifecycle for coverage") +def step_given_executor_failing_stub(context: Context) -> None: + context.executor = PlanExecutor( + lifecycle_service=context.lifecycle, + execution_context=None, + ) + # Patch the internal execute actor to raise + context.executor._execute_actor = MagicMock() + context.executor._execute_actor.execute.side_effect = RuntimeError( + "Boom in execute" + ) + + +@when("I call run_execute expecting an exception from stub for coverage") +def step_run_execute_fail(context: Context) -> None: + try: + context.executor.run_execute(context.plan_id) + context.raised_exception = None + except RuntimeError as exc: + context.raised_exception = exc + + +@then("the lifecycle should have called fail_execute for coverage") +def step_check_fail_execute(context: Context) -> None: + assert context.raised_exception is not None, "Expected an exception" + context.lifecycle.fail_execute.assert_called_once() + call_args = context.lifecycle.fail_execute.call_args + assert context.plan_id in call_args[0] + assert "RuntimeError" in call_args[0][1] + + +# --------------------------------------------------------------------------- +# PlanExecutor.run_execute validation +# --------------------------------------------------------------------------- + + +@when("I call run_execute with an empty plan_id for coverage") +def step_run_execute_empty_plan_id(context: Context) -> None: + try: + context.executor.run_execute("") + context.raised_exception = None + except Exception as exc: + context.raised_exception = exc + + +# --------------------------------------------------------------------------- +# PlanExecutor._build_decisions +# --------------------------------------------------------------------------- + + +@when("I call build_decisions for the plan for coverage") +def step_call_build_decisions(context: Context) -> None: + plan = context.lifecycle.get_plan(context.plan_id) + context.built_decisions = context.executor._build_decisions(plan) + + +@then("the decisions should match the plan definition of done steps") +def step_check_built_decisions(context: Context) -> None: + decisions = context.built_decisions + assert len(decisions) == 2 + assert decisions[0].step_text == "Implement feature" + assert decisions[1].step_text == "Write tests" + assert decisions[0].parent_id is None + assert decisions[1].parent_id is not None diff --git a/features/steps/plan_lifecycle_service_persistence_coverage_steps.py b/features/steps/plan_lifecycle_service_persistence_coverage_steps.py new file mode 100644 index 000000000..f9fa986e0 --- /dev/null +++ b/features/steps/plan_lifecycle_service_persistence_coverage_steps.py @@ -0,0 +1,155 @@ +"""Step definitions for plan_lifecycle_service_persistence_coverage.feature. + +Targets uncovered persistence-fallback lines in PlanLifecycleService: +- Lines 340-341: get_action() persistence fallback when action not in memory +- Lines 376-380: get_action_by_name() persistence fallback when action not + in memory and linear scan also misses +""" + +from __future__ import annotations + +from contextlib import contextmanager +from unittest.mock import MagicMock + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.application.services.plan_lifecycle_service import ( + PlanLifecycleService, +) +from cleveragents.domain.models.core.action import Action, ActionState +from cleveragents.domain.models.core.plan import NamespacedName + + +def _make_action(name: str) -> Action: + """Create a real Action domain object with the given namespaced name.""" + return Action( + namespaced_name=NamespacedName.parse(name), + description=f"Persisted action {name}", + definition_of_done="All tests pass", + strategy_actor="openai/gpt-4", + execution_actor="openai/gpt-4", + state=ActionState.AVAILABLE, + ) + + +def _build_mock_uow(action_by_name_map: dict[str, Action | None]) -> MagicMock: + """Build a mock UnitOfWork whose transaction context returns actions. + + ``action_by_name_map`` maps action name strings to Action objects + (or None for not-found). The mock ctx.actions.get_by_name(name) will + look up the name in this map and return the corresponding value. + """ + mock_uow = MagicMock() + mock_ctx = MagicMock() + mock_ctx.actions.get_by_name.side_effect = lambda n: action_by_name_map.get(n) + + @contextmanager + def _transaction(): + yield mock_ctx + + mock_uow.transaction = _transaction + return mock_uow + + +# ----------------------------------------------------------------- +# Background +# ----------------------------------------------------------------- + + +@given("a plan lifecycle service with a mock unit of work") +def step_create_service_with_mock_uow(context: Context) -> None: + """Create a PlanLifecycleService backed by a mock UnitOfWork.""" + settings = MagicMock() + # Start with an empty action map; scenarios will populate it. + context.action_by_name_map: dict[str, Action | None] = {} + mock_uow = _build_mock_uow(context.action_by_name_map) + context.service = PlanLifecycleService(settings=settings, unit_of_work=mock_uow) + context.result_action = None + + +# ----------------------------------------------------------------- +# get_action persistence fallback (lines 340-341) +# ----------------------------------------------------------------- + + +@given('an action "{name}" exists only in the persistence layer') +def step_action_in_persistence_only(context: Context, name: str) -> None: + """Add an action to the mock persistence layer but NOT to in-memory cache.""" + action = _make_action(name) + # Add to the mock UoW lookup map so ctx.actions.get_by_name finds it + context.action_by_name_map[name] = action + # Ensure it is NOT in the in-memory cache + context.service._actions.pop(name, None) + + +@when('I call get_action with "{name}"') +def step_call_get_action(context: Context, name: str) -> None: + """Call service.get_action() which should fall back to persistence.""" + context.result_action = context.service.get_action(name) + + +@then('the returned action should have namespaced name "{name}"') +def step_check_returned_action_name(context: Context, name: str) -> None: + """Verify the returned action has the expected namespaced name.""" + assert context.result_action is not None, "Expected an action, got None" + assert str(context.result_action.namespaced_name) == name, ( + f"Expected namespaced name '{name}', " + f"got '{context.result_action.namespaced_name}'" + ) + + +@then('the action "{name}" should now be cached in memory') +def step_check_action_cached(context: Context, name: str) -> None: + """Verify the action was added to the in-memory _actions cache.""" + assert name in context.service._actions, ( + f"Expected '{name}' in _actions cache, " + f"but found keys: {list(context.service._actions.keys())}" + ) + assert str(context.service._actions[name].namespaced_name) == name + + +# ----------------------------------------------------------------- +# get_action_by_name persistence fallback (lines 376-380) +# ----------------------------------------------------------------- + + +@given('an action "{name}" exists only in the persistence layer for name lookup') +def step_action_in_persistence_for_name_lookup(context: Context, name: str) -> None: + """Add an action to the mock persistence layer for get_action_by_name. + + The action must not be in the in-memory _actions dict at all, so that + both the direct dict lookup AND the linear scan miss, forcing the + persistence fallback on lines 376-380. + """ + action = _make_action(name) + # Add to the mock UoW lookup map + context.action_by_name_map[name] = action + # Ensure it is NOT in the in-memory cache + context.service._actions.pop(name, None) + + +@when('I call get_action_by_name with "{name}"') +def step_call_get_action_by_name(context: Context, name: str) -> None: + """Call service.get_action_by_name() which should fall back to persistence.""" + context.result_action = context.service.get_action_by_name(name) + + +@then('the returned action from name lookup should have namespaced name "{name}"') +def step_check_returned_action_by_name(context: Context, name: str) -> None: + """Verify the returned action has the expected namespaced name.""" + assert context.result_action is not None, "Expected an action, got None" + assert str(context.result_action.namespaced_name) == name, ( + f"Expected namespaced name '{name}', " + f"got '{context.result_action.namespaced_name}'" + ) + + +@then('the action "{name}" should now be cached in memory after name lookup') +def step_check_action_cached_after_name_lookup(context: Context, name: str) -> None: + """Verify the action was added to the in-memory _actions cache.""" + assert name in context.service._actions, ( + f"Expected '{name}' in _actions cache, " + f"but found keys: {list(context.service._actions.keys())}" + ) + assert str(context.service._actions[name].namespaced_name) == name diff --git a/features/steps/repl_coverage_steps.py b/features/steps/repl_coverage_steps.py new file mode 100644 index 000000000..4ba889eb8 --- /dev/null +++ b/features/steps/repl_coverage_steps.py @@ -0,0 +1,736 @@ +"""Behave step implementations for repl_coverage.feature. + +Provides full unit-level coverage of every function and branch in +``src/cleveragents/cli/commands/repl.py``. +""" + +from __future__ import annotations + +import os +import tempfile +from io import StringIO +from pathlib import Path +from unittest.mock import MagicMock, call, patch + +from behave import given, then, when +from behave.runner import Context + +# =================================================================== +# Helpers +# =================================================================== + + +class _FakeInputCov: + """Feed a list of strings then raise EOFError.""" + + def __init__(self, lines: list[str]) -> None: + self._lines = list(lines) + self._idx = 0 + + def __call__(self, prompt: str = "") -> str: + if self._idx >= len(self._lines): + raise EOFError + line = self._lines[self._idx] + self._idx += 1 + if line == "__KEYBOARD_INTERRUPT__": + raise KeyboardInterrupt + return line + + +def _make_console_and_buf(): + """Return a (Console, StringIO) pair for capturing Rich output.""" + from rich.console import Console + + buf = StringIO() + console = Console(file=buf, no_color=True, width=200) + return console, buf + + +# =================================================================== +# Constants +# =================================================================== + + +@given("the repl module is imported") +def step_import_repl_module(context: Context) -> None: + import cleveragents.cli.commands.repl as mod + + context.repl_mod = mod + + +@then("the constant _MULTILINE_CONTINUATION equals backslash") +def step_check_multiline_continuation(context: Context) -> None: + assert context.repl_mod._MULTILINE_CONTINUATION == "\\" + + +@then('the constant _LAST_COMMAND_TOKEN equals "!!"') +def step_check_last_command_token(context: Context) -> None: + assert context.repl_mod._LAST_COMMAND_TOKEN == "!!" + + +@then("the _REPL_COMMANDS list is non-empty") +def step_repl_commands_non_empty(context: Context) -> None: + assert len(context.repl_mod._REPL_COMMANDS) > 0 + + +@then("the _BUILTIN_COMMANDS list contains help exit and quit") +def step_builtin_commands(context: Context) -> None: + builtins_list = context.repl_mod._BUILTIN_COMMANDS + for token in (":help", ":exit", ":quit"): + assert token in builtins_list, f"{token} not in _BUILTIN_COMMANDS" + + +# =================================================================== +# _get_prompt_context +# =================================================================== + + +@given("neither CLEVERAGENTS_PROJECT nor CLEVERAGENTS_PLAN is set") +def step_clear_prompt_env(context: Context) -> None: + context.saved_project = os.environ.pop("CLEVERAGENTS_PROJECT", None) + context.saved_plan = os.environ.pop("CLEVERAGENTS_PLAN", None) + + def _restore(): + if context.saved_project is not None: + os.environ["CLEVERAGENTS_PROJECT"] = context.saved_project + if context.saved_plan is not None: + os.environ["CLEVERAGENTS_PLAN"] = context.saved_plan + + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(_restore) + + +@given('CLEVERAGENTS_PROJECT is set to "proj1" and CLEVERAGENTS_PLAN is unset') +def step_project_only_env(context: Context) -> None: + context.saved_project = os.environ.get("CLEVERAGENTS_PROJECT") + context.saved_plan = os.environ.get("CLEVERAGENTS_PLAN") + os.environ["CLEVERAGENTS_PROJECT"] = "proj1" + os.environ.pop("CLEVERAGENTS_PLAN", None) + + def _restore(): + if context.saved_project is not None: + os.environ["CLEVERAGENTS_PROJECT"] = context.saved_project + else: + os.environ.pop("CLEVERAGENTS_PROJECT", None) + if context.saved_plan is not None: + os.environ["CLEVERAGENTS_PLAN"] = context.saved_plan + + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(_restore) + + +@given('CLEVERAGENTS_PROJECT is set to "proj1" and CLEVERAGENTS_PLAN is set to "plan1"') +def step_project_and_plan_env(context: Context) -> None: + context.saved_project = os.environ.get("CLEVERAGENTS_PROJECT") + context.saved_plan = os.environ.get("CLEVERAGENTS_PLAN") + os.environ["CLEVERAGENTS_PROJECT"] = "proj1" + os.environ["CLEVERAGENTS_PLAN"] = "plan1" + + def _restore(): + if context.saved_project is not None: + os.environ["CLEVERAGENTS_PROJECT"] = context.saved_project + else: + os.environ.pop("CLEVERAGENTS_PROJECT", None) + if context.saved_plan is not None: + os.environ["CLEVERAGENTS_PLAN"] = context.saved_plan + else: + os.environ.pop("CLEVERAGENTS_PLAN", None) + + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(_restore) + + +@when("I call _get_prompt_context") +def step_call_get_prompt_context(context: Context) -> None: + from cleveragents.cli.commands.repl import _get_prompt_context + + context.prompt_result = _get_prompt_context() + + +@then('the prompt result is "{expected}"') +def step_check_prompt_result(context: Context, expected: str) -> None: + assert context.prompt_result == expected, ( + f"Expected {expected!r}, got {context.prompt_result!r}" + ) + + +# =================================================================== +# _setup_history +# =================================================================== + + +@given("a temporary history file that exists") +def step_temp_history_exists(context: Context) -> None: + fd, path = tempfile.mkstemp(suffix="_hist") + os.close(fd) + context.history_path = Path(path) + + def _cleanup(): + context.history_path.unlink(missing_ok=True) + + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(_cleanup) + + +@given("a temporary history path that does not exist") +def step_temp_history_not_exists(context: Context) -> None: + tmpdir = tempfile.mkdtemp() + context.history_path = Path(tmpdir) / "subdir" / "nonexistent_history" + + def _cleanup(): + import shutil + + shutil.rmtree(tmpdir, ignore_errors=True) + + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(_cleanup) + + +@when("I call _setup_history with that path") +def step_call_setup_history(context: Context) -> None: + from cleveragents.cli.commands.repl import _setup_history + + with patch("cleveragents.cli.commands.repl.readline") as mock_rl: + _setup_history(context.history_path) + context.mock_readline = mock_rl + + +@then("readline.read_history_file is called with the path") +def step_rl_read_called(context: Context) -> None: + context.mock_readline.read_history_file.assert_called_once_with( + str(context.history_path) + ) + + +@then("readline.read_history_file is not called") +def step_rl_read_not_called(context: Context) -> None: + context.mock_readline.read_history_file.assert_not_called() + + +@then("readline.set_history_length is called with 10000") +def step_rl_set_length(context: Context) -> None: + context.mock_readline.set_history_length.assert_called_once_with(10_000) + + +# =================================================================== +# _save_history +# =================================================================== + + +@given("a temporary history path for saving") +def step_temp_save_path(context: Context) -> None: + context.save_path = Path(tempfile.mkdtemp()) / "save_hist" + + +@when("I call _save_history with that path") +def step_call_save_history(context: Context) -> None: + from cleveragents.cli.commands.repl import _save_history + + with patch("cleveragents.cli.commands.repl.readline") as mock_rl: + _save_history(context.save_path) + context.mock_readline = mock_rl + + +@then("readline.write_history_file is called with the path") +def step_rl_write_called(context: Context) -> None: + context.mock_readline.write_history_file.assert_called_once_with( + str(context.save_path) + ) + + +@given("readline.write_history_file will raise OSError") +def step_rl_write_raises(context: Context) -> None: + context.oserror_flag = True + + +@when("I call _save_history with a dummy path") +def step_call_save_history_oserror(context: Context) -> None: + from cleveragents.cli.commands.repl import _save_history + + with patch("cleveragents.cli.commands.repl.readline") as mock_rl: + mock_rl.write_history_file.side_effect = OSError("disk full") + context.exc = None + try: + _save_history(Path("/tmp/dummy_hist")) + except Exception as exc: + context.exc = exc + + +@then("no exception is raised") +def step_no_exception(context: Context) -> None: + assert context.exc is None, f"Unexpected exception: {context.exc}" + + +# =================================================================== +# _setup_completer +# =================================================================== + + +@when("I call _setup_completer") +def step_call_setup_completer(context: Context) -> None: + from cleveragents.cli.commands.repl import _setup_completer + + with patch("cleveragents.cli.commands.repl.readline") as mock_rl: + _setup_completer() + context.mock_readline = mock_rl + # Grab the completer function that was passed to set_completer + context.completer_fn = mock_rl.set_completer.call_args[0][0] + + +@then("readline.set_completer is called with a callable") +def step_rl_set_completer(context: Context) -> None: + assert context.mock_readline.set_completer.called + assert callable(context.completer_fn) + + +@then("readline.set_completer_delims is called") +def step_rl_set_delims(context: Context) -> None: + context.mock_readline.set_completer_delims.assert_called_once() + + +@then("readline.parse_and_bind is called with tab complete") +def step_rl_parse_bind(context: Context) -> None: + context.mock_readline.parse_and_bind.assert_called_once_with("tab: complete") + + +@given("the completer is installed via _setup_completer") +def step_install_completer(context: Context) -> None: + from cleveragents.cli.commands.repl import _setup_completer + + with patch("cleveragents.cli.commands.repl.readline") as mock_rl: + _setup_completer() + context.completer_fn = mock_rl.set_completer.call_args[0][0] + + +@when('I query the completer with text "{text}" and state {state:d}') +def step_query_completer(context: Context, text: str, state: int) -> None: + context.completer_result = context.completer_fn(text, state) + + +@when("I query the completer with empty text and state {state:d}") +def step_query_completer_empty(context: Context, state: int) -> None: + context.completer_result = context.completer_fn("", state) + + +@then('the completer returns "{expected}"') +def step_completer_returns(context: Context, expected: str) -> None: + assert context.completer_result == expected, ( + f"Expected {expected!r}, got {context.completer_result!r}" + ) + + +@then("the completer returns None") +def step_completer_returns_none(context: Context) -> None: + assert context.completer_result is None + + +@then("the completer returns a non-None value") +def step_completer_returns_non_none(context: Context) -> None: + assert context.completer_result is not None, "Expected non-None from completer" + + +# =================================================================== +# _read_multiline +# =================================================================== + + +@given('builtins.input will return "continued line"') +def step_input_returns_single(context: Context) -> None: + context.fake_continuation_lines = ["continued line"] + context.multiline_initial = "first\\" + + +@given("builtins.input will return chained continuations") +def step_input_returns_chained(context: Context) -> None: + context.fake_continuation_lines = ["second\\", "third"] + context.multiline_initial = "first\\" + + +@given("builtins.input will raise EOFError on continuation") +def step_input_raises_eof(context: Context) -> None: + context.fake_continuation_lines = "__EOF__" + context.multiline_initial = "only\\" + + +@given("builtins.input will raise KeyboardInterrupt on continuation") +def step_input_raises_kbi(context: Context) -> None: + context.fake_continuation_lines = "__KBI__" + context.multiline_initial = "only\\" + + +@when("I call _read_multiline with initial line ending in backslash") +def step_call_read_multiline(context: Context) -> None: + from cleveragents.cli.commands.repl import _read_multiline + + if context.fake_continuation_lines == "__EOF__": + fake = MagicMock(side_effect=EOFError) + elif context.fake_continuation_lines == "__KBI__": + fake = MagicMock(side_effect=KeyboardInterrupt) + else: + fake = _FakeInputCov(context.fake_continuation_lines) + + with patch("builtins.input", fake): + context.multiline_result = _read_multiline(context.multiline_initial) + + +@then('the multiline result is "{expected}"') +def step_check_multiline_result(context: Context, expected: str) -> None: + assert context.multiline_result == expected, ( + f"Expected {expected!r}, got {context.multiline_result!r}" + ) + + +# =================================================================== +# _print_repl_help +# =================================================================== + + +@when("I call _print_repl_help") +def step_call_print_repl_help(context: Context) -> None: + from cleveragents.cli.commands.repl import _print_repl_help + + console, buf = _make_console_and_buf() + context.help_exc = None + try: + with patch("cleveragents.cli.commands.repl._console", console): + _print_repl_help() + except Exception as exc: + context.help_exc = exc + context.help_output = buf.getvalue() + + +@then("no exception is raised from _print_repl_help") +def step_no_help_exception(context: Context) -> None: + assert context.help_exc is None, f"Unexpected exception: {context.help_exc}" + + +# =================================================================== +# dispatch_command +# =================================================================== + + +@given("the root Typer app is mocked to succeed") +def step_mock_typer_success(context: Context) -> None: + context.dispatch_typer_side_effect = None + + +@given("the root Typer app raises SystemExit with code 42") +def step_mock_typer_sysexit_42(context: Context) -> None: + context.dispatch_typer_side_effect = SystemExit(42) + + +@given("the root Typer app raises SystemExit with None code") +def step_mock_typer_sysexit_none(context: Context) -> None: + context.dispatch_typer_side_effect = SystemExit(None) + + +@given("the root Typer app raises KeyboardInterrupt") +def step_mock_typer_kbi(context: Context) -> None: + context.dispatch_typer_side_effect = KeyboardInterrupt() + + +@given("the root Typer app raises RuntimeError boom") +def step_mock_typer_runtime_error(context: Context) -> None: + context.dispatch_typer_side_effect = RuntimeError("boom") + + +@when('I call dispatch_command with args "{arg}"') +def step_call_dispatch_command(context: Context, arg: str) -> None: + from cleveragents.cli.commands.repl import dispatch_command + + console, buf = _make_console_and_buf() + mock_app = MagicMock() + if context.dispatch_typer_side_effect is not None: + mock_app.side_effect = context.dispatch_typer_side_effect + + with ( + patch("cleveragents.cli.commands.repl._console", console), + patch( + "cleveragents.cli.main._register_subcommands", + MagicMock(), + ), + patch("cleveragents.cli.main.app", mock_app), + ): + context.dispatch_rc = dispatch_command([arg]) + context.dispatch_output = buf.getvalue() + + +@then("the dispatch return code is {code:d}") +def step_check_dispatch_rc(context: Context, code: int) -> None: + assert context.dispatch_rc == code, ( + f"Expected dispatch rc={code}, got {context.dispatch_rc}" + ) + + +@then('the dispatch error output contains "boom"') +def step_dispatch_output_boom(context: Context) -> None: + assert "boom" in context.dispatch_output, ( + f"Expected 'boom' in output: {context.dispatch_output}" + ) + + +# =================================================================== +# run_repl — shared helpers +# =================================================================== + + +def _run_repl_cov( + lines: list[str], + *, + no_history: bool = True, + mock_dispatch: bool = False, + mock_setup_hist: bool = False, + mock_save_hist: bool = False, +): + """Run the REPL with mocked input/console and optionally mocked dispatch.""" + from cleveragents.cli.commands.repl import run_repl + + console, buf = _make_console_and_buf() + fake = _FakeInputCov(lines) + + patches = [ + patch("cleveragents.cli.commands.repl._console", console), + patch("builtins.input", fake), + ] + + mock_dispatch_obj = None + mock_setup_obj = None + mock_save_obj = None + + if mock_dispatch: + m = MagicMock(return_value=0) + patches.append(patch("cleveragents.cli.commands.repl.dispatch_command", m)) + mock_dispatch_obj = m + + if mock_setup_hist: + m = MagicMock() + patches.append(patch("cleveragents.cli.commands.repl._setup_history", m)) + mock_setup_obj = m + + if mock_save_hist: + m = MagicMock() + patches.append(patch("cleveragents.cli.commands.repl._save_history", m)) + mock_save_obj = m + + for p in patches: + p.start() + try: + code = run_repl(no_history=no_history) + finally: + for p in patches: + p.stop() + + return code, buf.getvalue(), mock_dispatch_obj, mock_setup_obj, mock_save_obj + + +# --- Given steps for REPL input sequences (using tables) --- + + +@given("the REPL input sequence is") +def step_repl_input_from_table(context: Context) -> None: + context.cov_repl_lines = [row["line"] for row in context.table] + + +@given("the REPL input sequence is empty") +def step_repl_input_empty(context: Context) -> None: + context.cov_repl_lines = [] + + +@given('the REPL input sequence for multiline is "version \\" then "" then ":exit"') +def step_repl_input_multiline(context: Context) -> None: + context.cov_repl_lines = ["version \\", "", ":exit"] + + +@given("the REPL input sequence for interrupt then exit") +def step_repl_input_kbi(context: Context) -> None: + context.cov_repl_lines = ["__KEYBOARD_INTERRUPT__", ":exit"] + + +@given("the REPL input sequence has an unclosed quote then exit") +def step_repl_input_shlex_error(context: Context) -> None: + context.cov_repl_lines = ['"unclosed', ":exit"] + + +@given("dispatch_command is mocked for run_repl") +def step_mock_dispatch_for_repl(context: Context) -> None: + context.mock_dispatch_flag = True + + +# --- When steps --- + + +@when("I run the REPL loop") +def step_run_repl_loop(context: Context) -> None: + mock_dispatch = getattr(context, "mock_dispatch_flag", False) + code, output, mock_d, _, _ = _run_repl_cov( + context.cov_repl_lines, + no_history=True, + mock_dispatch=mock_dispatch, + ) + context.cov_repl_exit_code = code + context.cov_repl_output = output + context.cov_mock_dispatch = mock_d + + +@when("I run the REPL loop with history enabled") +def step_run_repl_loop_history(context: Context) -> None: + code, output, _, mock_setup, mock_save = _run_repl_cov( + context.cov_repl_lines, + no_history=False, + mock_setup_hist=True, + mock_save_hist=True, + ) + context.cov_repl_exit_code = code + context.cov_repl_output = output + context.cov_mock_setup_hist = mock_setup + context.cov_mock_save_hist = mock_save + + +@when("I run the REPL loop with no_history") +def step_run_repl_loop_no_history(context: Context) -> None: + code, output, _, mock_setup, mock_save = _run_repl_cov( + context.cov_repl_lines, + no_history=True, + mock_setup_hist=True, + mock_save_hist=True, + ) + context.cov_repl_exit_code = code + context.cov_repl_output = output + context.cov_mock_setup_hist = mock_setup + context.cov_mock_save_hist = mock_save + + +# =================================================================== +# run_repl — Then steps +# =================================================================== + + +@then("the REPL loop exit code is {code:d}") +def step_check_repl_loop_ec(context: Context, code: int) -> None: + assert context.cov_repl_exit_code == code, ( + f"Expected {code}, got {context.cov_repl_exit_code}" + ) + + +@then('the REPL loop output contains "{text}"') +def step_repl_loop_output_contains(context: Context, text: str) -> None: + assert text in context.cov_repl_output, ( + f"Expected {text!r} in output:\n{context.cov_repl_output}" + ) + + +@then('dispatch_command was called at least 2 times with "{arg}"') +def step_dispatch_called_twice(context: Context, arg: str) -> None: + assert context.cov_mock_dispatch is not None + calls_matching = [ + c for c in context.cov_mock_dispatch.call_args_list if c == call([arg]) + ] + assert len(calls_matching) >= 2, ( + f"Expected dispatch(['{arg}']) >= 2 times, got {len(calls_matching)}: " + f"{context.cov_mock_dispatch.call_args_list}" + ) + + +@then('dispatch_command was called with args "{arg}"') +def step_dispatch_called_with(context: Context, arg: str) -> None: + assert context.cov_mock_dispatch is not None + context.cov_mock_dispatch.assert_any_call([arg]) + + +@then("_setup_history was called") +def step_setup_hist_called(context: Context) -> None: + assert context.cov_mock_setup_hist.called + + +@then("_setup_history was not called") +def step_setup_hist_not_called(context: Context) -> None: + assert not context.cov_mock_setup_hist.called + + +@then("_save_history was called") +def step_save_hist_called(context: Context) -> None: + assert context.cov_mock_save_hist.called + + +@then("_save_history was not called") +def step_save_hist_not_called(context: Context) -> None: + assert not context.cov_mock_save_hist.called + + +# =================================================================== +# repl_callback +# =================================================================== + + +@given("sys.stdout.isatty returns True") +def step_isatty_true(context: Context) -> None: + context.isatty_value = True + + +@given("sys.stdout.isatty returns False") +def step_isatty_false(context: Context) -> None: + context.isatty_value = False + + +@given("run_repl is mocked to return 0") +def step_mock_run_repl(context: Context) -> None: + context.mock_run_repl_rc = 0 + + +@given("CLEVERAGENTS_FORCE_REPL is set in the environment") +def step_force_repl_set(context: Context) -> None: + os.environ["CLEVERAGENTS_FORCE_REPL"] = "1" + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append( + lambda: os.environ.pop("CLEVERAGENTS_FORCE_REPL", None) + ) + + +@given("CLEVERAGENTS_FORCE_REPL is not set") +def step_force_repl_unset(context: Context) -> None: + os.environ.pop("CLEVERAGENTS_FORCE_REPL", None) + + +@when("I call repl_callback") +def step_call_repl_callback(context: Context) -> None: + from cleveragents.cli.commands.repl import repl_callback + + mock_isatty = MagicMock(return_value=context.isatty_value) + mock_run = MagicMock(return_value=getattr(context, "mock_run_repl_rc", 0)) + + context.cov_repl_callback_exc = None + context.cov_mock_run_repl = mock_run + + with ( + patch("cleveragents.cli.commands.repl.sys") as mock_sys, + patch("cleveragents.cli.commands.repl.run_repl", mock_run), + ): + mock_sys.stdout.isatty = mock_isatty + try: + repl_callback(no_history=False, history_path=Path("/tmp/fake_hist")) + except SystemExit as exc: + context.cov_repl_callback_exc = exc + + +@then("run_repl was invoked") +def step_run_repl_invoked(context: Context) -> None: + assert context.cov_mock_run_repl.called, "run_repl was not called" + + +@then("run_repl was not invoked") +def step_run_repl_not_invoked(context: Context) -> None: + assert not context.cov_mock_run_repl.called, "run_repl should not have been called" + + +@then("repl_callback raises SystemExit with code 0") +def step_callback_sysexit_0(context: Context) -> None: + assert context.cov_repl_callback_exc is not None, "Expected SystemExit" + assert context.cov_repl_callback_exc.code == 0, ( + f"Expected SystemExit(0), got SystemExit({context.cov_repl_callback_exc.code})" + ) diff --git a/features/steps/resource_handler_service_coverage_steps.py b/features/steps/resource_handler_service_coverage_steps.py new file mode 100644 index 000000000..dccd7b9d8 --- /dev/null +++ b/features/steps/resource_handler_service_coverage_steps.py @@ -0,0 +1,514 @@ +"""Step definitions for ResourceHandlerService full coverage tests. + +All step patterns are prefixed with 'rhs' to avoid collisions with other +step definition files in the same features/steps directory. +""" + +from __future__ import annotations + +import contextlib +from unittest.mock import MagicMock, patch + +from behave import given, then, when + +from cleveragents.application.services.resource_handler_service import ( + ResourceHandlerService, +) +from cleveragents.core.exceptions import NotFoundError +from cleveragents.domain.models.core.resource_slot import BindingResult +from cleveragents.resource.handlers.resolver import HandlerResolutionError +from cleveragents.tool.context import BoundResource + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_resource_mock( + resource_id, + resource_type_name, + location=None, + sandbox_strategy=None, +): + """Create a mock Resource with the given attributes.""" + resource = MagicMock() + resource.resource_id = resource_id + resource.resource_type_name = resource_type_name + resource.location = location + # sandbox_strategy=None means no override; "none" is a valid strategy string + resource.sandbox_strategy = sandbox_strategy + return resource + + +def _make_type_spec_mock( + name, + handler=None, + sandbox_strategy="copy_on_write", +): + """Create a mock ResourceTypeSpec with the given attributes.""" + spec = MagicMock() + spec.name = name + spec.handler = handler + spec.sandbox_strategy = sandbox_strategy + return spec + + +def _make_binding_obj( + slot_name, + resource_id=None, + deferred=False, +): + """Create a BindingResult with the given attributes.""" + return BindingResult( + slot_name=slot_name, + resource_id=resource_id, + binding_mode="parameter" if deferred else "contextual", + deferred=deferred, + ) + + +def _make_sandbox_mock(sandbox_path): + """Create a mock Sandbox with a context carrying sandbox_path.""" + sandbox = MagicMock() + if sandbox_path is not None: + sandbox.context = MagicMock() + sandbox.context.sandbox_path = sandbox_path + else: + sandbox.context = None + return sandbox + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("a mock sandbox manager for rhs") +def step_rhs_mock_sandbox_manager(context): + context.rhs_sandbox_manager = MagicMock() + context.rhs_sandbox_call_args = None + + +@given("a mock resource lookup for rhs") +def step_rhs_mock_resource_lookup(context): + context.rhs_resources = {} + context.rhs_resource_lookup_error = {} + + def _lookup(name_or_id): + if name_or_id in context.rhs_resource_lookup_error: + raise context.rhs_resource_lookup_error[name_or_id] + return context.rhs_resources[name_or_id] + + context.rhs_resource_lookup = _lookup + + +@given("a mock type lookup for rhs") +def step_rhs_mock_type_lookup(context): + context.rhs_type_specs = {} + + def _lookup(name): + return context.rhs_type_specs[name] + + context.rhs_type_lookup = _lookup + + +@given("a ResourceHandlerService under test") +def step_rhs_service_instance(context): + context.rhs_service = ResourceHandlerService( + sandbox_manager=context.rhs_sandbox_manager, + resource_lookup=context.rhs_resource_lookup, + type_lookup=context.rhs_type_lookup, + ) + context.rhs_result = None + context.rhs_error = None + context.rhs_resolve_handler_patcher = None + + +# --------------------------------------------------------------------------- +# Binding setup +# --------------------------------------------------------------------------- + + +@given('an rhs binding that is deferred with slot name "{slot}"') +def step_rhs_deferred_binding(context, slot): + context.rhs_binding = _make_binding_obj(slot_name=slot, deferred=True) + + +@given('an rhs binding with no resource_id and slot name "{slot}"') +def step_rhs_binding_no_resource_id(context, slot): + context.rhs_binding = _make_binding_obj( + slot_name=slot, resource_id=None, deferred=False + ) + + +@given('an rhs non-deferred binding with slot "{slot}" and resource_id "{rid}"') +def step_rhs_non_deferred_binding(context, slot, rid): + context.rhs_binding = _make_binding_obj( + slot_name=slot, resource_id=rid, deferred=False + ) + + +# --------------------------------------------------------------------------- +# Resource setup — single step for location + strategy +# --------------------------------------------------------------------------- + + +@given( + 'rhs resource "{rid}" typed "{rtype}" located at "{loc}" with strategy "{strat}"' +) +def step_rhs_resource_with_loc_and_strat(context, rid, rtype, loc, strat): + # Convert the string "none" to Python None for sandbox_strategy + strategy = None if strat == "none" else strat + context.rhs_resources[rid] = _make_resource_mock( + resource_id=rid, + resource_type_name=rtype, + location=loc, + sandbox_strategy=strategy, + ) + + +@given('rhs resource "{rid}" typed "{rtype}" with no location') +def step_rhs_resource_no_location(context, rid, rtype): + context.rhs_resources[rid] = _make_resource_mock( + resource_id=rid, + resource_type_name=rtype, + location=None, + sandbox_strategy=None, + ) + + +# --------------------------------------------------------------------------- +# Type spec setup +# --------------------------------------------------------------------------- + + +@given('an rhs type spec "{name}" with no handler and sandbox strategy "{strat}"') +def step_rhs_type_spec_no_handler(context, name, strat): + context.rhs_type_specs[name] = _make_type_spec_mock( + name=name, handler=None, sandbox_strategy=strat + ) + + +@given( + 'an rhs type spec "{name}" with handler "{handler}" and sandbox strategy "{strat}"' +) +def step_rhs_type_spec_with_handler(context, name, handler, strat): + context.rhs_type_specs[name] = _make_type_spec_mock( + name=name, handler=handler, sandbox_strategy=strat + ) + + +# --------------------------------------------------------------------------- +# Sandbox manager setup +# --------------------------------------------------------------------------- + + +@given('the rhs sandbox manager returns a sandbox with path "{path}"') +def step_rhs_sandbox_returns_path(context, path): + sandbox = _make_sandbox_mock(sandbox_path=path) + + def _capture_call(**kwargs): + context.rhs_sandbox_call_args = kwargs + return sandbox + + context.rhs_sandbox_manager.get_or_create_sandbox = MagicMock( + side_effect=_capture_call + ) + + +@given("the rhs sandbox manager returns a sandbox with no context") +def step_rhs_sandbox_returns_no_context(context): + sandbox = _make_sandbox_mock(sandbox_path=None) + context.rhs_sandbox_manager.get_or_create_sandbox = MagicMock(return_value=sandbox) + + +# --------------------------------------------------------------------------- +# Handler mock setup +# --------------------------------------------------------------------------- + + +@given("the rhs resolve_handler function returns a mock handler") +def step_rhs_resolve_handler_returns_mock(context): + context.rhs_mock_handler = MagicMock() + context.rhs_resolve_handler_patcher = patch( + "cleveragents.application.services.resource_handler_service.resolve_handler", + return_value=context.rhs_mock_handler, + ) + context.rhs_resolve_handler_patcher.start() + + +@given('the rhs mock handler resolves to a BoundResource with sandbox_path "{path}"') +def step_rhs_mock_handler_returns_bound(context, path): + def _resolve(**kwargs): + return BoundResource( + slot_name=kwargs["slot_name"], + resource_id=kwargs["resource"].resource_id, + resource_type=kwargs["resource"].resource_type_name, + sandbox_path=path, + access=kwargs.get("access", "read_only"), + ) + + context.rhs_mock_handler.resolve = MagicMock(side_effect=_resolve) + + +@given("the rhs resolve_handler function raises HandlerResolutionError") +def step_rhs_resolve_handler_raises(context): + context.rhs_resolve_handler_patcher = patch( + "cleveragents.application.services.resource_handler_service.resolve_handler", + side_effect=HandlerResolutionError("handler not found"), + ) + context.rhs_resolve_handler_patcher.start() + + +# --------------------------------------------------------------------------- +# Error setup for resource lookup +# --------------------------------------------------------------------------- + + +@given('the rhs resource lookup raises NotFoundError for "{rid}"') +def step_rhs_resource_lookup_raises(context, rid): + context.rhs_resource_lookup_error[rid] = NotFoundError( + message=f"Resource '{rid}' not found" + ) + + +# --------------------------------------------------------------------------- +# Binding list setup +# --------------------------------------------------------------------------- + + +@given("an rhs binding list with:") +def step_rhs_binding_list(context): + context.rhs_bindings = [] + for row in context.table: + slot_name = row["slot_name"] + resource_id = row["resource_id"] if row["resource_id"] else None + deferred = row["deferred"].lower() == "true" + context.rhs_bindings.append( + _make_binding_obj( + slot_name=slot_name, resource_id=resource_id, deferred=deferred + ) + ) + + +# --------------------------------------------------------------------------- +# Cleanup helper +# --------------------------------------------------------------------------- + + +def _rhs_cleanup_patcher(context): + """Stop the resolve_handler patcher if active.""" + patcher = getattr(context, "rhs_resolve_handler_patcher", None) + if patcher is not None: + with contextlib.suppress(RuntimeError): + patcher.stop() + context.rhs_resolve_handler_patcher = None + + +# --------------------------------------------------------------------------- +# When: resolve_binding +# --------------------------------------------------------------------------- + + +@when("I resolve the rhs single binding") +def step_rhs_resolve_single_binding_default(context): + try: + context.rhs_result = context.rhs_service.resolve_binding( + binding=context.rhs_binding, + plan_id="default-plan", + access="read_only", + ) + except Exception as exc: + context.rhs_error = exc + finally: + _rhs_cleanup_patcher(context) + + +@when('I resolve the rhs single binding with plan_id "{pid}" and access "{access}"') +def step_rhs_resolve_single_binding(context, pid, access): + try: + context.rhs_result = context.rhs_service.resolve_binding( + binding=context.rhs_binding, + plan_id=pid, + access=access, + ) + except Exception as exc: + context.rhs_error = exc + finally: + _rhs_cleanup_patcher(context) + + +# --------------------------------------------------------------------------- +# When: resolve_bindings +# --------------------------------------------------------------------------- + + +@when('I resolve all rhs bindings with plan_id "{pid}" and access "{access}"') +def step_rhs_resolve_all_bindings(context, pid, access): + try: + context.rhs_result = context.rhs_service.resolve_bindings( + bindings=context.rhs_bindings, + plan_id=pid, + access=access, + ) + except Exception as exc: + context.rhs_error = exc + finally: + _rhs_cleanup_patcher(context) + + +@when("I resolve all rhs bindings expecting an error") +def step_rhs_resolve_all_bindings_error(context): + try: + context.rhs_result = context.rhs_service.resolve_bindings( + bindings=context.rhs_bindings, + plan_id="error-plan", + access="read_only", + ) + except Exception as exc: + context.rhs_error = exc + finally: + _rhs_cleanup_patcher(context) + + +# --------------------------------------------------------------------------- +# When: resolve_resource +# --------------------------------------------------------------------------- + + +@when( + 'I resolve the rhs resource directly with plan_id "{pid}" slot "{slot}" and access "{access}"' +) +def step_rhs_resolve_resource_directly(context, pid, slot, access): + # Use the most recently added resource + rid = list(context.rhs_resources.keys())[-1] + resource = context.rhs_resources[rid] + try: + context.rhs_result = context.rhs_service.resolve_resource( + resource=resource, + plan_id=pid, + slot_name=slot, + access=access, + ) + except Exception as exc: + context.rhs_error = exc + finally: + _rhs_cleanup_patcher(context) + + +# --------------------------------------------------------------------------- +# Then: BoundResource assertions +# --------------------------------------------------------------------------- + + +@then('the rhs result is a BoundResource with slot "{slot}" and sandbox_path "{path}"') +def step_rhs_assert_bound_resource(context, slot, path): + assert context.rhs_error is None, f"Unexpected error: {context.rhs_error}" + assert isinstance(context.rhs_result, BoundResource), ( + f"Expected BoundResource, got {type(context.rhs_result)}" + ) + assert context.rhs_result.slot_name == slot, ( + f"Expected slot_name '{slot}', got '{context.rhs_result.slot_name}'" + ) + assert context.rhs_result.sandbox_path == path, ( + f"Expected sandbox_path '{path}', got '{context.rhs_result.sandbox_path}'" + ) + + +@then('the rhs BoundResource has resource_id "{rid}" and access "{access}"') +def step_rhs_assert_bound_resource_details(context, rid, access): + assert context.rhs_result.resource_id == rid, ( + f"Expected resource_id '{rid}', got '{context.rhs_result.resource_id}'" + ) + assert context.rhs_result.access == access, ( + f"Expected access '{access}', got '{context.rhs_result.access}'" + ) + + +# --------------------------------------------------------------------------- +# Then: error assertions +# --------------------------------------------------------------------------- + + +@then('an rhs ValueError is raised with message containing "{fragment}"') +def step_rhs_assert_value_error(context, fragment): + assert context.rhs_error is not None, ( + "Expected a ValueError but no error was raised" + ) + assert isinstance(context.rhs_error, ValueError), ( + f"Expected ValueError, got {type(context.rhs_error).__name__}: {context.rhs_error}" + ) + assert fragment in str(context.rhs_error), ( + f"Expected '{fragment}' in error message, got: {context.rhs_error}" + ) + + +@then("an rhs NotFoundError is raised") +def step_rhs_assert_not_found_error(context): + assert context.rhs_error is not None, ( + "Expected a NotFoundError but no error was raised" + ) + assert isinstance(context.rhs_error, NotFoundError), ( + f"Expected NotFoundError, got {type(context.rhs_error).__name__}: {context.rhs_error}" + ) + + +@then('an rhs RuntimeError is raised with message containing "{fragment}"') +def step_rhs_assert_runtime_error(context, fragment): + assert context.rhs_error is not None, ( + "Expected a RuntimeError but no error was raised" + ) + assert isinstance(context.rhs_error, RuntimeError), ( + f"Expected RuntimeError, got {type(context.rhs_error).__name__}: {context.rhs_error}" + ) + assert fragment in str(context.rhs_error), ( + f"Expected '{fragment}' in error message, got: {context.rhs_error}" + ) + + +# --------------------------------------------------------------------------- +# Then: dict result assertions +# --------------------------------------------------------------------------- + + +@then('the rhs result dict has keys "{key1}" and "{key2}"') +def step_rhs_assert_dict_keys(context, key1, key2): + assert context.rhs_error is None, f"Unexpected error: {context.rhs_error}" + assert isinstance(context.rhs_result, dict), ( + f"Expected dict, got {type(context.rhs_result)}" + ) + assert key1 in context.rhs_result, f"Key '{key1}' not found in result dict" + assert key2 in context.rhs_result, f"Key '{key2}' not found in result dict" + + +@then('the rhs result dict does not contain key "{key}"') +def step_rhs_assert_dict_missing_key(context, key): + assert key not in context.rhs_result, ( + f"Key '{key}' should not be in result dict but was found" + ) + + +@then("the rhs result dict is empty") +def step_rhs_assert_dict_empty(context): + assert context.rhs_error is None, f"Unexpected error: {context.rhs_error}" + assert isinstance(context.rhs_result, dict), ( + f"Expected dict, got {type(context.rhs_result)}" + ) + assert len(context.rhs_result) == 0, ( + f"Expected empty dict, got {len(context.rhs_result)} entries" + ) + + +# --------------------------------------------------------------------------- +# Then: sandbox manager call assertions +# --------------------------------------------------------------------------- + + +@then('the rhs sandbox manager was called with strategy "{strategy}"') +def step_rhs_assert_sandbox_strategy(context, strategy): + assert context.rhs_error is None, f"Unexpected error: {context.rhs_error}" + assert context.rhs_sandbox_call_args is not None, ( + "sandbox_manager.get_or_create_sandbox was not called" + ) + actual = context.rhs_sandbox_call_args.get("sandbox_strategy") + assert actual == strategy, f"Expected sandbox_strategy '{strategy}', got '{actual}'" diff --git a/features/steps/skill_cli_coverage_steps.py b/features/steps/skill_cli_coverage_steps.py new file mode 100644 index 000000000..29b16fa6f --- /dev/null +++ b/features/steps/skill_cli_coverage_steps.py @@ -0,0 +1,201 @@ +# pyright: reportRedeclaration=false +"""Step definitions for skill CLI uncovered-lines coverage.""" + +from __future__ import annotations + +from unittest.mock import patch + +from behave import given, then, when +from typer.testing import CliRunner + +from cleveragents.cli.commands.skill import ( + _get_skill_service, + _reset_skill_service, +) +from cleveragents.cli.commands.skill import ( + app as skill_app, +) +from cleveragents.domain.models.core.skill import ( + Skill, + SkillAgentSource, + SkillInclude, + SkillMcpSource, +) + +# ── helpers ───────────────────────────────────────────────── + + +def _make_skill( + name: str, + description: str = "test skill", + tool_refs: list[str] | None = None, + includes: list[SkillInclude] | None = None, + mcp_servers: list[SkillMcpSource] | None = None, + agent_skills: list[SkillAgentSource] | None = None, +) -> Skill: + return Skill( + name=name, + description=description, + tool_refs=tool_refs or [], + includes=includes or [], + mcp_servers=mcp_servers or [], + agent_skills=agent_skills or [], + ) + + +# ── Given ─────────────────────────────────────────────────── + + +@given("a clean skill service") +def step_clean_skill_service(context): + _reset_skill_service() + context.runner = CliRunner() + context.service = _get_skill_service() + + +@given("a skill with an include pointing to a non-registered skill") +def step_skill_with_unregistered_include(context): + skill = _make_skill( + name="local/inc-parent", + includes=[SkillInclude(name="local/missing-child")], + ) + context.service._skills[skill.name] = skill + context.skill_name = skill.name + + +@given("a registered skill for capability error testing") +def step_skill_for_capability_error(context): + skill = _make_skill( + name="local/cap-error", + tool_refs=["builtin/read-file"], + ) + context.service._skills[skill.name] = skill + context.skill_name = skill.name + + +@given("a registered skill for dependent actors testing") +def step_skill_for_dependent_actors(context): + skill = _make_skill( + name="local/dep-actors", + tool_refs=["builtin/write-file"], + ) + context.service._skills[skill.name] = skill + context.skill_name = skill.name + + +@given("a registered skill with agent_skill sources") +def step_skill_with_agent_skills(context): + skill = _make_skill( + name="local/with-agents", + agent_skills=[SkillAgentSource(path="./skills/my-agent")], + ) + context.service._skills[skill.name] = skill + context.skill_name = skill.name + + +@given("a registered skill for removal with actor dependents") +def step_skill_for_removal_actors(context): + skill = _make_skill( + name="local/removable-actors", + tool_refs=["builtin/exec"], + ) + context.service._skills[skill.name] = skill + context.skill_name = skill.name + + +# ── When ──────────────────────────────────────────────────── + + +@when("I run skill show for the skill with unregistered include") +def step_run_show_unregistered_include(context): + context.result = context.runner.invoke(skill_app, ["show", context.skill_name]) + + +@when("I run skill show with capability summary raising ValueError") +def step_run_show_capability_error(context): + with patch.object( + context.service, + "compute_capability_summary", + side_effect=ValueError("boom"), + ): + context.result = context.runner.invoke(skill_app, ["show", context.skill_name]) + + +@when("I run skill show with dependents returning actors") +def step_run_show_dependent_actors(context): + with patch.object( + context.service, + "get_dependents", + return_value={"skills": [], "actors": ["actor/alpha", "actor/beta"]}, + ): + context.result = context.runner.invoke(skill_app, ["show", context.skill_name]) + + +@when("I run skill tools for the agent_skill skill") +def step_run_tools_agent_skill(context): + context.result = context.runner.invoke(skill_app, ["tools", context.skill_name]) + + +@when("I run skill remove with dependents returning actors") +def step_run_remove_with_actor_dependents(context): + with patch.object( + context.service, + "get_dependents", + return_value={ + "skills": [], + "actors": ["actor/gamma", "actor/delta"], + }, + ): + context.result = context.runner.invoke( + skill_app, + ["remove", context.skill_name, "--yes"], + ) + + +# ── Then ──────────────────────────────────────────────────── + + +@then("the show output contains the not registered marker") +def step_show_not_registered(context): + assert context.result.exit_code == 0, ( + f"exit_code={context.result.exit_code}\n{context.result.output}" + ) + assert "not registered" in context.result.output + + +@then("the show output contains OK and no capability panel") +def step_show_ok_no_capability(context): + assert context.result.exit_code == 0, ( + f"exit_code={context.result.exit_code}\n{context.result.output}" + ) + assert "OK" in context.result.output + # The capability panel should be absent because the exception was caught + assert "Capability Summary" not in context.result.output + + +@then("the show output contains the actors reference line") +def step_show_actors_reference(context): + assert context.result.exit_code == 0, ( + f"exit_code={context.result.exit_code}\n{context.result.output}" + ) + assert "actor/alpha" in context.result.output + assert "actor/beta" in context.result.output + assert "Actors" in context.result.output + + +@then("the tools output contains agent_skill source type") +def step_tools_agent_skill_source(context): + assert context.result.exit_code == 0, ( + f"exit_code={context.result.exit_code}\n{context.result.output}" + ) + assert "agent_skill" in context.result.output + + +@then("the remove output contains the actor dependency warning") +def step_remove_actor_warning(context): + assert context.result.exit_code == 0, ( + f"exit_code={context.result.exit_code}\n{context.result.output}" + ) + assert "actor(s) reference this skill" in context.result.output + assert "actor/gamma" in context.result.output + assert "actor/delta" in context.result.output diff --git a/features/steps/skill_registry_steps.py b/features/steps/skill_registry_steps.py index 028239713..b13c48bca 100644 --- a/features/steps/skill_registry_steps.py +++ b/features/steps/skill_registry_steps.py @@ -179,7 +179,7 @@ def step_register_skills_table(context: Context) -> None: description=row["description"], ) svc.add_skill(skill) - _commit_pending(context) + _commit_pending(context) @given('a valid skill named "{name}" with overrides for "{tool_name}"') -- 2.52.0