From 9f83786ca4d73e06d0b7da14284a092a4adc7785 Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Thu, 2 Apr 2026 09:43:45 +0000 Subject: [PATCH] fix(cli): remove extra --mode flag from validation attach 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 --- features/consolidated_tool.feature | 30 ++----------------- features/steps/tool_cli_steps.py | 12 -------- .../tool_registry_service_coverage_steps.py | 10 ++++--- ...egistry_service_fallback_coverage_steps.py | 1 - ...gistry_service_uncovered_branches_steps.py | 1 - features/steps/tool_registry_steps.py | 2 -- features/tool_cli.feature | 5 ---- robot/helper_wf04_multi_project_dependency.py | 2 -- robot/helper_wf07_cicd.py | 1 - robot/wf02_test_generation_validation.py | 1 - .../services/tool_registry_service.py | 20 ++++++++----- src/cleveragents/cli/commands/validation.py | 9 ++---- .../infrastructure/database/repositories.py | 1 + 13 files changed, 25 insertions(+), 70 deletions(-) diff --git a/features/consolidated_tool.feature b/features/consolidated_tool.feature index 7a83a31f2..356850273 100644 --- a/features/consolidated_tool.feature +++ b/features/consolidated_tool.feature @@ -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 diff --git a/features/steps/tool_cli_steps.py b/features/steps/tool_cli_steps.py index 7e85f159b..a12335429 100644 --- a/features/steps/tool_cli_steps.py +++ b/features/steps/tool_cli_steps.py @@ -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}"' diff --git a/features/steps/tool_registry_service_coverage_steps.py b/features/steps/tool_registry_service_coverage_steps.py index f2d5d522e..70c64e7dc 100644 --- a/features/steps/tool_registry_service_coverage_steps.py +++ b/features/steps/tool_registry_service_coverage_steps.py @@ -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: diff --git a/features/steps/tool_registry_service_fallback_coverage_steps.py b/features/steps/tool_registry_service_fallback_coverage_steps.py index f642d7858..0049c2142 100644 --- a/features/steps/tool_registry_service_fallback_coverage_steps.py +++ b/features/steps/tool_registry_service_fallback_coverage_steps.py @@ -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: diff --git a/features/steps/tool_registry_service_uncovered_branches_steps.py b/features/steps/tool_registry_service_uncovered_branches_steps.py index 2f92a783f..974013b29 100644 --- a/features/steps/tool_registry_service_uncovered_branches_steps.py +++ b/features/steps/tool_registry_service_uncovered_branches_steps.py @@ -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: diff --git a/features/steps/tool_registry_steps.py b/features/steps/tool_registry_steps.py index 0c19ee8ad..e6dbcd724 100644 --- a/features/steps/tool_registry_steps.py +++ b/features/steps/tool_registry_steps.py @@ -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() diff --git a/features/tool_cli.feature b/features/tool_cli.feature index b1f9b0652..a69840940 100644 --- a/features/tool_cli.feature +++ b/features/tool_cli.feature @@ -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" diff --git a/robot/helper_wf04_multi_project_dependency.py b/robot/helper_wf04_multi_project_dependency.py index f66e05f50..895a39c01 100644 --- a/robot/helper_wf04_multi_project_dependency.py +++ b/robot/helper_wf04_multi_project_dependency.py @@ -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, ) diff --git a/robot/helper_wf07_cicd.py b/robot/helper_wf07_cicd.py index 5d0d19669..18a08b876 100644 --- a/robot/helper_wf07_cicd.py +++ b/robot/helper_wf07_cicd.py @@ -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" diff --git a/robot/wf02_test_generation_validation.py b/robot/wf02_test_generation_validation.py index 93ad4dc8f..f0247daa7 100644 --- a/robot/wf02_test_generation_validation.py +++ b/robot/wf02_test_generation_validation.py @@ -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}, ) diff --git a/src/cleveragents/application/services/tool_registry_service.py b/src/cleveragents/application/services/tool_registry_service.py index a87355980..c52fa95ac 100644 --- a/src/cleveragents/application/services/tool_registry_service.py +++ b/src/cleveragents/application/services/tool_registry_service.py @@ -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, diff --git a/src/cleveragents/cli/commands/validation.py b/src/cleveragents/cli/commands/validation.py index d931b97b3..82c951288 100644 --- a/src/cleveragents/cli/commands/validation.py +++ b/src/cleveragents/cli/commands/validation.py @@ -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, diff --git a/src/cleveragents/infrastructure/database/repositories.py b/src/cleveragents/infrastructure/database/repositories.py index b193719b4..2866b3a28 100644 --- a/src/cleveragents/infrastructure/database/repositories.py +++ b/src/cleveragents/infrastructure/database/repositories.py @@ -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, ) -- 2.52.0