Replace panics with returned errors in llm.RegisterProvider

RegisterProvider now returns an error for an empty name, a nil factory,
or a duplicate registration instead of panicking, making registration
composable and testable without recover(). RegisterAllProviders caches
the one-time registration error (sync.Once cannot return a value) and
returns it to every caller.

Updated all call sites: hexailsp, hexaicli and hexaiaction propagate the
error with context; test TestMains fail fast via panic. Added unit tests
covering empty name, nil factory, duplicate, success and idempotent
re-registration.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

8 files changed, 94 insertions, 19 deletions

diff --git a/internal/hexaiaction/cmdentry.go b/internal/hexaiaction/cmdentry.go
index d60f172..ee78307 100644
--- a/internal/hexaiaction/cmdentry.go
+++ b/internal/hexaiaction/cmdentry.go
@@ -25,7 +25,9 @@ type Options struct {
 // RunCommand is the CLI orchestrator used by cmd/hexai-tmux-action. It runs in tmux
 // split-pane mode by default, or child mode when -ui-child is set.
 func RunCommand(ctx context.Context, opts Options, stdin io.Reader, stdout, stderr io.Writer) error {
-	llm.RegisterAllProviders()
+	if err := llm.RegisterAllProviders(); err != nil {
+		return fmt.Errorf("failed to register LLM providers: %w", err)
+	}
 	if opts.UIChild {
 		return runChild(ctx, opts.Infile, opts.Outfile, stdout, stderr)
 	}
diff --git a/internal/hexaicli/run.go b/internal/hexaicli/run.go
index df37f82..94dcd12 100644
--- a/internal/hexaicli/run.go
+++ b/internal/hexaicli/run.go
@@ -92,7 +92,9 @@ func cliTemperatureFromEntry(cfg appconfig.App, provider string, entry appconfig
 // Run executes the Hexai CLI behavior given arguments and I/O streams.
 // It assumes flags have already been parsed by the caller.
 func Run(ctx context.Context, args []string, stdin io.Reader, stdout, stderr io.Writer) error {
-	llm.RegisterAllProviders()
+	if err := llm.RegisterAllProviders(); err != nil {
+		return fmt.Errorf("failed to register LLM providers: %w", err)
+	}
 	return NewRunner().Run(ctx, args, stdin, stdout, stderr)
 }
 
diff --git a/internal/hexailsp/run.go b/internal/hexailsp/run.go
index 242a013..25d1929 100644
--- a/internal/hexailsp/run.go
+++ b/internal/hexailsp/run.go
@@ -54,7 +54,9 @@ func Run(logPath string, stdin io.Reader, stdout io.Writer, stderr io.Writer) er
 
 // RunWithConfig is like Run but accepts an explicit config file path.
 func RunWithConfig(logPath string, configPath string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
-	llm.RegisterAllProviders()
+	if err := llm.RegisterAllProviders(); err != nil {
+		return fmt.Errorf("failed to register LLM providers: %w", err)
+	}
 	return runWithConfigDependencies(logPath, configPath, stdin, stdout, stderr, defaultRunDependencies())
 }
 
diff --git a/internal/hexailsp/run_test.go b/internal/hexailsp/run_test.go
index fa4d535..b061f17 100644
--- a/internal/hexailsp/run_test.go
+++ b/internal/hexailsp/run_test.go
@@ -18,7 +18,9 @@ import (
 // TestMain registers all built-in LLM providers before tests run, mirroring
 // the explicit registration done in production binaries via RunWithConfig.
 func TestMain(m *testing.M) {
-	llm.RegisterAllProviders()
+	if err := llm.RegisterAllProviders(); err != nil {
+		panic(err)
+	}
 	os.Exit(m.Run())
 }
 
diff --git a/internal/llm/provider.go b/internal/llm/provider.go
index f866c83..dbfa603 100644
--- a/internal/llm/provider.go
+++ b/internal/llm/provider.go
@@ -3,6 +3,7 @@ package llm
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -116,37 +117,57 @@ var (
 	providerRegistryMu    sync.RWMutex
 	providerRegistry      = map[string]ProviderFactory{}
 	registerProvidersOnce sync.Once
+	// registerProvidersErr caches the outcome of the one-time built-in
+	// registration so repeated RegisterAllProviders calls return it too.
+	registerProvidersErr error
 )
 
-// RegisterProvider registers a provider factory by normalized name.
-// Panics on empty name, nil factory, or duplicate registration.
-func RegisterProvider(name string, factory ProviderFactory) {
+// RegisterProvider registers a provider factory by normalized name. It returns
+// an error on an empty name, a nil factory, or a duplicate registration instead
+// of panicking, so callers can decide how to handle misconfiguration. Returning
+// an error keeps registration composable and testable without recover().
+func RegisterProvider(name string, factory ProviderFactory) error {
 	normalized := normalizeProvider(name)
 	if normalized == "" {
-		panic("llm: provider name cannot be empty")
+		return errors.New("llm: provider name cannot be empty")
 	}
 	if factory == nil {
-		panic("llm: provider factory cannot be nil")
+		return errors.New("llm: provider factory cannot be nil")
 	}
 	providerRegistryMu.Lock()
 	defer providerRegistryMu.Unlock()
 	if _, exists := providerRegistry[normalized]; exists {
-		panic("llm: provider already registered: " + normalized)
+		return fmt.Errorf("llm: provider already registered: %s", normalized)
 	}
 	providerRegistry[normalized] = factory
+	return nil
 }
 
 // RegisterAllProviders registers all built-in LLM providers (anthropic, openai,
 // openrouter, ollama, yousearch). It is safe to call from multiple entry points
-// because the actual registration runs only once via sync.Once.
-func RegisterAllProviders() {
+// because the actual registration runs only once via sync.Once. The error from
+// the one-time registration is cached so every caller observes the same result,
+// even though sync.Once only runs the closure once.
+func RegisterAllProviders() error {
 	registerProvidersOnce.Do(func() {
-		RegisterProvider("anthropic", anthropicProviderFactory)
-		RegisterProvider("openai", openAIProviderFactory)
-		RegisterProvider("openrouter", openRouterProviderFactory)
-		RegisterProvider("ollama", ollamaProviderFactory)
-		RegisterProvider("yousearch", youSearchProviderFactory)
+		builtins := []struct {
+			name    string
+			factory ProviderFactory
+		}{
+			{"anthropic", anthropicProviderFactory},
+			{"openai", openAIProviderFactory},
+			{"openrouter", openRouterProviderFactory},
+			{"ollama", ollamaProviderFactory},
+			{"yousearch", youSearchProviderFactory},
+		}
+		for _, b := range builtins {
+			if err := RegisterProvider(b.name, b.factory); err != nil {
+				registerProvidersErr = err
+				return
+			}
+		}
 	})
+	return registerProvidersErr
 }
 
 // NewFromConfig creates an LLM client using only the supplied configuration.
diff --git a/internal/llm/provider_more_test.go b/internal/llm/provider_more_test.go
index 18cd49a..407c684 100644
--- a/internal/llm/provider_more_test.go
+++ b/internal/llm/provider_more_test.go
@@ -13,6 +13,48 @@ func TestWithOptions_Apply(t *testing.T) {
 	}
 }
 
+func TestRegisterProvider_EmptyName(t *testing.T) {
+	// An empty (or whitespace-only) name must be rejected with an error
+	// instead of panicking, and must not mutate the registry.
+	if err := RegisterProvider("   ", func(Config, ProviderKeys) (Client, error) { return nil, nil }); err == nil {
+		t.Fatalf("expected error for empty provider name, got nil")
+	}
+}
+
+func TestRegisterProvider_NilFactory(t *testing.T) {
+	// A nil factory is a programming error and must surface as an error.
+	if err := RegisterProvider("with-nil-factory", nil); err == nil {
+		t.Fatalf("expected error for nil factory, got nil")
+	}
+}
+
+func TestRegisterProvider_Duplicate(t *testing.T) {
+	// "openai" is registered by RegisterAllProviders in TestMain, so a second
+	// registration under the same normalized name must return an error.
+	if err := RegisterProvider("OpenAI", func(Config, ProviderKeys) (Client, error) { return nil, nil }); err == nil {
+		t.Fatalf("expected error for duplicate provider, got nil")
+	}
+}
+
+func TestRegisterProvider_Success(t *testing.T) {
+	// A fresh, valid registration succeeds and is then resolvable.
+	name := "test-register-success"
+	if err := RegisterProvider(name, func(Config, ProviderKeys) (Client, error) { return nil, nil }); err != nil {
+		t.Fatalf("unexpected error registering provider: %v", err)
+	}
+	if _, ok := lookupProviderFactory(name); !ok {
+		t.Fatalf("provider %q not found after successful registration", name)
+	}
+}
+
+func TestRegisterAllProviders_Idempotent(t *testing.T) {
+	// RegisterAllProviders ran in TestMain; calling it again must return the
+	// same cached (nil) error without re-registering and tripping a duplicate.
+	if err := RegisterAllProviders(); err != nil {
+		t.Fatalf("RegisterAllProviders returned error on repeat call: %v", err)
+	}
+}
+
 func TestNewFromConfig_Success_OpenAI(t *testing.T) {
 	// OpenAI success
 	oc := Config{Provider: "openai", OpenAIBaseURL: "http://x", OpenAIModel: "gpt"}
diff --git a/internal/llm/test_helpers_test.go b/internal/llm/test_helpers_test.go
index b6553bf..0280af2 100644
--- a/internal/llm/test_helpers_test.go
+++ b/internal/llm/test_helpers_test.go
@@ -8,7 +8,9 @@ import (
 // TestMain registers all built-in providers before any test runs, mirroring
 // the explicit registration that happens in production binaries.
 func TestMain(m *testing.M) {
-	RegisterAllProviders()
+	if err := RegisterAllProviders(); err != nil {
+		panic(err)
+	}
 	os.Exit(m.Run())
 }
 
diff --git a/internal/llmutils/client_test.go b/internal/llmutils/client_test.go
index 3e302d7..f1a4758 100644
--- a/internal/llmutils/client_test.go
+++ b/internal/llmutils/client_test.go
@@ -10,7 +10,9 @@ import (
 
 // TestMain registers all built-in LLM providers before tests run.
 func TestMain(m *testing.M) {
-	llm.RegisterAllProviders()
+	if err := llm.RegisterAllProviders(); err != nil {
+		panic(err)
+	}
 	os.Exit(m.Run())
 }