From 7e55d522b4cb00e6973b49967c15bb720e142084 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 04:19:52 +0000 Subject: [PATCH] fix(domain): add missing execute hook to ToolLifecycle model to satisfy spec four-stage lifecycle - What was implemented - Added execute: str | None = Field(default=None, description="Hook for tool execution") to ToolLifecycle in src/cleveragents/domain/models/core/tool.py, completing the four-stage lifecycle (discover / activate / execute / deactivate) as required by the spec. - Updated Tool.as_cli_dict() to render the lifecycle block (including execute) when set, and to omit the lifecycle block entirely when no hooks are configured, keeping CLI output clean for tools without lifecycle hooks. - Created features/tool_lifecycle_execute_hook.feature with 8 Behave scenarios covering: - issue-capture TDD test - all four hooks (discover, activate, execute, deactivate) - execute defaults to None - execute set independently - from_config parsing - as_cli_dict rendering - Created features/steps/tool_lifecycle_execute_hook_steps.py with all step definitions implementing Given/When/Then for the scenarios. - Key design decisions - The execute field follows the same pattern as the existing discover, activate, and deactivate fields: str | None with Field(default=None). - as_cli_dict() renders the lifecycle block only when at least one hook is set (non-None), ensuring clean output for tools that do not utilize lifecycle hooks. - The issue-capture TDD scenario uses @tdd_issue and @tdd_issue_2820 tags (without @tdd_expected_fail since the fix is in place), aligning with the test-driven approach described in the metadata. - Impact and considerations - Changes are additive and backward-compatible; tools configured without lifecycle hooks will continue to render without a lifecycle block. - Introduces Behave-based acceptance tests to validate the four-stage lifecycle handling and CLI rendering. - Affects only ToolLifecycle modeling, CLI rendering, and associated tests; no changes to external APIs. ISSUES CLOSED: #2820 --- .../tool_lifecycle_execute_hook_steps.py | 259 ++++++++++++++++++ features/tool_lifecycle_execute_hook.feature | 58 ++++ src/cleveragents/domain/models/core/tool.py | 18 ++ 3 files changed, 335 insertions(+) create mode 100644 features/steps/tool_lifecycle_execute_hook_steps.py create mode 100644 features/tool_lifecycle_execute_hook.feature diff --git a/features/steps/tool_lifecycle_execute_hook_steps.py b/features/steps/tool_lifecycle_execute_hook_steps.py new file mode 100644 index 000000000..8c4a5b515 --- /dev/null +++ b/features/steps/tool_lifecycle_execute_hook_steps.py @@ -0,0 +1,259 @@ +"""Step definitions for ToolLifecycle execute hook tests (issue #2820).""" + +from typing import Any + +from behave import then, when +from behave.runner import Context + +from cleveragents.domain.models.core.tool import ( + Tool, + ToolLifecycle, + ToolSource, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_builtin_tool(**overrides: Any) -> Tool: + """Create a minimal builtin Tool with optional overrides.""" + defaults: dict[str, Any] = { + "name": "local/test-tool", + "description": "A test tool", + "source": ToolSource.BUILTIN, + } + defaults.update(overrides) + return Tool(**defaults) + + +# --------------------------------------------------------------------------- +# When steps — ToolLifecycle construction +# --------------------------------------------------------------------------- + + +@when('I create a tool lifecycle with execute hook "{execute}"') +def step_create_lifecycle_with_execute(context: Context, execute: str) -> None: + """Create a ToolLifecycle with only the execute hook set.""" + context.tool_lifecycle = ToolLifecycle(execute=execute) + context.tool_model_error = None + + +@when("I create a tool lifecycle with all four hooks set") +def step_create_lifecycle_all_four(context: Context) -> None: + """Create a ToolLifecycle with all four lifecycle hooks set.""" + context.tool_lifecycle = ToolLifecycle( + discover="hooks.discover", + activate="hooks.activate", + execute="hooks.execute", + deactivate="hooks.deactivate", + ) + context.tool_model_error = None + + +@when("I create a tool lifecycle with no hooks") +def step_create_lifecycle_no_hooks(context: Context) -> None: + """Create a ToolLifecycle with no hooks (all default to None).""" + context.tool_lifecycle = ToolLifecycle() + context.tool_model_error = None + + +@when('I create a tool lifecycle with only execute hook "{execute}"') +def step_create_lifecycle_only_execute(context: Context, execute: str) -> None: + """Create a ToolLifecycle with only the execute hook set (all others None).""" + context.tool_lifecycle = ToolLifecycle(execute=execute) + context.tool_model_error = None + + +# --------------------------------------------------------------------------- +# When steps — Tool.from_config() with lifecycle +# --------------------------------------------------------------------------- + + +@when('I load a tool from config with lifecycle execute "{execute}"') +def step_load_tool_from_config_with_lifecycle_execute( + context: Context, execute: str +) -> None: + """Load a Tool from config dict that includes a lifecycle.execute hook.""" + config: dict[str, Any] = { + "name": "local/lifecycle-tool", + "description": "Tool with lifecycle execute hook", + "source": "builtin", + "lifecycle": { + "execute": execute, + }, + } + context.tool_model = Tool.from_config(config) + context.tool_model_error = None + + +@when("I load a tool from config with lifecycle execute None") +def step_load_tool_from_config_with_lifecycle_execute_none(context: Context) -> None: + """Load a Tool from config dict that includes a lifecycle with execute=None.""" + config: dict[str, Any] = { + "name": "local/lifecycle-tool-none", + "description": "Tool with lifecycle execute=None", + "source": "builtin", + "lifecycle": { + "discover": "hooks.discover", + }, + } + context.tool_model = Tool.from_config(config) + context.tool_model_error = None + + +# --------------------------------------------------------------------------- +# When steps — Tool.as_cli_dict() with lifecycle +# --------------------------------------------------------------------------- + + +@when('I create a tool with lifecycle execute "{execute}" and call as_cli_dict') +def step_create_tool_with_lifecycle_execute_and_cli_dict( + context: Context, execute: str +) -> None: + """Create a Tool with a lifecycle.execute hook and call as_cli_dict.""" + tool = _make_builtin_tool( + lifecycle=ToolLifecycle(execute=execute), + ) + context.tool_cli_dict = tool.as_cli_dict() + context.tool_model_error = None + + +@when("I create a tool with no lifecycle and call as_cli_dict") +def step_create_tool_no_lifecycle_and_cli_dict(context: Context) -> None: + """Create a Tool with no lifecycle and call as_cli_dict.""" + tool = _make_builtin_tool() + context.tool_cli_dict = tool.as_cli_dict() + context.tool_model_error = None + + +# --------------------------------------------------------------------------- +# Then steps — ToolLifecycle field assertions +# --------------------------------------------------------------------------- + + +@then('the tool lifecycle execute should be "{expected}"') +def step_check_lifecycle_execute(context: Context, expected: str) -> None: + """Assert the lifecycle execute hook equals the expected value.""" + # Support both context.tool_lifecycle and context.tool_model.lifecycle + if hasattr(context, "tool_lifecycle"): + actual = context.tool_lifecycle.execute + else: + assert context.tool_model.lifecycle is not None, ( + "Expected tool_model.lifecycle to be set, but it is None" + ) + actual = context.tool_model.lifecycle.execute + assert actual == expected, ( + f"Expected lifecycle.execute == {expected!r}, got {actual!r}" + ) + + +@then("the tool lifecycle execute should be None") +def step_check_lifecycle_execute_none(context: Context) -> None: + """Assert the lifecycle execute hook is None.""" + if hasattr(context, "tool_lifecycle"): + actual = context.tool_lifecycle.execute + elif context.tool_model.lifecycle is not None: + actual = context.tool_model.lifecycle.execute + else: + actual = None + assert actual is None, f"Expected lifecycle.execute to be None, got {actual!r}" + + +@then("the tool lifecycle discover should be None") +def step_check_lifecycle_discover_none(context: Context) -> None: + """Assert the lifecycle discover hook is None.""" + if hasattr(context, "tool_lifecycle"): + actual = context.tool_lifecycle.discover + elif context.tool_model.lifecycle is not None: + actual = context.tool_model.lifecycle.discover + else: + actual = None + assert actual is None, f"Expected lifecycle.discover to be None, got {actual!r}" + + +@then('the tool lifecycle activate should be "{expected}"') +def step_check_lifecycle_activate(context: Context, expected: str) -> None: + """Assert the lifecycle activate hook equals the expected value.""" + if hasattr(context, "tool_lifecycle"): + actual = context.tool_lifecycle.activate + else: + assert context.tool_model.lifecycle is not None + actual = context.tool_model.lifecycle.activate + assert actual == expected, ( + f"Expected lifecycle.activate == {expected!r}, got {actual!r}" + ) + + +@then("the tool lifecycle activate should be None") +def step_check_lifecycle_activate_none(context: Context) -> None: + """Assert the lifecycle activate hook is None.""" + if hasattr(context, "tool_lifecycle"): + actual = context.tool_lifecycle.activate + elif context.tool_model.lifecycle is not None: + actual = context.tool_model.lifecycle.activate + else: + actual = None + assert actual is None, f"Expected lifecycle.activate to be None, got {actual!r}" + + +@then('the tool lifecycle deactivate should be "{expected}"') +def step_check_lifecycle_deactivate(context: Context, expected: str) -> None: + """Assert the lifecycle deactivate hook equals the expected value.""" + if hasattr(context, "tool_lifecycle"): + actual = context.tool_lifecycle.deactivate + else: + assert context.tool_model.lifecycle is not None + actual = context.tool_model.lifecycle.deactivate + assert actual == expected, ( + f"Expected lifecycle.deactivate == {expected!r}, got {actual!r}" + ) + + +@then("the tool lifecycle deactivate should be None") +def step_check_lifecycle_deactivate_none(context: Context) -> None: + """Assert the lifecycle deactivate hook is None.""" + if hasattr(context, "tool_lifecycle"): + actual = context.tool_lifecycle.deactivate + elif context.tool_model.lifecycle is not None: + actual = context.tool_model.lifecycle.deactivate + else: + actual = None + assert actual is None, f"Expected lifecycle.deactivate to be None, got {actual!r}" + + +# --------------------------------------------------------------------------- +# Then steps — as_cli_dict lifecycle assertions +# --------------------------------------------------------------------------- + + +@then('the tool cli dict lifecycle should have key "{key}"') +def step_check_cli_dict_lifecycle_has_key(context: Context, key: str) -> None: + """Assert the lifecycle dict in as_cli_dict contains the given key.""" + assert "lifecycle" in context.tool_cli_dict, ( + "Expected 'lifecycle' key in cli dict, but it was absent" + ) + lc = context.tool_cli_dict["lifecycle"] + assert key in lc, ( + f"Expected lifecycle dict to have key {key!r}, got keys: {list(lc.keys())}" + ) + + +@then('the tool cli dict lifecycle execute should be "{expected}"') +def step_check_cli_dict_lifecycle_execute(context: Context, expected: str) -> None: + """Assert the lifecycle.execute value in as_cli_dict equals expected.""" + assert "lifecycle" in context.tool_cli_dict, ( + "Expected 'lifecycle' key in cli dict, but it was absent" + ) + actual = context.tool_cli_dict["lifecycle"].get("execute") + assert actual == expected, ( + f"Expected lifecycle.execute == {expected!r}, got {actual!r}" + ) + + +@then('the tool cli dict should not have key "{key}"') +def step_check_cli_dict_missing_key(context: Context, key: str) -> None: + """Assert the cli dict does NOT contain the given key.""" + assert key not in context.tool_cli_dict, ( + f"Expected cli dict NOT to have key {key!r}, but it was present" + ) diff --git a/features/tool_lifecycle_execute_hook.feature b/features/tool_lifecycle_execute_hook.feature new file mode 100644 index 000000000..4c9fdffd7 --- /dev/null +++ b/features/tool_lifecycle_execute_hook.feature @@ -0,0 +1,58 @@ +Feature: ToolLifecycle domain model execute hook + Verify that ToolLifecycle exposes the execute hook required by the + spec four-stage lifecycle (discover / activate / execute / deactivate). + + # --------------------------------------------------------------- + # TDD issue-capture test for #2820 + # The @tdd_expected_fail tag is removed once the fix is in place. + # --------------------------------------------------------------- + + @tdd_issue @tdd_issue_2820 + Scenario: ToolLifecycle has an execute field (issue capture) + When I create a tool lifecycle with execute hook "my.module.execute" + Then the tool lifecycle execute should be "my.module.execute" + + # --------------------------------------------------------------- + # Positive scenarios — all four lifecycle hooks present + # --------------------------------------------------------------- + + Scenario: ToolLifecycle with all four hooks set + When I create a tool lifecycle with all four hooks set + Then the tool lifecycle discover should be "hooks.discover" + And the tool lifecycle activate should be "hooks.activate" + And the tool lifecycle execute should be "hooks.execute" + And the tool lifecycle deactivate should be "hooks.deactivate" + + Scenario: ToolLifecycle execute defaults to None + When I create a tool lifecycle with no hooks + Then the tool lifecycle execute should be None + + Scenario: ToolLifecycle execute can be set independently + When I create a tool lifecycle with only execute hook "run.tool" + Then the tool lifecycle execute should be "run.tool" + And the tool lifecycle discover should be None + And the tool lifecycle activate should be None + And the tool lifecycle deactivate should be None + + # --------------------------------------------------------------- + # Round-trip YAML serialisation/deserialisation + # --------------------------------------------------------------- + + Scenario: Tool from_config parses execute hook from YAML lifecycle block + When I load a tool from config with lifecycle execute "my.execute.hook" + Then the tool model should be created + And the tool lifecycle execute should be "my.execute.hook" + + Scenario: Tool from_config with execute=None preserves None + When I load a tool from config with lifecycle execute None + Then the tool model should be created + And the tool lifecycle execute should be None + + Scenario: Tool as_cli_dict renders execute hook when set + When I create a tool with lifecycle execute "cli.execute.hook" and call as_cli_dict + Then the tool cli dict lifecycle should have key "execute" + And the tool cli dict lifecycle execute should be "cli.execute.hook" + + Scenario: Tool as_cli_dict omits execute when None + When I create a tool with no lifecycle and call as_cli_dict + Then the tool cli dict should not have key "lifecycle" diff --git a/src/cleveragents/domain/models/core/tool.py b/src/cleveragents/domain/models/core/tool.py index 7cced1ab7..4d2e3aaba 100644 --- a/src/cleveragents/domain/models/core/tool.py +++ b/src/cleveragents/domain/models/core/tool.py @@ -277,10 +277,14 @@ class ToolLifecycle(BaseModel): Each hook is a dotted Python path or shell command string that the runtime invokes at the corresponding lifecycle stage. + + The four-stage lifecycle matches the spec contract: + ``discover`` → ``activate`` → ``execute`` → ``deactivate``. """ discover: str | None = Field(default=None, description="Hook for tool discovery") activate: str | None = Field(default=None, description="Hook for tool activation") + execute: str | None = Field(default=None, description="Hook for tool execution") deactivate: str | None = Field( default=None, description="Hook for tool deactivation" ) @@ -530,6 +534,20 @@ class Tool(BaseModel): result["timeout"] = self.timeout + if self.lifecycle is not None: + lc = self.lifecycle + lc_dict: dict[str, str | None] = {} + if lc.discover is not None: + lc_dict["discover"] = lc.discover + if lc.activate is not None: + lc_dict["activate"] = lc.activate + if lc.execute is not None: + lc_dict["execute"] = lc.execute + if lc.deactivate is not None: + lc_dict["deactivate"] = lc.deactivate + if lc_dict: + result["lifecycle"] = lc_dict + ee = self.execution_environment if ee.mode != EnvironmentPreferenceMode.NONE: ee_dict: dict[str, str] = {"mode": ee.mode.value} -- 2.52.0