feat: add validate-policy and test-policy CLI commands

[yvonnedevlinrh]

Closes #178 (part of #87)

Summary

Adds two new CLI commands that mirror the MCP validate_policy and test_policy tools, closing the CLI/MCP parity gap for policy validation workflows.

New Commands

complypack validate-policy <file> --platform <platform>

Runs three checks in sequence:

1. Syntax validation — is the policy syntactically valid?
2. Contract validation — do all input.* references exist in the platform schema?
3. Lint — does the policy follow style and quality rules?

Contract and lint checks are skipped when syntax errors exist. Lint warnings are non-fatal and do not affect the valid/invalid verdict.

complypack test-policy <file> --platform <platform>

Executes the policy's test suite. When --test-data is provided, validates the JSON fixture against the platform CUE schema before running tests. When omitted, test-data validation is skipped and tests run directly.

Flags

Both commands support:

Flag	Description
--platform (required)	Target platform for schema validation
--evaluator	Evaluator ID (auto-detected if omitted)
--schema	Platform schema override (repeatable, e.g., platform=source)
--format / -f	Output format: human (default), text, json

test-policy additionally supports:

Flag	Description
--test-data	Path to JSON test-data file for CUE schema pre-validation

Output Formats

• human — styled terminal output with Unicode symbols and color (default)
• text — plain bracketed labels ([VALID]/[INVALID], [PASS]/[FAIL]) for CI/grep
• json — structured JSON matching the MCP tool response shape exactly

Architecture

Follows the thin transport layer pattern per AGENTS.md. Both commands wire directly to existing domain functions in internal/evaluator/ and internal/schema/ — no new business logic in the CLI layer.

Files Changed

File	Change
cmd/complypack/cli/validate_policy.go	New — command, runner, 3 output formatters
cmd/complypack/cli/validate_policy_test.go	New — 14 test functions
cmd/complypack/cli/test_policy.go	New — command, runner, CUE validation, 3 output formatters
cmd/complypack/cli/test_policy_test.go	New — 17 test functions
cmd/complypack/cli/root.go	Modified — 2 AddCommand calls added

Verification

• go build ./... — clean
• go test ./cmd/complypack/cli/ — 31 new tests pass
• golangci-lint run ./cmd/complypack/cli/ — 0 issues

[trevor-vaughan]

👀 review looks reasonable, I'm sure the LLMs will find something 😄

[sonupreetam]

Review Council Report

Mode: Code Review (OpenSpec) | Branch: opsx/add-validate-test-policy-cli | Reviewers: 9/9 discovered Divisor agents

CI Gate

• go build ./... — PASS
• go vet ./... — PASS
• go test -race -count=1 ./... — PASS (1 pre-existing failure in internal/registry on main, unrelated to this PR)
• Gaze — not installed

Council Verdict: REQUEST CHANGES

All 9 reviewers returned REQUEST CHANGES. Findings converge on three core themes:

#	Severity	Finding	Reviewers
1	CRITICAL	Business logic in CLI transport layer — violates AGENTS.md "thin transport layers"	9/9
2	HIGH	Exit code 0 on validation/test failure — unusable in CI/CD	5/9
3	HIGH	Evaluator resolution logic duplicated 3x (both new commands + MCP handler)	6/9
4	MEDIUM	Tests violate "transport layer tests only verify wiring" — CUE domain logic tested in CLI test files	3/9
5	MEDIUM	README.md missing documentation for new commands	1/9
6	MEDIUM	Lint error silently discarded without user feedback	2/9
7	MEDIUM	Bare error return from buildSchemaRefs missing wrapping context	1/9
8	MEDIUM	Missing test coverage: invalid --format, test-data schema failure via CLI, shallow JSON assertions	1/9

Plus ~10 LOW findings (see inline comments).

Recommended Approach

1. Extract domain functions first — Create ValidatePolicy(), TestPolicy(), ValidateData(), and Resolve() in domain packages (e.g., internal/evaluator/). Both CLI and MCP handlers call these. This resolves CRITICAL + both HIGHs.
2. Add non-zero exit codes — Return errors when validation/tests fail.
3. Move CUE validation tests to the domain package.
4. Add missing test coverage and update README.

The code quality is strong — good flag design, consistent output formatting, solid test coverage patterns. The issue is structural, not qualitative.

---

Review generated by the Review Council (9 Divisor agents). Website docs issue filed: unbound-force/website#215.

[sonupreetam]

CRITICAL — Business logic in CLI transport layer (9/9 reviewers)

runValidatePolicy contains the full orchestration pipeline: evaluator resolution (L115-134), syntax validation (L140), CUE schema loading (L151-165), contract checking (L168-178), lint execution (L186), and result aggregation (L196). This is the same workflow that handleValidatePolicy in internal/mcp/tools.go:236-281 implements independently.

AGENTS.md: "No business logic in these layers. When adding a new MCP tool or CLI command, write the logic as an exported function in the appropriate domain package first, then wire it from the transport layer."

Suggested fix: Extract a domain function:

// internal/evaluator/validate.go
func ValidatePolicy(ctx context.Context, content, platform, evalID string, schemaRefs []config.SchemaRef) (*ValidationResult, error)

Both CLI and MCP call this shared function. The CLI command becomes thin wiring: parse flags → call evaluator.ValidatePolicy() → format output.

[yvonnedevlinrh]

Fixed in 114c1de. Extracted evaluator.ValidatePolicy() into internal/evaluator/validate.go as a domain function. runValidatePolicy is now thin transport: read file → resolve evaluator → load schema → call domain function → format output. The MCP handler (handleValidatePolicy) was refactored to use the same domain function.

[sonupreetam]

CRITICAL — Business logic in CLI transport layer (9/9 reviewers)

Same violation as validate_policy.go. runTestPolicy contains CUE test-data validation (L130-168), evaluator resolution (L171-190), and test execution orchestration (L198-213) — all duplicated from handleTestPolicy in internal/mcp/tools.go:284-326.

Suggested fix: Extract a domain function:

// internal/evaluator/test.go
func TestPolicy(ctx context.Context, content, platform, evalID string, schemaRefs []config.SchemaRef, testData map[string]interface{}) (*TestPolicyResult, error)

[yvonnedevlinrh]

Fixed in 114c1de. Extracted evaluator.TestPolicy() into internal/evaluator/test.go as a domain function. runTestPolicy is now thin transport wiring. Both CLI and MCP handlers call the same domain function.

[sonupreetam]

CRITICAL — Duplicated domain logic (9/9 reviewers)

validateTestData() and collectCUEValidationErrors() (L218-252) are near-identical copies of validateTestDataAgainstSchema() and collectCUEErrors() in internal/mcp/tools.go:21-57. Same CUE context creation, unification, concrete validation, and error extraction logic.

Suggested fix: Extract to a shared domain function:

// internal/schema/validate.go
func ValidateData(data map[string]interface{}, cueSchema cue.Value) []string

Both CLI and MCP call this single implementation.

[yvonnedevlinrh]

Fixed in 114c1de. Extracted schema.ValidateData() and collectCUEErrors() into internal/schema/schema.go. Removed the duplicated validateTestData()/collectCUEValidationErrors() from the CLI and validateTestDataAgainstSchema()/collectCUEErrors() from MCP. Both transport layers now call schema.ValidateData().

[sonupreetam]

HIGH — Evaluator resolution duplicated 3x (6/9 reviewers)

This evaluator resolution block (check if evalID set → lookup; else auto-detect → error if 0 or >1) is copy-pasted identically in:

• validate_policy.go:115-134 (here)
• test_policy.go:171-190
• internal/mcp/tools.go:216-233 (resolveEvaluator())

Also note L133: eval, _ = evalRegistry.Get(ids[0]) discards the error. While logically safe, a defensive check would prevent nil-dereference panics if the registry implementation changes.

Suggested fix: Extract to evaluator.Resolve(registry, evalID) (Evaluator, error) in internal/evaluator/. All three call sites use it.

[yvonnedevlinrh]

Fixed in 114c1de. Added Resolve(id string) (Evaluator, error) to evaluator.Registry in internal/evaluator/registry.go. Handles both explicit lookup and auto-selection when a single evaluator is registered. All three call sites (validate-policy CLI, test-policy CLI, MCP) now delegate to this method.

[sonupreetam]

MEDIUM — Bare error return missing context (Adversary)

When buildSchemaRefs returns an error, it's returned bare without wrapping. Every other error return in this function uses fmt.Errorf("...: %w", err) consistently.

Suggested fix:

return fmt.Errorf("building schema refs: %w", err)

Same applies to test_policy.go:142.

[yvonnedevlinrh]

Addressed in 114c1de. The call sites in both validate_policy.go and test_policy.go wrap the error with fmt.Errorf("building schema refs: %w", err) to provide context about which operation failed.

[sonupreetam]

LOW — testResultDetail duplicates evaluator.TestResults (Architect, Guard, Envoy)

This struct has identical fields to evaluator.TestResults (Total, Passed, Failed, Errors). The CLI copies fields from evaluator.TestResults at L204-212. The intermediate type adds no value — use *evaluator.TestResults directly in testPolicyResult.

Also: the JSON tags on this struct (L40-45) are misleading since the struct is never directly marshaled — writeTestPolicyJSON manually builds a map[string]interface{} instead.

[yvonnedevlinrh]

Fixed in 114c1de. Removed testResultDetail entirely. The evaluator.TestPolicyResult domain type (which embeds *evaluator.TestResults) is now used directly by the CLI formatters. No presentation-layer duplication.

[sonupreetam]

LOW — --test-data flag help could mention skip behavior (SRE)

The flag description says "Path to JSON test-data file for CUE schema pre-validation" but doesn't mention that tests are skipped when validation fails. This is significant for CI users reading --help.

Suggested fix:

"Path to JSON test-data file; validated against CUE schema before tests run (tests skipped on failure)"

[yvonnedevlinrh]

Fixed in 114c1de. Updated help text to: "Path to JSON test-data file for CUE schema pre-validation (omit to skip)".

[sonupreetam]

LOW — buildSchemaRefs tests belong in flags_test.go (Scribe)

TestBuildSchemaRefs_WithFlags and TestBuildSchemaRefs_WithoutFlags test a function defined in flags.go but live in validate_policy_test.go. Move them to flags_test.go for co-location and discoverability.

[sonupreetam]

LOW — Shared test fixtures belong in a dedicated file (Tester, Herald, Envoy)

regoValidPolicyCLI, regoSyntaxErrorPolicyCLI, regoContractViolationPolicyCLI, and writePolicyFile are defined here but used by both validate_policy_test.go and test_policy_test.go. Move them to helpers_test.go (which already exists in this package) for discoverability.

[yvonnedevlinrh]

Fixed in 114c1de. Moved regoValidPolicyCLI, regoSyntaxErrorPolicyCLI, regoContractViolationPolicyCLI constants and the writePolicyFile helper from validate_policy_test.go to helpers_test.go. Both test files now use the shared fixtures.

[sonupreetam]

MEDIUM — Missing test coverage (Tester)

Three gaps in the test suite:

1. No test for invalid --format value: The default branch in writeTestPolicyOutput (L264) and writeValidatePolicyOutput (validate_policy.go:211) are untested.

2. No end-to-end test for --test-data schema validation failure via CLI: TestValidateTestData_Invalid tests the function directly but no CLI test exercises the path where invalid test-data causes testDataValid: false, testsExecuted: false in JSON output.

3. Shallow assertions here: This test asserts parsed["results"] is NotNil (L407) but doesn't check actual counts. Add:

results := parsed["results"].(map[string]interface{})
assert.Equal(t, float64(0), results["total"])
assert.Equal(t, float64(0), results["passed"])
assert.Equal(t, float64(0), results["failed"])

[yvonnedevlinrh]

Fixed in 114c1de. Added:

• TestValidatePolicyEndToEnd_InvalidFormat — verifies error on --format invalid-fmt
• TestTestPolicyEndToEnd_InvalidFormat — same for test-policy
• TestTestPolicyEndToEnd_InvalidTestData — E2E with invalid CUE data (containers: "not-a-list"), asserts errTestsFailed, testDataValid=false, testsExecuted=false, and non-empty testDataErrors in JSON output

Duplicate review — GitHub processed the initial request despite returning HTTP 504. Dismissing this copy; see review #4914720100 for the canonical version.

[sonupreetam]

Review Council — PR #227

Mode: Code Review (OpenSpec)
Branch: opsx/add-validate-test-policy-cli → main
CI: go build, go vet, go test -race — all pass

---

Council Verdicts

Reviewer	Verdict
Adversary (security)	APPROVE
Architect (structure)	APPROVE
Guard (intent/scope)	APPROVE
Tester (coverage)	REQUEST CHANGES
SRE (operations)	APPROVE
Curator (documentation)	REQUEST CHANGES

Architect Alignment Score: 9/10 — exemplary domain extraction, thin transport layers, clean CLI/MCP parity.

---

Findings Requiring Action

All findings below are introduced by this PR (not pre-existing).

CRITICAL — Missing domain-level unit tests for evaluator.ValidatePolicy() and evaluator.TestPolicy()

internal/evaluator/validate.go and internal/evaluator/test.go are new exported domain functions with non-trivial logic (pipeline orchestration with short-circuiting, error normalization, nil-slice initialization). Per AGENTS.md: "Domain package tests cover logic and edge cases." Neither function has any unit test in internal/evaluator/. The CLI end-to-end tests exercise them indirectly, but that violates the architectural testing split.

Recommendation: Add internal/evaluator/validate_test.go and internal/evaluator/test_test.go with table-driven tests using mockEvaluator (already exists in registry_test.go). Cover at minimum:

• ValidatePolicy: valid policy, syntax errors (contract/lint skipped), contract violations, lint warnings, contract-check infrastructure error
• TestPolicy: tests execute normally, test-data errors cause short-circuit, evaluator.Test returns error, nil Errors slice normalization

HIGH — No tests for evaluator.Registry.Resolve()

internal/evaluator/registry.go:53-68 — Resolve() is new in this PR and has 3 distinct code paths (explicit ID lookup, auto-select with one evaluator, ambiguous error with multiple). No dedicated tests exist.

Recommendation: Add TestResolve in registry_test.go covering all three paths plus the zero-evaluators case.

MEDIUM — README missing CLI documentation for new commands

The README CLI Usage section documents every other command but omits validate-policy and test-policy. Website docs issue #215 is already filed.

Recommendation: Add subsections under CLI Usage for both commands, following the existing pattern.

---

Non-blocking Observations (LOW)

• Minor DRY duplication in domain-to-map conversion between CLI and MCP layers — acceptable as transport-layer serialization (Architect)
• Exit code behavior (0 = valid/pass, 1 = invalid/fail) not documented in --help text (SRE)
• lintError key presence varies in JSON output — consider always including it as null for schema stability (SRE)
• hasFailed and convertValidatePolicyResult are untested pure functions — good candidates for small table-driven tests (Tester)
• Duplicate Rego fixture strings between CLI and MCP test files (Tester)

---

What Looks Good

• Domain extraction is clean — ValidatePolicy, TestPolicy, ValidateData live in domain packages, both CLI and MCP delegate to them
• Thin transport layers — CLI commands parse flags, read files, call domain functions, format output; no business logic leaks
• Three output formats (human/text/json) with auto-detection, consistent with existing commands
• Non-zero exit codes on validation/test failure — CI-friendly
• No new dependencies, no secrets, no security concerns
• All error paths properly wrapped with context
• Defensive nil-slice initialization prevents null in JSON output

Reviewed by: divisor-adversary, divisor-architect, divisor-guard, divisor-testing, divisor-sre, divisor-curator