diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 0fb5df4..dc3ce41 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -222,6 +222,8 @@ Stage 2 is complete when the built-in transport sends unauthenticated requests for optional missing sources, still enforces explicit requirements, and all existing provider-response behavior remains green. +**Status:** Complete. + ## Stage 3: Align the Public Contract, Durable Documentation, and Full Validation ### Objective diff --git a/internal/llm/openai_compatible_client.go b/internal/llm/openai_compatible_client.go index b5da684..3247a09 100644 --- a/internal/llm/openai_compatible_client.go +++ b/internal/llm/openai_compatible_client.go @@ -133,13 +133,18 @@ func (c *OpenAICompatibleClient) Generate(ctx context.Context, req domain.Genera return nil, fmt.Errorf("%w: failed to create request: %v", ErrRequestFailed, err) } httpReq.Header.Set("Content-Type", "application/json") - if apiKey := strings.TrimSpace(req.Target.APIKey); apiKey != "" { - httpReq.Header.Set("Authorization", "Bearer "+apiKey) - } else if envName := strings.TrimSpace(req.Target.APIKeyEnv); envName != "" { - apiKey := strings.TrimSpace(os.Getenv(envName)) - if apiKey == "" { + apiKey := strings.TrimSpace(req.Target.APIKey) + envName := strings.TrimSpace(req.Target.APIKeyEnv) + if apiKey == "" && envName != "" { + apiKey = strings.TrimSpace(os.Getenv(envName)) + } + if apiKey == "" && req.Target.APIKeyRequired { + if envName != "" { return nil, fmt.Errorf("%w: api key environment variable %q is not set", ErrInvalidRequest, envName) } + return nil, fmt.Errorf("%w: api key is required", ErrInvalidRequest) + } + if apiKey != "" { httpReq.Header.Set("Authorization", "Bearer "+apiKey) } diff --git a/internal/llm/openai_compatible_client_test.go b/internal/llm/openai_compatible_client_test.go index 486fa37..0877e01 100644 --- a/internal/llm/openai_compatible_client_test.go +++ b/internal/llm/openai_compatible_client_test.go @@ -501,36 +501,61 @@ func checkCompleteRequestAndResponseMapping(t *testing.T) { func TestOpenAICompatibleClientAuthentication(t *testing.T) { tests := []struct { - name string - configureEnv func(*testing.T) - target domain.ExecutionTarget - wantAuth string - wantErr error - wantCallCount int + name string + configureEnv func(*testing.T) + target domain.ExecutionTarget + wantAuthorization string + wantError error + wantCallCount int }{ { name: "direct key takes precedence over environment", configureEnv: func(t *testing.T) { - t.Setenv("PROMPTKIT_TEST_API_KEY", "env-key") + t.Setenv("PROMPTKIT_TEST_API_KEY", " env-key ") }, target: domain.ExecutionTarget{ APIKeyEnv: "PROMPTKIT_TEST_API_KEY", - APIKey: "direct-llm-key", + APIKey: " direct-llm-key ", }, - wantAuth: "Bearer direct-llm-key", - wantCallCount: 1, + wantAuthorization: "Bearer direct-llm-key", + wantCallCount: 1, + }, + { + name: "environment key supplies authorization", + configureEnv: func(t *testing.T) { + t.Setenv("PROMPTKIT_TEST_API_KEY", " env-key ") + }, + target: domain.ExecutionTarget{APIKeyEnv: "PROMPTKIT_TEST_API_KEY"}, + wantAuthorization: "Bearer env-key", + wantCallCount: 1, }, { name: "no key omits authorization", wantCallCount: 1, }, { - name: "missing environment key fails before transport", + name: "optional missing environment omits authorization", configureEnv: func(t *testing.T) { t.Setenv("PROMPTKIT_MISSING_KEY", "") }, - target: domain.ExecutionTarget{APIKeyEnv: "PROMPTKIT_MISSING_KEY"}, - wantErr: ErrInvalidRequest, + target: domain.ExecutionTarget{APIKeyEnv: "PROMPTKIT_MISSING_KEY"}, + wantCallCount: 1, + }, + { + name: "required missing environment fails before transport", + configureEnv: func(t *testing.T) { + t.Setenv("PROMPTKIT_MISSING_KEY", "") + }, + target: domain.ExecutionTarget{ + APIKeyEnv: "PROMPTKIT_MISSING_KEY", + APIKeyRequired: true, + }, + wantError: ErrInvalidRequest, + }, + { + name: "required target without source fails before transport", + target: domain.ExecutionTarget{APIKeyRequired: true}, + wantError: ErrInvalidRequest, }, } @@ -545,9 +570,9 @@ func TestOpenAICompatibleClientAuthentication(t *testing.T) { request.Target = tc.target _, err := client.Generate(context.Background(), request) - if tc.wantErr != nil { - if !errors.Is(err, tc.wantErr) { - t.Fatalf("error = %v, want %v", err, tc.wantErr) + if tc.wantError != nil { + if !errors.Is(err, tc.wantError) { + t.Fatalf("error = %v, want %v", err, tc.wantError) } } else if err != nil { t.Fatalf("generate: %v", err) @@ -556,8 +581,13 @@ func TestOpenAICompatibleClientAuthentication(t *testing.T) { t.Fatalf("provider calls = %d, want %d", got, tc.wantCallCount) } if tc.wantCallCount == 1 { - if got := provider.lastRequest(t).header.Get("Authorization"); got != tc.wantAuth { - t.Fatalf("Authorization = %q, want %q", got, tc.wantAuth) + values := provider.lastRequest(t).header.Values("Authorization") + if tc.wantAuthorization == "" { + if len(values) != 0 { + t.Fatalf("Authorization values = %q, want absent", values) + } + } else if len(values) != 1 || values[0] != tc.wantAuthorization { + t.Fatalf("Authorization values = %q, want [%q]", values, tc.wantAuthorization) } } })