feat(models): ModelClient registry, mock, OpenAI-compatible client

leo-aa88 · leo-aa88 · commit 2e4d308c4779 · 2026-04-11T18:28:19.000-03:00
Add GenerateRequest/Response, ModelClient, Registry from Project providers, ResolveAPIKeyFrom (env:VAR), MockClient for deterministic tests, and OpenAIClient posting to /v1/chat/completions. Tests cover mock JSON agent output, unknown provider namespace, httptest OpenAI path, and API key resolution (issue #17). Made-with: Cursor
diff --git a/internal/models/doc.go b/internal/models/doc.go
@@ -1,2 +1,5 @@
 // Package models abstracts model providers and client interfaces.
+//
+// [Registry] resolves namespace/model_id strings using Project.spec.providers.models.
+// Use [MockClient] for deterministic tests; [OpenAIClient] is the MVP OpenAI-compatible backend (§12.2 F).
 package models
diff --git a/internal/models/key.go b/internal/models/key.go
@@ -0,0 +1,27 @@
+package models
+
+import (
+	"fmt"
+	"os"
+	"strings"
+)
+
+// ResolveAPIKeyFrom parses apiKeyFrom values from project YAML (§7.1), e.g. "env:OPENAI_API_KEY".
+func ResolveAPIKeyFrom(spec string) (string, error) {
+	spec = strings.TrimSpace(spec)
+	if spec == "" {
+		return "", fmt.Errorf("models: empty apiKeyFrom")
+	}
+	if strings.HasPrefix(spec, "env:") {
+		name := strings.TrimSpace(strings.TrimPrefix(spec, "env:"))
+		if name == "" {
+			return "", fmt.Errorf("models: apiKeyFrom env: requires a variable name")
+		}
+		v := os.Getenv(name)
+		if v == "" {
+			return "", fmt.Errorf("models: environment variable %q is not set", name)
+		}
+		return v, nil
+	}
+	return "", fmt.Errorf("models: unsupported apiKeyFrom %q (MVP: env:VAR only)", spec)
+}
diff --git a/internal/models/mock.go b/internal/models/mock.go
@@ -0,0 +1,21 @@
+package models
+
+import "context"
+
+// MockClient returns a fixed response for tests and offline agent steps (design doc §12.2 F MVP).
+type MockClient struct {
+	// Content is returned as GenerateResponse.Content verbatim (often JSON for structured output tests).
+	Content string
+	Meta    *GenerateMeta
+}
+
+// Generate returns deterministic output without calling the network.
+func (m *MockClient) Generate(ctx context.Context, req GenerateRequest) (GenerateResponse, error) {
+	_ = ctx
+	_ = req
+	meta := GenerateMeta{DurationMs: 1, CostUSD: 0.001}
+	if m.Meta != nil {
+		meta = *m.Meta
+	}
+	return GenerateResponse{Content: m.Content, Meta: meta}, nil
+}
diff --git a/internal/models/models_test.go b/internal/models/models_test.go
@@ -0,0 +1,150 @@
+package models
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
+)
+
+// Agent-step style: mock returns JSON that unmarshals into a fixed schema (issue #17 acceptance).
+func TestMockClient_usableForAgentStructuredOutput(t *testing.T) {
+	ctx := context.Background()
+	cli := &MockClient{
+		Content: `{"summary":"done","findings":[{"id":"f1"}]}`,
+		Meta:    &GenerateMeta{DurationMs: 42, CostUSD: 0.02},
+	}
+	resp, err := cli.Generate(ctx, GenerateRequest{
+		Model:    "mock/test",
+		Messages: []ChatMessage{{Role: "user", Content: "run"}},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	var decoded struct {
+		Summary  string `json:"summary"`
+		Findings []struct {
+			ID string `json:"id"`
+		} `json:"findings"`
+	}
+	if err := json.Unmarshal([]byte(resp.Content), &decoded); err != nil {
+		t.Fatalf("decode mock output: %v", err)
+	}
+	if decoded.Summary != "done" || len(decoded.Findings) != 1 || decoded.Findings[0].ID != "f1" {
+		t.Fatalf("got %+v", decoded)
+	}
+	if resp.Meta.DurationMs != 42 || resp.Meta.CostUSD != 0.02 {
+		t.Fatalf("meta %+v", resp.Meta)
+	}
+}
+
+func TestRegistry_unknownProviderNamespace(t *testing.T) {
+	g := &spec.ProjectGraph{
+		Spec: spec.ProjectSpec{
+			Providers: &spec.ProjectProviders{
+				Models: map[string]spec.ModelProviderConfig{
+					"openai": {Type: "openai", APIKeyFrom: "env:OPENAI_API_KEY"},
+				},
+			},
+		},
+	}
+	reg := NewRegistry(g)
+	_, _, err := reg.ClientFor("anthropic/claude-3")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if !strings.Contains(err.Error(), "unknown provider namespace") || !strings.Contains(err.Error(), "anthropic") {
+		t.Fatalf("got %v", err)
+	}
+}
+
+func TestRegistry_modelRefFormat(t *testing.T) {
+	g := &spec.ProjectGraph{
+		Spec: spec.ProjectSpec{
+			Providers: &spec.ProjectProviders{
+				Models: map[string]spec.ModelProviderConfig{
+					"openai": {Type: "openai", APIKeyFrom: "env:OPENAI_API_KEY"},
+				},
+			},
+		},
+	}
+	reg := NewRegistry(g)
+	_, _, err := reg.ClientFor("badref")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if !strings.Contains(err.Error(), "namespace/model_id") {
+		t.Fatalf("got %v", err)
+	}
+}
+
+func TestRegistry_resolvesOpenAIAndModelID(t *testing.T) {
+	t.Setenv("OPENAI_API_KEY", "sk-test")
+	g := &spec.ProjectGraph{
+		Spec: spec.ProjectSpec{
+			Providers: &spec.ProjectProviders{
+				Models: map[string]spec.ModelProviderConfig{
+					"openai": {Type: "openai", APIKeyFrom: "env:OPENAI_API_KEY"},
+				},
+			},
+		},
+	}
+	reg := NewRegistry(g)
+	cli, id, err := reg.ClientFor("openai/gpt-4.1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if id != "gpt-4.1" {
+		t.Fatalf("model id %q", id)
+	}
+	if _, ok := cli.(*OpenAIClient); !ok {
+		t.Fatalf("want *OpenAIClient, got %T", cli)
+	}
+}
+
+func TestOpenAIClient_Generate_usesChatCompletions(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/v1/chat/completions" {
+			t.Errorf("path %s", r.URL.Path)
+			http.NotFound(w, r)
+			return
+		}
+		auth := r.Header.Get("Authorization")
+		if auth != "Bearer sk-mock" {
+			t.Errorf("Authorization %q", auth)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"choices":[{"message":{"content":"hello"}}]}`))
+	}))
+	defer srv.Close()
+
+	c := &OpenAIClient{APIKey: "sk-mock", BaseURL: srv.URL + "/v1", HTTPClient: srv.Client()}
+	resp, err := c.Generate(context.Background(), GenerateRequest{
+		Model: "gpt-4.1",
+		Messages: []ChatMessage{
+			{Role: "user", Content: "hi"},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.Content != "hello" {
+		t.Fatalf("content %q", resp.Content)
+	}
+}
+
+func TestResolveAPIKeyFrom_env(t *testing.T) {
+	t.Setenv("MY_KEY", "abc")
+	got, err := ResolveAPIKeyFrom("env:MY_KEY")
+	if err != nil || got != "abc" {
+		t.Fatalf("%q %v", got, err)
+	}
+	_, err = ResolveAPIKeyFrom("env:MISSING_XYZ_404")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+}
diff --git a/internal/models/openai.go b/internal/models/openai.go
@@ -0,0 +1,116 @@
+package models
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"strings"
+	"time"
+
+	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
+)
+
+const defaultOpenAIBase = "https://api.openai.com/v1"
+
+// OpenAIClient is a minimal OpenAI-compatible chat client (design doc §12.2 F MVP).
+type OpenAIClient struct {
+	APIKey     string
+	BaseURL    string
+	HTTPClient *http.Client
+}
+
+// NewOpenAIClientFromConfig builds a client using apiKeyFrom (e.g. env:OPENAI_API_KEY) from project YAML.
+func NewOpenAIClientFromConfig(cfg spec.ModelProviderConfig) (*OpenAIClient, error) {
+	key, err := ResolveAPIKeyFrom(cfg.APIKeyFrom)
+	if err != nil {
+		return nil, err
+	}
+	return &OpenAIClient{APIKey: key, BaseURL: defaultOpenAIBase, HTTPClient: http.DefaultClient}, nil
+}
+
+func (c *OpenAIClient) base() string {
+	if c != nil && strings.TrimSpace(c.BaseURL) != "" {
+		return strings.TrimRight(strings.TrimSpace(c.BaseURL), "/")
+	}
+	return defaultOpenAIBase
+}
+
+func (c *OpenAIClient) http() *http.Client {
+	if c != nil && c.HTTPClient != nil {
+		return c.HTTPClient
+	}
+	return http.DefaultClient
+}
+
+// Generate calls POST /v1/chat/completions on the configured base URL.
+func (c *OpenAIClient) Generate(ctx context.Context, req GenerateRequest) (GenerateResponse, error) {
+	if c == nil || c.APIKey == "" {
+		return GenerateResponse{}, fmt.Errorf("models: openai client not configured")
+	}
+	start := time.Now()
+
+	type msg struct {
+		Role    string `json:"role"`
+		Content string `json:"content"`
+	}
+	payload := struct {
+		Model    string `json:"model"`
+		Messages []msg  `json:"messages"`
+	}{
+		Model: req.Model,
+	}
+	for _, m := range req.Messages {
+		payload.Messages = append(payload.Messages, msg{Role: m.Role, Content: m.Content})
+	}
+	body, err := json.Marshal(payload)
+	if err != nil {
+		return GenerateResponse{}, err
+	}
+	httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, c.base()+"/chat/completions", bytes.NewReader(body))
+	if err != nil {
+		return GenerateResponse{}, err
+	}
+	httpReq.Header.Set("Content-Type", "application/json")
+	httpReq.Header.Set("Authorization", "Bearer "+c.APIKey)
+	resp, err := c.http().Do(httpReq)
+	if err != nil {
+		return GenerateResponse{}, err
+	}
+	defer resp.Body.Close()
+	b, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return GenerateResponse{}, err
+	}
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return GenerateResponse{}, fmt.Errorf("models: openai HTTP %d: %s", resp.StatusCode, truncateErrBody(b))
+	}
+	var out struct {
+		Choices []struct {
+			Message struct {
+				Content string `json:"content"`
+			} `json:"message"`
+		} `json:"choices"`
+	}
+	if err := json.Unmarshal(b, &out); err != nil {
+		return GenerateResponse{}, fmt.Errorf("models: decode openai response: %w", err)
+	}
+	if len(out.Choices) == 0 {
+		return GenerateResponse{}, fmt.Errorf("models: openai returned no choices")
+	}
+	return GenerateResponse{
+		Content: out.Choices[0].Message.Content,
+		Meta:    GenerateMeta{DurationMs: time.Since(start).Milliseconds(), CostUSD: 0},
+	}, nil
+}
+
+func truncateErrBody(b []byte) string {
+	const n = 500
+	s := string(b)
+	if len(s) <= n {
+		return s
+	}
+	return s[:n] + "..."
+}
diff --git a/internal/models/registry.go b/internal/models/registry.go
@@ -0,0 +1,60 @@
+package models
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
+)
+
+// Registry resolves model references using Project.spec.providers.models (design doc §7.1, issue #17).
+type Registry struct {
+	models map[string]spec.ModelProviderConfig
+}
+
+// NewRegistry returns a registry from the merged project graph.
+func NewRegistry(g *spec.ProjectGraph) *Registry {
+	var m map[string]spec.ModelProviderConfig
+	if g != nil && g.Spec.Providers != nil && g.Spec.Providers.Models != nil {
+		m = g.Spec.Providers.Models
+	}
+	return &Registry{models: m}
+}
+
+// ClientFor resolves modelRef in the form "namespace/model_id" (e.g. "openai/gpt-4.1").
+// The returned modelID is the segment after the first slash and should be passed as GenerateRequest.Model.
+func (r *Registry) ClientFor(modelRef string) (client ModelClient, modelID string, err error) {
+	modelRef = strings.TrimSpace(modelRef)
+	if modelRef == "" {
+		return nil, "", fmt.Errorf("models: empty model reference")
+	}
+	i := strings.IndexByte(modelRef, '/')
+	if i <= 0 || i == len(modelRef)-1 {
+		return nil, "", fmt.Errorf("models: model %q must be namespace/model_id", modelRef)
+	}
+	ns := modelRef[:i]
+	id := modelRef[i+1:]
+	if r == nil || r.models == nil {
+		return nil, "", fmt.Errorf("models: unknown provider namespace %q", ns)
+	}
+	cfg, ok := r.models[ns]
+	if !ok {
+		return nil, "", fmt.Errorf("models: unknown provider namespace %q", ns)
+	}
+
+	switch strings.ToLower(strings.TrimSpace(cfg.Type)) {
+	case "openai":
+		cl, err := NewOpenAIClientFromConfig(cfg)
+		if err != nil {
+			return nil, "", err
+		}
+		return cl, id, nil
+	case "mock":
+		return &MockClient{
+			Content: `{"summary":"mock","findings":[]}`,
+			Meta:    &GenerateMeta{DurationMs: 1, CostUSD: 0},
+		}, id, nil
+	default:
+		return nil, "", fmt.Errorf("models: unsupported provider type %q for namespace %q", cfg.Type, ns)
+	}
+}
diff --git a/internal/models/types.go b/internal/models/types.go
