refactor(cli): convert beads verdict os.Exit calls to typed exit-error (ag-x22j #beads-exit-as-typed-error)

cli/cmd/ao/beads.go

@@ -35,6 +35,27 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// beadsExitError carries a verdict exit code out through cobra's RunE so
+// Execute() can map it to os.Exit() WITHOUT calling os.Exit() mid-command —
+// which would skip deferred cleanup (temp files, lock release) and kill the
+// test binary, making these paths untestable.
+//
+// The `ao beads verify|lint|audit` commands use exit-code-as-verdict: exit 1
+// means "stale citations / flagged beads found", a normal diagnostic outcome,
+// not an internal failure. The verdict text already went to stdout, so the
+// error carries no message (Error() == ""); the returning RunE sets
+// cmd.SilenceErrors so cobra emits no spurious "Error:" line, while genuine
+// errors returned elsewhere in those commands still print normally. Mirrors
+// gateExitError (validate.go) and doctorExitError (doctor_surface.go).
+type beadsExitError struct {
+	code int
+}
+
+func (e *beadsExitError) Error() string { return "" }
+
+// ExitCode returns the process exit code this verdict maps to.
+func (e *beadsExitError) ExitCode() int { return e.code }
+
 // execBD is the single entry point for shelling out to bd. Tests override
 // this to avoid a hard dependency on the real binary. Production code calls
 // `bd` via PATH; if absent, the caller emits a graceful warning and returns.
@@ -204,7 +225,10 @@ func runBeadsVerify(cmd *cobra.Command, args []string) error {
 	}
 	emitVerifyHuman(os.Stdout, report, beadsVerifyVerbose)
 	if report.StaleCount > 0 {
-		os.Exit(1)
+		if cmd != nil {
+			cmd.SilenceErrors = true
+		}
+		return &beadsExitError{code: 1}
 	}
 	return nil
 }
@@ -639,7 +663,10 @@ func runBeadsLint(cmd *cobra.Command, args []string) error {
 		emitLintHuman(os.Stdout, report)
 	}
 	if report.StaleBeads > 0 {
-		os.Exit(1)
+		if cmd != nil {
+			cmd.SilenceErrors = true
+		}
+		return &beadsExitError{code: 1}
 	}
 	return nil
 }

cli/cmd/ao/beads_audit_cluster.go

@@ -210,7 +210,10 @@ func runBeadsAudit(cmd *cobra.Command, args []string) error {
 		emitAuditHuman(os.Stdout, report)
 	}
 	if beadsAuditStrict && auditFlaggedCount(report) > 0 {
-		os.Exit(1)
+		if cmd != nil {
+			cmd.SilenceErrors = true
+		}
+		return &beadsExitError{code: 1}
 	}
 	return nil
 }

cli/cmd/ao/beads_exit_test.go

@@ -0,0 +1,131 @@
+// Tests for the exit-code-as-verdict path of `ao beads verify|lint|audit`.
+//
+// These commands previously called os.Exit(1) mid-RunE to signal a stale /
+// flagged verdict, which skipped deferred cleanup and killed the test binary,
+// making the verdict path untestable. They now return a *beadsExitError that
+// Execute() maps to the process exit code. These tests pin that contract.
+
+// practices: [dora-metrics, lean-startup]
+package main
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+// staleBeadShow is a `bd show` body that cites a file which cannot exist from
+// any working directory, so verifyBead classifies it STALE deterministically.
+const staleBeadShow = `○ na-stale · Stale bead for verdict testing   [● P2 · OPEN]
+Owner: Test · Type: task
+
+DESCRIPTION
+This bead cites cli/cmd/ao/this-file-does-not-exist-zzz999.go which is missing,
+forcing a stale citation regardless of the test's working directory.
+`
+
+func TestBeadsExitError_Contract(t *testing.T) {
+	e := &beadsExitError{code: 1}
+	// Error() is intentionally empty: the verdict text already went to stdout
+	// and the command silences cobra's error print.
+	if e.Error() != "" {
+		t.Fatalf("Error() = %q, want empty", e.Error())
+	}
+	if e.ExitCode() != 1 {
+		t.Fatalf("ExitCode() = %d, want 1", e.ExitCode())
+	}
+	// It must be discoverable via errors.As, the way Execute() maps it.
+	var target *beadsExitError
+	if !errors.As(error(e), &target) {
+		t.Fatalf("errors.As failed to match *beadsExitError")
+	}
+}
+
+func TestRunBeadsVerify_ReturnsExitErrorOnStale(t *testing.T) {
+	origBD, origAvail := execBD, bdAvailable
+	defer func() { execBD, bdAvailable = origBD, origAvail }()
+
+	bdAvailable = func() bool { return true }
+	execBD = func(args ...string) ([]byte, error) {
+		return []byte(staleBeadShow), nil
+	}
+
+	cmd := &cobra.Command{}
+	err := runBeadsVerify(cmd, []string{"na-stale"})
+
+	var bxErr *beadsExitError
+	if !errors.As(err, &bxErr) {
+		t.Fatalf("runBeadsVerify on stale bead: got %v, want *beadsExitError", err)
+	}
+	if bxErr.ExitCode() != 1 {
+		t.Fatalf("verdict exit code = %d, want 1", bxErr.ExitCode())
+	}
+	// The verdict return must silence cobra so no spurious "Error:" prints.
+	if !cmd.SilenceErrors {
+		t.Fatalf("expected cmd.SilenceErrors=true on verdict return")
+	}
+}
+
+func TestRunBeadsVerify_NilCmdDoesNotPanicOnStale(t *testing.T) {
+	origBD, origAvail := execBD, bdAvailable
+	defer func() { execBD, bdAvailable = origBD, origAvail }()
+
+	bdAvailable = func() bool { return true }
+	execBD = func(args ...string) ([]byte, error) {
+		return []byte(staleBeadShow), nil
+	}
+
+	// Direct callers (and older tests) pass a nil cmd; the nil-guard must hold.
+	err := runBeadsVerify(nil, []string{"na-stale"})
+	var bxErr *beadsExitError
+	if !errors.As(err, &bxErr) {
+		t.Fatalf("runBeadsVerify(nil, ...) on stale: got %v, want *beadsExitError", err)
+	}
+}
+
+func TestRunBeadsLint_ReturnsExitErrorOnStale(t *testing.T) {
+	origBD, origAvail := execBD, bdAvailable
+	defer func() { execBD, bdAvailable = origBD, origAvail }()
+
+	bdAvailable = func() bool { return true }
+	execBD = func(args ...string) ([]byte, error) {
+		if len(args) > 0 && args[0] == "list" {
+			return []byte("○ na-stale · Stale bead   [● P2 · OPEN]\n"), nil
+		}
+		// `show` for the listed bead → a stale body.
+		return []byte(staleBeadShow), nil
+	}
+
+	cmd := &cobra.Command{}
+	err := runBeadsLint(cmd, []string{})
+
+	var bxErr *beadsExitError
+	if !errors.As(err, &bxErr) {
+		t.Fatalf("runBeadsLint with a stale bead: got %v, want *beadsExitError", err)
+	}
+	if bxErr.ExitCode() != 1 {
+		t.Fatalf("verdict exit code = %d, want 1", bxErr.ExitCode())
+	}
+	if !cmd.SilenceErrors {
+		t.Fatalf("expected cmd.SilenceErrors=true on verdict return")
+	}
+}
+
+func TestRunBeadsLint_CleanReturnsNil(t *testing.T) {
+	origBD, origAvail := execBD, bdAvailable
+	defer func() { execBD, bdAvailable = origBD, origAvail }()
+
+	bdAvailable = func() bool { return true }
+	execBD = func(args ...string) ([]byte, error) {
+		if len(args) > 0 && args[0] == "list" {
+			return []byte("○ na-clean · Clean bead   [● P2 · OPEN]\n"), nil
+		}
+		// A `show` body with no file citations → no stale citations.
+		return []byte("○ na-clean · Clean bead   [● P2 · OPEN]\nOwner: Test · Type: task\n\nDESCRIPTION\nNo citations here.\n"), nil
+	}
+
+	if err := runBeadsLint(&cobra.Command{}, []string{}); err != nil {
+		t.Fatalf("runBeadsLint with a clean bead: got %v, want nil", err)
+	}
+}

cli/cmd/ao/root.go

@@ -107,6 +107,14 @@ func Execute() {
 			}
 			os.Exit(gateErr.ExitCode())
 		}
+		var beadsErr *beadsExitError
+		if errors.As(err, &beadsErr) {
+			// The exit code IS the verdict in `ao beads verify|lint|audit`:
+			// 1 means stale/flagged beads found. The verdict already went to
+			// stdout and the command silenced cobra's error print, so there is
+			// nothing more to surface here — just map to the process exit code.
+			os.Exit(beadsErr.ExitCode())
+		}
 		printRequiredFlagHint(executedCmd, err)
 		os.Exit(1)
 	}