UAT: `SkillRegistry` calls inconsistent method names on `_tool_registry` — `get_tool()` vs `get()` causes `AttributeError` at runtime #2475

New issue

Open

opened 2026-04-03 18:36:43 +00:00 by freemo · 2 comments

freemo commented

2026-04-03 18:36:43 +00:00

Owner

Metadata

Branch: fix/skill-registry-tool-registry-method-consistency
Commit Message: fix(skills): standardize SkillRegistry._tool_registry method calls to resolve AttributeError
Milestone: v3.2.0
Parent Epic: #392

Description

The SkillRegistry class in src/cleveragents/skills/registry.py calls two different method names on the same _tool_registry object across three methods, causing AttributeError at runtime depending on which concrete registry implementation is injected.

Inconsistent Call Sites

Method	Line	Call
`validate_plan()`	274	`self._tool_registry.get_tool(entry.name)`
`refresh()`	325	`self._tool_registry.get(entry.name)`
`validate_skill()`	371	`self._tool_registry.get_tool(ref)`

Root Cause

The _tool_registry field is typed as Any | None and can be either:

ToolRegistry (tool/registry.py): exposes get() but NOT get_tool()
ToolRegistryService (application/services/tool_registry_service.py): exposes get_tool() but NOT get()

This means:

If a ToolRegistry is injected → validate_plan() and validate_skill() raise AttributeError: 'ToolRegistry' object has no attribute 'get_tool'
If a ToolRegistryService is injected → refresh() raises AttributeError: 'ToolRegistryService' object has no attribute 'get'

Expected Behavior (from spec)

Per docs/specification.md, the SkillRegistry should consistently use a single, well-defined method name to look up tools from the tool registry. The _tool_registry field should be typed to a concrete interface (not Any) to enforce this contract at type-check time and prevent future regressions.

Steps to Reproduce

Create a SkillRegistry with a ToolRegistry instance as tool_registry
Call validate_plan() or validate_skill() with a skill that has tool references
Observe: AttributeError: 'ToolRegistry' object has no attribute 'get_tool'

Code Locations

src/cleveragents/skills/registry.py line 274 — get_tool() call in validate_plan()
src/cleveragents/skills/registry.py line 325 — get() call in refresh()
src/cleveragents/skills/registry.py line 371 — get_tool() call in validate_skill()

Subtasks

Audit ToolRegistry and ToolRegistryService to determine the canonical method name for tool lookup (or define a shared ToolRegistryProtocol interface)
Define a ToolRegistryProtocol (or use an existing ABC) with a single consistent method signature for tool lookup
Update SkillRegistry._tool_registry field type annotation from Any | None to the protocol/interface type
Standardize all three call sites (validate_plan() line 274, refresh() line 325, validate_skill() line 371) to use the same method name
Ensure ToolRegistry and ToolRegistryService both implement the agreed method name (add alias or rename as needed)
Write BDD scenarios covering validate_plan(), refresh(), and validate_skill() with both ToolRegistry and ToolRegistryService injected
Run nox -e typecheck — confirm no Any-typed suppression remains on _tool_registry
Run nox (all default sessions) and fix any errors
Verify coverage >= 97% via nox -e coverage_report

Definition of Done

All three call sites in SkillRegistry use the same method name for tool lookup
_tool_registry is typed to a concrete protocol/interface, not Any
ToolRegistry and ToolRegistryService both satisfy the protocol
BDD tests cover all three affected methods with both registry implementations
nox -e typecheck passes with zero type errors on the affected files
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-registry-tool-registry-method-consistency` - **Commit Message**: `fix(skills): standardize SkillRegistry._tool_registry method calls to resolve AttributeError` - **Milestone**: v3.2.0 - **Parent Epic**: #392 ## Description The `SkillRegistry` class in `src/cleveragents/skills/registry.py` calls two **different** method names on the same `_tool_registry` object across three methods, causing `AttributeError` at runtime depending on which concrete registry implementation is injected. ### Inconsistent Call Sites | Method | Line | Call | |---|---|---| | `validate_plan()` | 274 | `self._tool_registry.get_tool(entry.name)` | | `refresh()` | 325 | `self._tool_registry.get(entry.name)` | | `validate_skill()` | 371 | `self._tool_registry.get_tool(ref)` | ### Root Cause The `_tool_registry` field is typed as `Any | None` and can be either: - **`ToolRegistry`** (`tool/registry.py`): exposes `get()` but **NOT** `get_tool()` - **`ToolRegistryService`** (`application/services/tool_registry_service.py`): exposes `get_tool()` but **NOT** `get()` This means: - If a `ToolRegistry` is injected → `validate_plan()` and `validate_skill()` raise `AttributeError: 'ToolRegistry' object has no attribute 'get_tool'` - If a `ToolRegistryService` is injected → `refresh()` raises `AttributeError: 'ToolRegistryService' object has no attribute 'get'` ### Expected Behavior (from spec) Per `docs/specification.md`, the `SkillRegistry` should consistently use a single, well-defined method name to look up tools from the tool registry. The `_tool_registry` field should be typed to a concrete interface (not `Any`) to enforce this contract at type-check time and prevent future regressions. ### Steps to Reproduce 1. Create a `SkillRegistry` with a `ToolRegistry` instance as `tool_registry` 2. Call `validate_plan()` or `validate_skill()` with a skill that has tool references 3. Observe: `AttributeError: 'ToolRegistry' object has no attribute 'get_tool'` ### Code Locations - `src/cleveragents/skills/registry.py` line 274 — `get_tool()` call in `validate_plan()` - `src/cleveragents/skills/registry.py` line 325 — `get()` call in `refresh()` - `src/cleveragents/skills/registry.py` line 371 — `get_tool()` call in `validate_skill()` ## Subtasks - [ ] Audit `ToolRegistry` and `ToolRegistryService` to determine the canonical method name for tool lookup (or define a shared `ToolRegistryProtocol` interface) - [ ] Define a `ToolRegistryProtocol` (or use an existing ABC) with a single consistent method signature for tool lookup - [ ] Update `SkillRegistry._tool_registry` field type annotation from `Any | None` to the protocol/interface type - [ ] Standardize all three call sites (`validate_plan()` line 274, `refresh()` line 325, `validate_skill()` line 371) to use the same method name - [ ] Ensure `ToolRegistry` and `ToolRegistryService` both implement the agreed method name (add alias or rename as needed) - [ ] Write BDD scenarios covering `validate_plan()`, `refresh()`, and `validate_skill()` with both `ToolRegistry` and `ToolRegistryService` injected - [ ] Run `nox -e typecheck` — confirm no `Any`-typed suppression remains on `_tool_registry` - [ ] Run `nox` (all default sessions) and fix any errors - [ ] Verify coverage >= 97% via `nox -e coverage_report` ## Definition of Done - [ ] All three call sites in `SkillRegistry` use the same method name for tool lookup - [ ] `_tool_registry` is typed to a concrete protocol/interface, not `Any` - [ ] `ToolRegistry` and `ToolRegistryService` both satisfy the protocol - [ ] BDD tests cover all three affected methods with both registry implementations - [ ] `nox -e typecheck` passes with zero type errors on the affected files - [ ] 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:36:48 +00:00

freemo added a new dependency

2026-04-03 18:37:29 +00:00

#392 Epic: Actor YAML & Compiler

freemo commented

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

Author

Owner

Issue triaged by project owner:

State: Verified
MoSCoW: Should Have — Spec compliance or quality improvement that should be included in the milestone.

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 that should be included in the milestone. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo removed this from the v3.2.0 milestone

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

freemo commented

2026-04-06 22:30:40 +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.