fix(data-integrity): remove silent argument swap in ValidationAttachmentRepository.attach

Restore the TDD test files that were incorrectly deleted, fix any formatting/import issues, commit all staged changes, and force-push with lease.

The branch is supposed to fix issue #7492 (silent argument swap in
ValidationAttachmentRepository.attach). The code fix is correct (the swap
heuristic was removed). However, the TDD test files were DELETED in the
latest commit instead of being fixed properly. The tests must remain in
the codebase as regression guards per the TDD bug fix workflow.

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}"
+    )

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