diff --git a/features/consolidated_tool.feature b/features/consolidated_tool.feature index 220217a9e..b602a6087 100644 --- a/features/consolidated_tool.feature +++ b/features/consolidated_tool.feature @@ -968,35 +968,35 @@ Feature: Consolidated Tool Scenario: List tools with tool_type filter returns matching tools Given a tool registry - And a registered tool spec named "test/filtered" with tool_type "tool" + And a registered tool spec named "test/filtered" typed as "tool" When I list tools with tool_type "tool" Then the tool list should contain 1 tools Scenario: List tools with tool_type validation returns only validations Given a tool registry - And a registered tool spec named "test/my-tool" with tool_type "tool" - And a registered tool spec named "test/my-validation" with tool_type "validation" + And a registered tool spec named "test/my-tool" typed as "tool" + And a registered tool spec named "test/my-validation" typed as "validation" When I list tools with tool_type "validation" Then the tool list should contain 1 tools Scenario: List tools with tool_type tool excludes validations Given a tool registry - And a registered tool spec named "test/plain-tool" with tool_type "tool" - And a registered tool spec named "test/plain-validation" with tool_type "validation" + And a registered tool spec named "test/plain-tool" typed as "tool" + And a registered tool spec named "test/plain-validation" typed as "validation" When I list tools with tool_type "tool" Then the tool list should contain 1 tools Scenario: List tools without tool_type filter returns all tools Given a tool registry - And a registered tool spec named "test/all-tool" with tool_type "tool" - And a registered tool spec named "test/all-validation" with tool_type "validation" + And a registered tool spec named "test/all-tool" typed as "tool" + And a registered tool spec named "test/all-validation" typed as "validation" When I list all tools Then the tool list should contain 2 tools Scenario: List tools with tool_type None returns all tools Given a tool registry - And a registered tool spec named "test/none-tool" with tool_type "tool" - And a registered tool spec named "test/none-validation" with tool_type "validation" + And a registered tool spec named "test/none-tool" typed as "tool" + And a registered tool spec named "test/none-validation" typed as "validation" When I list all tools Then the tool list should contain 2 tools @@ -1008,7 +1008,7 @@ Feature: Consolidated Tool Scenario: ToolSpec with validation tool_type is excluded from tool filter Given a tool registry - And a registered tool spec named "test/excl-validation" with tool_type "validation" + And a registered tool spec named "test/excl-validation" typed as "validation" When I list tools with tool_type "tool" Then the tool list should contain 0 tools diff --git a/features/steps/tdd_cli_incomplete_subcommand_registration_steps.py b/features/steps/tdd_cli_incomplete_subcommand_registration_steps.py index ff29b04e8..c8edcb6c3 100644 --- a/features/steps/tdd_cli_incomplete_subcommand_registration_steps.py +++ b/features/steps/tdd_cli_incomplete_subcommand_registration_steps.py @@ -22,7 +22,7 @@ from behave import given, then, when # type: ignore[import-untyped] from behave.runner import Context # type: ignore[import-untyped] from typer.testing import CliRunner -runner = CliRunner(mix_stderr=False) +runner = CliRunner() @given("the CLI subcommand import raises an ImportError during registration") diff --git a/features/steps/tool_cli_steps.py b/features/steps/tool_cli_steps.py index a12335429..77ea46562 100644 --- a/features/steps/tool_cli_steps.py +++ b/features/steps/tool_cli_steps.py @@ -522,6 +522,19 @@ def step_run_validation_attach_project( ) +@when( + 'I run validation CLI attach with named option "{resource}" "{val_name}" ' + 'option "{option}" value "{value}"' +) +def step_run_validation_attach_named_option( + context: Context, resource: str, val_name: str, option: str, value: str +) -> None: + with _patch_val_svc(context): + context.validation_result = _runner.invoke( + validation_app, ["attach", resource, val_name, option, value] + ) + + @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 70c64e7dc..cb67c114f 100644 --- a/features/steps/tool_registry_service_coverage_steps.py +++ b/features/steps/tool_registry_service_coverage_steps.py @@ -147,6 +147,7 @@ def step_repo_returns_sentinel(context: Context) -> None: context.cov_tool_repo._get_by_name_return = { "name": "local/some-check", "mode": "required", + "tool_type": "validation", } diff --git a/features/steps/tool_runtime_steps.py b/features/steps/tool_runtime_steps.py index 48d396c7d..e823b21b9 100644 --- a/features/steps/tool_runtime_steps.py +++ b/features/steps/tool_runtime_steps.py @@ -4,7 +4,7 @@ import json import threading from typing import Any -from behave import given, then, when +from behave import given, then, use_step_matcher, when from cleveragents.domain.models.core.tool import ToolCapability from cleveragents.tool.registry import ToolRegistry @@ -58,7 +58,10 @@ def step_given_tool_spec_with_handler(context: Any, name: str) -> None: ) -@given('a registered tool spec named "{name}"') +use_step_matcher("re") + + +@given(r'a registered tool spec named "(?P[^"]+)"') def step_given_registered_tool_spec(context: Any, name: str) -> None: spec = ToolSpec( name=name, @@ -68,7 +71,7 @@ def step_given_registered_tool_spec(context: Any, name: str) -> None: context.registry.register(spec) -@given('a registered tool spec named "{name}" with tool_type "{tt}"') +@given(r'a registered tool spec named "(?P[^"]+)" typed as "(?P[^"]+)"') def step_given_registered_tool_spec_with_type(context: Any, name: str, tt: str) -> None: spec = ToolSpec( name=name, @@ -79,6 +82,9 @@ def step_given_registered_tool_spec_with_type(context: Any, name: str, tt: str) context.registry.register(spec) +use_step_matcher("parse") + + @given('a registered tool spec named "{name}" with an adder handler') def step_given_registered_adder(context: Any, name: str) -> None: spec = ToolSpec( diff --git a/features/steps/validation_attach_type_guard_steps.py b/features/steps/validation_attach_type_guard_steps.py index 148724ce0..429807995 100644 --- a/features/steps/validation_attach_type_guard_steps.py +++ b/features/steps/validation_attach_type_guard_steps.py @@ -125,6 +125,82 @@ def step_invoke_validation_attach(context: Context, name: str, resource: str) -> context.last_result = result +@when( + 'I invoke validation attach with named option "{option}" "{value}" ' + 'for "{name}" to "{resource}"' +) +def step_invoke_validation_attach_named_option( + context: Context, option: str, value: str, name: str, resource: str +) -> None: + """Invoke the validation attach CLI command with a single named option.""" + result = context.runner.invoke( + validation_app, + ["attach", resource, name, "--format", "plain", option, value], + ) + context.last_result = result + + +@when( + 'I invoke validation attach with named options "{option1}" "{value1}" ' + '"{option2}" "{value2}" for "{name}" to "{resource}"' +) +def step_invoke_validation_attach_named_options( + context: Context, + option1: str, + value1: str, + option2: str, + value2: str, + name: str, + resource: str, +) -> None: + """Invoke the validation attach CLI command with multiple named options.""" + result = context.runner.invoke( + validation_app, + [ + "attach", + resource, + name, + "--format", + "plain", + option1, + value1, + option2, + value2, + ], + ) + context.last_result = result + + +@when( + 'I invoke validation attach with positional arg "{arg}" ' + 'for "{name}" to "{resource}"' +) +def step_invoke_validation_attach_positional_arg( + context: Context, arg: str, name: str, resource: str +) -> None: + """Invoke the validation attach CLI command with a positional key=value arg.""" + result = context.runner.invoke( + validation_app, + ["attach", resource, name, "--format", "plain", arg], + ) + context.last_result = result + + +@when( + 'I invoke validation attach with dangling option "{option}" ' + 'for "{name}" to "{resource}"' +) +def step_invoke_validation_attach_dangling_option( + context: Context, option: str, name: str, resource: str +) -> None: + """Invoke the validation attach CLI command with a named option missing its value.""" + result = context.runner.invoke( + validation_app, + ["attach", resource, name, "--format", "plain", option], + ) + context.last_result = result + + # --------------------------------------------------------------------------- # Then: rejection # --------------------------------------------------------------------------- diff --git a/features/tool_cli.feature b/features/tool_cli.feature index a69840940..b2885e857 100644 --- a/features/tool_cli.feature +++ b/features/tool_cli.feature @@ -202,12 +202,12 @@ 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 extra args + Scenario: Attach validation with extra args using named option format 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" + When I run validation CLI attach with named option "resource/r1" "local/test-val" option "--threshold" value "80" Then the validation CLI attach should succeed - Scenario: Attach validation with invalid arg format + Scenario: Attach validation with invalid arg format (not a named option) Given a mocked validation exists for attaching When I run validation CLI attach with extra args "resource/r1" "local/test-val" arg is "badarg" Then the validation CLI command should abort diff --git a/features/validation_attach_named_options.feature b/features/validation_attach_named_options.feature new file mode 100644 index 000000000..75c593ac5 --- /dev/null +++ b/features/validation_attach_named_options.feature @@ -0,0 +1,47 @@ +Feature: Validation attach uses --key value named option format for extra arguments + As a CleverAgents user + I want the "agents validation attach" command to accept extra arguments + as named options in the format "--key value" (e.g. "--coverage-threshold 90") + So that the CLI is consistent with the specification and other commands + + Background: + Given a validation attach type guard test runner + And a validation attach type guard mocked environment + + # --- Spec-compliant named option format --- + + Scenario: Attach accepts a single named option argument + Given a genuine validation "local/run-tests" is registered with tool_type "validation" + When I invoke validation attach with named option "--coverage-threshold" "90" for "local/run-tests" to "local/api-repo" + Then the validation attach should succeed + + Scenario: Attach accepts a named option with hyphenated key + Given a genuine validation "local/lint-check" is registered with tool_type "validation" + When I invoke validation attach with named option "--max-line-length" "120" for "local/lint-check" to "git-checkout/my-repo" + Then the validation attach should succeed + + Scenario: Attach accepts multiple named option arguments + Given a genuine validation "local/coverage-check" is registered with tool_type "validation" + When I invoke validation attach with named options "--threshold" "80" "--strict" "true" for "local/coverage-check" to "local/api-repo" + Then the validation attach should succeed + + # --- Rejection of old positional key=value format --- + + Scenario: Attach rejects positional key=value argument format + Given a genuine validation "local/run-tests" is registered with tool_type "validation" + When I invoke validation attach with positional arg "coverage_threshold=90" for "local/run-tests" to "local/api-repo" + Then the validation attach should be rejected + And the rejection output should contain "Invalid argument format" + + Scenario: Attach rejects bare positional argument without equals sign + Given a genuine validation "local/run-tests" is registered with tool_type "validation" + When I invoke validation attach with positional arg "badarg" for "local/run-tests" to "local/api-repo" + Then the validation attach should be rejected + + # --- Named option missing value --- + + Scenario: Attach rejects named option without a value + Given a genuine validation "local/run-tests" is registered with tool_type "validation" + When I invoke validation attach with dangling option "--coverage-threshold" for "local/run-tests" to "local/api-repo" + Then the validation attach should be rejected + And the rejection output should contain "Missing value" diff --git a/src/cleveragents/cli/commands/validation.py b/src/cleveragents/cli/commands/validation.py index b39e92324..4e6346c04 100644 --- a/src/cleveragents/cli/commands/validation.py +++ b/src/cleveragents/cli/commands/validation.py @@ -259,8 +259,12 @@ def add( raise typer.Abort() from exc -@app.command("attach") +@app.command( + "attach", + context_settings={"allow_extra_args": True, "ignore_unknown_options": True}, +) def attach( + ctx: typer.Context, resource: Annotated[ str, typer.Argument(help="Resource reference to attach validation to"), @@ -269,10 +273,6 @@ def attach( str, typer.Argument(help="Namespaced name of the validation"), ], - args: Annotated[ - list[str] | None, - typer.Argument(help="Additional arguments (key=value pairs)"), - ] = None, project: Annotated[ str | None, typer.Option("--project", "-p", help="Project scope"), @@ -291,24 +291,46 @@ def attach( The validation mode (required or informational) is determined by the validation's registered definition, not overridden at attach time. + Validation-specific arguments are passed as named options using the + ``--key value`` format (e.g. ``--coverage-threshold 90``). Hyphens in + option names are converted to underscores in the stored argument dict + (e.g. ``--coverage-threshold`` becomes ``coverage_threshold``). + 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 --project local/api-service \\ + local/api-repo local/run-tests --coverage-threshold 90 """ try: - # Parse extra args + # Parse extra named options from ctx.args (--key value format). + # ctx.args contains all unrecognised tokens after Typer's own parsing. extra_args: dict[str, str] | None = None - if args: + raw_extra = list(ctx.args) + if raw_extra: extra_args = {} - for arg in args: - if "=" not in arg: + i = 0 + while i < len(raw_extra): + token = raw_extra[i] + if not token.startswith("--"): console.print( - f"[red]Invalid argument format:[/red] {arg} " - "(expected key=value)" + f"[red]Invalid argument format:[/red] {token!r} " + "(expected --key value named option format, " + "e.g. --coverage-threshold 90)" ) raise typer.Abort() - key, val = arg.split("=", 1) + key = token[2:].replace( + "-", "_" + ) # --coverage-threshold → coverage_threshold + if i + 1 >= len(raw_extra) or raw_extra[i + 1].startswith("--"): + console.print( + f"[red]Missing value for option:[/red] {token} " + "(expected --key value, e.g. --coverage-threshold 90)" + ) + raise typer.Abort() + val = raw_extra[i + 1] extra_args[key] = val + i += 2 service = _get_tool_registry_service() attachment = service.attach_validation(