From 4d45c428f8b768c46becd27ab4a199520432cf1d Mon Sep 17 00:00:00 2001
From: PJ <ipriyanshujain@gmail.com>
Date: Thu, 13 Aug 2026 00:33:57 +0530
Subject: [PATCH] feat(testrun): report violations as a typed error under
 exit-on-violation

---
 internal/testrun/testrun.go      | 46 +++++++++++++++++++++++++-------
 internal/testrun/testrun_test.go | 30 +++++++++++++++++++++
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/internal/testrun/testrun.go b/internal/testrun/testrun.go
index ce6f188..86b753b 100644
--- a/internal/testrun/testrun.go
+++ b/internal/testrun/testrun.go
@@ -38,6 +38,10 @@ type Options struct {
 	// Arm labels the experiment cell this run belongs to and is recorded in
 	// meta.json so a directory of runs can be attributed to a cell.
 	Arm string
+	// ExitOnViolation stops the run at the first violation and reports the
+	// recorded violations as a ViolationsError, so a caller (CI) can tell
+	// "the run found the bug" from "the run finished clean".
+	ExitOnViolation bool
 	// Generator selects the action picker: "llm" or the default seeded picker.
 	Generator string
 
@@ -189,15 +193,16 @@ func Execute(ctx context.Context, options Options, stdout io.Writer) error {
 		fmt.Fprintf(stdout, "running for %s (seed=%d)\n", options.Duration, seed)
 	}
 	summary, err := runner.Run(ctx, runner.Options{
-		Duration:    options.Duration,
-		MaxSteps:    options.MaxSteps,
-		IdleTimeout: 1 * time.Second,
-		BundleID:    options.BundleID,
-		Driver:      activeDriver,
-		Verifier:    verifierInstance,
-		TraceWriter: traceWriter,
-		Logger:      newProgressLogger(stdout),
-		Generator:   options.Generator,
+		Duration:        options.Duration,
+		MaxSteps:        options.MaxSteps,
+		IdleTimeout:     1 * time.Second,
+		BundleID:        options.BundleID,
+		Driver:          activeDriver,
+		Verifier:        verifierInstance,
+		TraceWriter:     traceWriter,
+		Logger:          newProgressLogger(stdout),
+		Generator:       options.Generator,
+		StopOnViolation: options.ExitOnViolation,
 	})
 
 	terminateCtx, terminateCancel := context.WithTimeout(context.Background(), 5*time.Second)
@@ -210,9 +215,32 @@ func Execute(ctx context.Context, options Options, stdout io.Writer) error {
 
 	fmt.Fprintf(stdout, "\nelapsed: %s\n", summary.EndTime.Sub(summary.StartTime).Round(time.Millisecond))
 	runner.RenderSummary(stdout, summary, options.Platform)
+	return runOutcome(options, summary)
+}
+
+// runOutcome turns a finished run into the pipeline's result. Without
+// --exit-on-violation a run that found violations is still a successful run
+// (the summary reports them), which is the behaviour every existing caller
+// depends on.
+func runOutcome(options Options, summary runner.Summary) error {
+	if options.ExitOnViolation && len(summary.Violations) > 0 {
+		return ViolationsError{Count: len(summary.Violations)}
+	}
 	return nil
 }
 
+// ViolationsError reports a run that recorded violations under
+// --exit-on-violation. It is deliberately distinct from every other error the
+// pipeline returns: those mean the harness broke, this one means the run did
+// its job and found something.
+type ViolationsError struct {
+	Count int
+}
+
+func (e ViolationsError) Error() string {
+	return fmt.Sprintf("%d violation record(s)", e.Count)
+}
+
 // bundleInputs holds the pre-driver assembly: alias map, seed, esbuild defines,
 // and the resolved spec-API/goja-runtime paths the bundler consumes.
 type bundleInputs struct {
diff --git a/internal/testrun/testrun_test.go b/internal/testrun/testrun_test.go
index 2df82c4..d5658ad 100644
--- a/internal/testrun/testrun_test.go
+++ b/internal/testrun/testrun_test.go
@@ -1,12 +1,14 @@
 package testrun
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/priyanshujain/sanderling/internal/runner"
 	"github.com/priyanshujain/sanderling/internal/verifier"
 )
 
@@ -231,3 +233,31 @@ func TestBuildRunMeta_OmitsModelWhenSpecDeclaresNoLLMGenerator(t *testing.T) {
 		t.Errorf("model recorded without a spec-declared llm generator: %q", meta.Model)
 	}
 }
+
+// TestRunOutcome_ReportsViolationsOnlyUnderTheFlag pins the CI contract: the
+// typed error is what makes `sanderling test` exit 2, and it must appear only
+// when the caller asked for it. A run that finds violations without the flag
+// stays a successful run, which is what every existing invocation expects.
+func TestRunOutcome_ReportsViolationsOnlyUnderTheFlag(t *testing.T) {
+	violated := runner.Summary{
+		Steps:      7,
+		Violations: []runner.ViolationRecord{{StepIndex: 3, Properties: []string{"balanceMoves"}}},
+	}
+	clean := runner.Summary{Steps: 7}
+
+	if err := runOutcome(Options{}, violated); err != nil {
+		t.Errorf("without --exit-on-violation a violated run must succeed, got %v", err)
+	}
+	if err := runOutcome(Options{ExitOnViolation: true}, clean); err != nil {
+		t.Errorf("a clean run must succeed under --exit-on-violation, got %v", err)
+	}
+
+	err := runOutcome(Options{ExitOnViolation: true}, violated)
+	var violations ViolationsError
+	if !errors.As(err, &violations) {
+		t.Fatalf("expected a ViolationsError, got %v", err)
+	}
+	if violations.Count != 1 {
+		t.Errorf("count: got %d, want 1", violations.Count)
+	}
+}