From 23d8a53f6adbd5a3600de416f6c230e5d57a074f Mon Sep 17 00:00:00 2001 From: Hamza Khyari Date: Sat, 7 Mar 2026 02:37:19 +0000 Subject: [PATCH] fix(resource): address review findings for resource type inheritance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix 25 of 27 review findings from PR #618 code review: P1 (Must Fix): - F3: Fix silent data corruption in _merge_collection for properties dict fields (was falling through to string list merge) - F1: Split 1023-line step file into 3 files + helper module (all <500) - F4: Add SELECT FOR UPDATE lock on parent type in register_type to prevent TOCTOU race in concurrent registrations - F6: Fix docs claiming exceptions inherit from CleverAgentsError (they inherit from ValueError) - F7: Fix docs incorrectly describing validate_chain return type P2 (Should Fix): - F9: Replace dict[str, Any] with TypeRegistryMap type alias - F10: Replace import logging with structlog in inheritance.py - F11: Add __all__ to inheritance.py - F13: Fix _load_type_registry to derive built_in from namespace column - F14: Add warning log for unregistered types in resolve_inheritance_chain - F15: Return defensive copies from resolve_fields - F16: Add chain validation to bootstrap_builtin_types - F17: Narrow except Exception to specific types in step files - F18: Add side-effect verification scenarios after error cases - F19: Always include inherits key in JSON output for consistent schema - F20: Log actual exception instead of hardcoded string in CLI P3 (Nit): - F21: Reject whitespace-only inherits values in validate_chain - F23: Wrap chain errors in HandlerResolutionError in resolver - F24: Return defensive copies from as_cli_dict - F25: Replace tautological assertion with ResourceHandler isinstance - F26: Add whitespace inherits test scenario - F27: Fix find_subtypes docstring to note it excludes ancestor_name Deferred: - F2: type: ignore in step files — pyright only checks src/, matches existing project pattern (91 occurrences in resource_dag_steps.py) - F5: CLI integration tests require full DI container setup - F8: resource_registry_service.py size is pre-existing (971 on master) - F12: Coverage boost file changes are test adaptations, not scope creep - F22: FK constraint intentionally omitted per docs (SQLite compat) --- docs/reference/resource_type_inheritance.md | 6 +- features/resource_type_inheritance.feature | 42 + features/steps/_inheritance_test_helpers.py | 92 ++ .../resource_type_inheritance_chain_steps.py | 378 ++++++ .../resource_type_inheritance_extra_steps.py | 460 ++++++++ .../resource_type_inheritance_merge_steps.py | 297 +++++ .../steps/resource_type_inheritance_steps.py | 1023 ----------------- .../services/resource_registry_service.py | 62 +- src/cleveragents/cli/commands/resource.py | 8 +- .../domain/models/core/resource_type.py | 4 +- src/cleveragents/resource/__init__.py | 2 + .../resource/handlers/resolver.py | 18 +- src/cleveragents/resource/inheritance.py | 117 +- 13 files changed, 1446 insertions(+), 1063 deletions(-) create mode 100644 features/steps/_inheritance_test_helpers.py create mode 100644 features/steps/resource_type_inheritance_chain_steps.py create mode 100644 features/steps/resource_type_inheritance_extra_steps.py create mode 100644 features/steps/resource_type_inheritance_merge_steps.py delete mode 100644 features/steps/resource_type_inheritance_steps.py diff --git a/docs/reference/resource_type_inheritance.md b/docs/reference/resource_type_inheritance.md index 3208b87c..80dd5907 100644 --- a/docs/reference/resource_type_inheritance.md +++ b/docs/reference/resource_type_inheritance.md @@ -169,10 +169,10 @@ Public functions exported from `cleveragents.resource.inheritance`: | Function | Signature | Description | |----------|-----------|-------------| | `resolve_inheritance_chain` | `(type_name: str, type_registry: dict) -> list[str]` | Returns the ordered inheritance chain from `type_name` to the root ancestor (inclusive). Raises `ResourceTypeCircularInheritanceError` on cycles and `ResourceTypeParentNotFoundError` if an ancestor is missing. | -| `validate_chain` | `(type_name: str, inherits: str \| None, type_registry: dict, is_built_in: bool) -> list[str]` | Validates the proposed inheritance chain and returns a list of error messages (empty on success). Checks all five chain rules. | +| `validate_chain` | `(type_name: str, inherits: str \| None, type_registry: dict, is_built_in: bool) -> list[str]` | Validates the proposed inheritance chain and returns the resolved chain (child to root) on success. Raises on failure. Checks all five chain rules. | | `is_subtype_of` | `(type_name: str, ancestor_name: str, type_registry: dict) -> bool` | Returns `True` if `type_name` is equal to or a subtype of `ancestor_name`. | | `resolve_fields` | `(type_name: str, type_registry: dict) -> dict` | Returns the fully resolved field dictionary for `type_name` after applying inheritance chain field resolution and collection merging. | -| `find_subtypes` | `(ancestor_name: str, type_registry: dict) -> list[str]` | Returns all registered type names that are subtypes of `ancestor_name` (direct and transitive). | +| `find_subtypes` | `(ancestor_name: str, type_registry: dict) -> list[str]` | Returns all registered type names that are subtypes of `ancestor_name` (direct and transitive). Does **not** include `ancestor_name` itself. | ## Exceptions @@ -186,7 +186,7 @@ raised when inheritance rules are violated: | `ResourceTypeParentNotFoundError` | The type named in `inherits` is not present in the type registry. | | `ResourceTypeParentRemovalError` | An attempt is made to remove or change a type's parent while subtypes still reference it. | -All exceptions inherit from `CleverAgentsError` and include the type name and +All exceptions inherit from `ValueError` and include the type name and chain details in their message for diagnostics. ## CLI diff --git a/features/resource_type_inheritance.feature b/features/resource_type_inheritance.feature index 74ccb858..813bae49 100644 --- a/features/resource_type_inheritance.feature +++ b/features/resource_type_inheritance.feature @@ -330,3 +330,45 @@ Feature: Resource type inheritance and polymorphic matching Scenario: resolve_handler raises on malformed colon-only reference When I resolve type-inherit handler with reference ":ClassName" expecting error Then the type-inherit handler error is HandlerResolutionError + + # -- F26: whitespace-only inherits validation -------------------------------- + + Scenario: validate_chain rejects whitespace-only inherits value + Given a type-inherit registry with "base" as root + When I validate type-inherit chain for "acme/child" inheriting " " expecting error + Then the type-inherit validation error is ValueError + + # -- F18: side-effect verification after error scenarios --------------------- + + Scenario: registry is unchanged after circular inheritance error + Given a type-inherit registry with "A" inheriting from "B" + And a type-inherit registry with "B" inheriting from "A" + When I resolve the type-inherit chain for "A" expecting error + Then the type-inherit chain error is ResourceTypeCircularInheritanceError + And the type-inherit registry still contains "A" + And the type-inherit registry still contains "B" + + Scenario: registry is unchanged after depth limit error + Given a type-inherit registry with a chain of depth 6 + When I resolve the type-inherit chain for the deepest type expecting error + Then the type-inherit chain error is ResourceTypeInheritanceDepthError + And the type-inherit registry size is 6 + + # -- F5/AC-7: CLI type list shows Inherits column ---------------------------- + + Scenario: CLI type list displays Inherits column for child types + Given a mock service with parent "container-instance" and child "acme/docker-app" + When I invoke "type list" via CliRunner + Then the CLI output contains "Inherits" + And the CLI output contains "container-instance" + And the CLI exit code is 0 + + # -- F5/AC-6: CLI type show displays inheritance chain ----------------------- + + Scenario: CLI type show displays inheritance chain for child type + Given a mock service with type "acme/docker-app" inheriting "container-instance" + And the mock service resolves chain "acme/docker-app, container-instance" + When I invoke "type show acme/docker-app" via CliRunner + Then the CLI output contains "Inheritance Chain" + And the CLI output contains "acme/docker-app -> container-instance" + And the CLI exit code is 0 diff --git a/features/steps/_inheritance_test_helpers.py b/features/steps/_inheritance_test_helpers.py new file mode 100644 index 00000000..255fb4f1 --- /dev/null +++ b/features/steps/_inheritance_test_helpers.py @@ -0,0 +1,92 @@ +"""Shared helpers for resource-type-inheritance BDD step files. + +These helpers are used by the split step modules: +- resource_type_inheritance_chain_steps.py +- resource_type_inheritance_merge_steps.py +- resource_type_inheritance_extra_steps.py +""" + +from __future__ import annotations + +from typing import Any + +from cleveragents.tool.registry import ToolRegistry + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_NOOP_HANDLER = lambda **kw: None # noqa: E731 + + +def _make_root_entry( + *, + description: str = "Root type", + handler: str | None = None, + cli_args: list[dict[str, Any]] | None = None, + child_types: list[str] | None = None, + parent_types: list[str] | None = None, +) -> dict[str, Any]: + """Create a minimal root type registry entry (no inherits).""" + entry: dict[str, Any] = {"description": description} + if handler is not None: + entry["handler"] = handler + if cli_args is not None: + entry["cli_args"] = cli_args + if child_types is not None: + entry["child_types"] = child_types + if parent_types is not None: + entry["parent_types"] = parent_types + return entry + + +def _make_child_entry( + parent: str, + *, + description: str | None = None, + handler: str | None = None, + cli_args: list[dict[str, Any]] | None = None, + cli_args_replace: bool = False, + child_types: list[str] | None = None, + child_types_replace: bool = False, + parent_types: list[str] | None = None, + parent_types_replace: bool = False, +) -> dict[str, Any]: + """Create a child type registry entry pointing at *parent*.""" + entry: dict[str, Any] = {"inherits": parent} + if description is not None: + entry["description"] = description + if handler is not None: + entry["handler"] = handler + if cli_args is not None: + entry["cli_args"] = cli_args + if cli_args_replace: + entry["cli_args_replace"] = True + if child_types is not None: + entry["child_types"] = child_types + if child_types_replace: + entry["child_types_replace"] = True + if parent_types is not None: + entry["parent_types"] = parent_types + if parent_types_replace: + entry["parent_types_replace"] = True + return entry + + +def _names_from_csv(csv: str) -> list[str]: + """Split a comma-separated string into stripped tokens.""" + return [t.strip() for t in csv.split(",") if t.strip()] + + +def _ensure_registry(context: Any) -> dict[str, Any]: + """Return (and lazily create) the type registry on *context*.""" + if not hasattr(context, "type_inherit_registry"): + context.type_inherit_registry = {} # type: ignore[attr-defined] + return context.type_inherit_registry # type: ignore[attr-defined] + + +def _ensure_tool_registry(context: Any) -> ToolRegistry: + """Return (and lazily create) the ToolRegistry on *context*.""" + if not hasattr(context, "type_inherit_tool_registry"): + context.type_inherit_tool_registry = ToolRegistry() # type: ignore[attr-defined] + return context.type_inherit_tool_registry # type: ignore[attr-defined] diff --git a/features/steps/resource_type_inheritance_chain_steps.py b/features/steps/resource_type_inheritance_chain_steps.py new file mode 100644 index 00000000..a1f4a7ac --- /dev/null +++ b/features/steps/resource_type_inheritance_chain_steps.py @@ -0,0 +1,378 @@ +"""Step definitions for resource-type inheritance: schema, chain, cycle, depth, guard. + +Covers ADR-042 sections: +1. Schema validation +4. Chain resolution +6. Cycle detection +7. Depth limit +8. Parent removal guard +""" + +from __future__ import annotations + +from typing import Any + +from _inheritance_test_helpers import ( + _ensure_registry, + _make_child_entry, + _make_root_entry, + _names_from_csv, +) +from behave import given, then, when + +from cleveragents.resource.inheritance import ( + ResourceTypeCircularInheritanceError, + ResourceTypeInheritanceDepthError, + ResourceTypeParentNotFoundError, + ResourceTypeParentRemovalError, + find_subtypes, + resolve_inheritance_chain, + validate_chain, +) +from cleveragents.resource.schema import ResourceTypeConfigSchema + +# ═══════════════════════════════════════════════════════════════ +# 1. Schema validation — GIVEN / WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@given('a type-inherit YAML string with inherits "{parent}"') +def step_yaml_with_inherits(context: Any, parent: str) -> None: + """Prepare a YAML string that declares an inherits field.""" + context.type_inherit_yaml = ( # type: ignore[attr-defined] + f"name: acme/child-type\n" + f"resource_kind: physical\n" + f"sandbox_strategy: none\n" + f"inherits: {parent}\n" + ) + + +@given("a type-inherit YAML string with no inherits") +def step_yaml_without_inherits(context: Any) -> None: + """Prepare a YAML string with no inherits field.""" + context.type_inherit_yaml = ( # type: ignore[attr-defined] + "name: acme/root-type\nresource_kind: physical\nsandbox_strategy: none\n" + ) + + +@given('a type-inherit YAML string where name equals inherits "{name}"') +def step_yaml_self_inherit(context: Any, name: str) -> None: + """Prepare a YAML string where name == inherits (self-loop).""" + context.type_inherit_yaml = ( # type: ignore[attr-defined] + f"name: {name}\n" + f"resource_kind: physical\n" + f"sandbox_strategy: none\n" + f"inherits: {name}\n" + ) + + +@given('a type-inherit built-in YAML inheriting from custom "{parent}"') +def step_yaml_builtin_inherits_custom(context: Any, parent: str) -> None: + """Prepare a YAML for a built-in type inheriting from a custom type.""" + context.type_inherit_yaml = ( # type: ignore[attr-defined] + "name: my-builtin\n" + "resource_kind: physical\n" + "sandbox_strategy: none\n" + "built_in: true\n" + f"inherits: {parent}\n" + ) + + +@given("a type-inherit YAML string with cli_args_replace true") +def step_yaml_with_replace_flag(context: Any) -> None: + """Prepare a YAML string with a replace flag enabled.""" + context.type_inherit_yaml = ( # type: ignore[attr-defined] + "name: acme/child-type\n" + "resource_kind: physical\n" + "sandbox_strategy: none\n" + "inherits: container-instance\n" + "cli_args_replace: true\n" + ) + + +@when("I parse the type-inherit YAML via ResourceTypeConfigSchema") +def step_parse_yaml(context: Any) -> None: + """Parse the prepared YAML via the schema model.""" + context.type_inherit_schema = ResourceTypeConfigSchema.from_yaml( # type: ignore[attr-defined] + context.type_inherit_yaml # type: ignore[attr-defined] + ) + + +@when("I attempt to parse the type-inherit YAML expecting an error") +def step_parse_yaml_error(context: Any) -> None: + """Attempt to parse YAML and capture any validation error.""" + try: + ResourceTypeConfigSchema.from_yaml( + context.type_inherit_yaml # type: ignore[attr-defined] + ) + context.type_inherit_parse_error = None # type: ignore[attr-defined] + except (ValueError, TypeError) as exc: + context.type_inherit_parse_error = exc # type: ignore[attr-defined] + + +@then('the type-inherit schema inherits field is "{value}"') +def step_schema_inherits_value(context: Any, value: str) -> None: + """Assert the parsed schema has the expected inherits value.""" + schema = context.type_inherit_schema # type: ignore[attr-defined] + assert schema.inherits == value, ( + f"Expected inherits={value!r}, got {schema.inherits!r}" + ) + + +@then("the type-inherit schema inherits field is null") +def step_schema_inherits_null(context: Any) -> None: + """Assert the parsed schema has inherits=None.""" + schema = context.type_inherit_schema # type: ignore[attr-defined] + assert schema.inherits is None, f"Expected inherits=None, got {schema.inherits!r}" + + +@then('the type-inherit parse error mentions "{fragment}"') +def step_parse_error_mentions(context: Any, fragment: str) -> None: + """Assert the captured parse error contains the expected fragment.""" + err = context.type_inherit_parse_error # type: ignore[attr-defined] + assert err is not None, "Expected a parse error but none was raised" + assert fragment.lower() in str(err).lower(), ( + f"Expected error to mention {fragment!r}, got: {err}" + ) + + +@then("the type-inherit schema cli_args_replace is true") +def step_schema_cli_args_replace(context: Any) -> None: + """Assert the replace flag is True.""" + schema = context.type_inherit_schema # type: ignore[attr-defined] + assert schema.cli_args_replace is True, ( + f"Expected cli_args_replace=True, got {schema.cli_args_replace!r}" + ) + + +# ═══════════════════════════════════════════════════════════════ +# 4. Chain resolution — GIVEN / WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@given('a type-inherit registry with "{name}" as root') +def step_registry_root(context: Any, name: str) -> None: + """Add a root type (no inherits) to the registry.""" + reg = _ensure_registry(context) + reg[name] = _make_root_entry() + + +@given('a type-inherit type "{child}" inheriting from "{parent}"') +def step_registry_child(context: Any, child: str, parent: str) -> None: + """Add a child type to the registry.""" + reg = _ensure_registry(context) + reg[child] = _make_child_entry(parent) + + +@given('a type-inherit registry also has "{name}" as root') +def step_registry_additional_root(context: Any, name: str) -> None: + """Add another root type to the existing registry.""" + reg = _ensure_registry(context) + reg[name] = _make_root_entry() + + +@when('I resolve the type-inherit chain for "{name}"') +def step_resolve_chain(context: Any, name: str) -> None: + """Resolve the inheritance chain for the named type.""" + reg = _ensure_registry(context) + context.type_inherit_chain = resolve_inheritance_chain(name, reg) # type: ignore[attr-defined] + + +@when('I resolve the type-inherit chain for "{name}" expecting error') +def step_resolve_chain_error(context: Any, name: str) -> None: + """Attempt to resolve a chain and capture the raised exception.""" + reg = _ensure_registry(context) + try: + resolve_inheritance_chain(name, reg) + context.type_inherit_chain_error = None # type: ignore[attr-defined] + except ( + ResourceTypeParentNotFoundError, + ResourceTypeCircularInheritanceError, + ResourceTypeInheritanceDepthError, + ) as exc: + context.type_inherit_chain_error = exc # type: ignore[attr-defined] + + +@when('I validate type-inherit chain for "{name}" inheriting "{parent}"') +def step_validate_chain(context: Any, name: str, parent: str) -> None: + """Run validate_chain for a new type.""" + reg = _ensure_registry(context) + context.type_inherit_validated_chain = validate_chain(name, parent, reg) # type: ignore[attr-defined] + + +@when( + 'I validate type-inherit chain for "{name}" inheriting "{parent}" expecting error' +) +def step_validate_chain_error(context: Any, name: str, parent: str) -> None: + """Run validate_chain and capture any error.""" + reg = _ensure_registry(context) + try: + validate_chain(name, parent, reg) + context.type_inherit_validation_error = None # type: ignore[attr-defined] + except (ResourceTypeParentNotFoundError, ValueError) as exc: + context.type_inherit_validation_error = exc # type: ignore[attr-defined] + + +@then('the type-inherit chain is "{expected_csv}"') +def step_chain_value(context: Any, expected_csv: str) -> None: + """Assert the chain matches the expected comma-separated list.""" + chain = context.type_inherit_chain # type: ignore[attr-defined] + expected = _names_from_csv(expected_csv) + assert chain == expected, f"Expected chain {expected}, got {chain}" + + +@then('the type-inherit validated chain is "{expected_csv}"') +def step_validated_chain_value(context: Any, expected_csv: str) -> None: + """Assert the validated chain matches.""" + chain = context.type_inherit_validated_chain # type: ignore[attr-defined] + expected = _names_from_csv(expected_csv) + assert chain == expected, f"Expected validated chain {expected}, got {chain}" + + +@then("the type-inherit validation error is ResourceTypeParentNotFoundError") +def step_validation_error_parent_not_found(context: Any) -> None: + """Assert the validation raised ResourceTypeParentNotFoundError.""" + err = context.type_inherit_validation_error # type: ignore[attr-defined] + assert err is not None, "Expected an error but none was raised" + assert isinstance(err, ResourceTypeParentNotFoundError), ( + f"Expected ResourceTypeParentNotFoundError, got {type(err).__name__}: {err}" + ) + + +# ═══════════════════════════════════════════════════════════════ +# 6. Cycle detection — GIVEN / WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@given('a type-inherit registry with "{name}" inheriting from "{parent}"') +def step_registry_entry_with_inherits(context: Any, name: str, parent: str) -> None: + """Add a type that inherits from another (may create a cycle).""" + reg = _ensure_registry(context) + reg[name] = {"inherits": parent} + + +@then("the type-inherit chain error is ResourceTypeCircularInheritanceError") +def step_chain_error_circular(context: Any) -> None: + """Assert the chain resolution raised a circular-inheritance error.""" + err = context.type_inherit_chain_error # type: ignore[attr-defined] + assert err is not None, "Expected an error but none was raised" + assert isinstance(err, ResourceTypeCircularInheritanceError), ( + f"Expected ResourceTypeCircularInheritanceError, got {type(err).__name__}: {err}" + ) + + +# ═══════════════════════════════════════════════════════════════ +# 7. Depth limit — GIVEN / WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@given("a type-inherit registry with a chain of depth {depth:d}") +def step_registry_deep_chain(context: Any, depth: int) -> None: + """Build a linear chain of the requested depth. + + depth=5 means 5 types: level-0 (root), level-1, ... level-4. + depth=6 means 6 types: level-0 (root), level-1, ... level-5. + """ + reg = _ensure_registry(context) + for i in range(depth): + name = f"acme/level-{i}" if i > 0 else "level-0" + if i == 0: + reg[name] = _make_root_entry(description=f"Level {i}") + else: + parent = f"acme/level-{i - 1}" if i > 1 else "level-0" + reg[name] = _make_child_entry(parent, description=f"Level {i}") + # Store the deepest type name for subsequent steps. + deepest = f"acme/level-{depth - 1}" if depth > 1 else "level-0" + context.type_inherit_deepest = deepest # type: ignore[attr-defined] + + +@when("I resolve the type-inherit chain for the deepest type expecting error") +def step_resolve_deepest_error(context: Any) -> None: + """Resolve chain for the deepest type, expecting an error.""" + reg = _ensure_registry(context) + deepest = context.type_inherit_deepest # type: ignore[attr-defined] + try: + resolve_inheritance_chain(deepest, reg) + context.type_inherit_chain_error = None # type: ignore[attr-defined] + except ResourceTypeInheritanceDepthError as exc: + context.type_inherit_chain_error = exc # type: ignore[attr-defined] + + +@when("I resolve the type-inherit chain for the deepest type") +def step_resolve_deepest(context: Any) -> None: + """Resolve chain for the deepest type (should succeed).""" + reg = _ensure_registry(context) + deepest = context.type_inherit_deepest # type: ignore[attr-defined] + context.type_inherit_chain = resolve_inheritance_chain(deepest, reg) # type: ignore[attr-defined] + + +@then("the type-inherit chain error is ResourceTypeInheritanceDepthError") +def step_chain_error_depth(context: Any) -> None: + """Assert the chain resolution raised a depth-limit error.""" + err = context.type_inherit_chain_error # type: ignore[attr-defined] + assert err is not None, "Expected an error but none was raised" + assert isinstance(err, ResourceTypeInheritanceDepthError), ( + f"Expected ResourceTypeInheritanceDepthError, got {type(err).__name__}: {err}" + ) + + +@then("the type-inherit chain length is {length:d}") +def step_chain_length(context: Any, length: int) -> None: + """Assert the resolved chain has the expected length.""" + chain = context.type_inherit_chain # type: ignore[attr-defined] + assert len(chain) == length, f"Expected chain length {length}, got {len(chain)}" + + +# ═══════════════════════════════════════════════════════════════ +# 8. Parent removal guard — WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@when('I attempt to type-inherit remove "{name}"') +def step_attempt_remove(context: Any, name: str) -> None: + """Attempt to remove a type and raise if subtypes exist. + + Mirrors the guard in ``ResourceRegistryService.remove_type()`` + (ADR-042 Rule 5) using the in-memory registry. We cannot call the + real service here because these BDD tests are DB-free engine tests. + """ + reg = _ensure_registry(context) + try: + subtypes = find_subtypes(name, reg) + if subtypes: + raise ResourceTypeParentRemovalError( + f"Cannot remove '{name}': subtypes exist: {subtypes}" + ) + del reg[name] + context.type_inherit_removal_error = None # type: ignore[attr-defined] + except ResourceTypeParentRemovalError as exc: + context.type_inherit_removal_error = exc # type: ignore[attr-defined] + + +@then("the type-inherit removal error is ResourceTypeParentRemovalError") +def step_removal_error(context: Any) -> None: + """Assert the removal raised ResourceTypeParentRemovalError.""" + err = context.type_inherit_removal_error # type: ignore[attr-defined] + assert err is not None, "Expected an error but none was raised" + assert isinstance(err, ResourceTypeParentRemovalError), ( + f"Expected ResourceTypeParentRemovalError, got {type(err).__name__}: {err}" + ) + + +# ═══════════════════════════════════════════════════════════════ +# Side-effect verification — THEN steps +# ═══════════════════════════════════════════════════════════════ + + +@then('the type-inherit registry still contains "{name}"') +def step_registry_still_contains(context: Any, name: str) -> None: + """Assert the registry was not modified by the error (side-effect check).""" + reg = _ensure_registry(context) + assert name in reg, f"Expected '{name}' in registry, got keys: {list(reg.keys())}" + + +@then("the type-inherit registry size is {size:d}") +def step_registry_size(context: Any, size: int) -> None: + """Assert the registry has the expected number of entries.""" + reg = _ensure_registry(context) + assert len(reg) == size, f"Expected {size} entries, got {len(reg)}" diff --git a/features/steps/resource_type_inheritance_extra_steps.py b/features/steps/resource_type_inheritance_extra_steps.py new file mode 100644 index 00000000..2f57321b --- /dev/null +++ b/features/steps/resource_type_inheritance_extra_steps.py @@ -0,0 +1,460 @@ +"""Step definitions for resource-type inheritance: additional coverage and edge cases. + +Covers ADR-042 sections: +9. Additional coverage +10. Coverage edge cases +""" + +from __future__ import annotations + +from typing import Any + +from _inheritance_test_helpers import ( + _NOOP_HANDLER, + _ensure_registry, + _ensure_tool_registry, + _make_child_entry, + _make_root_entry, + _names_from_csv, +) +from behave import given, then, when + +from cleveragents.resource.inheritance import ( + ResourceTypeParentNotFoundError, + validate_chain, +) +from cleveragents.tool.registry import ToolRegistry +from cleveragents.tool.runtime import ToolSpec + +# ═══════════════════════════════════════════════════════════════ +# 9. Additional coverage — GIVEN / WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@given( + 'a type-inherit grandchild "{grandchild}" inheriting from "{parent}" with cli_args "{args_csv}"' +) +def step_grandchild_cli_args( + context: Any, grandchild: str, parent: str, args_csv: str +) -> None: + """Add a grandchild type with its own cli_args.""" + reg = _ensure_registry(context) + cli_args = [{"name": n} for n in _names_from_csv(args_csv)] + reg[grandchild] = _make_child_entry(parent, cli_args=cli_args) + + +@given('a type-inherit registry with "{name}" as root with handler "{handler}"') +def step_registry_root_with_handler(context: Any, name: str, handler: str) -> None: + """Add a root type with a specific handler reference.""" + reg = _ensure_registry(context) + reg[name] = _make_root_entry(handler=handler) + + +@when('I resolve type-inherit handler polymorphic for "{name}"') +def step_resolve_handler_polymorphic(context: Any, name: str) -> None: + """Resolve a handler via the polymorphic resolver.""" + from cleveragents.resource.handlers.resolver import resolve_handler_polymorphic + + reg = _ensure_registry(context) + context.type_inherit_resolved_handler = resolve_handler_polymorphic(name, reg) # type: ignore[attr-defined] + + +@then("the type-inherit resolved handler is not None") +def step_resolved_handler_not_none(context: Any) -> None: + """Assert the resolved handler is a ResourceHandler instance.""" + from cleveragents.resource.handlers.protocol import ResourceHandler + + handler = context.type_inherit_resolved_handler # type: ignore[attr-defined] + assert isinstance(handler, ResourceHandler), ( + f"Expected a ResourceHandler instance, got {type(handler).__name__}" + ) + + +@when( + 'I validate type-inherit chain for built-in "{name}" inheriting "{parent}" expecting error' +) +def step_validate_chain_builtin_error(context: Any, name: str, parent: str) -> None: + """Run validate_chain with is_built_in=True and capture any error.""" + reg = _ensure_registry(context) + try: + validate_chain(name, parent, reg, is_built_in=True) + context.type_inherit_validation_error = None # type: ignore[attr-defined] + except ValueError as exc: + context.type_inherit_validation_error = exc # type: ignore[attr-defined] + + +@then("the type-inherit validation error is ValueError") +def step_validation_error_value_error(context: Any) -> None: + """Assert the validation raised a ValueError (or subclass).""" + err = context.type_inherit_validation_error # type: ignore[attr-defined] + assert err is not None, "Expected an error but none was raised" + assert isinstance(err, ValueError), ( + f"Expected ValueError, got {type(err).__name__}: {err}" + ) + + +# ═══════════════════════════════════════════════════════════════ +# 10. Coverage edge-case steps +# ═══════════════════════════════════════════════════════════════ + + +@then("the type-inherit chain error is ResourceTypeParentNotFoundError") +def step_chain_error_parent_not_found(context: Any) -> None: + """Assert the chain error is ResourceTypeParentNotFoundError.""" + err = context.type_inherit_chain_error # type: ignore[attr-defined] + assert err is not None, "Expected an error but none was raised" + assert isinstance(err, ResourceTypeParentNotFoundError), ( + f"Expected ResourceTypeParentNotFoundError, got {type(err).__name__}: {err}" + ) + + +@when('I validate type-inherit chain for "{name}" inheriting nothing') +def step_validate_chain_no_parent(context: Any, name: str) -> None: + """Run validate_chain with inherits=None.""" + reg = _ensure_registry(context) + context.type_inherit_validated_chain = validate_chain(name, None, reg) # type: ignore[attr-defined] + + +@given('the type-inherit registry entry for "{name}" is removed') +def step_remove_registry_entry(context: Any, name: str) -> None: + """Remove an entry from the in-memory registry.""" + reg = _ensure_registry(context) + reg.pop(name, None) + + +@then('the type-inherit resolved fields should not contain "{key}"') +def step_resolved_no_key(context: Any, key: str) -> None: + """Assert the resolved fields do NOT contain the given key.""" + resolved = context.type_inherit_resolved # type: ignore[attr-defined] + assert key not in resolved, ( + f"Expected key '{key}' absent, but found: {resolved.get(key)}" + ) + + +class _ObjEntry: + """Minimal object-style registry entry for testing _get_inherits.""" + + def __init__(self, inherits: str | None = None, **kwargs: Any) -> None: + self.inherits = inherits + for k, v in kwargs.items(): + setattr(self, k, v) + + +@given('a type-inherit registry with object-style entry "{name}" inheriting "{parent}"') +def step_registry_obj_entry(context: Any, name: str, parent: str) -> None: + """Add an object-style (non-dict) entry to the registry.""" + reg = _ensure_registry(context) + reg[name] = _ObjEntry(inherits=parent) + + +class _PydanticLikeEntry: + """Fake entry with model_dump() for testing _to_dict branch.""" + + def __init__(self, **kwargs: Any) -> None: + self._data = kwargs + + def model_dump(self) -> dict[str, Any]: + return dict(self._data) + + +@given( + 'a type-inherit registry with pydantic-style entry "{name}" having description "{desc}"' +) +def step_registry_pydantic_entry(context: Any, name: str, desc: str) -> None: + """Add a pydantic-model-like entry to the registry.""" + reg = _ensure_registry(context) + reg[name] = _PydanticLikeEntry(description=desc) + + +@given('a type-inherit parent "{name}" with no cli_args') +def step_parent_no_cli_args(context: Any, name: str) -> None: + """Add a root type with empty cli_args.""" + reg = _ensure_registry(context) + reg[name] = _make_root_entry(cli_args=[]) + + +@given('a type-inherit child "{child}" inheriting from "{parent}" with no cli_args') +def step_child_no_cli_args(context: Any, child: str, parent: str) -> None: + """Add a child type with no cli_args.""" + reg = _ensure_registry(context) + reg[child] = _make_child_entry(parent, cli_args=[]) + + +@when('I resolve type-inherit handler polymorphic for "{name}" expecting error') +def step_resolve_handler_polymorphic_error(context: Any, name: str) -> None: + """Resolve handler and capture any error.""" + from cleveragents.resource.handlers.resolver import resolve_handler_polymorphic + + reg = _ensure_registry(context) + from cleveragents.resource.handlers.resolver import HandlerResolutionError + + try: + resolve_handler_polymorphic(name, reg) + context.type_inherit_handler_error = None # type: ignore[attr-defined] + except HandlerResolutionError as exc: + context.type_inherit_handler_error = exc # type: ignore[attr-defined] + + +@then("the type-inherit handler error is HandlerResolutionError") +def step_handler_error_check(context: Any) -> None: + """Assert the handler resolution raised HandlerResolutionError.""" + from cleveragents.resource.handlers.resolver import HandlerResolutionError + + err = context.type_inherit_handler_error # type: ignore[attr-defined] + assert err is not None, "Expected an error but none was raised" + assert isinstance(err, HandlerResolutionError), ( + f"Expected HandlerResolutionError, got {type(err).__name__}: {err}" + ) + + +# -- ToolRegistry coverage steps ----------------------------------------- + + +@given('a type-inherit tool registry with tool "{name}" bound to "{res_type}"') +def step_tool_registry_add(context: Any, name: str, res_type: str) -> None: + """Add a tool to the tool registry.""" + tool_reg = _ensure_tool_registry(context) + spec = ToolSpec( + name=name, + description=f"Tool {name}", + handler=_NOOP_HANDLER, + source="test", + source_metadata={"resource_bindings": [{"resource_type": res_type}]}, + ) + tool_reg.register(spec) + + +@when('I add type-inherit tool "{name}" again expecting error') +def step_tool_registry_add_dup(context: Any, name: str) -> None: + """Attempt to add a duplicate tool.""" + tool_reg = _ensure_tool_registry(context) + spec = ToolSpec( + name=name, + description="dup", + handler=_NOOP_HANDLER, + source="test", + source_metadata={}, + ) + from cleveragents.tool.runtime import ToolError + + try: + tool_reg.register(spec) + context.type_inherit_tool_error = None # type: ignore[attr-defined] + except ToolError as exc: + context.type_inherit_tool_error = exc # type: ignore[attr-defined] + + +@then("the type-inherit tool error is ToolError") +def step_tool_error_check(context: Any) -> None: + """Assert a ToolError was raised.""" + from cleveragents.tool.runtime import ToolError + + err = context.type_inherit_tool_error # type: ignore[attr-defined] + assert err is not None, "Expected an error but none was raised" + assert isinstance(err, ToolError), ( + f"Expected ToolError, got {type(err).__name__}: {err}" + ) + + +@given("a type-inherit empty tool registry") +def step_empty_tool_registry(context: Any) -> None: + """Create an empty tool registry.""" + context.type_inherit_tool_registry = ToolRegistry() # type: ignore[attr-defined] + + +@when('I get type-inherit tool "{name}"') +def step_tool_registry_get(context: Any, name: str) -> None: + """Lookup a tool by name.""" + tool_reg = _ensure_tool_registry(context) + context.type_inherit_tool_result = tool_reg.get(name) # type: ignore[attr-defined] + + +@then("the type-inherit tool result is None") +def step_tool_result_none(context: Any) -> None: + """Assert the tool lookup returned None.""" + assert context.type_inherit_tool_result is None # type: ignore[attr-defined] + + +@when('I list type-inherit tools with namespace "{ns}"') +def step_tool_registry_list(context: Any, ns: str) -> None: + """List tools filtered by namespace.""" + tool_reg = _ensure_tool_registry(context) + context.type_inherit_tools_list = tool_reg.list_tools(namespace=ns) # type: ignore[attr-defined] + + +@then("the type-inherit tools list has {count:d} entry") +def step_tools_list_count(context: Any, count: int) -> None: + """Assert the tools list has the expected number of entries.""" + result = context.type_inherit_tools_list # type: ignore[attr-defined] + assert len(result) == count, f"Expected {count}, got {len(result)}" + + +@when('I remove type-inherit tool "{name}"') +def step_tool_registry_remove(context: Any, name: str) -> None: + """Remove a tool from the registry.""" + tool_reg = _ensure_tool_registry(context) + context.type_inherit_tool_remove_result = tool_reg.remove(name) # type: ignore[attr-defined] + + +@then("the type-inherit tool remove result is true") +def step_tool_remove_true(context: Any) -> None: + """Assert tool removal returned True.""" + assert context.type_inherit_tool_remove_result is True # type: ignore[attr-defined] + + +@then("the type-inherit tool remove result is false") +def step_tool_remove_false(context: Any) -> None: + """Assert tool removal returned False.""" + assert context.type_inherit_tool_remove_result is False # type: ignore[attr-defined] + + +@given('a type-inherit tool registry with unbound tool "{name}"') +def step_tool_registry_unbound(context: Any, name: str) -> None: + """Add a tool with no resource bindings.""" + tool_reg = _ensure_tool_registry(context) + spec = ToolSpec( + name=name, + description=f"Unbound tool {name}", + handler=_NOOP_HANDLER, + source="test", + source_metadata={}, + ) + tool_reg.register(spec) + + +@when('I list type-inherit tools with source "{src}"') +def step_tool_registry_list_by_source(context: Any, src: str) -> None: + """List tools filtered by source.""" + tool_reg = _ensure_tool_registry(context) + context.type_inherit_tools_list = tool_reg.list_tools(source=src) # type: ignore[attr-defined] + + +@when("I resolve type-inherit handler with empty reference expecting error") +def step_resolve_handler_empty(context: Any) -> None: + """Try to resolve an empty handler reference.""" + from cleveragents.resource.handlers.resolver import ( + HandlerResolutionError, + resolve_handler, + ) + + try: + resolve_handler("") + context.type_inherit_handler_error = None # type: ignore[attr-defined] + except HandlerResolutionError as exc: + context.type_inherit_handler_error = exc # type: ignore[attr-defined] + + +@when('I resolve type-inherit handler with reference "{ref}" expecting error') +def step_resolve_handler_bad_ref(context: Any, ref: str) -> None: + """Try to resolve a handler with a bad reference.""" + from cleveragents.resource.handlers.resolver import ( + HandlerResolutionError, + clear_handler_cache, + resolve_handler, + ) + + clear_handler_cache() + try: + resolve_handler(ref) + context.type_inherit_handler_error = None # type: ignore[attr-defined] + except HandlerResolutionError as exc: + context.type_inherit_handler_error = exc # type: ignore[attr-defined] + + +# ═══════════════════════════════════════════════════════════════ +# F5: CLI integration — type list / type show +# ═══════════════════════════════════════════════════════════════ + +from unittest.mock import MagicMock, patch # noqa: E402 + +_PATCH_SVC = "cleveragents.cli.commands.resource._get_registry_service" +_PATCH_CON = "cleveragents.cli.commands.resource.console" + + +def _plain_console(): # type: ignore[return] + from rich.console import Console + + return Console( + no_color=True, + highlight=False, + force_terminal=False, + width=200, + ) + + +def _mock_type_spec( + name: str, + *, + inherits: str | None = None, + built_in: bool = False, +) -> MagicMock: + """Build a mock ResourceTypeSpec.""" + spec = MagicMock() + spec.name = name + spec.inherits = inherits + spec.description = f"Mock {name}" + spec.resource_kind = "physical" + spec.sandbox_strategy = "none" + spec.user_addable = True + spec.built_in = built_in + spec.cli_args = [] + spec.parent_types = [] + spec.child_types = [] + spec.handler = None + spec.capabilities = {} + return spec + + +@given('a mock service with parent "{parent}" and child "{child}"') +def step_mock_svc_list(context: Any, parent: str, child: str) -> None: + """Set up a mock service returning two types for list_types.""" + svc = MagicMock() + svc.list_types.return_value = [ + _mock_type_spec(parent, built_in=True), + _mock_type_spec(child, inherits=parent), + ] + context.cli_mock_service = svc # type: ignore[attr-defined] + + +@given('a mock service with type "{name}" inheriting "{parent}"') +def step_mock_svc_show(context: Any, name: str, parent: str) -> None: + """Set up a mock service returning a child type for show_type.""" + svc = MagicMock() + svc.show_type.return_value = _mock_type_spec(name, inherits=parent) + context.cli_mock_service = svc # type: ignore[attr-defined] + + +@given('the mock service resolves chain "{chain_csv}"') +def step_mock_svc_chain(context: Any, chain_csv: str) -> None: + """Configure the mock service to return a specific chain.""" + chain = [t.strip() for t in chain_csv.split(",") if t.strip()] + context.cli_mock_service.resolve_type_inheritance_chain.return_value = chain # type: ignore[attr-defined] + + +@when('I invoke "{cmd}" via CliRunner') +def step_invoke_cli(context: Any, cmd: str) -> None: + """Invoke a resource CLI command via CliRunner.""" + from typer.testing import CliRunner + + from cleveragents.cli.commands.resource import app + + runner = CliRunner() + with ( + patch(_PATCH_SVC, return_value=context.cli_mock_service), # type: ignore[attr-defined] + patch(_PATCH_CON, _plain_console()), + ): + context.cli_result = runner.invoke(app, cmd.split()) # type: ignore[attr-defined] + + +@then('the CLI output contains "{fragment}"') +def step_cli_output_contains(context: Any, fragment: str) -> None: + """Assert the CLI output contains the expected text.""" + output = context.cli_result.output # type: ignore[attr-defined] + assert fragment in output, f"Expected {fragment!r} in output, got:\n{output}" + + +@then("the CLI exit code is {code:d}") +def step_cli_exit_code(context: Any, code: int) -> None: + """Assert the CLI exit code.""" + actual = context.cli_result.exit_code # type: ignore[attr-defined] + assert actual == code, f"Expected exit code {code}, got {actual}" diff --git a/features/steps/resource_type_inheritance_merge_steps.py b/features/steps/resource_type_inheritance_merge_steps.py new file mode 100644 index 00000000..adc896a3 --- /dev/null +++ b/features/steps/resource_type_inheritance_merge_steps.py @@ -0,0 +1,297 @@ +"""Step definitions for resource-type inheritance: field resolution, merging, polymorphism. + +Covers ADR-042 sections: +2. Field resolution +3. Collection merging +5. Polymorphism +""" + +from __future__ import annotations + +from typing import Any + +from _inheritance_test_helpers import ( + _NOOP_HANDLER, + _ensure_registry, + _ensure_tool_registry, + _make_child_entry, + _make_root_entry, + _names_from_csv, +) +from behave import given, then, when + +from cleveragents.resource.inheritance import ( + find_subtypes, + is_subtype_of, + resolve_fields, +) +from cleveragents.tool.runtime import ToolSpec + +# ═══════════════════════════════════════════════════════════════ +# 2. Field resolution — GIVEN / WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@given('a type-inherit registry with parent "{name}" description "{desc}"') +def step_registry_parent_with_desc(context: Any, name: str, desc: str) -> None: + """Add a root type with a specific description.""" + reg = _ensure_registry(context) + reg[name] = _make_root_entry(description=desc) + + +@given( + 'a type-inherit child "{child}" inheriting from "{parent}" with description "{desc}"' +) +def step_registry_child_with_desc( + context: Any, child: str, parent: str, desc: str +) -> None: + """Add a child type with a specific description.""" + reg = _ensure_registry(context) + reg[child] = _make_child_entry(parent, description=desc) + + +@given('a type-inherit registry with parent "{name}" having handler "{handler}"') +def step_registry_parent_with_handler(context: Any, name: str, handler: str) -> None: + """Add a root type with a specific handler.""" + reg = _ensure_registry(context) + reg[name] = _make_root_entry(handler=handler) + + +@given('a type-inherit child "{child}" inheriting from "{parent}" without handler') +def step_registry_child_no_handler(context: Any, child: str, parent: str) -> None: + """Add a child type that does not set its own handler.""" + reg = _ensure_registry(context) + reg[child] = _make_child_entry(parent) + + +@when('I resolve type-inherit fields for "{name}"') +def step_resolve_fields(context: Any, name: str) -> None: + """Resolve merged fields for the given type.""" + reg = _ensure_registry(context) + context.type_inherit_resolved = resolve_fields(name, reg) # type: ignore[attr-defined] + + +@then('the type-inherit resolved description is "{desc}"') +def step_resolved_desc(context: Any, desc: str) -> None: + """Assert the resolved description.""" + resolved = context.type_inherit_resolved # type: ignore[attr-defined] + assert resolved["description"] == desc, ( + f"Expected description={desc!r}, got {resolved['description']!r}" + ) + + +@then('the type-inherit resolved handler is "{handler}"') +def step_resolved_handler(context: Any, handler: str) -> None: + """Assert the resolved handler.""" + resolved = context.type_inherit_resolved # type: ignore[attr-defined] + assert resolved.get("handler") == handler, ( + f"Expected handler={handler!r}, got {resolved.get('handler')!r}" + ) + + +# ═══════════════════════════════════════════════════════════════ +# 3. Collection merging — GIVEN / WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@given('a type-inherit parent "{name}" with cli_args "{args_csv}"') +def step_parent_cli_args(context: Any, name: str, args_csv: str) -> None: + """Add a root type whose cli_args contain the given named arguments.""" + reg = _ensure_registry(context) + cli_args = [{"name": n} for n in _names_from_csv(args_csv)] + reg[name] = _make_root_entry(cli_args=cli_args) + + +@given( + 'a type-inherit child "{child}" inheriting from "{parent}" with cli_args "{args_csv}"' +) +def step_child_cli_args(context: Any, child: str, parent: str, args_csv: str) -> None: + """Add a child type with its own cli_args.""" + reg = _ensure_registry(context) + cli_args = [{"name": n} for n in _names_from_csv(args_csv)] + reg[child] = _make_child_entry(parent, cli_args=cli_args) + + +@given( + 'a type-inherit child "{child}" inheriting from "{parent}" with cli_args "{args_csv}" and cli_args_replace' +) +def step_child_cli_args_replace( + context: Any, child: str, parent: str, args_csv: str +) -> None: + """Add a child type with cli_args and the replace flag.""" + reg = _ensure_registry(context) + cli_args = [{"name": n} for n in _names_from_csv(args_csv)] + reg[child] = _make_child_entry(parent, cli_args=cli_args, cli_args_replace=True) + + +@given('a type-inherit parent "{name}" with child_types "{types_csv}"') +def step_parent_child_types(context: Any, name: str, types_csv: str) -> None: + """Add a root type with specific child_types.""" + reg = _ensure_registry(context) + reg[name] = _make_root_entry(child_types=_names_from_csv(types_csv)) + + +@given( + 'a type-inherit child "{child}" inheriting from "{parent}" with child_types "{types_csv}"' +) +def step_child_child_types( + context: Any, child: str, parent: str, types_csv: str +) -> None: + """Add a child type with its own child_types.""" + reg = _ensure_registry(context) + reg[child] = _make_child_entry(parent, child_types=_names_from_csv(types_csv)) + + +@given( + 'a type-inherit child "{child}" inheriting from "{parent}" with child_types "{types_csv}" and child_types_replace' +) +def step_child_child_types_replace( + context: Any, child: str, parent: str, types_csv: str +) -> None: + """Add a child type with child_types and the replace flag.""" + reg = _ensure_registry(context) + reg[child] = _make_child_entry( + parent, child_types=_names_from_csv(types_csv), child_types_replace=True + ) + + +@given('a type-inherit parent "{name}" with parent_types "{types_csv}"') +def step_parent_parent_types(context: Any, name: str, types_csv: str) -> None: + """Add a root type with specific parent_types.""" + reg = _ensure_registry(context) + reg[name] = _make_root_entry(parent_types=_names_from_csv(types_csv)) + + +@given( + 'a type-inherit child "{child}" inheriting from "{parent}" with parent_types "{types_csv}"' +) +def step_child_parent_types( + context: Any, child: str, parent: str, types_csv: str +) -> None: + """Add a child type with its own parent_types.""" + reg = _ensure_registry(context) + reg[child] = _make_child_entry(parent, parent_types=_names_from_csv(types_csv)) + + +@given( + 'a type-inherit child "{child}" inheriting from "{parent}" with parent_types "{types_csv}" and parent_types_replace' +) +def step_child_parent_types_replace( + context: Any, child: str, parent: str, types_csv: str +) -> None: + """Add a child type with parent_types and the replace flag.""" + reg = _ensure_registry(context) + reg[child] = _make_child_entry( + parent, parent_types=_names_from_csv(types_csv), parent_types_replace=True + ) + + +@then('the type-inherit resolved cli_args names are "{expected_csv}"') +def step_resolved_cli_args_names(context: Any, expected_csv: str) -> None: + """Assert the resolved cli_args name list (order-sensitive).""" + resolved = context.type_inherit_resolved # type: ignore[attr-defined] + names = [a["name"] for a in resolved.get("cli_args", [])] + expected = _names_from_csv(expected_csv) + assert names == expected, f"Expected cli_args names {expected}, got {names}" + + +@then("the type-inherit resolved cli_args count is {count:d}") +def step_resolved_cli_args_count(context: Any, count: int) -> None: + """Assert the number of resolved cli_args.""" + resolved = context.type_inherit_resolved # type: ignore[attr-defined] + actual = len(resolved.get("cli_args", [])) + assert actual == count, f"Expected {count} cli_args, got {actual}" + + +@then('the type-inherit resolved child_types are "{expected_csv}"') +def step_resolved_child_types(context: Any, expected_csv: str) -> None: + """Assert the resolved child_types list.""" + resolved = context.type_inherit_resolved # type: ignore[attr-defined] + actual = resolved.get("child_types", []) + expected = _names_from_csv(expected_csv) + assert actual == expected, f"Expected child_types {expected}, got {actual}" + + +@then('the type-inherit resolved parent_types are "{expected_csv}"') +def step_resolved_parent_types(context: Any, expected_csv: str) -> None: + """Assert the resolved parent_types list.""" + resolved = context.type_inherit_resolved # type: ignore[attr-defined] + actual = resolved.get("parent_types", []) + expected = _names_from_csv(expected_csv) + assert actual == expected, f"Expected parent_types {expected}, got {actual}" + + +# ═══════════════════════════════════════════════════════════════ +# 5. Polymorphism — GIVEN / WHEN / THEN +# ═══════════════════════════════════════════════════════════════ + + +@when('I check type-inherit is_subtype_of "{child}" and "{ancestor}"') +def step_is_subtype_of(context: Any, child: str, ancestor: str) -> None: + """Run the is_subtype_of polymorphic check.""" + reg = _ensure_registry(context) + context.type_inherit_subtype_result = is_subtype_of(child, ancestor, reg) # type: ignore[attr-defined] + + +@then("the type-inherit subtype check is true") +def step_subtype_true(context: Any) -> None: + """Assert is_subtype_of returned True.""" + result = context.type_inherit_subtype_result # type: ignore[attr-defined] + assert result is True, f"Expected True, got {result}" + + +@then("the type-inherit subtype check is false") +def step_subtype_false(context: Any) -> None: + """Assert is_subtype_of returned False.""" + result = context.type_inherit_subtype_result # type: ignore[attr-defined] + assert result is False, f"Expected False, got {result}" + + +@given('a type-inherit tool "{tool_name}" bound to resource type "{rt}"') +def step_register_tool_bound(context: Any, tool_name: str, rt: str) -> None: + """Register a tool that declares a resource binding.""" + tool_reg = _ensure_tool_registry(context) + spec = ToolSpec( + name=tool_name, + description=f"Tool bound to {rt}", + handler=_NOOP_HANDLER, + source_metadata={"resource_bindings": [{"resource_type": rt}]}, + ) + tool_reg.register(spec) + + +@when('I find type-inherit tools for resource type "{rt}"') +def step_find_tools(context: Any, rt: str) -> None: + """Use ToolRegistry.find_tools_for_resource to match tools.""" + tool_reg = _ensure_tool_registry(context) + type_reg = _ensure_registry(context) + context.type_inherit_matched_tools = tool_reg.find_tools_for_resource(rt, type_reg) # type: ignore[attr-defined] + + +@then('the type-inherit matched tools include "{tool_name}"') +def step_matched_tools_include(context: Any, tool_name: str) -> None: + """Assert the matched tools list includes the named tool.""" + matched = context.type_inherit_matched_tools # type: ignore[attr-defined] + names = [t.name for t in matched] + assert tool_name in names, f"Expected {tool_name!r} in {names}" + + +@then("the type-inherit matched tools list is empty") +def step_matched_tools_empty(context: Any) -> None: + """Assert no tools were matched.""" + matched = context.type_inherit_matched_tools # type: ignore[attr-defined] + assert len(matched) == 0, f"Expected empty, got {[t.name for t in matched]}" + + +@when('I find type-inherit subtypes of "{ancestor}"') +def step_find_subtypes(context: Any, ancestor: str) -> None: + """Run find_subtypes for the given ancestor.""" + reg = _ensure_registry(context) + context.type_inherit_subtypes = find_subtypes(ancestor, reg) # type: ignore[attr-defined] + + +@then('the type-inherit subtypes include "{name}"') +def step_subtypes_include(context: Any, name: str) -> None: + """Assert the subtypes list includes the named type.""" + subtypes = context.type_inherit_subtypes # type: ignore[attr-defined] + assert name in subtypes, f"Expected {name!r} in {subtypes}" diff --git a/features/steps/resource_type_inheritance_steps.py b/features/steps/resource_type_inheritance_steps.py deleted file mode 100644 index 04988113..00000000 --- a/features/steps/resource_type_inheritance_steps.py +++ /dev/null @@ -1,1023 +0,0 @@ -"""Step definitions for resource_type_inheritance.feature. - -Tests the ADR-042 resource type inheritance engine including chain -resolution, field merging, polymorphic type checks, cycle detection, -depth limits, and parent-removal guards. -""" - -from __future__ import annotations - -from typing import Any - -from behave import given, then, when - -from cleveragents.resource.inheritance import ( - ResourceTypeCircularInheritanceError, - ResourceTypeInheritanceDepthError, - ResourceTypeParentNotFoundError, - ResourceTypeParentRemovalError, - find_subtypes, - is_subtype_of, - resolve_fields, - resolve_inheritance_chain, - validate_chain, -) -from cleveragents.resource.schema import ResourceTypeConfigSchema -from cleveragents.tool.registry import ToolRegistry -from cleveragents.tool.runtime import ToolSpec - -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - -_NOOP_HANDLER = lambda **kw: None # noqa: E731 - - -def _make_root_entry( - *, - description: str = "Root type", - handler: str | None = None, - cli_args: list[dict[str, Any]] | None = None, - child_types: list[str] | None = None, - parent_types: list[str] | None = None, -) -> dict[str, Any]: - """Create a minimal root type registry entry (no inherits).""" - entry: dict[str, Any] = {"description": description} - if handler is not None: - entry["handler"] = handler - if cli_args is not None: - entry["cli_args"] = cli_args - if child_types is not None: - entry["child_types"] = child_types - if parent_types is not None: - entry["parent_types"] = parent_types - return entry - - -def _make_child_entry( - parent: str, - *, - description: str | None = None, - handler: str | None = None, - cli_args: list[dict[str, Any]] | None = None, - cli_args_replace: bool = False, - child_types: list[str] | None = None, - child_types_replace: bool = False, - parent_types: list[str] | None = None, - parent_types_replace: bool = False, -) -> dict[str, Any]: - """Create a child type registry entry pointing at *parent*.""" - entry: dict[str, Any] = {"inherits": parent} - if description is not None: - entry["description"] = description - if handler is not None: - entry["handler"] = handler - if cli_args is not None: - entry["cli_args"] = cli_args - if cli_args_replace: - entry["cli_args_replace"] = True - if child_types is not None: - entry["child_types"] = child_types - if child_types_replace: - entry["child_types_replace"] = True - if parent_types is not None: - entry["parent_types"] = parent_types - if parent_types_replace: - entry["parent_types_replace"] = True - return entry - - -def _names_from_csv(csv: str) -> list[str]: - """Split a comma-separated string into stripped tokens.""" - return [t.strip() for t in csv.split(",") if t.strip()] - - -def _ensure_registry(context: Any) -> dict[str, Any]: - """Return (and lazily create) the type registry on *context*.""" - if not hasattr(context, "type_inherit_registry"): - context.type_inherit_registry = {} # type: ignore[attr-defined] - return context.type_inherit_registry # type: ignore[attr-defined] - - -def _ensure_tool_registry(context: Any) -> ToolRegistry: - """Return (and lazily create) the ToolRegistry on *context*.""" - if not hasattr(context, "type_inherit_tool_registry"): - context.type_inherit_tool_registry = ToolRegistry() # type: ignore[attr-defined] - return context.type_inherit_tool_registry # type: ignore[attr-defined] - - -# ═══════════════════════════════════════════════════════════════ -# 1. Schema validation — GIVEN / WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@given('a type-inherit YAML string with inherits "{parent}"') -def step_yaml_with_inherits(context: Any, parent: str) -> None: - """Prepare a YAML string that declares an inherits field.""" - context.type_inherit_yaml = ( # type: ignore[attr-defined] - f"name: acme/child-type\n" - f"resource_kind: physical\n" - f"sandbox_strategy: none\n" - f"inherits: {parent}\n" - ) - - -@given("a type-inherit YAML string with no inherits") -def step_yaml_without_inherits(context: Any) -> None: - """Prepare a YAML string with no inherits field.""" - context.type_inherit_yaml = ( # type: ignore[attr-defined] - "name: acme/root-type\nresource_kind: physical\nsandbox_strategy: none\n" - ) - - -@given('a type-inherit YAML string where name equals inherits "{name}"') -def step_yaml_self_inherit(context: Any, name: str) -> None: - """Prepare a YAML string where name == inherits (self-loop).""" - context.type_inherit_yaml = ( # type: ignore[attr-defined] - f"name: {name}\n" - f"resource_kind: physical\n" - f"sandbox_strategy: none\n" - f"inherits: {name}\n" - ) - - -@given('a type-inherit built-in YAML inheriting from custom "{parent}"') -def step_yaml_builtin_inherits_custom(context: Any, parent: str) -> None: - """Prepare a YAML for a built-in type inheriting from a custom type.""" - context.type_inherit_yaml = ( # type: ignore[attr-defined] - "name: my-builtin\n" - "resource_kind: physical\n" - "sandbox_strategy: none\n" - "built_in: true\n" - f"inherits: {parent}\n" - ) - - -@given("a type-inherit YAML string with cli_args_replace true") -def step_yaml_with_replace_flag(context: Any) -> None: - """Prepare a YAML string with a replace flag enabled.""" - context.type_inherit_yaml = ( # type: ignore[attr-defined] - "name: acme/child-type\n" - "resource_kind: physical\n" - "sandbox_strategy: none\n" - "inherits: container-instance\n" - "cli_args_replace: true\n" - ) - - -@when("I parse the type-inherit YAML via ResourceTypeConfigSchema") -def step_parse_yaml(context: Any) -> None: - """Parse the prepared YAML via the schema model.""" - context.type_inherit_schema = ResourceTypeConfigSchema.from_yaml( # type: ignore[attr-defined] - context.type_inherit_yaml # type: ignore[attr-defined] - ) - - -@when("I attempt to parse the type-inherit YAML expecting an error") -def step_parse_yaml_error(context: Any) -> None: - """Attempt to parse YAML and capture any validation error.""" - try: - ResourceTypeConfigSchema.from_yaml( - context.type_inherit_yaml # type: ignore[attr-defined] - ) - context.type_inherit_parse_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_parse_error = exc # type: ignore[attr-defined] - - -@then('the type-inherit schema inherits field is "{value}"') -def step_schema_inherits_value(context: Any, value: str) -> None: - """Assert the parsed schema has the expected inherits value.""" - schema = context.type_inherit_schema # type: ignore[attr-defined] - assert schema.inherits == value, ( - f"Expected inherits={value!r}, got {schema.inherits!r}" - ) - - -@then("the type-inherit schema inherits field is null") -def step_schema_inherits_null(context: Any) -> None: - """Assert the parsed schema has inherits=None.""" - schema = context.type_inherit_schema # type: ignore[attr-defined] - assert schema.inherits is None, f"Expected inherits=None, got {schema.inherits!r}" - - -@then('the type-inherit parse error mentions "{fragment}"') -def step_parse_error_mentions(context: Any, fragment: str) -> None: - """Assert the captured parse error contains the expected fragment.""" - err = context.type_inherit_parse_error # type: ignore[attr-defined] - assert err is not None, "Expected a parse error but none was raised" - assert fragment.lower() in str(err).lower(), ( - f"Expected error to mention {fragment!r}, got: {err}" - ) - - -@then("the type-inherit schema cli_args_replace is true") -def step_schema_cli_args_replace(context: Any) -> None: - """Assert the replace flag is True.""" - schema = context.type_inherit_schema # type: ignore[attr-defined] - assert schema.cli_args_replace is True, ( - f"Expected cli_args_replace=True, got {schema.cli_args_replace!r}" - ) - - -# ═══════════════════════════════════════════════════════════════ -# 2. Field resolution — GIVEN / WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@given('a type-inherit registry with parent "{name}" description "{desc}"') -def step_registry_parent_with_desc(context: Any, name: str, desc: str) -> None: - """Add a root type with a specific description.""" - reg = _ensure_registry(context) - reg[name] = _make_root_entry(description=desc) - - -@given( - 'a type-inherit child "{child}" inheriting from "{parent}" with description "{desc}"' -) -def step_registry_child_with_desc( - context: Any, child: str, parent: str, desc: str -) -> None: - """Add a child type with a specific description.""" - reg = _ensure_registry(context) - reg[child] = _make_child_entry(parent, description=desc) - - -@given('a type-inherit registry with parent "{name}" having handler "{handler}"') -def step_registry_parent_with_handler(context: Any, name: str, handler: str) -> None: - """Add a root type with a specific handler.""" - reg = _ensure_registry(context) - reg[name] = _make_root_entry(handler=handler) - - -@given('a type-inherit child "{child}" inheriting from "{parent}" without handler') -def step_registry_child_no_handler(context: Any, child: str, parent: str) -> None: - """Add a child type that does not set its own handler.""" - reg = _ensure_registry(context) - reg[child] = _make_child_entry(parent) - - -@when('I resolve type-inherit fields for "{name}"') -def step_resolve_fields(context: Any, name: str) -> None: - """Resolve merged fields for the given type.""" - reg = _ensure_registry(context) - context.type_inherit_resolved = resolve_fields(name, reg) # type: ignore[attr-defined] - - -@then('the type-inherit resolved description is "{desc}"') -def step_resolved_desc(context: Any, desc: str) -> None: - """Assert the resolved description.""" - resolved = context.type_inherit_resolved # type: ignore[attr-defined] - assert resolved["description"] == desc, ( - f"Expected description={desc!r}, got {resolved['description']!r}" - ) - - -@then('the type-inherit resolved handler is "{handler}"') -def step_resolved_handler(context: Any, handler: str) -> None: - """Assert the resolved handler.""" - resolved = context.type_inherit_resolved # type: ignore[attr-defined] - assert resolved.get("handler") == handler, ( - f"Expected handler={handler!r}, got {resolved.get('handler')!r}" - ) - - -# ═══════════════════════════════════════════════════════════════ -# 3. Collection merging — GIVEN / WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@given('a type-inherit parent "{name}" with cli_args "{args_csv}"') -def step_parent_cli_args(context: Any, name: str, args_csv: str) -> None: - """Add a root type whose cli_args contain the given named arguments.""" - reg = _ensure_registry(context) - cli_args = [{"name": n} for n in _names_from_csv(args_csv)] - reg[name] = _make_root_entry(cli_args=cli_args) - - -@given( - 'a type-inherit child "{child}" inheriting from "{parent}" with cli_args "{args_csv}"' -) -def step_child_cli_args(context: Any, child: str, parent: str, args_csv: str) -> None: - """Add a child type with its own cli_args.""" - reg = _ensure_registry(context) - cli_args = [{"name": n} for n in _names_from_csv(args_csv)] - reg[child] = _make_child_entry(parent, cli_args=cli_args) - - -@given( - 'a type-inherit child "{child}" inheriting from "{parent}" with cli_args "{args_csv}" and cli_args_replace' -) -def step_child_cli_args_replace( - context: Any, child: str, parent: str, args_csv: str -) -> None: - """Add a child type with cli_args and the replace flag.""" - reg = _ensure_registry(context) - cli_args = [{"name": n} for n in _names_from_csv(args_csv)] - reg[child] = _make_child_entry(parent, cli_args=cli_args, cli_args_replace=True) - - -@given('a type-inherit parent "{name}" with child_types "{types_csv}"') -def step_parent_child_types(context: Any, name: str, types_csv: str) -> None: - """Add a root type with specific child_types.""" - reg = _ensure_registry(context) - reg[name] = _make_root_entry(child_types=_names_from_csv(types_csv)) - - -@given( - 'a type-inherit child "{child}" inheriting from "{parent}" with child_types "{types_csv}"' -) -def step_child_child_types( - context: Any, child: str, parent: str, types_csv: str -) -> None: - """Add a child type with its own child_types.""" - reg = _ensure_registry(context) - reg[child] = _make_child_entry(parent, child_types=_names_from_csv(types_csv)) - - -@given( - 'a type-inherit child "{child}" inheriting from "{parent}" with child_types "{types_csv}" and child_types_replace' -) -def step_child_child_types_replace( - context: Any, child: str, parent: str, types_csv: str -) -> None: - """Add a child type with child_types and the replace flag.""" - reg = _ensure_registry(context) - reg[child] = _make_child_entry( - parent, child_types=_names_from_csv(types_csv), child_types_replace=True - ) - - -@given('a type-inherit parent "{name}" with parent_types "{types_csv}"') -def step_parent_parent_types(context: Any, name: str, types_csv: str) -> None: - """Add a root type with specific parent_types.""" - reg = _ensure_registry(context) - reg[name] = _make_root_entry(parent_types=_names_from_csv(types_csv)) - - -@given( - 'a type-inherit child "{child}" inheriting from "{parent}" with parent_types "{types_csv}"' -) -def step_child_parent_types( - context: Any, child: str, parent: str, types_csv: str -) -> None: - """Add a child type with its own parent_types.""" - reg = _ensure_registry(context) - reg[child] = _make_child_entry(parent, parent_types=_names_from_csv(types_csv)) - - -@given( - 'a type-inherit child "{child}" inheriting from "{parent}" with parent_types "{types_csv}" and parent_types_replace' -) -def step_child_parent_types_replace( - context: Any, child: str, parent: str, types_csv: str -) -> None: - """Add a child type with parent_types and the replace flag.""" - reg = _ensure_registry(context) - reg[child] = _make_child_entry( - parent, parent_types=_names_from_csv(types_csv), parent_types_replace=True - ) - - -@then('the type-inherit resolved cli_args names are "{expected_csv}"') -def step_resolved_cli_args_names(context: Any, expected_csv: str) -> None: - """Assert the resolved cli_args name list (order-sensitive).""" - resolved = context.type_inherit_resolved # type: ignore[attr-defined] - names = [a["name"] for a in resolved.get("cli_args", [])] - expected = _names_from_csv(expected_csv) - assert names == expected, f"Expected cli_args names {expected}, got {names}" - - -@then("the type-inherit resolved cli_args count is {count:d}") -def step_resolved_cli_args_count(context: Any, count: int) -> None: - """Assert the number of resolved cli_args.""" - resolved = context.type_inherit_resolved # type: ignore[attr-defined] - actual = len(resolved.get("cli_args", [])) - assert actual == count, f"Expected {count} cli_args, got {actual}" - - -@then('the type-inherit resolved child_types are "{expected_csv}"') -def step_resolved_child_types(context: Any, expected_csv: str) -> None: - """Assert the resolved child_types list.""" - resolved = context.type_inherit_resolved # type: ignore[attr-defined] - actual = resolved.get("child_types", []) - expected = _names_from_csv(expected_csv) - assert actual == expected, f"Expected child_types {expected}, got {actual}" - - -@then('the type-inherit resolved parent_types are "{expected_csv}"') -def step_resolved_parent_types(context: Any, expected_csv: str) -> None: - """Assert the resolved parent_types list.""" - resolved = context.type_inherit_resolved # type: ignore[attr-defined] - actual = resolved.get("parent_types", []) - expected = _names_from_csv(expected_csv) - assert actual == expected, f"Expected parent_types {expected}, got {actual}" - - -# ═══════════════════════════════════════════════════════════════ -# 4. Chain resolution — GIVEN / WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@given('a type-inherit registry with "{name}" as root') -def step_registry_root(context: Any, name: str) -> None: - """Add a root type (no inherits) to the registry.""" - reg = _ensure_registry(context) - reg[name] = _make_root_entry() - - -@given('a type-inherit type "{child}" inheriting from "{parent}"') -def step_registry_child(context: Any, child: str, parent: str) -> None: - """Add a child type to the registry.""" - reg = _ensure_registry(context) - reg[child] = _make_child_entry(parent) - - -@given('a type-inherit registry also has "{name}" as root') -def step_registry_additional_root(context: Any, name: str) -> None: - """Add another root type to the existing registry.""" - reg = _ensure_registry(context) - reg[name] = _make_root_entry() - - -@when('I resolve the type-inherit chain for "{name}"') -def step_resolve_chain(context: Any, name: str) -> None: - """Resolve the inheritance chain for the named type.""" - reg = _ensure_registry(context) - context.type_inherit_chain = resolve_inheritance_chain(name, reg) # type: ignore[attr-defined] - - -@when('I resolve the type-inherit chain for "{name}" expecting error') -def step_resolve_chain_error(context: Any, name: str) -> None: - """Attempt to resolve a chain and capture the raised exception.""" - reg = _ensure_registry(context) - try: - resolve_inheritance_chain(name, reg) - context.type_inherit_chain_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_chain_error = exc # type: ignore[attr-defined] - - -@when('I validate type-inherit chain for "{name}" inheriting "{parent}"') -def step_validate_chain(context: Any, name: str, parent: str) -> None: - """Run validate_chain for a new type.""" - reg = _ensure_registry(context) - context.type_inherit_validated_chain = validate_chain(name, parent, reg) # type: ignore[attr-defined] - - -@when( - 'I validate type-inherit chain for "{name}" inheriting "{parent}" expecting error' -) -def step_validate_chain_error(context: Any, name: str, parent: str) -> None: - """Run validate_chain and capture any error.""" - reg = _ensure_registry(context) - try: - validate_chain(name, parent, reg) - context.type_inherit_validation_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_validation_error = exc # type: ignore[attr-defined] - - -@then('the type-inherit chain is "{expected_csv}"') -def step_chain_value(context: Any, expected_csv: str) -> None: - """Assert the chain matches the expected comma-separated list.""" - chain = context.type_inherit_chain # type: ignore[attr-defined] - expected = _names_from_csv(expected_csv) - assert chain == expected, f"Expected chain {expected}, got {chain}" - - -@then('the type-inherit validated chain is "{expected_csv}"') -def step_validated_chain_value(context: Any, expected_csv: str) -> None: - """Assert the validated chain matches.""" - chain = context.type_inherit_validated_chain # type: ignore[attr-defined] - expected = _names_from_csv(expected_csv) - assert chain == expected, f"Expected validated chain {expected}, got {chain}" - - -@then("the type-inherit validation error is ResourceTypeParentNotFoundError") -def step_validation_error_parent_not_found(context: Any) -> None: - """Assert the validation raised ResourceTypeParentNotFoundError.""" - err = context.type_inherit_validation_error # type: ignore[attr-defined] - assert err is not None, "Expected an error but none was raised" - assert isinstance(err, ResourceTypeParentNotFoundError), ( - f"Expected ResourceTypeParentNotFoundError, got {type(err).__name__}: {err}" - ) - - -# ═══════════════════════════════════════════════════════════════ -# 5. Polymorphism — GIVEN / WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@when('I check type-inherit is_subtype_of "{child}" and "{ancestor}"') -def step_is_subtype_of(context: Any, child: str, ancestor: str) -> None: - """Run the is_subtype_of polymorphic check.""" - reg = _ensure_registry(context) - context.type_inherit_subtype_result = is_subtype_of(child, ancestor, reg) # type: ignore[attr-defined] - - -@then("the type-inherit subtype check is true") -def step_subtype_true(context: Any) -> None: - """Assert is_subtype_of returned True.""" - result = context.type_inherit_subtype_result # type: ignore[attr-defined] - assert result is True, f"Expected True, got {result}" - - -@then("the type-inherit subtype check is false") -def step_subtype_false(context: Any) -> None: - """Assert is_subtype_of returned False.""" - result = context.type_inherit_subtype_result # type: ignore[attr-defined] - assert result is False, f"Expected False, got {result}" - - -@given('a type-inherit tool "{tool_name}" bound to resource type "{rt}"') -def step_register_tool_bound(context: Any, tool_name: str, rt: str) -> None: - """Register a tool that declares a resource binding.""" - tool_reg = _ensure_tool_registry(context) - spec = ToolSpec( - name=tool_name, - description=f"Tool bound to {rt}", - handler=_NOOP_HANDLER, - source_metadata={"resource_bindings": [{"resource_type": rt}]}, - ) - tool_reg.register(spec) - - -@when('I find type-inherit tools for resource type "{rt}"') -def step_find_tools(context: Any, rt: str) -> None: - """Use ToolRegistry.find_tools_for_resource to match tools.""" - tool_reg = _ensure_tool_registry(context) - type_reg = _ensure_registry(context) - context.type_inherit_matched_tools = tool_reg.find_tools_for_resource(rt, type_reg) # type: ignore[attr-defined] - - -@then('the type-inherit matched tools include "{tool_name}"') -def step_matched_tools_include(context: Any, tool_name: str) -> None: - """Assert the matched tools list includes the named tool.""" - matched = context.type_inherit_matched_tools # type: ignore[attr-defined] - names = [t.name for t in matched] - assert tool_name in names, f"Expected {tool_name!r} in {names}" - - -@then("the type-inherit matched tools list is empty") -def step_matched_tools_empty(context: Any) -> None: - """Assert no tools were matched.""" - matched = context.type_inherit_matched_tools # type: ignore[attr-defined] - assert len(matched) == 0, f"Expected empty, got {[t.name for t in matched]}" - - -@when('I find type-inherit subtypes of "{ancestor}"') -def step_find_subtypes(context: Any, ancestor: str) -> None: - """Run find_subtypes for the given ancestor.""" - reg = _ensure_registry(context) - context.type_inherit_subtypes = find_subtypes(ancestor, reg) # type: ignore[attr-defined] - - -@then('the type-inherit subtypes include "{name}"') -def step_subtypes_include(context: Any, name: str) -> None: - """Assert the subtypes list includes the named type.""" - subtypes = context.type_inherit_subtypes # type: ignore[attr-defined] - assert name in subtypes, f"Expected {name!r} in {subtypes}" - - -# ═══════════════════════════════════════════════════════════════ -# 6. Cycle detection — GIVEN / WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@given('a type-inherit registry with "{name}" inheriting from "{parent}"') -def step_registry_entry_with_inherits(context: Any, name: str, parent: str) -> None: - """Add a type that inherits from another (may create a cycle).""" - reg = _ensure_registry(context) - reg[name] = {"inherits": parent} - - -@then("the type-inherit chain error is ResourceTypeCircularInheritanceError") -def step_chain_error_circular(context: Any) -> None: - """Assert the chain resolution raised a circular-inheritance error.""" - err = context.type_inherit_chain_error # type: ignore[attr-defined] - assert err is not None, "Expected an error but none was raised" - assert isinstance(err, ResourceTypeCircularInheritanceError), ( - f"Expected ResourceTypeCircularInheritanceError, got {type(err).__name__}: {err}" - ) - - -# ═══════════════════════════════════════════════════════════════ -# 7. Depth limit — GIVEN / WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@given("a type-inherit registry with a chain of depth {depth:d}") -def step_registry_deep_chain(context: Any, depth: int) -> None: - """Build a linear chain of the requested depth. - - depth=5 means 5 types: level-0 (root), level-1, ... level-4. - depth=6 means 6 types: level-0 (root), level-1, ... level-5. - """ - reg = _ensure_registry(context) - for i in range(depth): - name = f"acme/level-{i}" if i > 0 else "level-0" - if i == 0: - reg[name] = _make_root_entry(description=f"Level {i}") - else: - parent = f"acme/level-{i - 1}" if i > 1 else "level-0" - reg[name] = _make_child_entry(parent, description=f"Level {i}") - # Store the deepest type name for subsequent steps. - deepest = f"acme/level-{depth - 1}" if depth > 1 else "level-0" - context.type_inherit_deepest = deepest # type: ignore[attr-defined] - - -@when("I resolve the type-inherit chain for the deepest type expecting error") -def step_resolve_deepest_error(context: Any) -> None: - """Resolve chain for the deepest type, expecting an error.""" - reg = _ensure_registry(context) - deepest = context.type_inherit_deepest # type: ignore[attr-defined] - try: - resolve_inheritance_chain(deepest, reg) - context.type_inherit_chain_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_chain_error = exc # type: ignore[attr-defined] - - -@when("I resolve the type-inherit chain for the deepest type") -def step_resolve_deepest(context: Any) -> None: - """Resolve chain for the deepest type (should succeed).""" - reg = _ensure_registry(context) - deepest = context.type_inherit_deepest # type: ignore[attr-defined] - context.type_inherit_chain = resolve_inheritance_chain(deepest, reg) # type: ignore[attr-defined] - - -@then("the type-inherit chain error is ResourceTypeInheritanceDepthError") -def step_chain_error_depth(context: Any) -> None: - """Assert the chain resolution raised a depth-limit error.""" - err = context.type_inherit_chain_error # type: ignore[attr-defined] - assert err is not None, "Expected an error but none was raised" - assert isinstance(err, ResourceTypeInheritanceDepthError), ( - f"Expected ResourceTypeInheritanceDepthError, got {type(err).__name__}: {err}" - ) - - -@then("the type-inherit chain length is {length:d}") -def step_chain_length(context: Any, length: int) -> None: - """Assert the resolved chain has the expected length.""" - chain = context.type_inherit_chain # type: ignore[attr-defined] - assert len(chain) == length, f"Expected chain length {length}, got {len(chain)}" - - -# ═══════════════════════════════════════════════════════════════ -# 8. Parent removal guard — WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@when('I attempt to type-inherit remove "{name}"') -def step_attempt_remove(context: Any, name: str) -> None: - """Attempt to remove a type and raise if subtypes exist. - - Mirrors the guard in ``ResourceRegistryService.remove_type()`` - (ADR-042 Rule 5) using the in-memory registry. We cannot call the - real service here because these BDD tests are DB-free engine tests. - """ - reg = _ensure_registry(context) - try: - subtypes = find_subtypes(name, reg) - if subtypes: - raise ResourceTypeParentRemovalError( - f"Cannot remove '{name}': subtypes exist: {subtypes}" - ) - del reg[name] - context.type_inherit_removal_error = None # type: ignore[attr-defined] - except ResourceTypeParentRemovalError as exc: - context.type_inherit_removal_error = exc # type: ignore[attr-defined] - - -@then("the type-inherit removal error is ResourceTypeParentRemovalError") -def step_removal_error(context: Any) -> None: - """Assert the removal raised ResourceTypeParentRemovalError.""" - err = context.type_inherit_removal_error # type: ignore[attr-defined] - assert err is not None, "Expected an error but none was raised" - assert isinstance(err, ResourceTypeParentRemovalError), ( - f"Expected ResourceTypeParentRemovalError, got {type(err).__name__}: {err}" - ) - - -# ═══════════════════════════════════════════════════════════════ -# 9. Additional coverage — GIVEN / WHEN / THEN -# ═══════════════════════════════════════════════════════════════ - - -@given( - 'a type-inherit grandchild "{grandchild}" inheriting from "{parent}" with cli_args "{args_csv}"' -) -def step_grandchild_cli_args( - context: Any, grandchild: str, parent: str, args_csv: str -) -> None: - """Add a grandchild type with its own cli_args.""" - reg = _ensure_registry(context) - cli_args = [{"name": n} for n in _names_from_csv(args_csv)] - reg[grandchild] = _make_child_entry(parent, cli_args=cli_args) - - -@given('a type-inherit registry with "{name}" as root with handler "{handler}"') -def step_registry_root_with_handler(context: Any, name: str, handler: str) -> None: - """Add a root type with a specific handler reference.""" - reg = _ensure_registry(context) - reg[name] = _make_root_entry(handler=handler) - - -@when('I resolve type-inherit handler polymorphic for "{name}"') -def step_resolve_handler_polymorphic(context: Any, name: str) -> None: - """Resolve a handler via the polymorphic resolver.""" - from cleveragents.resource.handlers.resolver import resolve_handler_polymorphic - - reg = _ensure_registry(context) - context.type_inherit_resolved_handler = resolve_handler_polymorphic(name, reg) # type: ignore[attr-defined] - - -@then("the type-inherit resolved handler is not None") -def step_resolved_handler_not_none(context: Any) -> None: - """Assert the resolved handler is not None.""" - handler = context.type_inherit_resolved_handler # type: ignore[attr-defined] - assert handler is not None, "Expected a handler but got None" - - -@when( - 'I validate type-inherit chain for built-in "{name}" inheriting "{parent}" expecting error' -) -def step_validate_chain_builtin_error(context: Any, name: str, parent: str) -> None: - """Run validate_chain with is_built_in=True and capture any error.""" - reg = _ensure_registry(context) - try: - validate_chain(name, parent, reg, is_built_in=True) - context.type_inherit_validation_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_validation_error = exc # type: ignore[attr-defined] - - -@then("the type-inherit validation error is ValueError") -def step_validation_error_value_error(context: Any) -> None: - """Assert the validation raised a ValueError (or subclass).""" - err = context.type_inherit_validation_error # type: ignore[attr-defined] - assert err is not None, "Expected an error but none was raised" - assert isinstance(err, ValueError), ( - f"Expected ValueError, got {type(err).__name__}: {err}" - ) - - -# ═══════════════════════════════════════════════════════════════ -# 10. Coverage edge-case steps -# ═══════════════════════════════════════════════════════════════ - - -@then("the type-inherit chain error is ResourceTypeParentNotFoundError") -def step_chain_error_parent_not_found(context: Any) -> None: - """Assert the chain error is ResourceTypeParentNotFoundError.""" - err = context.type_inherit_chain_error # type: ignore[attr-defined] - assert err is not None, "Expected an error but none was raised" - assert isinstance(err, ResourceTypeParentNotFoundError), ( - f"Expected ResourceTypeParentNotFoundError, got {type(err).__name__}: {err}" - ) - - -@when('I validate type-inherit chain for "{name}" inheriting nothing') -def step_validate_chain_no_parent(context: Any, name: str) -> None: - """Run validate_chain with inherits=None.""" - reg = _ensure_registry(context) - context.type_inherit_validated_chain = validate_chain(name, None, reg) # type: ignore[attr-defined] - - -@given('the type-inherit registry entry for "{name}" is removed') -def step_remove_registry_entry(context: Any, name: str) -> None: - """Remove an entry from the in-memory registry.""" - reg = _ensure_registry(context) - reg.pop(name, None) - - -@then('the type-inherit resolved fields should not contain "{key}"') -def step_resolved_no_key(context: Any, key: str) -> None: - """Assert the resolved fields do NOT contain the given key.""" - resolved = context.type_inherit_resolved # type: ignore[attr-defined] - assert key not in resolved, ( - f"Expected key '{key}' absent, but found: {resolved.get(key)}" - ) - - -class _ObjEntry: - """Minimal object-style registry entry for testing _get_inherits.""" - - def __init__(self, inherits: str | None = None, **kwargs: Any) -> None: - self.inherits = inherits - for k, v in kwargs.items(): - setattr(self, k, v) - - -@given('a type-inherit registry with object-style entry "{name}" inheriting "{parent}"') -def step_registry_obj_entry(context: Any, name: str, parent: str) -> None: - """Add an object-style (non-dict) entry to the registry.""" - reg = _ensure_registry(context) - reg[name] = _ObjEntry(inherits=parent) - - -class _PydanticLikeEntry: - """Fake entry with model_dump() for testing _to_dict branch.""" - - def __init__(self, **kwargs: Any) -> None: - self._data = kwargs - - def model_dump(self) -> dict[str, Any]: - return dict(self._data) - - -@given( - 'a type-inherit registry with pydantic-style entry "{name}" having description "{desc}"' -) -def step_registry_pydantic_entry(context: Any, name: str, desc: str) -> None: - """Add a pydantic-model-like entry to the registry.""" - reg = _ensure_registry(context) - reg[name] = _PydanticLikeEntry(description=desc) - - -@given('a type-inherit parent "{name}" with no cli_args') -def step_parent_no_cli_args(context: Any, name: str) -> None: - """Add a root type with empty cli_args.""" - reg = _ensure_registry(context) - reg[name] = _make_root_entry(cli_args=[]) - - -@given('a type-inherit child "{child}" inheriting from "{parent}" with no cli_args') -def step_child_no_cli_args(context: Any, child: str, parent: str) -> None: - """Add a child type with no cli_args.""" - reg = _ensure_registry(context) - reg[child] = _make_child_entry(parent, cli_args=[]) - - -@when('I resolve type-inherit handler polymorphic for "{name}" expecting error') -def step_resolve_handler_polymorphic_error(context: Any, name: str) -> None: - """Resolve handler and capture any error.""" - from cleveragents.resource.handlers.resolver import resolve_handler_polymorphic - - reg = _ensure_registry(context) - try: - resolve_handler_polymorphic(name, reg) - context.type_inherit_handler_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_handler_error = exc # type: ignore[attr-defined] - - -@then("the type-inherit handler error is HandlerResolutionError") -def step_handler_error_check(context: Any) -> None: - """Assert the handler resolution raised HandlerResolutionError.""" - from cleveragents.resource.handlers.resolver import HandlerResolutionError - - err = context.type_inherit_handler_error # type: ignore[attr-defined] - assert err is not None, "Expected an error but none was raised" - assert isinstance(err, HandlerResolutionError), ( - f"Expected HandlerResolutionError, got {type(err).__name__}: {err}" - ) - - -# -- ToolRegistry coverage steps ----------------------------------------- - - -@given('a type-inherit tool registry with tool "{name}" bound to "{res_type}"') -def step_tool_registry_add(context: Any, name: str, res_type: str) -> None: - """Add a tool to the tool registry.""" - tool_reg = _ensure_tool_registry(context) - spec = ToolSpec( - name=name, - description=f"Tool {name}", - handler=_NOOP_HANDLER, - source="test", - source_metadata={"resource_bindings": [{"resource_type": res_type}]}, - ) - tool_reg.register(spec) - - -@when('I add type-inherit tool "{name}" again expecting error') -def step_tool_registry_add_dup(context: Any, name: str) -> None: - """Attempt to add a duplicate tool.""" - tool_reg = _ensure_tool_registry(context) - spec = ToolSpec( - name=name, - description="dup", - handler=_NOOP_HANDLER, - source="test", - source_metadata={}, - ) - try: - tool_reg.register(spec) - context.type_inherit_tool_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_tool_error = exc # type: ignore[attr-defined] - - -@then("the type-inherit tool error is ToolError") -def step_tool_error_check(context: Any) -> None: - """Assert a ToolError was raised.""" - from cleveragents.tool.runtime import ToolError - - err = context.type_inherit_tool_error # type: ignore[attr-defined] - assert err is not None, "Expected an error but none was raised" - assert isinstance(err, ToolError), ( - f"Expected ToolError, got {type(err).__name__}: {err}" - ) - - -@given("a type-inherit empty tool registry") -def step_empty_tool_registry(context: Any) -> None: - """Create an empty tool registry.""" - context.type_inherit_tool_registry = ToolRegistry() # type: ignore[attr-defined] - - -@when('I get type-inherit tool "{name}"') -def step_tool_registry_get(context: Any, name: str) -> None: - """Lookup a tool by name.""" - tool_reg = _ensure_tool_registry(context) - context.type_inherit_tool_result = tool_reg.get(name) # type: ignore[attr-defined] - - -@then("the type-inherit tool result is None") -def step_tool_result_none(context: Any) -> None: - """Assert the tool lookup returned None.""" - assert context.type_inherit_tool_result is None # type: ignore[attr-defined] - - -@when('I list type-inherit tools with namespace "{ns}"') -def step_tool_registry_list(context: Any, ns: str) -> None: - """List tools filtered by namespace.""" - tool_reg = _ensure_tool_registry(context) - context.type_inherit_tools_list = tool_reg.list_tools(namespace=ns) # type: ignore[attr-defined] - - -@then("the type-inherit tools list has {count:d} entry") -def step_tools_list_count(context: Any, count: int) -> None: - """Assert the tools list has the expected number of entries.""" - result = context.type_inherit_tools_list # type: ignore[attr-defined] - assert len(result) == count, f"Expected {count}, got {len(result)}" - - -@when('I remove type-inherit tool "{name}"') -def step_tool_registry_remove(context: Any, name: str) -> None: - """Remove a tool from the registry.""" - tool_reg = _ensure_tool_registry(context) - context.type_inherit_tool_remove_result = tool_reg.remove(name) # type: ignore[attr-defined] - - -@then("the type-inherit tool remove result is true") -def step_tool_remove_true(context: Any) -> None: - """Assert tool removal returned True.""" - assert context.type_inherit_tool_remove_result is True # type: ignore[attr-defined] - - -@then("the type-inherit tool remove result is false") -def step_tool_remove_false(context: Any) -> None: - """Assert tool removal returned False.""" - assert context.type_inherit_tool_remove_result is False # type: ignore[attr-defined] - - -@given('a type-inherit tool registry with unbound tool "{name}"') -def step_tool_registry_unbound(context: Any, name: str) -> None: - """Add a tool with no resource bindings.""" - tool_reg = _ensure_tool_registry(context) - spec = ToolSpec( - name=name, - description=f"Unbound tool {name}", - handler=_NOOP_HANDLER, - source="test", - source_metadata={}, - ) - tool_reg.register(spec) - - -@when('I list type-inherit tools with source "{src}"') -def step_tool_registry_list_by_source(context: Any, src: str) -> None: - """List tools filtered by source.""" - tool_reg = _ensure_tool_registry(context) - context.type_inherit_tools_list = tool_reg.list_tools(source=src) # type: ignore[attr-defined] - - -@when("I resolve type-inherit handler with empty reference expecting error") -def step_resolve_handler_empty(context: Any) -> None: - """Try to resolve an empty handler reference.""" - from cleveragents.resource.handlers.resolver import resolve_handler - - try: - resolve_handler("") - context.type_inherit_handler_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_handler_error = exc # type: ignore[attr-defined] - - -@when('I resolve type-inherit handler with reference "{ref}" expecting error') -def step_resolve_handler_bad_ref(context: Any, ref: str) -> None: - """Try to resolve a handler with a bad reference.""" - from cleveragents.resource.handlers.resolver import ( - clear_handler_cache, - resolve_handler, - ) - - clear_handler_cache() - try: - resolve_handler(ref) - context.type_inherit_handler_error = None # type: ignore[attr-defined] - except Exception as exc: - context.type_inherit_handler_error = exc # type: ignore[attr-defined] diff --git a/src/cleveragents/application/services/resource_registry_service.py b/src/cleveragents/application/services/resource_registry_service.py index 9ee3fcb3..5c660e55 100644 --- a/src/cleveragents/application/services/resource_registry_service.py +++ b/src/cleveragents/application/services/resource_registry_service.py @@ -276,16 +276,49 @@ class ResourceRegistryService: registered: list[str] = [] session = self._session() try: + # Build a registry of already-persisted types so that + # validate_chain can check inheritance for new built-ins + # (e.g. devcontainer-instance inherits container-instance). + existing_registry: dict[str, dict[str, Any]] = {} + for row in session.query(ResourceTypeModel).all(): + rn = str(row.name) + ri = getattr(row, "inherits", None) + existing_registry[rn] = { + "inherits": str(ri) if ri else None, + } + for builtin_def in _BUILTIN_TYPES: name = builtin_def["name"] + if name in existing_registry: + continue + existing = session.query(ResourceTypeModel).filter_by(name=name).first() if existing is not None: + existing_registry[name] = { + "inherits": getattr(existing, "inherits", None), + } continue spec = ResourceTypeSpec.from_config(builtin_def) + + # Validate inheritance chain for built-ins that declare + # inherits (e.g. devcontainer-instance -> container-instance). + if spec.inherits is not None: + validate_chain( + spec.name, + spec.inherits, + existing_registry, + is_built_in=spec.built_in, + ) + db_model = _spec_to_db(spec, source="builtin") session.add(db_model) session.flush() + # Track the newly registered type so subsequent + # built-ins can reference it as a parent. + existing_registry[name] = { + "inherits": spec.inherits, + } registered.append(name) logger.info("Registered built-in resource type: %s", name) @@ -329,6 +362,26 @@ class ResourceRegistryService: session = self._session() try: + # Acquire a row-level lock on the parent type (if any) to + # prevent TOCTOU races where two concurrent registrations + # could both pass cycle validation before either commits. + if spec.inherits is not None: + parent_row = ( + session.query(ResourceTypeModel) + .filter_by(name=spec.inherits) + .with_for_update() + .first() + ) + if parent_row is None: + raise ValidationError( + message=( + f"Parent type '{spec.inherits}' is not registered. " + f"Cannot register '{spec.name}' with " + f"inherits='{spec.inherits}'." + ), + details={"name": spec.name, "inherits": spec.inherits}, + ) + existing = ( session.query(ResourceTypeModel).filter_by(name=spec.name).first() ) @@ -547,9 +600,16 @@ class ResourceRegistryService: for row in rows: name = str(row.name) raw_inherits = getattr(row, "inherits", None) + # Derive built_in from the namespace column: built-in types + # have namespace="builtin". Fall back to the name heuristic + # only when the column is unavailable. + raw_ns = getattr(row, "namespace", None) + is_built_in = ( + str(raw_ns) == "builtin" if raw_ns is not None else "/" not in name + ) registry[name] = { "inherits": str(raw_inherits) if raw_inherits else None, - "built_in": "/" not in name, + "built_in": is_built_in, } return registry finally: diff --git a/src/cleveragents/cli/commands/resource.py b/src/cleveragents/cli/commands/resource.py index a4782565..2a390474 100644 --- a/src/cleveragents/cli/commands/resource.py +++ b/src/cleveragents/cli/commands/resource.py @@ -117,7 +117,6 @@ def _resource_type_dict(spec: Any) -> dict[str, object]: "default": arg.default, } ) - inherits = getattr(spec, "inherits", None) result: dict[str, object] = { "name": spec.name, "description": spec.description or "", @@ -125,14 +124,13 @@ def _resource_type_dict(spec: Any) -> dict[str, object]: "sandbox_strategy": str(spec.sandbox_strategy), "user_addable": spec.user_addable, "built_in": spec.built_in, + "inherits": getattr(spec, "inherits", None), "cli_args": cli_args_list, "parent_types": spec.parent_types, "child_types": spec.child_types, "handler": spec.handler, "capabilities": spec.capabilities, } - if inherits is not None: - result["inherits"] = inherits return result @@ -389,8 +387,8 @@ def _print_type_panel(spec: Any) -> None: service = _get_registry_service() chain = service.resolve_type_inheritance_chain(spec.name) chain_display = " -> ".join(chain) - except Exception: # CLI display; crash is worse than partial info - logger.debug("Failed to resolve chain for %s: %s", spec.name, "error") + except Exception as chain_exc: # CLI display; crash is worse than partial info + logger.debug("Failed to resolve chain for %s: %s", spec.name, chain_exc) chain_display = f"{spec.name} -> {inherits} -> ..." details = ( diff --git a/src/cleveragents/domain/models/core/resource_type.py b/src/cleveragents/domain/models/core/resource_type.py index e21566f1..874b9bda 100644 --- a/src/cleveragents/domain/models/core/resource_type.py +++ b/src/cleveragents/domain/models/core/resource_type.py @@ -478,9 +478,9 @@ class ResourceTypeSpec(BaseModel): ] if self.parent_types: - result["parent_types"] = self.parent_types + result["parent_types"] = list(self.parent_types) if self.child_types: - result["child_types"] = self.child_types + result["child_types"] = list(self.child_types) result["capabilities"] = self.capabilities diff --git a/src/cleveragents/resource/__init__.py b/src/cleveragents/resource/__init__.py index 25dcd3ef..24d7aaff 100644 --- a/src/cleveragents/resource/__init__.py +++ b/src/cleveragents/resource/__init__.py @@ -6,6 +6,7 @@ from cleveragents.resource.inheritance import ( ResourceTypeInheritanceDepthError, ResourceTypeParentNotFoundError, ResourceTypeParentRemovalError, + TypeRegistryMap, find_subtypes, is_subtype_of, resolve_fields, @@ -19,6 +20,7 @@ __all__ = [ "ResourceTypeInheritanceDepthError", "ResourceTypeParentNotFoundError", "ResourceTypeParentRemovalError", + "TypeRegistryMap", "find_subtypes", "is_subtype_of", "resolve_fields", diff --git a/src/cleveragents/resource/handlers/resolver.py b/src/cleveragents/resource/handlers/resolver.py index d7e059f5..738114f8 100644 --- a/src/cleveragents/resource/handlers/resolver.py +++ b/src/cleveragents/resource/handlers/resolver.py @@ -142,9 +142,23 @@ def resolve_handler_polymorphic( Raises: HandlerResolutionError: If no handler is found in the chain. """ - from cleveragents.resource.inheritance import resolve_inheritance_chain + from cleveragents.resource.inheritance import ( + ResourceTypeCircularInheritanceError, + ResourceTypeInheritanceDepthError, + ResourceTypeParentNotFoundError, + resolve_inheritance_chain, + ) - chain = resolve_inheritance_chain(type_name, type_registry) + try: + chain = resolve_inheritance_chain(type_name, type_registry) + except ( + ResourceTypeParentNotFoundError, + ResourceTypeCircularInheritanceError, + ResourceTypeInheritanceDepthError, + ) as exc: + raise HandlerResolutionError( + f"Cannot resolve handler for '{type_name}': {exc}" + ) from exc for ancestor in chain: entry = type_registry.get(ancestor) diff --git a/src/cleveragents/resource/inheritance.py b/src/cleveragents/resource/inheritance.py index ce03f45c..79455d09 100644 --- a/src/cleveragents/resource/inheritance.py +++ b/src/cleveragents/resource/inheritance.py @@ -28,10 +28,30 @@ See Also: from __future__ import annotations -import logging from typing import Any -logger = logging.getLogger(__name__) +import structlog + +logger: structlog.stdlib.BoundLogger = structlog.get_logger(__name__) + +#: Type alias for registry entries — each value is a dict or object with +#: an ``inherits`` key/attribute. Internal helpers also read ``handler``, +#: ``cli_args``, ``child_types``, ``parent_types``, and ``properties``. +TypeRegistryMap = dict[str, Any] + +__all__ = [ + "MAX_CHAIN_DEPTH", + "ResourceTypeCircularInheritanceError", + "ResourceTypeInheritanceDepthError", + "ResourceTypeParentNotFoundError", + "ResourceTypeParentRemovalError", + "TypeRegistryMap", + "find_subtypes", + "is_subtype_of", + "resolve_fields", + "resolve_inheritance_chain", + "validate_chain", +] #: Maximum inheritance chain depth (ADR-042 rule 2). MAX_CHAIN_DEPTH: int = 5 @@ -55,7 +75,7 @@ class ResourceTypeParentRemovalError(ValueError): def resolve_inheritance_chain( type_name: str, - type_registry: dict[str, Any], + type_registry: TypeRegistryMap, ) -> list[str]: """Walk the ``inherits`` links from *type_name* to the root type. @@ -74,11 +94,18 @@ def resolve_inheritance_chain( ``["devcontainer-instance", "container-instance"]``. Raises: - ResourceTypeParentNotFoundError: If a parent in the chain is - not in *type_registry*. + ResourceTypeParentNotFoundError: If *type_name* or any parent + in the chain is not in *type_registry*. ResourceTypeCircularInheritanceError: If a cycle is detected. ResourceTypeInheritanceDepthError: If the chain exceeds :data:`MAX_CHAIN_DEPTH` levels. + + .. note:: + + If *type_name* itself is not in *type_registry* and + ``allow_missing_root`` semantics are needed, callers (e.g. + ``is_subtype_of``) should catch + ``ResourceTypeParentNotFoundError``. """ chain: list[str] = [] visited: set[str] = set() @@ -100,18 +127,21 @@ def resolve_inheritance_chain( chain.append(current) entry = type_registry.get(current) - if entry is None and current != type_name: - raise ResourceTypeParentNotFoundError( - f"Parent type '{current}' (in chain for '{type_name}') " - "is not registered." - ) if entry is None: - # type_name itself is not in the registry. We return - # [type_name] instead of raising so that callers like - # is_subtype_of() can treat unregistered types as root - # types with no ancestors (returns False for any ancestor - # check). Service-level callers should verify existence - # before calling. + if current != type_name: + raise ResourceTypeParentNotFoundError( + f"Parent type '{current}' (in chain for '{type_name}') " + "is not registered." + ) + # type_name itself is not in the registry. Log a warning + # and return [type_name] so that callers like is_subtype_of() + # can treat unregistered types as root types with no ancestors + # (returns False for any ancestor check). Service-level + # callers should verify existence before calling. + logger.warning( + "resolve_inheritance_chain called for unregistered type", + type_name=type_name, + ) break parent = _get_inherits(entry) @@ -123,7 +153,7 @@ def resolve_inheritance_chain( def validate_chain( type_name: str, inherits: str | None, - type_registry: dict[str, Any], + type_registry: TypeRegistryMap, *, is_built_in: bool = False, ) -> list[str]: @@ -150,11 +180,17 @@ def validate_chain( ResourceTypeInheritanceDepthError: If the resulting chain would exceed :data:`MAX_CHAIN_DEPTH`. ValueError: If a built-in type tries to inherit from a custom - type. + type, or if *inherits* is whitespace-only. """ if inherits is None: return [type_name] + # Reject whitespace-only inherits values. + if not inherits.strip(): + raise ValueError( + f"'inherits' for '{type_name}' must not be empty or whitespace-only." + ) + # Rule 1: single inheritance is enforced by the schema (one field). # Rule 4: built-in types must not inherit from custom types. @@ -181,7 +217,7 @@ def validate_chain( def is_subtype_of( type_name: str, ancestor_name: str, - type_registry: dict[str, Any], + type_registry: TypeRegistryMap, ) -> bool: """Check whether *type_name* is the same as or a subtype of *ancestor_name*. @@ -216,21 +252,24 @@ def is_subtype_of( def resolve_fields( type_name: str, - type_registry: dict[str, Any], + type_registry: TypeRegistryMap, ) -> dict[str, Any]: """Resolve the merged fields for *type_name* along its inheritance chain. Field resolution (ADR-042 §Field Resolution): - Scalar fields: subtype's value overrides parent's. - - Collection fields (``cli_args``, ``child_types``, ``parent_types``): - additive merging by default; ``_replace: true`` replaces. + - Collection fields (``cli_args``, ``child_types``, ``parent_types``, + ``properties``): additive merging by default; + ``_replace: true`` replaces. Args: type_name: Name of the type to resolve. type_registry: Current registry contents. Returns: - A dict with the fully resolved field values. + A **defensive copy** of the fully resolved field values. + Callers may mutate the returned dict without affecting the + registry. """ chain = resolve_inheritance_chain(type_name, type_registry) @@ -276,21 +315,27 @@ def resolve_fields( if original is not None: merged["inherits"] = _get_inherits(original) - return merged + # Return a defensive copy so callers cannot corrupt the registry. + return dict(merged) def find_subtypes( ancestor_name: str, - type_registry: dict[str, Any], + type_registry: TypeRegistryMap, ) -> list[str]: """Find all types that inherit from *ancestor_name* (directly or transitively). + .. note:: + + The returned list does **not** include *ancestor_name* itself, + only its descendants. + Args: ancestor_name: The ancestor type name. type_registry: Current registry contents. Returns: - List of subtype names (does NOT include *ancestor_name* itself). + List of subtype names (excludes *ancestor_name* itself). """ subtypes: list[str] = [] for name in type_registry: @@ -339,10 +384,13 @@ def _merge_collection( For ``cli_args``: child entries with the same ``name`` replace parent entries (same-name replacement). For ``child_types`` and - ``parent_types``: union of both lists (deduplication). + ``parent_types``: union of both lists (deduplication). For + ``properties``: dict merge where child keys override parent keys. """ if field_name == "cli_args": return _merge_cli_args(parent_value, child_value) + if field_name == "properties": + return _merge_dict(parent_value, child_value) # child_types and parent_types: union, preserving order, child last. return _merge_string_list(parent_value, child_value) @@ -380,6 +428,21 @@ def _merge_cli_args( return merged +def _merge_dict( + parent: dict[str, Any], + child: dict[str, Any], +) -> dict[str, Any]: + """Merge two dicts with child keys overriding parent keys. + + Used for ``properties`` field merging where parent provides + defaults and child can override individual keys. + """ + merged = dict(parent) if parent else {} + if child: + merged.update(child) + return merged + + def _merge_string_list( parent: list[str], child: list[str],