diff --git a/internal/llm/openai_compatible_client.go b/internal/llm/openai_compatible_client.go index 8530382..86152ed 100644 --- a/internal/llm/openai_compatible_client.go +++ b/internal/llm/openai_compatible_client.go @@ -55,10 +55,11 @@ func NewOpenAICompatibleClient(cfg OpenAICompatibleConfig) (*OpenAICompatibleCli var client *http.Client if cfg.HTTPClient != nil { - client = cfg.HTTPClient - if client.Timeout == 0 { - client.Timeout = timeout + cloned := *cfg.HTTPClient + if cloned.Timeout == 0 { + cloned.Timeout = timeout } + client = &cloned } else { client = &http.Client{Timeout: timeout} } diff --git a/internal/llm/openai_compatible_client_test.go b/internal/llm/openai_compatible_client_test.go index 2323d36..839cf76 100644 --- a/internal/llm/openai_compatible_client_test.go +++ b/internal/llm/openai_compatible_client_test.go @@ -14,6 +14,64 @@ import ( "gitea.maximumdirect.net/eric/scriptorium/internal/domain" ) +func TestNewOpenAICompatibleClientDoesNotMutateSuppliedZeroTimeoutClient(t *testing.T) { + transport := http.DefaultTransport + supplied := &http.Client{Transport: transport} + + client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ + HTTPClient: supplied, + }) + if err != nil { + t.Fatalf("unexpected constructor error: %v", err) + } + + if supplied.Timeout != 0 { + t.Fatalf("expected supplied client timeout to remain zero, got %v", supplied.Timeout) + } + if client.httpClient == supplied { + t.Fatal("expected constructed client to use a cloned HTTP client") + } + if client.httpClient.Timeout != client.timeout { + t.Fatalf("expected cloned client timeout %v, got %v", client.timeout, client.httpClient.Timeout) + } + if client.httpClient.Timeout <= 0 { + t.Fatalf("expected constructed client to use a positive default timeout, got %v", client.httpClient.Timeout) + } + if client.httpClient.Transport != transport { + t.Fatal("expected cloned client to preserve the supplied transport") + } +} + +func TestNewOpenAICompatibleClientDoesNotMutateSuppliedNonzeroTimeoutClient(t *testing.T) { + transport := http.DefaultTransport + suppliedTimeout := 37 * time.Second + supplied := &http.Client{ + Timeout: suppliedTimeout, + Transport: transport, + } + + client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{ + Timeout: 2 * time.Second, + HTTPClient: supplied, + }) + if err != nil { + t.Fatalf("unexpected constructor error: %v", err) + } + + if supplied.Timeout != suppliedTimeout { + t.Fatalf("expected supplied client timeout to remain %v, got %v", suppliedTimeout, supplied.Timeout) + } + if client.httpClient == supplied { + t.Fatal("expected constructed client to use a cloned HTTP client") + } + if client.httpClient.Timeout != suppliedTimeout { + t.Fatalf("expected cloned client timeout %v, got %v", suppliedTimeout, client.httpClient.Timeout) + } + if client.httpClient.Transport != transport { + t.Fatal("expected cloned client to preserve the supplied transport") + } +} + func TestOpenAICompatibleClientGenerateSuccess(t *testing.T) { type observedRequest struct { Authorization string