diff --git a/features/steps/tdd_issue_7492_validation_attachment_argument_swap_steps.py b/features/steps/tdd_issue_7492_validation_attachment_argument_swap_steps.py new file mode 100644 index 000000000..8bbfc8e6b --- /dev/null +++ b/features/steps/tdd_issue_7492_validation_attachment_argument_swap_steps.py @@ -0,0 +1,281 @@ +"""Step definitions for TDD issue #7492: ValidationAttachmentRepository.attach argument swap bug. + +Tests that the attach() method preserves argument order and does not silently +swap validation_name and resource_id when resource_id contains a slash. +""" + +from __future__ import annotations + +import json +from typing import Any + +from behave import given, then, when +from behave.runner import Context +from sqlalchemy import create_engine +from sqlalchemy.orm import Session, sessionmaker + +from cleveragents.infrastructure.database.models import Base +from cleveragents.infrastructure.database.repositories import ( + ValidationAttachmentRepository, +) +from cleveragents.core.exceptions import BusinessRuleViolation + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("a validation attachment repository test environment") +def step_test_environment(context: Context) -> None: + """Set up the test environment for validation attachment tests.""" + # Create an in-memory SQLite database for testing + context.engine = create_engine("sqlite:///:memory:") + Base.metadata.create_all(context.engine) + context.SessionLocal = sessionmaker(bind=context.engine) + + +@given("a test database for validation attachments") +def step_test_database(context: Context) -> None: + """Ensure the test database is ready.""" + # Database is already created in the previous step + pass + + +# --------------------------------------------------------------------------- +# Given: Repository instance +# --------------------------------------------------------------------------- + + +@given("a validation attachment repository instance") +def step_repository_instance(context: Context) -> None: + """Create a ValidationAttachmentRepository instance.""" + + def session_factory() -> Session: + return context.SessionLocal() + + context.repository = ValidationAttachmentRepository(session_factory) + context.last_attachment = None + context.last_error = None + + +@given( + 'an existing attachment with validation_name="{validation_name}" and resource_id="{resource_id}"' +) +def step_existing_attachment( + context: Context, validation_name: str, resource_id: str +) -> None: + """Create an existing attachment in the database.""" + try: + context.repository.attach( + validation_name=validation_name, + resource_id=resource_id, + ) + except Exception as exc: + context.last_error = exc + + +# --------------------------------------------------------------------------- +# When: Call attach method +# --------------------------------------------------------------------------- + + +@when( + 'I call attach with validation_name="{validation_name}" and resource_id="{resource_id}"' +) +def step_call_attach_basic( + context: Context, validation_name: str, resource_id: str +) -> None: + """Call attach with basic parameters.""" + try: + context.last_attachment = context.repository.attach( + validation_name=validation_name, + resource_id=resource_id, + ) + context.last_error = None + except Exception as exc: + context.last_attachment = None + context.last_error = exc + + +@when( + 'I call attach with validation_name="{validation_name}" and resource_id="{resource_id}" and project_name="{project_name}" and plan_id="{plan_id}"' +) +def step_call_attach_with_scope( + context: Context, + validation_name: str, + resource_id: str, + project_name: str, + plan_id: str, +) -> None: + """Call attach with project_name and plan_id.""" + try: + context.last_attachment = context.repository.attach( + validation_name=validation_name, + resource_id=resource_id, + project_name=project_name, + plan_id=plan_id, + ) + context.last_error = None + except Exception as exc: + context.last_attachment = None + context.last_error = exc + + +@when( + 'I call attach with validation_name="{validation_name}" and resource_id="{resource_id}" and mode="{mode}"' +) +def step_call_attach_with_mode( + context: Context, validation_name: str, resource_id: str, mode: str +) -> None: + """Call attach with mode parameter.""" + try: + context.last_attachment = context.repository.attach( + validation_name=validation_name, + resource_id=resource_id, + mode=mode, + ) + context.last_error = None + except Exception as exc: + context.last_attachment = None + context.last_error = exc + + +@when( + 'I call attach with validation_name="{validation_name}" and resource_id="{resource_id}" and args={args_json}' +) +def step_call_attach_with_args( + context: Context, validation_name: str, resource_id: str, args_json: str +) -> None: + """Call attach with args parameter.""" + try: + args = json.loads(args_json) + context.last_attachment = context.repository.attach( + validation_name=validation_name, + resource_id=resource_id, + args=args, + ) + context.last_error = None + except Exception as exc: + context.last_attachment = None + context.last_error = exc + + +# --------------------------------------------------------------------------- +# Then: Verify attachment properties +# --------------------------------------------------------------------------- + + +@then('the attachment should have validation_name="{expected_value}"') +def step_verify_validation_name(context: Context, expected_value: str) -> None: + """Verify the validation_name field.""" + assert context.last_attachment is not None, "No attachment was created" + actual_value = context.last_attachment.get("validation_name") + assert actual_value == expected_value, ( + f"Expected validation_name={expected_value}, got {actual_value}" + ) + + +@then('the attachment should have resource_id="{expected_value}"') +def step_verify_resource_id(context: Context, expected_value: str) -> None: + """Verify the resource_id field.""" + assert context.last_attachment is not None, "No attachment was created" + actual_value = context.last_attachment.get("resource_id") + assert actual_value == expected_value, ( + f"Expected resource_id={expected_value}, got {actual_value}" + ) + + +@then('the attachment should have project_name="{expected_value}"') +def step_verify_project_name(context: Context, expected_value: str) -> None: + """Verify the project_name field.""" + assert context.last_attachment is not None, "No attachment was created" + actual_value = context.last_attachment.get("project_name") + assert actual_value == expected_value, ( + f"Expected project_name={expected_value}, got {actual_value}" + ) + + +@then('the attachment should have plan_id="{expected_value}"') +def step_verify_plan_id(context: Context, expected_value: str) -> None: + """Verify the plan_id field.""" + assert context.last_attachment is not None, "No attachment was created" + actual_value = context.last_attachment.get("plan_id") + assert actual_value == expected_value, ( + f"Expected plan_id={expected_value}, got {actual_value}" + ) + + +@then('the attachment should have mode="{expected_value}"') +def step_verify_mode(context: Context, expected_value: str) -> None: + """Verify the mode field.""" + assert context.last_attachment is not None, "No attachment was created" + actual_value = context.last_attachment.get("mode") + assert actual_value == expected_value, ( + f"Expected mode={expected_value}, got {actual_value}" + ) + + +@then('the attachment should NOT have validation_name="{unexpected_value}"') +def step_verify_not_validation_name(context: Context, unexpected_value: str) -> None: + """Verify the validation_name field is NOT a specific value.""" + assert context.last_attachment is not None, "No attachment was created" + actual_value = context.last_attachment.get("validation_name") + assert actual_value != unexpected_value, ( + f"validation_name should NOT be {unexpected_value}, but it is" + ) + + +@then('the attachment should NOT have resource_id="{unexpected_value}"') +def step_verify_not_resource_id(context: Context, unexpected_value: str) -> None: + """Verify the resource_id field is NOT a specific value.""" + assert context.last_attachment is not None, "No attachment was created" + actual_value = context.last_attachment.get("resource_id") + assert actual_value != unexpected_value, ( + f"resource_id should NOT be {unexpected_value}, but it is" + ) + + +@then('the attachment args should contain "{key}"') +def step_verify_args_contains_key(context: Context, key: str) -> None: + """Verify the args JSON contains a specific key.""" + assert context.last_attachment is not None, "No attachment was created" + args_json = context.last_attachment.get("args_json") + assert args_json is not None, "args_json is None" + args = json.loads(args_json) + assert key in args, f"args should contain key '{key}', but got {args}" + + +# --------------------------------------------------------------------------- +# Then: Verify result structure +# --------------------------------------------------------------------------- + + +@then("the result should be a dict") +def step_verify_result_is_dict(context: Context) -> None: + """Verify the result is a dictionary.""" + assert isinstance(context.last_attachment, dict), ( + f"Expected dict, got {type(context.last_attachment)}" + ) + + +@then('the result should have key "{key}"') +def step_verify_result_has_key(context: Context, key: str) -> None: + """Verify the result dict has a specific key.""" + assert context.last_attachment is not None, "No attachment was created" + assert key in context.last_attachment, f"Result should have key '{key}'" + + +# --------------------------------------------------------------------------- +# Then: Verify exceptions +# --------------------------------------------------------------------------- + + +@then("a DuplicateValidationAttachmentError should be raised") +def step_verify_duplicate_error(context: Context) -> None: + """Verify that a DuplicateValidationAttachmentError was raised.""" + assert context.last_error is not None, "Expected an error to be raised" + error_name = type(context.last_error).__name__ + assert error_name == "DuplicateValidationAttachmentError", ( + f"Expected DuplicateValidationAttachmentError, got {error_name}" + ) diff --git a/features/tdd_issue_7492_validation_attachment_argument_swap.feature b/features/tdd_issue_7492_validation_attachment_argument_swap.feature new file mode 100644 index 000000000..ed2a5c87a --- /dev/null +++ b/features/tdd_issue_7492_validation_attachment_argument_swap.feature @@ -0,0 +1,106 @@ +Feature: ValidationAttachmentRepository.attach does not swap arguments + As a CleverAgents developer + I want the ValidationAttachmentRepository.attach method to preserve argument order + So that validation names and resource IDs are stored correctly in the database + + Background: + Given a validation attachment repository test environment + And a test database for validation attachments + + # --- TDD red phase: prove the bug existed --- + + @tdd_issue_7492 + @tdd_expected_fail + Scenario: Bug: attach silently swaps arguments when resource_id contains slash + Given a validation attachment repository instance + When I call attach with validation_name="my-validator" and resource_id="ns/resource" + Then the attachment should have validation_name="my-validator" + And the attachment should have resource_id="ns/resource" + And the attachment should NOT have validation_name="ns/resource" + And the attachment should NOT have resource_id="my-validator" + + # --- TDD green phase: fix the bug --- + + @tdd_issue_7492 + Scenario: attach preserves arguments when resource_id contains slash + Given a validation attachment repository instance + When I call attach with validation_name="my-validator" and resource_id="ns/resource" + Then the attachment should have validation_name="my-validator" + And the attachment should have resource_id="ns/resource" + And the attachment should NOT have validation_name="ns/resource" + And the attachment should NOT have resource_id="my-validator" + + @tdd_issue_7492 + Scenario: attach preserves arguments when both contain slashes + Given a validation attachment repository instance + When I call attach with validation_name="ns/my-validator" and resource_id="ns/resource" + Then the attachment should have validation_name="ns/my-validator" + And the attachment should have resource_id="ns/resource" + + @tdd_issue_7492 + Scenario: attach preserves arguments when neither contains slashes + Given a validation attachment repository instance + When I call attach with validation_name="my-validator" and resource_id="simple-resource" + Then the attachment should have validation_name="my-validator" + And the attachment should have resource_id="simple-resource" + + @tdd_issue_7492 + Scenario: attach preserves arguments when validation_name contains slash + Given a validation attachment repository instance + When I call attach with validation_name="ns/my-validator" and resource_id="simple-resource" + Then the attachment should have validation_name="ns/my-validator" + And the attachment should have resource_id="simple-resource" + + # --- Edge cases --- + + @tdd_issue_7492 + Scenario: attach works with complex namespaced names + Given a validation attachment repository instance + When I call attach with validation_name="org/team/validator" and resource_id="project/resource/subresource" + Then the attachment should have validation_name="org/team/validator" + And the attachment should have resource_id="project/resource/subresource" + + @tdd_issue_7492 + Scenario: attach works with optional project_name and plan_id + Given a validation attachment repository instance + When I call attach with validation_name="my-validator" and resource_id="ns/resource" and project_name="my-project" and plan_id="01ARZ3NDEKTSV4RRFFQ69G5FAV" + Then the attachment should have validation_name="my-validator" + And the attachment should have resource_id="ns/resource" + And the attachment should have project_name="my-project" + And the attachment should have plan_id="01ARZ3NDEKTSV4RRFFQ69G5FAV" + + @tdd_issue_7492 + Scenario: attach works with mode parameter + Given a validation attachment repository instance + When I call attach with validation_name="my-validator" and resource_id="ns/resource" and mode="informational" + Then the attachment should have validation_name="my-validator" + And the attachment should have resource_id="ns/resource" + And the attachment should have mode="informational" + + @tdd_issue_7492 + Scenario: attach works with args parameter + Given a validation attachment repository instance + When I call attach with validation_name="my-validator" and resource_id="ns/resource" and args={"threshold": 0.8} + Then the attachment should have validation_name="my-validator" + And the attachment should have resource_id="ns/resource" + And the attachment args should contain "threshold" + + # --- Regression: ensure no silent behavior changes --- + + @tdd_issue_7492 + Scenario: attach returns correct attachment dict structure + Given a validation attachment repository instance + When I call attach with validation_name="my-validator" and resource_id="ns/resource" + Then the result should be a dict + And the result should have key "attachment_id" + And the result should have key "validation_name" + And the result should have key "resource_id" + And the result should have key "mode" + And the result should have key "created_at" + + @tdd_issue_7492 + Scenario: attach raises DuplicateValidationAttachmentError on duplicate + Given a validation attachment repository instance + And an existing attachment with validation_name="my-validator" and resource_id="ns/resource" + When I call attach with validation_name="my-validator" and resource_id="ns/resource" + Then a DuplicateValidationAttachmentError should be raised