diff --git a/features/bridge_remaining_coverage.feature b/features/bridge_remaining_coverage.feature new file mode 100644 index 000000000..d3a4d545a --- /dev/null +++ b/features/bridge_remaining_coverage.feature @@ -0,0 +1,63 @@ +Feature: Bridge remaining coverage gaps + As a developer + I want targeted tests for remaining missed lines and branches in bridge.py + So that coverage approaches 100% + + # Targets: line 35 (cancellation_reasons init), line 68 (task.cancel in async cleanup), + # lines 201-204 (string content branch in execute_graph), + # branches 39→38, 67→68, 182→184, 260→258 + + Scenario: Cancellation reason is recorded when cancelling a tracked task + Given a bridge instance prepared for cancellation reason tracking + When I cancel a tracked task providing reason "user requested abort" + Then the bridge cancellation_reasons mapping should contain the task + And the stored reason should equal "user requested abort" + + Scenario: Cancelling a None task raises a ValueError + Given a bridge instance prepared for cancellation reason tracking + When I try cancelling a None task with reason "some reason" + Then a ValueError mentioning "task must not be None" should be raised + + Scenario: Cancelling a task with empty reason raises a ValueError + Given a bridge instance prepared for cancellation reason tracking + When I try cancelling a valid task with an empty reason string + Then a ValueError mentioning "reason must be a non-empty string" should be raised + + Scenario: Async cleanup cancels a not-yet-done task on the bridge + Given a bridge holding a deliberately stalled coroutine task + When I perform async cleanup with a 200ms timeout on stalled tasks + Then every stalled task should have been cancelled or finished + And the bridge task tracking set should be empty after async cleanup + + Scenario: Async cleanup handles already-completed tasks without error + Given a bridge holding an already-finished async task + When I perform async cleanup with a 200ms timeout on stalled tasks + Then the bridge task tracking set should be empty after async cleanup + + Scenario: Graph executor inner coroutine processes string content directly + Given a bridge with a mock graph wired for direct coroutine invocation + When I invoke the executor coroutine with a plain string message body + Then the mock graph execute should have received a messages list with the string + And the executor coroutine should return a StreamMessage with graph metadata + + Scenario: Graph executor inner coroutine processes list content as fallback + Given a bridge with a mock graph wired for direct coroutine invocation + When I invoke the executor coroutine with a list payload as message body + Then the mock graph execute should have received a content key wrapping the list + + Scenario: Graph stream config includes correct type and publication fields + Given a bridge with a registered graph called "stream_verify" + When I obtain the stream configuration for graph "stream_verify" + Then the stream config name should equal "graph_stream_verify" + And the stream config publications should include "__output__" + + Scenario: State checkpointer operator applies to a message for a valid graph + Given a bridge with a registered graph called "ckpt_valid" + And the graph "ckpt_valid" has a checkpoint directory configured + When I construct and apply the checkpointer operator for "ckpt_valid" + Then the checkpointer should have invoked _save_checkpoint on the state manager + + Scenario: The __del__ method completes normally when cleanup_tasks succeeds + Given a fully functional bridge for destructor testing + When __del__ is called on the fully functional bridge + Then the __del__ call should complete without any error diff --git a/features/checkpoint_manager_coverage.feature b/features/checkpoint_manager_coverage.feature new file mode 100644 index 000000000..d4334cc58 --- /dev/null +++ b/features/checkpoint_manager_coverage.feature @@ -0,0 +1,195 @@ +Feature: CheckpointManager full coverage + As a developer + I want thorough tests for every method, branch, and edge case in checkpoint.py + So that line-rate and branch-rate reach 1.0 + + # --------------------------------------------------------------------------- + # SandboxCheckpoint Pydantic model + # --------------------------------------------------------------------------- + + Scenario: SandboxCheckpoint model can be constructed with all fields + Given I build a SandboxCheckpoint with valid fields + Then the constructed checkpoint model should expose all field values + And the constructed checkpoint model should be frozen + + Scenario: SandboxCheckpoint model uses empty dict as default metadata + Given I build a SandboxCheckpoint without explicit metadata + Then the constructed checkpoint metadata should be an empty dict + + # --------------------------------------------------------------------------- + # Checkpointable protocol + # --------------------------------------------------------------------------- + + Scenario: An object satisfying Checkpointable passes isinstance check + Given a mock object with sandbox_id and context properties + Then the mock object should satisfy the Checkpointable protocol + + Scenario: An object missing sandbox_id fails Checkpointable isinstance check + Given a mock object without a sandbox_id property + Then the mock object should not satisfy the Checkpointable protocol + + # --------------------------------------------------------------------------- + # CheckpointManager.__init__ + # --------------------------------------------------------------------------- + + Scenario: CheckpointManager initialises with empty state + Given a freshly created CheckpointManager + Then the manager internal checkpoints dict should be empty + And the manager should have a threading lock + + # --------------------------------------------------------------------------- + # create_checkpoint – context is None (metadata-only) + # --------------------------------------------------------------------------- + + Scenario: Creating a checkpoint when sandbox context is None produces a metadata-only snapshot + Given a freshly created CheckpointManager + And a mock sandbox whose context is None + When I invoke create_checkpoint with plan "plan-m1" phase "pre_execute" and no metadata + Then the returned checkpoint should have a non-empty snapshot_path directory + And the returned checkpoint sandbox_id should equal the mock sandbox id + + Scenario: Creating a checkpoint with explicit metadata converts values to strings + Given a freshly created CheckpointManager + And a mock sandbox whose context is None + When I invoke create_checkpoint with plan "plan-m2" phase "post_execute" and numeric metadata + Then the returned checkpoint metadata values should all be strings + + Scenario: Creating a checkpoint with None metadata defaults to empty dict + Given a freshly created CheckpointManager + And a mock sandbox whose context is None + When I invoke create_checkpoint with plan "plan-m3" phase "pre_apply" and None metadata + Then the returned checkpoint metadata should be an empty dict + + # --------------------------------------------------------------------------- + # create_checkpoint – context with sandbox_path + # --------------------------------------------------------------------------- + + Scenario: Creating a checkpoint with a real sandbox path snapshots the directory + Given a freshly created CheckpointManager + And a temporary sandbox directory with sample files + And a mock sandbox whose context points to the temporary directory + When I invoke create_checkpoint with plan "plan-s1" phase "pre_execute" and no metadata + Then the snapshot directory should contain copies of the sample files + + # --------------------------------------------------------------------------- + # _snapshot_directory – branches + # --------------------------------------------------------------------------- + + Scenario: _snapshot_directory with None path creates an empty snapshot dir + Given a freshly created CheckpointManager + When I call _snapshot_directory with a None sandbox_path + Then the returned snapshot should be an existing empty directory + + Scenario: _snapshot_directory with non-existent path creates an empty snapshot dir + Given a freshly created CheckpointManager + When I call _snapshot_directory with a non-existent sandbox_path + Then the returned snapshot should be an existing empty directory + + Scenario: _snapshot_directory with a valid path copies the tree + Given a freshly created CheckpointManager + And a temporary sandbox directory with sample files + When I call _snapshot_directory with the temporary sandbox_path + Then the returned snapshot should contain the sample files + + Scenario: _snapshot_directory raises SandboxError when copytree fails + Given a freshly created CheckpointManager + And a temporary sandbox directory with sample files + When I call _snapshot_directory with a path that triggers OSError + Then a SandboxError should have been raised + + # --------------------------------------------------------------------------- + # _cleanup_snapshot – branches + # --------------------------------------------------------------------------- + + Scenario: _cleanup_snapshot with empty string is a no-op + Given a freshly created CheckpointManager + When I call _cleanup_snapshot with an empty string + Then no error should occur from cleanup_snapshot + + Scenario: _cleanup_snapshot removes parent directory when it exists + Given a freshly created CheckpointManager + And a temporary snapshot directory tree for cleanup + When I call _cleanup_snapshot with the temporary snapshot path + Then the temporary snapshot parent should no longer exist + + Scenario: _cleanup_snapshot with non-existent parent is a no-op + Given a freshly created CheckpointManager + When I call _cleanup_snapshot with a non-existent path + Then no error should occur from cleanup_snapshot + + # --------------------------------------------------------------------------- + # rollback_to – all branches + # --------------------------------------------------------------------------- + + Scenario: Rollback returns false when snapshot_path is empty string + Given a freshly created CheckpointManager + And a SandboxCheckpoint with empty snapshot_path + When I invoke rollback_to on the crafted checkpoint + Then the rollback result should be false + + Scenario: Rollback returns false when snapshot_path does not exist on disk + Given a freshly created CheckpointManager + And a SandboxCheckpoint with a non-existent snapshot_path + When I invoke rollback_to on the crafted checkpoint + Then the rollback result should be false + + Scenario: Rollback returns false when sandbox_path is missing from metadata + Given a freshly created CheckpointManager + And a SandboxCheckpoint whose snapshot exists but metadata has no sandbox_path + When I invoke rollback_to on the crafted checkpoint + Then the rollback result should be false + + Scenario: Rollback returns false when sandbox_path directory does not exist + Given a freshly created CheckpointManager + And a SandboxCheckpoint whose snapshot exists but sandbox_path points nowhere + When I invoke rollback_to on the crafted checkpoint + Then the rollback result should be false + + Scenario: Rollback succeeds and restores files and subdirectories + Given a freshly created CheckpointManager + And a temporary sandbox directory with sample files + And a mock sandbox whose context points to the temporary directory + And a checkpoint was created for the temporary sandbox + And the temporary sandbox files are then modified + When I invoke rollback_to on the created checkpoint + Then the rollback result should be true + And the temporary sandbox should contain the original files + + Scenario: Rollback handles OSError during restore gracefully + Given a freshly created CheckpointManager + And a SandboxCheckpoint whose snapshot and sandbox exist but restore will fail + When I invoke rollback_to on the crafted checkpoint + Then the rollback result should be false + + # --------------------------------------------------------------------------- + # list_checkpoints – branches + # --------------------------------------------------------------------------- + + Scenario: Listing checkpoints for unknown sandbox returns empty list + Given a freshly created CheckpointManager + When I invoke list_checkpoints for sandbox "unknown-sb-99" + Then the checkpoint list should be empty + + Scenario: Listing checkpoints returns all created checkpoints in order + Given a freshly created CheckpointManager + And a mock sandbox whose context is None + And I create two checkpoints for the mock sandbox + When I invoke list_checkpoints for the mock sandbox + Then the checkpoint list should have 2 entries in creation order + + # --------------------------------------------------------------------------- + # delete_checkpoint – branches + # --------------------------------------------------------------------------- + + Scenario: Deleting a known checkpoint returns true and removes it + Given a freshly created CheckpointManager + And a mock sandbox whose context is None + And a single checkpoint is created for the mock sandbox + When I invoke delete_checkpoint with the created checkpoint id + Then the checkpoint delete result should be true + And listing checkpoints for the mock sandbox should return 0 entries + + Scenario: Deleting an unknown checkpoint id returns false + Given a freshly created CheckpointManager + When I invoke delete_checkpoint with id "no-such-cp-id" + Then the checkpoint delete result should be false diff --git a/features/config_cli_safety_net_coverage.feature b/features/config_cli_safety_net_coverage.feature new file mode 100644 index 000000000..815fc5878 --- /dev/null +++ b/features/config_cli_safety_net_coverage.feature @@ -0,0 +1,287 @@ +Feature: Config CLI safety-net coverage + As a developer + I want thorough safety-net tests for every function in config.py + So that 100% line and branch coverage is maintained as the code evolves + + # ===================================================================== + # _normalize_key (L91-96) + # ===================================================================== + + Scenario: safety-net _normalize_key converts dots to underscores + When the safety-net normalizer processes key "server.port" + Then the safety-net normalized result should be "server_port" + + Scenario: safety-net _normalize_key converts dashes to underscores + When the safety-net normalizer processes key "server-port" + Then the safety-net normalized result should be "server_port" + + Scenario: safety-net _normalize_key lowercases and strips whitespace + When the safety-net normalizer processes key " LOG_LEVEL " + Then the safety-net normalized result should be "log_level" + + Scenario: safety-net _normalize_key handles mixed separators + When the safety-net normalizer processes key "My.Log-Level" + Then the safety-net normalized result should be "my_log_level" + + # ===================================================================== + # _is_secret_key (L116-118) + # ===================================================================== + + Scenario: safety-net _is_secret_key detects api_key pattern + When the safety-net secret checker inspects key "openai_api_key" + Then the safety-net secret check result should be true + + Scenario: safety-net _is_secret_key detects token pattern + When the safety-net secret checker inspects key "auth_token" + Then the safety-net secret check result should be true + + Scenario: safety-net _is_secret_key detects password pattern + When the safety-net secret checker inspects key "db_password" + Then the safety-net secret check result should be true + + Scenario: safety-net _is_secret_key detects secret pattern + When the safety-net secret checker inspects key "client_secret" + Then the safety-net secret check result should be true + + Scenario: safety-net _is_secret_key rejects non-secret key + When the safety-net secret checker inspects key "log_level" + Then the safety-net secret check result should be false + + Scenario: safety-net _is_secret_key is case insensitive + When the safety-net secret checker inspects key "API_KEY" + Then the safety-net secret check result should be true + + # ===================================================================== + # _mask_value (L121-123) + # ===================================================================== + + Scenario: safety-net _mask_value always returns four asterisks + When the safety-net masker masks value "super-secret-123" + Then the safety-net masked output should be "****" + + Scenario: safety-net _mask_value masks empty string + When the safety-net masker masks an empty string value + Then the safety-net masked output should be "****" + + # ===================================================================== + # _env_var_for_key (L158-160) + # ===================================================================== + + Scenario: safety-net _env_var_for_key builds correct env var name + When the safety-net env var builder processes key "log_level" + Then the safety-net env var name should be "CLEVERAGENTS_LOG_LEVEL" + + Scenario: safety-net _env_var_for_key uppercases the key + When the safety-net env var builder processes key "server_port" + Then the safety-net env var name should be "CLEVERAGENTS_SERVER_PORT" + + # ===================================================================== + # _read_config_file (L126-131) + # ===================================================================== + + Scenario: safety-net _read_config_file returns empty dict when file absent + Given a safety-net isolated temp config directory + When the safety-net reader reads the config file + Then the safety-net read result should be an empty dict + + Scenario: safety-net _read_config_file reads existing toml data + Given a safety-net isolated temp config directory + And a safety-net toml config file containing key "debug_enabled" with value "true" + When the safety-net reader reads the config file + Then the safety-net read result should contain key "debug_enabled" + + # ===================================================================== + # _write_config_file (L134-155) - create new file + # ===================================================================== + + Scenario: safety-net _write_config_file creates file when absent + Given a safety-net isolated temp config directory + When the safety-net writer writes key "log_level" with value "INFO" + Then the safety-net config file should exist and contain key "log_level" + + # ===================================================================== + # _settings_fields (L62-70) + # ===================================================================== + + Scenario: safety-net _settings_fields returns populated dictionary + When the safety-net fields loader retrieves all settings fields + Then the safety-net fields result should be a non-empty dict + + # ===================================================================== + # _validate_key (L99-113) - unknown key path + # ===================================================================== + + Scenario: safety-net _validate_key raises on unknown key with helpful message + When the safety-net validator checks unknown key "zzz_nonexistent_xxy" + Then the safety-net validator should raise BadParameter with "Unknown configuration key" + + Scenario: safety-net _validate_key accepts valid key and returns normalized form + When the safety-net validator checks valid key "log.level" + Then the safety-net validator should return "log_level" + + # ===================================================================== + # _resolve_source (L163-174) - default path + # ===================================================================== + + Scenario: safety-net _resolve_source returns default when no env or file + Given a safety-net isolated temp config directory + When the safety-net source resolver checks key "log_level" + Then the safety-net resolved source should be "default" + + Scenario: safety-net _resolve_source returns config_file when key in file + Given a safety-net isolated temp config directory + And a safety-net toml config file containing key "log_level" with value "WARNING" + When the safety-net source resolver checks key "log_level" + Then the safety-net resolved source should be "config_file" + + # ===================================================================== + # _resolution_chain (L177-209) + # ===================================================================== + + Scenario: safety-net _resolution_chain returns four-entry list + Given a safety-net isolated temp config directory + When the safety-net chain builder builds chain for key "log_level" + Then the safety-net chain should have exactly 4 entries + And the safety-net chain sources should be "cli_flag, env_var, config_file, default" + + # ===================================================================== + # config_set - type coercion paths (L241-248) + # ===================================================================== + + Scenario: safety-net config set coerces boolean true + Given a safety-net isolated temp config directory + When the safety-net CLI sets key "debug_enabled" to value "true" with format "json" + Then the safety-net set output should be valid JSON + And the safety-net set JSON field "value" should be boolean true + + Scenario: safety-net config set coerces boolean false + Given a safety-net isolated temp config directory + When the safety-net CLI sets key "debug_enabled" to value "false" with format "json" + Then the safety-net set output should be valid JSON + And the safety-net set JSON field "value" should be boolean false + + Scenario: safety-net config set coerces integer value + Given a safety-net isolated temp config directory + When the safety-net CLI sets key "server_port" to value "8080" with format "json" + Then the safety-net set output should be valid JSON + And the safety-net set JSON field "value" should be integer 8080 + + Scenario: safety-net config set coerces float value + Given a safety-net isolated temp config directory + When the safety-net CLI sets key "server_port" to value "3.14" with format "json" + Then the safety-net set output should be valid JSON + And the safety-net set JSON field "value" should be float 3.14 + + Scenario: safety-net config set keeps string when not numeric or bool + Given a safety-net isolated temp config directory + When the safety-net CLI sets key "log_level" to value "DEBUG" with format "json" + Then the safety-net set output should be valid JSON + And the safety-net set JSON field "value" should be string "DEBUG" + + Scenario: safety-net config set shows previous value in result + Given a safety-net isolated temp config directory + And the safety-net CLI has previously set "log_level" to "INFO" + When the safety-net CLI sets key "log_level" to value "DEBUG" with format "json" + Then the safety-net set output should be valid JSON + And the safety-net set JSON field "previous_value" should be string "INFO" + + Scenario: safety-net config set rich format shows panel output + Given a safety-net isolated temp config directory + When the safety-net CLI sets key "log_level" to value "DEBUG" with format "rich" + Then the safety-net set rich output should contain "Configuration Updated" + And the safety-net set rich output should contain "log_level" + + # ===================================================================== + # config_get - rich format (L319-335) + # ===================================================================== + + Scenario: safety-net config get rich format displays panel and chain + Given a safety-net isolated temp config directory + When the safety-net CLI gets key "log_level" with format "rich" + Then the safety-net get rich output should contain "Configuration Value" + And the safety-net get rich output should contain "Resolution chain" + + Scenario: safety-net config get yaml format produces valid YAML + Given a safety-net isolated temp config directory + When the safety-net CLI gets key "log_level" with format "yaml" + Then the safety-net get output should be valid YAML + And the safety-net get YAML should contain key "key" + + Scenario: safety-net config get json format includes type field + Given a safety-net isolated temp config directory + When the safety-net CLI gets key "log_level" with format "json" + Then the safety-net get output should be valid JSON with type field + + # ===================================================================== + # config_list - YAML and plain formats (L426-431) + # ===================================================================== + + Scenario: safety-net config list yaml format produces valid YAML + Given a safety-net isolated temp config directory + When the safety-net CLI lists config with format "yaml" + Then the safety-net list output should be valid YAML list + + Scenario: safety-net config list plain format produces text output + Given a safety-net isolated temp config directory + When the safety-net CLI lists config with format "plain" + Then the safety-net list plain output should contain key-value lines + + Scenario: safety-net config list table format produces table output + Given a safety-net isolated temp config directory + When the safety-net CLI lists config with format "table" + Then the safety-net list table output should contain column headers + + # ===================================================================== + # config_list - combined key + value filters (L391-399) + # ===================================================================== + + Scenario: safety-net config list with both key and value filter + Given a safety-net isolated temp config directory + When the safety-net CLI lists config with key pattern "log" and value filter "INFO" + Then the safety-net combined filter result should succeed + + # ===================================================================== + # config_list - Path serialisation in non-rich (L428-429) + # ===================================================================== + + Scenario: safety-net config list json format serialises Path values to strings + Given a safety-net isolated temp config directory + And safety-net settings fields include a Path-typed value + When the safety-net CLI lists config with format "json" + Then the safety-net list JSON output should not contain "PosixPath" or "WindowsPath" + + # ===================================================================== + # config_list - secret masking in list (L408-411) + # ===================================================================== + + Scenario: safety-net config list masks secret values by default + Given a safety-net isolated temp config directory + And safety-net settings fields include a secret key with a non-pattern value + When the safety-net CLI lists all config in json format + Then the safety-net list JSON should contain masked value "****" for the secret key + + Scenario: safety-net config list reveals secrets with show-secrets flag + Given a safety-net isolated temp config directory + And safety-net settings fields include a secret key with a non-pattern value + When the safety-net CLI lists all config in json format with show-secrets + Then the safety-net list JSON should contain the actual secret value + + # ===================================================================== + # config_list - modified flag (L405-407) + # ===================================================================== + + Scenario: safety-net config list marks modified values + Given a safety-net isolated temp config directory + And safety-net settings fields have a value different from default + When the safety-net CLI lists all config in json format + Then the safety-net list JSON should include a modified flag set to true + + # ===================================================================== + # config_get - resolution chain active marker (L333) + # ===================================================================== + + Scenario: safety-net config get rich format marks active source in chain + Given a safety-net isolated temp config directory + And a safety-net patched console for capturing rich output + When the safety-net CLI gets key "log_level" with format "rich" + Then the safety-net captured console output should contain "active" diff --git a/features/decision_service_coverage.feature b/features/decision_service_coverage.feature new file mode 100644 index 000000000..e56eb7c39 --- /dev/null +++ b/features/decision_service_coverage.feature @@ -0,0 +1,156 @@ +Feature: DecisionService application-layer coverage + As a developer + I want thorough unit tests for every DecisionService method + So that decision_service.py achieves full line and branch coverage + + # All steps use the "dsvc-" prefix to avoid collisions with other step files. + + # ------------------------------------------------------------------ + # Constructor + # ------------------------------------------------------------------ + + Scenario: dsvc- constructor stores settings and unit_of_work + Given dsvc- a mock settings object and a mock UnitOfWork + When dsvc- I construct a DecisionService with those dependencies + Then dsvc- the service should store the settings + And dsvc- the service should store the unit_of_work + And dsvc- the service should have a bound structlog logger + + # ------------------------------------------------------------------ + # record_decision + # ------------------------------------------------------------------ + + Scenario: dsvc- record_decision persists and returns the decision + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- a sample root Decision object + When dsvc- I call record_decision with the sample decision + Then dsvc- the UoW transaction should have been entered + And dsvc- ctx.decisions.create should have been called with the decision + And dsvc- the returned decision should be the same object + + Scenario: dsvc- record_decision logs info and debug messages + Given dsvc- a DecisionService with a mocked UnitOfWork and captured logger + And dsvc- a sample root Decision object + When dsvc- I call record_decision with the sample decision + Then dsvc- the logger should have recorded an info call with "recording_decision" + And dsvc- the logger should have recorded a debug call with "decision_recorded" + + # ------------------------------------------------------------------ + # get_decision + # ------------------------------------------------------------------ + + Scenario: dsvc- get_decision retrieves a decision by ID + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo get method returns a Decision + When dsvc- I call get_decision with a known ID + Then dsvc- ctx.decisions.get should have been called with the ID + And dsvc- the returned value should be the expected Decision + + Scenario: dsvc- get_decision returns None when not found + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo get method returns None + When dsvc- I call get_decision with an unknown ID + Then dsvc- the returned value should be None + + Scenario: dsvc- get_decision logs a debug message + Given dsvc- a DecisionService with a mocked UnitOfWork and captured logger + And dsvc- the mock repo get method returns a Decision + When dsvc- I call get_decision with a known ID + Then dsvc- the logger should have recorded a debug call with "getting_decision" + + # ------------------------------------------------------------------ + # get_decisions_for_plan + # ------------------------------------------------------------------ + + Scenario: dsvc- get_decisions_for_plan returns list of decisions + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo get_by_plan method returns 3 decisions + When dsvc- I call get_decisions_for_plan with a plan ID + Then dsvc- ctx.decisions.get_by_plan should have been called with the plan ID + And dsvc- the returned list should have 3 decisions + + Scenario: dsvc- get_decisions_for_plan returns empty list when none exist + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo get_by_plan method returns 0 decisions + When dsvc- I call get_decisions_for_plan with a plan ID + Then dsvc- the returned list should have 0 decisions + + Scenario: dsvc- get_decisions_for_plan logs a debug message + Given dsvc- a DecisionService with a mocked UnitOfWork and captured logger + And dsvc- the mock repo get_by_plan method returns 3 decisions + When dsvc- I call get_decisions_for_plan with a plan ID + Then dsvc- the logger should have recorded a debug call with "getting_decisions_for_plan" + + # ------------------------------------------------------------------ + # get_decision_tree + # ------------------------------------------------------------------ + + Scenario: dsvc- get_decision_tree returns BFS-ordered list + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo get_tree method returns 4 decisions + When dsvc- I call get_decision_tree with a root ID + Then dsvc- ctx.decisions.get_tree should have been called with the root ID + And dsvc- the returned tree list should have 4 decisions + + Scenario: dsvc- get_decision_tree logs a debug message + Given dsvc- a DecisionService with a mocked UnitOfWork and captured logger + And dsvc- the mock repo get_tree method returns 4 decisions + When dsvc- I call get_decision_tree with a root ID + Then dsvc- the logger should have recorded a debug call with "getting_decision_tree" + + # ------------------------------------------------------------------ + # get_path_to_root + # ------------------------------------------------------------------ + + Scenario: dsvc- get_path_to_root returns leaf-to-root path + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo get_path_to_root method returns 3 decisions + When dsvc- I call get_path_to_root with a leaf ID + Then dsvc- ctx.decisions.get_path_to_root should have been called with the leaf ID + And dsvc- the returned path list should have 3 decisions + + Scenario: dsvc- get_path_to_root logs a debug message + Given dsvc- a DecisionService with a mocked UnitOfWork and captured logger + And dsvc- the mock repo get_path_to_root method returns 3 decisions + When dsvc- I call get_path_to_root with a leaf ID + Then dsvc- the logger should have recorded a debug call with "getting_path_to_root" + + # ------------------------------------------------------------------ + # mark_superseded + # ------------------------------------------------------------------ + + Scenario: dsvc- mark_superseded updates and returns the decision + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo update_superseded_by method returns a superseded Decision + When dsvc- I call mark_superseded with old and new IDs + Then dsvc- ctx.decisions.update_superseded_by should have been called with both IDs + And dsvc- the returned decision should be the superseded one + + Scenario: dsvc- mark_superseded logs info with both IDs + Given dsvc- a DecisionService with a mocked UnitOfWork and captured logger + And dsvc- the mock repo update_superseded_by method returns a superseded Decision + When dsvc- I call mark_superseded with old and new IDs + Then dsvc- the logger should have recorded an info call with "marking_superseded" + + # ------------------------------------------------------------------ + # list_by_type + # ------------------------------------------------------------------ + + Scenario: dsvc- list_by_type returns filtered decisions + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo list_by_type method returns 2 decisions + When dsvc- I call list_by_type with plan ID and type "strategy_choice" + Then dsvc- ctx.decisions.list_by_type should have been called with plan ID and type + And dsvc- the returned type list should have 2 decisions + + Scenario: dsvc- list_by_type returns empty when no matches + Given dsvc- a DecisionService with a mocked UnitOfWork + And dsvc- the mock repo list_by_type method returns 0 decisions + When dsvc- I call list_by_type with plan ID and type "error_recovery" + Then dsvc- the returned type list should have 0 decisions + + Scenario: dsvc- list_by_type logs a debug message + Given dsvc- a DecisionService with a mocked UnitOfWork and captured logger + And dsvc- the mock repo list_by_type method returns 2 decisions + When dsvc- I call list_by_type with plan ID and type "strategy_choice" + Then dsvc- the logger should have recorded a debug call with "listing_by_type" diff --git a/features/plan_apply_service_branch_coverage.feature b/features/plan_apply_service_branch_coverage.feature new file mode 100644 index 000000000..c427dc22d --- /dev/null +++ b/features/plan_apply_service_branch_coverage.feature @@ -0,0 +1,23 @@ +Feature: PlanApplyService branch coverage for handle_merge_failure logger path + As a developer + I want to cover the missed branch at line 426→432 in plan_apply_service.py + So that branch-rate reaches 1.0 + + # The missed branch is the exception-propagation path from the + # self._logger.error() call inside handle_merge_failure (line 426). + # When the logger raises, execution skips the ``return plan`` on line 431 + # and exits the function via the exception (line 432). + + Scenario: handle_merge_failure propagates exception when logger.error raises + Given pas_branch a service whose logger.error will raise RuntimeError + When pas_branch I call handle_merge_failure and capture any exception + Then pas_branch a RuntimeError should have been raised + And pas_branch the plan error_details should still contain merge_conflict + And pas_branch lifecycle _commit_plan should have been invoked before the error + And pas_branch lifecycle fail_apply should have been invoked before the error + + Scenario: handle_merge_failure returns plan when logger.error succeeds + Given pas_branch a service whose logger.error will succeed normally + When pas_branch I call handle_merge_failure normally + Then pas_branch the returned plan should not be None + And pas_branch the service logger.error should have been called with merge failure details diff --git a/features/plan_cli_uncovered_region_coverage.feature b/features/plan_cli_uncovered_region_coverage.feature new file mode 100644 index 000000000..173a1a93a --- /dev/null +++ b/features/plan_cli_uncovered_region_coverage.feature @@ -0,0 +1,195 @@ +Feature: Plan CLI uncovered region coverage (lines 1950-2273) + As a developer + I want targeted tests for the uncovered lines and branches in plan.py + So that coverage gaps at lines 1950-1954, 1956-1957, 2132, 2135-2143, + 2146-2184, 2202-2233, and 2231-2273 are closed + + # =================================================================== + # lifecycle_list_plans — empty result after filtering (lines 1950-1954) + # =================================================================== + + Scenario: uncov-rgn lifecycle-list shows "No plans found" when action filter excludes all + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn lifecycle service returns plans that do not match an action filter + When I uncov-rgn invoke lifecycle-list with action filter "nonexistent/action" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "No plans found" + + # =================================================================== + # lifecycle_list_plans — non-rich format output (lines 1956-1957) + # =================================================================== + + Scenario: uncov-rgn lifecycle-list outputs JSON when format is json + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn lifecycle service returns a single plan for listing + When I uncov-rgn invoke lifecycle-list with format "json" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "plan_id" + + Scenario: uncov-rgn lifecycle-list outputs YAML when format is yaml + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn lifecycle service returns a single plan for listing + When I uncov-rgn invoke lifecycle-list with format "yaml" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "plan_id" + + # =================================================================== + # revert_plan — CleverAgentsError handler (line 2132) + # =================================================================== + + Scenario: uncov-rgn revert_plan raises CleverAgentsError + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn lifecycle service revert raises CleverAgentsError + When I uncov-rgn invoke revert for plan "01ARZ3NDEKTSV4RRFFQ69G5FAA" + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Error" + + # =================================================================== + # _get_apply_service — function body (lines 2136-2143) + # =================================================================== + + Scenario: uncov-rgn _get_apply_service creates a PlanApplyService instance + Given an uncov-rgn mocked lifecycle service for apply service creation + When I uncov-rgn call _get_apply_service directly + Then the uncov-rgn apply service should be returned successfully + + # =================================================================== + # plan_diff — correction parameter branch (lines 2149-2184) + # =================================================================== + + Scenario: uncov-rgn plan diff with correction flag shows stub panel + Given an uncov-rgn CLI runner and mocked lifecycle service + When I uncov-rgn invoke diff for plan "PLAN-001" with correction "CORR-001" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "Correction Attempt" + And the uncov-rgn output should contain "CORR-001" + + Scenario: uncov-rgn plan diff without correction calls apply service + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn apply service diff returns formatted output "--- a/main.py" + When I uncov-rgn invoke diff for plan "PLAN-002" without correction + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "main.py" + + Scenario: uncov-rgn plan diff raises PlanError + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn apply service diff raises PlanError "missing changeset" + When I uncov-rgn invoke diff for plan "PLAN-003" without correction + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Diff Error" + + Scenario: uncov-rgn plan diff raises CleverAgentsError + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn apply service diff raises CleverAgentsError "service down" + When I uncov-rgn invoke diff for plan "PLAN-004" without correction + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Error" + + # =================================================================== + # plan_artifacts — full command coverage (lines 2202-2233) + # =================================================================== + + Scenario: uncov-rgn plan artifacts success in rich format + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn apply service artifacts returns "changeset-id: cs-100" + When I uncov-rgn invoke artifacts for plan "PLAN-005" in rich format + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "cs-100" + + Scenario: uncov-rgn plan artifacts success in json format + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn apply service artifacts returns '{"changeset": "cs-200"}' + When I uncov-rgn invoke artifacts for plan "PLAN-006" with format "json" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "cs-200" + + Scenario: uncov-rgn plan artifacts raises PlanError + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn apply service artifacts raises PlanError "no changeset" + When I uncov-rgn invoke artifacts for plan "PLAN-007" in rich format + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Artifacts Error" + + Scenario: uncov-rgn plan artifacts raises CleverAgentsError + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn apply service artifacts raises CleverAgentsError "timeout" + When I uncov-rgn invoke artifacts for plan "PLAN-008" in rich format + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Error" + + # =================================================================== + # correct_decision — full command coverage (lines 2231-2273+) + # =================================================================== + + Scenario: uncov-rgn correct revert with --yes succeeds in rich format + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn correction service returns a revert result with reverted decisions + When I uncov-rgn invoke correct in revert mode with --yes and guidance "Use Flask instead" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "Correction applied" + And the uncov-rgn output should contain "Reverted" + + Scenario: uncov-rgn correct append with --yes succeeds in rich format + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn correction service returns an append result with new decisions + When I uncov-rgn invoke correct in append mode with --yes and guidance "Add caching layer" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "Correction applied" + And the uncov-rgn output should contain "New decisions" + + Scenario: uncov-rgn correct revert with --yes succeeds in json format + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn correction service returns a revert result with reverted decisions + When I uncov-rgn invoke correct in revert mode with --yes, format "json", and guidance "Refactor" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "correction_id" + And the uncov-rgn output should contain "reverted_decisions" + + Scenario: uncov-rgn correct dry-run in rich format shows impact panel + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn correction service returns an impact analysis + When I uncov-rgn invoke correct in dry-run mode with rich format and guidance "Preview changes" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "Correction Impact" + And the uncov-rgn output should contain "Dry Run" + + Scenario: uncov-rgn correct dry-run in json format shows impact data + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn correction service returns an impact analysis + When I uncov-rgn invoke correct in dry-run mode with format "json" and guidance "Check impact" + Then the uncov-rgn command should exit normally + And the uncov-rgn output should contain "correction_id" + And the uncov-rgn output should contain "risk_level" + + Scenario: uncov-rgn correct raises ResourceNotFoundError + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn correction service request raises ResourceNotFoundError + When I uncov-rgn invoke correct in revert mode with --yes and guidance "Fix something" + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Not found" + + Scenario: uncov-rgn correct raises ValidationError + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn correction service request raises ValidationError + When I uncov-rgn invoke correct in revert mode with --yes and guidance "Validate me" + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Validation Error" + + Scenario: uncov-rgn correct raises CleverAgentsError + Given an uncov-rgn CLI runner and mocked lifecycle service + And the uncov-rgn correction service request raises CleverAgentsError + When I uncov-rgn invoke correct in revert mode with --yes and guidance "General error" + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Error" + + Scenario: uncov-rgn correct with invalid mode aborts + Given an uncov-rgn CLI runner and mocked lifecycle service + When I uncov-rgn invoke correct with invalid mode "rollback" and guidance "test" + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "Invalid mode" + + Scenario: uncov-rgn correct with empty guidance aborts + Given an uncov-rgn CLI runner and mocked lifecycle service + When I uncov-rgn invoke correct with valid mode "revert" and empty guidance + Then the uncov-rgn command should abort + And the uncov-rgn output should contain "guidance" diff --git a/features/plan_executor_edge_cases_coverage.feature b/features/plan_executor_edge_cases_coverage.feature new file mode 100644 index 000000000..3bff8843b --- /dev/null +++ b/features/plan_executor_edge_cases_coverage.feature @@ -0,0 +1,144 @@ +Feature: PlanExecutor edge-case coverage for checkpoint, rollback, and sandbox proxy paths + As a developer + I want tests that exercise the remaining uncovered lines and branches in plan_executor.py + So that line 378, lines 410-411, and checkpoint-related branches reach full coverage + + # Targets: + # - Line 378 / branch 377→378: _try_rollback_to_last_checkpoint with non-empty checkpoints + # - Lines 410-411: _resolve_sandbox_for_checkpoint falling through to _SandboxRootProxy + # - Lines 161-163 / branches 160→161, 162→163/164: _parse_steps non-empty path via run_strategize + # - Lines 417-419: run_strategize function definition exercised + + # ------------------------------------------------------------------ + # _resolve_sandbox_for_checkpoint → _SandboxRootProxy path + # ------------------------------------------------------------------ + + Scenario: Resolve sandbox returns SandboxRootProxy when only sandbox_root is set + Given an edge3 PlanExecutor with checkpoint manager and sandbox root but no execution context + When I edge3 resolve sandbox for checkpoint with plan id "EDGE3PLAN01" + Then the edge3 resolved sandbox should not be None + And the edge3 resolved sandbox should have a synthetic sandbox id + And the edge3 resolved sandbox context should have the sandbox path + + Scenario: Resolve sandbox returns SandboxRootProxy when execution context has no sandbox manager + Given an edge3 PlanExecutor with checkpoint manager and sandbox root and context without sandbox manager + When I edge3 resolve sandbox for checkpoint with plan id "EDGE3PLAN02" + Then the edge3 resolved sandbox should not be None + And the edge3 resolved sandbox should have a synthetic sandbox id + + Scenario: Resolve sandbox returns SandboxRootProxy when sandbox manager returns empty list + Given an edge3 PlanExecutor with checkpoint manager and sandbox root and context with empty sandboxes + When I edge3 resolve sandbox for checkpoint with plan id "EDGE3PLAN03" + Then the edge3 resolved sandbox should not be None + And the edge3 resolved sandbox should have a synthetic sandbox id + + Scenario: Resolve sandbox returns None when no sandbox root and no execution context + Given an edge3 PlanExecutor with checkpoint manager but no sandbox root and no execution context + When I edge3 resolve sandbox for checkpoint with plan id "EDGE3PLAN04" + Then the edge3 resolved sandbox should be None + + # ------------------------------------------------------------------ + # _try_rollback_to_last_checkpoint with non-empty checkpoints + # ------------------------------------------------------------------ + + Scenario: Rollback succeeds when checkpoint manager has checkpoints for sandbox root + Given an edge3 PlanExecutor with checkpoint manager and sandbox root but no execution context + And the edge3 checkpoint manager has existing checkpoints that rollback successfully + When I edge3 try rollback to last checkpoint for plan "EDGE3PLANRB" + Then the edge3 rollback result should be True + + Scenario: Rollback returns False when rollback_to raises an exception + Given an edge3 PlanExecutor with checkpoint manager and sandbox root but no execution context + And the edge3 checkpoint manager has checkpoints but rollback raises an exception + When I edge3 try rollback to last checkpoint for plan "EDGE3PLANRF" + Then the edge3 rollback result should be False + + Scenario: Rollback returns False when checkpoint manager lists no checkpoints + Given an edge3 PlanExecutor with checkpoint manager and sandbox root but no execution context + And the edge3 checkpoint manager returns no checkpoints + When I edge3 try rollback to last checkpoint for plan "EDGE3PLANRE" + Then the edge3 rollback result should be False + + # ------------------------------------------------------------------ + # _try_create_checkpoint via _SandboxRootProxy + # ------------------------------------------------------------------ + + Scenario: Create checkpoint succeeds through SandboxRootProxy path + Given an edge3 PlanExecutor with checkpoint manager and sandbox root but no execution context + And the edge3 checkpoint manager accepts checkpoint creation + When I edge3 try create checkpoint for plan "EDGE3PLANCP" with phase "pre_execute" + Then the edge3 checkpoint result should not be None + And the edge3 checkpoint manager should have been called with sandbox path metadata + + Scenario: Create checkpoint returns None when creation raises exception through proxy + Given an edge3 PlanExecutor with checkpoint manager and sandbox root but no execution context + And the edge3 checkpoint manager raises on create_checkpoint + When I edge3 try create checkpoint for plan "EDGE3PLANCF" with phase "pre_execute" + Then the edge3 checkpoint result should be None + + # ------------------------------------------------------------------ + # Stub execute with checkpoint rollback on failure + # ------------------------------------------------------------------ + + Scenario: Stub execute failure triggers rollback with sandbox root proxy and existing checkpoints + Given an edge3 mock lifecycle service for execute + And an edge3 plan in Execute-Queued state with decision root + And an edge3 PlanExecutor with checkpoint manager sandbox root and failing execute actor + And the edge3 checkpoint manager has existing checkpoints that rollback successfully + When I edge3 call run execute expecting failure + Then an edge3 exception should have been raised + And the edge3 checkpoint manager rollback_to should have been called + And the edge3 lifecycle should have called fail_execute for edge3 + + # ------------------------------------------------------------------ + # Runtime execute with checkpoint rollback on failure + # ------------------------------------------------------------------ + + Scenario: Runtime execute failure triggers rollback with sandbox root proxy + Given an edge3 mock lifecycle service for execute + And an edge3 mock execution context for runtime + And an edge3 plan in Execute-Queued state with decision root + And an edge3 PlanExecutor with runtime context checkpoint manager sandbox root and failing runtime actor + And the edge3 checkpoint manager has existing checkpoints that rollback successfully + When I edge3 call run execute expecting failure + Then an edge3 exception should have been raised + And the edge3 checkpoint manager rollback_to should have been called + + # ------------------------------------------------------------------ + # _parse_steps non-empty path exercised via run_strategize + # ------------------------------------------------------------------ + + Scenario: Run strategize exercises _parse_steps with multi-line definition + Given an edge3 mock lifecycle service for strategize + And an edge3 plan in Strategize phase with multi-line definition "- Build feature\n- Add tests\n- Deploy" + And an edge3 PlanExecutor for strategize without execution context + When I edge3 call run strategize successfully + Then the edge3 strategize result should have 3 decisions + And the edge3 lifecycle should have called complete_strategize for edge3 + + Scenario: Run strategize exercises _parse_steps with numbered definition + Given an edge3 mock lifecycle service for strategize + And an edge3 plan in Strategize phase with multi-line definition "1. First task\n2. Second task" + And an edge3 PlanExecutor for strategize without execution context + When I edge3 call run strategize successfully + Then the edge3 strategize result should have 2 decisions + + Scenario: Run strategize with empty definition falls back to default step + Given an edge3 mock lifecycle service for strategize + And an edge3 plan in Strategize phase with empty definition + And an edge3 PlanExecutor for strategize without execution context + When I edge3 call run strategize successfully + Then the edge3 strategize result should have 1 decisions + + # ------------------------------------------------------------------ + # Stub execute with checkpoint creation and sandbox root proxy + # ------------------------------------------------------------------ + + Scenario: Successful stub execute creates pre and post checkpoints via sandbox root proxy + Given an edge3 mock lifecycle service for execute + And an edge3 plan in Execute-Queued state with decision root + And an edge3 PlanExecutor with checkpoint manager and sandbox root for stub execute + And the edge3 checkpoint manager accepts checkpoint creation + When I edge3 call run execute successfully + Then the edge3 execute result should be an ExecuteResult + And the edge3 checkpoint manager create_checkpoint should have been called at least twice diff --git a/features/plan_lifecycle_coverage.feature b/features/plan_lifecycle_coverage.feature index 8fd6f3414..b457e9624 100644 --- a/features/plan_lifecycle_coverage.feature +++ b/features/plan_lifecycle_coverage.feature @@ -31,7 +31,7 @@ Feature: Plan lifecycle repository and service coverage Scenario: Delete returns false for non-existent plan When I delete a non-existent plan "01NONEXISTENT000000000000" from the repository - Then the delete result should be false + Then the plan delete result should be false Scenario: List plans with phase filter Given a coverage plan exists via the service diff --git a/features/repositories_remaining_branches_coverage.feature b/features/repositories_remaining_branches_coverage.feature new file mode 100644 index 000000000..94e6e24fa --- /dev/null +++ b/features/repositories_remaining_branches_coverage.feature @@ -0,0 +1,119 @@ +@unit @repository @remaining_branches +Feature: Repository remaining branches and lines coverage + Exercise the last uncovered lines and branches in repositories.py: + - ToolRepository.add DuplicateToolError re-raise (lines 3541-3542) + - ToolRepository.add DatabaseError wrap (line 3542) + - ToolRepository.get_by_name success path returning domain (line 3558) + - ResourceRepository.link_child when parent type row is absent (branch 2298→2310) + - ResourceRepository.resolve_namespaced_name ULID fallback (branch 2711→2714) + - ResourceRepository.get_children returning populated list (branch 2426→2420) + - ResourceRepository.get_parents returning populated list (branch 2459→2453) + - ResourceRepository._get_ancestors via link_child cycle detection (branch 2635→2633) + - ResourceRepository._build_cycle_path via link_child (branch 2661→2659) + - NamespacedProjectRepository.update success path (branch 2928→2931) + - AutomationProfileRepository.upsert inserting brand new profile (branch 4193→4200) + - ValidationAttachmentRepository.attach with project_name scope (branch 3633→3634) + - LifecyclePlanRepository.update with multiple project links (branch 1395→1394) + + # ── ToolRepository.add: DuplicateToolError re-raise ─────────── + + Scenario: remaining cov tool add re-raises DuplicateToolError for duplicate tool + Given remaining cov an in-memory database with tool tables + And remaining cov a tool "local/dup-tool" has been added + When remaining cov the same tool "local/dup-tool" is added again + Then remaining cov a DuplicateToolError should be raised for the duplicate + + # ── ToolRepository.add: DatabaseError wrapping ──────────────── + + Scenario: remaining cov tool add wraps non-duplicate DatabaseError + Given remaining cov a ToolRepository with a session that raises DatabaseError on create + When remaining cov a tool is added and a DatabaseError is expected + Then remaining cov a DatabaseError mentioning "Failed to add tool" should be raised + + # ── ToolRepository.get_by_name: success returns domain ──────── + + Scenario: remaining cov tool get_by_name returns domain object for existing tool + Given remaining cov an in-memory database with tool tables + And remaining cov a tool "local/findable-tool" has been added + When remaining cov get_by_name is invoked for "local/findable-tool" + Then remaining cov get_by_name should return a non-None result with name "local/findable-tool" + + # ── ResourceRepository.link_child: parent type row absent ───── + + Scenario: remaining cov link_child succeeds when parent type row is missing from DB + Given remaining cov an in-memory resource database + And remaining cov two resources with a type that has no ResourceTypeModel row + When remaining cov link_child is called for those resources + Then remaining cov the link should be created successfully + + # ── ResourceRepository.resolve_namespaced_name: ULID fallback ─ + + Scenario: remaining cov resolve_namespaced_name falls back to ULID lookup + Given remaining cov an in-memory resource database + And remaining cov a resource exists with a known ULID but no namespaced name + When remaining cov resolve_namespaced_name is called with the ULID + Then remaining cov the resource should be resolved successfully + + Scenario: remaining cov resolve_namespaced_name returns None for unknown ULID + Given remaining cov an in-memory resource database + When remaining cov resolve_namespaced_name is called with unknown ULID "01JQXG0000000000000000ZZZZ" + Then remaining cov resolve_namespaced_name should return None + + # ── ResourceRepository.get_children: populated list ─────────── + + Scenario: remaining cov get_children returns linked child resources + Given remaining cov an in-memory resource database + And remaining cov a parent resource linked to two child resources + When remaining cov get_children is called on the parent resource + Then remaining cov 2 child resources should be returned + + # ── ResourceRepository.get_parents: populated list ──────────── + + Scenario: remaining cov get_parents returns linked parent resources + Given remaining cov an in-memory resource database + And remaining cov a child resource linked from two parent resources + When remaining cov get_parents is called on the child resource + Then remaining cov 2 parent resources should be returned + + # ── ResourceRepository._get_ancestors + _build_cycle_path ───── + + Scenario: remaining cov link_child detects cycle and builds cycle path + Given remaining cov an in-memory resource database + And remaining cov resources "cyc-A" and "cyc-B" linked as A->B + When remaining cov link_child is called to create B->A forming a cycle + Then remaining cov a CycleDetectedError should be raised with a path + + # ── NamespacedProjectRepository.update: success path ────────── + + Scenario: remaining cov project update succeeds for existing project + Given remaining cov an in-memory project database + And remaining cov a project "local/updatable-proj" exists + When remaining cov the project "local/updatable-proj" is updated with new description + Then remaining cov the project update should succeed + + # ── AutomationProfileRepository.upsert: new insert path ────── + + Scenario: remaining cov profile upsert inserts a new profile when none exists + Given remaining cov an in-memory automation profile database + When remaining cov a brand new profile "local/fresh-profile" is upserted + Then remaining cov the profile "local/fresh-profile" should be retrievable + + # ── ValidationAttachmentRepository.attach: project_name scope ─ + + Scenario: remaining cov validation attach with project_name scope only + Given remaining cov an in-memory database with validation tables + When remaining cov a validation is attached using project scope "my-project" + Then remaining cov the returned attachment has project_name "my-project" + + Scenario: remaining cov validation attach with both project and plan scope + Given remaining cov an in-memory database with validation tables + When remaining cov a validation is attached using project-plan scope "proj-x" "PLAN-42" + Then remaining cov the returned attachment has project "proj-x" and plan "PLAN-42" + + # ── LifecyclePlanRepository.update: multiple project links ──── + + Scenario: remaining cov plan update iterates over multiple project links + Given remaining cov an in-memory lifecycle plan database + And remaining cov a plan with action "local/multi-link-action" exists + When remaining cov the plan is updated with 3 project links + Then remaining cov the plan should have 3 project links after retrieval diff --git a/features/steps/bridge_remaining_coverage_steps.py b/features/steps/bridge_remaining_coverage_steps.py new file mode 100644 index 000000000..a5ec1768d --- /dev/null +++ b/features/steps/bridge_remaining_coverage_steps.py @@ -0,0 +1,474 @@ +"""Steps for remaining uncovered lines/branches in bridge.py. + +Targets: +- Line 35: ``cancellation_reasons`` WeakKeyDictionary initialisation +- Line 68: ``task.cancel()`` inside ``cleanup_tasks_async`` +- Lines 201-204: string-content branch in the inner ``execute_graph`` coroutine +- Branches 39->38 (__del__ normal exit), 67->68 (not-done cancel in async cleanup), + 182->184 (valid graph in create_graph_stream), 260->258 (valid graph in checkpointer) +""" + +from __future__ import annotations + +import asyncio +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.langgraph.bridge import RxPyLangGraphBridge +from cleveragents.langgraph.state import GraphState +from cleveragents.reactive.stream_router import ( + ReactiveStreamRouter, + StreamMessage, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _build_bridge() -> tuple[RxPyLangGraphBridge, MagicMock]: + """Return a bridge backed by a mocked stream-router.""" + scheduler = MagicMock() + router = ReactiveStreamRouter(scheduler=scheduler) + router.agents = {"agent": MagicMock()} + bridge = RxPyLangGraphBridge(router) + return bridge, scheduler + + +def _build_mock_graph( + name: str, + *, + messages: list[dict[str, Any]] | None = None, + state_dict: dict[str, Any] | None = None, +) -> MagicMock: + """Build a mock LangGraph whose ``execute`` is an AsyncMock.""" + graph = MagicMock() + graph.name = name + + gs = MagicMock(spec=GraphState) + gs.messages = messages if messages is not None else [] + gs.to_dict = MagicMock(return_value=state_dict or {"messages": gs.messages}) + + graph.execute = AsyncMock(return_value=gs) + graph.get_execution_history = MagicMock(return_value=["s1"]) + graph.state_manager = MagicMock() + graph.state_manager.checkpoint_dir = None + graph.nodes = {} + return graph + + +# =================================================================== +# Scenario: Cancellation reason is recorded when cancelling a tracked task +# =================================================================== + + +@given("a bridge instance prepared for cancellation reason tracking") +def step_bridge_for_cancel_reason(context: Context) -> None: + bridge, _ = _build_bridge() + # Create an event loop and a dummy task + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + + async def _forever() -> None: + await asyncio.sleep(9999) + + task = loop.create_task(_forever()) + bridge._active_tasks.add(task) + + context.bridge = bridge + context.dummy_task = task + context.cancel_loop = loop + context.caught_error = None + + def _cleanup() -> None: + # Cancel all pending tasks and close the loop to avoid __del__ errors + for t in list(bridge._active_tasks): + t.cancel() + bridge._active_tasks.clear() + if not loop.is_closed(): + loop.run_until_complete(asyncio.sleep(0)) + loop.close() + asyncio.set_event_loop(asyncio.new_event_loop()) + + context._cleanup_handlers.append(_cleanup) + + +@when('I cancel a tracked task providing reason "{reason}"') +def step_cancel_task_with_given_reason(context: Context, reason: str) -> None: + loop = context.cancel_loop + + async def _do_cancel() -> None: + await context.bridge.cancel_task_with_reason(context.dummy_task, reason) + + loop.run_until_complete(_do_cancel()) + context.cancel_reason_used = reason + + +@then("the bridge cancellation_reasons mapping should contain the task") +def step_assert_cancellation_reasons_has_task(context: Context) -> None: + assert context.dummy_task in context.bridge.cancellation_reasons, ( + "Expected task to be present in cancellation_reasons" + ) + + +@then('the stored reason should equal "{expected}"') +def step_assert_stored_reason(context: Context, expected: str) -> None: + actual = context.bridge.cancellation_reasons.get(context.dummy_task) + assert actual == expected, f"Expected reason '{expected}', got '{actual}'" + # Cleanup + context.cancel_loop.close() + asyncio.set_event_loop(asyncio.new_event_loop()) + + +# =================================================================== +# Scenario: Cancelling a None task raises ValueError +# =================================================================== + + +@when('I try cancelling a None task with reason "{reason}"') +def step_cancel_none_task(context: Context, reason: str) -> None: + loop = context.cancel_loop + + async def _do_cancel() -> None: + await context.bridge.cancel_task_with_reason(None, reason) # type: ignore[arg-type] + + try: + loop.run_until_complete(_do_cancel()) + except ValueError as exc: + context.caught_error = exc + + +@then('a ValueError mentioning "{fragment}" should be raised') +def step_assert_value_error_with_message(context: Context, fragment: str) -> None: + assert context.caught_error is not None, "Expected a ValueError but none was raised" + assert isinstance(context.caught_error, ValueError), ( + f"Expected ValueError, got {type(context.caught_error).__name__}" + ) + assert fragment in str(context.caught_error), ( + f"Expected '{fragment}' in error message, got '{context.caught_error}'" + ) + # Cleanup + if hasattr(context, "cancel_loop") and not context.cancel_loop.is_closed(): + context.cancel_loop.close() + asyncio.set_event_loop(asyncio.new_event_loop()) + + +# =================================================================== +# Scenario: Cancelling a task with empty reason raises ValueError +# =================================================================== + + +@when("I try cancelling a valid task with an empty reason string") +def step_cancel_task_empty_reason(context: Context) -> None: + loop = context.cancel_loop + + async def _do_cancel() -> None: + await context.bridge.cancel_task_with_reason(context.dummy_task, "") + + try: + loop.run_until_complete(_do_cancel()) + except ValueError as exc: + context.caught_error = exc + + +# =================================================================== +# Scenario: Async cleanup cancels a not-yet-done task (line 68, branch 67->68) +# =================================================================== + + +@given("a bridge holding a deliberately stalled coroutine task") +def step_bridge_with_stalled_task(context: Context) -> None: + bridge, _ = _build_bridge() + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + + async def _stall() -> None: + await asyncio.sleep(99999) + + task = loop.create_task(_stall()) + bridge._active_tasks.add(task) + + context.bridge = bridge + context.stalled_task = task + context.async_cleanup_loop = loop + + +@when("I perform async cleanup with a 200ms timeout on stalled tasks") +def step_perform_async_cleanup(context: Context) -> None: + loop = context.async_cleanup_loop + loop.run_until_complete(context.bridge.cleanup_tasks_async(timeout=0.2)) + + +@then("every stalled task should have been cancelled or finished") +def step_assert_stalled_tasks_cancelled(context: Context) -> None: + task = context.stalled_task + assert task.cancelled() or task.done(), ( + f"Expected stalled task to be cancelled/done, state={task._state}" + ) + + +@then("the bridge task tracking set should be empty after async cleanup") +def step_assert_tracking_set_empty(context: Context) -> None: + assert len(context.bridge._active_tasks) == 0, ( + f"Expected empty _active_tasks, got {len(context.bridge._active_tasks)}" + ) + # Cleanup loop + if ( + hasattr(context, "async_cleanup_loop") + and not context.async_cleanup_loop.is_closed() + ): + context.async_cleanup_loop.close() + asyncio.set_event_loop(asyncio.new_event_loop()) + + +# =================================================================== +# Scenario: Async cleanup handles already-completed tasks +# =================================================================== + + +@given("a bridge holding an already-finished async task") +def step_bridge_with_finished_task(context: Context) -> None: + bridge, _ = _build_bridge() + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + + async def _instant() -> str: + return "done" + + task = loop.create_task(_instant()) + # Let the task complete + loop.run_until_complete(task) + assert task.done(), "Task should be done before adding to bridge" + bridge._active_tasks.add(task) + + context.bridge = bridge + context.async_cleanup_loop = loop + + +# =================================================================== +# Scenario: Graph executor inner coroutine with string content (lines 201-204) +# =================================================================== + + +@given("a bridge with a mock graph wired for direct coroutine invocation") +def step_bridge_for_direct_coroutine(context: Context) -> None: + bridge, _ = _build_bridge() + mock_graph = _build_mock_graph( + "direct_exec", + messages=[{"content": "coroutine-ok"}], + state_dict={"messages": [{"content": "coroutine-ok"}]}, + ) + bridge.graphs["direct_exec"] = mock_graph + context.bridge = bridge + context.direct_graph = mock_graph + context.exec_loop = asyncio.new_event_loop() + asyncio.set_event_loop(context.exec_loop) + + +@when("I invoke the executor coroutine with a plain string message body") +def step_invoke_executor_with_string(context: Context) -> None: + """Push a string message through the executor and await the task directly.""" + import rx # type: ignore + + bridge = context.bridge + executor_op = bridge._create_graph_executor({"graph": "direct_exec"}) + loop = context.exec_loop + + results: list[Any] = [] + errors: list[Any] = [] + msg = StreamMessage(content="hello from string", metadata={"src": "test"}) + + rx.just(msg).pipe(executor_op).subscribe( + on_next=lambda x: results.append(x), + on_error=lambda e: errors.append(e), + ) + + # Drain all pending tasks by awaiting them directly + pending = list(bridge._active_tasks) + if pending: + loop.run_until_complete(asyncio.gather(*pending, return_exceptions=True)) + else: + loop.run_until_complete(asyncio.sleep(0.05)) + + context.coroutine_results = results + context.coroutine_errors = errors + + +@then("the mock graph execute should have received a messages list with the string") +def step_assert_graph_received_string(context: Context) -> None: + assert len(context.coroutine_errors) == 0, ( + f"Coroutine raised errors: {context.coroutine_errors}" + ) + # Verify that graph.execute was called with input_data containing messages + call_args = context.direct_graph.execute.call_args + assert call_args is not None, "graph.execute was never called" + input_data = call_args[0][0] # First positional argument + assert "messages" in input_data, ( + f"Expected 'messages' key in input_data, got keys: {list(input_data.keys())}" + ) + assert input_data["messages"][0]["content"] == "hello from string", ( + f"Expected string content in messages, got: {input_data['messages']}" + ) + + +@then("the executor coroutine should return a StreamMessage with graph metadata") +def step_assert_coroutine_returns_stream_message(context: Context) -> None: + assert len(context.coroutine_results) > 0, "No results from executor coroutine" + result_msg = context.coroutine_results[0] + assert isinstance(result_msg, StreamMessage), ( + f"Expected StreamMessage, got {type(result_msg).__name__}" + ) + assert "graph" in result_msg.metadata, ( + f"Expected 'graph' in metadata, keys: {list(result_msg.metadata.keys())}" + ) + # Cleanup + if hasattr(context, "exec_loop") and not context.exec_loop.is_closed(): + context.exec_loop.close() + asyncio.set_event_loop(asyncio.new_event_loop()) + + +# =================================================================== +# Scenario: Graph executor inner coroutine with list content (else branch) +# =================================================================== + + +@when("I invoke the executor coroutine with a list payload as message body") +def step_invoke_executor_with_list(context: Context) -> None: + """Push a list (non-str, non-dict) message through the executor.""" + import rx # type: ignore + + bridge = context.bridge + executor_op = bridge._create_graph_executor({"graph": "direct_exec"}) + loop = context.exec_loop + + results: list[Any] = [] + errors: list[Any] = [] + list_payload = ["item_a", "item_b", "item_c"] + msg = StreamMessage(content=list_payload, metadata={}) + + rx.just(msg).pipe(executor_op).subscribe( + on_next=lambda x: results.append(x), + on_error=lambda e: errors.append(e), + ) + + pending = list(bridge._active_tasks) + if pending: + loop.run_until_complete(asyncio.gather(*pending, return_exceptions=True)) + else: + loop.run_until_complete(asyncio.sleep(0.05)) + + context.coroutine_results = results + context.coroutine_errors = errors + + +@then("the mock graph execute should have received a content key wrapping the list") +def step_assert_graph_received_list_wrapped(context: Context) -> None: + assert len(context.coroutine_errors) == 0, ( + f"Coroutine raised errors: {context.coroutine_errors}" + ) + call_args = context.direct_graph.execute.call_args + assert call_args is not None, "graph.execute was never called" + input_data = call_args[0][0] + assert "content" in input_data, ( + f"Expected 'content' key in input_data for list fallback, got: {list(input_data.keys())}" + ) + assert input_data["content"] == ["item_a", "item_b", "item_c"], ( + f"Expected list content, got: {input_data['content']}" + ) + # Cleanup + if hasattr(context, "exec_loop") and not context.exec_loop.is_closed(): + context.exec_loop.close() + asyncio.set_event_loop(asyncio.new_event_loop()) + + +# =================================================================== +# Scenario: Graph stream config with correct type/publication (branch 182->184) +# =================================================================== + + +@given('a bridge with a registered graph called "{name}"') +def step_bridge_with_named_registered_graph(context: Context, name: str) -> None: + bridge, _ = _build_bridge() + mock_graph = _build_mock_graph(name) + bridge.graphs[name] = mock_graph + context.bridge = bridge + + +@when('I obtain the stream configuration for graph "{name}"') +def step_obtain_stream_config(context: Context, name: str) -> None: + context.obtained_stream_config = context.bridge.create_graph_stream(name) + + +@then('the stream config name should equal "{expected}"') +def step_assert_stream_name_equals(context: Context, expected: str) -> None: + actual = context.obtained_stream_config.name + assert actual == expected, f"Expected stream name '{expected}', got '{actual}'" + + +@then('the stream config publications should include "{publication}"') +def step_assert_publications_include(context: Context, publication: str) -> None: + pubs = context.obtained_stream_config.publications + assert publication in pubs, f"Expected '{publication}' in publications {pubs}" + + +# =================================================================== +# Scenario: State checkpointer operator for valid graph (branch 260->258) +# =================================================================== + + +@given('the graph "{name}" has a checkpoint directory configured') +def step_configure_checkpoint_dir(context: Context, name: str) -> None: + graph = context.bridge.graphs[name] + graph.state_manager.checkpoint_dir = "/tmp/test_ckpt" + + +@when('I construct and apply the checkpointer operator for "{name}"') +def step_construct_and_apply_checkpointer(context: Context, name: str) -> None: + import rx # type: ignore + + checkpointer_op = context.bridge._create_state_checkpointer({"graph": name}) + msg = StreamMessage(content="ckpt-payload", metadata={"check": True}) + results: list[Any] = [] + rx.just(msg).pipe(checkpointer_op).subscribe( + on_next=lambda x: results.append(x), + ) + context.ckpt_results = results + + +@then("the checkpointer should have invoked _save_checkpoint on the state manager") +def step_assert_save_checkpoint_called(context: Context) -> None: + assert len(context.ckpt_results) > 0, "Checkpointer produced no output" + # The graph "ckpt_valid" should have had _save_checkpoint called + graph = context.bridge.graphs.get("ckpt_valid") + if graph is not None: + graph.state_manager._save_checkpoint.assert_called() + + +# =================================================================== +# Scenario: __del__ completes normally (branch 39->38 normal exit) +# =================================================================== + + +@given("a fully functional bridge for destructor testing") +def step_functional_bridge_for_del(context: Context) -> None: + bridge, _ = _build_bridge() + context.bridge = bridge + context.del_call_error = None + + +@when("__del__ is called on the fully functional bridge") +def step_call_del_on_functional_bridge(context: Context) -> None: + try: + context.bridge.__del__() + except Exception as exc: + context.del_call_error = exc + + +@then("the __del__ call should complete without any error") +def step_assert_del_no_error(context: Context) -> None: + assert context.del_call_error is None, ( + f"Expected no error from __del__, got {context.del_call_error!r}" + ) diff --git a/features/steps/checkpoint_manager_coverage_steps.py b/features/steps/checkpoint_manager_coverage_steps.py new file mode 100644 index 000000000..038204418 --- /dev/null +++ b/features/steps/checkpoint_manager_coverage_steps.py @@ -0,0 +1,652 @@ +"""Step definitions for checkpoint_manager_coverage.feature. + +Targets every method, branch, and edge case in checkpoint.py to achieve +full line-rate and branch-rate coverage. +""" + +from __future__ import annotations + +import os +import shutil +import tempfile +import threading +from datetime import datetime +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.infrastructure.sandbox.checkpoint import ( + Checkpointable, + CheckpointManager, + SandboxCheckpoint, +) +from cleveragents.infrastructure.sandbox.protocol import SandboxError + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_mock_sandbox( + sandbox_id: str = "sb-mock-001", + context: Any = None, +) -> MagicMock: + """Build a mock that satisfies the Checkpointable protocol.""" + sb = MagicMock() + sb.sandbox_id = sandbox_id + sb.context = context + return sb + + +def _make_temp_sandbox_dir() -> str: + """Create a temp dir with sample files and a subdirectory.""" + tmpdir = tempfile.mkdtemp(prefix="ca-cpmgr-test-") + os.makedirs(os.path.join(tmpdir, "subdir"), exist_ok=True) + with open(os.path.join(tmpdir, "file.txt"), "w") as f: + f.write("original\n") + with open(os.path.join(tmpdir, "subdir", "nested.txt"), "w") as f: + f.write("nested-original\n") + return tmpdir + + +def _register_cleanup(context: Context, path: str) -> None: + """Schedule a directory for cleanup after the scenario.""" + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(lambda: shutil.rmtree(path, ignore_errors=True)) + + +# =========================================================================== +# SandboxCheckpoint Pydantic model +# =========================================================================== + + +@given("I build a SandboxCheckpoint with valid fields") +def step_build_checkpoint_model(context: Context) -> None: + context.cp_model = SandboxCheckpoint( + checkpoint_id="cp-aaa", + sandbox_id="sb-bbb", + plan_id="plan-ccc", + phase="pre_execute", + created_at=datetime(2025, 1, 1, 12, 0, 0), + metadata={"key": "val"}, + snapshot_path="/tmp/snap", + ) + + +@then("the constructed checkpoint model should expose all field values") +def step_assert_checkpoint_fields(context: Context) -> None: + cp = context.cp_model + assert cp.checkpoint_id == "cp-aaa" + assert cp.sandbox_id == "sb-bbb" + assert cp.plan_id == "plan-ccc" + assert cp.phase == "pre_execute" + assert cp.created_at == datetime(2025, 1, 1, 12, 0, 0) + assert cp.metadata == {"key": "val"} + assert cp.snapshot_path == "/tmp/snap" + + +@then("the constructed checkpoint model should be frozen") +def step_assert_checkpoint_frozen(context: Context) -> None: + try: + context.cp_model.phase = "modified" + raise AssertionError("Expected ValidationError for frozen model") + except Exception: + pass # pydantic frozen model rejects mutation + + +@given("I build a SandboxCheckpoint without explicit metadata") +def step_build_checkpoint_no_metadata(context: Context) -> None: + context.cp_model = SandboxCheckpoint( + checkpoint_id="cp-ddd", + sandbox_id="sb-eee", + plan_id="plan-fff", + phase="post_execute", + created_at=datetime.now(), + snapshot_path="/tmp/snap2", + ) + + +@then("the constructed checkpoint metadata should be an empty dict") +def step_assert_empty_metadata_default(context: Context) -> None: + assert context.cp_model.metadata == {} + + +# =========================================================================== +# Checkpointable protocol +# =========================================================================== + + +@given("a mock object with sandbox_id and context properties") +def step_mock_checkpointable(context: Context) -> None: + class _Good: + @property + def sandbox_id(self) -> str: + return "sb-proto" + + @property + def context(self) -> Any: + return None + + context.proto_obj = _Good() + + +@then("the mock object should satisfy the Checkpointable protocol") +def step_assert_satisfies_checkpointable(context: Context) -> None: + assert isinstance(context.proto_obj, Checkpointable) + + +@given("a mock object without a sandbox_id property") +def step_mock_not_checkpointable(context: Context) -> None: + class _Bad: + @property + def context(self) -> Any: + return None + + context.proto_obj = _Bad() + + +@then("the mock object should not satisfy the Checkpointable protocol") +def step_assert_not_checkpointable(context: Context) -> None: + assert not isinstance(context.proto_obj, Checkpointable) + + +# =========================================================================== +# CheckpointManager.__init__ +# =========================================================================== + + +@given("a freshly created CheckpointManager") +def step_fresh_manager(context: Context) -> None: + context.mgr = CheckpointManager() + context.crafted_cp = None + context.cp_result = None + context.rollback_result = None + context.delete_result = None + context.cp_list = None + context.snapshot_result = None + context.raised_error = None + context.cleanup_error = None + + +@then("the manager internal checkpoints dict should be empty") +def step_assert_empty_checkpoints(context: Context) -> None: + assert context.mgr._checkpoints == {} + + +@then("the manager should have a threading lock") +def step_assert_has_lock(context: Context) -> None: + assert isinstance(context.mgr._lock, type(threading.RLock())) + + +# =========================================================================== +# create_checkpoint - context is None +# =========================================================================== + + +@given("a mock sandbox whose context is None") +def step_mock_sandbox_no_context(context: Context) -> None: + context.mock_sb = _make_mock_sandbox(sandbox_id="sb-nocontext") + + +@when( + 'I invoke create_checkpoint with plan "{plan_id}" phase "{phase}" and no metadata' +) +def step_create_cp_no_metadata(context: Context, plan_id: str, phase: str) -> None: + context.cp_result = context.mgr.create_checkpoint( + sandbox=context.mock_sb, + plan_id=plan_id, + phase=phase, + ) + + +@then("the returned checkpoint should have a non-empty snapshot_path directory") +def step_assert_snapshot_path_dir(context: Context) -> None: + sp = context.cp_result.snapshot_path + assert sp, "snapshot_path should not be empty" + assert os.path.isdir(sp), f"Expected directory at {sp}" + _register_cleanup(context, os.path.dirname(sp)) + + +@then("the returned checkpoint sandbox_id should equal the mock sandbox id") +def step_assert_cp_sandbox_id(context: Context) -> None: + assert context.cp_result.sandbox_id == context.mock_sb.sandbox_id + + +@when( + 'I invoke create_checkpoint with plan "{plan_id}" phase "{phase}" and numeric metadata' +) +def step_create_cp_numeric_metadata(context: Context, plan_id: str, phase: str) -> None: + context.cp_result = context.mgr.create_checkpoint( + sandbox=context.mock_sb, + plan_id=plan_id, + phase=phase, + metadata={"count": 42, "flag": True}, + ) + _register_cleanup(context, os.path.dirname(context.cp_result.snapshot_path)) + + +@then("the returned checkpoint metadata values should all be strings") +def step_assert_metadata_strings(context: Context) -> None: + for k, v in context.cp_result.metadata.items(): + assert isinstance(v, str), f"metadata[{k!r}] = {v!r} is not a string" + + +@when( + 'I invoke create_checkpoint with plan "{plan_id}" phase "{phase}" and None metadata' +) +def step_create_cp_none_metadata(context: Context, plan_id: str, phase: str) -> None: + context.cp_result = context.mgr.create_checkpoint( + sandbox=context.mock_sb, + plan_id=plan_id, + phase=phase, + metadata=None, + ) + _register_cleanup(context, os.path.dirname(context.cp_result.snapshot_path)) + + +@then("the returned checkpoint metadata should be an empty dict") +def step_assert_cp_metadata_empty(context: Context) -> None: + assert context.cp_result.metadata == {} + + +# =========================================================================== +# create_checkpoint - context with sandbox_path +# =========================================================================== + + +@given("a temporary sandbox directory with sample files") +def step_temp_sandbox_dir(context: Context) -> None: + context.temp_sandbox = _make_temp_sandbox_dir() + _register_cleanup(context, context.temp_sandbox) + + +@given("a mock sandbox whose context points to the temporary directory") +def step_mock_sandbox_with_context(context: Context) -> None: + ctx = MagicMock() + ctx.sandbox_path = context.temp_sandbox + context.mock_sb = _make_mock_sandbox(sandbox_id="sb-withctx", context=ctx) + + +@then("the snapshot directory should contain copies of the sample files") +def step_assert_snapshot_has_files(context: Context) -> None: + snap = context.cp_result.snapshot_path + _register_cleanup(context, os.path.dirname(snap)) + assert os.path.isfile(os.path.join(snap, "file.txt")) + assert os.path.isfile(os.path.join(snap, "subdir", "nested.txt")) + with open(os.path.join(snap, "file.txt")) as f: + assert f.read() == "original\n" + + +# =========================================================================== +# _snapshot_directory - branches +# =========================================================================== + + +@when("I call _snapshot_directory with a None sandbox_path") +def step_snapshot_none_path(context: Context) -> None: + context.snapshot_result = context.mgr._snapshot_directory(None, "cp-test-none") + _register_cleanup(context, os.path.dirname(context.snapshot_result)) + + +@then("the returned snapshot should be an existing empty directory") +def step_assert_snapshot_empty_dir(context: Context) -> None: + sp = context.snapshot_result + assert os.path.isdir(sp), f"Expected directory at {sp}" + assert os.listdir(sp) == [], f"Expected empty dir, got {os.listdir(sp)}" + + +@when("I call _snapshot_directory with a non-existent sandbox_path") +def step_snapshot_nonexistent_path(context: Context) -> None: + context.snapshot_result = context.mgr._snapshot_directory( + "/nonexistent/path/xyz", "cp-test-noexist" + ) + _register_cleanup(context, os.path.dirname(context.snapshot_result)) + + +@when("I call _snapshot_directory with the temporary sandbox_path") +def step_snapshot_valid_path(context: Context) -> None: + context.snapshot_result = context.mgr._snapshot_directory( + context.temp_sandbox, "cp-test-valid" + ) + _register_cleanup(context, os.path.dirname(context.snapshot_result)) + + +@then("the returned snapshot should contain the sample files") +def step_assert_snapshot_has_sample_files(context: Context) -> None: + sp = context.snapshot_result + assert os.path.isfile(os.path.join(sp, "file.txt")) + assert os.path.isfile(os.path.join(sp, "subdir", "nested.txt")) + + +@when("I call _snapshot_directory with a path that triggers OSError") +def step_snapshot_oserror(context: Context) -> None: + context.raised_error = None + with patch("shutil.copytree", side_effect=OSError("mock copy failure")): + try: + context.mgr._snapshot_directory(context.temp_sandbox, "cp-test-err") + except SandboxError as exc: + context.raised_error = exc + + +@then("a SandboxError should have been raised") +def step_assert_sandbox_error(context: Context) -> None: + assert context.raised_error is not None, "Expected SandboxError to be raised" + assert isinstance(context.raised_error, SandboxError) + assert "Failed to snapshot" in str(context.raised_error) + + +# =========================================================================== +# _cleanup_snapshot - branches +# =========================================================================== + + +@when("I call _cleanup_snapshot with an empty string") +def step_cleanup_empty_string(context: Context) -> None: + context.cleanup_error = None + try: + CheckpointManager._cleanup_snapshot("") + except Exception as exc: + context.cleanup_error = exc + + +@then("no error should occur from cleanup_snapshot") +def step_assert_no_cleanup_error(context: Context) -> None: + assert context.cleanup_error is None, ( + f"Expected no error, got {context.cleanup_error!r}" + ) + + +@given("a temporary snapshot directory tree for cleanup") +def step_temp_snapshot_for_cleanup(context: Context) -> None: + parent = tempfile.mkdtemp(prefix="ca-cleanup-test-") + snap = os.path.join(parent, "snapshot") + os.makedirs(snap, exist_ok=True) + with open(os.path.join(snap, "data.txt"), "w") as f: + f.write("to-be-cleaned\n") + context.cleanup_parent = parent + context.cleanup_snapshot = snap + + +@when("I call _cleanup_snapshot with the temporary snapshot path") +def step_cleanup_real_snapshot(context: Context) -> None: + context.cleanup_error = None + try: + CheckpointManager._cleanup_snapshot(context.cleanup_snapshot) + except Exception as exc: + context.cleanup_error = exc + + +@then("the temporary snapshot parent should no longer exist") +def step_assert_parent_removed(context: Context) -> None: + assert not os.path.exists(context.cleanup_parent), ( + f"Parent should have been removed: {context.cleanup_parent}" + ) + + +@when("I call _cleanup_snapshot with a non-existent path") +def step_cleanup_nonexistent(context: Context) -> None: + context.cleanup_error = None + try: + CheckpointManager._cleanup_snapshot("/nonexistent/snapshot/path") + except Exception as exc: + context.cleanup_error = exc + + +# =========================================================================== +# rollback_to - all branches +# =========================================================================== + + +@given("a SandboxCheckpoint with empty snapshot_path") +def step_cp_empty_snapshot(context: Context) -> None: + context.crafted_cp = SandboxCheckpoint( + checkpoint_id="cp-empty-snap", + sandbox_id="sb-rb1", + plan_id="plan-rb1", + phase="pre_execute", + created_at=datetime.now(), + metadata={}, + snapshot_path="", + ) + + +@when("I invoke rollback_to on the crafted checkpoint") +def step_rollback_crafted(context: Context) -> None: + if getattr(context, "rollback_should_patch_oserror", False): + # Patch os.listdir to raise on the second call (copy phase) + orig_listdir = os.listdir + call_count = {"n": 0} + + def _failing_listdir(path: str) -> list[str]: + call_count["n"] += 1 + if call_count["n"] >= 2: + raise OSError("simulated listdir failure") + return orig_listdir(path) + + with patch("os.listdir", side_effect=_failing_listdir): + context.rollback_result = context.mgr.rollback_to(context.crafted_cp) + else: + context.rollback_result = context.mgr.rollback_to(context.crafted_cp) + + +@then("the rollback result should be false") +def step_assert_rollback_false(context: Context) -> None: + assert context.rollback_result is False, "Expected rollback to return False" + + +@given("a SandboxCheckpoint with a non-existent snapshot_path") +def step_cp_nonexistent_snapshot(context: Context) -> None: + context.crafted_cp = SandboxCheckpoint( + checkpoint_id="cp-nosnap", + sandbox_id="sb-rb2", + plan_id="plan-rb2", + phase="pre_execute", + created_at=datetime.now(), + metadata={}, + snapshot_path="/nonexistent/snapshot/dir", + ) + + +@given("a SandboxCheckpoint whose snapshot exists but metadata has no sandbox_path") +def step_cp_no_sandbox_path_meta(context: Context) -> None: + # Create a real snapshot dir so the first guard passes + snap_parent = tempfile.mkdtemp(prefix="ca-rb-test-") + snap = os.path.join(snap_parent, "snapshot") + os.makedirs(snap, exist_ok=True) + _register_cleanup(context, snap_parent) + context.crafted_cp = SandboxCheckpoint( + checkpoint_id="cp-nometa", + sandbox_id="sb-rb3", + plan_id="plan-rb3", + phase="pre_execute", + created_at=datetime.now(), + metadata={}, # no sandbox_path key + snapshot_path=snap, + ) + + +@given("a SandboxCheckpoint whose snapshot exists but sandbox_path points nowhere") +def step_cp_sandbox_path_missing(context: Context) -> None: + snap_parent = tempfile.mkdtemp(prefix="ca-rb-test2-") + snap = os.path.join(snap_parent, "snapshot") + os.makedirs(snap, exist_ok=True) + _register_cleanup(context, snap_parent) + context.crafted_cp = SandboxCheckpoint( + checkpoint_id="cp-badpath", + sandbox_id="sb-rb4", + plan_id="plan-rb4", + phase="pre_execute", + created_at=datetime.now(), + metadata={"sandbox_path": "/nonexistent/sandbox/dir"}, + snapshot_path=snap, + ) + + +@given("a checkpoint was created for the temporary sandbox") +def step_create_cp_for_temp_sandbox(context: Context) -> None: + context.cp_result = context.mgr.create_checkpoint( + sandbox=context.mock_sb, + plan_id="plan-rb-ok", + phase="pre_execute", + metadata={"sandbox_path": context.temp_sandbox}, + ) + _register_cleanup(context, os.path.dirname(context.cp_result.snapshot_path)) + context.crafted_cp = context.cp_result + + +@given("the temporary sandbox files are then modified") +def step_modify_temp_sandbox(context: Context) -> None: + # Overwrite existing file + with open(os.path.join(context.temp_sandbox, "file.txt"), "w") as f: + f.write("modified!\n") + # Add a new file + with open(os.path.join(context.temp_sandbox, "extra.txt"), "w") as f: + f.write("extra\n") + # Add a new subdirectory with file + new_sub = os.path.join(context.temp_sandbox, "newdir") + os.makedirs(new_sub, exist_ok=True) + with open(os.path.join(new_sub, "added.txt"), "w") as f: + f.write("added\n") + + +@when("I invoke rollback_to on the created checkpoint") +def step_rollback_created_cp(context: Context) -> None: + context.rollback_result = context.mgr.rollback_to(context.cp_result) + + +@then("the rollback result should be true") +def step_assert_rollback_true(context: Context) -> None: + assert context.rollback_result is True, "Expected rollback to return True" + + +@then("the temporary sandbox should contain the original files") +def step_assert_temp_sandbox_restored(context: Context) -> None: + with open(os.path.join(context.temp_sandbox, "file.txt")) as f: + assert f.read() == "original\n", "file.txt should be restored" + with open(os.path.join(context.temp_sandbox, "subdir", "nested.txt")) as f: + assert f.read() == "nested-original\n", "nested.txt should be restored" + assert not os.path.exists(os.path.join(context.temp_sandbox, "extra.txt")), ( + "extra.txt should not exist after rollback" + ) + assert not os.path.exists(os.path.join(context.temp_sandbox, "newdir")), ( + "newdir should not exist after rollback" + ) + + +@given("a SandboxCheckpoint whose snapshot and sandbox exist but restore will fail") +def step_cp_rollback_oserror(context: Context) -> None: + # Create a real snapshot dir with a file + snap_parent = tempfile.mkdtemp(prefix="ca-rb-err-") + snap = os.path.join(snap_parent, "snapshot") + os.makedirs(snap, exist_ok=True) + with open(os.path.join(snap, "ok.txt"), "w") as f: + f.write("ok\n") + + # Create a real sandbox dir + sandbox_dir = tempfile.mkdtemp(prefix="ca-rb-err-sb-") + with open(os.path.join(sandbox_dir, "existing.txt"), "w") as f: + f.write("existing\n") + + _register_cleanup(context, snap_parent) + _register_cleanup(context, sandbox_dir) + + context.crafted_cp = SandboxCheckpoint( + checkpoint_id="cp-oserr", + sandbox_id="sb-rb-err", + plan_id="plan-rb-err", + phase="pre_execute", + created_at=datetime.now(), + metadata={"sandbox_path": sandbox_dir}, + snapshot_path=snap, + ) + # Flag the OSError scenario so the when step knows to patch + context.rollback_should_patch_oserror = True + + +# =========================================================================== +# list_checkpoints - branches +# =========================================================================== + + +@when('I invoke list_checkpoints for sandbox "{sandbox_id}"') +def step_list_checkpoints_by_id(context: Context, sandbox_id: str) -> None: + context.cp_list = context.mgr.list_checkpoints(sandbox_id) + + +@then("the checkpoint list should be empty") +def step_assert_list_empty(context: Context) -> None: + assert context.cp_list == [], f"Expected empty list, got {context.cp_list}" + + +@given("I create two checkpoints for the mock sandbox") +def step_create_two_checkpoints(context: Context) -> None: + context.mgr.create_checkpoint( + sandbox=context.mock_sb, + plan_id="plan-list", + phase="pre_execute", + ) + context.mgr.create_checkpoint( + sandbox=context.mock_sb, + plan_id="plan-list", + phase="post_execute", + ) + + +@when("I invoke list_checkpoints for the mock sandbox") +def step_list_checkpoints_mock_sb(context: Context) -> None: + context.cp_list = context.mgr.list_checkpoints(context.mock_sb.sandbox_id) + + +@then("the checkpoint list should have {count:d} entries in creation order") +def step_assert_list_count_ordered(context: Context, count: int) -> None: + assert len(context.cp_list) == count, ( + f"Expected {count} entries, got {len(context.cp_list)}" + ) + for i in range(1, len(context.cp_list)): + assert context.cp_list[i].created_at >= context.cp_list[i - 1].created_at + + +# =========================================================================== +# delete_checkpoint - branches +# =========================================================================== + + +@given("a single checkpoint is created for the mock sandbox") +def step_create_single_cp(context: Context) -> None: + context.cp_result = context.mgr.create_checkpoint( + sandbox=context.mock_sb, + plan_id="plan-del", + phase="pre_execute", + ) + _register_cleanup(context, os.path.dirname(context.cp_result.snapshot_path)) + + +@when("I invoke delete_checkpoint with the created checkpoint id") +def step_delete_created_cp(context: Context) -> None: + context.delete_result = context.mgr.delete_checkpoint( + context.cp_result.checkpoint_id + ) + + +@then("the checkpoint delete result should be true") +def step_assert_delete_true(context: Context) -> None: + assert context.delete_result is True, "Expected delete to return True" + + +@then("listing checkpoints for the mock sandbox should return {count:d} entries") +def step_assert_list_after_delete(context: Context, count: int) -> None: + result = context.mgr.list_checkpoints(context.mock_sb.sandbox_id) + assert len(result) == count, f"Expected {count} entries, got {len(result)}" + + +@when('I invoke delete_checkpoint with id "{cp_id}"') +def step_delete_unknown_cp(context: Context, cp_id: str) -> None: + context.delete_result = context.mgr.delete_checkpoint(cp_id) + + +@then("the checkpoint delete result should be false") +def step_assert_delete_false(context: Context) -> None: + assert context.delete_result is False, "Expected delete to return False" diff --git a/features/steps/config_cli_safety_net_coverage_steps.py b/features/steps/config_cli_safety_net_coverage_steps.py new file mode 100644 index 000000000..29a5e5ac5 --- /dev/null +++ b/features/steps/config_cli_safety_net_coverage_steps.py @@ -0,0 +1,684 @@ +"""Step definitions for config_cli_safety_net_coverage.feature. + +Safety-net tests exercising every function in config.py to maintain +100% line and branch coverage. All step text uses a 'safety-net' prefix +to avoid collisions with config_cli_steps.py and +config_cli_uncovered_branches_steps.py. +""" + +from __future__ import annotations + +import contextlib +import json +import os +import shutil +import tempfile +from io import StringIO +from pathlib import Path +from typing import Any +from unittest.mock import patch + +import typer +import yaml +from behave import given, then, when +from behave.runner import Context +from rich.console import Console +from typer.testing import CliRunner + +from cleveragents.cli.commands import config as config_mod +from cleveragents.cli.commands.config import ( + _env_var_for_key, + _is_secret_key, + _mask_value, + _normalize_key, + _read_config_file, + _resolution_chain, + _resolve_source, + _settings_fields, + _validate_key, + _write_config_file, +) +from cleveragents.cli.commands.config import app as config_app + +_runner = CliRunner() + + +# --------------------------------------------------------------------------- +# Helpers - isolated temp directory for config file operations +# --------------------------------------------------------------------------- + + +def _setup_safety_net_temp(context: Context) -> None: + """Redirect _CONFIG_DIR/_CONFIG_PATH to a fresh temp directory.""" + if not hasattr(context, "_sn_tmpdir"): + context._sn_tmpdir = tempfile.mkdtemp(prefix="cfg_safety_net_") + tmp = Path(context._sn_tmpdir) + context._sn_dir_patch = patch.object(config_mod, "_CONFIG_DIR", tmp) + context._sn_path_patch = patch.object( + config_mod, "_CONFIG_PATH", tmp / "config.toml" + ) + context._sn_dir_patch.start() + context._sn_path_patch.start() + + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers: list[Any] = [] + context._cleanup_handlers.append(lambda: _teardown_safety_net_temp(context)) + + +def _teardown_safety_net_temp(context: Context) -> None: + for attr in ("_sn_dir_patch", "_sn_path_patch"): + patcher = getattr(context, attr, None) + if patcher is not None: + with contextlib.suppress(RuntimeError): + patcher.stop() + tmpdir = getattr(context, "_sn_tmpdir", None) + if tmpdir and os.path.isdir(tmpdir): + shutil.rmtree(tmpdir, ignore_errors=True) + + +# =================================================================== +# Background / Given +# =================================================================== + + +@given("a safety-net isolated temp config directory") +def step_sn_temp_dir(context: Context) -> None: + _setup_safety_net_temp(context) + + +@given('a safety-net toml config file containing key "{key}" with value "{value}"') +def step_sn_write_toml(context: Context, key: str, value: str) -> None: + import tomlkit + + config_path: Path = config_mod._CONFIG_PATH # type: ignore[assignment] + config_path.parent.mkdir(parents=True, exist_ok=True) + doc = tomlkit.document() + # Coerce booleans + if value.lower() == "true": + doc[key] = True + elif value.lower() == "false": + doc[key] = False + else: + doc[key] = value + with open(config_path, "w") as fh: + tomlkit.dump(doc, fh) + + +@given('the safety-net CLI has previously set "{key}" to "{value}"') +def step_sn_pre_set(context: Context, key: str, value: str) -> None: + result = _runner.invoke(config_app, ["set", key, value]) + assert result.exit_code == 0, f"Pre-set failed: {result.output}" + + +@given('the safety-net env var "{var}" is set to "{value}"') +def step_sn_set_env(context: Context, var: str, value: str) -> None: + os.environ[var] = value + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(lambda: os.environ.pop(var, None)) + + +@given("safety-net settings fields include a Path-typed value") +def step_sn_mock_path_field(context: Context) -> None: + """Patch _settings_fields to include a Path value that must be serialised.""" + real_fields = _settings_fields() + # Inject a Path value into a known field + real_fields["log_level"] = Path("/mock/safety/net/path") + + context._sn_fields_patch = patch.object( + config_mod, "_settings_fields", return_value=real_fields + ) + context._sn_fields_patch.start() + context._cleanup_handlers.append(context._sn_fields_patch.stop) + + +@given("safety-net settings fields include a secret key with a non-pattern value") +def step_sn_mock_secret_field(context: Context) -> None: + """Patch _settings_fields to include a field whose name matches _SECRET_PATTERNS. + + Uses a value that does NOT match redact_value patterns (no sk-, tok_, etc.) + so that format_output's _redact_data won't double-mask it. + """ + # Use a plain secret value that won't match redact_value regex patterns + context._sn_actual_secret = "my-secret-value-here" + mock_fields = { + "log_level": "INFO", + "api_key": context._sn_actual_secret, + } + mock_defaults = { + "log_level": "INFO", + "api_key": None, + } + + context._sn_fields_patch2 = patch.object( + config_mod, "_settings_fields", return_value=mock_fields + ) + context._sn_defaults_patch2 = patch.object( + config_mod, "_settings_defaults", return_value=mock_defaults + ) + context._sn_resolve_patch2 = patch.object( + config_mod, "_resolve_source", return_value="default" + ) + context._sn_fields_patch2.start() + context._sn_defaults_patch2.start() + context._sn_resolve_patch2.start() + context._cleanup_handlers.append(context._sn_fields_patch2.stop) + context._cleanup_handlers.append(context._sn_defaults_patch2.stop) + context._cleanup_handlers.append(context._sn_resolve_patch2.stop) + + +@given("safety-net settings fields have a value different from default") +def step_sn_mock_modified_field(context: Context) -> None: + """Patch fields so that a value differs from its default.""" + mock_fields = {"log_level": "DEBUG"} + mock_defaults = {"log_level": "INFO"} + + context._sn_fields_patch3 = patch.object( + config_mod, "_settings_fields", return_value=mock_fields + ) + context._sn_defaults_patch3 = patch.object( + config_mod, "_settings_defaults", return_value=mock_defaults + ) + context._sn_resolve_patch3 = patch.object( + config_mod, "_resolve_source", return_value="config_file" + ) + context._sn_fields_patch3.start() + context._sn_defaults_patch3.start() + context._sn_resolve_patch3.start() + context._cleanup_handlers.append(context._sn_fields_patch3.stop) + context._cleanup_handlers.append(context._sn_defaults_patch3.stop) + context._cleanup_handlers.append(context._sn_resolve_patch3.stop) + + +# =================================================================== +# _normalize_key (L91-96) +# =================================================================== + + +@when('the safety-net normalizer processes key "{key}"') +def step_sn_normalize(context: Context, key: str) -> None: + context._sn_normalized = _normalize_key(key) + + +@then('the safety-net normalized result should be "{expected}"') +def step_sn_normalized_equals(context: Context, expected: str) -> None: + assert context._sn_normalized == expected, ( + f"Expected '{expected}', got '{context._sn_normalized}'" + ) + + +# =================================================================== +# _is_secret_key (L116-118) +# =================================================================== + + +@when('the safety-net secret checker inspects key "{key}"') +def step_sn_is_secret(context: Context, key: str) -> None: + context._sn_is_secret = _is_secret_key(key) + + +@then("the safety-net secret check result should be true") +def step_sn_secret_true(context: Context) -> None: + assert context._sn_is_secret is True, "Expected secret=True" + + +@then("the safety-net secret check result should be false") +def step_sn_secret_false(context: Context) -> None: + assert context._sn_is_secret is False, "Expected secret=False" + + +# =================================================================== +# _mask_value (L121-123) +# =================================================================== + + +@when('the safety-net masker masks value "{value}"') +def step_sn_mask(context: Context, value: str) -> None: + context._sn_masked = _mask_value(value) + + +@when("the safety-net masker masks an empty string value") +def step_sn_mask_empty(context: Context) -> None: + context._sn_masked = _mask_value("") + + +@then('the safety-net masked output should be "{expected}"') +def step_sn_masked_equals(context: Context, expected: str) -> None: + assert context._sn_masked == expected, ( + f"Expected '{expected}', got '{context._sn_masked}'" + ) + + +# =================================================================== +# _env_var_for_key (L158-160) +# =================================================================== + + +@when('the safety-net env var builder processes key "{key}"') +def step_sn_env_var(context: Context, key: str) -> None: + context._sn_env_var = _env_var_for_key(key) + + +@then('the safety-net env var name should be "{expected}"') +def step_sn_env_var_equals(context: Context, expected: str) -> None: + assert context._sn_env_var == expected, ( + f"Expected '{expected}', got '{context._sn_env_var}'" + ) + + +# =================================================================== +# _read_config_file (L126-131) +# =================================================================== + + +@when("the safety-net reader reads the config file") +def step_sn_read_config(context: Context) -> None: + context._sn_read_result = _read_config_file() + + +@then("the safety-net read result should be an empty dict") +def step_sn_read_empty(context: Context) -> None: + assert context._sn_read_result == {}, ( + f"Expected empty dict, got {context._sn_read_result}" + ) + + +@then('the safety-net read result should contain key "{key}"') +def step_sn_read_has_key(context: Context, key: str) -> None: + assert key in context._sn_read_result, ( + f"Key '{key}' not in {context._sn_read_result}" + ) + + +# =================================================================== +# _write_config_file (L134-155) - create new +# =================================================================== + + +@when('the safety-net writer writes key "{key}" with value "{value}"') +def step_sn_write_config(context: Context, key: str, value: str) -> None: + _write_config_file({key: value}) + + +@then('the safety-net config file should exist and contain key "{key}"') +def step_sn_file_has_key(context: Context, key: str) -> None: + import tomllib + + config_path: Path = config_mod._CONFIG_PATH # type: ignore[assignment] + assert config_path.exists(), f"Config file does not exist at {config_path}" + with open(config_path, "rb") as fh: + data = tomllib.load(fh) + assert key in data, f"Key '{key}' not in file data: {data}" + + +# =================================================================== +# _settings_fields (L62-70) +# =================================================================== + + +@when("the safety-net fields loader retrieves all settings fields") +def step_sn_settings_fields(context: Context) -> None: + context._sn_fields = _settings_fields() + + +@then("the safety-net fields result should be a non-empty dict") +def step_sn_fields_nonempty(context: Context) -> None: + assert isinstance(context._sn_fields, dict), ( + f"Expected dict, got {type(context._sn_fields)}" + ) + assert len(context._sn_fields) > 0, "Expected non-empty dict of fields" + + +# =================================================================== +# _validate_key (L99-113) - unknown key +# =================================================================== + + +@when('the safety-net validator checks unknown key "{key}"') +def step_sn_validate_unknown(context: Context, key: str) -> None: + context._sn_validate_exc = None + try: + _validate_key(key) + except typer.BadParameter as exc: + context._sn_validate_exc = exc + + +@then('the safety-net validator should raise BadParameter with "{msg}"') +def step_sn_validate_bad_param(context: Context, msg: str) -> None: + assert context._sn_validate_exc is not None, ( + "Expected typer.BadParameter but no exception raised" + ) + assert msg.lower() in str(context._sn_validate_exc).lower(), ( + f"Expected '{msg}' in: {context._sn_validate_exc}" + ) + + +@when('the safety-net validator checks valid key "{key}"') +def step_sn_validate_valid(context: Context, key: str) -> None: + context._sn_validate_result = _validate_key(key) + + +@then('the safety-net validator should return "{expected}"') +def step_sn_validate_returns(context: Context, expected: str) -> None: + assert context._sn_validate_result == expected, ( + f"Expected '{expected}', got '{context._sn_validate_result}'" + ) + + +# =================================================================== +# _resolve_source (L163-174) +# =================================================================== + + +@when('the safety-net source resolver checks key "{key}"') +def step_sn_resolve_source(context: Context, key: str) -> None: + context._sn_source = _resolve_source(key) + + +@then('the safety-net resolved source should be "{expected}"') +def step_sn_source_equals(context: Context, expected: str) -> None: + assert context._sn_source == expected, ( + f"Expected '{expected}', got '{context._sn_source}'" + ) + + +# =================================================================== +# _resolution_chain (L177-209) +# =================================================================== + + +@when('the safety-net chain builder builds chain for key "{key}"') +def step_sn_resolution_chain(context: Context, key: str) -> None: + context._sn_chain = _resolution_chain(key) + + +@then("the safety-net chain should have exactly {count:d} entries") +def step_sn_chain_count(context: Context, count: int) -> None: + assert len(context._sn_chain) == count, ( + f"Expected {count} entries, got {len(context._sn_chain)}" + ) + + +@then('the safety-net chain sources should be "{sources}"') +def step_sn_chain_sources(context: Context, sources: str) -> None: + expected = [s.strip() for s in sources.split(",")] + actual = [entry["source"] for entry in context._sn_chain] + assert actual == expected, f"Expected sources {expected}, got {actual}" + + +# =================================================================== +# config_set - type coercion (L241-248) +# =================================================================== + + +@when('the safety-net CLI sets key "{key}" to value "{value}" with format "{fmt}"') +def step_sn_cli_set(context: Context, key: str, value: str, fmt: str) -> None: + context._sn_result = _runner.invoke( + config_app, ["set", key, value, "--format", fmt] + ) + + +@then("the safety-net set output should be valid JSON") +def step_sn_set_valid_json(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + parsed = json.loads(result.output.strip()) + assert isinstance(parsed, dict), f"Expected dict, got {type(parsed)}" + context._sn_set_json = parsed + + +@then('the safety-net set JSON field "value" should be boolean true') +def step_sn_set_bool_true(context: Context) -> None: + assert context._sn_set_json["value"] is True, ( + f"Expected True, got {context._sn_set_json['value']!r}" + ) + + +@then('the safety-net set JSON field "value" should be boolean false') +def step_sn_set_bool_false(context: Context) -> None: + assert context._sn_set_json["value"] is False, ( + f"Expected False, got {context._sn_set_json['value']!r}" + ) + + +@then('the safety-net set JSON field "value" should be integer {expected:d}') +def step_sn_set_int(context: Context, expected: int) -> None: + val = context._sn_set_json["value"] + assert val == expected and isinstance(val, int), ( + f"Expected int {expected}, got {val!r} ({type(val).__name__})" + ) + + +@then('the safety-net set JSON field "value" should be float {expected:g}') +def step_sn_set_float(context: Context, expected: float) -> None: + val = context._sn_set_json["value"] + assert isinstance(val, float) and abs(val - expected) < 1e-9, ( + f"Expected float {expected}, got {val!r} ({type(val).__name__})" + ) + + +@then('the safety-net set JSON field "value" should be string "{expected}"') +def step_sn_set_string(context: Context, expected: str) -> None: + val = context._sn_set_json["value"] + assert val == expected and isinstance(val, str), ( + f"Expected string '{expected}', got {val!r} ({type(val).__name__})" + ) + + +@then('the safety-net set JSON field "previous_value" should be string "{expected}"') +def step_sn_set_previous(context: Context, expected: str) -> None: + val = context._sn_set_json["previous_value"] + assert val == expected, f"Expected previous '{expected}', got {val!r}" + + +@then('the safety-net set rich output should contain "{text}"') +def step_sn_set_rich_contains(context: Context, text: str) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + assert text in result.output, f"Expected '{text}' in output: {result.output[:500]}" + + +# =================================================================== +# config_get - rich and non-rich formats (L279-335) +# =================================================================== + + +@when('the safety-net CLI gets key "{key}" with format "{fmt}"') +def step_sn_cli_get(context: Context, key: str, fmt: str) -> None: + context._sn_result = _runner.invoke(config_app, ["get", key, "--format", fmt]) + + +@then('the safety-net get rich output should contain "{text}"') +def step_sn_get_rich_contains(context: Context, text: str) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + assert text in result.output, f"Expected '{text}' in output: {result.output[:500]}" + + +@then("the safety-net get output should be valid YAML") +def step_sn_get_valid_yaml(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + parsed = yaml.safe_load(result.output.strip()) + assert parsed is not None, "YAML parsed to None" + context._sn_get_yaml = parsed + + +@then('the safety-net get YAML should contain key "{key}"') +def step_sn_get_yaml_key(context: Context, key: str) -> None: + assert key in context._sn_get_yaml, ( + f"Key '{key}' not in YAML: {list(context._sn_get_yaml.keys())}" + ) + + +@then("the safety-net get output should be valid JSON with type field") +def step_sn_get_json_type(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + parsed = json.loads(result.output.strip()) + assert "type" in parsed, f"'type' not in JSON keys: {list(parsed.keys())}" + assert "key" in parsed, f"'key' not in JSON keys: {list(parsed.keys())}" + assert "resolution_chain" in parsed, ( + f"'resolution_chain' not in JSON: {list(parsed.keys())}" + ) + + +# =================================================================== +# config_list - formats (L426-449) +# =================================================================== + + +@when('the safety-net CLI lists config with format "{fmt}"') +def step_sn_cli_list_format(context: Context, fmt: str) -> None: + context._sn_result = _runner.invoke(config_app, ["list", "--format", fmt]) + + +@then("the safety-net list output should be valid YAML list") +def step_sn_list_valid_yaml(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + parsed = yaml.safe_load(result.output.strip()) + assert isinstance(parsed, list), f"Expected list, got {type(parsed)}" + assert len(parsed) > 0, "Expected non-empty YAML list" + + +@then("the safety-net list plain output should contain key-value lines") +def step_sn_list_plain(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + # Plain format should have key: value style lines + assert "key:" in result.output or "log_level" in result.output, ( + f"Expected plain key-value in output: {result.output[:500]}" + ) + + +@then("the safety-net list table output should contain column headers") +def step_sn_list_table(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + # Table format should have column header text + output = result.output.lower() + assert "key" in output or "value" in output, ( + f"Expected table headers in output: {result.output[:500]}" + ) + + +# =================================================================== +# config_list - combined key + value filter (L391-399) +# =================================================================== + + +@when( + 'the safety-net CLI lists config with key pattern "{pattern}" and value filter "{vfilter}"' +) +def step_sn_cli_list_combined(context: Context, pattern: str, vfilter: str) -> None: + context._sn_result = _runner.invoke( + config_app, ["list", pattern, "--filter-values", vfilter] + ) + + +@then("the safety-net combined filter result should succeed") +def step_sn_combined_ok(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + + +# =================================================================== +# config_list - Path serialisation (L428-429) +# =================================================================== + + +@then('the safety-net list JSON output should not contain "PosixPath" or "WindowsPath"') +def step_sn_list_no_path_obj(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + output = result.output + assert "PosixPath" not in output, f"Found PosixPath in output: {output[:500]}" + assert "WindowsPath" not in output, f"Found WindowsPath in output: {output[:500]}" + + +# =================================================================== +# config_list - secret masking (L408-411) +# =================================================================== + + +@when("the safety-net CLI lists all config in json format") +def step_sn_cli_list_json(context: Context) -> None: + context._sn_result = _runner.invoke(config_app, ["list", "--format", "json"]) + + +@when("the safety-net CLI lists all config in json format with show-secrets") +def step_sn_cli_list_json_secrets(context: Context) -> None: + context._sn_result = _runner.invoke( + config_app, ["list", "--format", "json", "--show-secrets"] + ) + + +@then('the safety-net list JSON should contain masked value "****" for the secret key') +def step_sn_list_masked(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + parsed = json.loads(result.output.strip()) + api_entries = [e for e in parsed if e.get("key") == "api_key"] + assert len(api_entries) > 0, f"No api_key entry found in: {parsed}" + assert api_entries[0]["value"] == "****", ( + f"Expected masked ****, got {api_entries[0]['value']!r}" + ) + + +@then("the safety-net list JSON should contain the actual secret value") +def step_sn_list_unmasked(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + parsed = json.loads(result.output.strip()) + api_entries = [e for e in parsed if e.get("key") == "api_key"] + assert len(api_entries) > 0, f"No api_key entry found in: {parsed}" + expected = context._sn_actual_secret + assert api_entries[0]["value"] == expected, ( + f"Expected '{expected}', got {api_entries[0]['value']!r}" + ) + + +# =================================================================== +# config_list - modified flag (L405-407) +# =================================================================== + + +@then("the safety-net list JSON should include a modified flag set to true") +def step_sn_list_modified(context: Context) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + parsed = json.loads(result.output.strip()) + modified_entries = [e for e in parsed if e.get("modified") is True] + assert len(modified_entries) > 0, ( + f"No entries with modified=True found in: {parsed}" + ) + + +# =================================================================== +# config_get - active marker via patched console (L333) +# =================================================================== + + +@given("a safety-net patched console for capturing rich output") +def step_sn_patch_console(context: Context) -> None: + """Patch the module-level console to capture rich output in a StringIO buffer.""" + context._sn_console_buf = StringIO() + context._sn_test_console = Console( + file=context._sn_console_buf, no_color=True, width=200 + ) + context._sn_console_patch = patch.object( + config_mod, "console", context._sn_test_console + ) + context._sn_console_patch.start() + context._cleanup_handlers.append(context._sn_console_patch.stop) + + +@then('the safety-net captured console output should contain "{text}"') +def step_sn_console_contains(context: Context, text: str) -> None: + result = context._sn_result + assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" + console_output = context._sn_console_buf.getvalue() + assert text in console_output, ( + f"Expected '{text}' in captured console output: {console_output[:500]}" + ) diff --git a/features/steps/decision_service_coverage_steps.py b/features/steps/decision_service_coverage_steps.py new file mode 100644 index 000000000..cd5422756 --- /dev/null +++ b/features/steps/decision_service_coverage_steps.py @@ -0,0 +1,474 @@ +"""Step definitions for decision_service_coverage.feature. + +Exercises every method of ``DecisionService`` using mocked UnitOfWork +and repository objects so that decision_service.py achieves full +line-rate and branch-rate coverage. + +All step names use the ``dsvc-`` prefix to avoid collisions with +existing step definitions in other feature files. +""" + +from __future__ import annotations + +from contextlib import contextmanager +from typing import Any +from unittest.mock import MagicMock + +from behave import given, then, when +from behave.runner import Context +from ulid import ULID + +from cleveragents.application.services.decision_service import DecisionService +from cleveragents.domain.models.core.decision import ( + Decision, + DecisionType, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_PLAN_ID = str(ULID()) +_DECISION_ID = str(ULID()) +_NEW_DECISION_ID = str(ULID()) +_LEAF_ID = str(ULID()) +_ROOT_ID = str(ULID()) + + +def _make_decision( + decision_id: str | None = None, + plan_id: str = _PLAN_ID, + parent_decision_id: str | None = None, + sequence_number: int = 0, + decision_type: DecisionType = DecisionType.PROMPT_DEFINITION, + superseded_by: str | None = None, +) -> Decision: + """Create a minimal valid Decision for testing.""" + kwargs: dict[str, Any] = { + "plan_id": plan_id, + "sequence_number": sequence_number, + "decision_type": decision_type, + "question": "Which approach?", + "chosen_option": "REST API", + } + if decision_id is not None: + kwargs["decision_id"] = decision_id + if parent_decision_id is not None: + kwargs["parent_decision_id"] = parent_decision_id + if superseded_by is not None: + kwargs["superseded_by"] = superseded_by + return Decision(**kwargs) + + +def _build_mock_uow() -> tuple[MagicMock, MagicMock, MagicMock]: + """Build a mock UnitOfWork with a mock transaction context manager. + + Returns: + (mock_uow, mock_ctx, mock_decisions_repo) + """ + mock_decisions = MagicMock(name="decisions_repo") + mock_ctx = MagicMock(name="uow_context") + mock_ctx.decisions = mock_decisions + + mock_uow = MagicMock(name="unit_of_work") + + @contextmanager + def _fake_transaction(): + yield mock_ctx + + mock_uow.transaction = _fake_transaction + return mock_uow, mock_ctx, mock_decisions + + +def _build_service(mock_uow: MagicMock) -> tuple[DecisionService, MagicMock]: + """Construct a DecisionService with mock settings and the given UoW. + + Returns: + (service, mock_settings) + """ + mock_settings = MagicMock(name="settings") + service = DecisionService(settings=mock_settings, unit_of_work=mock_uow) + return service, mock_settings + + +# --------------------------------------------------------------------------- +# Constructor +# --------------------------------------------------------------------------- + + +@given("dsvc- a mock settings object and a mock UnitOfWork") +def step_dsvc_mock_deps(context: Context) -> None: + context.dsvc_settings = MagicMock(name="settings") + context.dsvc_uow = MagicMock(name="unit_of_work") + + +@when("dsvc- I construct a DecisionService with those dependencies") +def step_dsvc_construct(context: Context) -> None: + context.dsvc_service = DecisionService( + settings=context.dsvc_settings, + unit_of_work=context.dsvc_uow, + ) + + +@then("dsvc- the service should store the settings") +def step_dsvc_check_settings(context: Context) -> None: + assert context.dsvc_service.settings is context.dsvc_settings, ( + "Service did not store settings" + ) + + +@then("dsvc- the service should store the unit_of_work") +def step_dsvc_check_uow(context: Context) -> None: + assert context.dsvc_service.unit_of_work is context.dsvc_uow, ( + "Service did not store unit_of_work" + ) + + +@then("dsvc- the service should have a bound structlog logger") +def step_dsvc_check_logger(context: Context) -> None: + # The _logger is a BoundLogger with service="decision" + assert hasattr(context.dsvc_service, "_logger"), "Service missing _logger attribute" + + +# --------------------------------------------------------------------------- +# Shared Given: service + mock UoW +# --------------------------------------------------------------------------- + + +@given("dsvc- a DecisionService with a mocked UnitOfWork") +def step_dsvc_service_with_mock_uow(context: Context) -> None: + mock_uow, mock_ctx, mock_decisions = _build_mock_uow() + service, _ = _build_service(mock_uow) + + # Attach a mock logger so non-logger scenarios can still verify calls + mock_logger = MagicMock(name="structlog_logger") + mock_logger.bind = MagicMock(return_value=mock_logger) + service._logger = mock_logger + + context.dsvc_service = service + context.dsvc_uow = mock_uow + context.dsvc_ctx = mock_ctx + context.dsvc_decisions = mock_decisions + context.dsvc_logger = mock_logger + + +@given("dsvc- a DecisionService with a mocked UnitOfWork and captured logger") +def step_dsvc_service_with_logger(context: Context) -> None: + mock_uow, mock_ctx, mock_decisions = _build_mock_uow() + service, _ = _build_service(mock_uow) + + # Replace the _logger with a MagicMock so we can assert calls + mock_logger = MagicMock(name="structlog_logger") + mock_logger.bind = MagicMock(return_value=mock_logger) + service._logger = mock_logger + + context.dsvc_service = service + context.dsvc_uow = mock_uow + context.dsvc_ctx = mock_ctx + context.dsvc_decisions = mock_decisions + context.dsvc_logger = mock_logger + + +# --------------------------------------------------------------------------- +# record_decision +# --------------------------------------------------------------------------- + + +@given("dsvc- a sample root Decision object") +def step_dsvc_sample_decision(context: Context) -> None: + context.dsvc_decision = _make_decision(plan_id=_PLAN_ID) + + +@when("dsvc- I call record_decision with the sample decision") +def step_dsvc_call_record(context: Context) -> None: + context.dsvc_result = context.dsvc_service.record_decision( + context.dsvc_decision, + ) + + +@then("dsvc- the UoW transaction should have been entered") +def step_dsvc_txn_entered(context: Context) -> None: + # The fact that ctx.decisions.create was called proves we entered + # the transaction context manager successfully. We verify that + # the mock context's decisions repo was accessed. + assert context.dsvc_decisions.create.called, ( + "Transaction context was not entered — create was never called" + ) + + +@then("dsvc- ctx.decisions.create should have been called with the decision") +def step_dsvc_create_called(context: Context) -> None: + context.dsvc_decisions.create.assert_called_once_with(context.dsvc_decision) + + +@then("dsvc- the returned decision should be the same object") +def step_dsvc_record_returns_same(context: Context) -> None: + assert context.dsvc_result is context.dsvc_decision, ( + "record_decision should return the same Decision instance" + ) + + +# --------------------------------------------------------------------------- +# record_decision logging +# --------------------------------------------------------------------------- + + +@then('dsvc- the logger should have recorded an info call with "{event}"') +def step_dsvc_logger_info(context: Context, event: str) -> None: + info_calls = context.dsvc_logger.info.call_args_list + events = [c.args[0] if c.args else c.kwargs.get("event", "") for c in info_calls] + assert event in events, f"Expected info event '{event}' in {events}" + + +@then('dsvc- the logger should have recorded a debug call with "{event}"') +def step_dsvc_logger_debug(context: Context, event: str) -> None: + debug_calls = context.dsvc_logger.debug.call_args_list + events = [c.args[0] if c.args else c.kwargs.get("event", "") for c in debug_calls] + assert event in events, f"Expected debug event '{event}' in {events}" + + +# --------------------------------------------------------------------------- +# get_decision +# --------------------------------------------------------------------------- + + +@given("dsvc- the mock repo get method returns a Decision") +def step_dsvc_mock_get_returns(context: Context) -> None: + context.dsvc_expected_decision = _make_decision( + decision_id=_DECISION_ID, + plan_id=_PLAN_ID, + ) + context.dsvc_decisions.get.return_value = context.dsvc_expected_decision + + +@given("dsvc- the mock repo get method returns None") +def step_dsvc_mock_get_returns_none(context: Context) -> None: + context.dsvc_decisions.get.return_value = None + + +@when("dsvc- I call get_decision with a known ID") +def step_dsvc_call_get(context: Context) -> None: + context.dsvc_result = context.dsvc_service.get_decision(_DECISION_ID) + + +@when("dsvc- I call get_decision with an unknown ID") +def step_dsvc_call_get_unknown(context: Context) -> None: + context.dsvc_result = context.dsvc_service.get_decision("NONEXISTENT_ID_12345678") + + +@then("dsvc- ctx.decisions.get should have been called with the ID") +def step_dsvc_get_called(context: Context) -> None: + context.dsvc_decisions.get.assert_called_once_with(_DECISION_ID) + + +@then("dsvc- the returned value should be the expected Decision") +def step_dsvc_get_returns_expected(context: Context) -> None: + assert context.dsvc_result is context.dsvc_expected_decision, ( + "get_decision did not return the expected Decision" + ) + + +@then("dsvc- the returned value should be None") +def step_dsvc_result_is_none(context: Context) -> None: + assert context.dsvc_result is None, f"Expected None, got {context.dsvc_result!r}" + + +# --------------------------------------------------------------------------- +# get_decisions_for_plan +# --------------------------------------------------------------------------- + + +@given("dsvc- the mock repo get_by_plan method returns {count:d} decisions") +def step_dsvc_mock_get_by_plan(context: Context, count: int) -> None: + decisions = [ + _make_decision( + plan_id=_PLAN_ID, + sequence_number=i, + decision_type=( + DecisionType.PROMPT_DEFINITION + if i == 0 + else DecisionType.STRATEGY_CHOICE + ), + parent_decision_id=None if i == 0 else str(ULID()), + ) + for i in range(count) + ] + context.dsvc_decisions.get_by_plan.return_value = decisions + context.dsvc_expected_count = count + + +@when("dsvc- I call get_decisions_for_plan with a plan ID") +def step_dsvc_call_get_for_plan(context: Context) -> None: + context.dsvc_result = context.dsvc_service.get_decisions_for_plan(_PLAN_ID) + + +@then("dsvc- ctx.decisions.get_by_plan should have been called with the plan ID") +def step_dsvc_get_by_plan_called(context: Context) -> None: + context.dsvc_decisions.get_by_plan.assert_called_once_with(_PLAN_ID) + + +@then("dsvc- the returned list should have {count:d} decisions") +def step_dsvc_list_count(context: Context, count: int) -> None: + assert len(context.dsvc_result) == count, ( + f"Expected {count} decisions, got {len(context.dsvc_result)}" + ) + + +# --------------------------------------------------------------------------- +# get_decision_tree +# --------------------------------------------------------------------------- + + +@given("dsvc- the mock repo get_tree method returns {count:d} decisions") +def step_dsvc_mock_get_tree(context: Context, count: int) -> None: + root = _make_decision(decision_id=_ROOT_ID, plan_id=_PLAN_ID) + decisions = [root] + [ + _make_decision( + plan_id=_PLAN_ID, + sequence_number=i, + decision_type=DecisionType.STRATEGY_CHOICE, + parent_decision_id=_ROOT_ID, + ) + for i in range(1, count) + ] + context.dsvc_decisions.get_tree.return_value = decisions + context.dsvc_expected_tree_count = count + + +@when("dsvc- I call get_decision_tree with a root ID") +def step_dsvc_call_get_tree(context: Context) -> None: + context.dsvc_result = context.dsvc_service.get_decision_tree(_ROOT_ID) + + +@then("dsvc- ctx.decisions.get_tree should have been called with the root ID") +def step_dsvc_get_tree_called(context: Context) -> None: + context.dsvc_decisions.get_tree.assert_called_once_with(_ROOT_ID) + + +@then("dsvc- the returned tree list should have {count:d} decisions") +def step_dsvc_tree_count(context: Context, count: int) -> None: + assert len(context.dsvc_result) == count, ( + f"Expected {count} tree nodes, got {len(context.dsvc_result)}" + ) + + +# --------------------------------------------------------------------------- +# get_path_to_root +# --------------------------------------------------------------------------- + + +@given("dsvc- the mock repo get_path_to_root method returns {count:d} decisions") +def step_dsvc_mock_get_path(context: Context, count: int) -> None: + decisions = [ + _make_decision( + plan_id=_PLAN_ID, + sequence_number=count - 1 - i, + decision_type=( + DecisionType.PROMPT_DEFINITION + if i == count - 1 + else DecisionType.STRATEGY_CHOICE + ), + parent_decision_id=None if i == count - 1 else str(ULID()), + ) + for i in range(count) + ] + context.dsvc_decisions.get_path_to_root.return_value = decisions + context.dsvc_expected_path_count = count + + +@when("dsvc- I call get_path_to_root with a leaf ID") +def step_dsvc_call_get_path(context: Context) -> None: + context.dsvc_result = context.dsvc_service.get_path_to_root(_LEAF_ID) + + +@then("dsvc- ctx.decisions.get_path_to_root should have been called with the leaf ID") +def step_dsvc_get_path_called(context: Context) -> None: + context.dsvc_decisions.get_path_to_root.assert_called_once_with(_LEAF_ID) + + +@then("dsvc- the returned path list should have {count:d} decisions") +def step_dsvc_path_count(context: Context, count: int) -> None: + assert len(context.dsvc_result) == count, ( + f"Expected {count} path nodes, got {len(context.dsvc_result)}" + ) + + +# --------------------------------------------------------------------------- +# mark_superseded +# --------------------------------------------------------------------------- + + +@given("dsvc- the mock repo update_superseded_by method returns a superseded Decision") +def step_dsvc_mock_superseded(context: Context) -> None: + context.dsvc_superseded_decision = _make_decision( + decision_id=_DECISION_ID, + plan_id=_PLAN_ID, + superseded_by=_NEW_DECISION_ID, + ) + context.dsvc_decisions.update_superseded_by.return_value = ( + context.dsvc_superseded_decision + ) + + +@when("dsvc- I call mark_superseded with old and new IDs") +def step_dsvc_call_mark_superseded(context: Context) -> None: + context.dsvc_result = context.dsvc_service.mark_superseded( + _DECISION_ID, + _NEW_DECISION_ID, + ) + + +@then("dsvc- ctx.decisions.update_superseded_by should have been called with both IDs") +def step_dsvc_update_superseded_called(context: Context) -> None: + context.dsvc_decisions.update_superseded_by.assert_called_once_with( + _DECISION_ID, + _NEW_DECISION_ID, + ) + + +@then("dsvc- the returned decision should be the superseded one") +def step_dsvc_superseded_returned(context: Context) -> None: + assert context.dsvc_result is context.dsvc_superseded_decision, ( + "mark_superseded did not return the expected superseded Decision" + ) + + +# --------------------------------------------------------------------------- +# list_by_type +# --------------------------------------------------------------------------- + + +@given("dsvc- the mock repo list_by_type method returns {count:d} decisions") +def step_dsvc_mock_list_by_type(context: Context, count: int) -> None: + decisions = [ + _make_decision( + plan_id=_PLAN_ID, + sequence_number=i, + decision_type=DecisionType.STRATEGY_CHOICE, + parent_decision_id=str(ULID()), + ) + for i in range(count) + ] + context.dsvc_decisions.list_by_type.return_value = decisions + context.dsvc_expected_type_count = count + + +@when('dsvc- I call list_by_type with plan ID and type "{dtype}"') +def step_dsvc_call_list_by_type(context: Context, dtype: str) -> None: + context.dsvc_list_type_arg = dtype + context.dsvc_result = context.dsvc_service.list_by_type(_PLAN_ID, dtype) + + +@then("dsvc- ctx.decisions.list_by_type should have been called with plan ID and type") +def step_dsvc_list_by_type_called(context: Context) -> None: + context.dsvc_decisions.list_by_type.assert_called_once_with( + _PLAN_ID, + context.dsvc_list_type_arg, + ) + + +@then("dsvc- the returned type list should have {count:d} decisions") +def step_dsvc_type_list_count(context: Context, count: int) -> None: + assert len(context.dsvc_result) == count, ( + f"Expected {count} decisions by type, got {len(context.dsvc_result)}" + ) diff --git a/features/steps/plan_apply_service_branch_coverage_steps.py b/features/steps/plan_apply_service_branch_coverage_steps.py new file mode 100644 index 000000000..d131be92c --- /dev/null +++ b/features/steps/plan_apply_service_branch_coverage_steps.py @@ -0,0 +1,241 @@ +"""Step definitions for plan_apply_service_branch_coverage feature. + +Targets the missed branch at line 426→432 in plan_apply_service.py. +When self._logger.error() raises an exception inside handle_merge_failure, +execution skips the ``return plan`` statement and exits via exception +propagation. This file exercises both the normal (return) and exception +(propagation) paths. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] + +from cleveragents.application.services.plan_apply_service import ( + PlanApplyService, +) +from cleveragents.domain.models.core.plan import ( + PlanPhase, + PlanTimestamps, + ProcessingState, +) + +__all__: list[str] = [] + +# Plan / changeset IDs used across scenarios +_PLAN_ID = "01BRANCHTEST000000000001" + + +# ---------------------------------------------------------------------- +# Helpers +# ---------------------------------------------------------------------- + + +class _StubPhase: + """Lightweight stub that mirrors PlanPhase comparison.""" + + def __init__(self, phase: PlanPhase) -> None: + self._phase = phase + self.value = phase.value + + def __eq__(self, other: object) -> bool: + if isinstance(other, PlanPhase): + return self._phase == other + if isinstance(other, _StubPhase): + return self._phase == other._phase + return NotImplemented + + +class _StubState: + """Lightweight stub that mirrors ProcessingState comparison.""" + + def __init__(self, state: ProcessingState) -> None: + self._state = state + self.value = state.value + + def __eq__(self, other: object) -> bool: + if isinstance(other, ProcessingState): + return self._state == other + if isinstance(other, _StubState): + return self._state == other._state + return NotImplemented + + +def _make_mock_plan( + *, + plan_id: str = _PLAN_ID, + phase: PlanPhase = PlanPhase.EXECUTE, + state: ProcessingState = ProcessingState.COMPLETE, + error_details: dict[str, str] | None = None, +) -> MagicMock: + """Create a mock Plan with sensible defaults.""" + plan = MagicMock() + plan.identity.plan_id = plan_id + plan.phase = _StubPhase(phase) + plan.processing_state = _StubState(state) + plan.changeset_id = None + plan.validation_summary = None + plan.is_terminal = False + plan.error_details = error_details + plan.timestamps = PlanTimestamps() + plan.sandbox_refs = ["sandbox-ref-branch"] + return plan + + +def _make_lifecycle_mock(plan: MagicMock) -> MagicMock: + """Create a mock PlanLifecycleService that returns *plan*.""" + lifecycle = MagicMock() + lifecycle.get_plan.return_value = plan + lifecycle._commit_plan = MagicMock() + lifecycle.fail_apply = MagicMock(return_value=plan) + return lifecycle + + +# ====================================================================== +# Scenario: logger.error raises → exception propagates (branch 426→432) +# ====================================================================== + + +@given("pas_branch a service whose logger.error will raise RuntimeError") +def step_service_logger_error_raises(context: Context) -> None: + """Build a PlanApplyService and patch _logger.error to raise.""" + plan = _make_mock_plan(error_details=None) + context.branch_plan = plan + + lifecycle = _make_lifecycle_mock(plan) + context.branch_lifecycle = lifecycle + + service = PlanApplyService( + lifecycle_service=lifecycle, + changeset_store=None, + ) + + # Replace the logger's .error method so it raises + mock_logger = MagicMock() + mock_logger.error.side_effect = RuntimeError("simulated logger failure") + # Keep other logger methods working + mock_logger.info = MagicMock() + mock_logger.warning = MagicMock() + mock_logger.debug = MagicMock() + service._logger = mock_logger + + context.branch_service = service + context.branch_mock_logger = mock_logger + + +@when("pas_branch I call handle_merge_failure and capture any exception") +def step_call_handle_merge_failure_capture(context: Context) -> None: + """Call handle_merge_failure and capture any raised exception.""" + context.branch_raised_error = None + context.branch_returned_plan = None + try: + context.branch_returned_plan = context.branch_service.handle_merge_failure( + plan_id=_PLAN_ID, + conflict_details="conflicting changes in main.py", + ) + except RuntimeError as exc: + context.branch_raised_error = exc + + +@then("pas_branch a RuntimeError should have been raised") +def step_runtime_error_raised(context: Context) -> None: + """Assert that a RuntimeError was raised from the logger.""" + assert context.branch_raised_error is not None, ( + "Expected RuntimeError to propagate from handle_merge_failure, " + "but no exception was raised" + ) + assert isinstance(context.branch_raised_error, RuntimeError), ( + f"Expected RuntimeError, got {type(context.branch_raised_error).__name__}" + ) + assert "simulated logger failure" in str(context.branch_raised_error) + + +@then("pas_branch the plan error_details should still contain merge_conflict") +def step_plan_has_merge_conflict(context: Context) -> None: + """Assert error_details were set before the logger blew up.""" + details = context.branch_plan.error_details + assert details is not None, "error_details should have been set before the error" + assert "merge_conflict" in details, ( + f"Expected 'merge_conflict' in error_details, got keys: {list(details.keys())}" + ) + assert details["merge_conflict"] == "conflicting changes in main.py" + assert details["sandbox_rollback"] == "pending" + + +@then("pas_branch lifecycle _commit_plan should have been invoked before the error") +def step_commit_plan_invoked(context: Context) -> None: + """Assert _commit_plan was called (it runs before the logger call).""" + context.branch_lifecycle._commit_plan.assert_called_once() + + +@then("pas_branch lifecycle fail_apply should have been invoked before the error") +def step_fail_apply_invoked(context: Context) -> None: + """Assert fail_apply was called (it runs before the logger call).""" + context.branch_lifecycle.fail_apply.assert_called_once_with( + _PLAN_ID, + "Merge failed: conflicting changes in main.py", + ) + + +# ====================================================================== +# Scenario: logger.error succeeds → normal return (branch 426→431) +# ====================================================================== + + +@given("pas_branch a service whose logger.error will succeed normally") +def step_service_logger_error_succeeds(context: Context) -> None: + """Build a PlanApplyService with a working (mocked) logger.""" + plan = _make_mock_plan(error_details={"pre_existing": "value"}) + context.branch_plan = plan + + errored_plan = _make_mock_plan( + state=ProcessingState.ERRORED, + error_details={"pre_existing": "value", "merge_conflict": "test"}, + ) + lifecycle = _make_lifecycle_mock(plan) + lifecycle.fail_apply.return_value = errored_plan + context.branch_lifecycle = lifecycle + + service = PlanApplyService( + lifecycle_service=lifecycle, + changeset_store=None, + ) + + # Replace logger with a mock that does NOT raise + mock_logger = MagicMock() + service._logger = mock_logger + + context.branch_service = service + context.branch_mock_logger = mock_logger + + +@when("pas_branch I call handle_merge_failure normally") +def step_call_handle_merge_failure_normal(context: Context) -> None: + """Call handle_merge_failure expecting it to succeed.""" + context.branch_returned_plan = context.branch_service.handle_merge_failure( + plan_id=_PLAN_ID, + conflict_details="minor conflict in utils.py", + ) + + +@then("pas_branch the returned plan should not be None") +def step_returned_plan_not_none(context: Context) -> None: + """Assert the method returned a plan (did not raise).""" + assert context.branch_returned_plan is not None, ( + "Expected handle_merge_failure to return a plan, got None" + ) + + +@then( + "pas_branch the service logger.error should have been called with merge failure details" +) +def step_logger_error_called(context: Context) -> None: + """Assert logger.error was called with the expected arguments.""" + context.branch_mock_logger.error.assert_called_once_with( + "Merge failure handled", + plan_id=_PLAN_ID, + conflict_details="minor conflict in utils.py", + ) diff --git a/features/steps/plan_cli_uncovered_region_coverage_steps.py b/features/steps/plan_cli_uncovered_region_coverage_steps.py new file mode 100644 index 000000000..08d38824f --- /dev/null +++ b/features/steps/plan_cli_uncovered_region_coverage_steps.py @@ -0,0 +1,604 @@ +"""Step definitions for plan_cli_uncovered_region_coverage.feature. + +Covers uncovered lines/branches in cleveragents/cli/commands/plan.py: +- Lines 1950-1954: lifecycle_list_plans empty result after filtering +- Lines 1956-1957: lifecycle_list_plans non-rich format output +- Line 2132: revert_plan CleverAgentsError handler +- Lines 2136-2143: _get_apply_service function body +- Lines 2149-2184: plan_diff command (correction branch + error paths) +- Lines 2202-2233: plan_artifacts command (success + error paths) +- Lines 2231-2273: correct_decision command (execution, dry-run, error handlers) + +All step text uses the ``uncov-rgn`` prefix to avoid collisions with +other step files in the project. +""" + +from __future__ import annotations + +from datetime import datetime +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.cli.commands.plan import app as plan_app +from cleveragents.core.exceptions import ( + CleverAgentsError, + PlanError, + ResourceNotFoundError, + ValidationError, +) +from cleveragents.domain.models.core.plan import ( + NamespacedName, + Plan, + PlanIdentity, + PlanPhase, + PlanTimestamps, + ProcessingState, + ProjectLink, +) + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +_ULID_A = "01ARZ3NDEKTSV4RRFFQ69G5FAA" + +_PATCH_GET_LIFECYCLE = "cleveragents.cli.commands.plan._get_lifecycle_service" +_PATCH_GET_APPLY = "cleveragents.cli.commands.plan._get_apply_service" +_PATCH_CORRECTION_SVC = "cleveragents.cli.commands.plan.CorrectionService" +_PATCH_RESOLVE_ACTIVE = "cleveragents.cli.commands.plan._resolve_active_plan_id" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_plan( + *, + plan_id: str = _ULID_A, + name: str = "local/uncov-test-plan", + phase: PlanPhase = PlanPhase.STRATEGIZE, + processing_state: ProcessingState = ProcessingState.QUEUED, + action_name: str = "local/test-action", + project_links: list[ProjectLink] | None = None, +) -> Plan: + """Build a real Plan domain object for testing.""" + return Plan( + identity=PlanIdentity(plan_id=plan_id), + namespaced_name=NamespacedName.parse(name), + action_name=action_name, + description="Coverage test plan", + definition_of_done=None, + strategy_actor=None, + execution_actor=None, + phase=phase, + processing_state=processing_state, + project_links=project_links or [], + timestamps=PlanTimestamps( + created_at=datetime(2025, 6, 15, 10, 0, 0), + updated_at=datetime(2025, 6, 15, 11, 0, 0), + ), + reusable=True, + read_only=False, + created_by=None, + ) + + +# --------------------------------------------------------------------------- +# Given — CLI runner and shared mocked lifecycle service +# --------------------------------------------------------------------------- + + +@given("an uncov-rgn CLI runner and mocked lifecycle service") +def step_uncov_rgn_cli_runner(context: Context) -> None: + """Initialise a CliRunner, mock lifecycle service, and cleanup list.""" + context.uncov_runner = CliRunner() + context.uncov_mock_svc = MagicMock() + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + + p = patch(_PATCH_GET_LIFECYCLE, return_value=context.uncov_mock_svc) + p.start() + context._cleanup_handlers.append(p.stop) + + +# --------------------------------------------------------------------------- +# Given — lifecycle_list_plans scenarios +# --------------------------------------------------------------------------- + + +@given( + "the uncov-rgn lifecycle service returns plans that do not match an action filter" +) +def step_uncov_rgn_plans_no_match(context: Context) -> None: + """Return plans whose action_name differs from the filter value.""" + plans = [ + _make_plan(action_name="local/other-action"), + ] + context.uncov_mock_svc.list_plans.return_value = plans + + +@given("the uncov-rgn lifecycle service returns a single plan for listing") +def step_uncov_rgn_single_plan_for_list(context: Context) -> None: + """Return one plan so the list command renders in non-rich format.""" + plans = [_make_plan()] + context.uncov_mock_svc.list_plans.return_value = plans + + +# --------------------------------------------------------------------------- +# Given — revert_plan CleverAgentsError +# --------------------------------------------------------------------------- + + +@given("the uncov-rgn lifecycle service revert raises CleverAgentsError") +def step_uncov_rgn_revert_ca_error(context: Context) -> None: + """Configure revert_plan to raise CleverAgentsError.""" + context.uncov_mock_svc.revert_plan.side_effect = CleverAgentsError( + "revert service unavailable" + ) + + +# --------------------------------------------------------------------------- +# Given — _get_apply_service +# --------------------------------------------------------------------------- + + +@given("an uncov-rgn mocked lifecycle service for apply service creation") +def step_uncov_rgn_mock_lifecycle_for_apply(context: Context) -> None: + """Prepare patches for _get_apply_service test.""" + context.uncov_mock_lifecycle = MagicMock() + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + + +# --------------------------------------------------------------------------- +# Given — plan_diff scenarios +# --------------------------------------------------------------------------- + + +@given('the uncov-rgn apply service diff returns formatted output "{output}"') +def step_uncov_rgn_apply_diff_ok(context: Context, output: str) -> None: + """Set up a mock apply service whose diff() returns the given output.""" + mock_apply = MagicMock() + mock_apply.diff.return_value = output + context.uncov_mock_apply = mock_apply + + +@given('the uncov-rgn apply service diff raises PlanError "{msg}"') +def step_uncov_rgn_apply_diff_plan_error(context: Context, msg: str) -> None: + """Set up a mock apply service whose diff() raises PlanError.""" + mock_apply = MagicMock() + mock_apply.diff.side_effect = PlanError(msg) + context.uncov_mock_apply = mock_apply + + +@given('the uncov-rgn apply service diff raises CleverAgentsError "{msg}"') +def step_uncov_rgn_apply_diff_ca_error(context: Context, msg: str) -> None: + """Set up a mock apply service whose diff() raises CleverAgentsError.""" + mock_apply = MagicMock() + mock_apply.diff.side_effect = CleverAgentsError(msg) + context.uncov_mock_apply = mock_apply + + +# --------------------------------------------------------------------------- +# Given — plan_artifacts scenarios +# --------------------------------------------------------------------------- + + +@given('the uncov-rgn apply service artifacts returns "{output}"') +def step_uncov_rgn_apply_artifacts_ok_dq(context: Context, output: str) -> None: + """Set up a mock apply service whose artifacts() returns the given output.""" + mock_apply = MagicMock() + mock_apply.artifacts.return_value = output + context.uncov_mock_apply = mock_apply + + +@given("the uncov-rgn apply service artifacts returns '{output}'") +def step_uncov_rgn_apply_artifacts_ok_sq(context: Context, output: str) -> None: + """Set up a mock apply service with single-quoted output.""" + mock_apply = MagicMock() + mock_apply.artifacts.return_value = output + context.uncov_mock_apply = mock_apply + + +@given('the uncov-rgn apply service artifacts raises PlanError "{msg}"') +def step_uncov_rgn_apply_artifacts_plan_error(context: Context, msg: str) -> None: + """Set up a mock apply service whose artifacts() raises PlanError.""" + mock_apply = MagicMock() + mock_apply.artifacts.side_effect = PlanError(msg) + context.uncov_mock_apply = mock_apply + + +@given('the uncov-rgn apply service artifacts raises CleverAgentsError "{msg}"') +def step_uncov_rgn_apply_artifacts_ca_error(context: Context, msg: str) -> None: + """Set up a mock apply service whose artifacts() raises CleverAgentsError.""" + mock_apply = MagicMock() + mock_apply.artifacts.side_effect = CleverAgentsError(msg) + context.uncov_mock_apply = mock_apply + + +# --------------------------------------------------------------------------- +# Given — correct_decision scenarios +# --------------------------------------------------------------------------- + + +def _make_mock_correction_svc( + *, + request_return: object | None = None, + request_side_effect: Exception | None = None, + impact_return: object | None = None, + execute_return: object | None = None, +) -> MagicMock: + """Build a mock CorrectionService.""" + svc = MagicMock() + + if request_side_effect: + svc.request_correction.side_effect = request_side_effect + elif request_return: + svc.request_correction.return_value = request_return + else: + # Default request + svc.request_correction.return_value = SimpleNamespace( + correction_id="CORR-001", + mode=SimpleNamespace(value="revert"), + target_decision_id="DEC-001", + guidance="test guidance", + ) + + if impact_return: + svc.analyze_impact.return_value = impact_return + + if execute_return: + svc.execute_correction.return_value = execute_return + + return svc + + +@given( + "the uncov-rgn correction service returns a revert result with reverted decisions" +) +def step_uncov_rgn_correction_revert(context: Context) -> None: + """Mock CorrectionService for a successful revert execution.""" + request = SimpleNamespace( + correction_id="CORR-REVERT-01", + mode=SimpleNamespace(value="revert"), + target_decision_id="DEC-001", + guidance="Use Flask instead", + ) + result = SimpleNamespace( + correction_id="CORR-REVERT-01", + status=SimpleNamespace(value="applied"), + new_decisions=[], + reverted_decisions=["DEC-001", "DEC-002"], + ) + context.uncov_mock_correction = _make_mock_correction_svc( + request_return=request, + execute_return=result, + ) + + +@given("the uncov-rgn correction service returns an append result with new decisions") +def step_uncov_rgn_correction_append(context: Context) -> None: + """Mock CorrectionService for a successful append execution.""" + request = SimpleNamespace( + correction_id="CORR-APPEND-01", + mode=SimpleNamespace(value="append"), + target_decision_id="DEC-003", + guidance="Add caching layer", + ) + result = SimpleNamespace( + correction_id="CORR-APPEND-01", + status=SimpleNamespace(value="applied"), + new_decisions=["DEC-004", "DEC-005"], + reverted_decisions=[], + ) + context.uncov_mock_correction = _make_mock_correction_svc( + request_return=request, + execute_return=result, + ) + + +@given("the uncov-rgn correction service returns an impact analysis") +def step_uncov_rgn_correction_impact(context: Context) -> None: + """Mock CorrectionService for dry-run impact analysis.""" + request = SimpleNamespace( + correction_id="CORR-DRY-01", + mode=SimpleNamespace(value="revert"), + target_decision_id="DEC-010", + guidance="Preview changes", + ) + impact = SimpleNamespace( + affected_decisions=["DEC-010", "DEC-011"], + affected_files=["main.py", "utils.py"], + estimated_cost=3.0, + risk_level="medium", + ) + context.uncov_mock_correction = _make_mock_correction_svc( + request_return=request, + impact_return=impact, + ) + + +@given("the uncov-rgn correction service request raises ResourceNotFoundError") +def step_uncov_rgn_correction_rnf(context: Context) -> None: + """Mock CorrectionService to raise ResourceNotFoundError.""" + context.uncov_mock_correction = _make_mock_correction_svc( + request_side_effect=ResourceNotFoundError("decision DEC-999 not found"), + ) + + +@given("the uncov-rgn correction service request raises ValidationError") +def step_uncov_rgn_correction_val_error(context: Context) -> None: + """Mock CorrectionService to raise ValidationError.""" + context.uncov_mock_correction = _make_mock_correction_svc( + request_side_effect=ValidationError("plan_id must not be empty"), + ) + + +@given("the uncov-rgn correction service request raises CleverAgentsError") +def step_uncov_rgn_correction_ca_error(context: Context) -> None: + """Mock CorrectionService to raise CleverAgentsError.""" + context.uncov_mock_correction = _make_mock_correction_svc( + request_side_effect=CleverAgentsError("correction service unavailable"), + ) + + +# --------------------------------------------------------------------------- +# When — lifecycle_list_plans +# --------------------------------------------------------------------------- + + +@when('I uncov-rgn invoke lifecycle-list with action filter "{action}"') +def step_uncov_rgn_invoke_list_action(context: Context, action: str) -> None: + """Invoke lifecycle-list with an --action filter.""" + context.uncov_result = context.uncov_runner.invoke( + plan_app, + ["lifecycle-list", "--action", action], + ) + + +@when('I uncov-rgn invoke lifecycle-list with format "{fmt}"') +def step_uncov_rgn_invoke_list_fmt(context: Context, fmt: str) -> None: + """Invoke lifecycle-list with a given --format.""" + context.uncov_result = context.uncov_runner.invoke( + plan_app, + ["lifecycle-list", "--format", fmt], + ) + + +# --------------------------------------------------------------------------- +# When — revert_plan +# --------------------------------------------------------------------------- + + +@when('I uncov-rgn invoke revert for plan "{pid}"') +def step_uncov_rgn_invoke_revert(context: Context, pid: str) -> None: + """Invoke the revert command for a given plan ID.""" + context.uncov_result = context.uncov_runner.invoke( + plan_app, + ["revert", pid], + ) + + +# --------------------------------------------------------------------------- +# When — _get_apply_service +# --------------------------------------------------------------------------- + + +@when("I uncov-rgn call _get_apply_service directly") +def step_uncov_rgn_call_get_apply(context: Context) -> None: + """Invoke _get_apply_service and capture the result.""" + from cleveragents.cli.commands.plan import _get_apply_service + + mock_pas_cls = MagicMock() + mock_pas_instance = MagicMock() + mock_pas_cls.return_value = mock_pas_instance + + with ( + patch( + _PATCH_GET_LIFECYCLE, + return_value=context.uncov_mock_lifecycle, + ), + patch( + "cleveragents.cli.commands.plan.PlanApplyService", + mock_pas_cls, + create=True, + ), + patch( + "cleveragents.application.services.plan_apply_service.PlanApplyService", + mock_pas_cls, + ), + ): + context.uncov_apply_result = _get_apply_service() + + +# --------------------------------------------------------------------------- +# When — plan_diff +# --------------------------------------------------------------------------- + + +@when('I uncov-rgn invoke diff for plan "{pid}" with correction "{corr}"') +def step_uncov_rgn_invoke_diff_correction( + context: Context, pid: str, corr: str +) -> None: + """Invoke the diff command with --correction flag.""" + context.uncov_result = context.uncov_runner.invoke( + plan_app, + ["diff", pid, "--correction", corr], + ) + + +@when('I uncov-rgn invoke diff for plan "{pid}" without correction') +def step_uncov_rgn_invoke_diff_no_correction(context: Context, pid: str) -> None: + """Invoke the diff command without --correction flag.""" + with patch(_PATCH_GET_APPLY, return_value=context.uncov_mock_apply): + context.uncov_result = context.uncov_runner.invoke( + plan_app, + ["diff", pid], + ) + + +# --------------------------------------------------------------------------- +# When — plan_artifacts +# --------------------------------------------------------------------------- + + +@when('I uncov-rgn invoke artifacts for plan "{pid}" in rich format') +def step_uncov_rgn_invoke_artifacts_rich(context: Context, pid: str) -> None: + """Invoke the artifacts command in default rich format.""" + with patch(_PATCH_GET_APPLY, return_value=context.uncov_mock_apply): + context.uncov_result = context.uncov_runner.invoke( + plan_app, + ["artifacts", pid], + ) + + +@when('I uncov-rgn invoke artifacts for plan "{pid}" with format "{fmt}"') +def step_uncov_rgn_invoke_artifacts_fmt(context: Context, pid: str, fmt: str) -> None: + """Invoke the artifacts command with a given --format.""" + with patch(_PATCH_GET_APPLY, return_value=context.uncov_mock_apply): + context.uncov_result = context.uncov_runner.invoke( + plan_app, + ["artifacts", pid, "--format", fmt], + ) + + +# --------------------------------------------------------------------------- +# When — correct_decision +# --------------------------------------------------------------------------- + + +def _invoke_correct( + context: Context, + *, + mode: str = "revert", + guidance: str = "test guidance", + yes: bool = True, + dry_run: bool = False, + fmt: str = "rich", + plan_id: str = "PLAN-ACTIVE-01", + decision_id: str = "DEC-001", +) -> None: + """Helper to invoke the correct command with patches.""" + args = ["correct", decision_id, "--mode", mode, "--guidance", guidance] + if yes: + args.append("--yes") + if dry_run: + args.append("--dry-run") + if fmt != "rich": + args.extend(["--format", fmt]) + args.extend(["--plan", plan_id]) + + mock_correction = getattr(context, "uncov_mock_correction", MagicMock()) + + # Patch CorrectionService to return our mock when instantiated + with patch( + "cleveragents.application.services.correction_service.CorrectionService", + return_value=mock_correction, + ): + context.uncov_result = context.uncov_runner.invoke(plan_app, args) + + +@when('I uncov-rgn invoke correct in revert mode with --yes and guidance "{guidance}"') +def step_uncov_rgn_correct_revert_yes(context: Context, guidance: str) -> None: + """Invoke correct with revert mode, --yes, and given guidance.""" + _invoke_correct(context, mode="revert", guidance=guidance, yes=True) + + +@when('I uncov-rgn invoke correct in append mode with --yes and guidance "{guidance}"') +def step_uncov_rgn_correct_append_yes(context: Context, guidance: str) -> None: + """Invoke correct with append mode, --yes, and given guidance.""" + _invoke_correct(context, mode="append", guidance=guidance, yes=True) + + +@when( + 'I uncov-rgn invoke correct in revert mode with --yes, format "{fmt}", ' + 'and guidance "{guidance}"' +) +def step_uncov_rgn_correct_revert_fmt( + context: Context, fmt: str, guidance: str +) -> None: + """Invoke correct with revert mode, --yes, custom format, and given guidance.""" + _invoke_correct(context, mode="revert", guidance=guidance, yes=True, fmt=fmt) + + +@when( + "I uncov-rgn invoke correct in dry-run mode with rich format " + 'and guidance "{guidance}"' +) +def step_uncov_rgn_correct_dryrun_rich(context: Context, guidance: str) -> None: + """Invoke correct with dry-run flag in rich format.""" + _invoke_correct( + context, mode="revert", guidance=guidance, yes=True, dry_run=True, fmt="rich" + ) + + +@when( + 'I uncov-rgn invoke correct in dry-run mode with format "{fmt}" ' + 'and guidance "{guidance}"' +) +def step_uncov_rgn_correct_dryrun_fmt( + context: Context, fmt: str, guidance: str +) -> None: + """Invoke correct with dry-run flag in custom format.""" + _invoke_correct( + context, mode="revert", guidance=guidance, yes=True, dry_run=True, fmt=fmt + ) + + +@when('I uncov-rgn invoke correct with invalid mode "{mode}" and guidance "{guidance}"') +def step_uncov_rgn_correct_invalid_mode( + context: Context, mode: str, guidance: str +) -> None: + """Invoke correct with an invalid mode.""" + _invoke_correct(context, mode=mode, guidance=guidance, yes=True) + + +@when('I uncov-rgn invoke correct with valid mode "{mode}" and empty guidance') +def step_uncov_rgn_correct_empty_guidance(context: Context, mode: str) -> None: + """Invoke correct with empty guidance string.""" + _invoke_correct(context, mode=mode, guidance="", yes=True) + + +# --------------------------------------------------------------------------- +# Then — assertions +# --------------------------------------------------------------------------- + + +@then("the uncov-rgn command should exit normally") +def step_uncov_rgn_exit_ok(context: Context) -> None: + """Assert the CLI exited with code 0.""" + assert context.uncov_result.exit_code == 0, ( + f"Expected exit_code=0, got {context.uncov_result.exit_code}. " + f"Output: {context.uncov_result.output}" + ) + + +@then("the uncov-rgn command should abort") +def step_uncov_rgn_abort(context: Context) -> None: + """Assert the CLI exited with a non-zero code (abort).""" + assert context.uncov_result.exit_code != 0, ( + f"Expected non-zero exit_code, got {context.uncov_result.exit_code}. " + f"Output: {context.uncov_result.output}" + ) + + +@then('the uncov-rgn output should contain "{text}"') +def step_uncov_rgn_output_contains(context: Context, text: str) -> None: + """Assert the CLI output contains the expected text.""" + output = context.uncov_result.output + assert text in output, ( + f"Expected output to contain '{text}'. Actual output:\n{output}" + ) + + +@then("the uncov-rgn apply service should be returned successfully") +def step_uncov_rgn_apply_ok(context: Context) -> None: + """Assert _get_apply_service returned a non-None object.""" + assert context.uncov_apply_result is not None, ( + "Expected _get_apply_service to return a value, got None" + ) diff --git a/features/steps/plan_executor_edge_cases_coverage_steps.py b/features/steps/plan_executor_edge_cases_coverage_steps.py new file mode 100644 index 000000000..15fef8b28 --- /dev/null +++ b/features/steps/plan_executor_edge_cases_coverage_steps.py @@ -0,0 +1,569 @@ +"""Step definitions for plan_executor_edge_cases_coverage.feature. + +Targets uncovered lines and branches in plan_executor.py: + - Line 378 / branch 377→378: _try_rollback_to_last_checkpoint with non-empty checkpoints + - Lines 410-411: _resolve_sandbox_for_checkpoint → _SandboxRootProxy fallback + - Lines 161-163 / branches 160→161, 162→163: _parse_steps non-empty path + - Lines 417-419: run_strategize function definition + +All step names use the 'edge3' prefix to avoid collisions with existing step files. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.application.services.plan_executor import ( + ExecuteResult, + PlanExecutor, +) +from cleveragents.domain.models.core.plan import ( + PlanPhase, + PlanTimestamps, + ProcessingState, +) +from cleveragents.infrastructure.sandbox.checkpoint import SandboxCheckpoint + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +EDGE3_PLAN_ID = "01KEDGE3PLANID00000000PLN" +EDGE3_ROOT_ID = "01KEDGE3ROOTID00000000RTD" +EDGE3_SANDBOX_ROOT = "/tmp/edge3-sandbox" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _edge3_make_plan( + *, + phase: PlanPhase = PlanPhase.EXECUTE, + state: ProcessingState = ProcessingState.QUEUED, + definition_of_done: str | None = "Implement feature\nWrite tests", + decision_root_id: str | None = EDGE3_ROOT_ID, +) -> MagicMock: + """Build a mock plan object with sensible defaults.""" + plan = MagicMock() + plan.phase = phase + plan.state = state + plan.definition_of_done = definition_of_done + plan.decision_root_id = decision_root_id + plan.invariants = [] + plan.timestamps = PlanTimestamps() + plan.changeset_id = None + plan.sandbox_refs = [] + plan.error_details = None + plan.read_only = False + return plan + + +def _edge3_make_lifecycle(plan: Any | None = None) -> MagicMock: + """Build a mock lifecycle service.""" + 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 + + +def _edge3_make_checkpoint_mock() -> SandboxCheckpoint: + """Create a mock SandboxCheckpoint.""" + from datetime import UTC, datetime + + return SandboxCheckpoint( + checkpoint_id="01KEDGE3CPID0000000000CP", + sandbox_id="root-EDGE3PLAN", + plan_id="EDGE3PLAN", + phase="pre_execute", + created_at=datetime.now(tz=UTC), + metadata={}, + snapshot_path="/tmp/edge3-snapshot", + ) + + +# --------------------------------------------------------------------------- +# _resolve_sandbox_for_checkpoint → _SandboxRootProxy path +# --------------------------------------------------------------------------- + + +@given( + "an edge3 PlanExecutor with checkpoint manager and sandbox root but no execution context" +) +def step_edge3_given_executor_sandbox_root_no_ctx(context: Context) -> None: + """Create PlanExecutor with checkpoint_manager and sandbox_root, no exec context.""" + context.edge3_checkpoint_mgr = MagicMock() + context.edge3_executor = PlanExecutor( + lifecycle_service=_edge3_make_lifecycle(), + sandbox_root=EDGE3_SANDBOX_ROOT, + execution_context=None, + checkpoint_manager=context.edge3_checkpoint_mgr, + ) + + +@given( + "an edge3 PlanExecutor with checkpoint manager and sandbox root and context without sandbox manager" +) +def step_edge3_given_executor_ctx_no_sbmgr(context: Context) -> None: + """Create PlanExecutor with exec context that has no sandbox_manager attr.""" + context.edge3_checkpoint_mgr = MagicMock() + # Execution context without sandbox_manager attribute + exec_ctx = MagicMock(spec=[]) # empty spec → no attributes + context.edge3_executor = PlanExecutor( + lifecycle_service=_edge3_make_lifecycle(), + sandbox_root=EDGE3_SANDBOX_ROOT, + execution_context=exec_ctx, + checkpoint_manager=context.edge3_checkpoint_mgr, + ) + + +@given( + "an edge3 PlanExecutor with checkpoint manager and sandbox root and context with empty sandboxes" +) +def step_edge3_given_executor_ctx_empty_sandboxes(context: Context) -> None: + """Create PlanExecutor with exec context whose sandbox_manager returns [].""" + context.edge3_checkpoint_mgr = MagicMock() + exec_ctx = MagicMock() + exec_ctx.sandbox_manager = MagicMock() + exec_ctx.sandbox_manager.list_sandboxes.return_value = [] + context.edge3_executor = PlanExecutor( + lifecycle_service=_edge3_make_lifecycle(), + sandbox_root=EDGE3_SANDBOX_ROOT, + execution_context=exec_ctx, + checkpoint_manager=context.edge3_checkpoint_mgr, + ) + + +@given( + "an edge3 PlanExecutor with checkpoint manager but no sandbox root and no execution context" +) +def step_edge3_given_executor_no_sandbox_no_ctx(context: Context) -> None: + """Create PlanExecutor with checkpoint_manager but no sandbox_root and no exec context.""" + context.edge3_checkpoint_mgr = MagicMock() + context.edge3_executor = PlanExecutor( + lifecycle_service=_edge3_make_lifecycle(), + sandbox_root=None, + execution_context=None, + checkpoint_manager=context.edge3_checkpoint_mgr, + ) + + +@when('I edge3 resolve sandbox for checkpoint with plan id "{plan_id}"') +def step_edge3_resolve_sandbox(context: Context, plan_id: str) -> None: + """Call _resolve_sandbox_for_checkpoint directly.""" + context.edge3_resolved_sandbox = ( + context.edge3_executor._resolve_sandbox_for_checkpoint(plan_id) + ) + + +@then("the edge3 resolved sandbox should not be None") +def step_edge3_resolved_not_none(context: Context) -> None: + """Verify resolved sandbox is not None.""" + assert context.edge3_resolved_sandbox is not None, ( + "Expected a sandbox-like object but got None" + ) + + +@then("the edge3 resolved sandbox should have a synthetic sandbox id") +def step_edge3_resolved_has_sandbox_id(context: Context) -> None: + """Verify the resolved sandbox has a sandbox_id starting with 'root-'.""" + sid = context.edge3_resolved_sandbox.sandbox_id + assert sid.startswith("root-"), ( + f"Expected sandbox_id starting with 'root-', got '{sid}'" + ) + + +@then("the edge3 resolved sandbox context should have the sandbox path") +def step_edge3_resolved_has_sandbox_path(context: Context) -> None: + """Verify the resolved sandbox context has the correct sandbox_path.""" + ctx = context.edge3_resolved_sandbox.context + assert ctx is not None, "Expected context to be non-None" + assert ctx.sandbox_path == EDGE3_SANDBOX_ROOT, ( + f"Expected sandbox_path='{EDGE3_SANDBOX_ROOT}', got '{ctx.sandbox_path}'" + ) + + +@then("the edge3 resolved sandbox should be None") +def step_edge3_resolved_is_none(context: Context) -> None: + """Verify resolved sandbox is None.""" + assert context.edge3_resolved_sandbox is None, ( + f"Expected None but got {context.edge3_resolved_sandbox}" + ) + + +# --------------------------------------------------------------------------- +# _try_rollback_to_last_checkpoint with non-empty checkpoints +# --------------------------------------------------------------------------- + + +@given( + "the edge3 checkpoint manager has existing checkpoints that rollback successfully" +) +def step_edge3_given_checkpoints_rollback_ok(context: Context) -> None: + """Configure checkpoint_manager to return checkpoints and succeed on rollback.""" + cp = _edge3_make_checkpoint_mock() + context.edge3_checkpoint_mgr.list_checkpoints.return_value = [cp] + context.edge3_checkpoint_mgr.rollback_to.return_value = True + + +@given("the edge3 checkpoint manager has checkpoints but rollback raises an exception") +def step_edge3_given_checkpoints_rollback_raises(context: Context) -> None: + """Configure checkpoint_manager with checkpoints but rollback_to raises.""" + cp = _edge3_make_checkpoint_mock() + context.edge3_checkpoint_mgr.list_checkpoints.return_value = [cp] + context.edge3_checkpoint_mgr.rollback_to.side_effect = RuntimeError( + "edge3 rollback boom" + ) + + +@given("the edge3 checkpoint manager returns no checkpoints") +def step_edge3_given_no_checkpoints(context: Context) -> None: + """Configure checkpoint_manager to return empty checkpoints list.""" + context.edge3_checkpoint_mgr.list_checkpoints.return_value = [] + + +@when('I edge3 try rollback to last checkpoint for plan "{plan_id}"') +def step_edge3_try_rollback(context: Context, plan_id: str) -> None: + """Call _try_rollback_to_last_checkpoint directly.""" + context.edge3_rollback_result = ( + context.edge3_executor._try_rollback_to_last_checkpoint(plan_id) + ) + + +@then("the edge3 rollback result should be True") +def step_edge3_rollback_true(context: Context) -> None: + """Verify rollback returned True.""" + assert context.edge3_rollback_result is True, ( + f"Expected True, got {context.edge3_rollback_result}" + ) + + +@then("the edge3 rollback result should be False") +def step_edge3_rollback_false(context: Context) -> None: + """Verify rollback returned False.""" + assert context.edge3_rollback_result is False, ( + f"Expected False, got {context.edge3_rollback_result}" + ) + + +# --------------------------------------------------------------------------- +# _try_create_checkpoint via _SandboxRootProxy +# --------------------------------------------------------------------------- + + +@given("the edge3 checkpoint manager accepts checkpoint creation") +def step_edge3_given_checkpoint_create_ok(context: Context) -> None: + """Configure checkpoint_manager.create_checkpoint to return a checkpoint.""" + cp = _edge3_make_checkpoint_mock() + context.edge3_checkpoint_mgr.create_checkpoint.return_value = cp + + +@given("the edge3 checkpoint manager raises on create_checkpoint") +def step_edge3_given_checkpoint_create_fails(context: Context) -> None: + """Configure checkpoint_manager.create_checkpoint to raise.""" + context.edge3_checkpoint_mgr.create_checkpoint.side_effect = RuntimeError( + "edge3 create boom" + ) + + +@when('I edge3 try create checkpoint for plan "{plan_id}" with phase "{phase}"') +def step_edge3_try_create_checkpoint( + context: Context, plan_id: str, phase: str +) -> None: + """Call _try_create_checkpoint directly.""" + context.edge3_checkpoint_result = context.edge3_executor._try_create_checkpoint( + plan_id, phase + ) + + +@then("the edge3 checkpoint result should not be None") +def step_edge3_checkpoint_not_none(context: Context) -> None: + """Verify checkpoint creation returned a checkpoint.""" + assert context.edge3_checkpoint_result is not None, ( + "Expected a SandboxCheckpoint but got None" + ) + + +@then("the edge3 checkpoint result should be None") +def step_edge3_checkpoint_is_none(context: Context) -> None: + """Verify checkpoint creation returned None (non-fatal failure).""" + assert context.edge3_checkpoint_result is None, ( + f"Expected None but got {context.edge3_checkpoint_result}" + ) + + +@then("the edge3 checkpoint manager should have been called with sandbox path metadata") +def step_edge3_checkpoint_called_with_path(context: Context) -> None: + """Verify create_checkpoint was called with sandbox_path in metadata.""" + context.edge3_checkpoint_mgr.create_checkpoint.assert_called_once() + call_kwargs = context.edge3_checkpoint_mgr.create_checkpoint.call_args + # Keyword arg 'metadata' or positional + metadata = call_kwargs.kwargs.get("metadata") or call_kwargs[1].get("metadata", {}) + assert "sandbox_path" in metadata, ( + f"Expected 'sandbox_path' in metadata, got keys: {list(metadata.keys())}" + ) + assert metadata["sandbox_path"] == EDGE3_SANDBOX_ROOT, ( + f"Expected sandbox_path='{EDGE3_SANDBOX_ROOT}', got '{metadata['sandbox_path']}'" + ) + + +# --------------------------------------------------------------------------- +# Stub execute with checkpoint rollback on failure +# --------------------------------------------------------------------------- + + +@given("an edge3 mock lifecycle service for execute") +def step_edge3_given_lifecycle_execute(context: Context) -> None: + """Create a mock lifecycle service for execute scenarios.""" + context.edge3_lifecycle = _edge3_make_lifecycle() + context.edge3_plan_id = EDGE3_PLAN_ID + + +@given("an edge3 plan in Execute-Queued state with decision root") +def step_edge3_given_plan_execute_queued(context: Context) -> None: + """Set up a plan in Execute-Queued state with decision root.""" + plan = _edge3_make_plan( + phase=PlanPhase.EXECUTE, + state=ProcessingState.QUEUED, + definition_of_done="Implement feature\nWrite tests", + decision_root_id=EDGE3_ROOT_ID, + ) + context.edge3_lifecycle.get_plan.return_value = plan + context.edge3_mock_plan = plan + + +@given( + "an edge3 PlanExecutor with checkpoint manager sandbox root and failing execute actor" +) +def step_edge3_given_executor_failing_stub_with_cp(context: Context) -> None: + """Create PlanExecutor with checkpoint manager, sandbox root, and a failing execute actor.""" + context.edge3_checkpoint_mgr = MagicMock() + context.edge3_executor = PlanExecutor( + lifecycle_service=context.edge3_lifecycle, + sandbox_root=EDGE3_SANDBOX_ROOT, + execution_context=None, + checkpoint_manager=context.edge3_checkpoint_mgr, + ) + # Make execute actor always fail + context.edge3_executor._execute_actor = MagicMock() + context.edge3_executor._execute_actor.execute.side_effect = RuntimeError( + "edge3 stub execute boom" + ) + + +@when("I edge3 call run execute expecting failure") +def step_edge3_run_execute_fail(context: Context) -> None: + """Call run_execute expecting an exception.""" + context.edge3_raised = None + try: + context.edge3_executor.run_execute(context.edge3_plan_id) + except Exception as exc: + context.edge3_raised = exc + + +@then("an edge3 exception should have been raised") +def step_edge3_exception_raised(context: Context) -> None: + """Verify an exception was raised.""" + assert context.edge3_raised is not None, "Expected an exception but none was raised" + + +@then("the edge3 checkpoint manager rollback_to should have been called") +def step_edge3_rollback_called(context: Context) -> None: + """Verify checkpoint_manager.rollback_to was called.""" + context.edge3_checkpoint_mgr.rollback_to.assert_called_once() + + +@then("the edge3 lifecycle should have called fail_execute for edge3") +def step_edge3_check_fail_execute(context: Context) -> None: + """Verify lifecycle.fail_execute was called.""" + context.edge3_lifecycle.fail_execute.assert_called_once() + call_args = context.edge3_lifecycle.fail_execute.call_args[0] + assert context.edge3_plan_id in call_args, ( + f"Expected plan_id in fail_execute args, got {call_args}" + ) + + +# --------------------------------------------------------------------------- +# Runtime execute with checkpoint rollback on failure +# --------------------------------------------------------------------------- + + +@given("an edge3 mock execution context for runtime") +def step_edge3_given_exec_ctx_runtime(context: Context) -> None: + """Create a mock execution context for runtime mode.""" + from cleveragents.application.services.plan_execution_context import ( + PlanExecutionContext, + ) + + context.edge3_exec_ctx = PlanExecutionContext(plan_id=EDGE3_PLAN_ID) + + +@given( + "an edge3 PlanExecutor with runtime context checkpoint manager sandbox root and failing runtime actor" +) +def step_edge3_given_executor_runtime_failing_with_cp(context: Context) -> None: + """Create PlanExecutor with runtime context, checkpoint manager, and failing runtime.""" + from cleveragents.application.services.plan_execution_context import ( + RuntimeExecuteActor, + ) + from cleveragents.tool.registry import ToolRegistry + from cleveragents.tool.runner import ToolRunner + + context.edge3_checkpoint_mgr = MagicMock() + runner = ToolRunner(registry=ToolRegistry()) + context.edge3_executor = PlanExecutor( + lifecycle_service=context.edge3_lifecycle, + tool_runner=runner, + sandbox_root=EDGE3_SANDBOX_ROOT, + execution_context=context.edge3_exec_ctx, + checkpoint_manager=context.edge3_checkpoint_mgr, + ) + # Patch RuntimeExecuteActor.execute to raise + patcher = patch.object( + RuntimeExecuteActor, + "execute", + side_effect=RuntimeError("edge3 runtime execute boom"), + ) + patcher.start() + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(patcher.stop) + + +# --------------------------------------------------------------------------- +# _parse_steps non-empty path exercised via run_strategize +# --------------------------------------------------------------------------- + + +@given("an edge3 mock lifecycle service for strategize") +def step_edge3_given_lifecycle_strategize(context: Context) -> None: + """Create a mock lifecycle service for strategize scenarios.""" + context.edge3_lifecycle = _edge3_make_lifecycle() + context.edge3_plan_id = EDGE3_PLAN_ID + + +@given('an edge3 plan in Strategize phase with multi-line definition "{defn}"') +def step_edge3_given_plan_strategize_multiline(context: Context, defn: str) -> None: + """Set up a plan in Strategize phase with a multi-line definition.""" + raw = defn.replace("\\n", "\n") + plan = _edge3_make_plan( + phase=PlanPhase.STRATEGIZE, + state=ProcessingState.QUEUED, + definition_of_done=raw, + decision_root_id=None, + ) + context.edge3_lifecycle.get_plan.return_value = plan + context.edge3_mock_plan = plan + + +@given("an edge3 plan in Strategize phase with empty definition") +def step_edge3_given_plan_strategize_empty_defn(context: Context) -> None: + """Set up a plan in Strategize phase with empty definition_of_done.""" + plan = _edge3_make_plan( + phase=PlanPhase.STRATEGIZE, + state=ProcessingState.QUEUED, + definition_of_done="", + decision_root_id=None, + ) + context.edge3_lifecycle.get_plan.return_value = plan + context.edge3_mock_plan = plan + + +@given("an edge3 PlanExecutor for strategize without execution context") +def step_edge3_given_executor_strategize_no_ctx(context: Context) -> None: + """Create a PlanExecutor for strategize without execution context.""" + context.edge3_executor = PlanExecutor( + lifecycle_service=context.edge3_lifecycle, + execution_context=None, + ) + + +@when("I edge3 call run strategize successfully") +def step_edge3_run_strategize(context: Context) -> None: + """Call run_strategize and store the result.""" + context.edge3_raised = None + try: + context.edge3_strat_result = context.edge3_executor.run_strategize( + context.edge3_plan_id + ) + except Exception as exc: + context.edge3_raised = exc + + +@then("the edge3 strategize result should have {n:d} decisions") +def step_edge3_check_decision_count(context: Context, n: int) -> None: + """Verify the strategize result decision count.""" + assert context.edge3_raised is None, f"Unexpected error: {context.edge3_raised}" + actual = len(context.edge3_strat_result.decisions) + assert actual == n, f"Expected {n} decisions, got {actual}" + + +@then("the edge3 lifecycle should have called complete_strategize for edge3") +def step_edge3_check_complete_strategize(context: Context) -> None: + """Verify complete_strategize was called.""" + context.edge3_lifecycle.complete_strategize.assert_called_once_with( + context.edge3_plan_id + ) + + +# --------------------------------------------------------------------------- +# Successful stub execute with checkpoint creation via sandbox root proxy +# --------------------------------------------------------------------------- + + +@given( + "an edge3 PlanExecutor with checkpoint manager and sandbox root for stub execute" +) +def step_edge3_given_executor_cp_sandbox_stub(context: Context) -> None: + """Create PlanExecutor with checkpoint manager and sandbox root for stub execute.""" + context.edge3_checkpoint_mgr = MagicMock() + context.edge3_executor = PlanExecutor( + lifecycle_service=context.edge3_lifecycle, + sandbox_root=EDGE3_SANDBOX_ROOT, + execution_context=None, + checkpoint_manager=context.edge3_checkpoint_mgr, + ) + + +@when("I edge3 call run execute successfully") +def step_edge3_run_execute_success(context: Context) -> None: + """Call run_execute expecting success.""" + context.edge3_raised = None + try: + context.edge3_exec_result = context.edge3_executor.run_execute( + context.edge3_plan_id + ) + except Exception as exc: + context.edge3_raised = exc + + +@then("the edge3 execute result should be an ExecuteResult") +def step_edge3_check_exec_result_type(context: Context) -> None: + """Verify the result is an ExecuteResult.""" + assert context.edge3_raised is None, f"Unexpected error: {context.edge3_raised}" + assert isinstance(context.edge3_exec_result, ExecuteResult), ( + f"Expected ExecuteResult, got {type(context.edge3_exec_result).__name__}" + ) + + +@then( + "the edge3 checkpoint manager create_checkpoint should have been called at least twice" +) +def step_edge3_checkpoint_create_called_twice(context: Context) -> None: + """Verify create_checkpoint was called at least twice (pre and post execute).""" + call_count = context.edge3_checkpoint_mgr.create_checkpoint.call_count + assert call_count >= 2, ( + f"Expected create_checkpoint called >= 2 times, got {call_count}" + ) diff --git a/features/steps/plan_lifecycle_coverage_steps.py b/features/steps/plan_lifecycle_coverage_steps.py index ba50807b7..f1ab4a621 100644 --- a/features/steps/plan_lifecycle_coverage_steps.py +++ b/features/steps/plan_lifecycle_coverage_steps.py @@ -373,7 +373,7 @@ def step_delete_nonexistent(context: Context, plan_id: str) -> None: session.close() -@then("the delete result should be false") +@then("the plan delete result should be false") def step_verify_delete_false(context: Context) -> None: assert context.cov_result is False, f"Expected False, got {context.cov_result}" diff --git a/features/steps/repositories_remaining_branches_coverage_steps.py b/features/steps/repositories_remaining_branches_coverage_steps.py new file mode 100644 index 000000000..0a2d901a0 --- /dev/null +++ b/features/steps/repositories_remaining_branches_coverage_steps.py @@ -0,0 +1,809 @@ +"""Step definitions for repositories_remaining_branches_coverage.feature. + +Targets the last uncovered lines and branches in repositories.py: +- ToolRepository.add: DuplicateToolError re-raise (L3541-3542), + DatabaseError wrapping (L3542) +- ToolRepository.get_by_name: success path (L3558) +- ResourceRepository.link_child: parent_type_row absent (branch 2298->2310) +- ResourceRepository.resolve_namespaced_name: ULID fallback (branch 2711->2714) +- ResourceRepository.get_children: populated list (branch 2426->2420) +- ResourceRepository.get_parents: populated list (branch 2459->2453) +- ResourceRepository._get_ancestors + _build_cycle_path (branches 2635->2633, 2661->2659) +- NamespacedProjectRepository.update: success path (branch 2928->2931) +- AutomationProfileRepository.upsert: new insert (branch 4193->4200) +- ValidationAttachmentRepository.attach: project_name scope (branch 3633->3634) +- LifecyclePlanRepository.update: multiple project links (branch 1395->1394) +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from types import SimpleNamespace +from typing import Any + +from behave import given, then, when +from behave.runner import Context +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from cleveragents.infrastructure.database.models import ( + Base, + ResourceLinkModel, + ResourceModel, + ResourceTypeModel, +) +from cleveragents.infrastructure.database.repositories import ( + CycleDetectedError, + DuplicateToolError, + NamespacedProjectRepository, + ResourceRepository, + ToolRepository, + ValidationAttachmentRepository, +) + +# ── helpers ───────────────────────────────────────────────────────────── + + +def _make_engine_and_factory(): + """Create a fresh in-memory SQLite DB with all tables.""" + engine = create_engine("sqlite:///:memory:") + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine) + return engine, factory + + +def _now_iso() -> str: + return datetime.now(tz=UTC).isoformat() + + +def _make_tool_ns( + name: str = "local/test-tool", tool_type: str = "tool" +) -> SimpleNamespace: + """Create a tool domain-like SimpleNamespace suitable for ToolRepository.add.""" + return SimpleNamespace( + name=name, + description=f"Tool {name}", + tool_type=tool_type, + source="builtin", + input_schema=None, + output_schema=None, + capability=None, + resource_slots=None, + lifecycle_json=None, + code=None, + mcp_server=None, + mcp_tool_name=None, + agent_skill_path=None, + timeout=300, + wraps=None, + transform=None, + mode=None, + argument_mapping_json=None, + ) + + +def _ensure_resource_type_row(factory, type_name: str) -> None: + """Insert a ResourceTypeModel row directly if absent.""" + session = factory() + existing = session.query(ResourceTypeModel).filter_by(name=type_name).first() + if existing: + return + now = _now_iso() + ns = type_name.split("/")[0] if "/" in type_name else "builtin" + row = ResourceTypeModel( + name=type_name, + namespace=ns, + description="test type", + resource_kind="physical", + sandbox_strategy="none", + user_addable=True, + handler_ref=None, + args_schema_json=None, + allowed_parent_types_json=None, + allowed_child_types_json=None, + auto_discover_json=None, + capabilities_json='{"read": true, "write": true, "sandbox": true, "checkpoint": false}', + equivalence_json=None, + source=None, + created_at=now, + updated_at=now, + ) + session.add(row) + session.commit() + + +def _create_resource_row( + factory, + resource_id: str, + type_name: str = "test/default-type", + name: str | None = None, + namespace: str | None = None, +) -> str: + """Insert a ResourceModel row directly and return the resource_id.""" + session = factory() + now = _now_iso() + row = ResourceModel( + resource_id=resource_id, + namespaced_name=name, + namespace=namespace, + type_name=type_name, + resource_kind="physical", + location=None, + description="test resource", + read_only=False, + auto_discovered=False, + sandbox_strategy=None, + content_hash=None, + properties_json=None, + metadata_json=None, + created_at=now, + updated_at=now, + ) + session.add(row) + session.commit() + return resource_id + + +def _create_link(factory, parent_id: str, child_id: str) -> None: + """Insert a ResourceLinkModel row directly.""" + session = factory() + now = _now_iso() + session.add( + ResourceLinkModel(parent_id=parent_id, child_id=child_id, created_at=now) + ) + session.commit() + + +# ═══════════════════════════════════════════════════════════════════════ +# ToolRepository.add: DuplicateToolError re-raise (lines 3541-3542) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov an in-memory database with tool tables") +def step_remaining_cov_tool_db(context: Context) -> None: + engine, factory = _make_engine_and_factory() + context.rbc_engine = engine + context.rbc_session_factory = factory + context.rbc_tool_repo = ToolRepository(session_factory=factory) + context.rbc_error = None + + +@given('remaining cov a tool "{name}" has been added') +def step_remaining_cov_tool_added(context: Context, name: str) -> None: + context.rbc_tool_repo.add(_make_tool_ns(name)) + + +@when('remaining cov the same tool "{name}" is added again') +def step_remaining_cov_tool_add_dup(context: Context, name: str) -> None: + # Mock create() to raise DuplicateToolError directly. + # This is necessary because the real create() is wrapped with + # @database_retry, which retries DatabaseError subclasses (including + # DuplicateToolError). By mocking create(), we bypass the retry + # decorator and directly exercise the ``except DuplicateToolError: raise`` + # path in add() (lines 3540-3541). + from unittest.mock import patch + + dup_error = DuplicateToolError(name) + with patch.object(context.rbc_tool_repo, "create", side_effect=dup_error): + try: + context.rbc_tool_repo.add(_make_tool_ns(name)) + context.rbc_error = None + except Exception as exc: + context.rbc_error = exc + + +@then("remaining cov a DuplicateToolError should be raised for the duplicate") +def step_remaining_cov_assert_dup_tool(context: Context) -> None: + assert isinstance(context.rbc_error, DuplicateToolError), ( + f"Expected DuplicateToolError, got {type(context.rbc_error).__name__}: " + f"{context.rbc_error}" + ) + + +# ═══════════════════════════════════════════════════════════════════════ +# ToolRepository.add: DatabaseError wrapping (line 3542) +# ═══════════════════════════════════════════════════════════════════════ + + +@given( + "remaining cov a ToolRepository with a session that raises DatabaseError on create" +) +def step_remaining_cov_tool_repo_db_error(context: Context) -> None: + from cleveragents.core.exceptions import DatabaseError as CoreDatabaseError + + _engine, factory = _make_engine_and_factory() + + # Create a subclass that overrides create() to raise a DatabaseError + # (not DuplicateToolError) to exercise line 3542-3543 + class _FailingToolRepo(ToolRepository): + def create(self, tool: Any) -> Any: + raise CoreDatabaseError("Simulated create failure") + + context.rbc_tool_repo = _FailingToolRepo(session_factory=factory) + context.rbc_error = None + + +@when("remaining cov a tool is added and a DatabaseError is expected") +def step_remaining_cov_tool_add_db_error(context: Context) -> None: + from cleveragents.core.exceptions import DatabaseError as CoreDatabaseError + + try: + context.rbc_tool_repo.add(_make_tool_ns("local/db-error-tool")) + context.rbc_error = None + except CoreDatabaseError as exc: + context.rbc_error = exc + except Exception as exc: + context.rbc_error = exc + + +@then('remaining cov a DatabaseError mentioning "Failed to add tool" should be raised') +def step_remaining_cov_assert_db_error(context: Context) -> None: + from cleveragents.core.exceptions import DatabaseError as CoreDatabaseError + + assert context.rbc_error is not None, "Expected a DatabaseError" + assert isinstance(context.rbc_error, CoreDatabaseError), ( + f"Expected DatabaseError, got {type(context.rbc_error).__name__}: " + f"{context.rbc_error}" + ) + assert "Failed to add tool" in str(context.rbc_error), ( + f"Expected 'Failed to add tool' in: {context.rbc_error}" + ) + + +# ═══════════════════════════════════════════════════════════════════════ +# ToolRepository.get_by_name: success returns domain (line 3558) +# ═══════════════════════════════════════════════════════════════════════ + + +@when('remaining cov get_by_name is invoked for "{name}"') +def step_remaining_cov_get_by_name(context: Context, name: str) -> None: + context.rbc_result = context.rbc_tool_repo.get_by_name(name) + + +@then('remaining cov get_by_name should return a non-None result with name "{name}"') +def step_remaining_cov_assert_get_by_name(context: Context, name: str) -> None: + result = context.rbc_result + assert result is not None, f"Expected non-None result for '{name}'" + # _to_legacy_domain returns a SimpleNamespace with .name + actual_name = result.name if hasattr(result, "name") else result.get("name", "") + assert actual_name == name, f"Expected name '{name}', got '{actual_name}'" + + +# ═══════════════════════════════════════════════════════════════════════ +# ResourceRepository.link_child: parent type row absent (branch 2298→2310) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov an in-memory resource database") +def step_remaining_cov_res_db(context: Context) -> None: + engine, factory = _make_engine_and_factory() + context.rbc_engine = engine + context.rbc_factory = factory + context.rbc_res_repo = ResourceRepository(factory) + context.rbc_error = None + + +@given("remaining cov two resources with a type that has no ResourceTypeModel row") +def step_remaining_cov_res_no_type_row(context: Context) -> None: + # Insert resources directly with a type_name whose ResourceTypeModel + # does NOT exist. This means parent_type_row will be None, exercising + # the branch where we skip allowed_children validation. + session = context.rbc_factory() + now = _now_iso() + for rid in ["res-notype-parent", "res-notype-child"]: + row = ResourceModel( + resource_id=rid, + namespaced_name=None, + namespace=None, + type_name="ghost-type", + resource_kind="physical", + location=None, + description="test", + read_only=False, + auto_discovered=False, + sandbox_strategy=None, + content_hash=None, + properties_json=None, + metadata_json=None, + created_at=now, + updated_at=now, + ) + session.add(row) + session.commit() + context.rbc_link_parent = "res-notype-parent" + context.rbc_link_child = "res-notype-child" + + +@when("remaining cov link_child is called for those resources") +def step_remaining_cov_link_child_no_type(context: Context) -> None: + try: + context.rbc_res_repo.link_child(context.rbc_link_parent, context.rbc_link_child) + context.rbc_error = None + except Exception as exc: + context.rbc_error = exc + + +@then("remaining cov the link should be created successfully") +def step_remaining_cov_assert_link_ok(context: Context) -> None: + assert context.rbc_error is None, f"Unexpected error: {context.rbc_error}" + session = context.rbc_factory() + link = ( + session.query(ResourceLinkModel) + .filter_by(parent_id=context.rbc_link_parent, child_id=context.rbc_link_child) + .first() + ) + assert link is not None, "Expected a ResourceLinkModel row" + + +# ═══════════════════════════════════════════════════════════════════════ +# ResourceRepository.resolve_namespaced_name: ULID fallback (branch 2711→2714) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov a resource exists with a known ULID but no namespaced name") +def step_remaining_cov_res_ulid_only(context: Context) -> None: + from ulid import ULID + + rid = str(ULID()) + _ensure_resource_type_row(context.rbc_factory, "test/ulid-type") + _create_resource_row( + context.rbc_factory, + rid, + type_name="test/ulid-type", + name=None, + ) + context.rbc_ulid_resource_id = rid + + +@when("remaining cov resolve_namespaced_name is called with the ULID") +def step_remaining_cov_resolve_ulid(context: Context) -> None: + context.rbc_result = context.rbc_res_repo.resolve_namespaced_name( + context.rbc_ulid_resource_id + ) + + +@then("remaining cov the resource should be resolved successfully") +def step_remaining_cov_assert_resolved(context: Context) -> None: + assert context.rbc_result is not None, "Expected resource, got None" + actual_id = ( + context.rbc_result.resource_id + if hasattr(context.rbc_result, "resource_id") + else context.rbc_result.get("resource_id", "") + ) + assert actual_id == context.rbc_ulid_resource_id, ( + f"Expected ID {context.rbc_ulid_resource_id}, got {actual_id}" + ) + + +@when('remaining cov resolve_namespaced_name is called with unknown ULID "{ulid}"') +def step_remaining_cov_resolve_unknown(context: Context, ulid: str) -> None: + context.rbc_result = context.rbc_res_repo.resolve_namespaced_name(ulid) + + +@then("remaining cov resolve_namespaced_name should return None") +def step_remaining_cov_assert_resolve_none(context: Context) -> None: + assert context.rbc_result is None, f"Expected None, got {context.rbc_result}" + + +# ═══════════════════════════════════════════════════════════════════════ +# ResourceRepository.get_children: populated list (branch 2426→2420) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov a parent resource linked to two child resources") +def step_remaining_cov_parent_with_children(context: Context) -> None: + from ulid import ULID + + _ensure_resource_type_row(context.rbc_factory, "test/link-type") + parent_id = str(ULID()) + child1_id = str(ULID()) + child2_id = str(ULID()) + for rid in [parent_id, child1_id, child2_id]: + _create_resource_row(context.rbc_factory, rid, type_name="test/link-type") + _create_link(context.rbc_factory, parent_id, child1_id) + _create_link(context.rbc_factory, parent_id, child2_id) + context.rbc_parent_id = parent_id + + +@when("remaining cov get_children is called on the parent resource") +def step_remaining_cov_get_children(context: Context) -> None: + context.rbc_children = context.rbc_res_repo.get_children(context.rbc_parent_id) + + +@then("remaining cov {n:d} child resources should be returned") +def step_remaining_cov_assert_children_count(context: Context, n: int) -> None: + assert len(context.rbc_children) == n, ( + f"Expected {n} children, got {len(context.rbc_children)}" + ) + + +# ═══════════════════════════════════════════════════════════════════════ +# ResourceRepository.get_parents: populated list (branch 2459→2453) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov a child resource linked from two parent resources") +def step_remaining_cov_child_with_parents(context: Context) -> None: + from ulid import ULID + + _ensure_resource_type_row(context.rbc_factory, "test/link-type") + child_id = str(ULID()) + parent1_id = str(ULID()) + parent2_id = str(ULID()) + for rid in [child_id, parent1_id, parent2_id]: + _create_resource_row(context.rbc_factory, rid, type_name="test/link-type") + _create_link(context.rbc_factory, parent1_id, child_id) + _create_link(context.rbc_factory, parent2_id, child_id) + context.rbc_child_id = child_id + + +@when("remaining cov get_parents is called on the child resource") +def step_remaining_cov_get_parents(context: Context) -> None: + context.rbc_parents = context.rbc_res_repo.get_parents(context.rbc_child_id) + + +@then("remaining cov {n:d} parent resources should be returned") +def step_remaining_cov_assert_parents_count(context: Context, n: int) -> None: + assert len(context.rbc_parents) == n, ( + f"Expected {n} parents, got {len(context.rbc_parents)}" + ) + + +# ═══════════════════════════════════════════════════════════════════════ +# ResourceRepository._get_ancestors + _build_cycle_path (branches 2635→2633, 2661→2659) +# ═══════════════════════════════════════════════════════════════════════ + + +@given('remaining cov resources "cyc-A" and "cyc-B" linked as A->B') +def step_remaining_cov_cycle_setup(context: Context) -> None: + _ensure_resource_type_row(context.rbc_factory, "test/link-type") + _create_resource_row(context.rbc_factory, "cyc-A", type_name="test/link-type") + _create_resource_row(context.rbc_factory, "cyc-B", type_name="test/link-type") + _create_link(context.rbc_factory, "cyc-A", "cyc-B") + + +@when("remaining cov link_child is called to create B->A forming a cycle") +def step_remaining_cov_link_cycle(context: Context) -> None: + try: + context.rbc_res_repo.link_child("cyc-B", "cyc-A") + context.rbc_error = None + except Exception as exc: + context.rbc_error = exc + + +@then("remaining cov a CycleDetectedError should be raised with a path") +def step_remaining_cov_assert_cycle(context: Context) -> None: + assert isinstance(context.rbc_error, CycleDetectedError), ( + f"Expected CycleDetectedError, got " + f"{type(context.rbc_error).__name__}: {context.rbc_error}" + ) + assert hasattr(context.rbc_error, "path"), "Error should have 'path' attribute" + assert len(context.rbc_error.path) > 0, "Cycle path should not be empty" + + +# ═══════════════════════════════════════════════════════════════════════ +# NamespacedProjectRepository.update: success path (branch 2928→2931) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov an in-memory project database") +def step_remaining_cov_proj_db(context: Context) -> None: + engine, factory = _make_engine_and_factory() + context.rbc_engine = engine + context.rbc_proj_factory = factory + context.rbc_proj_repo = NamespacedProjectRepository(session_factory=factory) + context.rbc_error = None + + +@given('remaining cov a project "{ns_name}" exists') +def step_remaining_cov_proj_exists(context: Context, ns_name: str) -> None: + from cleveragents.domain.models.core.project import NamespacedProject + + parts = ns_name.split("/", 1) + namespace = parts[0] if len(parts) == 2 else "local" + name = parts[1] if len(parts) == 2 else parts[0] + + project = NamespacedProject( + name=name, + namespace=namespace, + description="Original description", + ) + context.rbc_proj_repo.create(project) + context.rbc_project = project + + +@when('remaining cov the project "{ns_name}" is updated with new description') +def step_remaining_cov_proj_update(context: Context, ns_name: str) -> None: + from cleveragents.domain.models.core.project import NamespacedProject + + parts = ns_name.split("/", 1) + namespace = parts[0] if len(parts) == 2 else "local" + name = parts[1] if len(parts) == 2 else parts[0] + + updated = NamespacedProject( + name=name, + namespace=namespace, + description="Updated description via remaining cov test", + ) + try: + context.rbc_proj_repo.update(updated) + context.rbc_error = None + except Exception as exc: + context.rbc_error = exc + + +@then("remaining cov the project update should succeed") +def step_remaining_cov_assert_proj_updated(context: Context) -> None: + assert context.rbc_error is None, f"Unexpected error: {context.rbc_error}" + # Verify the update persisted + result = context.rbc_proj_repo.get(context.rbc_project.namespaced_name) + assert result is not None, "Project not found after update" + assert result.description == "Updated description via remaining cov test", ( + f"Expected updated description, got '{result.description}'" + ) + + +# ═══════════════════════════════════════════════════════════════════════ +# AutomationProfileRepository.upsert: new insert (branch 4193→4200) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov an in-memory automation profile database") +def step_remaining_cov_ap_db(context: Context) -> None: + engine, factory = _make_engine_and_factory() + context.rbc_engine = engine + context.rbc_ap_factory = factory + + from cleveragents.infrastructure.database.repositories import ( + AutomationProfileRepository, + ) + + context.rbc_ap_repo = AutomationProfileRepository(session_factory=factory) + context.rbc_error = None + + +@when('remaining cov a brand new profile "{name}" is upserted') +def step_remaining_cov_ap_new_upsert(context: Context, name: str) -> None: + from cleveragents.domain.models.core.automation_profile import AutomationProfile + + profile = AutomationProfile( + name=name, + description="Brand new profile", + schema_version="1.0", + ) + try: + context.rbc_ap_repo.upsert(profile) + context.rbc_error = None + except Exception as exc: + context.rbc_error = exc + + +@then('remaining cov the profile "{name}" should be retrievable') +def step_remaining_cov_assert_ap_exists(context: Context, name: str) -> None: + assert context.rbc_error is None, f"Unexpected error: {context.rbc_error}" + result = context.rbc_ap_repo.get_by_name(name) + assert result is not None, f"Profile '{name}' not found after upsert" + assert result.name == name, f"Expected name '{name}', got '{result.name}'" + + +# ═══════════════════════════════════════════════════════════════════════ +# ValidationAttachmentRepository.attach: project_name scope (branch 3633→3634) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov an in-memory database with validation tables") +def step_remaining_cov_val_db(context: Context) -> None: + engine, factory = _make_engine_and_factory() + context.rbc_engine = engine + context.rbc_val_factory = factory + context.rbc_val_repo = ValidationAttachmentRepository(session_factory=factory) + context.rbc_error = None + + +@when('remaining cov a validation is attached using project scope "{proj}"') +def step_remaining_cov_attach_with_project(context: Context, proj: str) -> None: + try: + context.rbc_attachment = context.rbc_val_repo.attach( + validation_name="local/check-proj", + resource_id="res-proj-1", + mode="required", + project_name=proj, + ) + context.rbc_error = None + except Exception as exc: + context.rbc_error = exc + + +@then('remaining cov the returned attachment has project_name "{proj}"') +def step_remaining_cov_assert_attach_project(context: Context, proj: str) -> None: + assert context.rbc_error is None, f"Unexpected error: {context.rbc_error}" + att = context.rbc_attachment + assert isinstance(att, dict), f"Expected dict, got {type(att)}" + assert att.get("project_name") == proj, ( + f"Expected project_name '{proj}', got '{att.get('project_name')}'" + ) + + +@when( + 'remaining cov a validation is attached using project-plan scope "{proj}" "{plan}"' +) +def step_remaining_cov_attach_with_project_and_plan( + context: Context, + proj: str, + plan: str, +) -> None: + try: + context.rbc_attachment = context.rbc_val_repo.attach( + validation_name="local/check-proj-plan", + resource_id="res-proj-plan-1", + mode="informational", + project_name=proj, + plan_id=plan, + ) + context.rbc_error = None + except Exception as exc: + context.rbc_error = exc + + +@then('remaining cov the returned attachment has project "{proj}" and plan "{plan}"') +def step_remaining_cov_assert_attach_project_plan( + context: Context, + proj: str, + plan: str, +) -> None: + assert context.rbc_error is None, f"Unexpected error: {context.rbc_error}" + att = context.rbc_attachment + assert isinstance(att, dict), f"Expected dict, got {type(att)}" + assert att.get("project_name") == proj, ( + f"Expected project_name '{proj}', got '{att.get('project_name')}'" + ) + assert att.get("plan_id") == plan, ( + f"Expected plan_id '{plan}', got '{att.get('plan_id')}'" + ) + + +# ═══════════════════════════════════════════════════════════════════════ +# LifecyclePlanRepository.update: multiple project links (branch 1395→1394) +# ═══════════════════════════════════════════════════════════════════════ + + +@given("remaining cov an in-memory lifecycle plan database") +def step_remaining_cov_plan_db(context: Context) -> None: + engine, factory = _make_engine_and_factory() + session = factory() + context.rbc_engine = engine + context.rbc_plan_factory = factory + context.rbc_plan_session = session + # Use a single-session factory so flush + commit are visible + context.rbc_plan_session_factory = lambda: session + + from cleveragents.infrastructure.database.repositories import ( + ActionRepository, + LifecyclePlanRepository, + ) + + context.rbc_action_repo = ActionRepository( + session_factory=context.rbc_plan_session_factory, + ) + context.rbc_plan_repo = LifecyclePlanRepository( + session_factory=context.rbc_plan_session_factory, + ) + context.rbc_error = None + + +@given('remaining cov a plan with action "{action_name}" exists') +def step_remaining_cov_plan_exists(context: Context, action_name: str) -> None: + from ulid import ULID + + from cleveragents.domain.models.core.action import Action, ActionState + from cleveragents.domain.models.core.plan import ( + NamespacedName, + Plan, + PlanIdentity, + PlanPhase, + PlanTimestamps, + ProcessingState, + ) + + parts = action_name.split("/", 1) + namespace = parts[0] if len(parts) == 2 else "local" + short = parts[1] if len(parts) == 2 else parts[0] + + action = Action( + namespaced_name=NamespacedName(namespace=namespace, name=short), + description="test action", + long_description=None, + definition_of_done="verify it works", + strategy_actor="local/strategist", + execution_actor="local/executor", + estimation_actor=None, + review_actor=None, + arguments=[], + reusable=True, + read_only=False, + state=ActionState("available"), + created_at=datetime.now(), + updated_at=datetime.now(), + created_by=None, + tags=[], + ) + context.rbc_action_repo.create(action) + context.rbc_plan_session.commit() + + now = datetime.now() + plan = Plan( + identity=PlanIdentity(plan_id=str(ULID()), attempt=1), + namespaced_name=NamespacedName(namespace="local", name="multi-link-plan"), + action_name=action_name, + description="plan for multi-link test", + definition_of_done="verify links", + phase=PlanPhase.STRATEGIZE, + processing_state=ProcessingState.QUEUED, + strategy_actor="local/strategist", + execution_actor="local/executor", + timestamps=PlanTimestamps(created_at=now, updated_at=now), + error_message=None, + error_details=None, + created_by=None, + tags=[], + reusable=True, + read_only=False, + ) + context.rbc_plan_repo.create(plan) + context.rbc_plan_session.commit() + context.rbc_plan = plan + + +@when("remaining cov the plan is updated with 3 project links") +def step_remaining_cov_plan_update_links(context: Context) -> None: + from cleveragents.domain.models.core.plan import ( + PlanTimestamps, + ProjectLink, + ) + + plan = context.rbc_plan + updated = plan.model_copy( + update={ + "project_links": [ + ProjectLink( + project_name="local/proj-alpha", alias="alpha", read_only=False + ), + ProjectLink( + project_name="local/proj-beta", alias="beta", read_only=True + ), + ProjectLink( + project_name="local/proj-gamma", alias=None, read_only=False + ), + ], + "timestamps": PlanTimestamps( + created_at=plan.timestamps.created_at, + updated_at=datetime.now(), + ), + } + ) + try: + context.rbc_plan_repo.update(updated) + context.rbc_plan_session.commit() + context.rbc_plan = updated + context.rbc_error = None + except Exception as exc: + context.rbc_error = exc + + +@then("remaining cov the plan should have 3 project links after retrieval") +def step_remaining_cov_assert_plan_links(context: Context) -> None: + assert context.rbc_error is None, f"Unexpected error: {context.rbc_error}" + plan_id = context.rbc_plan.identity.plan_id + retrieved = context.rbc_plan_repo.get(plan_id) + assert retrieved is not None, f"Plan '{plan_id}' not found after update" + assert len(retrieved.project_links) == 3, ( + f"Expected 3 project links, got {len(retrieved.project_links)}" + ) + link_names = sorted(pl.project_name for pl in retrieved.project_links) + assert link_names == sorted( + [ + "local/proj-alpha", + "local/proj-beta", + "local/proj-gamma", + ] + ), f"Unexpected link names: {link_names}"