cleveragents/cleveragents-core

Fork 3

UAT: `agents skill add` does not detect circular includes at registration time — spec requires cycle detection on add #2579

New issue

Open

opened 2026-04-03 18:59:05 +00:00 by freemo · 2 comments

freemo commented

2026-04-03 18:59:05 +00:00

Owner

Metadata

Branch: fix/skill-add-cycle-detection
Commit Message: fix(skills): detect circular includes at skill registration time
Milestone: v3.2.0
Parent Epic: #392

Bug Report

Feature Area

Actor, Skill & Tool Abstraction — agents skill add CLI command / SkillService.add_skill()

What Was Tested

Code analysis of src/cleveragents/application/services/skill_service.py — the add_skill() method, and src/cleveragents/domain/models/core/skill.py — the SkillResolver cycle detection logic.

Expected Behavior (from spec)

The specification (docs/specification.md, section "Rules for skill composition") states:

1. Circular includes are forbidden. The system validates the include graph at registration time and rejects cycles.

When a skill is added that would create a circular include chain (e.g., local/a includes local/b which includes local/a), the agents skill add command must detect this cycle and reject the registration with a clear error.

Actual Behavior

SkillService.add_skill() (src/cleveragents/application/services/skill_service.py, lines 73-120) does not run cycle detection at registration time. The method stores the skill in self._skills[name] without checking for circular include chains.

The SkillResolver class (src/cleveragents/domain/models/core/skill.py) does implement cycle detection via DFS in _resolve_recursive(), but this is only invoked when resolve_tools() is called — not at registration time.

Code Location

src/cleveragents/application/services/skill_service.py — add_skill() method (lines 73-120): no cycle detection
src/cleveragents/domain/models/core/skill.py — SkillResolver._resolve_recursive(): cycle detection exists but is not called at add time

Steps to Reproduce

name: local/skill-a
description: Skill A
includes:
  - name: local/skill-b

agents skill add --config skill-a.yaml

name: local/skill-b
description: Skill B
includes:
  - name: local/skill-a

agents skill add --config skill-b.yaml

Observe: Both skills register successfully, creating a circular include chain
Expected: Step 2 should fail with "Cycle detected in skill includes: local/skill-b -> local/skill-a -> local/skill-b"

Impact

Severity: High
Priority: High
Circular includes cause infinite recursion / ValueError when resolve_tools() is called later (e.g., during agents skill tools, agents skill refresh, or actor execution)
The error occurs at runtime rather than at registration time, making it harder to diagnose
Violates the spec's explicit guarantee that the include graph is validated at registration time

Subtasks

In SkillService.add_skill(), after storing the new skill, call SkillResolver().resolve_tools() on the new skill with the current self._skills lookup to trigger cycle detection
If a ValueError with "Cycle detected" is raised, remove the newly added skill from self._skills and re-raise as a user-facing error
Add Behave tests for the cycle detection failure case (A→B→A, A→B→C→A)
Ensure the check also runs when update=True
Verify coverage >= 97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

agents skill add fails with a clear error when a circular include chain is detected
Error message shows the full cycle path (e.g., "local/a -> local/b -> local/a")
All subtasks above are completed and checked off
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly
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%

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

## Metadata - **Branch**: `fix/skill-add-cycle-detection` - **Commit Message**: `fix(skills): detect circular includes at skill registration time` - **Milestone**: v3.2.0 - **Parent Epic**: #392 ## Bug Report ### Feature Area Actor, Skill & Tool Abstraction — `agents skill add` CLI command / `SkillService.add_skill()` ### What Was Tested Code analysis of `src/cleveragents/application/services/skill_service.py` — the `add_skill()` method, and `src/cleveragents/domain/models/core/skill.py` — the `SkillResolver` cycle detection logic. ### Expected Behavior (from spec) The specification (`docs/specification.md`, section "Rules for skill composition") states: > **1. Circular includes are forbidden.** The system validates the include graph at registration time and rejects cycles. When a skill is added that would create a circular include chain (e.g., `local/a` includes `local/b` which includes `local/a`), the `agents skill add` command must detect this cycle and reject the registration with a clear error. ### Actual Behavior `SkillService.add_skill()` (`src/cleveragents/application/services/skill_service.py`, lines 73-120) does **not** run cycle detection at registration time. The method stores the skill in `self._skills[name]` without checking for circular include chains. The `SkillResolver` class (`src/cleveragents/domain/models/core/skill.py`) **does** implement cycle detection via DFS in `_resolve_recursive()`, but this is only invoked when `resolve_tools()` is called — not at registration time. ### Code Location - `src/cleveragents/application/services/skill_service.py` — `add_skill()` method (lines 73-120): no cycle detection - `src/cleveragents/domain/models/core/skill.py` — `SkillResolver._resolve_recursive()`: cycle detection exists but is not called at add time ### Steps to Reproduce 1. Register skill A: ```yaml name: local/skill-a description: Skill A includes: - name: local/skill-b ``` `agents skill add --config skill-a.yaml` 2. Register skill B that creates a cycle: ```yaml name: local/skill-b description: Skill B includes: - name: local/skill-a ``` `agents skill add --config skill-b.yaml` 3. Observe: Both skills register successfully, creating a circular include chain 4. Expected: Step 2 should fail with "Cycle detected in skill includes: local/skill-b -> local/skill-a -> local/skill-b" ### Impact - **Severity**: High - **Priority**: High - Circular includes cause infinite recursion / `ValueError` when `resolve_tools()` is called later (e.g., during `agents skill tools`, `agents skill refresh`, or actor execution) - The error occurs at runtime rather than at registration time, making it harder to diagnose - Violates the spec's explicit guarantee that the include graph is validated at registration time ## Subtasks - [ ] In `SkillService.add_skill()`, after storing the new skill, call `SkillResolver().resolve_tools()` on the new skill with the current `self._skills` lookup to trigger cycle detection - [ ] If a `ValueError` with "Cycle detected" is raised, remove the newly added skill from `self._skills` and re-raise as a user-facing error - [ ] Add Behave tests for the cycle detection failure case (A→B→A, A→B→C→A) - [ ] Ensure the check also runs when `update=True` - [ ] Verify coverage >= 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done - [ ] `agents skill add` fails with a clear error when a circular include chain is detected - [ ] Error message shows the full cycle path (e.g., "local/a -> local/b -> local/a") - [ ] All subtasks above are completed and checked off - [ ] A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation - [ ] The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly - [ ] 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% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added this to the v3.2.0 milestone

2026-04-03 18:59:20 +00:00

freemo commented

2026-04-03 19:01:19 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
MoSCoW: Should Have — Spec compliance or quality improvement.

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

Issue triaged by project owner: - **State**: Verified - **MoSCoW**: Should Have — Spec compliance or quality improvement. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo added a new dependency

2026-04-03 19:02:15 +00:00

#392 Epic: Actor YAML & Compiler

freemo referenced this issue

2026-04-03 19:03:52 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 1591 — v3.7.0 #2593

freemo added a new dependency

2026-04-03 19:08:44 +00:00

#2597 fix(ci): restore all CI quality gates to passing on master — no suppression, no bypasses

freemo referenced this issue

2026-04-03 19:14:03 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 1601 — v3.7.0 #2603

freemo referenced this issue

2026-04-03 19:24:12 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 1611 — v3.7.0 #2605

freemo referenced this issue

2026-04-03 19:34:18 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 1621 — v3.7.0 #2607

freemo referenced this issue

2026-04-03 19:44:25 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 1631 — v3.7.0 #2613

freemo referenced this issue

2026-04-05 08:15:03 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094

freemo referenced this issue

2026-04-05 20:29:17 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3572

freemo removed this from the v3.2.0 milestone

2026-04-06 22:29:47 +00:00

freemo commented

2026-04-06 22:30:02 +00:00

Author

Owner

This issue has been moved to the backlog as part of an aggressive grooming of the v3.2.0 milestone. It has been deemed non-critical for the minimal viability of the milestone and will be addressed in a future release.

freemo removed a dependency

2026-04-14 22:18:54 +00:00

#2597 fix(ci): restore all CI quality gates to passing on master — no suppression, no bypasses