Improve error handling for local generation with stderr inclusion

README.md

@@ -219,6 +219,7 @@ provider implementation entirely (via a Go build constraint), so selecting
 `apfel`/`--apfel` on Linux or Windows fails immediately with a clear error
 instead of trying (and failing) to exec a nonexistent `apfel` binary.
 
+- Failed commands also include `apfel`'s stderr output in the error message.
 - Does not support `--list-models` (it has no concept of a model catalog
   — it just talks to whatever on-device model `apfel` provides); using
   `--list-models --apfel`/`--list-models --provider apfel` is an error.
@@ -237,6 +238,7 @@ error.
 
 - `--model` selects the `fm` model when supplied; `fm` currently provides
   the `system` model.
+- Failed commands also include `fm`'s stderr output in the error message.
 - Does not support `--list-models`; using
   `--list-models --fm`/`--list-models --provider fm` is an error.
 

go.mod

@@ -1,3 +1,3 @@
 module github.com/evilmarty/lazycommit
 
-go 1.27.0
+go 1.27.3

provider/apfel.go

@@ -9,7 +9,6 @@ package provider
 
 import (
 	"context"
-	"fmt"
 	"strconv"
 	"strings"
 
@@ -51,7 +50,7 @@ func (p *ApfelProvider) Generate(_ context.Context, prompt string) (string, erro
 
 	out, err := p.runner()("apfel", args)
 	if err != nil {
-		return "", fmt.Errorf("local generation with apfel failed: %w", err)
+		return "", localGenerationError("apfel", err)
 	}
 
 	return strings.TrimSpace(string(out)), nil

provider/apfel_test.go

@@ -5,6 +5,8 @@ package provider
 import (
 	"context"
 	"errors"
+	"fmt"
+	"os/exec"
 	"strings"
 	"testing"
 )
@@ -56,13 +58,57 @@ func TestApfelProviderDefaultMaxTokens(t *testing.T) {
 }
 
 func TestApfelProviderRunnerError(t *testing.T) {
+	runErr := errors.New("apfel not found")
 	p := &ApfelProvider{
 		Runner: func(name string, args []string) ([]byte, error) {
-			return nil, errors.New("apfel not found")
+			return nil, runErr
 		},
 	}
-	if _, err := p.Generate(context.Background(), "p"); err == nil {
-		t.Fatal("expected error")
+	if _, err := p.Generate(context.Background(), "p"); !errors.Is(err, runErr) {
+		t.Fatalf("expected wrapped runner error, got %v", err)
+	}
+}
+
+func TestApfelProviderExitError(t *testing.T) {
+	for _, tt := range []struct {
+		name    string
+		stderr  string
+		wrapped bool
+	}{
+		{name: "stderr", stderr: "  Model unavailable\n"},
+		{name: "wrapped stderr", stderr: "Model unavailable", wrapped: true},
+		{name: "empty stderr"},
+		{name: "whitespace stderr", stderr: " \n"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			_, runErr := exec.Command("sh", "-c", `printf '%s' "$1" >&2; exit 1`,
+				"sh", tt.stderr).Output()
+			var exitErr *exec.ExitError
+			if !errors.As(runErr, &exitErr) {
+				t.Fatalf("expected exit error, got %v", runErr)
+			}
+			if tt.wrapped {
+				runErr = fmt.Errorf("command failed: %w", runErr)
+			}
+			p := &ApfelProvider{
+				Runner: func(string, []string) ([]byte, error) {
+					return []byte("partial output"), runErr
+				},
+			}
+
+			got, err := p.Generate(context.Background(), "p")
+			if got != "" || !errors.Is(err, runErr) {
+				t.Fatalf("Generate() = %q, %v; want empty output and wrapped runner error", got, err)
+			}
+			want := "local generation with apfel failed"
+			if stderr := strings.TrimSpace(tt.stderr); stderr != "" {
+				want += ": " + stderr
+			}
+			want += ": " + runErr.Error()
+			if err.Error() != want {
+				t.Errorf("error = %q, want %q", err.Error(), want)
+			}
+		})
 	}
 }
 

provider/fm.go

@@ -4,7 +4,6 @@ package provider
 
 import (
 	"context"
-	"fmt"
 	"strings"
 
 	"github.com/evilmarty/lazycommit/internal/cmdrunner"
@@ -36,7 +35,7 @@ func (p *FoundationModelProvider) Generate(_ context.Context, prompt string) (st
 
 	out, err := p.runner()("fm", args)
 	if err != nil {
-		return "", fmt.Errorf("local generation with fm failed: %w", err)
+		return "", localGenerationError("fm", err)
 	}
 
 	return strings.TrimSpace(string(out)), nil

provider/fm_test.go

@@ -5,7 +5,10 @@ package provider
 import (
 	"context"
 	"errors"
+	"fmt"
+	"os/exec"
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -54,14 +57,63 @@ func TestFoundationModelProviderGenerateWithoutModel(t *testing.T) {
 }
 
 func TestFoundationModelProviderRunnerError(t *testing.T) {
+	runErr := errors.New("fm unavailable")
 	p := &FoundationModelProvider{
 		Runner: func(name string, args []string) ([]byte, error) {
-			return nil, errors.New("fm unavailable")
+			return nil, runErr
 		},
 	}
 
-	if _, err := p.Generate(context.Background(), "p"); err == nil {
-		t.Fatal("expected error")
+	if _, err := p.Generate(context.Background(), "p"); !errors.Is(err, runErr) {
+		t.Fatalf("expected wrapped runner error, got %v", err)
+	}
+}
+
+func TestFoundationModelProviderExitStatus(t *testing.T) {
+	for _, tt := range []struct {
+		name    string
+		status  int
+		wrapped bool
+		stderr  string
+	}{
+		{name: "license required", status: 69},
+		{name: "wrapped license error", status: 69, wrapped: true},
+		{name: "license stderr", status: 69, stderr: "Please run sudo fm license"},
+		{name: "wrapped stderr", status: 69, wrapped: true, stderr: "Please run sudo fm license"},
+		{name: "other failure", status: 1},
+		{name: "other failure stderr", status: 1, stderr: "Model unavailable"},
+		{name: "whitespace stderr", status: 1, stderr: " \n"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			_, runErr := exec.Command("sh", "-c", `printf '%s' "$1" >&2; exit "$2"`,
+				"sh", tt.stderr, fmt.Sprint(tt.status)).Output()
+			var exitErr *exec.ExitError
+			if !errors.As(runErr, &exitErr) || exitErr.ExitCode() != tt.status {
+				t.Fatalf("expected exit status %d, got %v", tt.status, runErr)
+			}
+			if tt.wrapped {
+				runErr = fmt.Errorf("command failed: %w", runErr)
+			}
+			p := &FoundationModelProvider{
+				Runner: func(string, []string) ([]byte, error) {
+					return []byte("partial output"), runErr
+				},
+			}
+
+			got, err := p.Generate(context.Background(), "p")
+			if got != "" || !errors.Is(err, runErr) {
+				t.Fatalf("Generate() = %q, %v; want empty output and wrapped runner error", got, err)
+			}
+			if stderr := strings.TrimSpace(tt.stderr); stderr != "" {
+				if !strings.Contains(err.Error(), ": "+stderr+": ") {
+					t.Errorf("missing stderr output: %v", err)
+				}
+			} else {
+				if want := "local generation with fm failed: " + runErr.Error(); err.Error() != want {
+					t.Errorf("error = %q, want %q", err.Error(), want)
+				}
+			}
+		})
 	}
 }
 

provider/local_darwin.go

@@ -0,0 +1,21 @@
+//go:build darwin
+
+package provider
+
+import (
+	"errors"
+	"fmt"
+	"os/exec"
+	"strings"
+)
+
+func localGenerationError(name string, err error) error {
+	message := "local generation with " + name + " failed"
+	var exitErr *exec.ExitError
+	if errors.As(err, &exitErr) {
+		if stderr := strings.TrimSpace(string(exitErr.Stderr)); stderr != "" {
+			message += ": " + stderr
+		}
+	}
+	return fmt.Errorf("%s: %w", message, err)
+}