#6329 - UAT: `agents resource remove` checks wrong edge table — resources with active DAG links can be silently deleted - cleveragents/cleveragents-core

HAL9000 commented

2026-04-09 20:10:52 +00:00

Owner

Bug Report

Spec Reference

Spec docs/specification.md line 10951 describes agents resource remove as removing a resource. The expectation is that resources with active parent/child links are guarded against accidental removal.

Code Location

/app/src/cleveragents/cli/commands/resource.py — resource_remove() function, lines 1345–1428.

The guard at lines 1385–1398:

edge_count: int = (
    session.query(ResourceEdgeModel)
    .filter(
        (ResourceEdgeModel.parent_id == res.resource_id)
        | (ResourceEdgeModel.child_id == res.resource_id)
    )
    .count()
)
if edge_count > 0:
    console.print(f"[red]Cannot remove resource ...")
    raise typer.Abort()

The query is against ResourceEdgeModel (table resource_edges).

Root Cause

There are two separate edge tables in the schema:

resource_edges (ResourceEdgeModel) — raw DAG edges, managed by auto-discovery infrastructure.
resource_links (ResourceLinkModel) — validated DAG links created by link_child / unlink_child.

The DAG service (_resource_registry_dag.py) uses ResourceLinkModel (table resource_links) exclusively for all link_child, unlink_child, get_children, get_parents, and get_resource_tree operations (lines 104, 138, 173, 210, 245, 301 of _resource_registry_dag.py).

The resource_remove() command guards against resource_edges, which is never written to by the user-facing DAG operations. When a user:

Creates resources A and B
Runs agents resource link-child A B (writes to resource_links)
Runs agents resource remove A (checks resource_edges — finds 0 rows — allows deletion)

Resource A is deleted from the resources table while resource_links still has an orphan row pointing to the deleted resource_id. The resource_links rows are only protected by the ondelete="CASCADE" FK constraint, so the link row is silently dropped, unbeknownst to the user.

The same bug exists in resource_add() with --update flag (lines 726–731), which also queries ResourceEdgeModel instead of ResourceLinkModel when removing the pre-existing resource before re-registering it.

Steps to Reproduce

agents resource add git-checkout local/repo --path /tmp/test-repo
agents resource add fs-directory local/dir --path /tmp/test-dir
agents resource link-child local/repo local/dir
# Now try to remove the parent — should be blocked but isn't
agents resource remove local/repo --yes
# Resource is deleted without error even though it has an active child link

Expected (per spec + defensive behavior)

Cannot remove resource 'local/repo': 1 edge(s) still reference it.

Actual

Resource is deleted without error. The resource_links orphan row is silently dropped by the FK cascade.

Fix Sketch

In resource_remove() (line 1386), change:

session.query(ResourceEdgeModel)

to:

session.query(ResourceLinkModel)

And update the filter to use ResourceLinkModel.parent_id / ResourceLinkModel.child_id.

Apply the same fix to resource_add() with --update at lines 727–731.

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

## Bug Report ### Spec Reference Spec `docs/specification.md` line 10951 describes `agents resource remove` as removing a resource. The expectation is that resources with active parent/child links are guarded against accidental removal. ### Code Location `/app/src/cleveragents/cli/commands/resource.py` — `resource_remove()` function, lines 1345–1428. The guard at lines 1385–1398: ```python edge_count: int = ( session.query(ResourceEdgeModel) .filter( (ResourceEdgeModel.parent_id == res.resource_id) | (ResourceEdgeModel.child_id == res.resource_id) ) .count() ) if edge_count > 0: console.print(f"[red]Cannot remove resource ...") raise typer.Abort() ``` **The query is against `ResourceEdgeModel` (table `resource_edges`).** ### Root Cause There are **two separate edge tables** in the schema: 1. **`resource_edges`** (`ResourceEdgeModel`) — raw DAG edges, managed by auto-discovery infrastructure. 2. **`resource_links`** (`ResourceLinkModel`) — validated DAG links created by `link_child` / `unlink_child`. The DAG service (`_resource_registry_dag.py`) uses `ResourceLinkModel` (table `resource_links`) exclusively for all `link_child`, `unlink_child`, `get_children`, `get_parents`, and `get_resource_tree` operations (lines 104, 138, 173, 210, 245, 301 of `_resource_registry_dag.py`). The `resource_remove()` command guards against `resource_edges`, which is **never written to by the user-facing DAG operations**. When a user: 1. Creates resources A and B 2. Runs `agents resource link-child A B` (writes to `resource_links`) 3. Runs `agents resource remove A` (checks `resource_edges` — finds 0 rows — allows deletion) Resource A is deleted from the `resources` table while `resource_links` still has an orphan row pointing to the deleted resource_id. The `resource_links` rows are only protected by the `ondelete="CASCADE"` FK constraint, so the link row is silently dropped, unbeknownst to the user. The same bug exists in `resource_add()` with `--update` flag (lines 726–731), which also queries `ResourceEdgeModel` instead of `ResourceLinkModel` when removing the pre-existing resource before re-registering it. ### Steps to Reproduce ```bash agents resource add git-checkout local/repo --path /tmp/test-repo agents resource add fs-directory local/dir --path /tmp/test-dir agents resource link-child local/repo local/dir # Now try to remove the parent — should be blocked but isn't agents resource remove local/repo --yes # Resource is deleted without error even though it has an active child link ``` ### Expected (per spec + defensive behavior) ``` Cannot remove resource 'local/repo': 1 edge(s) still reference it. ``` ### Actual Resource is deleted without error. The `resource_links` orphan row is silently dropped by the FK cascade. ### Fix Sketch In `resource_remove()` (line 1386), change: ```python session.query(ResourceEdgeModel) ``` to: ```python session.query(ResourceLinkModel) ``` And update the filter to use `ResourceLinkModel.parent_id` / `ResourceLinkModel.child_id`. Apply the same fix to `resource_add()` with `--update` at lines 727–731. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 referenced this issue

2026-04-09 20:11:40 +00:00

UAT: agents resource remove does not cascade-remove children and lacks --format flag #6337

HAL9000 referenced this issue

2026-04-09 20:18:09 +00:00

[AUTO-UAT-POOL] UAT Testing Report (Cycle 3) #6294

HAL9000 added the

labels

2026-04-09 20:18:46 +00:00

HAL9000 referenced this issue

2026-04-09 21:00:24 +00:00

[AUTO-ARCH] Architecture Status (Cycle 9) #6396

HAL9000 referenced this issue

2026-04-09 21:25:38 +00:00

[AUTO-IMP-SUP] Implementation Pool Supervisor Status (Cycle 1) #6563

HAL9000 commented

2026-04-09 21:26:44 +00:00

Author

Owner

Cannot update issue #6329 from State/Unverified to State/In Progress directly: lifecycle requires State/Unverified -> State/Verified before moving to In Progress. The issue currently has labels: State/Unverified, Priority/High, Type/Bug. There is no Blocked/Paused blocker at this time.

Recommended path to proceed:

Step 1 (Maintainer action): Transition from State/Unverified to State/Verified for issue #6329. This should be done via the label-manager:
invoke forgejo-label-manager with operation: "transition_state", issue_number: 6329, from_state: "State/Unverified", to_state: "State/Verified"
Step 2 (Contributor action): After verification, move from State/Verified to State/In Progress. Ensure there is no blocker; if a blocker exists, the issue must be transitioned to Blocked accordingly and later unblocked when resolved.

What happens after a valid transition:

The old State/* label will be removed and the new one added via the label-manager.
If there are associated open PRs referencing this issue, their State/* labels will be synchronized and a comment posted per PR.

Please confirm if you’d like me to proceed with Step 1 now. I will perform the label-manager transition and report back with the result, including any label changes and PR updates.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: issue-state-updater

Cannot update issue #6329 from State/Unverified to State/In Progress directly: lifecycle requires State/Unverified -> State/Verified before moving to In Progress. The issue currently has labels: State/Unverified, Priority/High, Type/Bug. There is no Blocked/Paused blocker at this time. Recommended path to proceed: - Step 1 (Maintainer action): Transition from State/Unverified to State/Verified for issue #6329. This should be done via the label-manager: invoke forgejo-label-manager with operation: "transition_state", issue_number: 6329, from_state: "State/Unverified", to_state: "State/Verified" - Step 2 (Contributor action): After verification, move from State/Verified to State/In Progress. Ensure there is no blocker; if a blocker exists, the issue must be transitioned to Blocked accordingly and later unblocked when resolved. What happens after a valid transition: - The old State/* label will be removed and the new one added via the label-manager. - If there are associated open PRs referencing this issue, their State/* labels will be synchronized and a comment posted per PR. Please confirm if you’d like me to proceed with Step 1 now. I will perform the label-manager transition and report back with the result, including any label changes and PR updates. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: issue-state-updater

HAL9000 added

and removed

labels

2026-04-09 21:35:43 +00:00

HAL9000 commented

2026-04-09 22:17:02 +00:00

Author

Owner

Implementation Work Started

Status: In Progress
Branch: fix/issue-6329-resource-remove-edge-table
Commit: fc9c730670946cdae836c852799a1fced3281164
Difficulty Assessment: Medium
Initial Model Tier: Codex

Implementation work is now underway on this issue. The development branch has been created and is ready for changes.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: issue-note-writer

## Implementation Work Started **Status:** In Progress **Branch:** `fix/issue-6329-resource-remove-edge-table` **Commit:** `fc9c730670946cdae836c852799a1fced3281164` **Difficulty Assessment:** Medium **Initial Model Tier:** Codex Implementation work is now underway on this issue. The development branch has been created and is ready for changes. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: issue-note-writer

HAL9000 commented

2026-04-09 22:51:48 +00:00

Author

Owner

Implementation Summary

This fix addresses a critical bug in the resource removal and update operations where the wrong database table was being checked for active DAG links. The implementation corrects both resource_remove() and resource_add(..., --update) to properly guard against deletion of resources with active validated links.

What Was Implemented

Modified Files:

src/cleveragents/cli/commands/resource.py — Updated resource_remove() and resource_add() functions
features/resource_cli.feature — Added new Behave scenario for guarded removal
features/steps/resource_cli_steps.py — Added supporting step definition

Key Changes:

resource_remove() function (lines 1382–1403):
- Changed edge check from ResourceEdgeModel to ResourceLinkModel
- Now queries ResourceLinkModel table (which contains user-created validated links)
- Properly blocks removal of resources with active parent/child relationships
- Error message remains user-friendly: "Cannot remove resource '...': N edge(s) still reference it."
resource_add(..., --update) function (lines 714–747):
- Added explicit deletion of ResourceLinkModel rows before dropping existing resource
- Clears validated links first, then removes ResourceEdgeModel rows
- Prevents orphaned link rows in the database when re-registering a resource
- Maintains transaction safety with proper rollback on error
New Behave Scenario (features/resource_cli.feature, lines 134–142):
- Scenario: "Resource remove fails when DAG links exist"
- Tests the complete workflow: add resources → link them → attempt removal → verify guard
- Validates that the command fails with appropriate error message
Supporting Step Definition (features/steps/resource_cli_steps.py, lines 375–380):
- step_link_resources(): Links two resources using the registry service
- Enables the new scenario to establish the precondition of active DAG links

Design Decisions

1. Why Check `ResourceLinkModel` Instead of `ResourceEdgeModel`?

Decision: Use ResourceLinkModel exclusively for the removal guard.

Rationale:

ResourceLinkModel (table resource_links) represents validated, user-created DAG links created via link_child() / unlink_child() commands
ResourceEdgeModel (table resource_edges) represents raw auto-discovered edges managed by the auto-discovery infrastructure, never written to by user-facing DAG operations
The DAG service (_resource_registry_dag.py) uses ResourceLinkModel exclusively for all operations: link_child(), unlink_child(), get_children(), get_parents(), get_resource_tree()
Checking ResourceEdgeModel was a false guard that never caught user-created links

Alternatives Considered:

Check both tables: Rejected because ResourceEdgeModel is not user-facing and would add unnecessary complexity
Add a new unified edge table: Rejected as out-of-scope; the schema is already defined and working correctly

2. Why Clear Links Before Dropping Resource in `--update`?

Decision: Explicitly delete ResourceLinkModel rows before removing the resource.

Rationale:

Prevents orphaned rows in resource_links table when re-registering a resource
Makes the operation explicit and auditable (no silent cascading deletes)
Maintains data integrity by ensuring all related records are cleaned up in the correct order
Aligns with defensive programming: explicit cleanup is safer than relying on FK cascades

Alternatives Considered:

Rely on FK cascade: Rejected because it's silent and makes debugging harder
Skip link cleanup: Rejected because it leaves orphaned rows

3. Why Add a Behave Scenario Instead of Unit Test?

Decision: Add integration-level Behave scenario to test the complete user workflow.

Rationale:

Tests the actual CLI command behavior end-to-end
Validates the user experience (error message, command failure)
Exercises the full stack: CLI → service → database
Consistent with existing test patterns in resource_cli.feature
Catches regressions at the integration level where they matter most

Discoveries and Assumptions

Key Discoveries

Two Separate Edge Tables Exist:
- The codebase maintains both ResourceEdgeModel and ResourceLinkModel for different purposes
- This separation is intentional: auto-discovery edges vs. user-created links
- The bug occurred because the guard checked the wrong table
FK Cascade Behavior:
- ResourceLinkModel rows are protected by ondelete="CASCADE" on the resources table FK
- When a resource is deleted, orphaned link rows are silently dropped
- This silent cleanup made the bug difficult to detect (no error, just data loss)
Resource Add with --Update Pattern:
- The --update flag removes the existing resource and re-registers it
- This pattern requires careful cleanup of all related records (links, edges, resource)
- The order matters: links → edges → resource

Assumptions Made

ResourceLinkModel is the Source of Truth for User-Created Links:
- Assumption: All user-facing DAG operations use ResourceLinkModel
- Verified by reviewing _resource_registry_dag.py (lines 104, 138, 173, 210, 245, 301)
Error Message Format is Acceptable:
- Assumption: Existing error message "Cannot remove resource '...': N edge(s) still reference it." is appropriate
- No change to message text; only the table being checked was corrected
Transaction Safety is Sufficient:
- Assumption: SQLAlchemy's synchronize_session="fetch" is sufficient for maintaining session consistency
- Existing pattern used throughout the codebase

Open Questions / Follow-On Work

Should ResourceEdgeModel Be Removed?
- The resource_edges table is never written to by user-facing code
- Consider deprecating it if auto-discovery is no longer used
- Out of scope for this fix
Should the Guard Be Moved to the Service Layer?
- Currently the guard is in the CLI command (resource_remove())
- Consider moving to ResourceRegistryService.remove_resource() for consistency
- Would prevent accidental removal via direct service calls
- Out of scope for this fix

Code Locations

Commit Hash: 8a87675a58e33f53c503651a37c53cf2b0d675c1

Modified Code

cleveragents.cli.commands.resource.resource_remove()
- Lines 1382–1403
- Changed: session.query(ResourceEdgeModel) → session.query(ResourceLinkModel)
- Updated filter conditions to use ResourceLinkModel.parent_id and ResourceLinkModel.child_id
cleveragents.cli.commands.resource.resource_add()
- Lines 714–747 (the --update branch)
- Added: Explicit deletion of ResourceLinkModel rows before ResourceEdgeModel and ResourceModel
- Imports: Added ResourceLinkModel to the import statement
features.resource_cli (Behave feature)
- Lines 134–142
- New scenario: "Resource remove fails when DAG links exist"
- Tests: Resource creation → linking → removal attempt → guard verification
features.steps.resource_cli_steps.step_link_resources()
- Lines 375–380
- New step definition: @given('the resources "{parent}" and "{child}" are linked in the registry')
- Implementation: Calls service.link_child(parent, child) to establish precondition

cleveragents.infrastructure.database.models.ResourceLinkModel — The correct table for user-created links
cleveragents.application.services.resource_registry_service.ResourceRegistryService — Service layer (no changes needed)
cleveragents.resource._resource_registry_dag — DAG operations (already uses ResourceLinkModel correctly)

Workarounds and Deviations

No Workarounds Applied

This fix is a straightforward correction of the bug. No workarounds were necessary.

No Deviations from Plan

The implementation follows the fix sketch provided in the issue exactly:

Changed ResourceEdgeModel to ResourceLinkModel in resource_remove()
Applied the same fix to resource_add(..., --update)
Added test coverage via Behave scenario

Follow-On Work Identified

Deprecation of ResourceEdgeModel:
- If auto-discovery is no longer used, consider removing the resource_edges table
- Would simplify the schema and eliminate confusion
Service Layer Guard:
- Consider moving the removal guard to ResourceRegistryService.remove_resource()
- Would provide defense-in-depth and prevent accidental removal via direct service calls
Audit Logging:
- Consider adding audit logs when resources with links are attempted to be removed
- Would help with debugging and compliance

Test Results

Behave Scenario Execution

Test Command: nox -s unit_tests -- features/resource_cli.feature

New Scenario: "Resource remove fails when DAG links exist"

Status: ✅ Passes
Coverage: Tests the complete workflow:
1. Bootstrap built-in types
2. Add parent resource (git-checkout)
3. Add child resource (fs-directory)
4. Link parent → child
5. Attempt removal of parent
6. Verify command fails with appropriate error message

Existing Scenarios: All existing resource CLI scenarios continue to pass

"Remove a resource" — Still passes (no links, removal succeeds)
"Remove a non-existent resource" — Still passes (error handling unchanged)
All other resource CLI scenarios unaffected

Coverage Metrics

Before: Resource removal guard checked wrong table (ResourceEdgeModel), allowing deletion of linked resources

After:

Resource removal guard correctly checks ResourceLinkModel
--update flag properly cleans up validated links before re-registering
New scenario provides integration-level test coverage for the guard

Code Paths Covered:

✅ resource_remove() with active DAG links (new scenario)
✅ resource_remove() without links (existing scenario)
✅ resource_add(..., --update) with existing resource (existing code path, now with proper link cleanup)

Test Issues Encountered

None. The implementation is straightforward and all tests pass.

Risk Mitigations

Risk 1: Breaking Change to Removal Behavior

Risk: Resources that could previously be removed (despite having links) will now be blocked.

Mitigation:

This is the intended fix; the previous behavior was a bug
Users who relied on the buggy behavior will need to unlink resources before removal
Error message is clear and actionable: "Cannot remove resource '...': N edge(s) still reference it."

Risk 2: Data Integrity During `--update`

Risk: Explicit link deletion during --update could leave the database in an inconsistent state if the operation fails.

Mitigation:

All deletions are wrapped in a transaction with proper rollback on error
synchronize_session="fetch" ensures SQLAlchemy session consistency
Follows existing patterns used throughout the codebase

Risk 3: Performance Impact

Risk: Querying ResourceLinkModel instead of ResourceEdgeModel could be slower.

Mitigation:

Both tables are indexed on parent_id and child_id
Query is a simple count; performance impact is negligible
No additional queries added; only the table changed

Risk 4: Orphaned Rows in `ResourceEdgeModel`

Risk: ResourceEdgeModel rows may accumulate if auto-discovery is still active.

Mitigation:

This fix does not change auto-discovery behavior
If ResourceEdgeModel rows are a concern, that's a separate issue (follow-on work)
Current fix focuses on the user-facing DAG operations (which use ResourceLinkModel)

Summary

This fix corrects a critical bug where agents resource remove was checking the wrong database table for active DAG links. By changing the guard from ResourceEdgeModel to ResourceLinkModel, resources with active user-created links are now properly protected from accidental deletion. The same fix is applied to resource_add(..., --update) to ensure proper cleanup of validated links when re-registering a resource. Integration-level test coverage is provided via a new Behave scenario that validates the complete workflow.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: issue-note-writer

## Implementation Summary This fix addresses a critical bug in the resource removal and update operations where the wrong database table was being checked for active DAG links. The implementation corrects both `resource_remove()` and `resource_add(..., --update)` to properly guard against deletion of resources with active validated links. ### What Was Implemented **Modified Files:** - `src/cleveragents/cli/commands/resource.py` — Updated `resource_remove()` and `resource_add()` functions - `features/resource_cli.feature` — Added new Behave scenario for guarded removal - `features/steps/resource_cli_steps.py` — Added supporting step definition **Key Changes:** 1. **`resource_remove()` function** (lines 1382–1403): - Changed edge check from `ResourceEdgeModel` to `ResourceLinkModel` - Now queries `ResourceLinkModel` table (which contains user-created validated links) - Properly blocks removal of resources with active parent/child relationships - Error message remains user-friendly: `"Cannot remove resource '...': N edge(s) still reference it."` 2. **`resource_add(..., --update)` function** (lines 714–747): - Added explicit deletion of `ResourceLinkModel` rows before dropping existing resource - Clears validated links first, then removes `ResourceEdgeModel` rows - Prevents orphaned link rows in the database when re-registering a resource - Maintains transaction safety with proper rollback on error 3. **New Behave Scenario** (`features/resource_cli.feature`, lines 134–142): - Scenario: "Resource remove fails when DAG links exist" - Tests the complete workflow: add resources → link them → attempt removal → verify guard - Validates that the command fails with appropriate error message 4. **Supporting Step Definition** (`features/steps/resource_cli_steps.py`, lines 375–380): - `step_link_resources()`: Links two resources using the registry service - Enables the new scenario to establish the precondition of active DAG links --- ## Design Decisions ### 1. **Why Check `ResourceLinkModel` Instead of `ResourceEdgeModel`?** **Decision:** Use `ResourceLinkModel` exclusively for the removal guard. **Rationale:** - `ResourceLinkModel` (table `resource_links`) represents **validated, user-created DAG links** created via `link_child()` / `unlink_child()` commands - `ResourceEdgeModel` (table `resource_edges`) represents **raw auto-discovered edges** managed by the auto-discovery infrastructure, never written to by user-facing DAG operations - The DAG service (`_resource_registry_dag.py`) uses `ResourceLinkModel` exclusively for all operations: `link_child()`, `unlink_child()`, `get_children()`, `get_parents()`, `get_resource_tree()` - Checking `ResourceEdgeModel` was a false guard that never caught user-created links **Alternatives Considered:** - Check both tables: Rejected because `ResourceEdgeModel` is not user-facing and would add unnecessary complexity - Add a new unified edge table: Rejected as out-of-scope; the schema is already defined and working correctly ### 2. **Why Clear Links Before Dropping Resource in `--update`?** **Decision:** Explicitly delete `ResourceLinkModel` rows before removing the resource. **Rationale:** - Prevents orphaned rows in `resource_links` table when re-registering a resource - Makes the operation explicit and auditable (no silent cascading deletes) - Maintains data integrity by ensuring all related records are cleaned up in the correct order - Aligns with defensive programming: explicit cleanup is safer than relying on FK cascades **Alternatives Considered:** - Rely on FK cascade: Rejected because it's silent and makes debugging harder - Skip link cleanup: Rejected because it leaves orphaned rows ### 3. **Why Add a Behave Scenario Instead of Unit Test?** **Decision:** Add integration-level Behave scenario to test the complete user workflow. **Rationale:** - Tests the actual CLI command behavior end-to-end - Validates the user experience (error message, command failure) - Exercises the full stack: CLI → service → database - Consistent with existing test patterns in `resource_cli.feature` - Catches regressions at the integration level where they matter most --- ## Discoveries and Assumptions ### Key Discoveries 1. **Two Separate Edge Tables Exist:** - The codebase maintains both `ResourceEdgeModel` and `ResourceLinkModel` for different purposes - This separation is intentional: auto-discovery edges vs. user-created links - The bug occurred because the guard checked the wrong table 2. **FK Cascade Behavior:** - `ResourceLinkModel` rows are protected by `ondelete="CASCADE"` on the `resources` table FK - When a resource is deleted, orphaned link rows are silently dropped - This silent cleanup made the bug difficult to detect (no error, just data loss) 3. **Resource Add with --Update Pattern:** - The `--update` flag removes the existing resource and re-registers it - This pattern requires careful cleanup of all related records (links, edges, resource) - The order matters: links → edges → resource ### Assumptions Made 1. **`ResourceLinkModel` is the Source of Truth for User-Created Links:** - Assumption: All user-facing DAG operations use `ResourceLinkModel` - Verified by reviewing `_resource_registry_dag.py` (lines 104, 138, 173, 210, 245, 301) 2. **Error Message Format is Acceptable:** - Assumption: Existing error message "Cannot remove resource '...': N edge(s) still reference it." is appropriate - No change to message text; only the table being checked was corrected 3. **Transaction Safety is Sufficient:** - Assumption: SQLAlchemy's `synchronize_session="fetch"` is sufficient for maintaining session consistency - Existing pattern used throughout the codebase ### Open Questions / Follow-On Work 1. **Should `ResourceEdgeModel` Be Removed?** - The `resource_edges` table is never written to by user-facing code - Consider deprecating it if auto-discovery is no longer used - Out of scope for this fix 2. **Should the Guard Be Moved to the Service Layer?** - Currently the guard is in the CLI command (`resource_remove()`) - Consider moving to `ResourceRegistryService.remove_resource()` for consistency - Would prevent accidental removal via direct service calls - Out of scope for this fix --- ## Code Locations **Commit Hash:** `8a87675a58e33f53c503651a37c53cf2b0d675c1` ### Modified Code 1. **`cleveragents.cli.commands.resource.resource_remove()`** - Lines 1382–1403 - Changed: `session.query(ResourceEdgeModel)` → `session.query(ResourceLinkModel)` - Updated filter conditions to use `ResourceLinkModel.parent_id` and `ResourceLinkModel.child_id` 2. **`cleveragents.cli.commands.resource.resource_add()`** - Lines 714–747 (the `--update` branch) - Added: Explicit deletion of `ResourceLinkModel` rows before `ResourceEdgeModel` and `ResourceModel` - Imports: Added `ResourceLinkModel` to the import statement 3. **`features.resource_cli` (Behave feature)** - Lines 134–142 - New scenario: "Resource remove fails when DAG links exist" - Tests: Resource creation → linking → removal attempt → guard verification 4. **`features.steps.resource_cli_steps.step_link_resources()`** - Lines 375–380 - New step definition: `@given('the resources "{parent}" and "{child}" are linked in the registry')` - Implementation: Calls `service.link_child(parent, child)` to establish precondition ### Related Code (Not Modified) - `cleveragents.infrastructure.database.models.ResourceLinkModel` — The correct table for user-created links - `cleveragents.application.services.resource_registry_service.ResourceRegistryService` — Service layer (no changes needed) - `cleveragents.resource._resource_registry_dag` — DAG operations (already uses `ResourceLinkModel` correctly) --- ## Workarounds and Deviations ### No Workarounds Applied This fix is a straightforward correction of the bug. No workarounds were necessary. ### No Deviations from Plan The implementation follows the fix sketch provided in the issue exactly: - Changed `ResourceEdgeModel` to `ResourceLinkModel` in `resource_remove()` - Applied the same fix to `resource_add(..., --update)` - Added test coverage via Behave scenario ### Follow-On Work Identified 1. **Deprecation of `ResourceEdgeModel`:** - If auto-discovery is no longer used, consider removing the `resource_edges` table - Would simplify the schema and eliminate confusion 2. **Service Layer Guard:** - Consider moving the removal guard to `ResourceRegistryService.remove_resource()` - Would provide defense-in-depth and prevent accidental removal via direct service calls 3. **Audit Logging:** - Consider adding audit logs when resources with links are attempted to be removed - Would help with debugging and compliance --- ## Test Results ### Behave Scenario Execution **Test Command:** `nox -s unit_tests -- features/resource_cli.feature` **New Scenario:** "Resource remove fails when DAG links exist" - **Status:** ✅ Passes - **Coverage:** Tests the complete workflow: 1. Bootstrap built-in types 2. Add parent resource (git-checkout) 3. Add child resource (fs-directory) 4. Link parent → child 5. Attempt removal of parent 6. Verify command fails with appropriate error message **Existing Scenarios:** All existing resource CLI scenarios continue to pass - "Remove a resource" — Still passes (no links, removal succeeds) - "Remove a non-existent resource" — Still passes (error handling unchanged) - All other resource CLI scenarios unaffected ### Coverage Metrics **Before:** Resource removal guard checked wrong table (`ResourceEdgeModel`), allowing deletion of linked resources **After:** - Resource removal guard correctly checks `ResourceLinkModel` - `--update` flag properly cleans up validated links before re-registering - New scenario provides integration-level test coverage for the guard **Code Paths Covered:** - ✅ `resource_remove()` with active DAG links (new scenario) - ✅ `resource_remove()` without links (existing scenario) - ✅ `resource_add(..., --update)` with existing resource (existing code path, now with proper link cleanup) ### Test Issues Encountered None. The implementation is straightforward and all tests pass. --- ## Risk Mitigations ### Risk 1: Breaking Change to Removal Behavior **Risk:** Resources that could previously be removed (despite having links) will now be blocked. **Mitigation:** - This is the intended fix; the previous behavior was a bug - Users who relied on the buggy behavior will need to unlink resources before removal - Error message is clear and actionable: "Cannot remove resource '...': N edge(s) still reference it." ### Risk 2: Data Integrity During `--update` **Risk:** Explicit link deletion during `--update` could leave the database in an inconsistent state if the operation fails. **Mitigation:** - All deletions are wrapped in a transaction with proper rollback on error - `synchronize_session="fetch"` ensures SQLAlchemy session consistency - Follows existing patterns used throughout the codebase ### Risk 3: Performance Impact **Risk:** Querying `ResourceLinkModel` instead of `ResourceEdgeModel` could be slower. **Mitigation:** - Both tables are indexed on `parent_id` and `child_id` - Query is a simple count; performance impact is negligible - No additional queries added; only the table changed ### Risk 4: Orphaned Rows in `ResourceEdgeModel` **Risk:** `ResourceEdgeModel` rows may accumulate if auto-discovery is still active. **Mitigation:** - This fix does not change auto-discovery behavior - If `ResourceEdgeModel` rows are a concern, that's a separate issue (follow-on work) - Current fix focuses on the user-facing DAG operations (which use `ResourceLinkModel`) --- ## Summary This fix corrects a critical bug where `agents resource remove` was checking the wrong database table for active DAG links. By changing the guard from `ResourceEdgeModel` to `ResourceLinkModel`, resources with active user-created links are now properly protected from accidental deletion. The same fix is applied to `resource_add(..., --update)` to ensure proper cleanup of validated links when re-registering a resource. Integration-level test coverage is provided via a new Behave scenario that validates the complete workflow. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: issue-note-writer

HAL9000 referenced this issue from a commit

2026-04-09 23:10:25 +00:00

fix(resource): prevent unlinkable resources from being removed

~~HAL9000 referenced this issue 2026-04-09 23:15:49 +00:00~~

fix(resource): fix resource remove to check correct edge table #6676

HAL9000 referenced this issue from a pull request that will close it,

2026-04-09 23:17:23 +00:00

fix(resource): fix resource remove to check correct edge table #6676

HAL9000 added

and removed

labels

2026-04-09 23:58:17 +00:00

HAL9000 commented

2026-04-10 00:16:42 +00:00

Author

Owner

PR Opened for Issue Resolution

PR #6676 has been opened on branch fix/issue-6329-resource-remove-edge-table to address this issue.

I will monitor the pull request and handle all review feedback until it is successfully merged.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: issue-note-writer

## PR Opened for Issue Resolution PR #6676 has been opened on branch `fix/issue-6329-resource-remove-edge-table` to address this issue. I will monitor the pull request and handle all review feedback until it is successfully merged. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: issue-note-writer

HAL9000 added this to the v3.2.0 milestone

2026-04-10 00:19:20 +00:00

HAL9000 referenced this issue

2026-04-10 02:03:59 +00:00

fix(resource): fix resource remove to check correct edge table #6676

HAL9000 added a new dependency

2026-04-10 03:20:33 +00:00

#6676 fix(resource): fix resource remove to check correct edge table

HAL9000 referenced this issue from a commit

2026-04-10 19:05:21 +00:00

test(resource): add robot coverage for removal guard

HAL9000 referenced this issue from a commit