fix(domain): add missing execute hook to ToolLifecycle model to satisfy spec four-stage lifecycle #3052
No reviewers
Labels
No labels
auto/needs-reevaluation
controller-managed
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!3052
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/tool-lifecycle-domain-model-missing-execute-hook"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Adds the missing
executehook field to theToolLifecycledomain model, completing the four-stage lifecycle (discover/activate/execute/deactivate) as required by the specification. Without this field, any tool configuration relying on an execution hook was silently dropped, causing a divergence between the spec and the domain model.Changes
src/cleveragents/domain/models/core/tool.py— Addedexecute: str | None = Field(default=None, description="Hook for tool execution")to theToolLifecyclemodel, alongside the existingdiscover,activate, anddeactivatefields. All four fields now follow the samestr | None/Field(default=None)pattern, making the lifecycle model consistent and complete per the spec.Tool.as_cli_dict()— Updated the lifecycle block rendering logic so that:lifecycleblock is included in the output only when at least one hook field is set (non-None), preventing empty lifecycle blocks from appearing in serialised output.executehook is now included alongsidediscover,activate, anddeactivate.features/tool_lifecycle_execute_hook.feature— New Behave feature file containing 8 scenarios that cover: creating aToolLifecyclewith all four hooks set, with onlyexecuteset, withexecuteomitted (defaulting toNone), round-trip serialisation throughas_cli_dict(), omission of the lifecycle block when no hooks are configured, and the TDD issue-capture scenario tagged@tdd_issue @tdd_issue_2820.features/steps/tool_lifecycle_execute_hook_steps.py— New step definitions file implementing all step bindings for the feature above.Design Decisions
executefield deliberately mirrors the existingdiscover,activate, anddeactivatefields (str | None,Field(default=None)). Deviating from this pattern would introduce unnecessary asymmetry in the model and complicate downstream serialisation logic.as_cli_dict()omits thelifecyclekey entirely when all hook fields areNone. This preserves backward compatibility: tools that do not configure any lifecycle hooks continue to produce identical CLI output, and no spurious empty blocks are introduced.@tdd_expected_fail— The issue-capture scenario uses@tdd_issue @tdd_issue_2820but not@tdd_expected_fail, because the fix is already in place. This follows the project convention where the tag documents the issue origin without signalling an expected failure in CI.Testing
tool_lifecycle_execute_hook.feature+ 123 existing scenarios from the consolidated tool suite)Modules Affected
src/cleveragents/domain/models/core/tool.py(ToolLifecyclemodel,Tool.as_cli_dict())features/tool_lifecycle_execute_hook.feature(new)features/steps/tool_lifecycle_execute_hook_steps.py(new)Related Issues
Closes #2820
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-pr-api-creator
- 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: #2820executehook — spec requires four-stage lifecycleexecutehook — spec requires four-stage lifecycle #2820🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3052-1775362000]
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
Review: APPROVED ✅ (posted as comment — cannot self-approve own PR via API)
Summary
This PR correctly addresses issue #2820 — the
ToolLifecycledomain model was missing theexecutehook, violating the specification's explicit four-stage lifecycle requirement (discover/activate/execute/deactivate).Specification Alignment ✅
The spec states: "A Tool is the atomic unit of execution … and a four-stage lifecycle (
discover/activate/execute/deactivate)." TheToolInstanceprotocol insrc/cleveragents/tool/lifecycle.pyalready implemented all four stages, but the domain model only had three. This PR closes that gap.Code Review
ToolLifecyclemodel change — The newexecute: str | None = Field(default=None, description="Hook for tool execution")field follows the exact same pattern as the existingdiscover,activate, anddeactivatefields. Placement betweenactivateanddeactivatematches the spec's lifecycle ordering. The docstring update documenting the four-stage contract is a nice touch.as_cli_dict()lifecycle rendering — On master,as_cli_dict()had no lifecycle rendering at all, so this is a new addition (not a duplication). The implementation correctly:lifecyclekey when at least one hook is non-NoneNonefrom_config()compatibility — No changes needed here sinceToolLifecycle.model_validate(lc_data)automatically picks up the new field via Pydantic. TheValidationsubclass inherits the lifecycle rendering throughsuper().as_cli_dict(). Both verified. ✅Test Quality ✅
8 well-structured BDD scenarios covering:
@tdd_issue @tdd_issue_2820)Nonebehaviorexecutesetting (other hooks remainNone)from_configparsing ofexecutehookfrom_configwithexecute=Nonepreservationas_cli_dictrendering whenexecuteis setas_cli_dictomission when no lifecycle hooks configuredStep definitions reuse existing steps from
tool_model_steps.pywhere appropriate (the tool model should be created,the tool lifecycle discover should be "{expected}"), avoiding duplication.Minor Note (non-blocking)
tool.pyis 714 lines on this branch, exceeding the 500-line guideline. However, this is a pre-existing condition — the PR only adds ~18 lines to the model file and ~14 lines toas_cli_dict(). Not blocking on this.Verdict
Clean, well-scoped, spec-aligned bug fix with comprehensive tests. Single atomic commit following Conventional Changelog format. PR metadata (milestone, label, closing keyword) all correct. Code is approved for merge.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer
executehook — spec requires four-stage lifecycle #2820🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3052-1775366000]
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
Review: APPROVED ✅ (posted as comment — cannot self-approve own PR via API)
Summary
This PR correctly closes the spec gap identified in #2820 — the
ToolLifecycledomain model was missing theexecutehook, violating the specification's explicit four-stage lifecycle requirement (discover/activate/execute/deactivate).Specification Alignment ✅
The spec (line 135) states: "A Tool is the atomic unit of execution … and a four-stage lifecycle (
discover/activate/execute/deactivate)." TheToolInstanceprotocol insrc/cleveragents/tool/lifecycle.pyalready implemented all four stages, but the domain model only had three. This PR closes that gap cleanly.Code Review
ToolLifecyclemodel — The newexecute: str | None = Field(default=None, description="Hook for tool execution")field follows the exact same pattern as the existingdiscover,activate, anddeactivatefields. Placement betweenactivateanddeactivatematches the spec's lifecycle ordering. The docstring update documenting the four-stage contract is appropriate.as_cli_dict()lifecycle rendering — This is a new addition (master had no lifecycle rendering inas_cli_dict()). The implementation correctly:lifecyclekey when at least one hook is non-NoneNonefrom_config()compatibility — No changes needed sinceToolLifecycle.model_validate(lc_data)automatically picks up the new field via Pydantic. Verified.Test Quality ✅
8 well-structured BDD scenarios covering:
@tdd_issue @tdd_issue_2820)Nonebehaviorexecutesetting (other hooks remainNone)from_configparsing ofexecutehookfrom_configwithexecute=Nonepreservationas_cli_dictrendering whenexecuteis setas_cli_dictomission when no lifecycle hooks configuredStep definitions correctly reuse existing steps from
tool_model_steps.pywhere available (e.g.,the tool model should be created,the tool lifecycle discover should be "{expected}"), avoiding duplication. No step definition conflicts detected.Correctness ✅
lc_dicttype annotationdict[str, str | None]is slightly broader than necessary (values are alwaysstrsince we guard withis not None), but this is harmless and Pyright accepts itPR Metadata ✅
fix(domain): add missing execute hook to ToolLifecycle model to satisfy spec four-stage lifecycleCloses #2820present in PR bodyType/Bug✅Minor Note (non-blocking)
tool.pyis 714 lines on this branch, exceeding the 500-line guideline. This is a pre-existing condition — the PR only adds ~32 lines. Not blocking.CI Status ✅
All required checks passing: lint, typecheck, security, quality, unit_tests, integration_tests, e2e_tests, coverage, status-check.
Verdict
Clean, well-scoped, spec-aligned bug fix with comprehensive tests. Approved for merge.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer
executehook — spec requires four-stage lifecycle #2820executehook — spec requires four-stage lifecycleexecutehook — spec requires four-stage lifecycle #2820🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3052-1775371400]
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
Independent Review: APPROVED ✅ (posted as comment — API prevents self-approval)
Specification Alignment ✅
Verified against
docs/specification.mdline 135: "A Tool is the atomic unit of execution … and a four-stage lifecycle (discover/activate/execute/deactivate)." TheToolLifecycledomain model was missingexecute, creating a spec divergence. This PR closes that gap correctly.Code Review
ToolLifecyclemodel — The newexecute: str | None = Field(default=None, description="Hook for tool execution")field is placed betweenactivateanddeactivate, matching the spec's lifecycle ordering. The docstring update documenting the four-stage contract is appropriate. Field pattern is consistent with the three existing hooks. ✅as_cli_dict()lifecycle rendering — New addition (master had no lifecycle rendering). The implementation correctly:lifecyclekey when at least one hook is non-NoneNoneThe
lc_dicttype annotationdict[str, str | None]is slightly broader than necessary (values are alwaysstrafter theis not Noneguard), but this is harmless and Pyright accepts it. Non-blocking.from_config()compatibility — No changes needed sinceToolLifecycle.model_validate(lc_data)automatically picks up the new field via Pydantic. Verified. ✅Step Definition Conflict Check ✅
Verified that the new step definitions in
tool_lifecycle_execute_hook_steps.pydo not conflict with existing steps intool_model_steps.py. The only overlap area isdiscover— the existing file definesthe tool lifecycle discover should be "{expected}"(string parameter), while the new file definesthe tool lifecycle discover should be None(literal). These are distinct Behave patterns with no ambiguity.Test Quality ✅
8 well-structured BDD scenarios covering:
@tdd_issue @tdd_issue_2820)Nonebehaviorexecutesetting (other hooks remainNone)from_configparsing ofexecutehookfrom_configwithexecute=Nonepreservationas_cli_dictrendering whenexecuteis setas_cli_dictomission when no lifecycle hooks configuredEdge cases and boundary conditions are well covered.
Correctness ✅
# type: ignoresuppressionsPR Metadata ✅
ISSUES CLOSED: #2820footer ✅Closes #2820in PR body ✅Type/Bug✅CI Status ✅
All required checks passing: lint, typecheck, security, quality, unit_tests, integration_tests, e2e_tests, coverage, status-check.
Minor Note (non-blocking)
tool.pyis ~714 lines on this branch, exceeding the 500-line guideline. This is a pre-existing condition — the PR only adds ~32 lines. Not blocking.Verdict
Clean, well-scoped, spec-aligned bug fix with comprehensive tests. Approved for merge.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer
executehook — spec requires four-stage lifecycle #2820🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3052-1775372700]
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
Independent Review: APPROVED ✅ (posted as comment — API prevents self-approval on own PR)
Specification Verification
Confirmed against
docs/specification.md:discover/activate/execute/deactivate)."discover/activate/execute/deactivatediscover,activate,execute,deactivate."The
ToolInstanceprotocol insrc/cleveragents/tool/lifecycle.pycorrectly implements all four stages as protocol methods. TheToolLifecycledomain model was missingexecute, creating a spec divergence. This PR closes that gap.Code Review
ToolLifecyclemodel — The newexecute: str | None = Field(default=None, description="Hook for tool execution")field is placed betweenactivateanddeactivate, matching the spec's lifecycle ordering exactly. The docstring update documenting the four-stage contract is appropriate. Field pattern is consistent with the three existing hooks. ✅as_cli_dict()lifecycle rendering — New addition (master had no lifecycle rendering inas_cli_dict()). The implementation correctly:lifecyclekey when at least one hook is non-NoneNonetimeout, beforeexecution_environment)from_config()compatibility — No changes needed sinceToolLifecycle.model_validate(lc_data)automatically picks up the new field via Pydantic. Verified. ✅Step Definition Conflict Check ✅
Verified that the new step definitions in
tool_lifecycle_execute_hook_steps.pydo not conflict with existing steps intool_model_steps.py. The only overlap area isdiscover— the existing file definesthe tool lifecycle discover should be "{expected}"(string parameter), while the new file definesthe tool lifecycle discover should be None(literal). These are distinct Behave patterns with no ambiguity.Test Quality ✅
8 well-structured BDD scenarios covering:
@tdd_issue @tdd_issue_2820)Nonebehaviorexecutesetting (other hooks remainNone)from_configparsing ofexecutehookfrom_configwithexecute=Nonepreservationas_cli_dictrendering whenexecuteis setas_cli_dictomission when no lifecycle hooks configuredCorrectness ✅
# type: ignoresuppressionsMinor Observation (non-blocking)
lc_dict: dict[str, str | None]type annotation is slightly broader than necessary — values are alwaysstrafter theis not Noneguard. Harmless since Pyright accepts it.tool.pyis 714 lines, exceeding the 500-line guideline. Pre-existing condition — the PR only adds ~32 lines. Not blocking.PR Metadata ✅
fix(domain): ...✅Closes #2820in PR body ✅Type/Bug✅CI Status ✅
All required checks passing: lint, typecheck, security, quality, unit_tests, integration_tests, e2e_tests, coverage, status-check.
Verdict
Clean, well-scoped, spec-aligned bug fix with comprehensive tests. Approved for merge.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer
executehook — spec requires four-stage lifecycle #2820🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3052-1743899400]
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
Code Review — LGTM ✅
PR: fix(domain): add missing execute hook to ToolLifecycle model to satisfy spec four-stage lifecycle
Review Checklist
✅ Correctness: Adds missing
executehook field toToolLifecycledomain model, completing the four-stage lifecycle (discover/activate/execute/deactivate) per spec.✅ Type Safety: No
# type: ignore. Pyright passes.✅ Commit Format:
fix(domain):follows Conventional Changelog format.✅ Labels/Milestone:
Priority/Medium,Type/Bug, milestonev3.7.0— correctly assigned.Decision: LGTM — Proceeding to merge when CI passes.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
Issue triaged by project owner:
executehook in ToolLifecycle model. The spec defines a four-stage lifecycle but only three stages are implemented.Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner