UAT: `agents resource add --update` bypasses service layer with raw SQL #3872

New issue

Open

opened 2026-04-06 07:04:31 +00:00 by freemo · 0 comments

freemo commented

2026-04-06 07:04:31 +00:00

Owner

Summary

The agents resource add --update CLI command bypasses the ResourceRegistryService service layer when removing an existing resource before re-registering it. It directly manipulates ResourceModel and ResourceEdgeModel via raw SQLAlchemy queries, violating the repository pattern (ADR-007) and skipping any service-layer business logic (e.g., project-link cleanup, audit events).

Metadata

Commit Message: fix(resource): route resource add --update removal through service layer
Branch: fix/resource-add-update-service-layer
Milestone: Backlog
Parent Epic: #398

Background and Context

During UAT testing of the agents resource add --update CLI command, a service-layer bypass was discovered in src/cleveragents/cli/commands/resource.py. The --update branch of resource_add() directly accesses ResourceModel and ResourceEdgeModel via raw SQLAlchemy queries instead of delegating to ResourceRegistryService. This violates ADR-007 (Repository Pattern) and skips all service-layer business logic including project-link cleanup and future audit events.

Current Behavior

The --update branch in resource_add() (lines ~430-460 of src/cleveragents/cli/commands/resource.py) directly accesses the session factory and issues raw SQL deletes:

# Current broken implementation in resource_add() --update branch:
if update:
    try:
        existing = service.show_resource(name)
        # Remove the existing resource so we can re-register
        session = service._session()
        try:
            from cleveragents.infrastructure.database.models import (
                ResourceEdgeModel,
                ResourceModel,
            )
            # Remove edges first — raw SQL, no service layer
            session.query(ResourceEdgeModel).filter(...).delete(...)
            session.query(ResourceModel).filter_by(...).delete(...)
            session.commit()
        except Exception:
            session.rollback()
            raise
        finally:
            session.close()
    except NotFoundError:
        pass

Steps to reproduce:

Register a resource: agents resource add local/my-repo
Link it to a project: agents project resource add my-project local/my-repo
Re-register with update: agents resource add --update local/my-repo
Observe: project_resource_link rows for local/my-repo are left as orphans

Expected Behavior

The --update removal path calls service.remove_resource() (or equivalent service method, per issue #3865), which handles all cleanup atomically — including ProjectResourceLinkModel rows — before re-registering the resource.

Acceptance Criteria

agents resource add --update calls ResourceRegistryService.remove_resource() for the pre-removal step (no raw SQL in CLI)
The update path does not leave orphaned project_resource_link rows
The service._session() call is removed from CLI code
Unit tests (Behave) cover the --update path including project-link cleanup

Subtasks

Refactor the --update path in resource_add() to call service.remove_resource() (once that method exists per issue #3865) instead of raw SQL
Ensure the update path also cleans up ProjectResourceLinkModel rows before re-registering
Add BDD scenario: agents resource add --update on an existing resource that is linked to a project re-registers cleanly without leaving orphaned links
Run nox (all default sessions), fix any errors
Verify coverage >= 97% via nox -s coverage_report

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
agents resource add --update calls ResourceRegistryService.remove_resource() for the pre-removal step (no raw SQL in CLI).
The update path does not leave orphaned project_resource_link rows.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly (fix(resource): route resource add --update removal through service layer), followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch fix/resource-add-update-service-layer.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
All nox stages pass.
Coverage >= 97%.

Additional Context

This is the same pattern as the resource remove bypass (issue #3865); both should be fixed together
The service._session() call from CLI code is a code smell — CLI should not access the session factory directly
The spec (ADR-007: Repository Pattern) requires all data access to go through the repository/service layer

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

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

## Summary The `agents resource add --update` CLI command bypasses the `ResourceRegistryService` service layer when removing an existing resource before re-registering it. It directly manipulates `ResourceModel` and `ResourceEdgeModel` via raw SQLAlchemy queries, violating the repository pattern (ADR-007) and skipping any service-layer business logic (e.g., project-link cleanup, audit events). ## Metadata - **Commit Message**: `fix(resource): route resource add --update removal through service layer` - **Branch**: `fix/resource-add-update-service-layer` - **Milestone**: Backlog - **Parent Epic**: #398 ## Background and Context During UAT testing of the `agents resource add --update` CLI command, a service-layer bypass was discovered in `src/cleveragents/cli/commands/resource.py`. The `--update` branch of `resource_add()` directly accesses `ResourceModel` and `ResourceEdgeModel` via raw SQLAlchemy queries instead of delegating to `ResourceRegistryService`. This violates ADR-007 (Repository Pattern) and skips all service-layer business logic including project-link cleanup and future audit events. ## Current Behavior The `--update` branch in `resource_add()` (lines ~430-460 of `src/cleveragents/cli/commands/resource.py`) directly accesses the session factory and issues raw SQL deletes: ```python # Current broken implementation in resource_add() --update branch: if update: try: existing = service.show_resource(name) # Remove the existing resource so we can re-register session = service._session() try: from cleveragents.infrastructure.database.models import ( ResourceEdgeModel, ResourceModel, ) # Remove edges first — raw SQL, no service layer session.query(ResourceEdgeModel).filter(...).delete(...) session.query(ResourceModel).filter_by(...).delete(...) session.commit() except Exception: session.rollback() raise finally: session.close() except NotFoundError: pass ``` **Steps to reproduce:** 1. Register a resource: `agents resource add local/my-repo` 2. Link it to a project: `agents project resource add my-project local/my-repo` 3. Re-register with update: `agents resource add --update local/my-repo` 4. Observe: `project_resource_link` rows for `local/my-repo` are left as orphans ## Expected Behavior The `--update` removal path calls `service.remove_resource()` (or equivalent service method, per issue #3865), which handles all cleanup atomically — including `ProjectResourceLinkModel` rows — before re-registering the resource. ## Acceptance Criteria - [ ] `agents resource add --update` calls `ResourceRegistryService.remove_resource()` for the pre-removal step (no raw SQL in CLI) - [ ] The update path does not leave orphaned `project_resource_link` rows - [ ] The `service._session()` call is removed from CLI code - [ ] Unit tests (Behave) cover the `--update` path including project-link cleanup ## Subtasks - [ ] Refactor the `--update` path in `resource_add()` to call `service.remove_resource()` (once that method exists per issue #3865) instead of raw SQL - [ ] Ensure the update path also cleans up `ProjectResourceLinkModel` rows before re-registering - [ ] Add BDD scenario: `agents resource add --update` on an existing resource that is linked to a project re-registers cleanly without leaving orphaned links - [ ] Run `nox` (all default sessions), fix any errors - [ ] Verify coverage >= 97% via `nox -s coverage_report` ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - `agents resource add --update` calls `ResourceRegistryService.remove_resource()` for the pre-removal step (no raw SQL in CLI). - The update path does not leave orphaned `project_resource_link` rows. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly (`fix(resource): route resource add --update removal through service layer`), followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch `fix/resource-add-update-service-layer`. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass. - Coverage >= 97%. ## Additional Context - This is the same pattern as the `resource remove` bypass (issue #3865); both should be fixed together - The `service._session()` call from CLI code is a code smell — CLI should not access the session factory directly - The spec (ADR-007: Repository Pattern) requires all data access to go through the repository/service layer > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.6.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added a new dependency

2026-04-06 07:04:48 +00:00

#398 Epic: Post-MVP Resources

freemo added the

Points

label

2026-04-06 07:09:32 +00:00

freemo referenced this issue

2026-04-06 07:10:24 +00:00

UAT: agents resource type add --update silently aborts instead of updating the existing type #3888

freemo referenced this issue

2026-04-06 07:11:08 +00:00