feat(#3345): improve error messages for Rego compilation failures

[fullsend-ai-coder]

Intercept rego_type_error and rego_parse_error from OPA/conftest compilation and wrap them with a user-friendly message that includes the bundled OPA version and remediation guidance.

Changes:

• Add internal/evaluator/rego_errors.go with isRegoCompilationError,
  bundledOPAVersion, and wrapRegoError helper functions
• Apply wrapRegoError in conftest_evaluator.go error paths from
  TestRunner.Run() and LoadWithData()
• Apply wrapRegoError in opa_evaluator.go error path from
  LoadWithData()
• Add unit tests for all new functions covering rego and non-rego
  error cases, OPA version lookup, and error preservation

The original low-level error is preserved via fmt.Errorf %w wrapping so it remains available for debugging.

---

Closes #3345

Post-script verification

• Branch is not main/master (agent/3345-rego-error-messages)
• Secret scan passed (gitleaks — d134e495b18cf8493cfea8791e310b58f0320797..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:53 AM UTC · Completed 8:59 AM UTC
Commit: 47d3320 · View workflow run →

[fullsend-ai-review]

Review — comment

PR: feat(#3345): improve error messages for Rego compilation failures
Scope: Adds wrapRegoError helper to intercept OPA/Rego compilation errors and wrap them with user-friendly messages including the bundled OPA version and remediation guidance. Extracts disallowedBuiltins to a shared package-level variable.

The change is well-scoped to issue #3345 and addresses all four acceptance criteria. The architectural placement in internal/evaluator/rego_errors.go is appropriate, the error chain is properly preserved via %w, and the test coverage is solid for the new helper functions. Two medium-severity findings are worth noting but do not block merge.

---

Findings

1. Misleading error message for non-builtin compilation errors — medium

File: internal/evaluator/rego_errors.go

isRegoCompilationError triggers on rego_type_error, rego_parse_error, and rego_compile_error. However, wrapRegoError always produces the message:

"the policy references Rego built-in functions not available in this version of Conforma CLI"

This message is only accurate for the subset of rego_type_error involving undefined built-in functions. For a rego_parse_error (syntax error in Rego code), rego_compile_error (e.g., duplicate rule definitions), or a rego_type_error about a type mismatch (not an undefined function), the message is misleading — users could be directed to upgrade the CLI when the actual problem is a policy syntax error.

Remediation: Either (a) narrow the detection to also check for "undefined function" in the message, or (b) make the wrapper message more generic: "policy compilation error: the policy failed to compile with this version of Conforma CLI" and present the built-in function restriction as one of several possible causes rather than the asserted cause.

2. Mutable package-level disallowedBuiltins slice — medium

File: internal/evaluator/rego_errors.go, line 32

The security-critical list of disallowed OPA built-in functions was previously an inline literal inside strictCapabilities(), re-created on every call and therefore immutable by construction. The PR extracts it to a package-level var disallowedBuiltins = []string{...}. Go slices are mutable reference types — any code within package evaluator can modify the underlying array (e.g., disallowedBuiltins[0] = ""), which would silently change the security policy for subsequent calls to strictCapabilities.

The practical risk is limited: the variable is unexported, the package is under internal/, and sets.NewString(disallowedBuiltins...) copies the values. However, this weakens the defense-in-depth posture compared to the inline literal.

Remediation: Consider using a function that returns a fresh slice each time:

func getDisallowedBuiltins() []string {
    return []string{"opa.runtime", "http.send", "net.lookup_ip_addr"}
}

Then use sets.NewString(getDisallowedBuiltins()...) in strictCapabilities and strings.Join(getDisallowedBuiltins(), ", ") in wrapRegoError.

3. Inconsistent error wrapping across call sites — low

File: internal/evaluator/opa_evaluator.go, line 97

In opa_evaluator.go, the error is first wrapped with fmt.Errorf("load: %w", err) and then passed to wrapRegoError. In conftest_evaluator.go, the two error paths pass the raw error directly. This means the "Details:" section shows load: loading policies: ... from the OPA evaluator path but just loading policies: ... from the conftest path for the same underlying error. For non-rego errors, the wrapRegoError passthrough preserves the load: prefix, which is fine — but for rego errors, the double-wrapping creates inconsistent user-facing output.

4. Snapshot hardcodes OPA version v1.15.2 — low

File: features/__snapshots__/validate_image.snap

The acceptance test snapshot hardcodes v1.15.2 in two places within the error message. The snapshot framework supports ${EC_VERSION} substitutions via the vars map, but no ${OPA_VERSION} variable exists. Whenever the OPA dependency is bumped in go.mod, this snapshot will fail and require regeneration with UPDATE_SNAPS=true make acceptance. This is a maintenance burden for routine dependency updates.

---

Verdict: The change is a clear UX improvement that addresses the linked issue. The medium-severity findings identify areas where the error message could mislead users (Finding 1) and where defense-in-depth is marginally reduced (Finding 2). Neither blocks merge, but addressing Finding 1 before merge would improve the feature's accuracy for edge cases.

Previous run

Review — approve

PR: feat(#3345): improve error messages for Rego compilation failures
Author: fullsend-ai-coder[bot]

Summary

This PR adds user-friendly error wrapping for OPA/Rego compilation failures, addressing issue #3345. When a policy fails to compile due to rego_type_error, rego_parse_error, or rego_compile_error, the raw compiler output is now wrapped with an actionable message that includes the bundled OPA version and remediation steps. The original error is preserved via %w wrapping for debugging.

Changes reviewed

File	Assessment
internal/evaluator/rego_errors.go	✅ New file — clean implementation with isRegoCompilationError, bundledOPAVersion, and wrapRegoError helpers. Nil handling, error chain preservation via %w, and graceful "unknown" fallback for build info are all correct.
internal/evaluator/rego_errors_test.go	✅ Thorough unit tests covering all code paths: nil errors, each rego error type, non-rego passthrough, version lookup with mocked readBuildInfo, unknown version message adjustments, and errors.Is chain preservation. Build tag //go:build unit matches project conventions.
internal/evaluator/conftest_evaluator.go	✅ Two wrapRegoError calls added at the correct error paths in conftestRunner.Run (after TestRunner.Run and after LoadWithData). The disallowedBuiltins extraction from the inline set to the shared variable in rego_errors.go is a clean DRY improvement — the comment cross-references the source of truth.
internal/evaluator/opa_evaluator.go	✅ Single wrapRegoError call wraps the LoadWithData error. The existing fmt.Errorf("load: %w", err) prefix is preserved inside the wrapped message's Details section, which is correct.
features/__snapshots__/validate_image.snap	✅ Snapshot updated to reflect the new error format. The v1.15.2 version matches go.mod.

Correctness

• Error chain integrity: All three wrapping sites use named returns or direct return, and wrapRegoError uses fmt.Errorf with %w so errors.Is/errors.As continue to work through the chain. Verified no double-wrapping occurs — each error path has exactly one wrapRegoError call.
• Non-rego passthrough: wrapRegoError returns the original error unchanged for non-matching errors, so unrelated error paths are unaffected.
• disallowedBuiltins consistency: The shared var slice is used by both strictCapabilities (capability filtering) and wrapRegoError (error message), ensuring the lists stay in sync.
• No missing call sites: Verified all conftest.LoadWithData calls in both evaluators and the TestRunner.Run call in the conftest evaluator are covered. The conftest.LoadCapabilities and runtime evaluation paths correctly do not wrap, since they don't produce rego compilation errors.

Security

• No new trust boundaries, external inputs, or privilege changes.
• The disallowedBuiltins refactor preserves the existing security restrictions (disabling opa.runtime, http.send, net.lookup_ip_addr) without modification.
• PR body and issue text contain no injection patterns.

Findings

[low] Error message wording is overly specific for parse/compile errors — rego_errors.go:80

The primary message says "the policy references Rego built-in functions not available in this version", which accurately describes rego_type_error: undefined function cases but is misleading for rego_parse_error (syntax issues) or rego_compile_error (general compilation failures) where the root cause may not involve built-in functions at all. The "less common causes" section does mention syntax, but it's framed as secondary.

A more accurate primary message might be: "the policy could not be compiled with the OPA version bundled in this version of Conforma CLI" — generic enough for all three error types while still directing the user toward version compatibility.

This is non-blocking because the original error is always preserved in the Details section, and users can diagnose the actual cause from there. Consider as a follow-up improvement.

Previous run (2)

Review — PR #3381

feat(#3345): improve error messages for Rego compilation failures

Summary

This PR implements the error-wrapping feature requested in #3345. It intercepts rego_type_error, rego_parse_error, and rego_compile_error from OPA/conftest compilation and wraps them with a user-friendly message including the bundled OPA version, a list of restricted built-in functions, and remediation guidance. The original error is preserved via %w for debugging. All four acceptance criteria from the issue are met.

The implementation is clean: a new rego_errors.go file isolates the error-wrapping logic, the disallowedBuiltins list is extracted as a shared single-source-of-truth, and the existing readBuildInfo override pattern (from internal/version/version.go) is reused. Tests are thorough — covering all three error patterns, nil/non-rego passthrough, unknown-version graceful degradation, and error chain preservation.

Findings

1. disallowedBuiltins as mutable package-level var weakens defense-in-depth — medium / security

File: internal/evaluator/rego_errors.go, line 32

The security-critical disallowedBuiltins list — which controls which OPA built-in functions are blocked in the sandbox (opa.runtime, http.send, net.lookup_ip_addr) — was previously a local variable inside strictCapabilities(), inaccessible to other code. This PR promotes it to a mutable package-level var ([]string slice).

As a package-level var, any code in the evaluator package can mutate or reassign it at runtime. If the slice were emptied (e.g., disallowedBuiltins = disallowedBuiltins[:0]), strictCapabilities() would call sets.NewString() with no arguments, producing an empty set — meaning no builtins would be filtered, re-enabling opa.runtime (environment variable access) and http.send/net.lookup_ip_addr (network access). This is a fail-open condition.

The PR itself establishes the readBuildInfo var as test-injectable in this same file, creating a convention that encourages mutation of package-level vars.

Remediation: Replace the mutable var with a function returning a fresh slice copy:

func disallowedBuiltinNames() []string {
    return []string{"opa.runtime", "http.send", "net.lookup_ip_addr"}
}

Update strictCapabilities to sets.NewString(disallowedBuiltinNames()...) and wrapRegoError to strings.Join(disallowedBuiltinNames(), ", "). This prevents any caller from mutating the canonical list while preserving the single-source-of-truth benefit.

2. Multi-line original error indentation is inconsistent — low / error-handling

File: internal/evaluator/rego_errors.go, line 95

The format string "Original error:\n %w" indents only the first line of the original error with two spaces. For multi-line errors (the common case — multiple rego_type_error lines), subsequent lines appear unindented, creating visual inconsistency in the output. The snapshot test confirms this is the current behavior.

Remediation: Consider splitting the original error by newlines and indenting each line, or document this as intentional.

Verdict

comment-only — The medium-severity security finding about the mutable disallowedBuiltins var is worth noting but does not block the PR. The current code is functionally correct and the risk requires an intentional modification within the same package. The change is well-scoped, well-tested, and achieves its stated goal.

Previous run (3)

Review — approve

PR: #3381 — feat(#3345): improve error messages for Rego compilation failures

Summary

This PR intercepts rego_type_error, rego_parse_error, and rego_compile_error from OPA/conftest compilation and wraps them with user-friendly messages that include the bundled OPA version and remediation guidance. The original error is preserved via %w wrapping.

Changes reviewed

File	Assessment
internal/evaluator/rego_errors.go	New file — clean implementation with isRegoCompilationError, bundledOPAVersion, and wrapRegoError helpers. The disallowedBuiltins variable provides a single source of truth shared with strictCapabilities.
internal/evaluator/rego_errors_test.go	Comprehensive unit tests covering nil/empty errors, all three rego error types, multi-error messages, build info edge cases, and error wrapping preservation via errors.Is.
internal/evaluator/conftest_evaluator.go	wrapRegoError applied at both error paths in conftestRunner.Run (after TestRunner.Run() and LoadWithData()). The disallowedBuiltins list is correctly extracted from inline literals to the shared variable.
internal/evaluator/opa_evaluator.go	wrapRegoError applied around the LoadWithData error in compileEngine.
features/__snapshots__/validate_image.snap	Snapshot updated to reflect the new wrapped error format.

Findings

No blocking findings. The implementation directly satisfies all four acceptance criteria from issue #3345:

1. ✅ Produces helpful, actionable error messages for Rego compilation failures
2. ✅ Includes the bundled OPA version number (v1.15.2)
3. ✅ Suggests potential solutions (upgrade CLI, check restricted builtins, adjust policy syntax)
4. ✅ Preserves the original compiler error for debugging (via fmt.Errorf %w)

Notes

• Error detection approach: String-matching on err.Error() for rego error patterns is pragmatic — OPA doesn't expose typed errors for these compilation failures, so substring matching is the correct approach here.
• Single source of truth: Extracting disallowedBuiltins into a shared package-level variable is a good refactoring. The sets.NewString(disallowedBuiltins...) call in strictCapabilities creates a copy via spread, so the original slice is not mutated.
• Test design: The readBuildInfo variable injection for testing bundledOPAVersion follows standard Go patterns and avoids the need for integration tests against actual build metadata.
• Snapshot maintenance: The v1.15.2 version in the acceptance test snapshot will need updating when the OPA dependency is bumped. This is standard snapshot test behavior, documented in AGENTS.md (UPDATE_SNAPS=true make acceptance).

Previous run (4)

Review — PR #3381

Verdict: request-changes

Overview

This PR adds user-friendly error wrapping for OPA/Rego compilation failures, fulfilling all four acceptance criteria from issue #3345. The implementation is clean and well-tested:

• New rego_errors.go with isRegoCompilationError, bundledOPAVersion, and wrapRegoError helper functions
• Error wrapping applied at three compilation error sites (2 in conftest_evaluator.go, 1 in opa_evaluator.go)
• Original error preserved via fmt.Errorf %w wrapping
• 186 lines of unit tests covering all functions, edge cases, and error chain preservation
• Test injection pattern (var readBuildInfo) matches existing convention in internal/version/version.go:35

The code logic is correct. However, there is one blocking issue.

---

Findings

🔴 HIGH — Acceptance test snapshot not updated

File: features/__snapshots__/validate_image.snap:2166-2170

The "Dropping rego capabilities" acceptance test (features/validate_image.feature:799) uses snapshot matching on stderr. The current snapshot expects the raw OPA error format:

Error: error validating image .../acceptance/ec-happy-day of component Unnamed: load: loading policies: get compiler: 3 errors occurred:
.../main.rego:14: rego_type_error: undefined function opa.runtime
.../main.rego:22: rego_type_error: undefined function http.send
.../main.rego:33: rego_type_error: undefined function net.lookup_ip_addr

After this PR, wrapRegoError intercepts these errors and produces a completely different message starting with policy compilation error: the policy uses Rego features or built-in functions not available.... The snapshot must be updated (e.g., via UPDATE_SNAPS=true make acceptance) for this test to pass.

Remediation: Run the acceptance test with UPDATE_SNAPS=true and commit the updated snapshot file. This is also an opportunity to verify that the improved error message renders well in the full end-to-end context.

---

🟡 LOW — Error message refers to "ec-cli" instead of current project name

File: internal/evaluator/rego_errors.go:72-76

The wrapped error message says "the one bundled in ec-cli" and "Upgrade ec-cli". The project has been renamed — cmd/root/root_cmd.go:69 identifies it as "Conforma CLI". User-facing error messages should use the current name for consistency.

Remediation: Replace ec-cli with conforma (the CLI binary name) or Conforma CLI in the error message template.

---

🟡 LOW — Hardcoded restricted-function list creates maintenance coupling

File: internal/evaluator/rego_errors.go:74

The error message hardcodes "opa.runtime, http.send, net.lookup_ip_addr" as example restricted builtins. The authoritative disallowed list lives in strictCapabilities() at conftest_evaluator.go:1199-1205. If the disallowed set changes, this error message becomes stale. The "e.g." qualifier provides some cover, but could still mislead users.

Remediation (follow-up): Consider extracting the disallowed builtins into a shared package-level variable that both strictCapabilities() and wrapRegoError() reference, or add a code comment cross-referencing the two locations.

---

🟡 LOW — ec test command path not covered

File: cmd/test/test.go

The ec test command calls conftest's runner.TestRunner.Run() directly without going through the wrapped conftestRunner.Run(). Users running ec test with incompatible policies will still see raw rego errors. This is a minor gap since ec test is a conftest-compatibility command, but it could confuse users who see different error quality between ec validate and ec test.

---

🟡 LOW — Only two of five OPA error categories are matched

File: internal/evaluator/rego_errors.go:40

isRegoCompilationError detects rego_type_error and rego_parse_error. OPA also defines rego_compile_error, rego_unsafe_var_error, and rego_recursion_error. While the two covered patterns are the most common for version/capability mismatches, a rego_compile_error from other causes would not get the improved message. Consider adding rego_compile_error at minimum.

---

Labels: PR modifies evaluator error handling for improved UX

Previous run (5)

Review of PR #3381 — Improve error messages for Rego compilation failures

Verdict: request-changes — Two medium-severity findings that affect the PR's intended outcome and CI stability.

Summary

This PR adds a new rego_errors.go file with helper functions (isRegoCompilationError, bundledOPAVersion, wrapRegoError) that intercept rego_type_error and rego_parse_error from OPA/conftest compilation and wrap them with a user-friendly message including the bundled OPA version and remediation guidance. The wrapping is applied in three error paths: two in conftest_evaluator.go and one in opa_evaluator.go. Unit tests are comprehensive, and the %w wrapping correctly preserves the error chain.

The implementation is clean, well-structured, and addresses the core UX need described in #3345. However, two issues need resolution before merge.

Findings

1. Acceptance test snapshot not updated (medium — correctness)

File: features/__snapshots__/validate_image.snap (lines 2166–2170)

The "Dropping rego capabilities" acceptance test scenario (features/validate_image.feature:799) validates that ec validate image fails with raw rego_type_error messages when policies use sandboxed functions (opa.runtime, http.send, net.lookup_ip_addr). The snapshot currently expects:

Error: error validating image ... : load: loading policies: get compiler: 3 errors occurred:
.../main.rego:14: rego_type_error: undefined function opa.runtime
...

After this PR, wrapRegoError intercepts this error in conftestRunner.Run() (line 224), wrapping it with the "policy compilation error" message. The snapshot is not updated to reflect this change, so the acceptance test will fail in CI.

Remediation: Run acceptance tests with UPDATE_SNAPS=true make acceptance to regenerate the snapshot, or manually update the snapshot entry for [TestFeatures/Dropping rego capabilities:stderr - 1] to match the new wrapped error format.

2. Misleading error message for intentional capability restrictions (medium — correctness)

File: internal/evaluator/rego_errors.go (lines 62–73)

The wrapRegoError function attributes all rego_type_error occurrences to OPA version incompatibility:

"the policy uses Rego features or built-in functions not supported by the bundled OPA version (v1.15.2). This is likely caused by a version mismatch..."

However, rego_type_error also occurs when the strict capabilities sandbox intentionally strips dangerous built-ins (opa.runtime, http.send, net.lookup_ip_addr) — see strictCapabilities() in conftest_evaluator.go:1183. In this case the error is not a version mismatch; it's an intentional security restriction. A user whose policy calls opa.runtime() would be told to "upgrade ec-cli" when the actual issue is that the function is disallowed by design.

This is the exact error path validated by the "Dropping rego capabilities" acceptance test, so it's a realistic user scenario.

Remediation: Consider distinguishing between capability-restricted functions and genuinely unsupported features. For example, check whether the error mentions functions in the disallowed set from strictCapabilities() and produce a different message (e.g., "this function is restricted for security reasons"), or narrow the wrapping to only apply in opa_evaluator.go / error paths where capabilities are not being artificially restricted. Alternatively, adjust the message to mention capability restrictions as another possible cause.

What looks good

• Error chain preservation: Using fmt.Errorf with %w correctly preserves the original error for errors.Is/errors.As unwrapping.
• Test coverage: Unit tests cover nil errors, both error patterns, multi-error messages, non-matching errors, and build info edge cases. The readBuildInfo variable pattern for test injection is clean.
• Defensive nil handling: isRegoCompilationError(nil) correctly returns false, making wrapRegoError(nil) safe.
• Scope: The change is well-contained — only error wrapping is added, no behavioral changes to the evaluation pipeline itself.

---

Labels: PR modifies evaluator error handling and touches UX-facing error messages

[st3penta]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 12:00 PM UTC · Completed 12:12 PM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:12 PM UTC · Completed 12:25 PM UTC
Commit: 7c8ccca · View workflow run →

[st3penta]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 9:56 AM UTC · Completed 10:15 AM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:16 AM UTC · Completed 10:20 AM UTC
Commit: 7c8ccca · View workflow run →

Superseded by updated review

[codecov]

Codecov Report

❌ Patch coverage is 94.44444% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/evaluator/conftest_evaluator.go	66.66%	1 Missing ⚠️
internal/evaluator/opa_evaluator.go	0.00%	1 Missing ⚠️

Flag	Coverage Δ
acceptance	53.62% <86.11%> (+0.19%)	⬆️
generative	16.59% <0.00%> (-0.20%)	⬇️
integration	27.35% <2.77%> (-0.31%)	⬇️
unit	68.84% <91.66%> (-0.29%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
internal/evaluator/rego_errors.go	100.00% <100.00%> (ø)
internal/evaluator/conftest_evaluator.go	88.46% <66.66%> (-0.20%)	⬇️
internal/evaluator/opa_evaluator.go	88.08% <0.00%> (ø)

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[st3penta]

/fs-fix the outstanding items from my previous review:

1. When bundledOPAVersion() returns "unknown", the output reads "compatible with OPA unknown" and "bundled with Conforma CLI (unknown)". The version-specific lines should be conditionally adjusted or omitted when the version can't be determined.

2. TestWrapRegoError subtests all stub readBuildInfo to return "v1.15.2". No coverage for the "unknown" version path (build info unavailable or OPA not in deps).

3. The "handles multi-error with rego_type_error" subtest is missing the assert.ErrorIs(t, wrapped, origErr) assertion that the other wrapping subtests include.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 10:30 AM UTC · Completed 10:48 AM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:48 AM UTC · Completed 11:00 AM UTC
Commit: 7c8ccca · View workflow run →

[st3penta]

/fs-fix address my comment above

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 12:16 PM UTC · Completed 12:40 PM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:41 PM UTC · Completed 12:48 PM UTC
Commit: 7c8ccca · View workflow run →

[st3penta]

/fs-fix squash all commits into one. Also add a single no-op whitespace change (a space or newline) anywhere in the code, just to produce a non-empty file diff. No behavioral, logic, or message changes

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 2:03 PM UTC · Completed 2:14 PM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:15 PM UTC · Completed 2:30 PM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 2:36 PM UTC · Completed 2:45 PM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-retro]

PR #3381 added user-friendly error messages for Rego compilation failures in conforma/cli. The code agent created a well-structured implementation but used the stale project name 'ec-cli' (sourced from issue #3345's description) instead of 'Conforma CLI'. The PR required 5 fix iterations over 2 days before merge — at least 2 were avoidable with better agent behavior. The review agent provided genuine value (catching the capability-restriction correctness issue early and the naming problem) but repeated the mutable-state finding 3 times across reviews on unchanged code. The human reviewer uniquely caught edge cases (unknown version handling, error message information hierarchy) that agents missed across multiple runs.

Proposals filed

• Add product naming guidance to conforma/cli AGENTS.md to prevent stale name usage in generated code (in conforma/cli)
• Evidence for #1013: review agent repeated mutable-state finding 3 times on PR #3381 with unchanged code (in fullsend-ai/fullsend)
• Review agent should detect sentinel value interpolation in user-facing format strings (in fullsend-ai/fullsend)