diff --git a/features/steps/tdd_validation_attach_named_options_steps.py b/features/steps/tdd_validation_attach_named_options_steps.py new file mode 100644 index 000000000..8b0562640 --- /dev/null +++ b/features/steps/tdd_validation_attach_named_options_steps.py @@ -0,0 +1,172 @@ +"""Step definitions for validation attach named options TDD tests. + +Verifies that ``agents validation attach`` accepts ``--key value`` named +option format for extra validation arguments, as required by the spec. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.cli.commands.validation import app as validation_app + +_ATTACHMENT_ULID = "01NAMEDOPT000000000000001" + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("a validation attach named options test runner") +def step_named_options_runner(context: Context) -> None: + """Set up the CLI runner for named options tests.""" + context.named_opts_runner = CliRunner() + + +@given("a validation attach named options mocked environment") +def step_named_options_mock_env(context: Context) -> None: + """Set up the mocked service for named options tests.""" + context.named_opts_mock_service = MagicMock() + + context.named_opts_patcher = patch( + "cleveragents.cli.commands.validation._get_tool_registry_service", + return_value=context.named_opts_mock_service, + ) + context.named_opts_patcher.start() + context.add_cleanup(context.named_opts_patcher.stop) + + context.named_opts_result = None + + +# --------------------------------------------------------------------------- +# Given +# --------------------------------------------------------------------------- + + +@given('a genuine validation "{name}" is ready for named option attach') +def step_named_opts_validation_ready(context: Context, name: str) -> None: + """Configure the mock service to return a successful attachment.""" + mock_attachment = MagicMock() + mock_attachment.attachment_id = _ATTACHMENT_ULID + mock_attachment.validation_name = name + mock_attachment.resource_id = "git-checkout/my-repo" + mock_attachment.mode = "required" + mock_attachment.project_name = None + mock_attachment.plan_id = None + mock_attachment.created_at = "2026-01-01T00:00:00" + context.named_opts_mock_service.attach_validation.return_value = mock_attachment + context.named_opts_mock_service.attach_validation.side_effect = None + + +# --------------------------------------------------------------------------- +# When +# --------------------------------------------------------------------------- + + +@when('I invoke validation attach "{resource}" "{validation}" with args "{args_str}"') +def step_named_opts_invoke_with_args( + context: Context, resource: str, validation: str, args_str: str +) -> None: + """Invoke the validation attach CLI command with extra named options.""" + # Split args_str into individual tokens (space-separated) + extra_tokens = args_str.split() + cmd = ["attach", resource, validation, "--format", "plain", *extra_tokens] + context.named_opts_result = context.named_opts_runner.invoke( + validation_app, + cmd, + ) + + +@when('I invoke validation attach "{resource}" "{validation}" with no extra args') +def step_named_opts_invoke_no_args( + context: Context, resource: str, validation: str +) -> None: + """Invoke the validation attach CLI command without extra named options.""" + cmd = ["attach", resource, validation, "--format", "plain"] + context.named_opts_result = context.named_opts_runner.invoke( + validation_app, + cmd, + ) + + +# --------------------------------------------------------------------------- +# Then: success +# --------------------------------------------------------------------------- + + +@then("the named option attach should succeed") +def step_named_opts_attach_succeeds(context: Context) -> None: + """Verify the attach command exited with code 0.""" + result = context.named_opts_result + assert result is not None + assert result.exit_code == 0, ( + f"Expected exit 0 (success), got {result.exit_code}. Output: {result.output}" + ) + + +@then('the service should have received coverage-threshold as "{expected_val}"') +def step_named_opts_received_coverage_threshold( + context: Context, expected_val: str +) -> None: + """Verify the service received coverage-threshold in the args dict.""" + call_kwargs = context.named_opts_mock_service.attach_validation.call_args + assert call_kwargs is not None, "attach_validation was not called" + args_dict = call_kwargs.kwargs.get("args") or {} + assert "coverage-threshold" in args_dict, ( + f"Expected 'coverage-threshold' in args, got: {args_dict}" + ) + assert args_dict["coverage-threshold"] == expected_val, ( + f"Expected coverage-threshold={expected_val!r}, got {args_dict['coverage-threshold']!r}" + ) + + +@then('the service should have received threshold as "{expected_val}"') +def step_named_opts_received_threshold(context: Context, expected_val: str) -> None: + """Verify the service received threshold in the args dict.""" + call_kwargs = context.named_opts_mock_service.attach_validation.call_args + assert call_kwargs is not None, "attach_validation was not called" + args_dict = call_kwargs.kwargs.get("args") or {} + assert "threshold" in args_dict, f"Expected 'threshold' in args, got: {args_dict}" + assert args_dict["threshold"] == expected_val, ( + f"Expected threshold={expected_val!r}, got {args_dict['threshold']!r}" + ) + + +@then('the service should have received strict as "{expected_val}"') +def step_named_opts_received_strict(context: Context, expected_val: str) -> None: + """Verify the service received strict in the args dict.""" + call_kwargs = context.named_opts_mock_service.attach_validation.call_args + assert call_kwargs is not None, "attach_validation was not called" + args_dict = call_kwargs.kwargs.get("args") or {} + assert "strict" in args_dict, f"Expected 'strict' in args, got: {args_dict}" + assert args_dict["strict"] == expected_val, ( + f"Expected strict={expected_val!r}, got {args_dict['strict']!r}" + ) + + +# --------------------------------------------------------------------------- +# Then: rejection +# --------------------------------------------------------------------------- + + +@then("the named option attach should be rejected") +def step_named_opts_attach_rejected(context: Context) -> None: + """Verify the attach command exited with a non-zero code.""" + result = context.named_opts_result + assert result is not None + assert result.exit_code != 0, ( + f"Expected non-zero exit code (rejection), got {result.exit_code}. " + f"Output: {result.output}" + ) + + +@then('the rejection output should contain "{text}"') +def step_named_opts_rejection_contains(context: Context, text: str) -> None: + """Verify the rejection output contains the expected text.""" + output = context.named_opts_result.output + assert text in output, f"Expected {text!r} in rejection output, got: {output!r}" diff --git a/features/tdd_validation_attach_named_options.feature b/features/tdd_validation_attach_named_options.feature new file mode 100644 index 000000000..e7dfb419e --- /dev/null +++ b/features/tdd_validation_attach_named_options.feature @@ -0,0 +1,40 @@ +Feature: Validation attach accepts --key value named option format + As a CleverAgents user + I want the "agents validation attach" command to accept --key value named options + So that I can use the spec-compliant format for extra validation arguments + + Background: + Given a validation attach named options test runner + And a validation attach named options mocked environment + + # --- Spec-compliant named option format --- + + Scenario: Attach with a single named option --coverage-threshold 90 + Given a genuine validation "local/coverage-check" is ready for named option attach + When I invoke validation attach "git-checkout/my-repo" "local/coverage-check" with args "--coverage-threshold 90" + Then the named option attach should succeed + And the service should have received coverage-threshold as "90" + + Scenario: Attach with multiple named options + Given a genuine validation "local/lint-check" is ready for named option attach + When I invoke validation attach "git-checkout/my-repo" "local/lint-check" with args "--threshold 70 --strict true" + Then the named option attach should succeed + And the service should have received threshold as "70" + And the service should have received strict as "true" + + Scenario: Attach without any extra named options still works + Given a genuine validation "local/basic-check" is ready for named option attach + When I invoke validation attach "git-checkout/my-repo" "local/basic-check" with no extra args + Then the named option attach should succeed + + Scenario: Attach with a bare token (not --key value) is rejected + Given a genuine validation "local/coverage-check" is ready for named option attach + When I invoke validation attach "git-checkout/my-repo" "local/coverage-check" with args "coverage-threshold=90" + Then the named option attach should be rejected + And the rejection output should contain "Invalid argument format" + + Scenario: Attach with a named option missing its value is rejected + Given a genuine validation "local/coverage-check" is ready for named option attach + When I invoke validation attach "git-checkout/my-repo" "local/coverage-check" with args "--coverage-threshold" + Then the named option attach should be rejected + And the rejection output should contain "Missing value for option" diff --git a/robot/helper_validation_attach_named_options.py b/robot/helper_validation_attach_named_options.py new file mode 100644 index 000000000..5c7159467 --- /dev/null +++ b/robot/helper_validation_attach_named_options.py @@ -0,0 +1,224 @@ +"""Helper script for validation_attach_named_options.robot E2E tests. + +Each subcommand is a self-contained check that prints a sentinel on success. +Tests that ``agents validation attach`` accepts ``--key value`` named option +format for extra validation arguments, as required by the spec. +""" + +from __future__ import annotations + +import sys +from collections.abc import Callable +from pathlib import Path +from unittest.mock import MagicMock, patch + +# Ensure local source tree is importable +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from helpers_common import reset_global_state # noqa: E402 +from typer.testing import CliRunner # noqa: E402 + +from cleveragents.cli.commands.validation import app as validation_app # noqa: E402 + +runner = CliRunner() + +_ATTACHMENT_ULID = "01NAMEDOPT000000000000001" + + +def _make_mock_attachment(validation_name: str, resource_id: str) -> MagicMock: + """Create a mock attachment object.""" + mock_attachment = MagicMock() + mock_attachment.attachment_id = _ATTACHMENT_ULID + mock_attachment.validation_name = validation_name + mock_attachment.resource_id = resource_id + mock_attachment.mode = "required" + mock_attachment.project_name = None + mock_attachment.plan_id = None + mock_attachment.created_at = "2026-01-01T00:00:00" + return mock_attachment + + +# --------------------------------------------------------------------------- +# Subcommands +# --------------------------------------------------------------------------- + + +def attach_with_coverage_threshold() -> None: + """Verify that --coverage-threshold 90 named option is accepted and forwarded.""" + mock_svc = MagicMock() + mock_svc.attach_validation.return_value = _make_mock_attachment( + "local/coverage-check", "git-checkout/my-repo" + ) + with patch( + "cleveragents.cli.commands.validation._get_tool_registry_service", + return_value=mock_svc, + ): + result = runner.invoke( + validation_app, + [ + "attach", + "git-checkout/my-repo", + "local/coverage-check", + "--coverage-threshold", + "90", + "--format", + "plain", + ], + ) + + if result.exit_code != 0: + print( + f"FAIL: exit={result.exit_code} output={result.output!r} " + f"exception={result.exception!r}" + ) + sys.exit(1) + + # Verify the service received the correct args dict + call_kwargs = mock_svc.attach_validation.call_args + if call_kwargs is None: + print("FAIL: attach_validation was not called") + sys.exit(1) + + args_dict = call_kwargs.kwargs.get("args") or {} + if args_dict.get("coverage-threshold") != "90": + print(f"FAIL: expected args={{'coverage-threshold': '90'}}, got {args_dict!r}") + sys.exit(1) + + print("validation-attach-named-option-coverage-threshold-ok") + + +def attach_with_multiple_named_options() -> None: + """Verify that multiple --key value named options are all forwarded.""" + mock_svc = MagicMock() + mock_svc.attach_validation.return_value = _make_mock_attachment( + "local/lint-check", "git-checkout/my-repo" + ) + with patch( + "cleveragents.cli.commands.validation._get_tool_registry_service", + return_value=mock_svc, + ): + result = runner.invoke( + validation_app, + [ + "attach", + "git-checkout/my-repo", + "local/lint-check", + "--threshold", + "70", + "--strict", + "true", + "--format", + "plain", + ], + ) + + if result.exit_code != 0: + print( + f"FAIL: exit={result.exit_code} output={result.output!r} " + f"exception={result.exception!r}" + ) + sys.exit(1) + + call_kwargs = mock_svc.attach_validation.call_args + if call_kwargs is None: + print("FAIL: attach_validation was not called") + sys.exit(1) + + args_dict = call_kwargs.kwargs.get("args") or {} + if args_dict.get("threshold") != "70": + print(f"FAIL: expected threshold='70', got {args_dict!r}") + sys.exit(1) + if args_dict.get("strict") != "true": + print(f"FAIL: expected strict='true', got {args_dict!r}") + sys.exit(1) + + print("validation-attach-named-option-multiple-ok") + + +def attach_without_extra_args() -> None: + """Verify that attach without extra named options still works.""" + mock_svc = MagicMock() + mock_svc.attach_validation.return_value = _make_mock_attachment( + "local/basic-check", "git-checkout/my-repo" + ) + with patch( + "cleveragents.cli.commands.validation._get_tool_registry_service", + return_value=mock_svc, + ): + result = runner.invoke( + validation_app, + [ + "attach", + "git-checkout/my-repo", + "local/basic-check", + "--format", + "plain", + ], + ) + + if result.exit_code != 0: + print( + f"FAIL: exit={result.exit_code} output={result.output!r} " + f"exception={result.exception!r}" + ) + sys.exit(1) + + print("validation-attach-named-option-no-extra-args-ok") + + +def attach_with_positional_key_value_rejected() -> None: + """Verify that old-style positional key=value format is rejected.""" + mock_svc = MagicMock() + mock_svc.attach_validation.return_value = _make_mock_attachment( + "local/coverage-check", "git-checkout/my-repo" + ) + with patch( + "cleveragents.cli.commands.validation._get_tool_registry_service", + return_value=mock_svc, + ): + result = runner.invoke( + validation_app, + [ + "attach", + "git-checkout/my-repo", + "local/coverage-check", + "coverage-threshold=90", + "--format", + "plain", + ], + ) + + # Must be rejected (non-zero exit) and output must mention "Invalid argument format" + if result.exit_code != 0 and "Invalid argument format" in result.output: + print("validation-attach-positional-key-value-rejected-ok") + else: + print( + f"FAIL: exit={result.exit_code} " + f"output={result.output!r} " + f"(expected non-zero exit and 'Invalid argument format' in output)" + ) + sys.exit(1) + + +# --------------------------------------------------------------------------- +# Dispatcher +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Callable[[], None]] = { + "attach-with-coverage-threshold": attach_with_coverage_threshold, + "attach-with-multiple-named-options": attach_with_multiple_named_options, + "attach-without-extra-args": attach_without_extra_args, + "attach-with-positional-key-value-rejected": ( + attach_with_positional_key_value_rejected + ), +} + +if __name__ == "__main__": + if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS: + print(f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>") + sys.exit(1) + reset_global_state() + fn = _COMMANDS[sys.argv[1]] + fn() diff --git a/robot/validation_attach_named_options.robot b/robot/validation_attach_named_options.robot new file mode 100644 index 000000000..ef648d1ea --- /dev/null +++ b/robot/validation_attach_named_options.robot @@ -0,0 +1,44 @@ +*** Settings *** +Documentation Integration tests for the validation attach named option format. +... Verifies that ``agents validation attach`` accepts ``--key value`` +... named option format for extra validation arguments, as required +... by the specification. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment With Database Isolation +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_validation_attach_named_options.py + +*** Test Cases *** +Validation Attach Accepts Named Option Coverage Threshold + [Documentation] Attaching with --coverage-threshold 90 must succeed and forward the arg. + ${result}= Run Process ${PYTHON} ${HELPER} attach-with-coverage-threshold cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} validation-attach-named-option-coverage-threshold-ok + +Validation Attach Accepts Multiple Named Options + [Documentation] Attaching with multiple --key value pairs must succeed and forward all args. + ${result}= Run Process ${PYTHON} ${HELPER} attach-with-multiple-named-options cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} validation-attach-named-option-multiple-ok + +Validation Attach Works Without Extra Named Options + [Documentation] Attaching without extra named options must still succeed. + ${result}= Run Process ${PYTHON} ${HELPER} attach-without-extra-args cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} validation-attach-named-option-no-extra-args-ok + +Validation Attach Rejects Positional Key Equals Value Format + [Documentation] Old-style positional key=value format must be rejected with a clear error. + ${result}= Run Process ${PYTHON} ${HELPER} attach-with-positional-key-value-rejected cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} validation-attach-positional-key-value-rejected-ok diff --git a/src/cleveragents/cli/commands/validation.py b/src/cleveragents/cli/commands/validation.py index e27f426fc..5a4c0b47a 100644 --- a/src/cleveragents/cli/commands/validation.py +++ b/src/cleveragents/cli/commands/validation.py @@ -291,23 +291,22 @@ def attach( ) -> None: """Attach a validation to a resource. + Extra validation arguments are passed as named options using ``--key value`` + format (e.g. ``--coverage-threshold 90``). The ``--`` prefix is stripped + and the remaining key/value pairs are forwarded to the validation service. + 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 + agents validation attach git-checkout/my-repo local/coverage-check \ + --coverage-threshold 90 """ try: - # Parse extra named options from ctx.args (--key value format). - # ctx.args contains all unrecognised tokens after Typer's own parsing. + # Parse extra named options from ctx.args (--key value pairs). + # ctx.args contains the remaining tokens after known options are consumed. extra_args: dict[str, str] | None = None raw_extra = list(ctx.args) if raw_extra: @@ -315,25 +314,26 @@ def attach( i = 0 while i < len(raw_extra): token = raw_extra[i] - if not token.startswith("--"): + if token.startswith("--"): + key = token[2:] # strip leading "--" + if not key: + console.print( + "[red]Invalid option:[/red] bare '--' is not allowed" + ) + raise typer.Abort() + # Next token is the value + if i + 1 >= len(raw_extra): + console.print(f"[red]Missing value for option:[/red] {token}") + raise typer.Abort() + val = raw_extra[i + 1] + extra_args[key] = val + i += 2 + else: console.print( - f"[red]Invalid argument format:[/red] {token!r} " - "(expected --key value named option format, " - "e.g. --coverage-threshold 90)" + f"[red]Invalid argument format:[/red] {token} " + "(expected --key value named option)" ) raise typer.Abort() - 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(