diff --git a/cli/cmd/ao/beads.go b/cli/cmd/ao/beads.go index ae8e07293..75b101d68 100644 --- a/cli/cmd/ao/beads.go +++ b/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 } diff --git a/cli/cmd/ao/beads_audit_cluster.go b/cli/cmd/ao/beads_audit_cluster.go index abcde5d6c..61fce0241 100644 --- a/cli/cmd/ao/beads_audit_cluster.go +++ b/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 } diff --git a/cli/cmd/ao/beads_exit_test.go b/cli/cmd/ao/beads_exit_test.go new file mode 100644 index 000000000..378c06032 --- /dev/null +++ b/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) + } +} diff --git a/cli/cmd/ao/root.go b/cli/cmd/ao/root.go index 263508775..d7795538f 100644 --- a/cli/cmd/ao/root.go +++ b/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) }