cleveragents/cleveragents-core

Fork 3

UAT: `agents validation attach` does not reject plain Tools — only Validations should be attachable #2826

New issue

Closed

opened 2026-04-04 20:42:41 +00:00 by freemo · 4 comments

freemo commented

2026-04-04 20:42:41 +00:00

Owner

Background and Context

The specification (docs/specification.md line 22322) explicitly defines a type discriminator enforced by the Tool Registry:

"a plain Tool cannot be used where a Validation is specifically required. For example, agents validation attach rejects a name that resolves to a plain Tool rather than a Validation. This distinction is enforced by the type discriminator stored in the Tool Registry: each entry is tagged as either tool or validation."

The attach_validation method in src/cleveragents/application/services/tool_registry_service.py (lines 170–192) only verifies that a tool with the given name exists, but never checks that the retrieved entry carries tool_type == "validation". As a result, a plain Tool can be silently attached as if it were a Validation, violating the spec invariant and the type-safety guarantee of the Tool Registry.

Current Behavior

attach_validation calls get_by_name to confirm the named entry exists, then immediately reads its mode attribute and proceeds with the attach — with no guard on tool_type:

# Verify validation exists
existing = self._tool_repo.get_by_name(validation_name)
if existing is None:
    raise NotFoundError(...)

# BUG: No check that existing.tool_type == "validation"
# Read mode from the validation's registered definition
if isinstance(existing, dict):
    mode = existing.get("mode", "required")
else:
    mode = getattr(existing, "mode", "required")

Consequences:

agents validation attach <RESOURCE> local/some-plain-tool succeeds when it must fail.
The plain tool is silently treated as a Validation with a default mode of "required".
The type-discriminator enforcement described in the spec is entirely absent.

Steps to Reproduce:

Register a plain tool: agents tool add --config ./tools/my-plain-tool.yaml
Attempt to attach it as a validation: agents validation attach <RESOURCE> local/my-plain-tool
Observe that the attach succeeds — it should fail with a clear error.

Expected Behavior

attach_validation must assert existing.tool_type == "validation" (or ToolType.VALIDATION) immediately after the existence check.
If a plain Tool name is supplied, the method must raise an appropriate error (e.g., ValidationError or a new ToolTypeMismatchError) with a message such as:
"'local/my-plain-tool' is a Tool, not a Validation. Only Validations can be attached to resources."
The CLI (src/cleveragents/cli/commands/validation.py lines 220–260, attach command) must surface this error clearly to the user.

Acceptance Criteria

attach_validation raises an appropriate error when the named entry has tool_type != "validation".
The error message clearly identifies the name, its actual type, and what was expected.
The CLI displays the error without a traceback (user-friendly output).
A Behave scenario proves the bug: attaching a plain tool as a validation must fail with the correct error.
A Behave scenario proves the happy path: attaching a genuine Validation still succeeds.
A Robot Framework integration test covers the end-to-end CLI rejection flow.
All nox sessions pass (nox).
Coverage remains ≥ 97% (nox -s coverage_report).

Supporting Information

Spec reference: docs/specification.md line 22322
Service code: src/cleveragents/application/services/tool_registry_service.py lines 170–192 (attach_validation)
CLI code: src/cleveragents/cli/commands/validation.py lines 220–260 (attach command)
Parent Epic: #392 — Epic: Actor YAML & Compiler / Decisions + Validations + Invariants, v3.2.0

Metadata

Branch: fix/validation-attach-rejects-plain-tools
Commit Message: fix(tool-registry): reject plain Tools in attach_validation type-discriminator check
Milestone: v3.7.0
Parent Epic: #392

Subtasks

Write a failing Behave scenario that proves attach_validation accepts a plain Tool (TDD — red phase)
Add tool_type guard in attach_validation (tool_registry_service.py lines 170–192) to raise on non-Validation entries
Define or reuse an appropriate error class (ToolTypeMismatchError or ValidationError) with a descriptive message
Update the CLI attach command (validation.py lines 220–260) to catch and display the new error cleanly
Write a passing Behave scenario for the happy path (genuine Validation attaches successfully)
Write a Robot Framework integration test for the end-to-end CLI rejection flow
Verify coverage ≥ 97% via nox -s coverage_report
Run nox (all default sessions) and fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly (fix(tool-registry): reject plain Tools in attach_validation type-discriminator check), followed by a blank line, then additional lines providing relevant details about the implementation, and a footer ISSUES CLOSED: #<this issue number>.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly (fix/validation-attach-rejects-plain-tools).
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
All nox stages pass.
Coverage ≥ 97%.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Background and Context The specification (`docs/specification.md` line 22322) explicitly defines a type discriminator enforced by the Tool Registry: > "a plain Tool **cannot** be used where a Validation is specifically required. For example, `agents validation attach` rejects a name that resolves to a plain Tool rather than a Validation. This distinction is enforced by the `type` discriminator stored in the Tool Registry: each entry is tagged as either `tool` or `validation`." The `attach_validation` method in `src/cleveragents/application/services/tool_registry_service.py` (lines 170–192) only verifies that a tool with the given name exists, but never checks that the retrieved entry carries `tool_type == "validation"`. As a result, a plain Tool can be silently attached as if it were a Validation, violating the spec invariant and the type-safety guarantee of the Tool Registry. ## Current Behavior `attach_validation` calls `get_by_name` to confirm the named entry exists, then immediately reads its `mode` attribute and proceeds with the attach — with no guard on `tool_type`: ```python # Verify validation exists existing = self._tool_repo.get_by_name(validation_name) if existing is None: raise NotFoundError(...) # BUG: No check that existing.tool_type == "validation" # Read mode from the validation's registered definition if isinstance(existing, dict): mode = existing.get("mode", "required") else: mode = getattr(existing, "mode", "required") ``` Consequences: 1. `agents validation attach <RESOURCE> local/some-plain-tool` succeeds when it must fail. 2. The plain tool is silently treated as a Validation with a default mode of `"required"`. 3. The type-discriminator enforcement described in the spec is entirely absent. **Steps to Reproduce:** 1. Register a plain tool: `agents tool add --config ./tools/my-plain-tool.yaml` 2. Attempt to attach it as a validation: `agents validation attach <RESOURCE> local/my-plain-tool` 3. Observe that the attach succeeds — it should fail with a clear error. ## Expected Behavior - `attach_validation` must assert `existing.tool_type == "validation"` (or `ToolType.VALIDATION`) immediately after the existence check. - If a plain Tool name is supplied, the method must raise an appropriate error (e.g., `ValidationError` or a new `ToolTypeMismatchError`) with a message such as: `"'local/my-plain-tool' is a Tool, not a Validation. Only Validations can be attached to resources."` - The CLI (`src/cleveragents/cli/commands/validation.py` lines 220–260, `attach` command) must surface this error clearly to the user. ## Acceptance Criteria - [ ] `attach_validation` raises an appropriate error when the named entry has `tool_type != "validation"`. - [ ] The error message clearly identifies the name, its actual type, and what was expected. - [ ] The CLI displays the error without a traceback (user-friendly output). - [ ] A Behave scenario proves the bug: attaching a plain tool as a validation must fail with the correct error. - [ ] A Behave scenario proves the happy path: attaching a genuine Validation still succeeds. - [ ] A Robot Framework integration test covers the end-to-end CLI rejection flow. - [ ] All nox sessions pass (`nox`). - [ ] Coverage remains ≥ 97% (`nox -s coverage_report`). ## Supporting Information - **Spec reference:** `docs/specification.md` line 22322 - **Service code:** `src/cleveragents/application/services/tool_registry_service.py` lines 170–192 (`attach_validation`) - **CLI code:** `src/cleveragents/cli/commands/validation.py` lines 220–260 (`attach` command) - **Parent Epic:** #392 — Epic: Actor YAML & Compiler / Decisions + Validations + Invariants, v3.2.0 ## Metadata - **Branch**: `fix/validation-attach-rejects-plain-tools` - **Commit Message**: `fix(tool-registry): reject plain Tools in attach_validation type-discriminator check` - **Milestone**: v3.7.0 - **Parent Epic**: #392 ## Subtasks - [ ] Write a failing Behave scenario that proves `attach_validation` accepts a plain Tool (TDD — red phase) - [ ] Add `tool_type` guard in `attach_validation` (`tool_registry_service.py` lines 170–192) to raise on non-Validation entries - [ ] Define or reuse an appropriate error class (`ToolTypeMismatchError` or `ValidationError`) with a descriptive message - [ ] Update the CLI `attach` command (`validation.py` lines 220–260) to catch and display the new error cleanly - [ ] Write a passing Behave scenario for the happy path (genuine Validation attaches successfully) - [ ] Write a Robot Framework integration test for the end-to-end CLI rejection flow - [ ] Verify coverage ≥ 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions) and fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly (`fix(tool-registry): reject plain Tools in attach_validation type-discriminator check`), followed by a blank line, then additional lines providing relevant details about the implementation, and a footer `ISSUES CLOSED: #<this issue number>`. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly (`fix/validation-attach-rejects-plain-tools`). - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass. - Coverage ≥ 97%. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added this to the v3.7.0 milestone

2026-04-04 20:42:46 +00:00

freemo added a new dependency

2026-04-04 20:42:58 +00:00

#392 Epic: Actor YAML & Compiler

freemo referenced this issue

2026-04-04 20:50:23 +00:00

[CA-AUTO] Project Owner — project-owner-1 — Session Tracker #2793

freemo referenced this issue

2026-04-04 20:51:19 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.7.0 — Session Tracker #2803

freemo referenced this issue

2026-04-04 20:51:49 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.7.0 — Session Tracker #2803

freemo referenced this issue

2026-04-04 21:08:24 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.7.0 — Session Tracker #2803

freemo commented

2026-04-05 02:44:17 +00:00

Author

Owner

Starting implementation on branch fix/validation-attach-rejects-plain-tools.

Analyzing the codebase to understand the attach_validation flow in tool_registry_service.py and the CLI validation.py command. Will implement the type-discriminator guard to reject plain Tools.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

Starting implementation on branch `fix/validation-attach-rejects-plain-tools`. Analyzing the codebase to understand the `attach_validation` flow in `tool_registry_service.py` and the CLI `validation.py` command. Will implement the type-discriminator guard to reject plain Tools. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo referenced this issue

2026-04-05 03:10:48 +00:00

Task: Autonomous build progress report — 2026-04-05 #2874

freemo commented

2026-04-05 03:20:33 +00:00

Author

Owner

All subtasks complete. Quality gates passed (lint ✅, typecheck ✅, security scan ✅, dead code ✅). Creating PR.

Implementation summary:

Added ToolTypeMismatchError to cleveragents/core/exceptions.py — a new DomainError subclass with descriptive message including the offending tool name and actual type
Added type-discriminator guard in attach_validation (tool_registry_service.py) that checks tool_type on both dict and domain-object representations, raising ToolTypeMismatchError for non-validation entries
Updated CLI attach command (validation.py) to catch ToolTypeMismatchError and display a clean [red]Type error:[/red] message before aborting
Added Behave feature validation_attach_type_guard.feature with 4 scenarios (2 rejection, 2 happy-path)
Added Robot Framework integration test validation_attach_type_guard.robot with helper script

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

All subtasks complete. Quality gates passed (lint ✅, typecheck ✅, security scan ✅, dead code ✅). Creating PR. **Implementation summary:** - Added `ToolTypeMismatchError` to `cleveragents/core/exceptions.py` — a new `DomainError` subclass with descriptive message including the offending tool name and actual type - Added type-discriminator guard in `attach_validation` (`tool_registry_service.py`) that checks `tool_type` on both dict and domain-object representations, raising `ToolTypeMismatchError` for non-validation entries - Updated CLI `attach` command (`validation.py`) to catch `ToolTypeMismatchError` and display a clean `[red]Type error:[/red]` message before aborting - Added Behave feature `validation_attach_type_guard.feature` with 4 scenarios (2 rejection, 2 happy-path) - Added Robot Framework integration test `validation_attach_type_guard.robot` with helper script --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo referenced this issue from a commit

2026-04-05 03:26:38 +00:00

fix(tool-registry): reject plain Tools in attach_validation type-discriminator check

freemo referenced this issue from a pull request that will close it,

2026-04-05 03:37:22 +00:00

fix(tool-registry): reject plain Tools in attach_validation type-discriminator check #3010

freemo added a new dependency

2026-04-05 03:39:49 +00:00

#3010 fix(tool-registry): reject plain Tools in attach_validation type-discriminator check

freemo added the

State

In Review

label

2026-04-05 03:56:28 +00:00

freemo commented

2026-04-05 03:58:04 +00:00

Author

Owner

Transition attempt for issue #2826: State/In Progress -> State/In Review

Preconditions:

From State/In Progress to State/In Review is a valid transition per the Ticket Lifecycle rules.
No Blocked label present; no blocking issue referenced.

Actions performed:

Added label State/In Review (ID 844) to the issue.
Attempted to remove the old label State/In progress (ID 1322). However, the current Forgejo tools available in this session only support adding labels (no dedicated remove-label operation). Therefore, removal did not occur in this operation.

Current labels after this action:

State/In progress (1322)
State/In Review (844)
MoSCoW/Should have (884)
Priority/High (1330)
Type/Bug (1325)

Notes:

To complete the transition per policy, the remaining State/In progress label should be removed to ensure there is only one State/* label. If you approve, I can proceed by:
- Using an administrative or extended API to remove label 1322, or
- Having a maintainer manually remove the State/In progress label, after which I can confirm the final state.

Would you like me to attempt a removal via an elevated process or manual follow-up?

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-state-updater

Transition attempt for issue #2826: State/In Progress -> State/In Review Preconditions: - From State/In Progress to State/In Review is a valid transition per the Ticket Lifecycle rules. - No Blocked label present; no blocking issue referenced. Actions performed: - Added label State/In Review (ID 844) to the issue. - Attempted to remove the old label State/In progress (ID 1322). However, the current Forgejo tools available in this session only support adding labels (no dedicated remove-label operation). Therefore, removal did not occur in this operation. Current labels after this action: - State/In progress (1322) - State/In Review (844) - MoSCoW/Should have (884) - Priority/High (1330) - Type/Bug (1325) Notes: - To complete the transition per policy, the remaining State/In progress label should be removed to ensure there is only one State/* label. If you approve, I can proceed by: - Using an administrative or extended API to remove label 1322, or - Having a maintainer manually remove the State/In progress label, after which I can confirm the final state. Would you like me to attempt a removal via an elevated process or manual follow-up? --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-state-updater

freemo referenced this issue

2026-04-05 18:06:47 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo closed this issue

2026-04-05 21:14:59 +00:00

freemo referenced this issue from a commit

2026-04-05 21:15:00 +00:00

fix(tool-registry): reject plain Tools in attach_validation type-discriminator check

HAL9000 removed the

State

In Review

label

2026-04-08 20:18:13 +00:00

HAL9000 commented

2026-04-08 20:19:05 +00:00

Owner

State label reconciliation:

Corrected to: State/Completed
Reason: Issue is closed but had a non-terminal state label (State/In Review, State/Verified, or State/In Progress). CONTRIBUTING.md requires closed issues to have State/Completed or State/Wont Do.

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: backlog-groomer

State label reconciliation: - Corrected to: `State/Completed` - Reason: Issue is closed but had a non-terminal state label (`State/In Review`, `State/Verified`, or `State/In Progress`). CONTRIBUTING.md requires closed issues to have `State/Completed` or `State/Wont Do`. --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: backlog-groomer

HAL9000 referenced this issue

2026-04-08 20:19:39 +00:00

[Automated] CleverAgents Build Session - 2026-04-08 #4799