2 Commits

Author	SHA1	Message	Date
brent.edwards	a5cc81354d	fix: add --required/--informational flags to validation add CLI ... Add --required and --informational as mutually exclusive boolean options to the `agents validation add` CLI command. When specified, they override the `mode` field from the YAML config file. This aligns the CLI with the specification (specification.md line 22339) which states the validation mode can be set "via --required/--informational on agents validation add". The fix resolves the spec contradiction by: - Implementing the flags in the CLI (per Core Concepts > Validation Mode) - Updating the formal CLI reference to include the new flags - Adding an exception to Design Principle #3 for validation add - Removing the spurious positional name argument from the walkthrough TDD tests from #1102 now run normally with @tdd_expected_fail removed. New edge-case tests cover mutual exclusivity (both flags rejected). Also excludes tool/wrapping.py from semgrep exec/compile rules since that module intentionally uses exec() in a controlled sandbox for the validation transform feature. ISSUES CLOSED: #1038	2026-03-31 07:42:06 +00:00
brent.edwards	3abf25f17f	test: add TDD bug-capture test for #1038 — validation add --required flag (#1133) ... ## Summary Adds TDD bug-capture tests for bug #1038 — the `agents validation add` command is missing `--required`/`--informational` flags that the specification describes at line 22334 and in numerous walkthrough examples. ### Behave BDD Tests Four Behave BDD scenarios verify the correct expected behavior: 1. **`--required` flag is accepted** — invokes `validation add --config <file> --required` and asserts exit code 0 and mode `"required"` in output. 2. **`--informational` flag is accepted** — same pattern with `--informational`. 3. **`--required` overrides YAML config mode** — creates a YAML config with `mode: informational`, invokes with `--required`, and asserts the CLI output and service layer both reflect `mode: "required"`. 4. **`--informational` overrides YAML config mode** — creates a YAML config with `mode: required`, invokes with `--informational`, and asserts the CLI output and service layer both reflect `mode: "informational"`. ### Robot Framework Integration Tests Four Robot Framework integration tests exercise the same CLI paths as the Behave scenarios via a subprocess helper script: 1. **TDD Validation Add Required Flag Accepted** — verifies `--required` is recognised by the CLI. 2. **TDD Validation Add Informational Flag Accepted** — verifies `--informational` is recognised. 3. **TDD Validation Add Required Flag Overrides YAML Config** — verifies `--required` overrides a YAML config with `mode: informational`. 4. **TDD Validation Add Informational Flag Overrides YAML Config** — verifies `--informational` overrides a YAML config with `mode: required`. The helper script (`robot/helper_tdd_validation_required_flag.py`) uses `typer.testing.CliRunner` with mocked services, following the same pattern as other TDD Robot helpers (e.g., `helper_tdd_plan_apply_yes_flag.py`). ### Tags All scenarios (Behave and Robot) are tagged `@tdd_bug @tdd_bug_1038 @tdd_expected_fail`. Because the bug is still present, the underlying assertions fail (proving the bug exists) and the `@tdd_expected_fail` tag inverts the result so CI passes. Once bug #1038 is fixed, the tag must be removed. ### Spec Contradiction Note Rui Hu's investigation (issue #1038 comment #70755) found that the formal CLI reference (specification.md lines 9279–9290) does NOT include `--required`/`--informational` flags — they appear only in walkthrough examples and specification.md line 22334. Additionally, specification.md line 30761 states: "For entity registration commands (`actor add`, `skill add`, `tool add`, `validation add`, …), the YAML configuration file is the sole source of truth — the `--config` file fully defines the entity and no CLI override flags are accepted." The resolution may be to add the flags to the CLI OR to clean up the spec. This is documented in both the feature file header and step definition module docstring. ### Key Design Decisions - **YAML configs use flat `mode` key** — matches what `Validation.from_config()` actually reads (`config.get("mode", "required")`). The specification's Validation Configuration schema (specification.md line 34109) nests mode under a `validation` key, but the current `from_config()` reads a flat top-level `mode` key. A comment in the step definitions documents this discrepancy. - **Mock uses `side_effect = lambda v: v`** — the `register_tool` mock returns the actual Validation object passed by the CLI, so the CLI output reflects real CLI processing rather than hard-coded mock configuration. This prevents false positives where the mock masks an incomplete fix. - **False-positive guard** — the Then step verifying mode checks both CLI output (via `f"mode: {expected_mode}"` prefix match) AND `register_tool` call args to prevent the mock from masking an incomplete fix. - **Robot helper follows established pattern** — uses `typer.testing.CliRunner` with mocked `_get_tool_registry_service`, matching the pattern from `helper_tdd_plan_apply_yes_flag.py`. Reports real outcome (exit 0 + sentinel on bug fixed, exit 1 on bug present) and relies on `tdd_expected_fail_listener` for result inversion. - **Symmetric override scenarios** — both directions are tested: `--required` overriding informational YAML config, and `--informational` overriding required YAML config. - **Positional NAME omitted** — the bug report includes a positional NAME argument, but that is a separate spec inconsistency not under test here. - **Mutual exclusivity deferred** — testing what happens when both `--required` and `--informational` are passed simultaneously is deferred to the bug-fix PR for #1038. Closes #1102 Reviewed-on: #1133 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com> Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>	2026-03-28 18:09:27 +00:00