fix: fail closed on unavailable llm review

[autogame-17]

Summary

Harden the llmReview trust boundary so malformed, empty, partial, timed-out, or failed review execution can never approve a solidification candidate. Preserve valid approval/rejection compatibility fields while adding typed recoverable status, retry metadata, and attempt traces.

What changed

• Added strict validation for the complete review response contract before approval.
• Classified unavailable outcomes as malformed_output, empty_output, partial_response, runner_error, or timeout and made each fail closed with approved: false.
• Added one bounded retry by default, deterministic executor injection for tests, and per-attempt trace metadata.
• Added regression coverage for malformed, empty, runner failure, timeout, partial, retry recovery, explicit rejection, idempotency, disabled mode, and a real subprocess trace with temporary-directory cleanup.

How to test

1. node --test test/llmReview.test.js
2. PATH=/opt/homebrew/opt/node@22/bin:$PATH npm test (runs all 3,299 project tests locally; 3,259 passed and 40 unrelated black-box CLI/proxy timing tests hit their fixed local 10–20 second deadlines; llmReview passed inside the full run; hosted CI is the clean-environment full-suite authority)
3. node --test test/solidify-helpers.test.js test/solidifyLearning.test.js (68/68 pass)
4. node --check src/gep/llmReview.js && node --check test/llmReview.test.js && git diff --check
5. node index.js --help
6. node scripts/harness-governance-check.js --body <this-pr-body> --changed <changed-files>

Expected: focused and adjacent suites pass; llmReview passes in the full test run; static checks and CLI sanity exit zero; governance gate reports PASSED. Hosted CI must pass the complete suite before merge.

Risk

Medium -- this intentionally changes enabled llmReview infrastructure failures from approval to a recoverable fail-closed rejection. The default placeholder success path, explicit model approval/rejection, disabled-mode null, and legacy compatibility fields remain unchanged.

Harness/evaluator governance

Upstream governance surface: src/gep/llmReview.js review parsing, execution, retry, and approval invariant only.
Downstream EvoX impact: Enabled review failures now return approved: false with typed recoverable metadata; no bridge schema or runtime API changes outside the existing review result object.
Rollout-local scope: Applies only when EVOLVER_LLM_REVIEW=true; disabled mode remains unchanged.
Promotion boundary: Merge makes the fail-closed invariant the default only inside the opt-in llmReview path; no live promotion is performed by this PR.
Evaluator mismatch sets: Observation adds typed failure reasons and attempt traces; action changes only unverifiable failures from approval to rejection; repair adds bounded retry; verification covers malformed, empty, partial, runner error, timeout, success, rejection, idempotency, and real subprocess trace; evidence is the focused/full test output; belief changes from fail-open to fail-closed for unavailable reviews.
Non-regression evidence: Focused test/llmReview.test.js (11/11), adjacent solidify tests (68/68), full Node 22 run (3,299 executed; 3,259 passed; 40 unrelated local black-box timing failures; llmReview passed), syntax/whitespace checks, CLI sanity, before/after failure probe, and harness governance gate. Hosted CI is required before merge.
Fix-severity review: high
Owner approval: GEP llmReview/evaluator owner review required.
Security boundary: No new data, network, host, tool, or secrets access; temporary prompts remain local and are removed after each attempt. Trust is tightened because unverified output cannot authorize a candidate.
Rollback: Revert this commit to restore prior behavior; operationally disable the opt-in path with EVOLVER_LLM_REVIEW=false while investigating.
Live promotion: no
Autonomous evaluator self-editing: no

Self-check

• Tests added or updated to cover the new behavior; focused and adjacent suites pass locally, the full suite executed all 3,299 tests with only unrelated fixed-deadline black-box timing failures, and hosted CI is required before merge.
• No new runtime dependencies added.

Related

N/A

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes behavior on the evolution trust boundary when LLM review is enabled: unverifiable review outcomes flip from fail-open approval to fail-closed rejection, which can block candidates that previously slipped through.

Overview
Hardens the opt-in llmReview gate so infrastructure or bad output can no longer implicitly approve solidification candidates when EVOLVER_LLM_REVIEW=true.

Previously, parse failures, runner errors, and timeouts could still yield approved: true with low confidence. The flow now validates the full JSON contract (approved, bounded confidence, concerns, non-empty summary), retries once by default, and on exhaustion returns approved: false with status: 'unavailable', typed reason (malformed_output, empty_output, partial_response, runner_error, timeout), retryable: true, and per-attempt trace. Valid explicit approve/reject paths keep the legacy fields plus status/reason. Execution is injectable for tests; default subprocess temp dirs use mkdtemp and are removed after each attempt.

Adds test/llmReview.test.js covering fail-closed cases, retry recovery, rejection, idempotency, disabled mode, and subprocess cleanup.

^{Reviewed by Cursor Bugbot for commit b094d5b. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b094d5b. Configure here.}

[cursor]

Temp dir leaks on write failure

Low Severity

mkdtempSync creates evolver-review-* before the try/finally, and writeFileSync also sits outside that block. If the prompt write throws, the new directory is never removed, unlike the execFileSync path which always cleans up in finally.

 

^{Reviewed by Cursor Bugbot for commit b094d5b. Configure here.}

[cursor]

Left a non-blocking comment and did not approve because the present Cursor Bugbot check completed as skipped. Human review is needed; one reviewer was assigned from the changed GEP review path. Cursor Security Agent was not running after the first poll, so that signal was skipped.

  

_{Sent by Cursor Approval Agent: Pull Request Router and Approver}