fix(cli): remove extra --mode flag from validation attach
CI / build (push) Has been cancelled
CI / e2e_tests (push) Has been cancelled
CI / typecheck (push) Has been cancelled
CI / integration_tests (push) Has been cancelled
CI / quality (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / benchmark-publish (push) Has been cancelled
CI / security (push) Has been cancelled
CI / helm (push) Has been cancelled
CI / unit_tests (push) Has been cancelled
CI / benchmark-regression (push) Has been cancelled
CI / coverage (push) Has been cancelled
CI / docker (push) Has been cancelled
CI / status-check (push) Has been cancelled
CI / build (push) Has been cancelled
CI / e2e_tests (push) Has been cancelled
CI / typecheck (push) Has been cancelled
CI / integration_tests (push) Has been cancelled
CI / quality (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / benchmark-publish (push) Has been cancelled
CI / security (push) Has been cancelled
CI / helm (push) Has been cancelled
CI / unit_tests (push) Has been cancelled
CI / benchmark-regression (push) Has been cancelled
CI / coverage (push) Has been cancelled
CI / docker (push) Has been cancelled
CI / status-check (push) Has been cancelled
Remove the --mode/-m CLI flag from the validation attach command to align with the specification, which defines validation mode as an inherent property of the validation definition set at registration time via validation add, not as a per-attachment override. The service layer (ToolRegistryService.attach_validation) no longer accepts mode as a parameter; instead it reads the mode from the validation's registered definition. The ToolRegistryRepository _to_legacy_domain now exposes the mode field so the service can access it. All tests that passed --mode to the CLI or service have been updated or removed. The invalid-mode validation scenarios were removed since the mode is no longer caller-supplied. Coverage remains at 98.7%. ISSUES CLOSED: #913 Co-authored-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-committed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
This commit was merged in pull request #1305.
This commit is contained in:
@@ -584,7 +584,7 @@ Feature: Consolidated Tool
|
||||
Scenario: attach_validation raises NotFoundError when validation does not exist
|
||||
Given a mock-based tool registry service
|
||||
Given the mock tool repo returns None for get_by_name
|
||||
When I attempt to attach validation with mode "required" via the coverage service
|
||||
When I attempt to attach validation via the coverage service
|
||||
Then a coverage NotFoundError should be raised with message containing "not found"
|
||||
|
||||
# --- attach_validation: successful delegation ------------------------------
|
||||
@@ -593,7 +593,7 @@ Feature: Consolidated Tool
|
||||
Scenario: attach_validation delegates to attachment repo when validation exists
|
||||
Given a mock-based tool registry service
|
||||
Given the mock tool repo returns a sentinel for get_by_name
|
||||
When I attempt to attach validation with mode "required" via the coverage service
|
||||
When I attempt to attach validation via the coverage service
|
||||
Then no error should be raised by the coverage service
|
||||
|
||||
|
||||
@@ -661,15 +661,6 @@ Feature: Consolidated Tool
|
||||
Then the list result should be returned from list_all
|
||||
And no error should be raised by the fallback service
|
||||
|
||||
# --- attach_validation with invalid mode --------------------------------
|
||||
|
||||
|
||||
Scenario: attach_validation raises ValidationError for invalid mode
|
||||
Given a tool registry service with a standard mock repo
|
||||
When I attach validation with invalid mode "bogus" via the fallback service
|
||||
Then a fallback ValidationError should be raised with message containing "Invalid mode"
|
||||
|
||||
|
||||
# ============================================================
|
||||
# Originally from: tool_registry_service_uncovered_branches.feature
|
||||
# Feature: Tool Registry Service – uncovered branch paths
|
||||
@@ -746,23 +737,6 @@ Feature: Consolidated Tool
|
||||
When I trs branch list tools with namespace "local" type "tool" source "builtin"
|
||||
Then the trs branch list result should come from the repo
|
||||
|
||||
# --- attach_validation: invalid mode (L168-169) ----------------------------
|
||||
|
||||
|
||||
Scenario: attach_validation raises ValidationError for invalid mode
|
||||
Given a trs branch mock-based tool registry service
|
||||
Given a trs branch standard tool repo
|
||||
When I trs branch attach validation with mode "strict"
|
||||
Then a trs branch ValidationError should be raised with message containing "Invalid mode"
|
||||
|
||||
|
||||
Scenario: attach_validation raises ValidationError for empty mode
|
||||
Given a trs branch mock-based tool registry service
|
||||
Given a trs branch standard tool repo
|
||||
When I trs branch attach validation with mode "optional"
|
||||
Then a trs branch ValidationError should be raised with message containing "Invalid mode"
|
||||
|
||||
|
||||
# ============================================================
|
||||
# Originally from: tool_runtime.feature
|
||||
# Feature: Tool Runtime Core
|
||||
|
||||
@@ -522,18 +522,6 @@ def step_run_validation_attach_project(
|
||||
)
|
||||
|
||||
|
||||
@when(
|
||||
'I run validation CLI attach with mode "{resource}" "{val_name}" mode is "{mode}"'
|
||||
)
|
||||
def step_run_validation_attach_mode(
|
||||
context: Context, resource: str, val_name: str, mode: str
|
||||
) -> None:
|
||||
with _patch_val_svc(context):
|
||||
context.validation_result = _runner.invoke(
|
||||
validation_app, ["attach", resource, val_name, "--mode", mode]
|
||||
)
|
||||
|
||||
|
||||
@when(
|
||||
'I run validation CLI attach with extra args "{resource}" "{val_name}" '
|
||||
'arg is "{arg}"'
|
||||
|
||||
@@ -144,7 +144,10 @@ def step_repo_returns_none(context: Context) -> None:
|
||||
|
||||
@given("the mock tool repo returns a sentinel for get_by_name")
|
||||
def step_repo_returns_sentinel(context: Context) -> None:
|
||||
context.cov_tool_repo._get_by_name_return = {"name": "local/some-check"}
|
||||
context.cov_tool_repo._get_by_name_return = {
|
||||
"name": "local/some-check",
|
||||
"mode": "required",
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -181,13 +184,12 @@ def step_when_remove(context: Context, name: str) -> None:
|
||||
context.cov_error = exc
|
||||
|
||||
|
||||
@when('I attempt to attach validation with mode "{mode}" via the coverage service')
|
||||
def step_when_attach(context: Context, mode: str) -> None:
|
||||
@when("I attempt to attach validation via the coverage service")
|
||||
def step_when_attach(context: Context) -> None:
|
||||
try:
|
||||
context.cov_result = context.cov_service.attach_validation(
|
||||
resource_id="res-001",
|
||||
validation_name="local/some-check",
|
||||
mode=mode,
|
||||
)
|
||||
context.cov_error = None
|
||||
except Exception as exc:
|
||||
|
||||
@@ -327,7 +327,6 @@ def step_when_attach_invalid_mode(context: Context, mode: str) -> None:
|
||||
context.fb_result = context.fb_service.attach_validation(
|
||||
validation_name="local/check",
|
||||
resource_id="res-001",
|
||||
mode=mode,
|
||||
)
|
||||
context.fb_error = None
|
||||
except Exception as exc:
|
||||
|
||||
@@ -405,7 +405,6 @@ def step_trs_branch_attach_invalid_mode(context: Context, mode: str) -> None:
|
||||
context.trs_b_result = context.trs_b_service.attach_validation(
|
||||
validation_name="local/some-validation",
|
||||
resource_id="res-001",
|
||||
mode=mode,
|
||||
)
|
||||
context.trs_b_error = None
|
||||
except Exception as exc:
|
||||
|
||||
@@ -402,7 +402,6 @@ def step_service_attach(context: Context, val_name: str, res_id: str) -> None:
|
||||
context.service_attachment = context.tool_service.attach_validation(
|
||||
validation_name=val_name,
|
||||
resource_id=res_id,
|
||||
mode="required",
|
||||
)
|
||||
context.tool_session.commit()
|
||||
|
||||
@@ -419,7 +418,6 @@ def step_service_given_attached(context: Context, val_name: str, res_id: str) ->
|
||||
context.service_attachment = context.tool_service.attach_validation(
|
||||
validation_name=val_name,
|
||||
resource_id=res_id,
|
||||
mode="required",
|
||||
)
|
||||
context.tool_session.commit()
|
||||
|
||||
|
||||
@@ -202,11 +202,6 @@ Feature: Tool and Validation CLI commands
|
||||
When I run validation CLI attach scoped "resource/r1" "local/test-val" with project "myproj"
|
||||
Then the validation CLI attach should succeed
|
||||
|
||||
Scenario: Attach validation with mode informational
|
||||
Given a mocked validation exists for attaching
|
||||
When I run validation CLI attach with mode "resource/r1" "local/test-val" mode is "informational"
|
||||
Then the validation CLI attach should succeed
|
||||
|
||||
Scenario: Attach validation with extra args
|
||||
Given a mocked validation exists for attaching
|
||||
When I run validation CLI attach with extra args "resource/r1" "local/test-val" arg is "threshold=80"
|
||||
|
||||
@@ -289,7 +289,6 @@ def cmd_register_projects() -> None:
|
||||
tool_svc.attach_validation(
|
||||
validation_name=_VALIDATION_NAME,
|
||||
resource_id=info["resource"],
|
||||
mode="required",
|
||||
project_name=proj_name,
|
||||
)
|
||||
# Verify attachments
|
||||
@@ -637,7 +636,6 @@ def cmd_full_lifecycle() -> None:
|
||||
tool_svc.attach_validation(
|
||||
validation_name=_VALIDATION_NAME,
|
||||
resource_id=info["resource"],
|
||||
mode="required",
|
||||
project_name=proj_name,
|
||||
)
|
||||
|
||||
|
||||
@@ -244,7 +244,6 @@ def validation_attach() -> None:
|
||||
attachment = svc.attach_validation(
|
||||
validation_name=vname,
|
||||
resource_id=resource_id,
|
||||
mode="required",
|
||||
project_name="local/ci-workspace",
|
||||
)
|
||||
assert attachment is not None, f"Attachment for {vname} must be created"
|
||||
|
||||
@@ -282,7 +282,6 @@ def _register_attach_and_run_validation(
|
||||
tool_service.attach_validation(
|
||||
validation_name="local/auth-coverage",
|
||||
resource_id=resource_id,
|
||||
mode="required",
|
||||
plan_id=plan_id,
|
||||
args={"target": target},
|
||||
)
|
||||
|
||||
@@ -9,7 +9,7 @@ from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
from cleveragents.core.exceptions import NotFoundError, ValidationError
|
||||
from cleveragents.core.exceptions import NotFoundError
|
||||
from cleveragents.domain.models.core.tool import Tool
|
||||
from cleveragents.infrastructure.database.repositories import (
|
||||
ToolRegistryRepository,
|
||||
@@ -143,17 +143,19 @@ class ToolRegistryService:
|
||||
self,
|
||||
validation_name: str,
|
||||
resource_id: str,
|
||||
mode: str = "required",
|
||||
project_name: str | None = None,
|
||||
plan_id: str | None = None,
|
||||
args: dict[str, Any] | None = None,
|
||||
) -> Any:
|
||||
"""Attach a validation to a resource.
|
||||
|
||||
The validation mode (``required`` or ``informational``) is read from
|
||||
the validation's registered definition — it is not an attach-time
|
||||
override.
|
||||
|
||||
Args:
|
||||
validation_name: Name of the validation tool to attach.
|
||||
resource_id: Resource reference string.
|
||||
mode: ``required`` or ``informational``.
|
||||
project_name: Optional project scope.
|
||||
plan_id: Optional plan scope.
|
||||
args: Optional invocation arguments.
|
||||
@@ -165,9 +167,6 @@ class ToolRegistryService:
|
||||
NotFoundError: If the validation tool does not exist.
|
||||
DatabaseError: On persistence failure.
|
||||
"""
|
||||
if mode not in {"required", "informational"}:
|
||||
raise ValidationError(f"Invalid mode '{mode}'")
|
||||
|
||||
# Verify validation exists
|
||||
existing = self._tool_repo.get_by_name(validation_name)
|
||||
if existing is None:
|
||||
@@ -176,10 +175,17 @@ class ToolRegistryService:
|
||||
resource_id=validation_name,
|
||||
)
|
||||
|
||||
# Read mode from the validation's registered definition
|
||||
if isinstance(existing, dict):
|
||||
mode = existing.get("mode", "required")
|
||||
else:
|
||||
mode = getattr(existing, "mode", "required")
|
||||
mode_str: str = str(mode) if mode is not None else "required"
|
||||
|
||||
return self._attachment_repo.attach(
|
||||
validation_name=validation_name,
|
||||
resource_id=resource_id,
|
||||
mode=mode,
|
||||
mode=mode_str,
|
||||
project_name=project_name,
|
||||
plan_id=plan_id,
|
||||
args=args,
|
||||
|
||||
@@ -265,10 +265,6 @@ def attach(
|
||||
str | None,
|
||||
typer.Option("--plan", help="Plan ID scope"),
|
||||
] = None,
|
||||
mode: Annotated[
|
||||
str,
|
||||
typer.Option("--mode", "-m", help="Validation mode: required or informational"),
|
||||
] = "required",
|
||||
fmt: Annotated[
|
||||
str,
|
||||
typer.Option("--format", "-f", help=_FORMAT_HELP),
|
||||
@@ -276,10 +272,12 @@ def attach(
|
||||
) -> None:
|
||||
"""Attach a validation to a resource.
|
||||
|
||||
The validation mode (required or informational) is determined by the
|
||||
validation's registered definition, not overridden at attach time.
|
||||
|
||||
Examples:
|
||||
agents validation attach git-checkout/my-repo local/coverage-check
|
||||
agents validation attach --project myproj git-checkout/my-repo local/lint
|
||||
agents validation attach --mode informational res1 local/val1
|
||||
"""
|
||||
try:
|
||||
# Parse extra args
|
||||
@@ -300,7 +298,6 @@ def attach(
|
||||
attachment = service.attach_validation(
|
||||
validation_name=validation_name,
|
||||
resource_id=resource,
|
||||
mode=mode,
|
||||
project_name=project,
|
||||
plan_id=plan_id,
|
||||
args=extra_args,
|
||||
|
||||
@@ -3733,6 +3733,7 @@ class ToolRepository(ToolRegistryRepository):
|
||||
output_schema=row.output_schema,
|
||||
tool_type=cast(str, row.tool_type),
|
||||
source=cast(str, row.source),
|
||||
mode=cast(str, row.mode) if row.mode is not None else None,
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user