features/steps/tdd_validation_attach_named_options_steps.py

@@ -0,0 +1,171 @@
+"""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,
+    )
+    context.last_result = context.named_opts_result
+
+
+@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,
+    )
+    context.last_result = context.named_opts_result
+
+
+# ---------------------------------------------------------------------------
+# 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.
+
+    The CLI converts hyphens in option names to underscores when forwarding
+    to the service layer (``--coverage-threshold`` → ``coverage_threshold``).
+    """
+    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}"
+    )

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"

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()

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

src/cleveragents/cli/commands/validation.py

@@ -306,8 +306,8 @@ def attach(
             local/api-repo local/run-tests --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 +315,33 @@ def attach(
             i = 0
             while i < len(raw_extra):
                 token = raw_extra[i]
-                if not token.startswith("--"):
+                if token.startswith("--"):
+                    key = token[2:].replace(
+                        "-", "_"
+                    )  # --coverage-threshold → coverage_threshold
+                    if (
+                        not key
+                    ):  # pragma: no cover - Click consumes bare '--' before ctx.args
+                        console.print(
+                            "[red]Invalid option:[/red] bare '--' is not allowed"
+                        )
+                        raise typer.Abort()
+                    # Next token is the value (must not be another --flag)
+                    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
+                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(