fix(#1236): show exec error when validation script fails to run

[fullsend-ai-coder]

When a validation script cannot be executed (e.g., ENOENT or EACCES), CombinedOutput() returns empty output with a non-nil error. Previously the failure message used only the (empty) output, producing a blank "Validation failed:" message. Now falls back to the exec error string when output is empty, giving operators the actual diagnostic.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

---

Closes #1236

Post-script verification

• Branch is not main/master (fullsend/code/issue-1236)
• Secret scan passed (gitleaks — 7813c92add42ca3ba723cf83de489fd6a67d442a..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[github-actions]

Site preview

Preview: https://87d5c3a9-site.fullsend-ai.workers.dev

Commit: 724e5e0b1ee5c90bbfb506f1350efd14f629ed43

[fullsend-ai-review]

Review

Findings

Low

• [nil safety] internal/cli/run.go:1021 — validationFailMessage will panic with a nil-pointer dereference if execErr is nil and output is empty/whitespace. The current call site (line 697) is safe because it is guarded by if valErr == nil { break }, but the function itself has no nil guard, making it fragile if reused elsewhere in the package.
  Remediation: Add a nil check: if execErr != nil { return execErr.Error() } return "unknown error"

Previous run

Review

Findings

No findings.

Previous run (2)

Review

Findings

No findings.

Previous run (3)

Review

Findings

No findings.

[ralphbean]

/fs-fix is there a test that covers this new conditional?

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Extracted the empty-output fallback conditional into a testable validationFailMessage helper and added 4 unit tests covering all branches. All tests pass.

Fixed (1):

1. no test covering the empty-output fallback conditional (internal/cli/run.go): Extracted the inline msg-building logic into a validationFailMessage helper function and added four unit tests: output present (uses output), empty output (falls back to exec error), whitespace-only output (falls back to exec error), and output trimming.

Tests: passed

Decision points

• Extracted inline logic into a helper function rather than testing runAgent end-to-end (alternatives: Integration test of runAgent with a missing validation script, Table-driven test with mock exec.Cmd; rationale: runAgent has extensive dependencies (filesystem, subprocesses, harness config) making integration tests impractical. A small pure-function helper is the idiomatic Go approach and matches the existing test patterns in run_test.go.)

_{Updated by fullsend fix agent}

[fullsend-ai-review]

[low] nil safety

validationFailMessage will panic with a nil-pointer dereference if execErr is nil and output is empty/whitespace. The current call site (line 697) is safe because it is guarded by if valErr == nil { break }, but the function itself has no nil guard, making it fragile if reused elsewhere in the package.

Suggested fix: Add a nil check: if execErr != nil { return execErr.Error() } return "unknown error"

[fullsend-ai-retro]

Retro: PR #1238 — show exec error when validation script fails

Timeline: Issue #1236 opened 2026-05-20, code agent produced PR in ~9 min, review agent approved with 1 low nil-safety finding. PR sat for 6 days until a human (ralphbean) noticed missing tests and triggered /fs-fix. Fix agent added 4 tests in ~7 min. PR then waited another 13 days for human approval and merge (2026-06-08). Total: 19 days, 1 rework cycle.

Key observation: The code agent fixed the bug correctly but shipped no tests. A human had to manually request test coverage, causing a full rework cycle (fix agent + re-review). This is the primary efficiency loss. Existing issue #1354 covers the review agent detecting missing tests, but no open issue proposes that the code agent itself should proactively write regression tests for bug fixes.

Skipped proposals (covered by existing issues):

• Review agent re-flagged the same nil-safety finding on June 8 that it first raised on May 20 → covered by Review agent should deduplicate findings across iterations on the same PR #1013 (dedup findings across iterations)
• Low-severity nil-safety finding was never addressed before merge → covered by Auto-file tracking issues for unresolved low-severity review findings #1286 (auto-file tracking issues for unresolved low-severity findings)
• Review agent approval with residual findings → covered by Review agent approval with residual findings drives unnecessary author rework #1368

Proposals filed

• Code agent should include regression tests for bug-fix PRs (in fullsend-ai/fullsend)