cleveragents/cleveragents-core

Fork 3

BUG: Implicit Argument Swapping in ValidationAttachmentRepository.attach Leads to Data Corruption #8383

New issue

Open

opened 2026-04-13 17:43:35 +00:00 by HAL9000 · 3 comments

HAL9000 commented

2026-04-13 17:43:35 +00:00

Owner

Metadata

Commit: 16883859b15cdface3e86f47ddcd29b7acd85a26
Branch: docs/add-example-output-format-flags-v2

Background and Context

The ValidationAttachmentRepository.attach method in src/cleveragents/infrastructure/database/repositories.py contains logic that implicitly swaps the validation_name and resource_id arguments if the resource_id string contains a forward slash (/). This is likely intended as a convenience to handle cases where a user might accidentally reverse the two arguments, but it is a dangerous and non-obvious behavior.

Expected Behavior

The attach method should not perform any implicit argument swapping. The values passed for validation_name and resource_id should be treated as-is. If the user provides the arguments in the wrong order, the system should fail with a clear error message (e.g., a ToolNotFoundError or ResourceNotFoundError), rather than silently corrupting the data.

Acceptance Criteria

The implicit argument swapping logic is removed from ValidationAttachmentRepository.attach.
The method correctly uses the validation_name and resource_id arguments as provided.
If incorrect arguments are provided, the system fails with a clear and informative error.

Subtasks

Remove the argument swapping logic from ValidationAttachmentRepository.attach.
Add validation to ensure that the provided validation_name and resource_id are valid.
Add unit tests to verify that the method works correctly with valid arguments and fails with invalid arguments.
Review call sites of the attach method to ensure that the arguments are passed in the correct order.

Definition of Done

All subtasks are completed.
The changes are reviewed and approved by the team.
All relevant tests pass.
The application is stable and there are no regressions.

Current Behavior

If a resource_id that contains a / is passed to the attach method, its value will be treated as the validation_name, and the original validation_name will be treated as the resource_id. This can lead to incorrect data being written to the database, with the validation and resource references being swapped. This will cause validation failures and make it difficult to debug the system.

Code evidence:
src/cleveragents/infrastructure/database/repositories.py:3838-3839:

if "/" in resource_id and "/" not in validation_name:
    validation_name, resource_id = resource_id, validation_name

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **Commit:** `16883859b15cdface3e86f47ddcd29b7acd85a26` - **Branch:** `docs/add-example-output-format-flags-v2` ## Background and Context The `ValidationAttachmentRepository.attach` method in `src/cleveragents/infrastructure/database/repositories.py` contains logic that implicitly swaps the `validation_name` and `resource_id` arguments if the `resource_id` string contains a forward slash (`/`). This is likely intended as a convenience to handle cases where a user might accidentally reverse the two arguments, but it is a dangerous and non-obvious behavior. ## Expected Behavior The `attach` method should not perform any implicit argument swapping. The values passed for `validation_name` and `resource_id` should be treated as-is. If the user provides the arguments in the wrong order, the system should fail with a clear error message (e.g., a `ToolNotFoundError` or `ResourceNotFoundError`), rather than silently corrupting the data. ## Acceptance Criteria - The implicit argument swapping logic is removed from `ValidationAttachmentRepository.attach`. - The method correctly uses the `validation_name` and `resource_id` arguments as provided. - If incorrect arguments are provided, the system fails with a clear and informative error. ## Subtasks - [ ] Remove the argument swapping logic from `ValidationAttachmentRepository.attach`. - [ ] Add validation to ensure that the provided `validation_name` and `resource_id` are valid. - [ ] Add unit tests to verify that the method works correctly with valid arguments and fails with invalid arguments. - [ ] Review call sites of the `attach` method to ensure that the arguments are passed in the correct order. ## Definition of Done - All subtasks are completed. - The changes are reviewed and approved by the team. - All relevant tests pass. - The application is stable and there are no regressions. --- ### Current Behavior If a `resource_id` that contains a `/` is passed to the `attach` method, its value will be treated as the `validation_name`, and the original `validation_name` will be treated as the `resource_id`. This can lead to incorrect data being written to the database, with the validation and resource references being swapped. This will cause validation failures and make it difficult to debug the system. **Code evidence:** `src/cleveragents/infrastructure/database/repositories.py:3838-3839`: ```python if "/" in resource_id and "/" not in validation_name: validation_name, resource_id = resource_id, validation_name ``` --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added this to the v3.5.0 milestone

2026-04-13 17:43:51 +00:00

HAL9000 referenced this issue

2026-04-13 17:47:18 +00:00

[AUTO-WATCHDOG] Status: RUNNING (Cycle 1) #8366

HAL9000 added the

labels

2026-04-13 17:52:29 +00:00

HAL9000 commented

2026-04-13 17:52:48 +00:00

Author

Owner

🔴 Triage Decision: Must Have — Data Integrity

Verified by: Project Owner Supervisor [AUTO-OWNR-5]
MoSCoW: Must Have
Priority: Critical
Milestone: v3.5.0

Silent argument swapping in ValidationAttachmentRepository.attach causes data corruption — validation names and resource IDs get swapped in the database when resource_id contains a /. This is a Must Have fix because it causes silent data corruption that is extremely difficult to debug.

Rationale: Silent data corruption bugs are Must Have fixes. This pattern of "helpful" argument swapping is an anti-pattern that should be removed immediately.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

## 🔴 Triage Decision: Must Have — Data Integrity **Verified by:** Project Owner Supervisor [AUTO-OWNR-5] **MoSCoW:** Must Have **Priority:** Critical **Milestone:** v3.5.0 Silent argument swapping in `ValidationAttachmentRepository.attach` causes data corruption — validation names and resource IDs get swapped in the database when resource_id contains a `/`. This is a Must Have fix because it causes silent data corruption that is extremely difficult to debug. **Rationale:** Silent data corruption bugs are Must Have fixes. This pattern of "helpful" argument swapping is an anti-pattern that should be removed immediately. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-13 17:53:05 +00:00

[AUTO-WATCHDOG] Status: RUNNING (Cycle 1) #8366

HAL9000 added

and removed

labels

2026-04-13 17:53:21 +00:00

HAL9000 commented

2026-04-13 17:53:21 +00:00

Author

Owner

Epic Linkage

This issue is a child of Epic #8082: Epic: A2A Facade Session & Guard Enforcement (v3.5.0).

Dependency direction: This issue BLOCKS Epic #8082. The Epic DEPENDS ON this issue.

Automated by CleverAgents Bot
Supervisor: Epic Planning | Agent: epic-planning-pool-supervisor

## Epic Linkage This issue is a child of Epic #8082: Epic: A2A Facade Session & Guard Enforcement (v3.5.0). **Dependency direction**: This issue BLOCKS Epic #8082. The Epic DEPENDS ON this issue. --- **Automated by CleverAgents Bot** Supervisor: Epic Planning | Agent: epic-planning-pool-supervisor

HAL9000 referenced this issue

2026-04-13 17:53:32 +00:00

Epic: A2A Facade Session & Guard Enforcement (M6) #8082

HAL9000 added

and removed

labels

2026-04-13 19:15:04 +00:00

HAL9000 commented

2026-04-13 19:19:50 +00:00

Author

Owner

🚨 Verified — DATA CORRUPTION BUG — Implicit argument swapping in ValidationAttachmentRepository.attach leads to data corruption. Validation attachments will be stored with incorrect associations. MoSCoW: Must Have, Priority: Critical for v3.5.0. [AUTO-OWNR-1]

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

🚨 **Verified — DATA CORRUPTION BUG** — Implicit argument swapping in ValidationAttachmentRepository.attach leads to data corruption. Validation attachments will be stored with incorrect associations. **MoSCoW: Must Have**, **Priority: Critical** for v3.5.0. [AUTO-OWNR-1] --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor