fix(skills): validate included skills are registered before adding skill

- 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

features/skill_add_include_validation.feature

@@ -0,0 +1,61 @@
+@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

features/skill_cli.feature

@@ -21,6 +21,7 @@ Feature: Skill CLI commands
 
   Scenario: Add skill with all sections populated
     Given a full skill config YAML file at a temp path
+    And the skill "local/file-reader" is registered with tools
     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"
@@ -264,6 +265,7 @@ Feature: Skill CLI commands
 
   Scenario: Update skill with dependents shows Affected Actors panel
     Given a full skill config YAML file at a temp path
+    And the skill "local/file-reader" is registered with tools
     And a composed skill "local/uses-full" including "local/full-skill" is registered
     When I run skill CLI add with --config and --update pointing to the YAML file
     Then the skill CLI add should succeed

features/steps/skill_add_include_validation_steps.py

@@ -0,0 +1,226 @@
+"""Step definitions for skill_add_include_validation.feature.
+
+Tests that ``agents skill add`` validates included skills are registered
+before accepting a new skill registration (issue #2555).
+"""
+
+from __future__ import annotations
+
+import os
+import tempfile
+
+from behave import given, then
+from behave.runner import Context
+
+from cleveragents.cli.commands.skill import (
+    _get_skill_service,
+)
+from cleveragents.cli.commands.skill import app as skill_app  # noqa: F401
+from cleveragents.skills.schema import SkillConfigSchema
+
+# ---------------------------------------------------------------------------
+# YAML fixtures
+# ---------------------------------------------------------------------------
+
+_SINGLE_MISSING_INCLUDE_YAML = """\
+name: local/needs-missing-dep
+description: "Skill that includes an unregistered skill"
+tools:
+  - name: builtin/read_file
+includes:
+  - name: local/missing-dep
+"""
+
+_TWO_MISSING_INCLUDES_YAML = """\
+name: local/needs-two-deps
+description: "Skill that includes two unregistered skills"
+tools:
+  - name: builtin/read_file
+includes:
+  - name: local/dep-a
+  - name: local/dep-b
+"""
+
+_PARTIAL_INCLUDES_YAML = """\
+name: local/needs-partial-deps
+description: "Skill that includes one registered and one unregistered skill"
+tools:
+  - name: builtin/read_file
+includes:
+  - name: local/dep-registered
+  - name: local/dep-missing
+"""
+
+_VALID_INCLUDE_YAML = """\
+name: local/needs-base-tools
+description: "Skill that includes a registered skill"
+tools:
+  - name: builtin/read_file
+includes:
+  - name: local/base-tools
+"""
+
+_UPDATE_WITH_MISSING_INCLUDE_YAML = """\
+name: local/composable
+description: "Updated composable skill with unregistered include"
+tools:
+  - name: builtin/read_file
+includes:
+  - name: local/not-there
+"""
+
+_PARTIAL_REGISTERED_AND_MISSING_YAML = """\
+name: local/needs-partial-deps
+description: "Skill that includes one registered and one unregistered skill"
+tools:
+  - name: builtin/read_file
+includes:
+  - name: local/dep-registered
+  - name: local/dep-missing
+"""
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _write_temp_yaml(context: Context, content: str) -> str:
+    """Write YAML content to a temporary file and register cleanup."""
+    fd, path = tempfile.mkstemp(suffix=".yaml")
+    with os.fdopen(fd, "w") as fh:
+        fh.write(content)
+    if not hasattr(context, "_skill_cleanup"):
+        context._skill_cleanup = []
+    context._skill_cleanup.append(path)
+    return path
+
+
+# ---------------------------------------------------------------------------
+# Given steps
+# ---------------------------------------------------------------------------
+
+
+@given('a skill config YAML that includes "local/missing-dep" at a temp path')
+def step_yaml_single_missing_include(context: Context) -> None:
+    """Write a skill YAML that includes one unregistered skill."""
+    context.skill_yaml_path = _write_temp_yaml(context, _SINGLE_MISSING_INCLUDE_YAML)
+
+
+@given(
+    'a skill config YAML that includes "local/dep-a" and "local/dep-b" at a temp path'
+)
+def step_yaml_two_missing_includes(context: Context) -> None:
+    """Write a skill YAML that includes two unregistered skills."""
+    context.skill_yaml_path = _write_temp_yaml(context, _TWO_MISSING_INCLUDES_YAML)
+
+
+@given(
+    'a skill config YAML that includes "local/dep-registered" and "local/dep-missing" at a temp path'
+)
+def step_yaml_partial_includes(context: Context) -> None:
+    """Write a skill YAML that includes one registered and one unregistered skill."""
+    context.skill_yaml_path = _write_temp_yaml(
+        context, _PARTIAL_REGISTERED_AND_MISSING_YAML
+    )
+
+
+@given('a skill config YAML that includes "local/base-tools" at a temp path')
+def step_yaml_valid_include(context: Context) -> None:
+    """Write a skill YAML that includes a registered skill."""
+    context.skill_yaml_path = _write_temp_yaml(context, _VALID_INCLUDE_YAML)
+
+
+@given(
+    'a skill config YAML for "local/composable" that includes "local/not-there" at a temp path'
+)
+def step_yaml_update_with_missing_include(context: Context) -> None:
+    """Write a skill YAML for updating an existing skill with an unregistered include."""
+    context.skill_yaml_path = _write_temp_yaml(
+        context, _UPDATE_WITH_MISSING_INCLUDE_YAML
+    )
+
+
+# ---------------------------------------------------------------------------
+# Then steps — service-level assertions
+# ---------------------------------------------------------------------------
+
+
+@then("adding a skill with unregistered include via service raises ValueError")
+def step_service_raises_for_unregistered_include(context: Context) -> None:
+    """Assert SkillService.add_skill raises ValueError when include is missing."""
+    service = _get_skill_service()
+    schema = SkillConfigSchema(
+        name="local/test-validation",
+        description="Test skill",
+        includes=[{"name": "local/does-not-exist"}],
+    )
+    try:
+        service.add_skill(schema)
+        raise AssertionError(
+            "Expected ValueError for unregistered include, but no exception was raised"
+        )
+    except ValueError as exc:
+        msg = str(exc)
+        assert "not registered" in msg, (
+            f"Expected 'not registered' in error message, got: {msg}"
+        )
+        assert "local/does-not-exist" in msg, (
+            f"Expected missing skill name in error message, got: {msg}"
+        )
+
+
+@then(
+    "adding a skill with two unregistered includes via service raises ValueError listing both"
+)
+def step_service_raises_listing_both_missing(context: Context) -> None:
+    """Assert ValueError lists all missing included skills."""
+    service = _get_skill_service()
+    schema = SkillConfigSchema(
+        name="local/test-multi-validation",
+        description="Test skill",
+        includes=[
+            {"name": "local/missing-one"},
+            {"name": "local/missing-two"},
+        ],
+    )
+    try:
+        service.add_skill(schema)
+        raise AssertionError(
+            "Expected ValueError for unregistered includes, but no exception was raised"
+        )
+    except ValueError as exc:
+        msg = str(exc)
+        assert "local/missing-one" in msg, (
+            f"Expected 'local/missing-one' in error message, got: {msg}"
+        )
+        assert "local/missing-two" in msg, (
+            f"Expected 'local/missing-two' in error message, got: {msg}"
+        )
+
+
+@then('updating "{name}" with unregistered include via service raises ValueError')
+def step_service_update_raises_for_unregistered_include(
+    context: Context, name: str
+) -> None:
+    """Assert SkillService.add_skill with update=True also validates includes."""
+    service = _get_skill_service()
+    schema = SkillConfigSchema(
+        name=name,
+        description="Updated skill",
+        includes=[{"name": "local/unregistered-dep"}],
+    )
+    try:
+        service.add_skill(schema, update=True)
+        raise AssertionError(
+            "Expected ValueError for unregistered include on update, "
+            "but no exception was raised"
+        )
+    except ValueError as exc:
+        msg = str(exc)
+        assert "not registered" in msg, (
+            f"Expected 'not registered' in error message, got: {msg}"
+        )
+        assert "local/unregistered-dep" in msg, (
+            f"Expected missing skill name in error message, got: {msg}"
+        )

features/steps/skill_cli_coverage_r3_steps.py

@@ -348,7 +348,15 @@ def step_r3_pre_register_via_add(context: Context, name: str) -> None:
 
 @given('r3skill- a second temp YAML for "{name}" with different includes and MCP')
 def step_r3_second_yaml_diff_includes_mcp(context: Context, name: str) -> None:
-    """Create a second YAML config with different includes and MCP."""
+    """Create a second YAML config with different includes and MCP.
+
+    Pre-registers the included skill ``local/new-include`` so that the
+    pre-registration check added in issue #2555 does not reject the update.
+    """
+    # Ensure the included skill is registered before the update
+    new_include_skill = _make_skill("local/new-include", tool_refs=["builtin/echo"])
+    _register_skill(context, new_include_skill)
+
     yaml_content = f"""\
 name: {name}
 description: Updated skill with changed includes and MCP

src/cleveragents/application/services/skill_service.py

@@ -90,6 +90,7 @@ class SkillService:
 
         Raises:
             ValueError: If the skill already exists and ``update`` is ``False``.
+            ValueError: If any skill listed in ``includes`` is not registered.
         """
         if config is None:
             raise ValueError("config cannot be None")
@@ -103,6 +104,20 @@ class SkillService:
                 f"To overwrite, re-run with --update"
             )
 
+        # Pre-registration check: all included skills must already be registered.
+        # This applies both on initial add and on update (re-registration).
+        if config.includes:
+            missing = [
+                inc.name for inc in config.includes if inc.name not in self._skills
+            ]
+            if missing:
+                missing_list = ", ".join(f"'{m}'" for m in missing)
+                raise ValueError(
+                    f"Cannot register skill '{name}': the following included "
+                    f"skill(s) are not registered: {missing_list}. "
+                    f"Register them first with 'agents skill add'."
+                )
+
         is_new = name not in self._skills
         now = datetime.now()