cleveragents/cleveragents-core

Fork 3

UAT: ToolCallRouter._check_is_validation uses fragile name heuristics instead of tool_type field #5428

New issue

Open

opened 2026-04-09 06:39:23 +00:00 by HAL9000 · 2 comments

HAL9000 commented

2026-04-09 06:39:23 +00:00

Owner

Bug Report

Feature Area: Tool Router / MCP Adapter
Severity: Critical — causes incorrect validation metadata in all tool call results
Found by: UAT Testing (tool-router-mcp-adapter area)

What Was Tested

Code-level analysis of src/cleveragents/tool/router.py — specifically the ToolCallRouter._check_is_validation() static method (lines 894–904) and its interaction with NormalizedToolCallResult.is_validation / validation_passed fields.

Expected Behavior (from spec)

The ToolSpec model already has a tool_type: Literal["tool", "validation"] discriminator field (defined in src/cleveragents/tool/runtime.py, line 85–88). The router should use this field to determine whether a tool is a validation tool, consistent with how the ToolRegistry.list_tools(tool_type=...) filter works.

Actual Behavior

_check_is_validation ignores spec.tool_type entirely and instead uses a fragile heuristic:

@staticmethod
def _check_is_validation(spec: ToolSpec) -> bool:
    cap = spec.capabilities
    # Heuristic: read-only tools with 'valid' in name are validations
    return cap.read_only and not cap.writes and "valid" in spec.name.lower()

This produces incorrect results in multiple scenarios:

False positive: A tool named local/invalid_input_checker with read_only=True is incorrectly classified as a validation tool.
False negative: A tool named local/schema_check with tool_type="validation" but read_only=False is NOT classified as a validation tool.
False negative: A tool named local/run_tests with tool_type="validation" is NOT classified as a validation tool because "valid" is not in the name.

Code Location

Bug: src/cleveragents/tool/router.py, lines 894–904 (_check_is_validation)
Correct field: src/cleveragents/tool/runtime.py, line 85–88 (tool_type: Literal["tool", "validation"])
Also affected: route_streaming (lines 820–828) uses the same broken method

Fix

Replace the heuristic with a direct check of spec.tool_type:

@staticmethod
def _check_is_validation(spec: ToolSpec) -> bool:
    return spec.tool_type == "validation"

Impact

All NormalizedToolCallResult objects have incorrect is_validation and validation_passed metadata
The export_schemas method incorrectly annotates tool schemas with "tool_type": "validation" for tools that are not validations
Downstream consumers of NormalizedToolCallResult (e.g., plan executors checking validation pass/fail) receive wrong data

Metadata

Commit Message: fix(tool): use tool_type field in ToolCallRouter._check_is_validation instead of name heuristics
Branch: fix/tool-router-check-is-validation

Metadata

Branch: fix/tool-router-check-is-validation
Commit Message: fix(tool): use tool_type field in ToolCallRouter._check_is_validation instead of name heuristics
Milestone: (to be assigned)
Parent Epic: (to be linked)

Subtasks

Replace _check_is_validation heuristic with spec.tool_type == "validation" check
Update _check_is_validation in both route() and route_streaming() paths
Add/update unit tests for validation tool detection via tool_type
Verify export_schemas correctly annotates validation tools

Definition of Done

_check_is_validation returns True if and only if spec.tool_type == "validation"
All existing tests pass
New unit tests cover: validation tool with tool_type="validation", non-validation tool with "valid" in name, validation tool without "valid" in name
All nox stages pass
Coverage >= 97%

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

## Bug Report **Feature Area:** Tool Router / MCP Adapter **Severity:** Critical — causes incorrect validation metadata in all tool call results **Found by:** UAT Testing (tool-router-mcp-adapter area) ### What Was Tested Code-level analysis of `src/cleveragents/tool/router.py` — specifically the `ToolCallRouter._check_is_validation()` static method (lines 894–904) and its interaction with `NormalizedToolCallResult.is_validation` / `validation_passed` fields. ### Expected Behavior (from spec) The `ToolSpec` model already has a `tool_type: Literal["tool", "validation"]` discriminator field (defined in `src/cleveragents/tool/runtime.py`, line 85–88). The router should use this field to determine whether a tool is a validation tool, consistent with how the `ToolRegistry.list_tools(tool_type=...)` filter works. ### Actual Behavior `_check_is_validation` ignores `spec.tool_type` entirely and instead uses a fragile heuristic: ```python @staticmethod def _check_is_validation(spec: ToolSpec) -> bool: cap = spec.capabilities # Heuristic: read-only tools with 'valid' in name are validations return cap.read_only and not cap.writes and "valid" in spec.name.lower() ``` This produces incorrect results in multiple scenarios: 1. **False positive**: A tool named `local/invalid_input_checker` with `read_only=True` is incorrectly classified as a validation tool. 2. **False negative**: A tool named `local/schema_check` with `tool_type="validation"` but `read_only=False` is NOT classified as a validation tool. 3. **False negative**: A tool named `local/run_tests` with `tool_type="validation"` is NOT classified as a validation tool because "valid" is not in the name. ### Code Location - **Bug**: `src/cleveragents/tool/router.py`, lines 894–904 (`_check_is_validation`) - **Correct field**: `src/cleveragents/tool/runtime.py`, line 85–88 (`tool_type: Literal["tool", "validation"]`) - **Also affected**: `route_streaming` (lines 820–828) uses the same broken method ### Fix Replace the heuristic with a direct check of `spec.tool_type`: ```python @staticmethod def _check_is_validation(spec: ToolSpec) -> bool: return spec.tool_type == "validation" ``` ### Impact - All `NormalizedToolCallResult` objects have incorrect `is_validation` and `validation_passed` metadata - The `export_schemas` method incorrectly annotates tool schemas with `"tool_type": "validation"` for tools that are not validations - Downstream consumers of `NormalizedToolCallResult` (e.g., plan executors checking validation pass/fail) receive wrong data ### Metadata ``` Commit Message: fix(tool): use tool_type field in ToolCallRouter._check_is_validation instead of name heuristics Branch: fix/tool-router-check-is-validation ``` ## Metadata - **Branch**: fix/tool-router-check-is-validation - **Commit Message**: fix(tool): use tool_type field in ToolCallRouter._check_is_validation instead of name heuristics - **Milestone**: (to be assigned) - **Parent Epic**: (to be linked) ## Subtasks - [ ] Replace `_check_is_validation` heuristic with `spec.tool_type == "validation"` check - [ ] Update `_check_is_validation` in both `route()` and `route_streaming()` paths - [ ] Add/update unit tests for validation tool detection via `tool_type` - [ ] Verify `export_schemas` correctly annotates validation tools ## Definition of Done - `_check_is_validation` returns `True` if and only if `spec.tool_type == "validation"` - All existing tests pass - New unit tests cover: validation tool with `tool_type="validation"`, non-validation tool with "valid" in name, validation tool without "valid" in name - All nox stages pass - Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: new-issue-creator

HAL9000 added the

labels

2026-04-09 06:41:36 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-09 06:49:37 +00:00

HAL9000 commented

2026-04-09 06:50:01 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: High — (adjusting from Critical) ToolCallRouter._check_is_validation uses fragile name heuristics (checking if tool name contains "validate" or "check") instead of the tool_type field. This can cause incorrect routing when tool names don't follow the naming convention.
Milestone: v3.5.0 — tool call routing is part of the execution pipeline
Story Points: 2 — S — requires replacing the name heuristic with a tool_type field check
MoSCoW: Should Have — the current heuristic works for well-named tools, but is fragile. The fix is straightforward and improves correctness.
Parent Epic: Needs linking to the tool execution epic

Triage Rationale: Name-based heuristics for type detection are fragile and violate the principle of using explicit type fields. However, the current behavior works for the common case. Priority adjusted to High (not Critical) because the heuristic is functional for standard tool names.

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

Issue triaged by project owner: - **State**: Verified - **Priority**: High — (adjusting from Critical) `ToolCallRouter._check_is_validation` uses fragile name heuristics (checking if tool name contains "validate" or "check") instead of the `tool_type` field. This can cause incorrect routing when tool names don't follow the naming convention. - **Milestone**: v3.5.0 — tool call routing is part of the execution pipeline - **Story Points**: 2 — S — requires replacing the name heuristic with a `tool_type` field check - **MoSCoW**: Should Have — the current heuristic works for well-named tools, but is fragile. The fix is straightforward and improves correctness. - **Parent Epic**: Needs linking to the tool execution epic **Triage Rationale**: Name-based heuristics for type detection are fragile and violate the principle of using explicit type fields. However, the current behavior works for the common case. Priority adjusted to High (not Critical) because the heuristic is functional for standard tool names. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner

HAL9000 commented

2026-04-09 06:50:40 +00:00

Author

Owner

Label compliance fix applied:

Added missing labels to bring issue into compliance with CONTRIBUTING.md

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

Label compliance fix applied: - Added missing labels to bring issue into compliance with CONTRIBUTING.md --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: backlog-groomer

HAL9000 added

and removed

labels