UAT: agents validation attach does not reject plain Tools — spec requires type discriminator check #2912

Open
opened 2026-04-05 02:47:17 +00:00 by freemo · 5 comments
Owner

Metadata

  • Branch: fix/tool-registry-validation-type-discriminator
  • Commit Message: fix(validation): enforce type discriminator in attach_validation to reject plain tools
  • Milestone: v3.7.0
  • Parent Epic: #362

What Was Tested

The agents validation attach command and its backing service (ToolRegistryService.attach_validation()) were analyzed against the specification.

Expected Behavior (from spec)

Per docs/specification.md § Relationship to Tool:

"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."

Actual Behavior

In src/cleveragents/application/services/tool_registry_service.py, attach_validation() (lines 170–176) only checks whether the tool exists:

existing = self._tool_repo.get_by_name(validation_name)
if existing is None:
    raise NotFoundError(resource_type="validation", resource_id=validation_name)

There is no check that existing["tool_type"] == "validation". A plain tool (tool_type="tool") can be successfully attached as a validation attachment, violating the spec's type discriminator requirement.

Steps to Reproduce

  1. Register a plain tool: agents tool add --config plain-tool.yaml (where tool_type is "tool")
  2. Attempt to attach it as a validation: agents validation attach <resource> <plain-tool-name>
  3. Observe: the attach succeeds instead of raising an error

Code Location

  • src/cleveragents/application/services/tool_registry_service.py, method attach_validation(), lines 170–176
  • The fix requires adding: if isinstance(existing, dict) and existing.get("tool_type") != "validation": raise ValidationError(...) (or equivalent)

Severity

High — this allows plain tools to be silently treated as validations, bypassing the type safety guarantee that the spec explicitly requires.

Subtasks

  • Add tool_type discriminator check in ToolRegistryService.attach_validation() after the existence check
  • Raise an appropriate ValidationError (or domain-specific error) when a plain tool is passed where a validation is required
  • Add a Behave scenario: Given a plain tool is registered / When I call attach_validation with its name / Then a ValidationError is raised
  • Add a Robot Framework integration test verifying agents validation attach rejects a plain tool name with a non-zero exit code and descriptive error message
  • Verify no regression in existing attach_validation happy-path tests
  • Update docstring on attach_validation() to document the type discriminator enforcement

Definition of Done

  • attach_validation() raises an appropriate error when passed a name that resolves to a tool_type="tool" entry
  • New Behave scenario covers the rejection path and passes
  • New Robot Framework integration test covers the CLI rejection path and passes
  • No existing tests are broken
  • All nox stages pass
  • Coverage >= 97%

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

## Metadata - **Branch**: `fix/tool-registry-validation-type-discriminator` - **Commit Message**: `fix(validation): enforce type discriminator in attach_validation to reject plain tools` - **Milestone**: v3.7.0 - **Parent Epic**: #362 ## What Was Tested The `agents validation attach` command and its backing service (`ToolRegistryService.attach_validation()`) were analyzed against the specification. ## Expected Behavior (from spec) Per `docs/specification.md` § Relationship to Tool: > "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`." ## Actual Behavior In `src/cleveragents/application/services/tool_registry_service.py`, `attach_validation()` (lines 170–176) only checks whether the tool exists: ```python existing = self._tool_repo.get_by_name(validation_name) if existing is None: raise NotFoundError(resource_type="validation", resource_id=validation_name) ``` There is **no check** that `existing["tool_type"] == "validation"`. A plain tool (`tool_type="tool"`) can be successfully attached as a validation attachment, violating the spec's type discriminator requirement. ## Steps to Reproduce 1. Register a plain tool: `agents tool add --config plain-tool.yaml` (where `tool_type` is `"tool"`) 2. Attempt to attach it as a validation: `agents validation attach <resource> <plain-tool-name>` 3. Observe: the attach succeeds instead of raising an error ## Code Location - `src/cleveragents/application/services/tool_registry_service.py`, method `attach_validation()`, lines 170–176 - The fix requires adding: `if isinstance(existing, dict) and existing.get("tool_type") != "validation": raise ValidationError(...)` (or equivalent) ## Severity High — this allows plain tools to be silently treated as validations, bypassing the type safety guarantee that the spec explicitly requires. ## Subtasks - [ ] Add `tool_type` discriminator check in `ToolRegistryService.attach_validation()` after the existence check - [ ] Raise an appropriate `ValidationError` (or domain-specific error) when a plain tool is passed where a validation is required - [ ] Add a Behave scenario: `Given a plain tool is registered / When I call attach_validation with its name / Then a ValidationError is raised` - [ ] Add a Robot Framework integration test verifying `agents validation attach` rejects a plain tool name with a non-zero exit code and descriptive error message - [ ] Verify no regression in existing `attach_validation` happy-path tests - [ ] Update docstring on `attach_validation()` to document the type discriminator enforcement ## Definition of Done - [ ] `attach_validation()` raises an appropriate error when passed a name that resolves to a `tool_type="tool"` entry - [ ] New Behave scenario covers the rejection path and passes - [ ] New Robot Framework integration test covers the CLI rejection path and passes - [ ] No existing tests are broken - [ ] 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-05 02:47:23 +00:00
Author
Owner

Issue triaged by project owner:

  • State: Verified
  • Priority: High (confirmed)
  • MoSCoW: Should Have — validation attach should reject plain Tools

Valid UAT finding verified during batch triage.


Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: High (confirmed) - **MoSCoW**: Should Have — validation attach should reject plain Tools Valid UAT finding verified during batch triage. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner
Author
Owner

Starting implementation on branch fix/tool-registry-validation-type-discriminator.

Plan:

  1. TDD: Write failing Behave scenario first (plain tool rejected by attach_validation)
  2. Implement the tool_type discriminator check in ToolRegistryService.attach_validation()
  3. Add Robot Framework integration test for CLI rejection path
  4. Update docstring
  5. Run all quality gates

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

Starting implementation on branch `fix/tool-registry-validation-type-discriminator`. **Plan:** 1. TDD: Write failing Behave scenario first (plain tool rejected by `attach_validation`) 2. Implement the `tool_type` discriminator check in `ToolRegistryService.attach_validation()` 3. Add Robot Framework integration test for CLI rejection path 4. Update docstring 5. Run all quality gates --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
Author
Owner

All subtasks complete. Quality gates passed. Creating PR.

Implementation Summary:

  • Added tool_type discriminator check in ToolRegistryService.attach_validation() — raises ValidationError when tool_type != "validation"
  • New Behave TDD scenario: attach_validation raises ValidationError when a plain tool is passed — PASSES
  • New Robot Framework integration test: Validation Attach Rejects Plain Tool — PASSES (8/8 tests pass)
  • Lint: all checks passed
  • Type check: 0 errors, 0 warnings
  • Format: all files formatted

PR #3317 created on branch fix/tool-registry-validation-type-discriminator. PR review and merge handled by continuous review stream.


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

All subtasks complete. Quality gates passed. Creating PR. **Implementation Summary:** - Added `tool_type` discriminator check in `ToolRegistryService.attach_validation()` — raises `ValidationError` when `tool_type != "validation"` - New Behave TDD scenario: `attach_validation raises ValidationError when a plain tool is passed` — PASSES - New Robot Framework integration test: `Validation Attach Rejects Plain Tool` — PASSES (8/8 tests pass) - Lint: all checks passed - Type check: 0 errors, 0 warnings - Format: all files formatted PR #3317 created on branch `fix/tool-registry-validation-type-discriminator`. PR review and merge handled by continuous review stream. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
Author
Owner

Label compliance fix applied:

  • Removed conflicting label: State/Verified
  • Kept: State/In Review (more advanced state)
  • Reason: Issue had both State/Verified and State/In Review simultaneously. Per CONTRIBUTING.md, only one State/* label is permitted. State/In Review is the more advanced state and was retained.

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

Label compliance fix applied: - Removed conflicting label: `State/Verified` - Kept: `State/In Review` (more advanced state) - Reason: Issue had both `State/Verified` and `State/In Review` simultaneously. Per CONTRIBUTING.md, only one `State/*` label is permitted. `State/In Review` is the more advanced state and was retained. --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: ca-backlog-groomer
Author
Owner

Milestone Triage Decision: Moved to Backlog

This spec alignment issue has been moved out of v3.3.0 during aggressive milestone triage. While important for consistency, it does not relate to the core focus of Corrections + Subplans + Checkpoints.

Reasoning:

  • v3.3.0 focus: Essential corrections, subplan management, and checkpoint functionality
  • This issue: Spec alignment for CLI output formatting - consistency improvement
  • Impact: Specification compliance, not core corrections/subplans/checkpoints functionality

Will be addressed in a future milestone focused on CLI specification compliance and consistency.

**Milestone Triage Decision: Moved to Backlog** This spec alignment issue has been moved out of v3.3.0 during aggressive milestone triage. While important for consistency, it does not relate to the core focus of Corrections + Subplans + Checkpoints. **Reasoning:** - v3.3.0 focus: Essential corrections, subplan management, and checkpoint functionality - This issue: Spec alignment for CLI output formatting - consistency improvement - Impact: Specification compliance, not core corrections/subplans/checkpoints functionality Will be addressed in a future milestone focused on CLI specification compliance and consistency.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core#2912
No description provided.