fix(skill): persist skill registration to database after add #640

Merged

freemo merged 1 commit from fix/skill-add-not-registering into master

2026-03-08 22:06:15 +00:00

freemo commented

2026-03-08 02:58:11 +00:00

Owner

Summary

Fixes #620 — agents skill add followed by agents skill list now correctly shows the added skill.

Closes #620

Root Cause

The CLI skill.py used SkillService which stored skills in a pure in-memory OrderedDict. Each CLI process created a new empty SkillService, so data added during skill add was lost when the process exited.

Meanwhile, the codebase already had:

SkillModel — full SQLAlchemy model for the skills table (with SkillItemModel children)
SkillRepository — database-backed CRUD with session-factory pattern
SkillRegistryService — service layer wrapping the repository
Alembic migrations (c0_001, c0_002, m4_002) for the skills tables

The wiring between the CLI and the database layer was simply missing.

Changes

`src/cleveragents/application/services/skill_service.py`

Added optional skill_repo and session_factory parameters to __init__
When skill_repo is provided, the constructor pre-loads existing skills from DB
add_skill() now persists to DB via skill_repo.create() / update() + session.commit()
remove_skill() now persists deletion via skill_repo.delete() + session.commit()
Fallback to pure in-memory mode when no repo is provided (backwards compatible)

`src/cleveragents/cli/commands/skill.py`

_get_skill_service() now creates a DB-backed service following the same engine -> sessionmaker -> repository pattern used by tool.py
_reset_skill_service() now installs a fresh in-memory SkillService (instead of None)

Tests

Behave BDD: features/skill_add_persist.feature — 4 scenarios (round-trip, multiple, duplicates, remove)
Robot Framework: robot/skill_add_persist.robot — 3 integration smoke tests
ASV Benchmark: benchmarks/skill_add_persist_bench.py — add + list performance

Quality Gates

nox -s lint — PASSED
nox -s typecheck — 0 errors, 0 warnings
nox -s unit_tests — 9,110 scenarios, 0 failures
nox -s coverage_report — 97%

ISSUES CLOSED: #620

## Summary Fixes #620 — `agents skill add` followed by `agents skill list` now correctly shows the added skill. Closes #620 ## Root Cause The CLI `skill.py` used `SkillService` which stored skills in a pure in-memory `OrderedDict`. Each CLI process created a new empty `SkillService`, so data added during `skill add` was lost when the process exited. Meanwhile, the codebase already had: - `SkillModel` — full SQLAlchemy model for the `skills` table (with `SkillItemModel` children) - `SkillRepository` — database-backed CRUD with session-factory pattern - `SkillRegistryService` — service layer wrapping the repository - Alembic migrations (`c0_001`, `c0_002`, `m4_002`) for the skills tables The wiring between the CLI and the database layer was simply missing. ## Changes ### `src/cleveragents/application/services/skill_service.py` - Added optional `skill_repo` and `session_factory` parameters to `__init__` - When `skill_repo` is provided, the constructor pre-loads existing skills from DB - `add_skill()` now persists to DB via `skill_repo.create()` / `update()` + `session.commit()` - `remove_skill()` now persists deletion via `skill_repo.delete()` + `session.commit()` - Fallback to pure in-memory mode when no repo is provided (backwards compatible) ### `src/cleveragents/cli/commands/skill.py` - `_get_skill_service()` now creates a DB-backed service following the same `engine -> sessionmaker -> repository` pattern used by `tool.py` - `_reset_skill_service()` now installs a fresh in-memory `SkillService` (instead of `None`) ## Tests - **Behave BDD**: `features/skill_add_persist.feature` — 4 scenarios (round-trip, multiple, duplicates, remove) - **Robot Framework**: `robot/skill_add_persist.robot` — 3 integration smoke tests - **ASV Benchmark**: `benchmarks/skill_add_persist_bench.py` — add + list performance ## Quality Gates - `nox -s lint` — PASSED - `nox -s typecheck` — 0 errors, 0 warnings - `nox -s unit_tests` — 9,110 scenarios, 0 failures - `nox -s coverage_report` — 97% ISSUES CLOSED: #620

Rows
Columns