CHANGELOG.md

@@ -159,6 +159,9 @@ ensuring data is stored with proper parameter values.
   with ``@tdd_issue @tdd_issue_11035`` tags that exercise the DB query code path and
   verify the project-level budget is applied to ``CoreContextBudget`` and
   ``ContextRequest``.
+
+- **Removed silent argument swap in `ValidationAttachmentRepository.attach`** (#8177 / #7492): Removed the fragile heuristic that compared whether "/" appeared in ``resource_id`` versus ``validation_name`` to silently swap their values. This caused data corruption — validation names and resource IDs were stored with reversed values for any resource ID containing slashes but validation name not. Arguments now flow directly from caller to the persistence layer in their correct positional order. BDD regression coverage added in ``features/validation_argument_order_integrity.feature``.
+
 - **Guard cleanup_stale against execute/processing and execute/complete plans** (#11121):
   ``_create_sandbox_for_plan()`` in ``src/cleveragents/cli/commands/plan.py`` now
   skips ``GitWorktreeSandbox.cleanup_stale()`` when the plan is in
@@ -1038,4 +1041,4 @@ iteration` and data corruption under concurrent plan execution. All public
 - **TUI -- Permission Question Widget**: A new inline `PermissionQuestionWidget`
   renders permission requests directly in the conversation stream for single-file
   operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`),
-  navigate with arrow keys, confirm with `Enter`, or press `v` to open the full
+  navigate with arrow keys, confirm with `Enter`, or press `v` to open the full

CONTRIBUTORS.md

@@ -37,6 +37,7 @@ Below are some of the specific details of various contributions.
 * HAL 9000 has contributed the PlanResult.success derivation fix (PR #8214 / issue #7501): replaced the incorrect `error_message is None` heuristic with a dedicated `result_success` column in the plans table, ensuring plans with historical build errors are not incorrectly marked as failed after a successful apply.
 * HAL 9000 has contributed the mandatory PR compliance checklist to `implementation-pool-supervisor.md` (#9824): created a new agent definition with an embedded 8-item checklist ensuring workers always update CHANGELOG.md, CONTRIBUTORS.md, include commit footers (`ISSUES CLOSED: #N`), verify CI passes, add BDD tests, reference the parent Epic, apply labels via forgejo-label-manager, and assign milestones before creating PRs. Includes concrete examples for each subsection and compliance verification pseudocode.
 * HAL 9000 has contributed comprehensive milestone documentation for v3.6.0 (Advanced Concepts & Deferred Features) and v3.7.0 (TUI Implementation) (PR #9903): split into sub-documents covering context strategies, LLM backends, resource types, A2A rename, container tool execution, scope chain resolution, cost/safety budgets, E2E workflow tests, code review examples, plugin architecture, TUI layout, persona system, reference/command input, session management, configuration, and TuiMaterializer integration.
+* HAL 9000 has contributed the ValidationAttachmentRepository data-integrity fix (PR #8177 / issue #7492): removed the silent argument-swap heuristic (`"/" in resource_id`) from `ValidationAttachmentRepository.attach()` that corrupted validation names and resource IDs when resource IDs contained slashes, replacing it with straightforward parameter-pass-through so arguments flow directly to the persistence layer in their correct positional order. Added comprehensive BDD regression tests covering all boundary conditions.
 * HAL 9000 has contributed the LLMTraceRepository data-integrity fix (PR #8185 / issue #7505): replaced the unconditional `session.commit()` in `LLMTraceRepository.save()` with a dual-path implementation that respects the UnitOfWork pattern — flushing only when an external session is provided, and flushing + committing + closing when operating standalone. This eliminates premature transaction commits, loss of rollback capability, and a docstring/implementation mismatch.
 * HAL 9000 has contributed the ACMS Index Data Model and File Traversal Engine (PR #9664 / issue #9579): foundational data structures for indexed context entries with hot/warm/cold/archive storage tier classification, tag system, and a timeout-safe chunked file traversal engine for large projects with 10,000+ files.
 * HAL 9000 has contributed the error-suppression removal fix (PR #9247 / issue #9060): removed both `try...except Exception:` blocks in `register_registry_agents()` that silently suppressed errors from `actor_registry.list_actors()` and the route bridge refresh, enabling exceptions to propagate per CONTRIBUTING.md fail-fast policy. Added three Behave scenarios verifying RuntimeError, AttributeError, and TypeError propagation.

features/repositories_uncovered_branches.feature

@@ -99,11 +99,11 @@ Feature: Repository uncovered branches and lines
     When repo branch cov I attach validation "local/check-lint" to resource "res-1"
     Then repo branch cov the attachment is returned with correct fields
 
-  Scenario: repo branch cov validation attach with swapped args auto-corrects
+  Scenario: repo branch cov validation attach preserves argument order despite slash in resource_id
     Given repo branch cov an in-memory database with tool tables
     And repo branch cov a validation tool "local/check-fmt" exists
     When repo branch cov I attach with validation_name "res/123" and resource_id "local/check-fmt"
-    Then repo branch cov the attachment swaps them correctly
+    Then repo branch cov the attachment preserves argument order
 
   # ── AutomationProfileRepository schema version mismatch ────
 

features/steps/repositories_uncovered_branches_steps.py

@@ -411,9 +411,10 @@ def step_assert_attachment(context: Context):
 
 
 @when('repo branch cov I attach with validation_name "{vn}" and resource_id "{rid}"')
-def step_attach_swapped(context: Context, vn: str, rid: str):
-    """The validation_name contains '/' but resource_id doesn't (or vice versa)
-    so the repo auto-swaps them."""
+def step_attach_preserved(context: Context, vn: str, rid: str):
+    """Arguments are passed to ``attach()`` in their declared order; the
+    repository no longer swaps ``validation_name`` and ``resource_id`` based
+    on slash presence."""
     repo = ValidationAttachmentRepository(
         session_factory=context.rb_session_factory,
     )
@@ -425,14 +426,15 @@ def step_attach_swapped(context: Context, vn: str, rid: str):
     session.commit()
 
 
-@then("repo branch cov the attachment swaps them correctly")
-def step_assert_swapped(context: Context):
+@then("repo branch cov the attachment preserves argument order")
+def step_assert_preserved(context: Context):
     att = context.rb_result
-    # "res/123" has "/" and "local/check-fmt" has "/" too, but the swap
-    # logic checks: "/" in resource_id AND "/" not in validation_name
-    # With both having "/", no swap happens.  Let's just verify it stored.
+    # The repository no longer swaps validation_name and resource_id.
+    # Verify both arguments were stored exactly as provided.
     assert isinstance(att, dict)
     assert "attachment_id" in att
+    assert att["validation_name"] == "res/123"
+    assert att["resource_id"] == "local/check-fmt"
 
 
 # ── AutomationProfileRepository: upsert schema mismatch ────────

features/steps/validation_argument_order_integrity_steps.py

@@ -0,0 +1,243 @@
+"""Step definitions for BDD regression tests of ValidationAttachmentRepository.attach argument-order integrity (PR #8177 / issue #7492).
+
+These scenarios verify that ``attach()`` stores ``validation_name`` and
+``resource_id`` in the exact order provided by the caller — never swapping
+them based on slash-containing heuristics.
+"""
+
+from __future__ import annotations
+
+import json as _json_lib
+
+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 (
+    DuplicateValidationAttachmentError,
+    ValidationAttachmentRepository,
+)
+
+# ---------------------------------------------------------------------------
+# Background helpers
+# ---------------------------------------------------------------------------
+
+
+@given("a validation attachment repository test environment")
+def step_test_environment(context: Context) -> None:
+    """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 (already created above)."""
+    # No-op — ``step_test_environment`` already called ``create_all``.
+    pass
+
+
+# ---------------------------------------------------------------------------
+# Given: Repository instance
+# ---------------------------------------------------------------------------
+
+
+@given("a validation attachment repository instance")
+def step_repository_instance(context: Context) -> None:
+    """Create a ``ValidationAttachmentRepository`` instance and reset state."""
+
+    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 that will cause duplicate-error later."""
+    session = context.SessionLocal()
+    try:
+        context.repository.attach(
+            validation_name=validation_name,
+            resource_id=resource_id,
+        )
+        session.commit()
+    except Exception as exc:
+        # If attach raised an error here (e.g., due to constraints), store it.
+        context.last_error = exc
+
+
+# ---------------------------------------------------------------------------
+# When: Call attach method with various parameter combinations
+# ---------------------------------------------------------------------------
+
+
+@when("simple-attach validation {validation_name} on resource {resource_id}")
+def step_call_attach_basic(
+    context: Context, validation_name: str, resource_id: str
+) -> None:
+    """Call ``attach()`` with the simplest signature."""
+    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(
+    "scope-attach validation {validation_name} on resource {resource_id} with project {project_name} and plan {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 and plan scoping."""
+    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(
+    "scope-attach validation {validation_name} on resource {resource_id} with mode {mode}"
+)
+def step_call_attach_with_mode(
+    context: Context,
+    validation_name: str,
+    resource_id: str,
+    mode: str,
+) -> None:
+    """Call ``attach()`` with a custom mode."""
+    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(
+    "json-attach validation {validation_name} on resource {resource_id} with args {args_json}"
+)
+def step_call_attach_with_args(
+    context: Context,
+    validation_name: str,
+    resource_id: str,
+    args_json: str,
+) -> None:
+    """Call ``attach()`` with JSON-serialised arguments."""
+    try:
+        args = _json_lib.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 and error conditions
+# ---------------------------------------------------------------------------
+
+
+@then("the attachment should have validation_name={value}")
+def step_assert_validation_name(context: Context, value: str) -> None:
+    """Assert that ``validation_name`` was stored exactly as specified."""
+    assert context.last_error is None, (
+        f"Unexpected error during attach: {context.last_error}"
+    )
+    att = context.last_attachment
+    assert isinstance(att, dict), f"Expected dict attachment, got {type(att)}"
+    assert "attachment_id" in att, "Missing attachment_id in result"
+    assert (
+        att["validation_name"] == value
+    ), f"Expected validation_name={value!r}, got {att['validation_name']!r}"
+
+
+@then("the attachment should have resource_id={value}")
+def step_assert_resource_id(context: Context, value: str) -> None:
+    """Assert that ``resource_id`` was stored exactly as specified."""
+    assert context.last_error is None, (
+        f"Unexpected error during attach: {context.last_error}"
+    )
+    att = context.last_attachment
+    assert isinstance(att, dict), f"Expected dict attachment, got {type(att)}"
+    assert "attachment_id" in att, "Missing attachment_id in result"
+    assert (
+        att["resource_id"] == value
+    ), f"Expected resource_id={value!r}, got {att['resource_id']!r}"
+
+
+@then('the attachment should NOT have validation_name="{value}"')
+def step_assert_no_swap_validation_name(context: Context, value: str) -> None:
+    """Assert that ``validation_name`` was NOT the value we expected for resource_id (swap guard)."""
+    # Only meaningful when both values differ — used in regression checks.
+    if context.last_error is not None or context.last_attachment is None:
+        return  # error already verified
+    att = context.last_attachment
+    assert att["validation_name"] != value, (
+        f"The swap heuristic incorrectly assigned {value!r} to validation_name"
+    )
+
+
+@then('the attachment should NOT have resource_id="{value}"')
+def step_assert_no_swap_resource_id(context: Context, value: str) -> None:
+    """Assert that ``resource_id`` was NOT the value we expected for validation_name (swap guard)."""
+    if context.last_error is not None or context.last_attachment is None:
+        return
+    att = context.last_attachment
+    assert att["resource_id"] != value, (
+        f"The swap heuristic incorrectly assigned {value!r} to resource_id"
+    )
+
+
+@then("the attachment should have mode={value}")
+def step_assert_mode(context: Context, value: str) -> None:
+    """Assert that the ``mode`` field was stored correctly."""
+    assert context.last_error is None, (
+        f"Unexpected error during attach: {context.last_error}"
+    )
+    att = context.last_attachment
+    assert att["mode"] == value, f"Expected mode={value!r}, got {att['mode']!r}"
+
+
+@then("a DuplicateValidationAttachmentError is raised")
+def step_assert_duplicate_rejection(context: Context) -> None:
+    """Assert that attempting to attach a duplicate raises the correct error."""
+    assert context.last_error is not None, (
+        "Expected DuplicateValidationAttachmentError but no exception was raised"
+    )
+    assert isinstance(
+        context.last_error, DuplicateValidationAttachmentError
+    ), f"Expected DuplicateValidationAttachmentError, got {type(context.last_error).__name__}"

features/validation_argument_order_integrity.feature

@@ -0,0 +1,83 @@
+Feature: ValidationAttachmentRepository.attach preserves argument order (#8177 #7492)
+  As a CleverAgents developer
+  I want the ValidationAttachmentRepository.attach method to always store validation_name
+  and resource_id in their correct positional order
+  So that data is never silently corrupted by a fragile slash-heuristic
+
+  Background:
+    Given a validation attachment repository test environment
+    And a test database for validation attachments
+    And a validation attachment repository instance
+
+  # --- Core: arguments are stored exactly as provided (no swap) ---
+
+  @issue_7492
+  Scenario: attach preserves order when resource_id contains slash but validation_name does not
+    When simple-attach validation "my-validator" on resource "ns/resource"
+    Then the attachment should have validation_name="my-validator"
+    And the attachment should have resource_id="ns/resource"
+
+  @issue_7492
+  Scenario: attach preserves order when resource_id is a simple name with no slash
+    When simple-attach validation "my-validator" on resource "simple-id"
+    Then the attachment should have validation_name="my-validator"
+    And the attachment should have resource_id="simple-id"
+
+  @issue_7492
+  Scenario: attach preserves order when both contain slashes
+    When simple-attach validation "ns/validator" on resource "ns/resource"
+    Then the attachment should have validation_name="ns/validator"
+    And the attachment should have resource_id="ns/resource"
+
+  @issue_7492
+  Scenario: attach preserves order when neither contains slashes
+    When simple-attach validation "validator" on resource "resource"
+    Then the attachment should have validation_name="validator"
+    And the attachment should have resource_id="resource"
+
+  @issue_7492
+  Scenario: attach handles resource_id with multiple slashes
+    When simple-attach validation "my-validator" on resource "a/b/c/d"
+    Then the attachment should have validation_name="my-validator"
+    And the attachment should have resource_id="a/b/c/d"
+
+  @issue_7492
+  Scenario: attach handles validation_name with multiple slashes
+    When simple-attach validation "a/b/c/validator" on resource "resource"
+    Then the attachment should have validation_name="a/b/c/validator"
+    And the attachment should have resource_id="resource"
+
+  # --- Optional parameters: do not affect argument placement ---
+
+  @issue_7492
+  Scenario: attach with project_name and plan_id preserves order
+    When scope-attach validation "my-validator" on resource "ns/resource" with project "prod" and plan "plan-1"
+    Then the attachment should have validation_name="my-validator"
+    And the attachment should have resource_id="ns/resource"
+
+  @issue_7492
+  Scenario: attach with mode preserves order
+    When scope-attach validation "my-validator" on resource "ns/resource" with mode "informational"
+    Then the attachment should have validation_name="my-validator"
+    And the attachment should have resource_id="ns/resource"
+    But the attachment should have mode="informational"
+
+  @issue_7492
+  Scenario: attach with args preserves order
+    When json-attach validation "my-validator" on resource "ns/resource" with args {"key": "val"}
+    Then the attachment should have validation_name="my-validator"
+    And the attachment should have resource_id="ns/resource"
+
+  # --- Edge cases ---
+
+  @issue_7492
+  Scenario: attach handles empty strings correctly
+    When simple-attach validation "" on resource ""
+    Then the attachment should have validation_name=""
+    And the attachment should have resource_id=""
+
+  @issue_7492
+  Scenario: duplicate attachment rejection preserves original argument order
+    Given an existing attachment with validation_name="dup-val" and resource_id="dup-res"
+    When simple-attach validation "dup-val" on resource "dup-res"
+    Then a DuplicateValidationAttachmentError is raised