UAT: agents resource remove and resource add --update bypass repository pattern by directly accessing service._session() #3813

Closed
opened 2026-04-06 06:34:27 +00:00 by freemo · 1 comment
Owner

Metadata

  • Branch: fix/resource-cli-bypass-repository-pattern
  • Commit Message: fix(cli): route resource remove and resource add --update through service layer instead of bypassing repository pattern
  • Milestone: (none — backlog)
  • Parent Epic: #397

Background

The agents resource remove and agents resource add --update CLI commands in cleveragents.cli.commands.resource bypass the repository pattern by directly accessing service._session() and executing raw SQLAlchemy queries. This violates the clean architecture principle that CLI commands should only interact with the service layer, which in turn uses repositories.

Specifically:

  1. resource_remove() function (around line 1377 in resource.py): calls service._session() directly, then executes raw session.query(ResourceEdgeModel) and session.query(ResourceModel) queries, bypassing ResourceRepository.
  2. resource_add() function with --update flag (around line 719 in resource.py): calls service._session() directly, then executes raw session.query(ResourceEdgeModel) and session.query(ResourceModel) queries to remove the existing resource before re-adding it.

Expected behavior: The ResourceRegistryService should expose remove_resource(name: str) -> None and update_resource(name: str, ...) -> None methods that internally use ResourceRepository to perform the deletion/update. CLI commands should call these service methods, not access the session directly.

Actual behavior: CLI commands bypass the service layer and repository pattern by calling service._session() and executing raw SQLAlchemy queries. This:

  1. Violates the repository pattern (data access logic leaks into the CLI layer)
  2. Bypasses any business logic that should live in the service/repository layer
  3. Creates tight coupling between the CLI and the database schema
  4. Makes the code harder to test (requires a real database instead of a mock repository)

Code locations:

  • cleveragents.cli.commands.resource.resource_remove — calls service._session() directly
  • cleveragents.cli.commands.resource.resource_add — calls service._session() directly when --update flag is used
  • cleveragents.application.services.resource_registry_service.ResourceRegistryService — missing remove_resource() and update_resource() methods that should be added

Discovered during UAT testing of the Repository Pattern and Data Layer feature area.

Backlog note: This issue was discovered during autonomous operation
on milestone v3.5.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Subtasks

  • Add remove_resource(name: str) -> None method to ResourceRegistryService that uses ResourceRepository to delete the resource and its edges
  • Add update_resource(name: str, ...) -> None method to ResourceRegistryService (or extend add_resource with an update flag) that uses ResourceRepository to remove and re-add the resource atomically
  • Refactor resource_remove() in cleveragents.cli.commands.resource to call service.remove_resource(name) instead of service._session()
  • Refactor resource_add() in cleveragents.cli.commands.resource to call service.update_resource(name, ...) (or equivalent) instead of service._session() when --update flag is used
  • Remove all direct session.query(ResourceEdgeModel) and session.query(ResourceModel) calls from the CLI layer
  • Ensure remove_resource() and update_resource() handle edge deletion before resource deletion to respect foreign key constraints
  • Write Behave unit tests (in features/) for ResourceRegistryService.remove_resource() using a mock ResourceRepository
  • Write Behave unit tests (in features/) for ResourceRegistryService.update_resource() using a mock ResourceRepository
  • Write Behave unit tests (in features/) for the refactored resource_remove CLI command verifying it calls the service method
  • Write Behave unit tests (in features/) for the refactored resource_add --update CLI command verifying it calls the service method
  • Ensure all new and modified methods carry full static type annotations (no # type: ignore)

Definition of Done

  • ResourceRegistryService.remove_resource(name: str) -> None is implemented and uses ResourceRepository internally — no raw SQLAlchemy queries
  • ResourceRegistryService exposes a method for the update-resource flow that uses ResourceRepository internally — no raw SQLAlchemy queries
  • resource_remove() CLI command calls service.remove_resource() — no direct service._session() access
  • resource_add() CLI command with --update flag calls the service-layer update method — no direct service._session() access
  • No session.query(ResourceEdgeModel) or session.query(ResourceModel) calls exist in cleveragents.cli.commands.resource
  • All new service methods are covered by Behave unit tests using mock repositories (no real database required)
  • No # type: ignore suppressions introduced
  • All nox stages pass
  • Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/resource-cli-bypass-repository-pattern` - **Commit Message**: `fix(cli): route resource remove and resource add --update through service layer instead of bypassing repository pattern` - **Milestone**: *(none — backlog)* - **Parent Epic**: #397 ## Background The `agents resource remove` and `agents resource add --update` CLI commands in `cleveragents.cli.commands.resource` bypass the repository pattern by directly accessing `service._session()` and executing raw SQLAlchemy queries. This violates the clean architecture principle that CLI commands should only interact with the service layer, which in turn uses repositories. **Specifically:** 1. `resource_remove()` function (around line 1377 in `resource.py`): calls `service._session()` directly, then executes raw `session.query(ResourceEdgeModel)` and `session.query(ResourceModel)` queries, bypassing `ResourceRepository`. 2. `resource_add()` function with `--update` flag (around line 719 in `resource.py`): calls `service._session()` directly, then executes raw `session.query(ResourceEdgeModel)` and `session.query(ResourceModel)` queries to remove the existing resource before re-adding it. **Expected behavior**: The `ResourceRegistryService` should expose `remove_resource(name: str) -> None` and `update_resource(name: str, ...) -> None` methods that internally use `ResourceRepository` to perform the deletion/update. CLI commands should call these service methods, not access the session directly. **Actual behavior**: CLI commands bypass the service layer and repository pattern by calling `service._session()` and executing raw SQLAlchemy queries. This: 1. Violates the repository pattern (data access logic leaks into the CLI layer) 2. Bypasses any business logic that should live in the service/repository layer 3. Creates tight coupling between the CLI and the database schema 4. Makes the code harder to test (requires a real database instead of a mock repository) **Code locations**: - `cleveragents.cli.commands.resource.resource_remove` — calls `service._session()` directly - `cleveragents.cli.commands.resource.resource_add` — calls `service._session()` directly when `--update` flag is used - `cleveragents.application.services.resource_registry_service.ResourceRegistryService` — missing `remove_resource()` and `update_resource()` methods that should be added *Discovered during UAT testing of the Repository Pattern and Data Layer feature area.* > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.5.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. ## Subtasks - [ ] Add `remove_resource(name: str) -> None` method to `ResourceRegistryService` that uses `ResourceRepository` to delete the resource and its edges - [ ] Add `update_resource(name: str, ...) -> None` method to `ResourceRegistryService` (or extend `add_resource` with an `update` flag) that uses `ResourceRepository` to remove and re-add the resource atomically - [ ] Refactor `resource_remove()` in `cleveragents.cli.commands.resource` to call `service.remove_resource(name)` instead of `service._session()` - [ ] Refactor `resource_add()` in `cleveragents.cli.commands.resource` to call `service.update_resource(name, ...)` (or equivalent) instead of `service._session()` when `--update` flag is used - [ ] Remove all direct `session.query(ResourceEdgeModel)` and `session.query(ResourceModel)` calls from the CLI layer - [ ] Ensure `remove_resource()` and `update_resource()` handle edge deletion before resource deletion to respect foreign key constraints - [ ] Write Behave unit tests (in `features/`) for `ResourceRegistryService.remove_resource()` using a mock `ResourceRepository` - [ ] Write Behave unit tests (in `features/`) for `ResourceRegistryService.update_resource()` using a mock `ResourceRepository` - [ ] Write Behave unit tests (in `features/`) for the refactored `resource_remove` CLI command verifying it calls the service method - [ ] Write Behave unit tests (in `features/`) for the refactored `resource_add --update` CLI command verifying it calls the service method - [ ] Ensure all new and modified methods carry full static type annotations (no `# type: ignore`) ## Definition of Done - [ ] `ResourceRegistryService.remove_resource(name: str) -> None` is implemented and uses `ResourceRepository` internally — no raw SQLAlchemy queries - [ ] `ResourceRegistryService` exposes a method for the update-resource flow that uses `ResourceRepository` internally — no raw SQLAlchemy queries - [ ] `resource_remove()` CLI command calls `service.remove_resource()` — no direct `service._session()` access - [ ] `resource_add()` CLI command with `--update` flag calls the service-layer update method — no direct `service._session()` access - [ ] No `session.query(ResourceEdgeModel)` or `session.query(ResourceModel)` calls exist in `cleveragents.cli.commands.resource` - [ ] All new service methods are covered by Behave unit tests using mock repositories (no real database required) - [ ] No `# type: ignore` suppressions introduced - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator
Author
Owner

Superseded by #3865 and #3872

This issue covers both agents resource remove and agents resource add --update bypassing the service layer. These have been split into more detailed, actionable issues:

  • #3865: agents resource remove bypasses service layer and leaves orphaned project-resource links (includes the critical orphaned link cleanup detail)
  • #3872: agents resource add --update bypasses service layer with raw SQL (references #3865 for the shared remove_resource() method)

Both #3865 and #3872 were filed after this issue and contain more specific implementation guidance. Closing this broader issue in favor of the more detailed split issues. Please track fixes in #3865 and #3872.


Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

**Superseded by #3865 and #3872** This issue covers both `agents resource remove` and `agents resource add --update` bypassing the service layer. These have been split into more detailed, actionable issues: - **#3865**: `agents resource remove` bypasses service layer and leaves orphaned project-resource links (includes the critical orphaned link cleanup detail) - **#3872**: `agents resource add --update` bypasses service layer with raw SQL (references #3865 for the shared `remove_resource()` method) Both #3865 and #3872 were filed after this issue and contain more specific implementation guidance. Closing this broader issue in favor of the more detailed split issues. Please track fixes in #3865 and #3872. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#397 Epic: Server & Autonomy Infrastructure
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core#3813
No description provided.