Always exit 0 on a completed run; verdict lives in summary.verdict

[aaronbrethorst]

Summary

• A FAIL verdict no longer produces process exit code 1 — completed runs now always exit 0. The PASS/FAIL signal lives in summary.verdict (JSON) and the result-sink row's result_data. Exit 2 is reserved for "the validator could not run" (config/usage error).
• Why: the validator runs as a Render cron job; the caller reads the verdict from the sink. Exiting non-zero on a successful evaluation that surfaces real OBA bugs caused Render to mark the cron "failed", which is misleading (the validator did its job — it found bugs and reported them).
• JSON schema summary.exitCode tightens from enum: [0, 1] to const: 0 with a description; the field is kept for schema stability.

Test plan

• go test ./... green
• go vet ./... clean
• Local run against the public San Diego config (which produces a FAIL verdict via DNS errors): text mode prints FAIL (… 9 failed …) and the process exits 0. JSON mode emits summary.verdict=FAIL, summary.exitCode=0, process exits 0.
• New TestRunFailVerdictReturnsZero pins the policy end-to-end through cmd/oba-validator.run() — unit-level coverage on Report.ExitCode() alone wouldn't catch a regression in main.go.
• Existing exit-2 tests (TestUsageWhenNoArgs, TestRunJSONConfigErrorEmitsErrorJSON, …) still pin the config-error upper boundary.

Review notes

Historical design docs under docs/superpowers/specs/ (2026-05-24-oba-validator-design.md, 2026-05-25-render-deployment-design.md, 2026-05-25-json-ui-output-design.md, 2026-05-25-result-sink-design.md) still describe the old "0/1 on failure" contract. Per the project convention (CLAUDE.md is the living source of truth), historical specs are left as a record of decisions made at the time rather than retroactively rewritten. Happy to update them if you'd rather have specs reflect current behavior.

Backwards-compat note: callers (notably obacloud's reader) that parse summary.verdict rather than the process exit code or the exitCode field continue to work unchanged. Any consumer that was branching on exit == 1 to mean "FAIL" will need to look at summary.verdict (or the sink row's result_data) instead.

Summary by CodeRabbit

• Documentation

  • Clarified validator exit code behavior: exit code 0 now indicates successful report generation (PASS or FAIL), exit code 2 indicates configuration/usage errors.

• Bug Fixes

  • FAIL verdicts are now conveyed through the JSON report's verdict field rather than the process exit code.

• Tests

  • Added and updated test coverage for exit code semantics.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: efb2a577-40d8-4a78-b692-8d2ff896a22f

📥 Commits

Reviewing files that changed from the base of the PR and between 95237f7 and 20da9f8.

📒 Files selected for processing (8)

• CLAUDE.md
• README.md
• cmd/oba-validator/main_test.go
• report/document.go
• report/document_test.go
• schema/oba-validator-report.schema.json
• validator/result.go
• validator/result_test.go

---

📝 Walkthrough

Walkthrough

This PR redefines the validator's exit-code semantics: exit code 0 now always indicates a report was produced (regardless of PASS or FAIL verdict), while exit code 2 indicates a configuration/usage error. Verdicts are conveyed only via JSON summary.verdict, not process exit codes. Changes flow from core logic through schema, tests, and documentation.

Changes

Exit Code Semantics Overhaul

Layer / File(s)	Summary
Exit code logic always returns zero validator/result.go, validator/result_test.go	Report.ExitCode() is updated to always return 0 after a report is produced, removing the prior conditional logic that returned 1 for FAIL status. Unit tests are refactored: TestReportWorst now validates verdict status only, and a new TestReportExitCodeAlwaysZero test explicitly confirms exit code is 0 across all status values (Pass, Warn, Skip, Fail).
Report schema and document constraint schema/oba-validator-report.schema.json, report/document.go, report/document_test.go	The JSON report schema constrains summary.exitCode from enum [0, 1] to a constant value of 0. The Summary struct documentation is expanded to clarify that ExitCode is reserved for "validator could not run" scenarios while Verdict carries the PASS/FAIL status. An existing test assertion is updated to expect ExitCode == 0 when Verdict == "FAIL".
Integration validation of exit code behavior cmd/oba-validator/main_test.go	A new test TestRunFailVerdictReturnsZero verifies end-to-end behavior: a FAIL verdict from run() produces process exit code 0 and JSON summary.exitCode of 0 using stubbed OBA/FI servers. An existing sink-error test is updated to strictly assert exit code is 0 instead of merely disallowing exit code 2.
Developer and user documentation CLAUDE.md, README.md	Developer documentation in CLAUDE.md clarifies the new policy: exit code 0 means a report was produced, exit code 2 means config error, and verdicts are conveyed only in the JSON. README.md updates user-facing guidance and Render-job-context instructions to state that FAIL verdicts are not conveyed via exit status but must be read from summary.verdict in the JSON report or result sink.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• OneBusAway/server-validator#2: Introduced the --json output mode and summary.exitCode/verdict fields in the report schema and document structure that this PR now refines by decoupling exit codes from FAIL verdicts.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly summarizes the main change: validator now exits with code 0 on completed runs, with verdict reported in summary.verdict instead of exit codes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch deploy

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}