forked from cleveragents/cleveragents-core
freemo
1f94b7e8d5
- Implemented a pre-registration validation in SkillService.add_skill() to ensure all skills listed in the includes field are already registered in self._skills before accepting the new skill registration. This prevents registering a skill that depends on non-existent skills. - The validation applies to both the initial add and when update=True (re-registration), ensuring dependencies are consistently enforced across both creation and update flows. - If any included skills are missing, a clear ValueError is raised listing all missing skill names with guidance to register them first, providing complete visibility rather than failing on the first missing item. - Added new Behave feature file features/skill_add_include_validation.feature with 8 scenarios covering: single missing include, multiple missing includes, partial includes, all includes registered (success), update with missing include, and service-level assertions. Also added step definitions in features/steps/skill_add_include_validation_steps.py. - Updated features/skill_cli.feature to pre-register local/file-reader in scenarios that use the full skill YAML (which includes local/file-reader) to reflect the new validation behavior. - Updated features/steps/skill_cli_coverage_r3_steps.py to pre-register local/new-include in the update-with-different-includes scenario to align with the updated validation flow. - Rationale and design decisions: - Validation is placed in the service layer (SkillService.add_skill()) rather than the CLI layer to ensure consistent enforcement regardless of how the service is invoked. - The error message reports all missing includes at once for better user guidance and faster remediation. - Tests that relied on the full skill YAML were updated to pre-register dependencies to accurately reflect the validation changes and maintain test integrity. ISSUES CLOSED: #2555
62 lines
3.4 KiB
Gherkin
62 lines
3.4 KiB
Gherkin
@tdd_issue @tdd_issue_2555
|
|
Feature: skill add validates that included skills are registered
|
|
As a developer
|
|
I want `agents skill add` to fail immediately when included skills are not registered
|
|
So that I get a clear error at registration time rather than a confusing failure later
|
|
|
|
Background:
|
|
Given a skill CLI test runner
|
|
And the skill service is reset
|
|
|
|
# ───────────────────────────────────────────────────────
|
|
# Validation failure — unregistered includes
|
|
# ───────────────────────────────────────────────────────
|
|
|
|
Scenario: Add skill with unregistered include fails with clear error
|
|
Given a skill config YAML that includes "local/missing-dep" at a temp path
|
|
When I run skill CLI add with --config pointing to the YAML file
|
|
Then the skill CLI command should abort
|
|
And the skill CLI output should contain "not registered"
|
|
And the skill CLI output should contain "local/missing-dep"
|
|
|
|
Scenario: Add skill with multiple unregistered includes lists all missing skills
|
|
Given a skill config YAML that includes "local/dep-a" and "local/dep-b" at a temp path
|
|
When I run skill CLI add with --config pointing to the YAML file
|
|
Then the skill CLI command should abort
|
|
And the skill CLI output should contain "not registered"
|
|
And the skill CLI output should contain "local/dep-a"
|
|
And the skill CLI output should contain "local/dep-b"
|
|
|
|
Scenario: Add skill with partially registered includes fails listing only missing
|
|
Given the skill "local/dep-registered" is registered with tools
|
|
And a skill config YAML that includes "local/dep-registered" and "local/dep-missing" at a temp path
|
|
When I run skill CLI add with --config pointing to the YAML file
|
|
Then the skill CLI command should abort
|
|
And the skill CLI output should contain "local/dep-missing"
|
|
And the skill CLI output should not contain "local/dep-registered"
|
|
|
|
Scenario: Add skill with all includes registered succeeds
|
|
Given the skill "local/base-tools" is registered with tools
|
|
And a skill config YAML that includes "local/base-tools" at a temp path
|
|
When I run skill CLI add with --config pointing to the YAML file
|
|
Then the skill CLI add should succeed
|
|
And the skill CLI output should contain "Skill Registered"
|
|
|
|
Scenario: Update skill with unregistered include also fails
|
|
Given the skill "local/composable" is already registered
|
|
And a skill config YAML for "local/composable" that includes "local/not-there" at a temp path
|
|
When I run skill CLI add with --config and --update pointing to the YAML file
|
|
Then the skill CLI command should abort
|
|
And the skill CLI output should contain "not registered"
|
|
And the skill CLI output should contain "local/not-there"
|
|
|
|
Scenario: Service add_skill raises ValueError for unregistered include
|
|
Then adding a skill with unregistered include via service raises ValueError
|
|
|
|
Scenario: Service add_skill raises ValueError listing all missing includes
|
|
Then adding a skill with two unregistered includes via service raises ValueError listing both
|
|
|
|
Scenario: Service add_skill with update=True also validates includes
|
|
Given the skill "local/existing-skill" is already registered
|
|
Then updating "local/existing-skill" with unregistered include via service raises ValueError
|