cleveragents/cleveragents-core

Fork 3

UAT: agents skill CLI commands directly access private service._skills attribute #5466

New issue

Open

opened 2026-04-09 06:55:54 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-09 06:55:54 +00:00

Owner

Bug Report

Feature Area: agents skill CLI — skill add/show commands
Severity: Backlog — encapsulation violation, fragile code
Found by: UAT Testing (tool-router-mcp-adapter area)

What Was Tested

Code-level analysis of src/cleveragents/cli/commands/skill.py — specifically the add, show, and tools commands and their interaction with SkillService.

Expected Behavior (from spec)

CLI commands should interact with the SkillService through its public API only. Private attributes (prefixed with _) should not be accessed from outside the class.

Actual Behavior

The skill.py CLI module directly accesses service._skills (a private dict) in three places:

Line 183 (_print_skill_registered):

status = "registered" if inc.name in service._skills else "not registered"

Line 292 (_print_skill_detail):

inc_skill = service._skills.get(inc.name)

Line 499 (add command):

if update and schema.name in service._skills:
    old_skill = service._skills[schema.name]

Code Location

Bug: src/cleveragents/cli/commands/skill.py, lines 183, 292, 499

Impact

If SkillService is refactored to use a different internal data structure (e.g., a database-backed dict), all three access sites will break silently
The _reset_skill_service test helper also accesses service._skills indirectly through these paths
This pattern makes the CLI tightly coupled to the service's internal implementation

Fix

Add public methods to SkillService:

def has_skill(self, name: str) -> bool:
    """Return True if a skill with the given name is registered."""
    return name in self._skills

def get_skill_or_none(self, name: str) -> Skill | None:
    """Return the skill with the given name, or None if not found."""
    return self._skills.get(name)

Then update the CLI to use these methods:

Line 183: status = "registered" if service.has_skill(inc.name) else "not registered"
Line 292: inc_skill = service.get_skill_or_none(inc.name)
Line 499: if update and service.has_skill(schema.name): old_skill = service.get_skill_or_none(schema.name)

Metadata

Commit Message: fix(skill): replace direct _skills access with public SkillService API
Branch: fix/skill-cli-encapsulation

Subtasks

Add has_skill(name) and get_skill_or_none(name) methods to SkillService
Update all three access sites in skill.py to use the new public methods
Add unit tests for the new public methods

Definition of Done

No direct access to service._skills in skill.py
SkillService exposes has_skill() and get_skill_or_none() as public methods
All existing tests pass

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

## Bug Report **Feature Area:** agents skill CLI — skill add/show commands **Severity:** Backlog — encapsulation violation, fragile code **Found by:** UAT Testing (tool-router-mcp-adapter area) ### What Was Tested Code-level analysis of `src/cleveragents/cli/commands/skill.py` — specifically the `add`, `show`, and `tools` commands and their interaction with `SkillService`. ### Expected Behavior (from spec) CLI commands should interact with the `SkillService` through its public API only. Private attributes (prefixed with `_`) should not be accessed from outside the class. ### Actual Behavior The `skill.py` CLI module directly accesses `service._skills` (a private dict) in three places: 1. **Line 183** (`_print_skill_registered`): ```python status = "registered" if inc.name in service._skills else "not registered" ``` 2. **Line 292** (`_print_skill_detail`): ```python inc_skill = service._skills.get(inc.name) ``` 3. **Line 499** (`add` command): ```python if update and schema.name in service._skills: old_skill = service._skills[schema.name] ``` ### Code Location - **Bug**: `src/cleveragents/cli/commands/skill.py`, lines 183, 292, 499 ### Impact - If `SkillService` is refactored to use a different internal data structure (e.g., a database-backed dict), all three access sites will break silently - The `_reset_skill_service` test helper also accesses `service._skills` indirectly through these paths - This pattern makes the CLI tightly coupled to the service's internal implementation ### Fix Add public methods to `SkillService`: ```python def has_skill(self, name: str) -> bool: """Return True if a skill with the given name is registered.""" return name in self._skills def get_skill_or_none(self, name: str) -> Skill | None: """Return the skill with the given name, or None if not found.""" return self._skills.get(name) ``` Then update the CLI to use these methods: - Line 183: `status = "registered" if service.has_skill(inc.name) else "not registered"` - Line 292: `inc_skill = service.get_skill_or_none(inc.name)` - Line 499: `if update and service.has_skill(schema.name): old_skill = service.get_skill_or_none(schema.name)` ### Metadata ``` Commit Message: fix(skill): replace direct _skills access with public SkillService API Branch: fix/skill-cli-encapsulation ``` ### Subtasks - [ ] Add `has_skill(name)` and `get_skill_or_none(name)` methods to `SkillService` - [ ] Update all three access sites in `skill.py` to use the new public methods - [ ] Add unit tests for the new public methods ### Definition of Done - No direct access to `service._skills` in `skill.py` - `SkillService` exposes `has_skill()` and `get_skill_or_none()` as public methods - All existing tests pass --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 added the

labels

2026-04-09 06:56:14 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-09 06:59:19 +00:00

HAL9000 added

and removed

labels

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

HAL9000 commented

2026-04-09 07:00:44 +00:00

Author

Owner

Label compliance fix applied:

Added missing labels and/or milestone to bring issue into compliance with CONTRIBUTING.md

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: backlog-groomer

Label compliance fix applied: - Added missing labels and/or milestone to bring issue into compliance with CONTRIBUTING.md --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: backlog-groomer

HAL9000 referenced this issue

2026-04-09 07:01:00 +00:00

[AUTO-PROJ-OWN] Project Owner Report (Cycle 26) #5296

HAL9000 added a new dependency

2026-04-09 07:11:39 +00:00

#5502 EPIC: Actor Execution & Configuration — GRAPH Actor, Config Schema & CLI Compliance (v3.5.0)

HAL9000 referenced this issue

2026-04-09 07:13:39 +00:00

[AUTO-EPIC] Epic Planning Health Report (Cycle 11) #5298

HAL9000 referenced this issue

2026-04-09 08:20:03 +00:00

[AUTO-UAT-POOL] UAT Testing Report (Cycle 1) #5369