Merge pull request #5 from OneBusAway/deploy

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

Author: aaronbrethorst

CLAUDE.md

@@ -23,7 +23,7 @@ go run ./cmd/oba-validator [flags] <config.json | raw-json-string>
 OBA_VALIDATOR_LIVE=1 go test ./validator/ -run TestLiveKingCountyMetro -v
 ```
 
-Exit codes (also returned by `Report.ExitCode()`): `0` = no failures, `1` = ≥1 failure, `2` = config/usage error. Warnings and skips never affect the exit code.
+Exit codes: `0` = the validator produced a report (PASS *or* FAIL verdict); `2` = the validator could not run (config/usage error, or `validator.Run` returned an error). The verdict is deliberately *not* in the exit code — it lives in the JSON report's `summary.verdict` and the result-sink row's `result_data`, so a Render cron that surfaces real server bugs still completes as "succeeded" and the caller learns the verdict from the sink. `Report.ExitCode()` (and `summary.exitCode`) is always `0`.
 
 ## Architecture
 
@@ -49,7 +49,7 @@ Each check is a small struct in its own `check_*.go` file, returns `[]Result`, a
 
 This is the core design discipline. Severity is **evidence-based** — see `docs/superpowers/specs/2026-05-24-oba-validator-design.md`:
 
-- **`Fail`** only when the feed *has* an entity but the API contradicts or is missing it (genuine server breakage). Failures set exit code 1.
+- **`Fail`** only when the feed *has* an entity but the API contradicts or is missing it (genuine server breakage). A `Fail` drives the report's `summary.verdict` to `"FAIL"` but does **not** change the process exit code (see exit-code policy above).
 - **`Warn`** for valid-but-empty / unsamplable / unconfirmed conditions: empty feed, vehicle that moved, or an ID that didn't match on shape alone.
 - **`Skip`** when a prerequisite failed earlier in a dependent chain.
 

README.md

@@ -11,8 +11,10 @@ its REST API against the authoritative static GTFS and GTFS-realtime feeds
 Flags: `--json`, `--sample-size`, `--freshness`, `--timeout`, `--cache-dir`,
 `--no-cache`, `--refresh`.
 
-Exit codes: `0` = no failures, `1` = at least one failure, `2` = config/usage
-error. Warnings and skips do not affect the exit code.
+Exit codes: `0` = the validator produced a report (read `summary.verdict` for
+PASS/FAIL); `2` = the validator could not run (config/usage error). The process
+exit code is intentionally not used to convey a FAIL verdict — that's what the
+JSON report's `summary.verdict` and the result-sink row are for.
 
 ## Config
 
@@ -119,8 +121,10 @@ From Ruby (e.g. an obacloud job), this mirrors the existing pattern:
     start_command = "/app/entrypoint.sh #{encoded}"
 
 Validator flags go after the token (`/app/entrypoint.sh <base64> --json`). The
-job's exit status is the validator's exit code (`0` no failures, `1` ≥1 failure,
-`2` config/usage error), so a failed validation shows as a failed run.
+job's exit status is the validator's exit code — `0` when a report was produced
+(PASS *or* FAIL verdict), `2` only when the validator couldn't run (config/usage
+error). A FAIL verdict therefore shows as a *succeeded* Render run; the caller
+reads the verdict from `summary.verdict` in the JSON report or the sink row.
 
 See `docs/superpowers/specs/2026-05-24-oba-validator-design.md` for the validator
 design and `docs/superpowers/specs/2026-05-25-render-deployment-design.md` for the

cmd/oba-validator/main_test.go

@@ -130,6 +130,38 @@ func TestRunJSONOutputShape(t *testing.T) {
 	}
 }
 
+// A FAIL verdict must come back through run() as process exit code 0; the
+// PASS/FAIL signal lives in summary.verdict, not the exit code. This pins the
+// CLAUDE.md exit-code policy end-to-end (newStubOBA returns an empty agencies
+// list, which makes the endpoints check emit at least one Fail).
+func TestRunFailVerdictReturnsZero(t *testing.T) {
+	t.Setenv("ONEBUSAWAY_API_KEY", "")
+	obaSrv := newStubOBA(t)
+	feedSrv := newStubFeed(t)
+	cfg := `{"obaServerURL":"` + obaSrv.URL + `","apiKey":"k","dataSources":[{"staticGtfsFeedURL":"` + feedSrv.URL + `/gtfs.zip"}]}`
+	var stdout, stderr bytes.Buffer
+	code := run([]string{"oba-validator", "--json", "--no-cache", cfg}, &stdout, &stderr)
+
+	if code != 0 {
+		t.Fatalf("FAIL verdict must produce exit code 0, got %d\nstderr: %s", code, stderr.String())
+	}
+	var doc struct {
+		Summary struct {
+			Verdict  string `json:"verdict"`
+			ExitCode int    `json:"exitCode"`
+		} `json:"summary"`
+	}
+	if err := json.Unmarshal(stdout.Bytes(), &doc); err != nil {
+		t.Fatalf("stdout not JSON: %v\n%s", err, stdout.String())
+	}
+	if doc.Summary.Verdict != "FAIL" {
+		t.Errorf("summary.verdict = %q, want %q (stub returns empty agencies list)", doc.Summary.Verdict, "FAIL")
+	}
+	if doc.Summary.ExitCode != 0 {
+		t.Errorf("summary.exitCode = %d, want 0", doc.Summary.ExitCode)
+	}
+}
+
 func TestRunJSONConfigErrorRedactsInlineAPIKey(t *testing.T) {
 	t.Setenv("ONEBUSAWAY_API_KEY", "")
 	var stdout, stderr bytes.Buffer
@@ -235,9 +267,10 @@ func TestRunSinkErrorDoesNotAlterExitCode(t *testing.T) {
 	var stdout, stderr bytes.Buffer
 	code := run([]string{"oba-validator", "--json", "--no-cache", cfg}, &stdout, &stderr)
 
-	// Sink failure must not change the validator exit code (0 or 1, never 2).
-	if code == 2 {
-		t.Errorf("sink failure should not produce exit code 2, got %d", code)
+	// Sink failure must not change the validator exit code (always 0 on a
+	// completed run; 2 is reserved for config/usage errors).
+	if code != 0 {
+		t.Errorf("sink failure should not change exit code from 0, got %d", code)
 	}
 	if !strings.Contains(stderr.String(), "result sink write failed") {
 		t.Errorf("stderr should log the sink failure: %s", stderr.String())

report/document.go

@@ -42,7 +42,11 @@ type MetaSource struct {
 
 // Summary is the run-wide verdict and tallies.
 type Summary struct {
-	Verdict  string `json:"verdict"` // PASS | FAIL
+	Verdict string `json:"verdict"` // PASS | FAIL
+	// ExitCode mirrors validator.Report.ExitCode() and is always 0 for any
+	// completed run. The PASS/FAIL verdict lives in Verdict above; the process
+	// exit code is reserved for "the validator could not run" (exit 2 on
+	// config error). Retained for schema stability — see CLAUDE.md.
 	ExitCode int    `json:"exitCode"`
 	Total    int    `json:"total"`
 	Counts   Counts `json:"counts"`

report/document_test.go

@@ -105,8 +105,8 @@ func TestBuildDocument_CountsVerdictExit(t *testing.T) {
 	if doc.Summary.Total != 4 {
 		t.Errorf("total=%d want 4", doc.Summary.Total)
 	}
-	if doc.Summary.Verdict != "FAIL" || doc.Summary.ExitCode != 1 {
-		t.Errorf("verdict/exit = %q/%d want FAIL/1", doc.Summary.Verdict, doc.Summary.ExitCode)
+	if doc.Summary.Verdict != "FAIL" || doc.Summary.ExitCode != 0 {
+		t.Errorf("verdict/exit = %q/%d want FAIL/0", doc.Summary.Verdict, doc.Summary.ExitCode)
 	}
 	if doc.SchemaVersion != SchemaVersion {
 		t.Errorf("schemaVersion=%q", doc.SchemaVersion)

schema/oba-validator-report.schema.json

@@ -81,7 +81,11 @@
       "required": ["verdict", "exitCode", "total", "counts"],
       "properties": {
         "verdict": { "type": "string", "enum": ["PASS", "FAIL"] },
-        "exitCode": { "type": "integer", "enum": [0, 1] },
+        "exitCode": {
+          "type": "integer",
+          "const": 0,
+          "description": "Always 0 for a completed run. The PASS/FAIL verdict is conveyed by 'verdict' above, not by this field; the process exit code is reserved for 'the validator could not run' (exit 2 on config error). Field is retained for schema stability."
+        },
         "total": { "type": "integer", "minimum": 0 },
         "counts": { "$ref": "#/$defs/counts" }
       }

validator/result.go

@@ -31,10 +31,12 @@ func (r Report) Worst() Status {
 	return worst
 }
 
-// ExitCode is 1 if any result failed, else 0.
+// ExitCode is always 0 once a report has been produced. A FAIL verdict is
+// reported via Worst() and the JSON document's summary.verdict; the process
+// exit code is reserved for "the validator could not run" (config error in
+// main.go returns 2). This keeps the Render cron status green when the
+// validator successfully evaluated the OBA server, even if checks failed —
+// callers read the verdict from the result-sink row.
 func (r Report) ExitCode() int {
-	if r.Worst() == Fail {
-		return 1
-	}
 	return 0
 }

validator/result_test.go

@@ -2,17 +2,16 @@ package validator
 
 import "testing"
 
-func TestReportWorstAndExitCode(t *testing.T) {
+func TestReportWorst(t *testing.T) {
 	cases := []struct {
 		name     string
 		statuses []Status
 		worst    Status
-		exit     int
 	}{
-		{"all pass", []Status{Pass, Pass}, Pass, 0},
-		{"warn only", []Status{Pass, Warn, Skip}, Warn, 0},
-		{"any fail", []Status{Pass, Warn, Fail}, Fail, 1},
-		{"empty", nil, Pass, 0},
+		{"all pass", []Status{Pass, Pass}, Pass},
+		{"warn only", []Status{Pass, Warn, Skip}, Warn},
+		{"any fail", []Status{Pass, Warn, Fail}, Fail},
+		{"empty", nil, Pass},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -23,9 +22,18 @@ func TestReportWorstAndExitCode(t *testing.T) {
 			if got := r.Worst(); got != c.worst {
 				t.Errorf("Worst()=%v want %v", got, c.worst)
 			}
-			if got := r.ExitCode(); got != c.exit {
-				t.Errorf("ExitCode()=%d want %d", got, c.exit)
-			}
 		})
 	}
 }
+
+// ExitCode is intentionally constant: a completed run always returns 0,
+// including when checks failed. The FAIL verdict is conveyed via Worst() and
+// the JSON summary.verdict, not the process exit code.
+func TestReportExitCodeAlwaysZero(t *testing.T) {
+	for _, s := range []Status{Pass, Warn, Skip, Fail} {
+		r := Report{Results: []Result{{Status: s}}}
+		if got := r.ExitCode(); got != 0 {
+			t.Errorf("ExitCode() with %v result = %d, want 0", s, got)
+		}
+	}
+}